Skip to contentSkip to conversation

mfd: arizona: Update reset pin to use GPIOD

PATCH Superseded Charles Keepax 5 participants 7 messages Feb 19, 2018 – Feb 26, 2018

Subsystems (3)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/2 mfd: arizona: Update reset pin to use GPIOD

Now GPIOD has support for both pdata systems and for non-standard DT bindings the Arizona reset GPIO can be converted to use it.

---

Changes since v1:
- Only pass reset, instead of wlf,reset to devm_gpio_get, since reset is a standard property and doesn't need a vendor prefix.

Thanks,
Charles

 drivers/mfd/arizona-core.c        | 50 ++++++++++++++++++++++++++-------------
 include/linux/mfd/arizona/pdata.h |  3 ++-
 2 files changed, 35 insertions(+), 18 deletions(-)
drivers/mfd/arizona-core.c +33 -17
index 77875250abe5..30601db51ee6 100644
@@ -278,8 +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_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_value_cansleep(arizona->pdata.reset, 1);
 		usleep_range(1000, 5000);
 	}
 }
@@ -799,14 +798,25 @@ 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);
+	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);
+		switch (ret) {
+		case -ENOENT:
+			break;
+		case -EPROBE_DEFER:
+			return ret;
+		default:
+			dev_err(arizona->dev, "Reset GPIO malformed: %d\n",
+				ret);
+			break;
+		}
 
-		pdata->reset = 0;
+		pdata->reset = NULL;
 	}
 
 	ret = of_property_read_u32_array(arizona->dev->of_node,
@@ -1050,14 +1060,20 @@ 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;
+			else
+				dev_err(arizona->dev,
+					"Reset GPIO missing/malformed: %d\n",
+					ret);
+
+			arizona->pdata.reset = NULL;
 		}
 	}
 
include/linux/mfd/arizona/pdata.h +2 -1
index f72dc53848d7..0013075d4cda 100644
@@ -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;
Signature · 1 line
2.11.0
#1 lore raw

PATCH v2 2/2 mfd: arizona: Update DT doc to support more standard reset binding

---

Changes since v1:
- Drop vendor prefix from new binding since it is a standard property.

Thanks,
Charles

 Documentation/devicetree/bindings/mfd/arizona.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/mfd/arizona.txt +5 -1
index bdd017686ea5..a014afb07902 100644
@@ -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
Signature · 1 line
2.11.0
Reviewed-by on v2 2/2
#2 lore raw

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

Quoted 11 lines · Charles Keepax

---

Changes since v1:
- Drop vendor prefix from new binding since it is a standard property.

Thanks,
Charles

 Documentation/devicetree/bindings/mfd/arizona.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
#4 lore raw
In reply to Charles Keepax

drivers/mfd/arizona-core.c static int arizona_wait_for_boot(struct arizona *arizona) · line 282

Quoted 9 lines · 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.

static inline void arizona_enable_reset(struct arizona *arizona) {

drivers/mfd/arizona-core.c +1 -2
-       if (arizona->pdata.reset)
-               gpio_set_value_cansleep(arizona->pdata.reset, 0);
+       gpiod_set_value_cansleep(arizona->pdata.reset, 0);
 }

Did you try that code when !GPIOLIB ?

Or driver depends on it?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#5 lore raw
Quoted 12 lines · Andy Shevchenko

drivers/mfd/arizona-core.c static int arizona_wait_for_boot(struct arizona *arizona) · line 282

Quoted 9 lines · 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.

static inline void arizona_enable_reset(struct arizona *arizona) {

drivers/mfd/arizona-core.c +1 -2
-       if (arizona->pdata.reset)
-               gpio_set_value_cansleep(arizona->pdata.reset, 0);
+       gpiod_set_value_cansleep(arizona->pdata.reset, 0);
 }
Quoted 4 lines

Did you try that code when !GPIOLIB ?

Or driver depends on it?

Hmm... yeah ok maybe I need to leave that if (reset) in, didn't realise all those stubs straight up call WARNs. Although seems a bit of a shame as gpiod_set_value_cansleep will do the NULL check as well.

Looks like attempt 3 it is :-)

Thanks,
Charles

#6 lore raw
In reply to Charles Keepax
Thank you for the patch! Yet something to improve:

Hi Charles,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.16-rc2 next-20180220]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Charles-Keepax/mfd-arizona-Update-reset-pin-to-use-GPIOD/20180220-203926
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-randconfig-r0-02202307 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

drivers//mfd/arizona-core.c: In function 'arizona_enable_reset':

>> drivers//mfd/arizona-core.c:281:2: error: implicit declaration of function 'gpiod_set_value_cansleep' [-Werror=implicit-function-declaration]
     gpiod_set_value_cansleep(arizona->pdata.reset, 0);
     ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers//mfd/arizona-core.c: In function 'arizona_of_get_core_pdata':
>> drivers//mfd/arizona-core.c:801:17: error: implicit declaration of function 'devm_gpiod_get_from_of_node' [-Werror=implicit-function-declaration]
     pdata->reset = devm_gpiod_get_from_of_node(arizona->dev,
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers//mfd/arizona-core.c:804:10: error: 'GPIOD_OUT_LOW' undeclared (first use in this function)
             GPIOD_OUT_LOW,
             ^~~~~~~~~~~~~
   drivers//mfd/arizona-core.c:804:10: note: each undeclared identifier is reported only once for each function it appears in
   drivers//mfd/arizona-core.c: In function 'arizona_dev_init':
>> drivers//mfd/arizona-core.c:1065:26: error: implicit declaration of function 'devm_gpiod_get' [-Werror=implicit-function-declaration]
      arizona->pdata.reset = devm_gpiod_get(arizona->dev, "reset",
                             ^~~~~~~~~~~~~~
   drivers//mfd/arizona-core.c:1066:13: error: 'GPIOD_OUT_LOW' undeclared (first use in this function)
                GPIOD_OUT_LOW);
                ^~~~~~~~~~~~~

cc1: some warnings being treated as errors

vim +/gpiod_set_value_cansleep +281 drivers//mfd/arizona-core.c

278
279 static inline void arizona_enable_reset(struct arizona *arizona)
280 {

 > 281		gpiod_set_value_cansleep(arizona->pdata.reset, 0);

282 }
283

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation