[PATCH v5 1/2] mfd: arizona: Update reset pin to use GPIOD

[PATCH v5 1/2] mfd: arizona: Update reset pin to use GPIOD

[Charles Keepax]

Now GPIOD has support for both pdata systems and for non-standard DT
bindings the Arizona reset GPIO can be converted to use it. Worth
noting gpiod_set_raw_value_cansleep is used to match the behaviour
of the old GPIOs. This is because the part is fairly widely used and
it is unknown how many DTs are correctly setting active low through
device tree, so to avoid breaking any existing users it is best to
match the previous behaviour.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---

Changes since v4:
 - Don't use a switch statement for handling return values
 - Add some comments to the handling for the older binding
 - Use gpiod_set_raw_value_cansleep to avoid potential breakage

I have also removed Linus's reviewed-by was a bit border line but
I felt there have probably been enough code changes since he gave
it that it was probably safer to remove it.

Thanks,
Charles

 drivers/mfd/arizona-core.c        | 53 ++++++++++++++++++++++++++-------------
 include/linux/mfd/arizona/pdata.h |  3 ++-
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 77875250abe59..83f1c5a516d99 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -13,13 +13,12 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/err.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/core.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
@@ -279,7 +278,7 @@ static int arizona_wait_for_boot(struct arizona *arizona)
 static inline void arizona_enable_reset(struct arizona *arizona)
 {
 	if (arizona->pdata.reset)
-		gpio_set_value_cansleep(arizona->pdata.reset, 0);
+		gpiod_set_raw_value_cansleep(arizona->pdata.reset, 0);
 }
 
 static void arizona_disable_reset(struct arizona *arizona)
@@ -295,7 +294,7 @@ static void arizona_disable_reset(struct arizona *arizona)
 			break;
 		}
 
-		gpio_set_value_cansleep(arizona->pdata.reset, 1);
+		gpiod_set_raw_value_cansleep(arizona->pdata.reset, 1);
 		usleep_range(1000, 5000);
 	}
 }
@@ -799,14 +798,27 @@ static int arizona_of_get_core_pdata(struct arizona *arizona)
 	struct arizona_pdata *pdata = &arizona->pdata;
 	int ret, i;
 
