| Message ID | 20210607154931.2491499-1-sean.anderson@seco.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: clk: vc5: Add property for SD polarity | expand |
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@seco.com> 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. 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. [0] https://www.renesas.com/eu/en/document/mau/versaclock-6e-family-register-descriptions-and-programming-guide -- Luca
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@seco.com> > > 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. --Sean > > [0] > https://www.renesas.com/eu/en/document/mau/versaclock-6e-family-register-descriptions-and-programming-guide >
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@seco.com> >> >> 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
diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml index 28675b0b80f1..e6406c0db624 100644 --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml @@ -64,6 +64,10 @@ properties: maximum: 22760 description: Optional load capacitor for XTAL1 and XTAL2 + idt,sd-active-high: + $ref: /schemas/types.yaml#/definitions/flag + description: SD/OE pin polarity is active-high + patternProperties: "^OUT[1-4]$": type: object
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@seco.com> --- Documentation/devicetree/bindings/clock/idt,versaclock5.yaml | 4 ++++ 1 file changed, 4 insertions(+)