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:   Tue, 4 Apr 2023 15:40:00 +0000
From:   "Tilki, Ibrahim" <Ibrahim.Tilki@...log.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        "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

> > +  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.

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.

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?

Best regards,
Ibrahim