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

--=-WE5p0wNFoNN4V4RC6yVN
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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:
>=20
> This goes after "properties:" block.
>=20
>=20
> > +  "^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
>=20
> Use simple string. See pinmux bindings.
>=20
> > +            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 frequenc=
y.
> > +              - S35390A_MODE_PMIN1: Minute periodical output with 50% =
duty.
> > +              - S35390A_MODE_PMIN2: Minute periodical output L for 7.8=
1 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:
>=20
> 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
>=20
>=20
> > +            $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 =3D <1>;
> > +        #size-cells =3D <0>;
> > +
> > +        rtc: rtc@30 {
>=20
> Drop unused label
>=20
> > +            compatible =3D "sii,s35390a";
> > +            reg =3D <0x30>;
> > +            pinctrl-0 =3D <&rtc_pins_default>;
> > +            pinctrl-names =3D "default";
> > +
> > +            wakeup-parent =3D <&int>;
> > +
> > +            rtc_pins_default: pins {
> > +              int1-pins {
> > +                pins =3D S35390A_PIN_INT1;
> > +                sii,mode =3D <S35390A_MODE_WAKEUP>;
> > +              };
> > +              int2-pins {
> > +                pins =3D S35390A_PIN_INT2;
> > +                sii,mode =3D <S35390A_MODE_DISABLE>;
> > +              };
> > +            };
> > +        };
> > +    };
> > diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/D=
ocumentation/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
> > =20
> >  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/kerne=
l/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/rt=
c/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
>=20
>  don't get why this grew. None of these look like bindings.
>=20
>=20
> > +
> > +#define S35390A_PIN_INT1		"int1"
> > +#define S35390A_PIN_INT2		"int2"
>=20
> Drop entire header.
>=20
> > +
> > +#endif /* __DT_BINDINGS_RTC_S35390A_H */
> >=20
> > --=20
> > 2.54.0
> >=20


--=-WE5p0wNFoNN4V4RC6yVN
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part

-----BEGIN PGP SIGNATURE-----

iQJPBAABCAA5FiEEgnQYxPSsWOdyMMRzNHYf+OetQ9IFAmpxJSEbFIAAAAAABAAO
bWFudTIsMi41KzEuMTIsMiwyAAoJEDR2H/jnrUPSeeQQAJiJHHqdtepYKqV8bn1f
7GnPjLKXPm4ovd1ERvFtZssRegwaHA3BUiBGqcelTWyIqofLv9v8TjBli4xrxwd2
cKW6jOC/Xzbx37Cl90E+iSKQyxT1Bu6HS5Mk9+FC+ep8pL4a8EH9NyIdFDG17gc+
eS45B0WM20oEH/udzt6qk6UJNVjkEJmkMs09mzq1QOCTj7vJyCTYB3adlwOui47n
h1r1I5e9ZisU1WGfKgBBk9Gm+lX/uDOfDlYG6BaH7Mb7gOcrYqCuLGysGQJVridO
uf17IzliwtLic1kHBAU4NFlNSMMhncyjqrZkHHvXPxgul6CEYhj3V0uF/x9uFcra
Zw/wvhA+WSg/tW35LEhEocq+jArhuL0h/vePrp5JvPIgvm5wSGeccKlEHyKjx7Eq
scdZxvQLbIGj5peZfcwAiDN+u/2oyLFiD6sk2j7SMqIR7E94kYL0D3o0UZ0dZVLA
UTxw9J7M1kgcwzPZn8E7OAN1nWNmfECANidMH6aZ+rFdlxY4PNBFuDjzCipChCNd
arfl9ZLEUYQ5R1nBHaXQrdBYBHt8Hbdup+E0oAOLQUSqKwlFeZ1/8Z32aB3qVNBf
zudZq2CVMiT5rrATyZXjqh+qoITD5+ULhmAz+h6VpHYfirsa27asRaOeigBo3CsO
XEozlYZAQZK39Mxrfg471VLA
=q6sH
-----END PGP SIGNATURE-----

--=-WE5p0wNFoNN4V4RC6yVN--