linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] at24: move write-protect pin handling to nvmem core
@ 2019-12-19 11:51 Khouloud Touil
  2019-12-19 11:51 ` [PATCH v3 1/4] dt-bindings: nvmem: new optional property write-protect-gpios Khouloud Touil
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Khouloud Touil @ 2019-12-19 11:51 UTC (permalink / raw)
  To: bgolaszewski, robh+dt, mark.rutland, srinivas.kandagatla,
	baylibre-upstreaming
  Cc: linux-kernel, devicetree, linux-i2c, linus.walleij, Khouloud Touil

The write-protect pin handling looks like a standard property that
could benefit other users if available in the core nvmem framework.
    
Instead of modifying all the drivers to check this pin, make the
nvmem subsystem check if the write-protect GPIO being passed
through the nvmem_config or defined in the device tree and pull it
low whenever writing to the memory.

This patchset:

- adds support for the write-protect pin split into two parts.
The first patch modifies modifies the relevant binding document,
while the second modifies the nvmem code to pull the write-protect
GPIO low (if present) during write operations.

- removes support for the write-protect pin split into two parts.
The first patch modifies the relevant binding document to remove
the wp-gpio, while the second removes the relevant code in the
at24 driver.

Changes since v1:
-Add an explenation on how the wp-gpios works
-keep reference to the wp-gpios in the at24 binding

Changes since v2:
-Use the flag GPIO_ACTIVE_HIGH instead of 0


Khouloud Touil (4):
  dt-bindings: nvmem: new optional property write-protect-gpios
  nvmem: add support for the write-protect pin
  dt-bindings: at24: remove the optional property write-protect-gpios
  eeprom: at24: remove the write-protect pin support

 .../devicetree/bindings/eeprom/at24.yaml      |  6 +-----
 .../devicetree/bindings/nvmem/nvmem.yaml      | 11 +++++++++++
 drivers/misc/eeprom/at24.c                    |  9 ---------
 drivers/nvmem/core.c                          | 19 +++++++++++++++++--
 drivers/nvmem/nvmem.h                         |  2 ++
 include/linux/nvmem-provider.h                |  3 +++
 6 files changed, 34 insertions(+), 16 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/4] dt-bindings: nvmem: new optional property write-protect-gpios
  2019-12-19 11:51 [PATCH v3 0/4] at24: move write-protect pin handling to nvmem core Khouloud Touil
@ 2019-12-19 11:51 ` Khouloud Touil
  2020-01-03 23:40   ` Rob Herring
  2019-12-19 11:51 ` [PATCH v3 2/4] nvmem: add support for the write-protect pin Khouloud Touil
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Khouloud Touil @ 2019-12-19 11:51 UTC (permalink / raw)
  To: bgolaszewski, robh+dt, mark.rutland, srinivas.kandagatla,
	baylibre-upstreaming
  Cc: linux-kernel, devicetree, linux-i2c, linus.walleij, Khouloud Touil

Several memories have a write-protect pin, that when pulled high, it
blocks the write operation.

On some boards, this pin is connected to a GPIO and pulled high by
default, which forces the user to manually change its state before
writing.

Instead of modifying all the memory drivers to check this pin, make
the NVMEM subsystem check if the write-protect GPIO being passed
through the nvmem_config or defined in the device tree and pull it
low whenever writing to the memory.

Add a new optional property to the device tree binding document, which
allows to specify the GPIO line to which the write-protect pin is
connected.

Signed-off-by: Khouloud Touil <ktouil@baylibre.com>
---
 Documentation/devicetree/bindings/nvmem/nvmem.yaml | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 1c75a059206c..b43c6c65294e 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -34,6 +34,14 @@ properties:
     description:
       Mark the provider as read only.
 
