| Message ID | 20201209175708.16252-2-a-govindraju@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | eeprom: eeprom_93xx46: Add support for sending zero bits after address during read transfer | expand |
On Wed, Dec 09, 2020 at 11:27:07PM +0530, Aswath Govindraju wrote: > Dummy zero bits are sent before data during a read transfer. This causes > the data read to be shifted to the right. To fix this send zero bits after > the address during a read transfer. > > Add property to send zero bits after the address during a read transfer. When is this necessary? Why can't it be implied by the compatible string which should be specific to the chip model? > > Suggested-by: Vignesh Raghavendra <vigneshr@ti.com> > Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> > --- > Documentation/devicetree/bindings/misc/eeprom-93xx46.txt | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt > index a8ebb4621f79..09fb1cec54f0 100644 > --- a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt > +++ b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt > @@ -10,7 +10,9 @@ Optional properties: > - read-only : parameter-less property which disables writes to the EEPROM > - select-gpios : if present, specifies the GPIO that will be asserted prior to > each access to the EEPROM (e.g. for SPI bus multiplexing) > - > +- read-op-dummy-cycles: If present specifies the number of dummy zero-bits to > + be sent after the address during a read transfer to ignore any bits sent > + preceding the data. > Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt > apply. In particular, "reg" and "spi-max-frequency" properties must be given. > > -- > 2.17.1 >
Hi, On 11/12/20 9:03 am, Rob Herring wrote: > On Wed, Dec 09, 2020 at 11:27:07PM +0530, Aswath Govindraju wrote: >> Dummy zero bits are sent before data during a read transfer. This causes >> the data read to be shifted to the right. To fix this send zero bits after >> the address during a read transfer. >> >> Add property to send zero bits after the address during a read transfer. > > When is this necessary? Why can't it be implied by the compatible > string which should be specific to the chip model? > This is necessary for 93AA46A/B/C, 93LC46A/B/C, 93C46A/B/C eeproms, as it can be seen in section 2.7 of [1]. We were not sure if these were the only devices supported by the driver(eeprom_93xx46.c). So, in order to apply this only to the above listed devices, we thought that it would be better to apply this change when required by introducing a DT property. May I know how has this case been handled till now ?? If this is required by all the devices then we can drop the property and include the zero bit by default. [1]- https://www.mouser.com/datasheet/2/268/20001749K-277859.pdf Thanks, Aswath >> >> Suggested-by: Vignesh Raghavendra <vigneshr@ti.com> >> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> >> --- >> Documentation/devicetree/bindings/misc/eeprom-93xx46.txt | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt >> index a8ebb4621f79..09fb1cec54f0 100644 >> --- a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt >> +++ b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt >> @@ -10,7 +10,9 @@ Optional properties: >> - read-only : parameter-less property which disables writes to the EEPROM >> - select-gpios : if present, specifies the GPIO that will be asserted prior to >> each access to the EEPROM (e.g. for SPI bus multiplexing) >> - >> +- read-op-dummy-cycles: If present specifies the number of dummy zero-bits to >> + be sent after the address during a read transfer to ignore any bits sent >> + preceding the data. >> Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt >> apply. In particular, "reg" and "spi-max-frequency" properties must be given. >> >> -- >> 2.17.1 >>
On Fri, Dec 11, 2020 at 08:34:57PM +0530, Aswath Govindraju wrote: > Hi, > On 11/12/20 9:03 am, Rob Herring wrote: > > On Wed, Dec 09, 2020 at 11:27:07PM +0530, Aswath Govindraju wrote: > >> Dummy zero bits are sent before data during a read transfer. This causes > >> the data read to be shifted to the right. To fix this send zero bits after > >> the address during a read transfer. > >> > >> Add property to send zero bits after the address during a read transfer. > > > > When is this necessary? Why can't it be implied by the compatible > > string which should be specific to the chip model? > > > > This is necessary for 93AA46A/B/C, 93LC46A/B/C, 93C46A/B/C eeproms, as > it can be seen in section 2.7 of [1]. We were not sure if these were the > only devices supported by the driver(eeprom_93xx46.c). So, in order to > apply this only to the above listed devices, we thought that it would be > better to apply this change when required by introducing a DT property. > > May I know how has this case been handled till now ?? > No idea. From the at93c46d (which has a compatible string) datasheet it looks like it has the same thing. > If this is required by all the devices then we can drop the property and > include the zero bit by default. Looks like you need a combination of compatible strings for the above devices and a property for the ORG pin state on the C devices. I assume s/w needs to know if x8 or x16? Rob
Hi Rob, On 15/12/20 3:53 am, Rob Herring wrote: > On Fri, Dec 11, 2020 at 08:34:57PM +0530, Aswath Govindraju wrote: >> Hi, >> On 11/12/20 9:03 am, Rob Herring wrote: >>> On Wed, Dec 09, 2020 at 11:27:07PM +0530, Aswath Govindraju wrote: >>>> Dummy zero bits are sent before data during a read transfer. This causes >>>> the data read to be shifted to the right. To fix this send zero bits after >>>> the address during a read transfer. >>>> >>>> Add property to send zero bits after the address during a read transfer. >>> >>> When is this necessary? Why can't it be implied by the compatible >>> string which should be specific to the chip model? >>> >> >> This is necessary for 93AA46A/B/C, 93LC46A/B/C, 93C46A/B/C eeproms, as >> it can be seen in section 2.7 of [1]. We were not sure if these were the >> only devices supported by the driver(eeprom_93xx46.c). So, in order to >> apply this only to the above listed devices, we thought that it would be >> better to apply this change when required by introducing a DT property. >> >> May I know how has this case been handled till now ?? >> > > No idea. From the at93c46d (which has a compatible string) datasheet it > looks like it has the same thing. > >> If this is required by all the devices then we can drop the property and >> include the zero bit by default. > > Looks like you need a combination of compatible strings for the above > devices and a property for the ORG pin state on the C devices. I assume > s/w needs to know if x8 or x16? > Yes, there are separate properties for indicating different types of types of eeproms. So, do you think that it is better to add it as a seperate property?? Thanks, Aswath > Rob >
Hi Rob, On 15/12/20 9:42 pm, Aswath Govindraju wrote: > Hi Rob, > On 15/12/20 3:53 am, Rob Herring wrote: >> On Fri, Dec 11, 2020 at 08:34:57PM +0530, Aswath Govindraju wrote: >>> Hi, >>> On 11/12/20 9:03 am, Rob Herring wrote: >>>> On Wed, Dec 09, 2020 at 11:27:07PM +0530, Aswath Govindraju wrote: >>>>> Dummy zero bits are sent before data during a read transfer. This causes >>>>> the data read to be shifted to the right. To fix this send zero bits after >>>>> the address during a read transfer. >>>>> >>>>> Add property to send zero bits after the address during a read transfer. >>>> >>>> When is this necessary? Why can't it be implied by the compatible >>>> string which should be specific to the chip model? >>>> >>> >>> This is necessary for 93AA46A/B/C, 93LC46A/B/C, 93C46A/B/C eeproms, as >>> it can be seen in section 2.7 of [1]. We were not sure if these were the >>> only devices supported by the driver(eeprom_93xx46.c). So, in order to >>> apply this only to the above listed devices, we thought that it would be >>> better to apply this change when required by introducing a DT property. >>> >>> May I know how has this case been handled till now ?? >>> >> >> No idea. From the at93c46d (which has a compatible string) datasheet it >> looks like it has the same thing. >> >>> If this is required by all the devices then we can drop the property and >>> include the zero bit by default. >> >> Looks like you need a combination of compatible strings for the above >> devices and a property for the ORG pin state on the C devices. I assume >> s/w needs to know if x8 or x16? >> > Yes, there are separate properties for indicating different types of > types of eeproms. > Here I was saying about x8 or x16 using the data-size property. ORG pin state is implied through data-size property and an additional property is not required for ORG pin state. > So, do you think that it is better to add it as a seperate property?? > These are the available options to my knowledge, 1) As you mentioned earlier all the eeprom's supported by the driver send a dummy bit before the read data. This can be thought of a bug and add this change as a fix for it. This might a problem for users who are already using this driver and working around it using user space tools. 2) Add a special compatible string "eeprom-93xx46B", to add the extra dummy cycle and not add an additional property. 3) Add an additional property as proposed in this patch and use when required. Are there any other suggestions on solving this issue?? Thanks, Aswath
On Thu, Dec 17, 2020 at 7:48 AM Aswath Govindraju <a-govindraju@ti.com> wrote: > > Hi Rob, > > On 15/12/20 9:42 pm, Aswath Govindraju wrote: > > Hi Rob, > > On 15/12/20 3:53 am, Rob Herring wrote: > >> On Fri, Dec 11, 2020 at 08:34:57PM +0530, Aswath Govindraju wrote: > >>> Hi, > >>> On 11/12/20 9:03 am, Rob Herring wrote: > >>>> On Wed, Dec 09, 2020 at 11:27:07PM +0530, Aswath Govindraju wrote: > >>>>> Dummy zero bits are sent before data during a read transfer. This causes > >>>>> the data read to be shifted to the right. To fix this send zero bits after > >>>>> the address during a read transfer. > >>>>> > >>>>> Add property to send zero bits after the address during a read transfer. > >>>> > >>>> When is this necessary? Why can't it be implied by the compatible > >>>> string which should be specific to the chip model? > >>>> > >>> > >>> This is necessary for 93AA46A/B/C, 93LC46A/B/C, 93C46A/B/C eeproms, as > >>> it can be seen in section 2.7 of [1]. We were not sure if these were the > >>> only devices supported by the driver(eeprom_93xx46.c). So, in order to > >>> apply this only to the above listed devices, we thought that it would be > >>> better to apply this change when required by introducing a DT property. > >>> > >>> May I know how has this case been handled till now ?? > >>> > >> > >> No idea. From the at93c46d (which has a compatible string) datasheet it > >> looks like it has the same thing. > >> > >>> If this is required by all the devices then we can drop the property and > >>> include the zero bit by default. > >> > >> Looks like you need a combination of compatible strings for the above > >> devices and a property for the ORG pin state on the C devices. I assume > >> s/w needs to know if x8 or x16? > >> > > Yes, there are separate properties for indicating different types of > > types of eeproms. > > > > Here I was saying about x8 or x16 using the data-size property. ORG pin > state is implied through data-size property and an additional property > is not required for ORG pin state. Ah, I missed that property. > > > So, do you think that it is better to add it as a seperate property?? > > > > > These are the available options to my knowledge, > > 1) As you mentioned earlier all the eeprom's supported by the driver > send a dummy bit before the read data. This can be thought of a bug and > add this change as a fix for it. This might a problem for users who are > already using this driver and working around it using user space tools. > > 2) Add a special compatible string "eeprom-93xx46B", to add the extra > dummy cycle and not add an additional property. No. Genericish compatible strings are what cause the problem and this whole discussion. > 3) Add an additional property as proposed in this patch and use when > required. > > Are there any other suggestions on solving this issue?? You need a compatible string for each vendor+model. Period. Rob
Hi Rob, On 17/12/20 9:18 pm, Rob Herring wrote: > On Thu, Dec 17, 2020 at 7:48 AM Aswath Govindraju <a-govindraju@ti.com> wrote: >> >> Hi Rob, >> >> On 15/12/20 9:42 pm, Aswath Govindraju wrote: >>> Hi Rob, >>> On 15/12/20 3:53 am, Rob Herring wrote: >>>> On Fri, Dec 11, 2020 at 08:34:57PM +0530, Aswath Govindraju wrote: >>>>> Hi, >>>>> On 11/12/20 9:03 am, Rob Herring wrote: >>>>>> On Wed, Dec 09, 2020 at 11:27:07PM +0530, Aswath Govindraju wrote: >>>>>>> Dummy zero bits are sent before data during a read transfer. This causes >>>>>>> the data read to be shifted to the right. To fix this send zero bits after >>>>>>> the address during a read transfer. >>>>>>> >>>>>>> Add property to send zero bits after the address during a read transfer. >>>>>> >>>>>> When is this necessary? Why can't it be implied by the compatible >>>>>> string which should be specific to the chip model? >>>>>> >>>>> >>>>> This is necessary for 93AA46A/B/C, 93LC46A/B/C, 93C46A/B/C eeproms, as >>>>> it can be seen in section 2.7 of [1]. We were not sure if these were the >>>>> only devices supported by the driver(eeprom_93xx46.c). So, in order to >>>>> apply this only to the above listed devices, we thought that it would be >>>>> better to apply this change when required by introducing a DT property. >>>>> >>>>> May I know how has this case been handled till now ?? >>>>> >>>> >>>> No idea. From the at93c46d (which has a compatible string) datasheet it >>>> looks like it has the same thing. >>>> >>>>> If this is required by all the devices then we can drop the property and >>>>> include the zero bit by default. >>>> >>>> Looks like you need a combination of compatible strings for the above >>>> devices and a property for the ORG pin state on the C devices. I assume >>>> s/w needs to know if x8 or x16? >>>> >>> Yes, there are separate properties for indicating different types of >>> types of eeproms. >>> >> >> Here I was saying about x8 or x16 using the data-size property. ORG pin >> state is implied through data-size property and an additional property >> is not required for ORG pin state. > > Ah, I missed that property. > >> >>> So, do you think that it is better to add it as a seperate property?? >>> >> >> >> These are the available options to my knowledge, >> >> 1) As you mentioned earlier all the eeprom's supported by the driver >> send a dummy bit before the read data. This can be thought of a bug and >> add this change as a fix for it. This might a problem for users who are >> already using this driver and working around it using user space tools. >> >> 2) Add a special compatible string "eeprom-93xx46B", to add the extra >> dummy cycle and not add an additional property. > > No. Genericish compatible strings are what cause the problem and this > whole discussion. > >> 3) Add an additional property as proposed in this patch and use when >> required. >> >> Are there any other suggestions on solving this issue?? > > You need a compatible string for each vendor+model. Period. > Thank you for the comments. This change is required for microchip "93LC46B" model . I will add a new compatible string "microchip,93LC46B" and use it to implement the driver changes. Thanks, Aswath
diff --git a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt index a8ebb4621f79..09fb1cec54f0 100644 --- a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt +++ b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt @@ -10,7 +10,9 @@ Optional properties: - read-only : parameter-less property which disables writes to the EEPROM - select-gpios : if present, specifies the GPIO that will be asserted prior to each access to the EEPROM (e.g. for SPI bus multiplexing) - +- read-op-dummy-cycles: If present specifies the number of dummy zero-bits to + be sent after the address during a read transfer to ignore any bits sent + preceding the data. Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt apply. In particular, "reg" and "spi-max-frequency" properties must be given.
Dummy zero bits are sent before data during a read transfer. This causes the data read to be shifted to the right. To fix this send zero bits after the address during a read transfer. Add property to send zero bits after the address during a read transfer. Suggested-by: Vignesh Raghavendra <vigneshr@ti.com> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> --- Documentation/devicetree/bindings/misc/eeprom-93xx46.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)