[PATCH 0/3] eeprom: at25: Add DT support for 25lc040

[PATCH 0/3] eeprom: at25: Add DT support for 25lc040

[Geert Uytterhoeven]

	Hi all,

Some "atmel,at25" compatible SPI EEPROMs (e.g. Microchip 25lc040) use an
odd number of address bits.  This patch series adds support for
instantiating such devices from DT.

Do EEPROMs using 17 or 25 address bits, as mentioned in 
include/linux/spi/eeprom.h, really exist?
Or should we just limit it to a single odd value (9 bits)?

This has been tested with a bunch of 25lc040 EEPROMs.

Thanks!

Geert Uytterhoeven (3):
  eeprom: at25: Add DT support for EEPROMs with odd address bits
  dt-bindings: eeprom: at25: Grammar s/are can/can/
  dt-bindings: eeprom: at25: Document device-specific compatible values

 Documentation/devicetree/bindings/eeprom/at25.txt | 17 ++++++++++++-----
 drivers/misc/eeprom/at25.c                        |  4 ++++
 2 files changed, 16 insertions(+), 5 deletions(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] eeprom: at25: Add DT support for EEPROMs with odd address bits

[Geert Uytterhoeven]

Certain EEPROMS have a size that is larger than the number of address
bytes would allow, and store the MSB of the address in bit 3 of the
instruction byte.

This can be described in platform data using EE_INSTR_BIT3_IS_ADDR, or
in DT using the obsolete legacy "at25,addr-mode" property.
But currently there exists no non-deprecated way to describe this in DT.

Hence extend the existing "address-width" DT property to allow
specifying 9, 17, or 25 address bits, and enable support for that in the
driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
EEPROMs using 9 address bits are common (e.g. M95040, 25AA040/25LC040).
Do EEPROMs using 17 or 25 address bits, as mentioned in
include/linux/spi/eeprom.h, really exist?
Or should we just limit it to a single odd value (9 bits)?
---
 Documentation/devicetree/bindings/eeprom/at25.txt | 4 +++-
 drivers/misc/eeprom/at25.c                        | 4 ++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/eeprom/at25.txt b/Documentation/devicetree/bindings/eeprom/at25.txt
index 1d3447165c374f67..d00779e4ab4377b9 100644
--- a/Documentation/devicetree/bindings/eeprom/at25.txt
+++ b/Documentation/devicetree/bindings/eeprom/at25.txt
@@ -6,7 +6,9 @@ Required properties:
 - spi-max-frequency : max spi frequency to use
 - pagesize : size of the eeprom page
 - size : total eeprom size in bytes
-- address-width : number of address bits (one of 8, 16, or 24)
+- address-width : number of address bits (one of 8, 9, 16, 17, 24, or 25).
+  For odd values, the MSB of the address is sent as bit 3 of the instruction
+  byte, before the address byte(s).
 
 Optional properties:
 - spi-cpha : SPI shifted clock phase, as per spi-bus bindings.
diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index 5afe4cd165699060..a50a0f16fa0e1d1d 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -275,6 +275,10 @@ static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
 				"Error: missing \"address-width\" property\n");
 			return -ENODEV;
 		}
