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