Hi,
On 5/2/24 10:38 AM, Hans de Goede wrote:
Hi Weifeng,
On 5/2/24 6:02 AM, Weifeng Liu wrote:
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.
Ack, I have no objection against the changes and if Maximilian is ok with
it I can merge these right away as an interim fix, but I would really
like to know why the serdev core / tty code is registering a serdev
device for a serial port before it is ready to have serdev_device_open()
called on it. To me it seems that the root cause is in somewhere in
the 8250_dw code or the serdev core code.
I'm fine with this being merged as an interim fix once Andy's comments
have been addressed. Weifeng, in that case you can then also add
Reviewed-by: Maximilian Luz <luzmaximilian@xxxxxxxxx>
However, I too would like to see this fixed properly on the serdev side.
Resources sometimes not being ready is sometimes which drivers generally
speaking need to handle, but in this case the resource which is not
ready is the device the driver is binding to, so it seems that
the device is registered too soon.
If someone familiar with the serial / serdev code can provide some
insight here that would be great.
I will be away for the remainder of the month starting end of next week,
but I could try to look into this after that, provided no one else did by
then.
Best regards,
Max