+		if (val & 1) {
+			chip->flags |= EE_INSTR_BIT3_IS_ADDR;
+			val -= 1;
+		}
 		switch (val) {
 		case 8:
 			chip->flags |= EE_ADDR1;
-- 
2.7.4

Re: [PATCH 1/3] eeprom: at25: Add DT support for EEPROMs with odd address bits

[Geert Uytterhoeven]

On Thu, Nov 30, 2017 at 2:29 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> Certain EEPROMS have a size that is larger than the number of address
> bytes would allow, and store the MSB of the address in bit 3 of the
> instruction byte.
>
> This can be described in platform data using EE_INSTR_BIT3_IS_ADDR, or
> in DT using the obsolete legacy "at25,addr-mode" property.
> But currently there exists no non-deprecated way to describe this in DT.
>
> Hence extend the existing "address-width" DT property to allow
> specifying 9, 17, or 25 address bits, and enable support for that in the
> driver.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> EEPROMs using 9 address bits are common (e.g. M95040, 25AA040/25LC040).
> Do EEPROMs using 17 or 25 address bits, as mentioned in
> include/linux/spi/eeprom.h, really exist?
> Or should we just limit it to a single odd value (9 bits)?

At least for the real Atmel parts, only the AT25040 part uses odd (8 +
1 bit) addressing.
AT25M01 uses 3-byte addressing (it needs 17 bits).

So I tend to believe EEPROMs using 16 + 1  or 24 + 1 address bits (with the
extra bit in the instruction byte) do not exist?

> ---
>  Documentation/devicetree/bindings/eeprom/at25.txt | 4 +++-
>  drivers/misc/eeprom/at25.c                        | 4 ++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/eeprom/at25.txt b/Documentation/devicetree/bindings/eeprom/at25.txt
> index 1d3447165c374f67..d00779e4ab4377b9 100644
> --- a/Documentation/devicetree/bindings/eeprom/at25.txt
> +++ b/Documentation/devicetree/bindings/eeprom/at25.txt
> @@ -6,7 +6,9 @@ Required properties:
>  - spi-max-frequency : max spi frequency to use
>  - pagesize : size of the eeprom page
>  - size : total eeprom size in bytes
> -- address-width : number of address bits (one of 8, 16, or 24)
> +- address-width : number of address bits (one of 8, 9, 16, 17, 24, or 25).
> +  For odd values, the MSB of the address is sent as bit 3 of the instruction
> +  byte, before the address byte(s).

Alternatively, we can drop the binding change, i.e. keep on using
address-width = <8> for 512-byte '040...

>  Optional properties:
>  - spi-cpha : SPI shifted clock phase, as per spi-bus bindings.
> diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
> index 5afe4cd165699060..a50a0f16fa0e1d1d 100644
> --- a/drivers/misc/eeprom/at25.c
> +++ b/drivers/misc/eeprom/at25.c
> @@ -275,6 +275,10 @@ static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
>                                 "Error: missing \"address-width\" property\n");
>                         return -ENODEV;
>                 }
> +               if (val & 1) {
> +                       chip->flags |= EE_INSTR_BIT3_IS_ADDR;
> +                       val -= 1;
> +               }

... and handle it here like:

        if (chip->byte_len == 2U << val)
                chip->flags |= EE_INSTR_BIT3_IS_ADDR;

However, that would IMHO be a bit confusing, as the "address-width"
property is no longer the real address width, but indicates how many bits
are specified in address bytes sent after the read/write command.
So "address-bytes" = 1, 2, or 3 would be more correct ;-)

Or deprecate this whole "specify parameters using DT properties" business,
and derive them from the compatible value. But that would mean adding a
large and ever growing table to an old driver...

Thoughts?

Thanks again!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] eeprom: at25: Add DT support for EEPROMs with odd address bits

[Rob Herring]

On Mon, Dec 04, 2017 at 10:17:47AM +0100, Geert Uytterhoeven wrote:
> On Thu, Nov 30, 2017 at 2:29 PM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > Certain EEPROMS have a size that is larger than the number of address
> > bytes would allow, and store the MSB of the address in bit 3 of the
> > instruction byte.
> >
> > This can be described in platform data using EE_INSTR_BIT3_IS_ADDR, or
> > in DT using the obsolete legacy "at25,addr-mode" property.
> > But currently there exists no non-deprecated way to describe this in DT.
> >
> > Hence extend the existing "address-width" DT property to allow
> > specifying 9, 17, or 25 address bits, and enable support for that in the
> > driver.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > EEPROMs using 9 address bits are common (e.g. M95040, 25AA040/25LC040).
> > Do EEPROMs using 17 or 25 address bits, as mentioned in
> > include/linux/spi/eeprom.h, really exist?
> > Or should we just limit it to a single odd value (9 bits)?
> 
> At least for the real Atmel parts, only the AT25040 part uses odd (8 +
> 1 bit) addressing.

Seems like we should have a specific compatible for it.

> AT25M01 uses 3-byte addressing (it needs 17 bits).

Do you need to know it is 17-bit vs. 24-bits? I'm guessing not as the 
unused bits are probably don't care.

Rob

Re: [PATCH 1/3] eeprom: at25: Add DT support for EEPROMs with odd address bits

[Geert Uytterhoeven]

Hi Rob,

On Mon, Dec 4, 2017 at 10:17 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Dec 04, 2017 at 10:17:47AM +0100, Geert Uytterhoeven wrote:
>> On Thu, Nov 30, 2017 at 2:29 PM, Geert Uytterhoeven
>> <geert+renesas@glider.be> wrote:
>> > Certain EEPROMS have a size that is larger than the number of address
>> > bytes would allow, and store the MSB of the address in bit 3 of the
>> > instruction byte.
>> >
>> > This can be described in platform data using EE_INSTR_BIT3_IS_ADDR, or
>> > in DT using the obsolete legacy "at25,addr-mode" property.
>> > But currently there exists no non-deprecated way to describe this in DT.
>> >
>> > Hence extend the existing "address-width" DT property to allow
>> > specifying 9, 17, or 25 address bits, and enable support for that in the
>> > driver.
>> >
>> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> > ---
>> > EEPROMs using 9 address bits are common (e.g. M95040, 25AA040/25LC040).
>> > Do EEPROMs using 17 or 25 address bits, as mentioned in
>> > include/linux/spi/eeprom.h, really exist?
>> > Or should we just limit it to a single odd value (9 bits)?
>>
>> At least for the real Atmel parts, only the AT25040 part uses odd (8 +
>> 1 bit) addressing.
>
> Seems like we should have a specific compatible for it.

Possibly. But currently all configuration is done through DT properties, not
through matching on compatible values.

>> AT25M01 uses 3-byte addressing (it needs 17 bits).
>
> Do you need to know it is 17-bit vs. 24-bits? I'm guessing not as the
> unused bits are probably don't care.

The 17 bits can be derived from the EEPROM size in bytes (1 Mb = 128 KiB).
What is important to know is how to pass addresses to the device:
  1. 3 address bytes, OR
  2. 2 address bytes, and the odd MSB bit in the command byte.

But apparently the second scheme is not used for 17-bit addressing.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] eeprom: at25: Add DT support for EEPROMs with odd address bits

[Geert Uytterhoeven]

Hi Rob,

On Tue, Dec 5, 2017 at 9:57 AM, Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Mon, Dec 4, 2017 at 10:17 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Mon, Dec 04, 2017 at 10:17:47AM +0100, Geert Uytterhoeven wrote:
>>> On Thu, Nov 30, 2017 at 2:29 PM, Geert Uytterhoeven
>>> <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org> wrote:
>>> > Certain EEPROMS have a size that is larger than the number of address
>>> > bytes would allow, and store the MSB of the address in bit 3 of the
>>> > instruction byte.
>>> >
>>> > This can be described in platform data using EE_INSTR_BIT3_IS_ADDR, or
>>> > in DT using the obsolete legacy "at25,addr-mode" property.
>>> > But currently there exists no non-deprecated way to describe this in DT.
>>> >
>>> > Hence extend the existing "address-width" DT property to allow
>>> > specifying 9, 17, or 25 address bits, and enable support for that in the
>>> > driver.
>>> >
>>> > Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>>> > ---
>>> > EEPROMs using 9 address bits are common (e.g. M95040, 25AA040/25LC040).
>>> > Do EEPROMs using 17 or 25 address bits, as mentioned in
>>> > include/linux/spi/eeprom.h, really exist?
>>> > Or should we just limit it to a single odd value (9 bits)?
>>>
>>> At least for the real Atmel parts, only the AT25040 part uses odd (8 +
>>> 1 bit) addressing.
>>
>> Seems like we should have a specific compatible for it.
>
> Possibly. But currently all configuration is done through DT properties, not
> through matching on compatible values.

Adding compatible values for all known/used parts could quickly become a
large table.
E.g. Atmel/Microchip has 3 variants of 512-byte EEPROMs: AT25040B,
25LC040A, and 25AA040A. The former uses an 8-byte pagesize, while the
latter parts use 16-byte pagesizes.
Not to mention "compatible" parts from other manufacturers, and all other
supported size.

Currently all of this is configured through the "pagesize", "size", and
"address-width" DT properties, with matching on generic "atmel,at25".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] eeprom: at25: Add DT support for EEPROMs with odd address bits

[Rob Herring]

On Tue, Dec 5, 2017 at 3:09 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Rob,
>
> On Tue, Dec 5, 2017 at 9:57 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Mon, Dec 4, 2017 at 10:17 PM, Rob Herring <robh@kernel.org> wrote:
>>> On Mon, Dec 04, 2017 at 10:17:47AM +0100, Geert Uytterhoeven wrote:
>>>> On Thu, Nov 30, 2017 at 2:29 PM, Geert Uytterhoeven
>>>> <geert+renesas@glider.be> wrote:
>>>> > Certain EEPROMS have a size that is larger than the number of address
>>>> > bytes would allow, and store the MSB of the address in bit 3 of the
>>>> > instruction byte.
>>>> >
>>>> > This can be described in platform data using EE_INSTR_BIT3_IS_ADDR, or
>>>> > in DT using the obsolete legacy "at25,addr-mode" property.
>>>> > But currently there exists no non-deprecated way to describe this in DT.
>>>> >
>>>> > Hence extend the existing "address-width" DT property to allow
>>>> > specifying 9, 17, or 25 address bits, and enable support for that in the
>>>> > driver.
>>>> >
>>>> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> > ---
>>>> > EEPROMs using 9 address bits are common (e.g. M95040, 25AA040/25LC040).
>>>> > Do EEPROMs using 17 or 25 address bits, as mentioned in
>>>> > include/linux/spi/eeprom.h, really exist?
>>>> > Or should we just limit it to a single odd value (9 bits)?
>>>>
>>>> At least for the real Atmel parts, only the AT25040 part uses odd (8 +
>>>> 1 bit) addressing.
>>>
>>> Seems like we should have a specific compatible for it.
>>
>> Possibly. But currently all configuration is done through DT properties, not
>> through matching on compatible values.
>
> Adding compatible values for all known/used parts could quickly become a
> large table.
> E.g. Atmel/Microchip has 3 variants of 512-byte EEPROMs: AT25040B,
> 25LC040A, and 25AA040A. The former uses an 8-byte pagesize, while the
> latter parts use 16-byte pagesizes.
> Not to mention "compatible" parts from other manufacturers, and all other
> supported size.
>
> Currently all of this is configured through the "pagesize", "size", and
> "address-width" DT properties, with matching on generic "atmel,at25".

I wasn't suggesting throwing out all these. Just add a compatible for
the one oddball 9-bit part.

But I'm fine adding address-width=9 too.

Rob

Re: [PATCH 1/3] eeprom: at25: Add DT support for EEPROMs with odd address bits

[Geert Uytterhoeven]

Hi Rob,

On Tue, Dec 5, 2017 at 2:56 PM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Dec 5, 2017 at 3:09 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Tue, Dec 5, 2017 at 9:57 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> On Mon, Dec 4, 2017 at 10:17 PM, Rob Herring <robh@kernel.org> wrote:
>>>> On Mon, Dec 04, 2017 at 10:17:47AM +0100, Geert Uytterhoeven wrote:
>>>>> On Thu, Nov 30, 2017 at 2:29 PM, Geert Uytterhoeven
>>>>> <geert+renesas@glider.be> wrote:
>>>>> > Certain EEPROMS have a size that is larger than the number of address
>>>>> > bytes would allow, and store the MSB of the address in bit 3 of the
>>>>> > instruction byte.
>>>>> >
>>>>> > This can be described in platform data using EE_INSTR_BIT3_IS_ADDR, or
>>>>> > in DT using the obsolete legacy "at25,addr-mode" property.
>>>>> > But currently there exists no non-deprecated way to describe this in DT.
>>>>> >
>>>>> > Hence extend the existing "address-width" DT property to allow
>>>>> > specifying 9, 17, or 25 address bits, and enable support for that in the
>>>>> > driver.
>>>>> >
>>>>> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>> > ---
>>>>> > EEPROMs using 9 address bits are common (e.g. M95040, 25AA040/25LC040).
>>>>> > Do EEPROMs using 17 or 25 address bits, as mentioned in
>>>>> > include/linux/spi/eeprom.h, really exist?
>>>>> > Or should we just limit it to a single odd value (9 bits)?
>>>>>
>>>>> At least for the real Atmel parts, only the AT25040 part uses odd (8 +
>>>>> 1 bit) addressing.
>>>>
>>>> Seems like we should have a specific compatible for it.
>>>
>>> Possibly. But currently all configuration is done through DT properties, not
>>> through matching on compatible values.
>>
>> Adding compatible values for all known/used parts could quickly become a
>> large table.
>> E.g. Atmel/Microchip has 3 variants of 512-byte EEPROMs: AT25040B,
>> 25LC040A, and 25AA040A. The former uses an 8-byte pagesize, while the
>> latter parts use 16-byte pagesizes.
>> Not to mention "compatible" parts from other manufacturers, and all other
>> supported size.
>>
>> Currently all of this is configured through the "pagesize", "size", and
>> "address-width" DT properties, with matching on generic "atmel,at25".
>
> I wasn't suggesting throwing out all these. Just add a compatible for
> the one oddball 9-bit part.
>
> But I'm fine adding address-width=9 too.

OK. Then I'll go for the least intrusive solution (address-width=9).
These EEPROMs are fairly small and simple, and I can imagine them being
used on small systems too, so driver code/data size matters.

Stay tuned for v2.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] eeprom: at25: Add DT support for EEPROMs with odd address bits

[Ivo Sieben]

Hi Geert,

My 2 cents:

2017-12-04 10:17 GMT+01:00 Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>:
>> EEPROMs using 9 address bits are common (e.g. M95040, 25AA040/25LC040).
>> Do EEPROMs using 17 or 25 address bits, as mentioned in
>> include/linux/spi/eeprom.h, really exist?
>> Or should we just limit it to a single odd value (9 bits)?
>
> At least for the real Atmel parts, only the AT25040 part uses odd (8 +
> 1 bit) addressing.
> AT25M01 uses 3-byte addressing (it needs 17 bits).
>
> So I tend to believe EEPROMs using 16 + 1  or 24 + 1 address bits (with the
> extra bit in the instruction byte) do not exist?
>

I think you are right. Most likely this extra address bit option is
only used for 9 bit addressable chips.
I'm not an expert, but I know only the M95040 chip for which I
originally wrote the patch.
By then I decided to make it a bit broader (so also to be used as
address 17 & 25 bit addressing) but that might
not make any sense indeed.

>> @@ -6,7 +6,9 @@ Required properties:
>>  - spi-max-frequency : max spi frequency to use
>>  - pagesize : size of the eeprom page
>>  - size : total eeprom size in bytes
>> -- address-width : number of address bits (one of 8, 16, or 24)
>> +- address-width : number of address bits (one of 8, 9, 16, 17, 24, or 25).
>> +  For odd values, the MSB of the address is sent as bit 3 of the instruction
>> +  byte, before the address byte(s).
>
> Alternatively, we can drop the binding change, i.e. keep on using
> address-width = <8> for 512-byte '040...
>

As you also stated before: maybe it is more clear to leave only the
"9" value option documented
here, that looks to me the only valid use case for it.

>> +               if (val & 1) {
>> +                       chip->flags |= EE_INSTR_BIT3_IS_ADDR;
>> +                       val -= 1;
>> +               }
>
> ... and handle it here like:
>
>         if (chip->byte_len == 2U << val)
>                 chip->flags |= EE_INSTR_BIT3_IS_ADDR;
>
> However, that would IMHO be a bit confusing, as the "address-width"
> property is no longer the real address width, but indicates how many bits
> are specified in address bytes sent after the read/write command.
> So "address-bytes" = 1, 2, or 3 would be more correct ;-)
>
> Or deprecate this whole "specify parameters using DT properties" business,
> and derive them from the compatible value. But that would mean adding a
> large and ever growing table to an old driver...
>
> Thoughts?

I'm not a DT expert but to me your first proposal makes the most sense
to me and feels the most intuitive:
I would go for the address-with value 9 option here.

Since we only expect value 9 to be a valid option, maybe you could
rewrite it a bit to explicitly check for value 9:

if (val == 9) {
        chip->flags |= EE_INSTR_BIT3_IS_ADDR;
        val -= 1;
}

I think this is slightly more readable.

Hope this helps,

Regards,
Ivo Sieben
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] eeprom: at25: Add DT support for EEPROMs with odd address bits

[Geert Uytterhoeven]

Hi Ivo,

On Mon, Dec 4, 2017 at 11:00 PM, Ivo Sieben <meltedpianoman@gmail.com> wrote:
> 2017-12-04 10:17 GMT+01:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>>> EEPROMs using 9 address bits are common (e.g. M95040, 25AA040/25LC040).
>>> Do EEPROMs using 17 or 25 address bits, as mentioned in
>>> include/linux/spi/eeprom.h, really exist?
>>> Or should we just limit it to a single odd value (9 bits)?
>>
>> At least for the real Atmel parts, only the AT25040 part uses odd (8 +
>> 1 bit) addressing.
>> AT25M01 uses 3-byte addressing (it needs 17 bits).
>>
>> So I tend to believe EEPROMs using 16 + 1  or 24 + 1 address bits (with the
>> extra bit in the instruction byte) do not exist?
>>
>
> I think you are right. Most likely this extra address bit option is
> only used for 9 bit addressable chips.
> I'm not an expert, but I know only the M95040 chip for which I
> originally wrote the patch.
> By then I decided to make it a bit broader (so also to be used as
> address 17 & 25 bit addressing) but that might
> not make any sense indeed.
>
>>> @@ -6,7 +6,9 @@ Required properties:
>>>  - spi-max-frequency : max spi frequency to use
>>>  - pagesize : size of the eeprom page
>>>  - size : total eeprom size in bytes
>>> -- address-width : number of address bits (one of 8, 16, or 24)
>>> +- address-width : number of address bits (one of 8, 9, 16, 17, 24, or 25).
>>> +  For odd values, the MSB of the address is sent as bit 3 of the instruction
>>> +  byte, before the address byte(s).
>>
>> Alternatively, we can drop the binding change, i.e. keep on using
>> address-width = <8> for 512-byte '040...
>>
>
> As you also stated before: maybe it is more clear to leave only the
> "9" value option documented
> here, that looks to me the only valid use case for it.

OK.

>
>>> +               if (val & 1) {
>>> +                       chip->flags |= EE_INSTR_BIT3_IS_ADDR;
>>> +                       val -= 1;
>>> +               }
>>
>> ... and handle it here like:
>>
>>         if (chip->byte_len == 2U << val)
>>                 chip->flags |= EE_INSTR_BIT3_IS_ADDR;
>>
>> However, that would IMHO be a bit confusing, as the "address-width"
>> property is no longer the real address width, but indicates how many bits
>> are specified in address bytes sent after the read/write command.
>> So "address-bytes" = 1, 2, or 3 would be more correct ;-)
>>
>> Or deprecate this whole "specify parameters using DT properties" business,
>> and derive them from the compatible value. But that would mean adding a
>> large and ever growing table to an old driver...
>>
>> Thoughts?
>
> I'm not a DT expert but to me your first proposal makes the most sense
> to me and feels the most intuitive:
> I would go for the address-with value 9 option here.

OK.

> Since we only expect value 9 to be a valid option, maybe you could
> rewrite it a bit to explicitly check for value 9:
>
> if (val == 9) {
>         chip->flags |= EE_INSTR_BIT3_IS_ADDR;
>         val -= 1;
> }
>
> I think this is slightly more readable.

Sure.

> Hope this helps,

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 2/3] dt-bindings: eeprom: at25: Grammar s/are can/can/

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
 Documentation/devicetree/bindings/eeprom/at25.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/eeprom/at25.txt b/Documentation/devicetree/bindings/eeprom/at25.txt
index d00779e4ab4377b9..9b1782809386844d 100644
--- a/Documentation/devicetree/bindings/eeprom/at25.txt
+++ b/Documentation/devicetree/bindings/eeprom/at25.txt
@@ -15,7 +15,7 @@ Optional properties:
 - spi-cpol : SPI inverse clock polarity, as per spi-bus bindings.
 - read-only : this parameter-less property disables writes to the eeprom
 
-Obsolete legacy properties are can be used in place of "size", "pagesize",
+Obsolete legacy properties can be used in place of "size", "pagesize",
 "address-width", and "read-only":
 - at25,byte-len : total eeprom size in bytes
 - at25,addr-mode : addr-mode flags, as defined in include/linux/spi/eeprom.h
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] eeprom: at25: Add DT support for 25lc040

[Rob Herring]

On Thu, Nov 30, 2017 at 02:29:43PM +0100, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> Some "atmel,at25" compatible SPI EEPROMs (e.g. Microchip 25lc040) use an
> odd number of address bits.  This patch series adds support for
> instantiating such devices from DT.
> 
> Do EEPROMs using 17 or 25 address bits, as mentioned in 
> include/linux/spi/eeprom.h, really exist?
> Or should we just limit it to a single odd value (9 bits)?
> 
> This has been tested with a bunch of 25lc040 EEPROMs.
> 
> Thanks!
> 
> Geert Uytterhoeven (3):
>   eeprom: at25: Add DT support for EEPROMs with odd address bits
>   dt-bindings: eeprom: at25: Grammar s/are can/can/
>   dt-bindings: eeprom: at25: Document device-specific compatible values

2 and 3 are fixes and I'll apply for 4.15 if you don't mind.

