Re: [PATCH v2 2/5] dt-bindings: rtc: qcom-pm8xxx: document no-alarm flag

[Krzysztof Kozlowski <krzk@kernel.org>]

On 14/10/2024 14:58, Jonathan Marek wrote:
> On 10/14/24 3:34 AM, Krzysztof Kozlowski wrote:
>> On Sun, Oct 13, 2024 at 01:15:27AM -0400, Jonathan Marek wrote:
>>> Qualcomm x1e80100 firmware sets the ownership of the RTC alarm to ADSP.
>>> Thus writing to RTC alarm registers and receiving alarm interrupts is not
>>> possible.
>>>
>>> Add a no-alarm flag to support RTC on this platform.
>>>
>>> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
>>> ---
>>>   Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
>>> index d274bb7a534b5..210f76a819e90 100644
>>> --- a/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
>>> +++ b/Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
>>> @@ -40,6 +40,11 @@ properties:
>>>       description:
>>>         Indicates that the setting of RTC time is allowed by the host CPU.
>>>   
>>> +  no-alarm:
>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>> +    description:
>>> +      Indicates that RTC alarm is not owned by HLOS (Linux).
>>
>> This is not even properly used/tested, because you disable the RTC
>> entirely in your DTS.
>>
> 
> What? The next patch in this series is enabling RTC on x1e using this flag

D'oh, right, I must have looked at wrong diff hunks. I had somehow
impression you add status=reserved, but you just dropped it.

> 
>> I expect here unified property for all Qualcomm devices for this case.
>> We already have "remotely-controlled" and other flavors. I don't want
>> each device to express the same with different name...
>>
>> Also: missing vendor prefix.
>>
> 
> I don't care what the property is named (as long as its a bool 
> property), if you have a name you prefer I will use it.
> 
> The existing 'allow-set-time' property (also related to HLOS permissions 
> to the RTC) is also specific to this driver doesn't have a vendor prefix.

Yeah, that one sneaked in some years ago.

So you can set time, but not alarm? Some previous platforms could not
set time, but could set alarm?

I wonder whether we actually describe the real issue here. It looks like
group of band-aids.

Best regards,
Krzysztof