-	pdata->reset = of_get_named_gpio(arizona->dev->of_node, "wlf,reset", 0);
-	if (pdata->reset == -EPROBE_DEFER) {
-		return pdata->reset;
-	} else if (pdata->reset < 0) {
-		dev_err(arizona->dev, "Reset GPIO missing/malformed: %d\n",
-			pdata->reset);
+	/* Handle old non-standard DT binding */
+	pdata->reset = devm_gpiod_get_from_of_node(arizona->dev,
+						   arizona->dev->of_node,
+						   "wlf,reset", 0,
+						   GPIOD_OUT_LOW,
+						   "arizona /RESET");
+	if (IS_ERR(pdata->reset)) {
+		ret = PTR_ERR(pdata->reset);
 
-		pdata->reset = 0;
+		/*
+		 * Reset missing will be caught when other binding is read
+		 * but all other errors imply this binding is in use but has
+		 * encountered a problem so should be handled.
+		 */
+		if (ret == -EPROBE_DEFER)
+			return ret;
+		else if (ret != -ENOENT && ret != -ENOSYS)
+			dev_err(arizona->dev, "Reset GPIO malformed: %d\n",
+				ret);
+
+		pdata->reset = NULL;
 	}
 
 	ret = of_property_read_u32_array(arizona->dev->of_node,
@@ -1050,14 +1062,19 @@ int arizona_dev_init(struct arizona *arizona)
 		goto err_early;
 	}
 
-	if (arizona->pdata.reset) {
+	if (!arizona->pdata.reset) {
 		/* Start out with /RESET low to put the chip into reset */
-		ret = devm_gpio_request_one(arizona->dev, arizona->pdata.reset,
-					    GPIOF_DIR_OUT | GPIOF_INIT_LOW,
-					    "arizona /RESET");
-		if (ret != 0) {
-			dev_err(dev, "Failed to request /RESET: %d\n", ret);
-			goto err_dcvdd;
+		arizona->pdata.reset = devm_gpiod_get(arizona->dev, "reset",
+						      GPIOD_OUT_LOW);
+		if (IS_ERR(arizona->pdata.reset)) {
+			ret = PTR_ERR(arizona->pdata.reset);
+			if (ret == -EPROBE_DEFER)
+				goto err_dcvdd;
+
+			dev_err(arizona->dev,
+				"Reset GPIO missing/malformed: %d\n", ret);
+
+			arizona->pdata.reset = NULL;
 		}
 	}
 
diff --git a/include/linux/mfd/arizona/pdata.h b/include/linux/mfd/arizona/pdata.h
index f72dc53848d70..0013075d4cda0 100644
--- a/include/linux/mfd/arizona/pdata.h
+++ b/include/linux/mfd/arizona/pdata.h
@@ -56,6 +56,7 @@
 #define ARIZONA_MAX_PDM_SPK 2
 
 struct regulator_init_data;
+struct gpio_desc;
 
 struct arizona_micbias {
 	int mV;                    /** Regulated voltage */
@@ -77,7 +78,7 @@ struct arizona_micd_range {
 };
 
 struct arizona_pdata {
-	int reset;      /** GPIO controlling /RESET, if any */
+	struct gpio_desc *reset;      /** GPIO controlling /RESET, if any */
 
 	/** Regulator configuration for MICVDD */
 	struct arizona_micsupp_pdata micvdd;
-- 
2.11.0

[PATCH v5 2/2] mfd: arizona: Update DT doc to support more standard reset binding

[Charles Keepax]

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---

No changes since v4.

 Documentation/devicetree/bindings/mfd/arizona.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/arizona.txt b/Documentation/devicetree/bindings/mfd/arizona.txt
index bdd017686ea5a..a014afb079023 100644
--- a/Documentation/devicetree/bindings/mfd/arizona.txt
+++ b/Documentation/devicetree/bindings/mfd/arizona.txt
@@ -50,7 +50,7 @@ Required properties:
 
 Optional properties:
 
-  - wlf,reset : GPIO specifier for the GPIO controlling /RESET
+  - reset-gpios : GPIO specifier for the GPIO controlling /RESET
 
   - clocks: Should reference the clocks supplied on MCLK1 and MCLK2
   - clock-names: Should contains two strings:
@@ -70,6 +70,10 @@ Optional properties:
     Documentation/devicetree/bindings/regulator/regulator.txt
     (wm5102, wm5110, wm8280, wm8997, wm8998, wm1814)
 
+Deprecated properties:
+
+  - wlf,reset : GPIO specifier for the GPIO controlling /RESET
+
 Also see child specific device properties:
   Regulator - ../regulator/arizona-regulator.txt
   Extcon    - ../extcon/extcon-arizona.txt
-- 
2.11.0

Re: [PATCH v5 1/2] mfd: arizona: Update reset pin to use GPIOD

[Linus Walleij]

On Mon, Mar 12, 2018 at 4:52 PM, Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:

> Now GPIOD has support for both pdata systems and for non-standard DT
> bindings the Arizona reset GPIO can be converted to use it. Worth
> noting gpiod_set_raw_value_cansleep is used to match the behaviour
> of the old GPIOs. This is because the part is fairly widely used and
> it is unknown how many DTs are correctly setting active low through
> device tree, so to avoid breaking any existing users it is best to
> match the previous behaviour.
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>
> Changes since v4:
>  - Don't use a switch statement for handling return values
>  - Add some comments to the handling for the older binding
>  - Use gpiod_set_raw_value_cansleep to avoid potential breakage
>
> I have also removed Linus's reviewed-by was a bit border line but
> I felt there have probably been enough code changes since he gave
> it that it was probably safer to remove it.

It's fine.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

The use of gpiod_set_raw* should be done with caution, but this
is one of the cases where we need to take out the big hammer to
make sure things stay compatible.

I haven't read why it is like so but I guess because the right flags
in the device tree can not be guaranteed?

Yours,
Linus Walleij

Re: [PATCH v5 1/2] mfd: arizona: Update reset pin to use GPIOD

[Charles Keepax]

On Sat, Mar 24, 2018 at 03:32:05PM +0100, Linus Walleij wrote:
> On Mon, Mar 12, 2018 at 4:52 PM, Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> 
> > Now GPIOD has support for both pdata systems and for non-standard DT
> > bindings the Arizona reset GPIO can be converted to use it. Worth
> > noting gpiod_set_raw_value_cansleep is used to match the behaviour
> > of the old GPIOs. This is because the part is fairly widely used and
> > it is unknown how many DTs are correctly setting active low through
> > device tree, so to avoid breaking any existing users it is best to
> > match the previous behaviour.
> >
> > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> > ---
> The use of gpiod_set_raw* should be done with caution, but this
> is one of the cases where we need to take out the big hammer to
> make sure things stay compatible.
> 
> I haven't read why it is like so but I guess because the right flags
> in the device tree can not be guaranteed?
> 

Exactly yes there are a lot of these parts out there and since
they never required the DT flags to be set correctly before I
very much doubt all those DTs are set correctly.

Thanks,
Charles

Re: [PATCH v5 1/2] mfd: arizona: Update reset pin to use GPIOD

[Lee Jones]

On Mon, 12 Mar 2018, Charles Keepax wrote:

> Now GPIOD has support for both pdata systems and for non-standard DT
> bindings the Arizona reset GPIO can be converted to use it. Worth
> noting gpiod_set_raw_value_cansleep is used to match the behaviour
> of the old GPIOs. This is because the part is fairly widely used and
> it is unknown how many DTs are correctly setting active low through
> device tree, so to avoid breaking any existing users it is best to
> match the previous behaviour.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
> 
> Changes since v4:
>  - Don't use a switch statement for handling return values
>  - Add some comments to the handling for the older binding
>  - Use gpiod_set_raw_value_cansleep to avoid potential breakage
> 
> I have also removed Linus's reviewed-by was a bit border line but
> I felt there have probably been enough code changes since he gave
> it that it was probably safer to remove it.
> 
> Thanks,
> Charles
> 
>  drivers/mfd/arizona-core.c        | 53 ++++++++++++++++++++++++++-------------
>  include/linux/mfd/arizona/pdata.h |  3 ++-
>  2 files changed, 37 insertions(+), 19 deletions(-)

Applied with Linus' Ack, thanks

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog