[PATCH 00/11] mmc: document mmc_regulator_get_supply and fix users

[PATCH 00/11] mmc: document mmc_regulator_get_supply and fix users

[Wolfram Sang]

Fix the drivers and add some docs according to the outcome of this discussion
[1] we had recently. The motivation from the patch description:

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

The patches have been created with coccinelle. A branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/mmc-get-supply-docs

Looking forward to comments!

Kind regards,

   Wolfram

[1] https://patchwork.kernel.org/patch/9965823/

Wolfram Sang (11):
  mmc: add kerneldoc to mmc_regulator_get_supply()
  mmc: cavium: catch all errors when getting regulators
  mmc: dw_mmc: catch all errors when getting regulators
  mmc: meson-gx-mmc: catch all errors when getting regulators
  mmc: meson-mx-sdio: catch all errors when getting regulators
  mmc: mmci: catch all errors when getting regulators
  mmc: mtk-sd: catch all errors when getting regulators
  mmc: mxcmmc: catch all errors when getting regulators
  mmc: omap_hsmmc: catch all errors when getting regulators
  mmc: sdhci: catch all errors when getting regulators
  mmc: usdhi6rol0: catch all errors when getting regulators

 drivers/mmc/core/core.c          | 10 ++++++++++
 drivers/mmc/host/cavium.c        |  2 +-
 drivers/mmc/host/dw_mmc.c        |  2 +-
 drivers/mmc/host/meson-gx-mmc.c  |  2 +-
 drivers/mmc/host/meson-mx-sdio.c |  2 +-
 drivers/mmc/host/mmci.c          |  2 +-
 drivers/mmc/host/mtk-sd.c        |  2 +-
 drivers/mmc/host/mxcmmc.c        |  2 +-
 drivers/mmc/host/omap_hsmmc.c    |  2 +-
 drivers/mmc/host/sdhci.c         |  2 +-
 drivers/mmc/host/usdhi6rol0.c    |  2 +-
 11 files changed, 20 insertions(+), 10 deletions(-)

-- 
2.11.0

[PATCH 01/11] mmc: add kerneldoc to mmc_regulator_get_supply()

[Wolfram Sang]

Especially, make clear what the return value means.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/core/core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index c287d297710aa4..7859bb617a2f60 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1595,6 +1595,16 @@ EXPORT_SYMBOL_GPL(mmc_regulator_set_vqmmc);
 
 #endif /* CONFIG_REGULATOR */
 
+/**
+ * mmc_regulator_get_supply - try to get VMMC and VQMMC regulators for a host
+ * @mmc: the host to regulate
+ *
+ * Returns 0 or errno. errno should be handled, it is either a critical error
+ * or -EPROBE_DEFER. 0 means no critical error but it does not mean all
+ * regulators have been found because they all are optional. If you require
+ * certain regulators, you need to check seperately in your driver if they got
+ * populated after calling this function.
+ */
 int mmc_regulator_get_supply(struct mmc_host *mmc)
 {
 	struct device *dev = mmc_dev(mmc);
-- 
2.11.0

[PATCH 02/11] mmc: cavium: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/cavium.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c
index fbd29f00fca05e..ed5cefb8376838 100644
--- a/drivers/mmc/host/cavium.c
+++ b/drivers/mmc/host/cavium.c
@@ -967,7 +967,7 @@ static int cvm_mmc_of_parse(struct device *dev, struct cvm_mmc_slot *slot)
 	}
 
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		return ret;
 	/*
 	 * Legacy Octeon firmware has no regulator entry, fall-back to
-- 
2.11.0

[PATCH 03/11] mmc: dw_mmc: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 45289c5e0295d2..20d66f8189090d 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2741,7 +2741,7 @@ static int dw_mci_init_slot(struct dw_mci *host)
 
 	/*if there are external regulators, get them*/
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		goto err_host_allocated;
 
 	if (!mmc->ocr_avail)
-- 
2.11.0

[PATCH 04/11] mmc: meson-gx-mmc: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 85745ef179e22b..e0862d3f65b346 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -1190,7 +1190,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	/* Get regulators and the supported OCR mask */
 	host->vqmmc_enabled = false;
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		goto free_host;
 
 	ret = mmc_of_parse(mmc);
-- 
2.11.0

