Message-ID: <8fec6c89-548b-43b5-8361-869663a58573@wolfvision.net>
Date:   Thu, 26 Oct 2023 11:41:47 +0200
From:   Javier Carrasco <javier.carrasco@...fvision.net>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     Alessandro Zummo <a.zummo@...ertech.it>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-rtc@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/2] dt-bindings: rtc: nxp,pcf8563: add hiz-output
 property


On 26.10.23 02:50, Alexandre Belloni wrote:
> On 26/10/2023 01:23:21+0200, Javier Carrasco wrote:
>>>>> +  hiz-output:
>>>>> +    description:
>>>>> +      Use enabled if the output should stay in high-impedance. This
>>>>> +      mode will mask the output as an interrupt source.
>>>>> +      Use sleep if the otuput should be only active in sleep mode.
>>>>> +      This mode is compatible with any other output configuration.
>>>>> +      The disabled value acts as if the property was not defined.
>>>>> +    enum:
>>>>> +      - enabled
>>>>> +      - sleep
>>>>> +      - disabled
>>>>> +    default: disabled
>>>>> +
>>>>
>>>> If instead of using a custom property, you consider this as what it
>>>> actually is: pinmuxing, then everything else comes for free. With
>>>> pinctrl, you can define different states for runtime and sleep and they
>>>> will get applied automatically instead of open coding in the driver.
>>
>> I am not sure if your solution would cover all my needs:
>>
>> 1.- With pinctrl I can model the SoC pins, right? That would not stop
>> the RTC output though, so the 32 kHz signal would be generated anyways
>> even though the SoC would ignore it. That is one of the things I want to
>> avoid.
>>
> 
> No, you would model the INTA pin.
I am sorry for insisting on this topic, but if I get you right, I would
be modeling an interrupt pin (INTA) to keep it from generating a clock
signal when the RTC itself offers a high-impedance mode i.e. avoiding to
use the RTC feature.

Is that not a misuse of the INTA pin in the first place? If there was no
other option, that would be an easy fix, but why would we not implement
the hi-Z mode when it is available? If I see a pinctrl-* modeling an
interrupt pin, it is not obvious that I am doing that to stop the clock
signal and I would have to clarify it explicitly, especially if I am not
interested in the interrupt.

I would rather implement and document the hi-Z mode the RTC offers
instead of using another mode like INTA which actually can trigger
interrupts. If the implementation must be different is of course another
topic.
> 
>> 2.- What happens if the RTC output is a clock for an external device
>> that is only required when the SoC is in sleep mode? In that case I
>> would like the RTC driver to control the output with the modes it provides.
> 
> Even if I doubt this is a valid use case, this would be possible as long
> as the external device node has the correct pinctrl-* properties.
> 
> 
>>>>
>>>> Also, how you define this property means that everyone currently using
>>>> this RTC is going to have a new warning that they should just ignore.
>>>>
>>>>
>>> Thanks for your reply. The warning can only be triggered if the property
>>> is defined, so in principle no one could have that warning yet. Only the
>>> ones who actually define it and use an invalid value would get the warning.
>>>
>>> On the other hand I did not consider your approach, which might make
>>> this patch irrelevant. So I will have a look at it to make sure that it
>>> achieves the same results.
>>>
>>> Thanks again and best regards,
>>> Javier Carrasco
>>>
> 

Powered by blists - more mailing lists