[PATCH] spi: dw: Add deferred controller registration support

[PATCH] spi: dw: Add deferred controller registration support

[Serge Semin]

It's pretty possible to have the spi_register_controller() method
returning -EPROBE_DEFER status in case, for instance, if the GPIOs used
for the CS implementation aren't ready to be requested due to the
corresponding platform devices still pending to be probed. Let's make sure
the DW SSI driver won't print error message in that case by calling the
dev_err_probe() function if the SPI-registration procedure exited with
a non-zero status.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/spi/spi-dw-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index f87d97ccd2d6..99edddf9958b 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -955,7 +955,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 
 	ret = spi_register_controller(master);
 	if (ret) {
-		dev_err(&master->dev, "problem registering spi master\n");
+		dev_err_probe(dev, ret, "problem registering spi master\n");
 		goto err_dma_exit;
 	}
 
-- 
2.35.1

Re: [PATCH] spi: dw: Add deferred controller registration support

[Andy Shevchenko]

On Mon, Aug 22, 2022 at 9:19 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> It's pretty possible to have the spi_register_controller() method
> returning -EPROBE_DEFER status in case, for instance, if the GPIOs used
> for the CS implementation aren't ready to be requested due to the
> corresponding platform devices still pending to be probed. Let's make sure
> the DW SSI driver won't print error message in that case by calling the
> dev_err_probe() function if the SPI-registration procedure exited with
> a non-zero status.

Shouldn't there be a Fixes tag?

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  drivers/spi/spi-dw-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index f87d97ccd2d6..99edddf9958b 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -955,7 +955,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>
>         ret = spi_register_controller(master);
>         if (ret) {
> -               dev_err(&master->dev, "problem registering spi master\n");
> +               dev_err_probe(dev, ret, "problem registering spi master\n");
>                 goto err_dma_exit;
>         }
>
> --
> 2.35.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] spi: dw: Add deferred controller registration support

[Serge Semin]

On Tue, Aug 23, 2022 at 12:38:45AM +0300, Andy Shevchenko wrote:
> On Mon, Aug 22, 2022 at 9:19 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > It's pretty possible to have the spi_register_controller() method
> > returning -EPROBE_DEFER status in case, for instance, if the GPIOs used
> > for the CS implementation aren't ready to be requested due to the
> > corresponding platform devices still pending to be probed. Let's make sure
> > the DW SSI driver won't print error message in that case by calling the
> > dev_err_probe() function if the SPI-registration procedure exited with
> > a non-zero status.
> 

> Shouldn't there be a Fixes tag?
> 

The dev_err-line has been there since the driver was born in commit
e24c74527207 ("spi: controller driver for Designware SPI core") ,
which had happened for several years before the deferred probe was
added d1c3414c2a9d ("drivercore: Add driver probe deferral
mechanism"). Moreover I didn't think it was that much critical to have
the patch backported since the discovered problem just caused a false
error message logged. So IMO the fixes tag wasn't that much required
in this case.

Anyway if Mark decides, that it still would be good to have the patch
backported, here is the fixes tag which could be added during the
patch merging in:
Fixes: e24c74527207 ("spi: controller driver for Designware SPI core")

> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thanks.

-Sergey

> 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> >  drivers/spi/spi-dw-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > index f87d97ccd2d6..99edddf9958b 100644
> > --- a/drivers/spi/spi-dw-core.c
> > +++ b/drivers/spi/spi-dw-core.c
> > @@ -955,7 +955,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
> >
> >         ret = spi_register_controller(master);
> >         if (ret) {
> > -               dev_err(&master->dev, "problem registering spi master\n");
> > +               dev_err_probe(dev, ret, "problem registering spi master\n");
> >                 goto err_dma_exit;
> >         }
> >
> > --
> > 2.35.1
> >
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] spi: dw: Add deferred controller registration support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 648 bytes --]

On Mon, Aug 22, 2022 at 09:18:53PM +0300, Serge Semin wrote:
> It's pretty possible to have the spi_register_controller() method
> returning -EPROBE_DEFER status in case, for instance, if the GPIOs used
> for the CS implementation aren't ready to be requested due to the
> corresponding platform devices still pending to be probed. Let's make sure
> the DW SSI driver won't print error message in that case by calling the
> dev_err_probe() function if the SPI-registration procedure exited with
> a non-zero status.

This isn't adding any new support, it's making the logging less verbose
- deferred probe worked perfectly fine without this patch.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] spi: dw: Add deferred controller registration support

[Mark Brown]

On Mon, 22 Aug 2022 21:18:53 +0300, Serge Semin wrote:
> It's pretty possible to have the spi_register_controller() method
> returning -EPROBE_DEFER status in case, for instance, if the GPIOs used
> for the CS implementation aren't ready to be requested due to the
> corresponding platform devices still pending to be probed. Let's make sure
> the DW SSI driver won't print error message in that case by calling the
> dev_err_probe() function if the SPI-registration procedure exited with
> a non-zero status.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: dw: Add deferred controller registration support
      commit: 52c135d495ca019abbacf063f5ef43bd5b189070

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH] spi: dw: Add deferred controller registration support

[Serge Semin]

On Tue, Aug 23, 2022 at 01:06:12PM +0100, Mark Brown wrote:
> On Mon, Aug 22, 2022 at 09:18:53PM +0300, Serge Semin wrote:
> > It's pretty possible to have the spi_register_controller() method
> > returning -EPROBE_DEFER status in case, for instance, if the GPIOs used
> > for the CS implementation aren't ready to be requested due to the
> > corresponding platform devices still pending to be probed. Let's make sure
> > the DW SSI driver won't print error message in that case by calling the
> > dev_err_probe() function if the SPI-registration procedure exited with
> > a non-zero status.
> 

> This isn't adding any new support, it's making the logging less verbose
> - deferred probe worked perfectly fine without this patch.

Thanks for the note. Indeed a more correct subject would be something
like "Prevent error msg printed on deferred probe". Anyway since
you've already merged the patch in, no v2 then.

-Sergey