linux-kernel - Re: [PATCH v5 2/2] dt-bindings: rtc: add max313xx RTCs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 Apr 2023 08:16:03 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     "Tilki, Ibrahim" <Ibrahim.Tilki@...log.com>,
        "a.zummo@...ertech.it" <a.zummo@...ertech.it>,
        "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
        "jdelvare@...e.com" <jdelvare@...e.com>,
        "linux@...ck-us.net" <linux@...ck-us.net>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>
Cc:     "linux-rtc@...r.kernel.org" <linux-rtc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "Arslanbenzer, Zeynep" <Zeynep.Arslanbenzer@...log.com>
Subject: Re: [PATCH v5 2/2] dt-bindings: rtc: add max313xx RTCs

On 04/04/2023 17:40, Tilki, Ibrahim wrote:
>>> +  interrupts:
>>> +    description: |
>>> +      Alarm1 interrupt line of the RTC. Some of the RTCs have two interrupt
>>> +      lines and alarm1 interrupt muxing depends on the clockin/clockout
>>> +      configuration.
>>> +    maxItems: 1
>>> +
>>> +  "#clock-cells":
>>> +    description: |
>>> +      RTC can be used as a clock source through its clock output pin when
>>> +      supplied.
>>
>> This part is correct, but your implementation is not. I don't think you
>> can disable or enable interrupts, based on usage of clock. Either this
>> is clock (gated or not) or interrupt, not both.
>>
> 
> The driver doesn't enable or disable interrupts based on clock usage. It checks
> whether the IRQ is possible or not. Enablement of interrupt depends on the 
> "interrupts" property. The tricky part is that interrupt muxing depends on
> clock configuration.

I think it is not what your driver is doing. It checks if clock-cells...

> 
> Let me briefly explain the behavior of RTCs and the driver[v4]
> 
> MAX31328:
>   - Has a single pin which is either used as IRQ or clock output.
>   - Driver aborts probe with "-EOPNOTSUPP" when user requests irq and clockout
>     at the same time. In other words, when both "interrupts" and "#clock-cells"
>     properties are present. Otherwise, we are fine.

Why? These are fixed properties of the device. It is always a clock
controller and always has interrupt line. The choice between them is
depending on pins, so pin control.

> 
> MAX31331:
> MAX31334:
>   - Has two pins: INTA and INTB/CLOCKOUT. INTA pin is dedicated for interrupt.
>     INTB pin is used as either interrupt or clockout. The Alarm1 interrupt is
>     muxed into INTB by default. If the CLOCKOUT is enabled, Alarm1 irq is muxed
>     into INTA. We don't have further control over interrupt muxing.
>   - Driver checks for "#clock-cells". If it is present, it enables the clockout
>     so that we can get interrupt from INTA.
> 
> The Rest:
>   - Has two pins: INTA/CLKIN and INTB/CLOCKOUT. Alarm1 interrupt is muxed into
>     INTA by default, muxed into INTB if and only if we enable CLKIN.
>   - Driver aborts probe with -EOPNOTSUPP when user requests interrupt, clockin
>     and clockout at the same time. We can't have all three with two pins.
> 	
> 
> Unfortunately we don't have control over the interrupt muxing other than clock
> configuration. How should the driver approach this?

Pinmux with two states - interrupt or clock.

Best regards,
Krzysztof