Re: [PATCH v4 1/3] dt-bindings: clk: vc5: Add properties for configuring the SD/OE pin

[Luca Ceresoli <luca@lucaceresoli.net>]

Hi Sean,

On 01/07/21 20:20, Sean Anderson wrote:
> These properties allow configuring the SD/OE pin as described in the
> datasheet.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> 
> Changes in v4:
> - Specify that bindings should specify these properties, but don't make
>   any guarantees about the driver's behavior when they are not present.
> - Clarify description of idt,(en|dis)able-shutdown properties.
> - Make opposing properties mutually exclusive.
> - Add these properties to the example.
> 
> Changes in v3:
> - Add idt,disable-shutdown and idt,output-enable-active-low to allow for
>   a default of not changing the SP/SH bits at all.
> 
> Changes in v2:
> - Rename idt,sd-active-high to idt,output-enable-active-high
> - Add idt,enable-shutdown
> 
>  .../bindings/clock/idt,versaclock5.yaml       | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> index 28675b0b80f1..2631a175612b 100644
> --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> @@ -30,6 +30,21 @@ description: |
>      3 -- OUT3
>      4 -- OUT4
>  
> +  The idt,(en|dis)able-shutdown and idt,output-enable-active-(high|low)
> +  properties control the SH (en_global_shutdown) and SP bits of the
> +  Primary Source and Shutdown Register, respectively. Their behavior is
> +  summarized by the following table:
> +
> +  SH SP Output when the SD/OE pin is Low/High
> +  == == =====================================
> +   0  0 Active/Inactive
> +   0  1 Inactive/Active
> +   1  0 Active/Shutdown
> +   1  1 Inactive/Shutdown

I just got an update from Renesas support, which confirms our table as
far as possible:

> I find that SH will only work when the Polarity bit is set to Active Low (SP=0).  When I program SP=1 together with SH=1, the outputs are always off. It simply won’t work.  So the Shutdown function can only be used with Active Low polarity for the SD/OE pin.  There is a pull-down resistor on the chip so when the pin is left open, outputs will be enabled with Active Low polarity.
>  
> [...] What is a more accurate description is that you can only use the Shutdown function (SH=1) together with the Active Low polarity (SP=0) setting.  The combination of SH=1 and SP=1 is an ‘illegal’ combination.
>  
> We actually find that very few customers use the Shutdown function. It is only partly useful because it turns off more circuits than the Output Enable function but there is still significant power consumption in Shutdown. The general expectation of a Shutdown function is that the power consumption will go down to a few microamperes of leakage current when in Shutdown but this device will not do that.

The combination SH=1 and SP=1 is not well specified, so I can't confirm
whether it should be Inactive/Shutdown or Shutdown/Shutdown or whatever.
Also I would not define the SH=1 and SP=1 combination as "illegal" but
rather as "useless".

Definitely (and obviously) the outputs are never driven when SH=1 and
SP=1. So I think we can stick to your table and I think this patch is
fine, more details are not needed in the DT bindings IMO. The only open
point now is the remark by Geert about the 'if: true' construct.

-- 
Luca