mmc: pwrseq_simple: Make reset-gpios optional to match doc
PATCH 1/1 mmc: pwrseq_simple: Make reset-gpios optional to match doc
The DT binding doc says reset-gpios is an optional property but the code currently bails out if it is omitted.
Make the code match the documentation.
--- drivers/mmc/core/pwrseq_simple.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/mmc/core/pwrseq_simple.c +13 -8
@@ -29,15 +29,18 @@ struct mmc_pwrseq_simple {static void mmc_pwrseq_simple_set_gpios_value (struct mmc_pwrseq_simple *pwrseq,int value) { -int i;struct gpio_descs *reset_gpios = pwrseq->reset_gpios; -int values[reset_gpios->ndescs]; -for (i =0 ; i < reset_gpios->ndescs; i++) - values[i] = value; +if (!IS_ERR (reset_gpios)) { +int i; +int values[reset_gpios->ndescs]; -gpiod_set_array_value_cansleep (reset_gpios->ndescs, reset_gpios->desc, - values); +for (i =0 ; i < reset_gpios->ndescs; i++) + values[i] = value; + +gpiod_set_array_value_cansleep ( + reset_gpios->ndescs, reset_gpios->desc, values); + } }static void mmc_pwrseq_simple_pre_power_on (struct mmc_host *host)
@@ -79,7 +82,8 @@ static void mmc_pwrseq_simple_free(struct mmc_host *host)struct mmc_pwrseq_simple *pwrseq =container_of (host->pwrseq, struct mmc_pwrseq_simple, pwrseq); -gpiod_put_array (pwrseq->reset_gpios); +if (!IS_ERR (pwrseq->reset_gpios)) +gpiod_put_array (pwrseq->reset_gpios);if (!IS_ERR (pwrseq->ext_clk))clk_put (pwrseq->ext_clk);
@@ -112,7 +116,8 @@ struct mmc_pwrseq *mmc_pwrseq_simple_alloc(struct mmc_host *host, } pwrseq->reset_gpios =gpiod_get_array (dev,"reset" ,GPIOD_OUT_HIGH ); -if (IS_ERR (pwrseq->reset_gpios)) { +if (IS_ERR (pwrseq->reset_gpios) && +PTR_ERR (pwrseq->reset_gpios) != -ENOENT ) { ret =PTR_ERR (pwrseq->reset_gpios); goto clk_put; }
PATCH 1/1 mmc: pwrseq_simple: Make reset-gpios optional to match doc
[adding Tony Lindgren to cc list]
Hello Martin,
Quoted 52 lines · Martin Fuzzey
The DT binding doc says reset-gpios is an optional property but the code currently bails out if it is omitted.
Make the code match the documentation.
Signed-off-by Martin Fuzzey <[email protected]>--- drivers/mmc/core/pwrseq_simple.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)drivers/mmc/core/pwrseq_simple.c +11
-7@@ -29,15 +29,18 @@ struct mmc_pwrseq_simple {static void mmc_pwrseq_simple_set_gpios_value (struct mmc_pwrseq_simple *pwrseq,int value) { -int i;struct gpio_descs *reset_gpios = pwrseq->reset_gpios; -int values[reset_gpios->ndescs]; -for (i =0 ; i < reset_gpios->ndescs; i++) - values[i] = value; +if (!IS_ERR (reset_gpios)) { +int i; +int values[reset_gpios->ndescs]; -gpiod_set_array_value_cansleep (reset_gpios->ndescs, reset_gpios->desc, - values); +for (i =0 ; i < reset_gpios->ndescs; i++) + values[i] = value; + +gpiod_set_array_value_cansleep ( + reset_gpios->ndescs, reset_gpios->desc, values); + } }static void mmc_pwrseq_simple_pre_power_on (struct mmc_host *host)@@ -79,7 +82,8 @@ static void mmc_pwrseq_simple_free(struct mmc_host *host)struct mmc_pwrseq_simple *pwrseq =container_of (host->pwrseq, struct mmc_pwrseq_simple, pwrseq); -gpiod_put_array (pwrseq->reset_gpios); +if (!IS_ERR (pwrseq->reset_gpios)) +gpiod_put_array (pwrseq->reset_gpios);if (!IS_ERR (pwrseq->ext_clk))clk_put (pwrseq->ext_clk);
drivers/mmc/core/pwrseq_simple.c struct mmc_pwrseq *mmc_pwrseq_simple_alloc(struct mmc_host *host, · line 123
Quoted 9 lines
drivers/mmc/core/pwrseq_simple.c +2
-1@@ -112,7 +116,8 @@ struct mmc_pwrseq *mmc_pwrseq_simple_alloc(struct mmc_host *host, } pwrseq->reset_gpios =gpiod_get_array (dev,"reset" ,GPIOD_OUT_HIGH ); -if (IS_ERR (pwrseq->reset_gpios)) { +if (IS_ERR (pwrseq->reset_gpios) && +PTR_ERR (pwrseq->reset_gpios) != -ENOENT ) { ret =PTR_ERR (pwrseq->reset_gpios); goto clk_put; }
Tony already posted a fix for this before [0] but got some feedback so is likely to post a v2 soon.
[0]: http://www.spinics.net/lists/linux-mmc/msg34398.html
Best regards,
Javier
PATCH 1/1 mmc: pwrseq_simple: Make reset-gpios optional to match doc
Hi,
Quoted 5 lines · Javier Martinez Canillas
Quoted 4 lines · Martin Fuzzey
The DT binding doc says reset-gpios is an optional property but the code currently bails out if it is omitted.
Make the code match the documentation.
...
Quoted 2 lines
Tony already posted a fix for this before [0] but got some feedback so is likely to post a v2 soon.
Hmm I think my patch would end up looking pretty much like Martin's patch after fixing the comments. Based on what I understood, Ulf wants to keep pwrseq_simple, well, simple as discussed in [0].
It also seems Ulf also wants to leave out the allocation, and does want to add any deeper knowledge of the state of the GPIOs in pwrse-simple. Instead, we can do device specific pwrseq for things like Marvell 8787 and so on.
Ulf may have more comments, but $subject patch fixes the issue I'm seeing with wl12xx on omap5 devices:
Probably should be tagged Cc: stable # v4.4 when applied.
Regards,
Tony
Quoted 1 line
PATCH 1/1 mmc: pwrseq_simple: Make reset-gpios optional to match doc
In reply to Martin Fuzzey+Tony, Javier, linux-omap
Quoted 4 lines · Martin Fuzzey
The DT binding doc says reset-gpios is an optional property but the code currently bails out if it is omitted.
Make the code match the documentation.
As Tony pointed out, this is actually a regression. Can you please update the change-log according to that.
Quoted 2 lines
Signed-off-by Martin Fuzzey <[email protected]>
Please add a fixes tag:
Quoted 44 lines
--- drivers/mmc/core/pwrseq_simple.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)drivers/mmc/core/pwrseq_simple.c +11
-7@@ -29,15 +29,18 @@ struct mmc_pwrseq_simple {static void mmc_pwrseq_simple_set_gpios_value (struct mmc_pwrseq_simple *pwrseq,int value) { -int i;struct gpio_descs *reset_gpios = pwrseq->reset_gpios; -int values[reset_gpios->ndescs]; -for (i =0 ; i < reset_gpios->ndescs; i++) - values[i] = value; +if (!IS_ERR (reset_gpios)) { +int i; +int values[reset_gpios->ndescs]; -gpiod_set_array_value_cansleep (reset_gpios->ndescs, reset_gpios->desc, - values); +for (i =0 ; i < reset_gpios->ndescs; i++) + values[i] = value; + +gpiod_set_array_value_cansleep ( + reset_gpios->ndescs, reset_gpios->desc, values); + } }static void mmc_pwrseq_simple_pre_power_on (struct mmc_host *host)@@ -79,7 +82,8 @@ static void mmc_pwrseq_simple_free(struct mmc_host *host)struct mmc_pwrseq_simple *pwrseq =container_of (host->pwrseq, struct mmc_pwrseq_simple, pwrseq); -gpiod_put_array (pwrseq->reset_gpios); +if (!IS_ERR (pwrseq->reset_gpios)) +gpiod_put_array (pwrseq->reset_gpios);if (!IS_ERR (pwrseq->ext_clk))clk_put (pwrseq->ext_clk);
drivers/mmc/core/pwrseq_simple.c struct mmc_pwrseq *mmc_pwrseq_simple_alloc(struct mmc_host *host, · line 120
Quoted 6 lines
drivers/mmc/core/pwrseq_simple.c +2
-1@@ -112,7 +116,8 @@ struct mmc_pwrseq *mmc_pwrseq_simple_alloc(struct mmc_host *host, } pwrseq->reset_gpios =gpiod_get_array (dev,"reset" ,GPIOD_OUT_HIGH ); -if (IS_ERR (pwrseq->reset_gpios)) { +if (IS_ERR (pwrseq->reset_gpios) && +PTR_ERR (pwrseq->reset_gpios) != -ENOENT ) {
You need to check also for != -ENOSYS, as otherwise when CONFIG_GPIOLIB is unset we will treat that as an error. This is also what the original code did.
drivers/mmc/core/pwrseq_simple.c struct mmc_pwrseq *mmc_pwrseq_simple_alloc(struct mmc_host *host, · line 123
Quoted 4 lines
drivers/mmc/core/pwrseq_simple.c +0
-0ret =PTR_ERR (pwrseq->reset_gpios); goto clk_put; }
Kind regards
Uffe