From: Weifeng Liu <weifeng.liu.z@gmail.com>
To: platform-driver-x86@vger.kernel.org, linux-serial@vger.kernel.org
Cc: Weifeng Liu <weifeng.liu.z@gmail.com>,
Andy Shevchenko <andriy.shevchenko@intel.com>,
Maximilian Luz <luzmaximilian@gmail.com>,
Hans de Goede <hdegoede@redhat.com>
Subject: [PATCH v3 0/2] Defer probing of SAM if serdev device is not ready
Date: Sun, 5 May 2024 21:07:48 +0800 [thread overview]
Message-ID: <20240505130800.2546640-1-weifeng.liu.z@gmail.com> (raw)
v3 changes:
* better formatting, nothing special
v2 changes:
* resolves Andy's comments
Original letter:
Greetings,
This series is intended to remedy a race condition where surface
aggregator module (SAM) which is a serdev driver could fail to probe if
the underlying UART port is not ready to open. In such circumstance,
invoking serdev_device_open() gets errno -ENXIO, leading to failure in
probing of SAM. However, if the probe is retried in a short delay,
serdev_device_open() would work as expected and everything just goes
fine. As a workaround, adding the serial driver (8250_dw) into
initramfs or building it into the kernel image significantly mitigates
the likelihood of encountering this race condition, as in this way the
serial device would be initialized much earlier than probing of SAM.
However, IMO we should reliably avoid this sort of race condition. A
good way is returning -EPROBE_DEFER when serdev_device_open returns
-ENXIO so that the kernel will be able to retry the probing later. This
is what the first patch tries to do.
Though this solution might be a good enough solution for this specific
issue, I am wondering why this kind of race condition could ever happen,
i.e., why a serdes device could be not ready yet at the moment the
serdev driver gets called and tries to bind it. And even if this is an
expected behavior how serdev driver works, I do feel it a little bit
weird that we need to identify serdev_device_open() returning -ENXIO as
non-fatal error and thus return -EPROBE_DEFER manually in such case, as
I don't see this sort of behavior in other serdev driver. Maximilian
and Hans suggested discussing the root cause of the race condition here.
I will be grateful if you could provide some reasoning and insights on
this.
Following is the code path when the issue occurs:
ssam_serial_hub_probe()
serdev_device_open()
ctrl->ops->open() /* this callback being ttyport_open() */
tty->ops->open() /* this callback being uart_open() */
tty_port_open()
port->ops->activate() /* this callback being uart_port_activate() */
Find bit UPF_DEAD is set in uport->flags and fail with errno -ENXIO.
I notice that flag UPF_DEAD would be set in serial_core_register_port()
during calling serial_core_add_one_port() but don't have much idea
what's going on under the hood.
Additionally, add logs to the probe procedure of SAM in the second
patch, hoping it could help provide context to user when something
miserable happens.
Context of this series is available in [1].
Best regards,
Weifeng
Weifeng Liu (2):
platform/surface: aggregator: Defer probing when serdev is not ready
platform/surface: aggregator: Log critical errors during SAM probing
drivers/platform/surface/aggregator/core.c | 53 ++++++++++++++++------
1 file changed, 39 insertions(+), 14 deletions(-)
--
2.44.0
next reply other threads:[~2024-05-05 13:08 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-05 13:07 Weifeng Liu [this message]
2024-05-05 13:07 ` [PATCH v3 1/2] platform/surface: aggregator: Defer probing when serdev is not ready Weifeng Liu
2024-05-06 9:06 ` Andy Shevchenko
2024-05-05 13:07 ` [PATCH v3 2/2] platform/surface: aggregator: Log critical errors during SAM probing Weifeng Liu
2024-05-13 11:55 ` Hans de Goede
2024-05-06 14:41 ` [PATCH v3 0/2] Defer probing of SAM if serdev device is not ready Hans de Goede
2024-05-06 14:52 ` Andy Shevchenko
2024-05-07 13:44 ` Weifeng Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240505130800.2546640-1-weifeng.liu.z@gmail.com \
--to=weifeng.liu.z@gmail.com \
--cc=andriy.shevchenko@intel.com \
--cc=hdegoede@redhat.com \
--cc=linux-serial@vger.kernel.org \
--cc=luzmaximilian@gmail.com \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).