mbox series

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

Message ID 20191210154157.21930-1-ktouil@baylibre.com
Headers show
Series at24: move write-protect pin handling to nvmem core | expand

Message

Khouloud Touil Dec. 10, 2019, 3:41 p.m. UTC
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

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      |  9 +++++++++
 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, 32 insertions(+), 16 deletions(-)

Comments

Linus Walleij Dec. 16, 2019, 8:09 a.m. UTC | #1
On Tue, Dec 10, 2019 at 4:42 PM Khouloud Touil <ktouil@baylibre.com> wrote:

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

This is consistent IMO, we just specify that WP is active high
as in "when it is high, it actively protects against writing", so:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Linus Walleij Dec. 16, 2019, 8:13 a.m. UTC | #2
On Tue, Dec 10, 2019 at 4:42 PM Khouloud Touil <ktouil@baylibre.com> 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 support for the write-protect pin after adding it to the
> NVMEM subsystem.
>
> Signed-off-by: Khouloud Touil <ktouil@baylibre.com>

I wonder if this needs to be in the same patch that adds it to
the NVMEM subsystem, so as to avoid both code paths being
taken between the two patches (bisectability..)

However that is not the biggest thing in the universe and I'm
no bisectability-perfectionist, so:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Bartosz Golaszewski Dec. 16, 2019, 11:03 a.m. UTC | #3
pon., 16 gru 2019 o 09:13 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> On Tue, Dec 10, 2019 at 4:42 PM Khouloud Touil <ktouil@baylibre.com> 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 support for the write-protect pin after adding it to the
> > NVMEM subsystem.
> >
> > Signed-off-by: Khouloud Touil <ktouil@baylibre.com>
>
> I wonder if this needs to be in the same patch that adds it to
> the NVMEM subsystem, so as to avoid both code paths being
> taken between the two patches (bisectability..)
>
> However that is not the biggest thing in the universe and I'm
> no bisectability-perfectionist, so:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Yours,
> Linus Walleij

AFAIK Khouloud tested it and it's bisectable thanks to using the
optional gpiod_get() variant.

Best regards,
Bartosz Golaszewski
Srinivas Kandagatla Dec. 19, 2019, 10:51 a.m. UTC | #4
On 10/12/2019 15:41, Khouloud Touil wrote:
> 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
> 
> 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
> 

Thanks Khouloud for this patchset,

I can take this via nvmem tree once we get an ack on dt bindings from DT 
maintainers.


--srini
>   .../devicetree/bindings/eeprom/at24.yaml      |  6 +-----
>   .../devicetree/bindings/nvmem/nvmem.yaml      |  9 +++++++++
>   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, 32 insertions(+), 16 deletions(-)
>
Bartosz Golaszewski Dec. 19, 2019, 10:53 a.m. UTC | #5
czw., 19 gru 2019 o 11:51 Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> napisał(a):
>
>
>
> On 10/12/2019 15:41, Khouloud Touil wrote:
> > 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
> >
> > 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
> >
>
> Thanks Khouloud for this patchset,
>
> I can take this via nvmem tree once we get an ack on dt bindings from DT
> maintainers.
>

Hi Srinivas,

this will conflict with my at24 tree for this release - can you put
those patches (once they're fine) into an immutable branch for me to
merge in?

Bart

>
> --srini
> >   .../devicetree/bindings/eeprom/at24.yaml      |  6 +-----
> >   .../devicetree/bindings/nvmem/nvmem.yaml      |  9 +++++++++
> >   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, 32 insertions(+), 16 deletions(-)
> >
Srinivas Kandagatla Dec. 19, 2019, 10:56 a.m. UTC | #6
On 19/12/2019 10:53, Bartosz Golaszewski wrote:
> czw., 19 gru 2019 o 11:51 Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> napisał(a):
>>
>>
>>
>> On 10/12/2019 15:41, Khouloud Touil wrote:
>>> 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
>>>
>>> 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
>>>
>>
>> Thanks Khouloud for this patchset,
>>
>> I can take this via nvmem tree once we get an ack on dt bindings from DT
>> maintainers.
>>
> 
> Hi Srinivas,
> 
> this will conflict with my at24 tree for this release - can you put
> those patches (once they're fine) into an immutable branch for me to
> merge in?

I can ack nvmem core patch so that you can take it directly via at24 
tree if thats okay.


-srini
> 
> Bart
> 
>>
>> --srini
>>>    .../devicetree/bindings/eeprom/at24.yaml      |  6 +-----
>>>    .../devicetree/bindings/nvmem/nvmem.yaml      |  9 +++++++++
>>>    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, 32 insertions(+), 16 deletions(-)
>>>
Srinivas Kandagatla Dec. 19, 2019, 10:58 a.m. UTC | #7
On 10/12/2019 15:41, Khouloud Touil wrote:
> 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           | 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

Acked-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Bartosz Golaszewski Dec. 19, 2019, 10:59 a.m. UTC | #8
czw., 19 gru 2019 o 11:56 Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> napisał(a):
> >>
> >> Thanks Khouloud for this patchset,
> >>
> >> I can take this via nvmem tree once we get an ack on dt bindings from DT
> >> maintainers.
> >>
> >
> > Hi Srinivas,
> >
> > this will conflict with my at24 tree for this release - can you put
> > those patches (once they're fine) into an immutable branch for me to
> > merge in?
>
> I can ack nvmem core patch so that you can take it directly via at24
> tree if thats okay.
>

Sure, even better!

Bart