| Message ID | 20230418083505.466198-1-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] spi: spi-imx: Use dev_err_probe for failed DMA channel requests | expand |
Hi, gentle ping. Alexander Am Dienstag, 18. April 2023, 10:35:05 CEST schrieb Alexander Stein: > If dma_request_chan() fails, no error is shown nor any information is > shown in /sys/kernel/debug/devices_deferred if -EPROBE_DEFER is returned. > Use dev_err_probe to fix both problems. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > With this patch applied /sys/kernel/debug/devices_deferred actually > shows these lines on my platform: > 30820000.spi spi_imx: can't get the TX DMA channel! > 30830000.spi spi_imx: can't get the TX DMA channel! > > drivers/spi/spi-imx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c > index 34e5f81ec431e..b23325a3bb667 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -1318,7 +1318,7 @@ static int spi_imx_sdma_init(struct device *dev, > struct spi_imx_data *spi_imx, controller->dma_tx = dma_request_chan(dev, > "tx"); > if (IS_ERR(controller->dma_tx)) { > ret = PTR_ERR(controller->dma_tx); > - dev_dbg(dev, "can't get the TX DMA channel, error %d!\n", ret); > + dev_err_probe(dev, ret, "can't get the TX DMA channel! \n"); > controller->dma_tx = NULL; > goto err; > } > @@ -1327,7 +1327,7 @@ static int spi_imx_sdma_init(struct device *dev, > struct spi_imx_data *spi_imx, controller->dma_rx = dma_request_chan(dev, > "rx"); > if (IS_ERR(controller->dma_rx)) { > ret = PTR_ERR(controller->dma_rx); > - dev_dbg(dev, "can't get the RX DMA channel, error %d\n", ret); > + dev_err_probe(dev, ret, "can't get the RX DMA channel! \n"); > controller->dma_rx = NULL; > goto err; > }
On Wed, May 31, 2023 at 09:26:42AM +0200, Alexander Stein wrote: > Hi, > > gentle ping. Please don't send content free pings and please allow a reasonable time for review. People get busy, go on holiday, attend conferences and so on so unless there is some reason for urgency (like critical bug fixes) please allow at least a couple of weeks for review. If there have been review comments then people may be waiting for those to be addressed. Sending content free pings adds to the mail volume (if they are seen at all) which is often the problem and since they can't be reviewed directly if something has gone wrong you'll have to resend the patches anyway, so sending again is generally a better approach though there are some other maintainers who like them - if in doubt look at how patches for the subsystem are normally handled.
Hi Alexander, On Tue, Apr 18, 2023 at 5:35 AM Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > If dma_request_chan() fails, no error is shown nor any information is > shown in /sys/kernel/debug/devices_deferred if -EPROBE_DEFER is returned. > Use dev_err_probe to fix both problems. Running spi-imx without SDMA is valid and not an error, right? I am not sure I understood the real motivation for this patch.
Hi Fabio, Am Mittwoch, 31. Mai 2023, 15:14:37 CEST schrieb Fabio Estevam: > Hi Alexander, > > On Tue, Apr 18, 2023 at 5:35 AM Alexander Stein > > <alexander.stein@ew.tq-group.com> wrote: > > If dma_request_chan() fails, no error is shown nor any information is > > shown in /sys/kernel/debug/devices_deferred if -EPROBE_DEFER is returned. > > Use dev_err_probe to fix both problems. > > Running spi-imx without SDMA is valid and not an error, right? That might be true, but this is not what this patch is about. > I am not sure I understood the real motivation for this patch. If the call to dma_request_chan() for either DMA channel 'rx' and 'tx' returns -EPROBE_DEFER, spi-imx will return that error code as well. So far so good. You can check /sys/kernel/debug/devices_deferred to see which devices got deferred and not probed yet. Without this patch spi-imx will be listed without any additional information about cause of the deferral. Finding the cause with no additional information is cumbersome, it's hidden in dev_dbg(). Using this patch additional information is provided, e.g. "can't get the TX DMA channel!". So you know right away spi-imx is not probing because the DMA channel is not available. I hope this shows the motivation for this change. Best regards, Alexander
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 34e5f81ec431e..b23325a3bb667 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -1318,7 +1318,7 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx, controller->dma_tx = dma_request_chan(dev, "tx"); if (IS_ERR(controller->dma_tx)) { ret = PTR_ERR(controller->dma_tx); - dev_dbg(dev, "can't get the TX DMA channel, error %d!\n", ret); + dev_err_probe(dev, ret, "can't get the TX DMA channel!\n"); controller->dma_tx = NULL; goto err; } @@ -1327,7 +1327,7 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx, controller->dma_rx = dma_request_chan(dev, "rx"); if (IS_ERR(controller->dma_rx)) { ret = PTR_ERR(controller->dma_rx); - dev_dbg(dev, "can't get the RX DMA channel, error %d\n", ret); + dev_err_probe(dev, ret, "can't get the RX DMA channel!\n"); controller->dma_rx = NULL; goto err; }
If dma_request_chan() fails, no error is shown nor any information is shown in /sys/kernel/debug/devices_deferred if -EPROBE_DEFER is returned. Use dev_err_probe to fix both problems. Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- With this patch applied /sys/kernel/debug/devices_deferred actually shows these lines on my platform: 30820000.spi spi_imx: can't get the TX DMA channel! 30830000.spi spi_imx: can't get the TX DMA channel! drivers/spi/spi-imx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)