+  wp-gpios:
+    description:
+      GPIO to which the write-protect pin of the chip is connected.
+      The write-protect GPIO is asserted, when it's driven high
+      (logical '1') to block the write operation. It's deasserted,
+      when it's driven low (logical '0') to allow writing.
+    maxItems: 1
+
 patternProperties:
   "^.*@[0-9a-f]+$":
     type: object
@@ -63,9 +71,12 @@ patternProperties:
 
 examples:
   - |
+      #include <dt-bindings/gpio/gpio.h>
+
       qfprom: eeprom@700000 {
           #address-cells = <1>;
           #size-cells = <1>;
+          wp-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
 
           /* ... */
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/4] nvmem: add support for the write-protect pin
  2019-12-19 11:51 [PATCH v3 0/4] at24: move write-protect pin handling to nvmem core Khouloud Touil
  2019-12-19 11:51 ` [PATCH v3 1/4] dt-bindings: nvmem: new optional property write-protect-gpios Khouloud Touil
@ 2019-12-19 11:51 ` Khouloud Touil
  2019-12-19 11:51 ` [PATCH v3 3/4] dt-bindings: at24: remove the optional property write-protect-gpios Khouloud Touil
  2019-12-19 11:51 ` [PATCH v3 4/4] eeprom: at24: remove the write-protect pin support Khouloud Touil
  3 siblings, 0 replies; 7+ messages in thread
From: Khouloud Touil @ 2019-12-19 11:51 UTC (permalink / raw)
  To: bgolaszewski, robh+dt, mark.rutland, srinivas.kandagatla,
	baylibre-upstreaming
  Cc: linux-kernel, devicetree, linux-i2c, linus.walleij, Khouloud Touil

The write-protect pin handling looks like a standard property that
could benefit other users if available in the core nvmem framework.

Instead of modifying all the memory drivers to check this pin, make
the NVMEM subsystem check if the write-protect GPIO being passed
through the nvmem_config or defined in the device tree and pull it
low whenever writing to the memory.

There was a suggestion for introducing the gpiodesc from pdata, but
as pdata is already removed it could be replaced by adding it to
nvmem_config.

Reference: https://lists.96boards.org/pipermail/dev/2018-August/001056.html

Signed-off-by: Khouloud Touil <ktouil@baylibre.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/nvmem/core.c           | 19 +++++++++++++++++--
 drivers/nvmem/nvmem.h          |  2 ++
 include/linux/nvmem-provider.h |  3 +++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 9f1ee9c766ec..3e1c94c4eee8 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/nvmem-provider.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of.h>
 #include <linux/slab.h>
 #include "nvmem.h"
@@ -54,8 +55,14 @@ static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
 static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
 			   void *val, size_t bytes)
 {
-	if (nvmem->reg_write)
-		return nvmem->reg_write(nvmem->priv, offset, val, bytes);
+	int ret;
+
+	if (nvmem->reg_write) {
+		gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
+		ret = nvmem->reg_write(nvmem->priv, offset, val, bytes);
+		gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
+		return ret;
+	}
 
 	return -EINVAL;
 }
@@ -338,6 +345,14 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 		kfree(nvmem);
 		return ERR_PTR(rval);
 	}
+	if (config->wp_gpio)
+		nvmem->wp_gpio = config->wp_gpio;
+	else
+		nvmem->wp_gpio = gpiod_get_optional(config->dev, "wp",
+						    GPIOD_OUT_HIGH);
+	if (IS_ERR(nvmem->wp_gpio))
+		return PTR_ERR(nvmem->wp_gpio);
+
 
 	kref_init(&nvmem->refcnt);
 	INIT_LIST_HEAD(&nvmem->cells);
diff --git a/drivers/nvmem/nvmem.h b/drivers/nvmem/nvmem.h
index eb8ed7121fa3..be0d66d75c8a 100644
--- a/drivers/nvmem/nvmem.h
+++ b/drivers/nvmem/nvmem.h
@@ -9,6 +9,7 @@
 #include <linux/list.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/nvmem-provider.h>
