diff mbox series

spi: spi-geni-qcom: correctly handle -EPROBE_DEFER from dma_request_chan()

Message ID 20230615-topic-sm8550-upstream-fix-spi-geni-qcom-probe-v1-1-6da9bf2db4a4@linaro.org
State New
Headers
Series spi: spi-geni-qcom: correctly handle -EPROBE_DEFER from dma_request_chan() |

Commit Message

Neil Armstrong June 15, 2023, 9:23 a.m. UTC 
  Now spi_geni_grab_gpi_chan() errors are correctly reported, the
-EPROBE_DEFER error should be returned from probe in case the
GPI dma driver is built as module and/or not probed yet.

Fixes: b59c122484ec ("spi: spi-geni-qcom: Add support for GPI dma")
Fixes: 6532582c353f ("spi: spi-geni-qcom: fix error handling in spi_geni_grab_gpi_chan()")
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/spi/spi-geni-qcom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


---
base-commit: 925294c9aa184801cc0a451b69a18dd0fe7d847d
change-id: 20230615-topic-sm8550-upstream-fix-spi-geni-qcom-probe-9a97cb6b5ea6

Best regards,

Comments

Mark Brown June 15, 2023, 11:06 a.m. UTC | #1 
On Thu, Jun 15, 2023 at 11:23:17AM +0200, Neil Armstrong wrote:

>  			dev_dbg(mas->dev, "Using GPI DMA mode for SPI\n");
>  			break;
> -		}
> +		} else if (ret == -EPROBE_DEFER)
> +			goto out_pm;
>  		/*
>  		 * in case of failure to get gpi dma channel, we can still do the

Both sides of an if statement should have braces if one side does.
Dan Carpenter June 15, 2023, 3:12 p.m. UTC | #2 
Added the kernel-janitors mailing list to the CC.

On Thu, Jun 15, 2023 at 11:23:17AM +0200, Neil Armstrong wrote:
> Now spi_geni_grab_gpi_chan() errors are correctly reported, the
> -EPROBE_DEFER error should be returned from probe in case the
> GPI dma driver is built as module and/or not probed yet.
> 
> Fixes: b59c122484ec ("spi: spi-geni-qcom: Add support for GPI dma")
> Fixes: 6532582c353f ("spi: spi-geni-qcom: fix error handling in spi_geni_grab_gpi_chan()")
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

In olden days the kernel janitors list used to have a TODO list of
random tasks that a newbie could take on.  People loved the idea of a
TODO list but no one ever maintained it so in practice it was useless
after the first year.  I've create a new KTODO tag so we can
automatically create KTODO lists.  The idea is you write a one line
summary that starts with KTODO and the subsystem prefix like so:

KTODO: static analysis: Ensure that -EPROBE_DEFER is always propogated

Then people can use lei[1] to search for the tag and get the latest
TODO list.  Here is the command to search for all the KTODO items added
in the last six months.

lei q https://lore.kernel.org/all/ -o ~/Mail/KTODO --dedupe=mid 'KTODO AND rt:6.month.ago..'

The KTODO entry should have a short summary.  People can download thread
for more context if they want.  Here is the short summary:

The -EPROBE_DEFER error code is special and needs to propogated to the
callers.  If you have code like:

	err = returns_eprobe_defer();
	if (err)
		return -EINVAL;

Then that is a bug because it's returning -EINVAL instead return err;
Use static analysis to prevent this.

regards,
dan carpenter

[1] https://lpc.events/event/11/contributions/983/attachments/759/1421/Doing%20more%20with%20lore%20and%20b4.pdf
Dan Carpenter June 16, 2023, 11:18 a.m. UTC | #3 
On Thu, Jun 15, 2023 at 06:12:03PM +0300, Dan Carpenter wrote:
> Here is the command to search for all the KTODO items added
> in the last six months.
> 
> lei q https://lore.kernel.org/all/ -o ~/Mail/KTODO --dedupe=mid 'KTODO AND rt:6.month.ago..'
> 

I guess you need the -I option here.  I'm not sure what -I does.  I had
thought it might mean case insensitive search but that's not it.

lei q -I https://lore.kernel.org/all/ -o ~/Mail/KTODO --dedupe=mid 'KTODO AND rt:6.month.ago..'

Then grep ^KTODO ~/Mail/KTODO -R and cat the filename you want.

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 206cc04bb1ed..0ebcc5fe92de 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -661,7 +661,8 @@  static int spi_geni_init(struct spi_geni_master *mas)
 			geni_se_select_mode(se, GENI_GPI_DMA);
 			dev_dbg(mas->dev, "Using GPI DMA mode for SPI\n");
 			break;
-		}
+		} else if (ret == -EPROBE_DEFER)
+			goto out_pm;
 		/*
 		 * in case of failure to get gpi dma channel, we can still do the
 		 * FIFO mode, so fallthrough