Rob

> 
>  Documentation/devicetree/bindings/eeprom/at25.txt | 17 ++++++++++++-----
>  drivers/misc/eeprom/at25.c                        |  4 ++++
>  2 files changed, 16 insertions(+), 5 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] eeprom: at25: Add DT support for 25lc040

[Geert Uytterhoeven]

Hi Rob,

On Mon, Dec 4, 2017 at 10:22 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Nov 30, 2017 at 02:29:43PM +0100, Geert Uytterhoeven wrote:
>> Some "atmel,at25" compatible SPI EEPROMs (e.g. Microchip 25lc040) use an
>> odd number of address bits.  This patch series adds support for
>> instantiating such devices from DT.
>>
>> Do EEPROMs using 17 or 25 address bits, as mentioned in
>> include/linux/spi/eeprom.h, really exist?
>> Or should we just limit it to a single odd value (9 bits)?
>>
>> This has been tested with a bunch of 25lc040 EEPROMs.
>>
>> Thanks!
>>
>> Geert Uytterhoeven (3):
>>   eeprom: at25: Add DT support for EEPROMs with odd address bits
>>   dt-bindings: eeprom: at25: Grammar s/are can/can/
>>   dt-bindings: eeprom: at25: Document device-specific compatible values
>
> 2 and 3 are fixes and I'll apply for 4.15 if you don't mind.

Thanks, I don't mind.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] eeprom: at25: Add DT support for 25lc040

[Rob Herring]

On Tue, Dec 05, 2017 at 10:04:21AM +0100, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Mon, Dec 4, 2017 at 10:22 PM, Rob Herring <robh@kernel.org> wrote:
> > On Thu, Nov 30, 2017 at 02:29:43PM +0100, Geert Uytterhoeven wrote:
> >> Some "atmel,at25" compatible SPI EEPROMs (e.g. Microchip 25lc040) use an
> >> odd number of address bits.  This patch series adds support for
> >> instantiating such devices from DT.
> >>
> >> Do EEPROMs using 17 or 25 address bits, as mentioned in
> >> include/linux/spi/eeprom.h, really exist?
> >> Or should we just limit it to a single odd value (9 bits)?
> >>
> >> This has been tested with a bunch of 25lc040 EEPROMs.
> >>
> >> Thanks!
> >>
> >> Geert Uytterhoeven (3):
> >>   eeprom: at25: Add DT support for EEPROMs with odd address bits
> >>   dt-bindings: eeprom: at25: Grammar s/are can/can/
> >>   dt-bindings: eeprom: at25: Document device-specific compatible values
> >
> > 2 and 3 are fixes and I'll apply for 4.15 if you don't mind.
> 
> Thanks, I don't mind.

Both applied.

Rob

[PATCH 3/3] dt-bindings: eeprom: at25: Document device-specific compatible values

[Geert Uytterhoeven]

Document the recommended presence of a device-specific compatible value,
and list examples that are already in use or soon will be.
This will allow checkpatch to validate compatible values in DTS.

Update the example to match current best practices (generic node name,
specific compatible value first).

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/eeprom/at25.txt | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/eeprom/at25.txt b/Documentation/devicetree/bindings/eeprom/at25.txt
index 9b1782809386844d..7129affb2956bb0d 100644
--- a/Documentation/devicetree/bindings/eeprom/at25.txt
+++ b/Documentation/devicetree/bindings/eeprom/at25.txt
@@ -1,7 +1,12 @@
 EEPROMs (SPI) compatible with Atmel at25.
 
 Required properties:
-- compatible : "atmel,at25".
+- compatible : Should be "<vendor>,<type>", and generic value "atmel,at25".
+  Example "<vendor>,<type>" values:
+    "microchip,25lc040"
+    "st,m95m02"
+    "st,m95256"
+
 - reg : chip select number
 - spi-max-frequency : max spi frequency to use
 - pagesize : size of the eeprom page
@@ -24,8 +29,8 @@ Obsolete legacy properties can be used in place of "size", "pagesize",
 Additional compatible properties are also allowed.
 
 Example:
-	at25@0 {
-		compatible = "atmel,at25", "st,m95256";
+	eeprom@0 {
+		compatible = "st,m95256", "atmel,at25";
 		reg = <0>
 		spi-max-frequency = <5000000>;
 		spi-cpha;
-- 
2.7.4