From: Weifeng Liu <weifeng.liu.z@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: platform-driver-x86@vger.kernel.org,
linux-serial@vger.kernel.org,
Maximilian Luz <luzmaximilian@gmail.com>,
Hans de Goede <hdegoede@redhat.com>
Subject: Re: [PATCH v2 1/2] platform/surface: aggregator: Defer probing when serdev is not ready
Date: Sat, 04 May 2024 12:17:05 +0800 [thread overview]
Message-ID: <15e9c1278505677e813df48190a5cd9c200453c3.camel@gmail.com> (raw)
In-Reply-To: <ZjUY7xPXszBulKap@smile.fi.intel.com>
Hi Andy,
On Fri, 2024-05-03 at 20:03 +0300, Andy Shevchenko wrote:
> On Fri, May 03, 2024 at 11:08:46AM +0800, Weifeng Liu wrote:
> > This is an attempt to alleviate race conditions in the SAM driver where
> > essential resources like serial device and GPIO pins are not ready at
> > the time ssam_serial_hub_probe() is called. Instead of giving up
> > probing, a better way would be to defer the probing by returning
> > -EPROBE_DEFER, allowing the kernel try again later.
> >
> > However, there is no way of identifying all such cases from other real
> > errors in a few days. So let's take a gradual approach identify and
> > address these cases as they arise. This commit marks the initial step
> > in this process.
>
> ...
>
> > + /*
> > + * The following step can fail when it's called too early before the
> > + * underlying UART device is ready (in this case -ENXIO is returned).
> > + * Instead of simply giving up and losing everything, we can defer
> > + * the probing by returning -EPROBE_DEFER so that the kernel would be
> > + * able to retry later.
> > + */
>
> You can add the following to the
> serial_core.c (at the top after the headers)
>
> #undef ENXIO
> #define ENXIO __LINE__
>
> And I'm pretty much sure it will point out you to the uart_port_activate().
> If it's the case you may elaborate this in the comment.
> Otherwise you may add the same hack to other files and find the culprit.
Indeed, uart_port_activate() is the function where we gets errno -
ENXIO. Please see the cover letter [1] of this series:
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.
What confuses me is that when ssam_serial_hub_probe() (probe callback
of a serdev driver) gets called and tries to open the provided serdev
device by serdev_device_open(), it simply fails to open it... The
serdev device is not the kind of auxiliary devices but the main device
this driver is to manage. And I don't find other serdev driver doing
this sort of checking and returning -EPROBE_DEFER thing when opening
serdev device. Thus, from the perspective of a newcomer to this area,
I think this phenomenon is somewhat abnormal. Maybe somehow the
initializing of the UART device and probing of serdev driver are
interleaved...
Andy, do you have any idea what's going wrong here? Or do you think
this is an expected behavior?
>
> Also it might be that we add some error code substitution inside serdev core.
> At least there more data is available to make the (better) decision.
>
Agree. If failing in serdev_device_open() is common in a serdev
driver, we'd better handle it properly inside serdev core and convey
explicit semantics to the caller.
Best regards,
Weifeng
[1]
https://lore.kernel.org/linux-serial/20240503030900.1334763-1-weifeng.liu.z@gmail.com/T/#m40d73c44bf92ad45e4fca5ed5f01f9c11e30adb1
next prev parent reply other threads:[~2024-05-04 4:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 3:08 [PATCH v2 0/2] Defer probing of SAM if serdev device is not ready Weifeng Liu
2024-05-03 3:08 ` [PATCH v2 1/2] platform/surface: aggregator: Defer probing when serdev " Weifeng Liu
2024-05-03 17:03 ` Andy Shevchenko
2024-05-04 4:17 ` Weifeng Liu [this message]
2024-05-03 3:08 ` [PATCH v2 2/2] platform/surface: aggregator: Log critical errors during SAM probing Weifeng Liu
2024-05-03 16:36 ` Andy Shevchenko
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=15e9c1278505677e813df48190a5cd9c200453c3.camel@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).