RE: [PATCH] net: alacritech: Partially revert "net: alacritech: Switch to use dev_err_probe()"

From: Keller, Jacob E
Date: Fri Aug 30 2024 - 17:33:09 EST




> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Friday, August 30, 2024 1:34 PM
> To: Simon Horman <horms@xxxxxxxxxx>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>; Lino Sanfilippo
> <LinoSanfilippo@xxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Keller, Jacob E <jacob.e.keller@xxxxxxxxx>; Yang Ruibin
> <11162571@xxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] net: alacritech: Partially revert "net: alacritech: Switch to use
> dev_err_probe()"
>
> On Fri, Aug 30, 2024 at 07:28:44PM +0100, Simon Horman wrote:
> > On Fri, Aug 30, 2024 at 07:00:14PM +0200, Krzysztof Kozlowski wrote:
> > > This reverts commit bf4d87f884fe8a4b6b61fe4d0e05f293d08df61c because it
> > > introduced dev_err_probe() in non-probe path, which is not desired.
> > > Calling it after successful probe, dev_err_probe() will set deferred
> > > status on the device already probed. See also documentation of
> > > dev_err_probe().
> >
> > I agree that using dev_err_probe() outside of a probe path is
> > inappropriate. And I agree that your patch addresses that problem
> > in the context of changes made by the cited commit.
>
> Maybe device_set_deferred_probe_reason() could call device_is_bound()
> is check the device is not actually bound, and hence still in probe,
> and do a dev_warn(). That should help catch these errors.
>

That seems reasonable to me.

> I assume the developers submitting these patches are also using a
> bot. It would be good if the bot could be trained to follow the call
> paths and ensure it only reports cases which are probe.
>
> Andrew