[PATCH 0/4] at24: move write-protect pin handling to nvmem core

[PATCH 0/4] at24: move write-protect pin handling to nvmem core

[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.


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      |  6 ++++++
 drivers/misc/eeprom/at24.c                    |  9 ---------
 drivers/nvmem/core.c                          | 20 +++++++++++++++++--
 drivers/nvmem/nvmem.h                         |  2 ++
 include/linux/nvmem-provider.h                |  3 +++
 6 files changed, 29 insertions(+), 17 deletions(-)

-- 
2.17.1

[PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Khouloud Touil]

Many nvmem memory chips have a write-protect pin which, when pulled
high, blocks the write operations.

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 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 1c75a059206c..6724764af794 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -34,6 +34,11 @@ properties:
     description:
       Mark the provider as read only.
 
+  wp-gpios:
+    description:
+      GPIO to which the write-protect pin of the chip is connected.
+    maxItems: 1
+
 patternProperties:
   "^.*@[0-9a-f]+$":
     type: object
@@ -66,6 +71,7 @@ examples:
       qfprom: eeprom@700000 {
           #address-cells = <1>;
           #size-cells = <1>;
+          wp-gpios = <&gpio1 3 0>;
 
           /* ... */
 
-- 
2.17.1

Re: [PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Linus Walleij]

Hi Khouloud,

thanks for your patch!

I just have a semantic comment:

On Wed, Nov 20, 2019 at 3:21 PM Khouloud Touil <ktouil@baylibre.com> wrote:

> 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.

It is claimed that this should be pulled low to assert it so by
definition it is active low.

> +  wp-gpios:
> +    description:
> +      GPIO to which the write-protect pin of the chip is connected.
> +    maxItems: 1

Mandate that the flag in the second cell should be GPIO_ACTIVE_LOW

>  patternProperties:
>    "^.*@[0-9a-f]+$":
>      type: object
> @@ -66,6 +71,7 @@ examples:
>        qfprom: eeprom@700000 {
>            #address-cells = <1>;
>            #size-cells = <1>;
> +          wp-gpios = <&gpio1 3 0>;

#include <dt-bindings/gpio/gpio.h>
wp-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;

This will in Linux have the semantic effect that you need to
set the output high with gpio_set_val(d, 1) to assert it
(drive it low) but that really doesn't matter to the device tree
bindings, those are OS-agnostic: if the line is active low then
it should use this flag.

It has the upside that the day you need a write-protect that
is active high, it is simple to support that use case too.

Yours,
Linus Walleij

Re: [PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Bartosz Golaszewski]

pt., 22 lis 2019 o 13:41 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> Hi Khouloud,
>
> thanks for your patch!
>
> I just have a semantic comment:
>
> On Wed, Nov 20, 2019 at 3:21 PM Khouloud Touil <ktouil@baylibre.com> wrote:
>
> > 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.
>
> It is claimed that this should be pulled low to assert it so by
> definition it is active low.
>
> > +  wp-gpios:
> > +    description:
> > +      GPIO to which the write-protect pin of the chip is connected.
> > +    maxItems: 1
>
> Mandate that the flag in the second cell should be GPIO_ACTIVE_LOW
>
> >  patternProperties:
> >    "^.*@[0-9a-f]+$":
> >      type: object
> > @@ -66,6 +71,7 @@ examples:
> >        qfprom: eeprom@700000 {
> >            #address-cells = <1>;
> >            #size-cells = <1>;
> > +          wp-gpios = <&gpio1 3 0>;
>
> #include <dt-bindings/gpio/gpio.h>
> wp-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
>
> This will in Linux have the semantic effect that you need to
> set the output high with gpio_set_val(d, 1) to assert it
> (drive it low) but that really doesn't matter to the device tree
> bindings, those are OS-agnostic: if the line is active low then
> it should use this flag.
>
> It has the upside that the day you need a write-protect that
> is active high, it is simple to support that use case too.
>

Linus,

what about the existing bindings for at24 that don't mandate the
active-low flag? I'm afraid this would break the support for this
specific chip or lead to code duplication if we had this in both nvmem
and at24 with different logic.

Bartosz

> Yours,
> Linus Walleij

Re: [PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Linus Walleij]

On Fri, Nov 22, 2019 at 1:47 PM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:

> what about the existing bindings for at24 that don't mandate the
> active-low flag? I'm afraid this would break the support for this
> specific chip or lead to code duplication if we had this in both nvmem
> and at24 with different logic.

Hm yeah I realized this when I read patches 3 & 4.

I would to like this:

1. Add a new generic property
   writeprotect-gpios that mandates to use GPIO_ACTIVE_LOW
   and use this in the new example

2. Deprecate wp-gpios in the binding, keep it around but deprecated.

3. Add a quirk to gpiolib-of in the manner of the other quirks there
   (like for SPI) so that if we are dealing with some EEPROM node
   like at24 and the flag is zero, tag on GPIO_ACTIVE_LOW on
   the descriptor.

The driver will now handle the semantic of both cases
with gpiolib-of providing a quirk for the old binding.

This is how we solved this type of problem before.

Yours,
Linus Walleij

Re: [PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Bartosz Golaszewski]

pt., 22 lis 2019 o 13:53 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> On Fri, Nov 22, 2019 at 1:47 PM Bartosz Golaszewski
> <bgolaszewski@baylibre.com> wrote:
>
> > what about the existing bindings for at24 that don't mandate the
> > active-low flag? I'm afraid this would break the support for this
> > specific chip or lead to code duplication if we had this in both nvmem
> > and at24 with different logic.
>
> Hm yeah I realized this when I read patches 3 & 4.
>
> I would to like this:
>
> 1. Add a new generic property
>    writeprotect-gpios that mandates to use GPIO_ACTIVE_LOW
>    and use this in the new example
>
> 2. Deprecate wp-gpios in the binding, keep it around but deprecated.

This is a pretty standard property though - for instance it is
documented in the main mmc binding and doesn't mandate GPIO_ACTIVE_LOW
either. I think this is because nobody says that the write-protect
line must always be driver low to be asserted - this is highly
implementation-specific.

Bartosz

>
> 3. Add a quirk to gpiolib-of in the manner of the other quirks there
>    (like for SPI) so that if we are dealing with some EEPROM node
>    like at24 and the flag is zero, tag on GPIO_ACTIVE_LOW on
>    the descriptor.
>
> The driver will now handle the semantic of both cases
> with gpiolib-of providing a quirk for the old binding.
>
> This is how we solved this type of problem before.
>
> Yours,
> Linus Walleij

Re: [PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Linus Walleij]

On Fri, Nov 22, 2019 at 2:04 PM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:

> > I would to like this:
> >
> > 1. Add a new generic property
> >    writeprotect-gpios that mandates to use GPIO_ACTIVE_LOW
> >    and use this in the new example
> >
> > 2. Deprecate wp-gpios in the binding, keep it around but deprecated.
>
> This is a pretty standard property though - for instance it is
> documented in the main mmc binding and doesn't mandate GPIO_ACTIVE_LOW
> either. I think this is because nobody says that the write-protect
> line must always be driver low to be asserted - this is highly
> implementation-specific.

The MMC case is actually especially convoluted. It has always
respected the GPIO_ACTIVE_LOW flag, and that is used if
present. At the same time it *also* supported a bool
wp-inverted flag, with the specified semantic that if both
were specified (ACTIVE_LOW and wp-inverted) the result
would be nothing as it is a double logical inversion.

So that is why the quirk looks like this:

       /*
         * Handle MMC "cd-inverted" and "wp-inverted" semantics.
         */
        if (IS_ENABLED(CONFIG_MMC)) {
                /*
                 * Active low is the default according to the
                 * SDHCI specification and the device tree
                 * bindings. However the code in the current
                 * kernel was written such that the phandle
                 * flags were always respected, and "cd-inverted"
                 * would invert the flag from the device phandle.
                 */
                if (!strcmp(propname, "cd-gpios")) {
                        if (of_property_read_bool(np, "cd-inverted"))
                                *flags ^= OF_GPIO_ACTIVE_LOW;
                }
                if (!strcmp(propname, "wp-gpios")) {
                        if (of_property_read_bool(np, "wp-inverted"))
                                *flags ^= OF_GPIO_ACTIVE_LOW;
                }
        }

Nevermind MMC though.

The current code for at24 has an ambiguousness issue: if
the gpios cell 2 is specified as GPIO_ACTIVE_LOW
(which is in some sense correct) then the effect will be
that it is driven high to assert the wp, which is  ... rather
counterintuitive.

I could think of a compromise like this:

1. Keep "wp-gpios"

2. Add a quirk to gpiolib-of.c that will force that as active
   low no matter what flag is specified to the GPIO descriptor.

3. If some other flag that GPIO_ACTIVE_LOW is specified,
  print a warning and say the the (default) GPIO_ACTIVE_HIGH
  i.e. 0 is gonna be ignored and we forced the line to be
  active low.

4. The code still need to be modified to set the value
   to "1" to assert the line since the gpiolib now handles
   the inversion semantics.

5. Hope that no system with an active high wp ever comes
  into existence because then we are screwed and will have
  to create a new binding and deprecate the old binding
  anyway.

Yours,
Linus Walleij

Re: [PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Linus Walleij]

On Tue, Nov 26, 2019 at 4:18 PM Khouloud Touil <ktouil@baylibre.com> wrote:

> [Me]
>> 4. The code still need to be modified to set the value
>>    to "1" to assert the line since the gpiolib now handles
>>    the inversion semantics.

> By saying "assert the wp" do you mean enable the write operation or
> block it ?

Yeah one more layer of confusion, sorry :/

By "asserting WP" I mean driving the line to a state where
writing to the EEPROM is enabled, i.e. the default state is
that the EEPROM is write protected and when you "assert"
WP it becomes writable.

If you feel the inverse semantics are more intuitive (such that
WP comes up asserted and thus write protected), be my
guest :D

As long as it is unambiguously documented in the bindings
and with comments in the code I'm game for whatever the
at24 people feel is most appropriate. (You will set the standard
for everyone else.)

Yours.
Linus Walleij

Re: [PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Bartosz Golaszewski]

czw., 28 lis 2019 o 14:45 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> On Tue, Nov 26, 2019 at 4:18 PM Khouloud Touil <ktouil@baylibre.com> wrote:
>
> > [Me]
> >> 4. The code still need to be modified to set the value
> >>    to "1" to assert the line since the gpiolib now handles
> >>    the inversion semantics.
>
> > By saying "assert the wp" do you mean enable the write operation or
> > block it ?
>
> Yeah one more layer of confusion, sorry :/
>
> By "asserting WP" I mean driving the line to a state where
> writing to the EEPROM is enabled, i.e. the default state is
> that the EEPROM is write protected and when you "assert"
> WP it becomes writable.
>
> If you feel the inverse semantics are more intuitive (such that
> WP comes up asserted and thus write protected), be my
> guest :D
>

Ha! I've always assumed that "to assert the write-protect pin" means
to *protect* the EEPROM from writing. That's why it comes up as
asserted (logical '1' in the driver) and we need to deassert it (drive
it low, logical '0' in the driver) to enable writing. This is the
current behavior and I'd say in this case it's just a matter of very
explicit statement that this is how it works in the DT binding?

Rob: any thoughts on this?

Bartosz

> As long as it is unambiguously documented in the bindings
> and with comments in the code I'm game for whatever the
> at24 people feel is most appropriate. (You will set the standard
> for everyone else.)
>
> Yours.
> Linus Walleij

Re: [PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Rob Herring]

On Fri, Nov 29, 2019 at 09:47:01AM +0100, Bartosz Golaszewski wrote:
> czw., 28 lis 2019 o 14:45 Linus Walleij <linus.walleij@linaro.org> napisał(a):
> >
> > On Tue, Nov 26, 2019 at 4:18 PM Khouloud Touil <ktouil@baylibre.com> wrote:
> >
> > > [Me]
> > >> 4. The code still need to be modified to set the value
> > >>    to "1" to assert the line since the gpiolib now handles
> > >>    the inversion semantics.
> >
> > > By saying "assert the wp" do you mean enable the write operation or
> > > block it ?
> >
> > Yeah one more layer of confusion, sorry :/
> >
> > By "asserting WP" I mean driving the line to a state where
> > writing to the EEPROM is enabled, i.e. the default state is
> > that the EEPROM is write protected and when you "assert"
> > WP it becomes writable.
> >
> > If you feel the inverse semantics are more intuitive (such that
> > WP comes up asserted and thus write protected), be my
> > guest :D
> >
> 
> Ha! I've always assumed that "to assert the write-protect pin" means
> to *protect* the EEPROM from writing. That's why it comes up as
> asserted (logical '1' in the driver) and we need to deassert it (drive
> it low, logical '0' in the driver) to enable writing. This is the
> current behavior and I'd say in this case it's just a matter of very
> explicit statement that this is how it works in the DT binding?
> 
> Rob: any thoughts on this?

I agree with you. If it was called write-enable-gpios, then assert would 
be to enable writing.

Rob

Re: [PATCH 1/4] dt-bindings: nvmem: new optional property write-protect-gpios

[Peter Rosin]

On 2019-11-22 13:41, Linus Walleij wrote:
> Hi Khouloud,
> 
> thanks for your patch!
> 
> I just have a semantic comment:
> 
> On Wed, Nov 20, 2019 at 3:21 PM Khouloud Touil <ktouil@baylibre.com> wrote:
> 
>> 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.
> 
> It is claimed that this should be pulled low to assert it so by
> definition it is active low.
> 
>> +  wp-gpios:
>> +    description:
>> +      GPIO to which the write-protect pin of the chip is connected.
>> +    maxItems: 1
> 
> Mandate that the flag in the second cell should be GPIO_ACTIVE_LOW

What if something along that way from CPU to chip inverts the signal such
that the signal is no longer active-low when viewed from the CPU, even if
it still is active low when looking at the chip only?

Yes, these things happen for all kinds of hysterical reasons...

Cheers,
Peter

> 
>>  patternProperties:
>>    "^.*@[0-9a-f]+$":
>>      type: object
>> @@ -66,6 +71,7 @@ examples:
>>        qfprom: eeprom@700000 {
>>            #address-cells = <1>;
>>            #size-cells = <1>;
>> +          wp-gpios = <&gpio1 3 0>;
> 
> #include <dt-bindings/gpio/gpio.h>
> wp-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> 
> This will in Linux have the semantic effect that you need to
> set the output high with gpio_set_val(d, 1) to assert it
> (drive it low) but that really doesn't matter to the device tree
> bindings, those are OS-agnostic: if the line is active low then
> it should use this flag.
> 
> It has the upside that the day you need a write-protect that
> is active high, it is simple to support that use case too.
> 
> Yours,
> Linus Walleij
> 

[PATCH 2/4] nvmem: add support for the write-protect pin

[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>
---
 drivers/nvmem/core.c           | 20 ++++++++++++++++++--
 drivers/nvmem/nvmem.h          |  2 ++
 include/linux/nvmem-provider.h |  3 +++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 057d1ff87d5d..ae6c3455eb11 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;
 }
@@ -365,6 +372,15 @@ 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

Re: [PATCH 2/4] nvmem: add support for the write-protect pin

[Linus Walleij]

Hi Khouloud,

more comments!

On Wed, Nov 20, 2019 at 3:21 PM Khouloud Touil <ktouil@baylibre.com> wrote:

> +       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;
> +       }

Since I requested that the GPIO line shall be flagged as
active low in the device tree, make sure to invert this
and toss in a comment:

/*
 * We assert and deassert the write protection GPIO line.
 * This line is often active low, but that semantic is handled
 * in gpiolib in respons to flags in the machine description,
 * such as the device tree or ACPI.
 */
gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
ret = nvmem->reg_write(nvmem->priv, offset, val, bytes);
gpiod_set_value_cansleep(nvmem->wp_gpio, 0);

> @@ -365,6 +372,15 @@ 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);

GPIOD_OUT_LOW as it will be inverted.

Apart from this I like the idea in this patch!

Yours,
Linus Walleij

[PATCH 3/4] dt-bindings: at24: remove the optional property write-protect-gpios

[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>
---
 Documentation/devicetree/bindings/eeprom/at24.yaml | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
index e8778560d966..9894237ac432 100644
--- a/Documentation/devicetree/bindings/eeprom/at24.yaml
+++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
@@ -145,11 +145,6 @@ 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
-
   address-width:
     allOf:
       - $ref: /schemas/types.yaml#/definitions/uint32
@@ -181,7 +176,6 @@ examples:
           compatible = "microchip,24c32", "atmel,24c32";
           reg = <0x52>;
           pagesize = <32>;
-          wp-gpios = <&gpio1 3 0>;
           num-addresses = <8>;
       };
     };
-- 
2.17.1

Re: [PATCH 3/4] dt-bindings: at24: remove the optional property write-protect-gpios

[Rob Herring]

On Wed, Nov 20, 2019 at 03:20:37PM +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>
> ---
>  Documentation/devicetree/bindings/eeprom/at24.yaml | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> index e8778560d966..9894237ac432 100644
> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> @@ -145,11 +145,6 @@ 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
> -

At least 'wp-gpios: true' is still needed or you need to reference the 
common definition in nvmem.yaml. The reason being is every possible 
property (and child node) needs to be listed so we can have errors on 
any that aren't.

Rob

[PATCH 4/4] eeprom: at24: remove the write-protect pin support

[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>
---
 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 2cccd82a3106..eec2a34b7051 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