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

On 03/08/2026 23:33:05+0000, Markus Probst wrote:
> > 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.

Usually, the clock is registered in the CCF so it is disabled
automatically when not used.

You can simply add #clock-cells in the bindings and leave out the
implementation in the driver.

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



-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com