Re: [PATCH v2 1/5] dt-bindings: rtc: Add pinctrl for S35390A

Markus Probst <[email protected]> Mon, 03 Aug 2026 23:33:05 +0000
Newsgroups	gmane.linux.drivers.devicetree,gmane.linux.ports.arm.kernel,gmane.linux.kernel
Message-ID	<[email protected]>
On Mon, 2026-08-03 at 11:56 +0200, Krzysztof Kozlowski wrote:
> On Sat, Aug 01, 2026 at 06:43:00PM +0000, Markus Probst wrote:
> > +  wakeup-parent: true
> > +
> > +  wakeup-source: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +patternProperties:
> 
> This goes after "properties:" block.
> 
> 
> > +  "^pins":
> > +    type: object
> > +    patternProperties:
> > +      "-pins$":
> > +        type: object
> > +        properties:
> > +          pins:
> > +            $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pins
> > +
> > +          sii,mode:
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> 
> Use simple string. See pinmux bindings.
> 
> > +            description: |
> > +              Modes for the specific pin:
> > +              - S35390A_MODE_IGNORE: Preserve the previous state.
> > +              - S35390A_MODE_DISABLE: Disable all interrupts.
> > +              - S35390A_MODE_WAKEUP: Used for waking up the system.
> > +              - S35390A_MODE_ALARM: Userspace defined alarm.
> > +              - S35390A_MODE_FREQ: Periodical at configurable frequency.
> > +              - S35390A_MODE_PMIN1: Minute periodical output with 50% duty.
> > +              - S35390A_MODE_PMIN2: Minute periodical output L for 7.81 ms.
> > +                                    Can only be used with pin INT1.
> > +              - S35390A_MODE_32768KHZ: Raw 32.768 kHz output.
> > +                                       Can only be used with pin INT1.
> > +            minimum: 0
> > +            maximum: 7
> > +
> > +          sii,frequency:
> 
> Frequency needs -hz, but anyway if this is becoming a clock, then you
> must have clocks property and this is simply not needed... unless this
> is some periodical wakeup call, not a clock? Then simply -hz and
> property must be more descriptive (frequency of what? pulse?).
Yes, it would become a clock if configured. But in this case, the
"clocks" property seems to be a clock consumer property. It would be
providing the clock signal.

Anyway, it was added since it was only ~10 lines of code. Looking
through documentation, using the clock subsystem might be a bit more
driver work.
Since I don't have a specific use for it, I will drop it in the next
revision.

Thanks
- Markus Probst
> 
> 
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> > +            description: Frequency in Hz to be used with mode S35390A_MODE_FREQ.
> > +            minimum: 1
> > +            maximum: 31
> > +
> > +        required:
> > +          - sii,mode
> > +
> > +        if:
> > +          properties:
> > +            sii,mode:
> > +              const: 4
> > +        then:
> > +          required:
> > +            - sii,frequency
> > +        else:
> > +          properties:
> > +            sii,frequency: false
> > +
> > +        additionalProperties: false
> > +
> > +    additionalProperties: false
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/rtc/s35390a.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        rtc: rtc@30 {
> 
> Drop unused label
> 
> > +            compatible = "sii,s35390a";
> > +            reg = <0x30>;
> > +            pinctrl-0 = <&rtc_pins_default>;
> > +            pinctrl-names = "default";
> > +
> > +            wakeup-parent = <&int>;
> > +
> > +            rtc_pins_default: pins {
> > +              int1-pins {
> > +                pins = S35390A_PIN_INT1;
> > +                sii,mode = <S35390A_MODE_WAKEUP>;
> > +              };
> > +              int2-pins {
> > +                pins = S35390A_PIN_INT2;
> > +                sii,mode = <S35390A_MODE_DISABLE>;
> > +              };
> > +            };
> > +        };
> > +    };
> > diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
> > index f4d0eed98a08..7b3f682ef4d5 100644
> > --- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
> > +++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
> > @@ -81,8 +81,6 @@ properties:
> >        - ricoh,rv5c386
> >        # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
> >        - ricoh,rv5c387a
> > -      # 2-wire CMOS real-time clock
> > -      - sii,s35390a
> >        # ST SPEAr Real-time Clock
> >        - st,spear600-rtc
> >        # VIA/Wondermedia VT8500 Real-time Clock
> > @@ -105,5 +103,4 @@ required:
> >    - reg
> >  
> >  additionalProperties: false
> > -
> >  ...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 716acfc3d7c1..c85be1eefdb1 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -22696,6 +22696,7 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git
> >  F:	Documentation/admin-guide/rtc.rst
> >  F:	Documentation/devicetree/bindings/rtc/
> >  F:	drivers/rtc/
> > +F:	include/dt-bindings/rtc/
> >  F:	include/linux/rtc.h
> >  F:	include/linux/rtc/
> >  F:	include/uapi/linux/rtc.h
> > diff --git a/include/dt-bindings/rtc/s35390a.h b/include/dt-bindings/rtc/s35390a.h
> > new file mode 100644
> > index 000000000000..dcd0ca15f9e9
> > --- /dev/null
> > +++ b/include/dt-bindings/rtc/s35390a.h
> > @@ -0,0 +1,23 @@
> > +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
> > +
> > +#ifndef __DT_BINDINGS_RTC_S35390A_H
> > +#define __DT_BINDINGS_RTC_S35390A_H
> > +
> > +#define S35390A_MODE_IGNORE		0x0
> > +#define S35390A_MODE_DISABLE		0x1
> > +#define S35390A_MODE_WAKEUP		0x2
> > +#define S35390A_MODE_ALARM		0x3
> > +#define S35390A_MODE_FREQ		0x4
> > +#define S35390A_MODE_PMIN1		0x5
> > +#define S35390A_MODE_PMIN2		0x6
> > +#define S35390A_MODE_32768KHZ		0x7
> > +
> > +#define S35390A_MODE_INT1_MAX		0x7
> > +#define S35390A_MODE_INT2_MAX		0x5
> > +
> > +#define S35390A_FREQ_MASK		0x1F
> 
>  don't get why this grew. None of these look like bindings.
> 
> 
> > +
> > +#define S35390A_PIN_INT1		"int1"
> > +#define S35390A_PIN_INT2		"int2"
> 
> Drop entire header.
> 
> > +
> > +#endif /* __DT_BINDINGS_RTC_S35390A_H */
> > 
> > -- 
> > 2.54.0
> >
signature.asc (application/pgp-signature, 870 B)