+#include <linux/gpio/consumer.h>
 
 struct nvmem_device {
 	struct module		*owner;
@@ -26,6 +27,7 @@ struct nvmem_device {
 	struct list_head	cells;
 	nvmem_reg_read_t	reg_read;
 	nvmem_reg_write_t	reg_write;
+	struct gpio_desc	*wp_gpio;
 	void *priv;
 };
 
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index fe051323be0a..6d6f8e5d24c9 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -11,6 +11,7 @@
 
 #include <linux/err.h>
 #include <linux/errno.h>
+#include <linux/gpio/consumer.h>
 
 struct nvmem_device;
 struct nvmem_cell_info;
@@ -45,6 +46,7 @@ enum nvmem_type {
  * @word_size:	Minimum read/write access granularity.
  * @stride:	Minimum read/write access stride.
  * @priv:	User context passed to read/write callbacks.
+ * @wp-gpio:   Write protect pin
  *
  * Note: A default "nvmem<id>" name will be assigned to the device if
  * no name is specified in its configuration. In such case "<id>" is
@@ -58,6 +60,7 @@ struct nvmem_config {
 	const char		*name;
 	int			id;
 	struct module		*owner;
+	struct gpio_desc	*wp_gpio;
 	const struct nvmem_cell_info	*cells;
 	int			ncells;
 	enum nvmem_type		type;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 3/4] dt-bindings: at24: remove the optional property write-protect-gpios
  2019-12-19 11:51 [PATCH v3 0/4] at24: move write-protect pin handling to nvmem core Khouloud Touil
  2019-12-19 11:51 ` [PATCH v3 1/4] dt-bindings: nvmem: new optional property write-protect-gpios Khouloud Touil
  2019-12-19 11:51 ` [PATCH v3 2/4] nvmem: add support for the write-protect pin Khouloud Touil
@ 2019-12-19 11:51 ` Khouloud Touil
  2020-01-03 23:44   ` Rob Herring
  2019-12-19 11:51 ` [PATCH v3 4/4] eeprom: at24: remove the write-protect pin support Khouloud Touil
  3 siblings, 1 reply; 7+ messages in thread
From: Khouloud Touil @ 2019-12-19 11:51 UTC (permalink / raw)
  To: bgolaszewski, robh+dt, mark.rutland, srinivas.kandagatla,
	baylibre-upstreaming
  Cc: linux-kernel, devicetree, linux-i2c, linus.walleij, Khouloud Touil

NVMEM framework is an interface for the at24 EEPROMs as well as for
other drivers, instead of passing the wp-gpios over the different
drivers each time, it would be better to pass it over the NVMEM
subsystem once and for all.

Removing the optional property form the device tree binding document.

Signed-off-by: Khouloud Touil <ktouil@baylibre.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 Documentation/devicetree/bindings/eeprom/at24.yaml | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
index e8778560d966..75de83708146 100644
--- a/Documentation/devicetree/bindings/eeprom/at24.yaml
+++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
@@ -145,10 +145,7 @@ properties:
       over reads to the next slave address. Please consult the manual of
       your device.
 
-  wp-gpios:
-    description:
-      GPIO to which the write-protect pin of the chip is connected.
-    maxItems: 1
+  wp-gpios: true
 
   address-width:
     allOf:
@@ -181,7 +178,6 @@ examples:
           compatible = "microchip,24c32", "atmel,24c32";
           reg = <0x52>;
           pagesize = <32>;
-          wp-gpios = <&gpio1 3 0>;
           num-addresses = <8>;
       };
     };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 4/4] eeprom: at24: remove the write-protect pin support
  2019-12-19 11:51 [PATCH v3 0/4] at24: move write-protect pin handling to nvmem core Khouloud Touil
                   ` (2 preceding siblings ...)
  2019-12-19 11:51 ` [PATCH v3 3/4] dt-bindings: at24: remove the optional property write-protect-gpios Khouloud Touil
@ 2019-12-19 11:51 ` Khouloud Touil
  3 siblings, 0 replies; 7+ messages in thread
From: Khouloud Touil @ 2019-12-19 11:51 UTC (permalink / raw)
  To: bgolaszewski, robh+dt, mark.rutland, srinivas.kandagatla,
	baylibre-upstreaming
  Cc: linux-kernel, devicetree, linux-i2c, linus.walleij, Khouloud Touil

NVMEM framework is an interface for the at24 EEPROMs as well as for
other drivers, instead of passing the wp-gpios over the different
drivers each time, it would be better to pass it over the NVMEM
subsystem once and for all.

Removing the support for the write-protect pin after adding it to the
NVMEM subsystem.

Signed-off-by: Khouloud Touil <ktouil@baylibre.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/misc/eeprom/at24.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 0681d5fdd538..8fce49a6d9cd 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -22,7 +22,6 @@
 #include <linux/nvmem-provider.h>
 #include <linux/regmap.h>
 #include <linux/pm_runtime.h>
-#include <linux/gpio/consumer.h>
 
 /* Address pointer is 16 bit. */
 #define AT24_FLAG_ADDR16	BIT(7)
@@ -89,8 +88,6 @@ struct at24_data {
 
 	struct nvmem_device *nvmem;
 
-	struct gpio_desc *wp_gpio;
-
 	/*
 	 * Some chips tie up multiple I2C addresses; dummy devices reserve
 	 * them for us, and we'll use them with SMBus calls.
@@ -457,12 +454,10 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
 	 * from this host, but not from other I2C masters.
 	 */
 	mutex_lock(&at24->lock);
-	gpiod_set_value_cansleep(at24->wp_gpio, 0);
 
 	while (count) {
 		ret = at24_regmap_write(at24, buf, off, count);
 		if (ret < 0) {
-			gpiod_set_value_cansleep(at24->wp_gpio, 1);
 			mutex_unlock(&at24->lock);
 			pm_runtime_put(dev);
 			return ret;
@@ -472,7 +467,6 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
 		count -= ret;
 	}
 
-	gpiod_set_value_cansleep(at24->wp_gpio, 1);
 	mutex_unlock(&at24->lock);
 
 	pm_runtime_put(dev);
@@ -662,9 +656,6 @@ static int at24_probe(struct i2c_client *client)
 	at24->client[0].client = client;
 	at24->client[0].regmap = regmap;
 
-	at24->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_HIGH);
-	if (IS_ERR(at24->wp_gpio))
-		return PTR_ERR(at24->wp_gpio);
 
 	writable = !(flags & AT24_FLAG_READONLY);
 	if (writable) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: nvmem: new optional property write-protect-gpios
  2019-12-19 11:51 ` [PATCH v3 1/4] dt-bindings: nvmem: new optional property write-protect-gpios Khouloud Touil
@ 2020-01-03 23:40   ` Rob Herring
  0 siblings, 0 replies; 7+ messages in thread
From: Rob Herring @ 2020-01-03 23:40 UTC (permalink / raw)
  To: Khouloud Touil
  Cc: bgolaszewski, mark.rutland, srinivas.kandagatla,
	baylibre-upstreaming, linux-kernel, devicetree, linux-i2c,
	linus.walleij

On Thu, Dec 19, 2019 at 12:51:38PM +0100, Khouloud Touil wrote:
> Several memories have a write-protect pin, that when pulled high, it
> blocks the write operation.

Subject doesn't match the actual property name.

> 
> On some boards, this pin is connected to a GPIO and pulled high by
> default, which forces the user to manually change its state before
> writing.
> 
> Instead of modifying all the memory drivers to check this pin, make
> the NVMEM subsystem check if the write-protect GPIO being passed
> through the nvmem_config or defined in the device tree and pull it
> low whenever writing to the memory.
> 
> Add a new optional property to the device tree binding document, which
> allows to specify the GPIO line to which the write-protect pin is
> connected.
> 
> Signed-off-by: Khouloud Touil <ktouil@baylibre.com>
> ---
>  Documentation/devicetree/bindings/nvmem/nvmem.yaml | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> index 1c75a059206c..b43c6c65294e 100644
> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> @@ -34,6 +34,14 @@ properties:
>      description:
>        Mark the provider as read only.
>  
> +  wp-gpios:
> +    description:
> +      GPIO to which the write-protect pin of the chip is connected.
> +      The write-protect GPIO is asserted, when it's driven high
> +      (logical '1') to block the write operation. It's deasserted,
> +      when it's driven low (logical '0') to allow writing.
> +    maxItems: 1
> +
>  patternProperties:
>    "^.*@[0-9a-f]+$":
>      type: object
> @@ -63,9 +71,12 @@ patternProperties:
>  
>  examples:
>    - |
> +      #include <dt-bindings/gpio/gpio.h>
> +
>        qfprom: eeprom@700000 {
>            #address-cells = <1>;
>            #size-cells = <1>;
> +          wp-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
>  
>            /* ... */
>  
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 3/4] dt-bindings: at24: remove the optional property write-protect-gpios
  2019-12-19 11:51 ` [PATCH v3 3/4] dt-bindings: at24: remove the optional property write-protect-gpios Khouloud Touil
@ 2020-01-03 23:44   ` Rob Herring
  0 siblings, 0 replies; 7+ messages in thread
From: Rob Herring @ 2020-01-03 23:44 UTC (permalink / raw)
  To: Khouloud Touil
  Cc: bgolaszewski, mark.rutland, srinivas.kandagatla,
	baylibre-upstreaming, linux-kernel, devicetree, linux-i2c,
	linus.walleij

On Thu, Dec 19, 2019 at 12:51:40PM +0100, Khouloud Touil wrote:
> NVMEM framework is an interface for the at24 EEPROMs as well as for
> other drivers, instead of passing the wp-gpios over the different
> drivers each time, it would be better to pass it over the NVMEM
> subsystem once and for all.
> 
> Removing the optional property form the device tree binding document.
> 
> Signed-off-by: Khouloud Touil <ktouil@baylibre.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  Documentation/devicetree/bindings/eeprom/at24.yaml | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> index e8778560d966..75de83708146 100644
> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> @@ -145,10 +145,7 @@ properties:
>        over reads to the next slave address. Please consult the manual of
>        your device.
>  
> -  wp-gpios:
> -    description:
> -      GPIO to which the write-protect pin of the chip is connected.
> -    maxItems: 1
> +  wp-gpios: true
>  
>    address-width:
>      allOf:
> @@ -181,7 +178,6 @@ examples:
>            compatible = "microchip,24c32", "atmel,24c32";
>            reg = <0x52>;
>            pagesize = <32>;
> -          wp-gpios = <&gpio1 3 0>;

This is still valid, why is it being removed?

>            num-addresses = <8>;
>        };
>      };
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-01-03 23:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-19 11:51 [PATCH v3 0/4] at24: move write-protect pin handling to nvmem core Khouloud Touil
2019-12-19 11:51 ` [PATCH v3 1/4] dt-bindings: nvmem: new optional property write-protect-gpios Khouloud Touil
2020-01-03 23:40   ` Rob Herring
2019-12-19 11:51 ` [PATCH v3 2/4] nvmem: add support for the write-protect pin Khouloud Touil
2019-12-19 11:51 ` [PATCH v3 3/4] dt-bindings: at24: remove the optional property write-protect-gpios Khouloud Touil
2020-01-03 23:44   ` Rob Herring
2019-12-19 11:51 ` [PATCH v3 4/4] eeprom: at24: remove the write-protect pin support Khouloud Touil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).