[PATCH 05/11] mmc: meson-mx-sdio: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/meson-mx-sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-mx-sdio.c b/drivers/mmc/host/meson-mx-sdio.c
index 19b499bbe6912f..fcbc469a5b0e74 100644
--- a/drivers/mmc/host/meson-mx-sdio.c
+++ b/drivers/mmc/host/meson-mx-sdio.c
@@ -551,7 +551,7 @@ static int meson_mx_mmc_add_host(struct meson_mx_mmc_host *host)
 
 	/* Get regulators and the supported OCR mask */
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		return ret;
 
 	mmc->max_req_size = MESON_MX_SDIO_BOUNCE_REQ_SIZE;
-- 
2.11.0

[PATCH 06/11] mmc: mmci: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/mmci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index f1f54a81848951..e8a1bb1ae69423 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1658,7 +1658,7 @@ static int mmci_probe(struct amba_device *dev,
 
 	/* Get regulators and the supported OCR mask */
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		goto clk_disable;
 
 	if (!mmc->ocr_avail)
-- 
2.11.0

[PATCH 07/11] mmc: mtk-sd: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/mtk-sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 267f7ab08420eb..7c518a560089ce 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -1641,7 +1641,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
 	}
 
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		goto host_free;
 
 	host->src_clk = devm_clk_get(&pdev->dev, "source");
-- 
2.11.0

[PATCH 08/11] mmc: mxcmmc: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/mxcmmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index 328484b96620e8..8bd9f060be2ae0 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -1075,7 +1075,7 @@ static int mxcmci_probe(struct platform_device *pdev)
 		dat3_card_detect = true;
 
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		goto out_free;
 
 	if (!mmc->ocr_avail) {
-- 
2.11.0

[PATCH 09/11] mmc: omap_hsmmc: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/omap_hsmmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index f2a1137be8bc4b..071693ebfe1891 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -445,7 +445,7 @@ static int omap_hsmmc_reg_get(struct omap_hsmmc_host *host)
 
 
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		return ret;
 
 	/* Allow an aux regulator */
-- 
2.11.0

[PATCH 10/11] mmc: sdhci: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0d5fcca18c9ec1..1052b52045b190 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3238,7 +3238,7 @@ int sdhci_setup_host(struct sdhci_host *host)
 	 * available.
 	 */
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		return ret;
 
 	DBG("Version:   0x%08x | Present:  0x%08x\n",
-- 
2.11.0

[PATCH 11/11] mmc: usdhi6rol0: catch all errors when getting regulators

[Wolfram Sang]

Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
when probing gets deferred. This is currently a no-op, because this function
only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
somewhen, it will be for a reason. (This still doesn't change that getting
regulators is optional, so 0 can still mean no regulators found). So, let us a)
be future proof and b) have driver code which is easier to understand.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/usdhi6rol0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
index 64da6a88cfb90e..cdfeb15b6f0511 100644
--- a/drivers/mmc/host/usdhi6rol0.c
+++ b/drivers/mmc/host/usdhi6rol0.c
@@ -1757,7 +1757,7 @@ static int usdhi6_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ret = mmc_regulator_get_supply(mmc);
-	if (ret == -EPROBE_DEFER)
+	if (ret)
 		goto e_free_mmc;
 
 	ret = mmc_of_parse(mmc);
-- 
2.11.0

RE: [PATCH 00/11] mmc: document mmc_regulator_get_supply and fix users

[Fabrizio Castro]

Hi Wolfram,

thank you for putting me in CC, and of course I agree with the changes you are
suggesting. Have you tried running ./scripts/checkpatch.pl?

Thanks,
Fab

