| Message ID | 20220822181853.23063-1-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 52c135d495ca019abbacf063f5ef43bd5b189070 |
| Headers | show |
| Series | spi: dw: Add deferred controller registration support | expand |
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 >
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
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.
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
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
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; }
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(-)