Re: [PATCH] sc16is7xx: Defer probe if device read fails

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Mar 29, 2021 at 06:35:38PM +0000, Annaliese McDermond wrote:
> 
> 
> > On Mar 29, 2021, at 11:18 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > On Mon, Mar 29, 2021 at 03:40:35PM +0000, Annaliese McDermond wrote:
> >> A test was added to the probe function to ensure the device was
> >> actually connected and working before successfully completing a
> >> probe.  If the device was actually there, but the I2C bus was not
> >> ready yet for whatever reason, the probe fails permanently.
> >> 
> >> Change the probe so that we defer the probe on a regmap read
> >> failure so that we try the probe again when the dependent drivers
> >> are potentially loaded.  This should not affect the case where the
> >> device truly isn't present because the probe will never successfully
> >> complete.
> >> 
> >> Fixes: 2aa916e ("sc16is7xx: Read the LSR register for basic device presence check")
> > 
> > Please use the full 12 characters of the git commit id, as the
> > documentation asks for.  This should be:
> > 
> > Fixes: 2aa916e67db3 ("sc16is7xx: Read the LSR register for basic device presence check")
> 
> I’m sorry, I must have missed the section specifying that, and since I saw commits like
> 05962f95f9ac specifying 7 characters in their “fixes” line, I made the assumption that
> this was the correct length.
> 
> Would you like me to post a v2 patch with the hash changed?

Please do.

> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Annaliese McDermond <nh6z@nh6z.net>
> >> ---
> >> drivers/tty/serial/sc16is7xx.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> >> index f86ec2d2635b..9adb8362578c 100644
> >> --- a/drivers/tty/serial/sc16is7xx.c
> >> +++ b/drivers/tty/serial/sc16is7xx.c
> >> @@ -1196,7 +1196,7 @@ static int sc16is7xx_probe(struct device *dev,
> >> 	ret = regmap_read(regmap,
> >> 			  SC16IS7XX_LSR_REG << SC16IS7XX_REG_SHIFT, &val);
> >> 	if (ret < 0)
> >> -		return ret;
> >> +		return -EPROBE_DEFER;
> >> 
> >> 	/* Alloc port structure */
> >> 	s = devm_kzalloc(dev, struct_size(s, p, devtype->nr_uart), GFP_KERNEL);
> >> -- 
> >> 2.27.0
> > 
> > Any reason you did not cc: the tty maintainer with this change?
> 
> None other than I believed “serial drivers” to be more specific than “tty layer” when
> looking at the maintainers.  I’m happy to Cc Jiri if needed.

The tool, scripts/get_maintainer.pl is your friend, always run it on
your patch.

You also forgot to send it to me :)

thanks,

greg k-h