>
> Fix the drivers and add some docs according to the outcome of this discussion
> [1] we had recently. The motivation from the patch description:
>
> Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
> when probing gets deferred. This is currently a no-op, because this function
> only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
> somewhen, it will be for a reason. (This still doesn't change that getting
> regulators is optional, so 0 can still mean no regulators found). So, let us a)
> be future proof and b) have driver code which is easier to understand.
>
> The patches have been created with coccinelle. A branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/mmc-get-supply-docs
>
> Looking forward to comments!
>
> Kind regards,
>
>    Wolfram
>
> [1] https://patchwork.kernel.org/patch/9965823/
>
> Wolfram Sang (11):
>   mmc: add kerneldoc to mmc_regulator_get_supply()
>   mmc: cavium: catch all errors when getting regulators
>   mmc: dw_mmc: catch all errors when getting regulators
>   mmc: meson-gx-mmc: catch all errors when getting regulators
>   mmc: meson-mx-sdio: catch all errors when getting regulators
>   mmc: mmci: catch all errors when getting regulators
>   mmc: mtk-sd: catch all errors when getting regulators
>   mmc: mxcmmc: catch all errors when getting regulators
>   mmc: omap_hsmmc: catch all errors when getting regulators
>   mmc: sdhci: catch all errors when getting regulators
>   mmc: usdhi6rol0: catch all errors when getting regulators
>
>  drivers/mmc/core/core.c          | 10 ++++++++++
>  drivers/mmc/host/cavium.c        |  2 +-
>  drivers/mmc/host/dw_mmc.c        |  2 +-
>  drivers/mmc/host/meson-gx-mmc.c  |  2 +-
>  drivers/mmc/host/meson-mx-sdio.c |  2 +-
>  drivers/mmc/host/mmci.c          |  2 +-
>  drivers/mmc/host/mtk-sd.c        |  2 +-
>  drivers/mmc/host/mxcmmc.c        |  2 +-
>  drivers/mmc/host/omap_hsmmc.c    |  2 +-
>  drivers/mmc/host/sdhci.c         |  2 +-
>  drivers/mmc/host/usdhi6rol0.c    |  2 +-
>  11 files changed, 20 insertions(+), 10 deletions(-)
>
> --
> 2.11.0




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.

Re: [PATCH 00/11] mmc: document mmc_regulator_get_supply and fix users

[Ulf Hansson]

On 7 October 2017 at 12:36, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Fix the drivers and add some docs according to the outcome of this discussion
> [1] we had recently. The motivation from the patch description:
>
> Bail out everytime when mmc_regulator_get_supply() returns an errno, not only
> when probing gets deferred. This is currently a no-op, because this function
> only returns -EPROBE_DEFER or 0 right now. But if it will throw another error
> somewhen, it will be for a reason. (This still doesn't change that getting
> regulators is optional, so 0 can still mean no regulators found). So, let us a)
> be future proof and b) have driver code which is easier to understand.
>
> The patches have been created with coccinelle. A branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/mmc-get-supply-docs
>
> Looking forward to comments!
>
> Kind regards,
>
>    Wolfram
>
> [1] https://patchwork.kernel.org/patch/9965823/
>
> Wolfram Sang (11):
>   mmc: add kerneldoc to mmc_regulator_get_supply()
>   mmc: cavium: catch all errors when getting regulators
>   mmc: dw_mmc: catch all errors when getting regulators
>   mmc: meson-gx-mmc: catch all errors when getting regulators
>   mmc: meson-mx-sdio: catch all errors when getting regulators
>   mmc: mmci: catch all errors when getting regulators
>   mmc: mtk-sd: catch all errors when getting regulators
>   mmc: mxcmmc: catch all errors when getting regulators
>   mmc: omap_hsmmc: catch all errors when getting regulators
>   mmc: sdhci: catch all errors when getting regulators
>   mmc: usdhi6rol0: catch all errors when getting regulators
>
>  drivers/mmc/core/core.c          | 10 ++++++++++
>  drivers/mmc/host/cavium.c        |  2 +-
>  drivers/mmc/host/dw_mmc.c        |  2 +-
>  drivers/mmc/host/meson-gx-mmc.c  |  2 +-
>  drivers/mmc/host/meson-mx-sdio.c |  2 +-
>  drivers/mmc/host/mmci.c          |  2 +-
>  drivers/mmc/host/mtk-sd.c        |  2 +-
>  drivers/mmc/host/mxcmmc.c        |  2 +-
>  drivers/mmc/host/omap_hsmmc.c    |  2 +-
>  drivers/mmc/host/sdhci.c         |  2 +-
>  drivers/mmc/host/usdhi6rol0.c    |  2 +-
>  11 files changed, 20 insertions(+), 10 deletions(-)
>
> --

Looks good to me, however as pointed out by Fabrizio, you could run a
round of checkpatch (mainly about commit message format).

Or perhaps even coccinelle needs an update to be adopted to
requirement from checkpatch?

Kind regards
Uffe

Re: [PATCH 00/11] mmc: document mmc_regulator_get_supply and fix users

[Wolfram Sang]

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


> Or perhaps even coccinelle needs an update to be adopted to
> requirement from checkpatch?

No, it was my scripts. I missed to set the gitcommit filetype when
editing the message template. I fixed that in my scripts and will resend
in a minute. And I missed to set the flag for checkpatch checking. It is
now default. Thanks!


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