Re: [PATCH v2 1/2] platform/surface: aggregator: Defer probing when serdev is not ready

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx/T/#m40d73c44bf92ad45e4fca5ed5f01f9c11e30adb1





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux