Hi Sean, On 10/06/21 17:43, Sean Anderson wrote: > > > On 6/10/21 5:05 AM, Luca Ceresoli wrote: >> Hi Sean, >> >> On 07/06/21 17:49, Sean Anderson wrote: >>> This property allows setting the SD/OE pin's polarity to active-high, >>> instead of the default of active-low. >>> >>> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx> >> >> Thanks. >> >>> + idt,sd-active-high: >>> + $ref: /schemas/types.yaml#/definitions/flag >>> + description: SD/OE pin polarity is active-high >> >> I think the name "sd" is misleading. > > I do as well. After sending this patch, I reviewed the documentation > again and discovered that the functionality was not as clear as I > initially thought. > >> In the Renesas docs (which are very confusing on their own about this >> topic) this bit is called "SP" -- *S*D/OE *P*olarity. But actually it >> controls polarity of the SD/OE pin only if the pin is configured for >> "OE" function: >> >>> SP bit = “SD/OE pin Polarity Bit”: Set the polarity of the SD/OE >>> pin where outputs enable or disable. Only works with OE, not with SD. >> (VC6E register and programming guide [0]) >> >> As such I suggest you use either "sp" to keep the naming used in the >> Renesas docs or "oe" as it actually controls OE polarity only. I do >> prefer "sp" as it helps matching with the datasheets, but maybe adding a >> little more detail in bindings docs to clarify, as in: >> >> idt,sp-active-high: >> $ref: /schemas/types.yaml#/definitions/flag >> description: SD/OE pin polarity is active-high >> (only works when SD/OE pin is configured as OE) >> >> BTW is it only me finding the "Shutdown Function" of [0] completely >> confusing? Also, Table 24 has contradictory lines and missing lines. I'm >> sending a request to Renesas support to ask them to clarify it all. > > I rearranged the table to highlight which bits cause the output to > become inactive: > > SH SP OSn OEn SD/OE OUT > x x 1 0 x Active > 0 0 1 1 0 Active > 0 0 1 1 1 Inactive > 0 1 1 1 0 Inactive > 0 1 1 1 1 Active > 1 0 1 1 0 Active > 1 0 x x 1 Shutdown > 1 1 1 1 0 Inactive > 1 1 x x 1 Shutdown > x x 0 x x Inactive> > This may be condensed to > > SH SP SD/OE function for 0/1 > 0 0 Active/Inactive > 0 1 Inactive/Active > 1 0 Active/Shutdown > 1 1 Inactive/Shutdown > > According to the datasheet, the default settings are SH=0 and SP=0. So > perhaps a good set of properties would be > > idt,enable-shutdown: > Shutdown the device when the SD/OE pin is high. This would set > SH=1. > idt,output-enable-active-high: > Disable output when the SD/OE pin is low. This would set SP=1. Seems good. -- Luca
