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