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] [day] [month] [year] [list]
Message-ID:
 <AS4PR04MB9362B73FE631B14CA8412FB9FBEFA@AS4PR04MB9362.eurprd04.prod.outlook.com>
Date: Fri, 10 Oct 2025 05:21:21 +0000
From: Lakshay Piplani <lakshay.piplani@....com>
To: Conor Dooley <conor@...nel.org>
CC: "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
	"linux-rtc@...r.kernel.org" <linux-rtc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, Pankit Garg
	<pankit.garg@....com>, Vikash Bansal <vikash.bansal@....com>, Priyanka Jain
	<priyanka.jain@....com>, Shashank Rebbapragada
	<shashank.rebbapragada@....com>
Subject: RE: [EXT] Re: [PATCH v4 1/2] dt-bindings: rtc: Add pcf85053 support



> -----Original Message-----
> From: Conor Dooley <conor@...nel.org>
> Sent: Thursday, October 9, 2025 10:30 PM
> To: Lakshay Piplani <lakshay.piplani@....com>
> Cc: alexandre.belloni@...tlin.com; linux-rtc@...r.kernel.org; linux-
> kernel@...r.kernel.org; robh@...nel.org; krzk+dt@...nel.org;
> conor+dt@...nel.org; devicetree@...r.kernel.org; Pankit Garg
> <pankit.garg@....com>; Vikash Bansal <vikash.bansal@....com>; Priyanka
> Jain <priyanka.jain@....com>; Shashank Rebbapragada
> <shashank.rebbapragada@....com>
> Subject: Re: [EXT] Re: [PATCH v4 1/2] dt-bindings: rtc: Add pcf85053 support
> 
> On Thu, Oct 09, 2025 at 08:13:49AM +0000, Lakshay Piplani wrote:
> >
> > > -----Original Message-----
> > > From: Conor Dooley <conor@...nel.org>
> > > Sent: Wednesday, September 24, 2025 12:28 AM
> > > To: Lakshay Piplani <lakshay.piplani@....com>
> > > Cc: alexandre.belloni@...tlin.com; linux-rtc@...r.kernel.org; linux-
> > > kernel@...r.kernel.org; robh@...nel.org; krzk+dt@...nel.org;
> > > conor+dt@...nel.org; devicetree@...r.kernel.org; Pankit Garg
> > > <pankit.garg@....com>; Vikash Bansal <vikash.bansal@....com>;
> > > Priyanka Jain <priyanka.jain@....com>; Shashank Rebbapragada
> > > <shashank.rebbapragada@....com>
> > > Subject: [EXT] Re: [PATCH v4 1/2] dt-bindings: rtc: Add pcf85053
> > > support
> > >
> > > On Tue, Sep 23, 2025 at 05:04:40PM +0530, Lakshay Piplani wrote:
> > > > Add device tree bindings for NXP PCF85053 RTC chip.
> > > >
> > > > Signed-off-by: Pankit Garg <pankit.garg@....com>
> > > > Signed-off-by: Lakshay Piplani <lakshay.piplani@....com>
> > > > ---
> > > > V3 -> V4: Add dedicated nxp,pcf85053.yaml.
> > > >           Remove entry from trivial-rtc.yaml.
> > > > V2 -> V3: Moved MAINTAINERS file changes to the driver patch
> > > > V1 -> V2: Handled dt-bindings by trivial-rtc.yaml
> > > >
> > > >  .../devicetree/bindings/rtc/nxp,pcf85053.yaml | 128
> > > > ++++++++++++++++++
> > > >  1 file changed, 128 insertions(+)  create mode 100644
> > > > Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
> > > > b/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
> > > > new file mode 100644
> > > > index 000000000000..6b1c97358486
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
> > > > @@ -0,0 +1,128 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) #
> > > > +Copyright
> > > > +2025 NXP %YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/rtc/nxp,pcf85053.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: NXP PCF85053 Real Time Clock
> > > > +
> > > > +maintainers:
> > > > +  - Pankit Garg <pankit.garg@....com>
> > > > +  - Lakshay Piplani <lakshay.piplani@....com>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - nxp,pcf85053
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 1
> > > > +
> > > > +  nxp,interface:
> > > > +    $ref: /schemas/types.yaml#/definitions/string
> > > > +    enum: [ primary, secondary ]
> > > > +    description: |
> > > > +      Identifies this host's logical role in a multi-host topology for the
> > > > +      PCF85053 RTC. The device exposes a "TWO" ownership bit in the
> CTRL
> > > > +      register that gates which host may write time/alarm registers.
> > > > +        - "primary": Designated host that *may* claim write ownership
> (set
> > > > +          CTRL.TWO=1) **if** write-access is explicitly requested.
> > > > +        - "secondary": Peer host that writes only when CTRL.TWO=0
> (default).
> > > > +
> > > > +  nxp,write-access:
> > > > +    type: boolean
> > > > +    description: |
> > > > +      Request the driver to claim write ownership at probe time by setting
> > > > +      CTRL.TWO=1. This property is only valid when
> nxp,interface="primary".
> > > > +      The driver will not modify any other CTRL bits (HF/DM/etc.)
> > > > + and will
> > > not
> > > > +      clear any status/interrupt flags at probe.
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - nxp,interface
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +# Schema constraints matching driver:
> > > > +# 1) If nxp,write-access is present, nxp,interface must be "primary".
> > > > +#    Rationale: only the primary may claim ownership; driver will set
> > > TWO=1.
> > > > +# 2) If nxp,interface is "secondary", nxp,write-access must not be
> present.
> > > > +#    Rationale: secondary never claims ownership and cannot write
> > > CTRL/ST/alarm.
> > > > +#
> > > > +# Practical effect:
> > > > +# - Primary without 'nxp,write-access'; primary is read only;
> > > > +secondary
> > > may
> > > > +#   write time registers.
> > > > +# - Primary with 'nxp,write-access'; primary owns writes,
> > > > +secondary is read
> > > only.
> > > > +allOf:
> > > > +  - $ref: rtc.yaml#
> > > > +  - oneOf:
> > > > +      # Case 1: primary with write-access
> > > > +      - required: [ "nxp,write-access" ]
> > > > +        properties:
> > > > +          nxp,interface:
> > > > +            const: primary
> > > > +
> > > > +      # Case 2: primary without write-access
> > > > +      - properties:
> > > > +          nxp,interface:
> > > > +            const: primary
> > > > +        not:
> > > > +          required: [ "nxp,write-access" ]
> > >
> > > Aren't case 1 and case 2 here redundant? All you need to do is block
> > > interface == secondary when nxp,write-access is present, which your
> > > case
> > > 3 should be able to be modified to do via
> > >
> > > if:
> > >   properties:
> > >     nxp,interface:
> > >       const: secondary
> > > then:
> > >   properties:
> > >    nxp,write-access: false
> > >
> > > I think your description for nxp,write-access gets the point across
> > > about when it can be used, and the additional commentary is not really
> helpful.
> > >
> > Thanks for reviewing the patch.
> >
> > We kept both cases: primary with write-access and primary without
> > write-access, because the hardware supports three different ways it can be
> used, and we want to show that clearly in the bindings:
> >
> > Primary with nxp,write-access: primary host can write to the device.
> > Primary without nxp,write-access - primary host is read-only, and the
> secondary host can write.
> > Secondary - default role, with write access; when no primary host is
> claiming it.
> >
> > Even though both case 1 and 2 use nxp,interface = "primary", they behave
> differently.
> > Keeping both cases separate makes it easier to understand whether Primary
> host can write or not.
> 
> Just explain it properly in the property description, creating if/then/else stuff
> with additional commentary like this makes it more, rather than less,
> confusing.
> 

Understood - we'll simplify the schema by keeping only the if/then constraint and move the explanation
of the valid configurations into the property descriptions for nxp,interface and nxp,write-access rather than 
in comments.

> >
> > > > +
> > > > +      # Case 3: secondary (must not have write-access)
> > > > +      - properties:
> > > > +          nxp,interface:
> > > > +            const: secondary
> > > > +        not:
> > > > +          required: [ "nxp,write-access" ]
> > > > +
> > > > +examples:
> > > > +  # Single host example.
> > > > +  - |
> > > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > > +    i2c {
> > > > +      #address-cells = <1>;
> > > > +      #size-cells = <0>;
> > > > +
> > > > +      rtc@6f {
> > > > +        compatible = "nxp,pcf85053";
> > > > +        reg = <0x6f>;
> > > > +        nxp,interface = "primary";
> > > > +        nxp,write-access;
> > > > +        interrupt-parent = <&gpio2>;
> > > > +        interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> > > > +      };
> > > > +    };
> > > > +
> > > > +  # Dual-host example: one primary that claims writes; one
> > > > + secondary that
> > > never claims writes.
> > > > +  - |
> > > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > > +    i2c0 {
> > > > +      #address-cells = <1>;
> > > > +      #size-cells = <0>;
> > > > +
> > > > +      rtc@6f {
> > > > +        compatible = "nxp,pcf85053";
> > > > +        reg = <0x6f>;
> > > > +        nxp,interface = "primary";
> > > > +        nxp,write-access;
> > > > +        interrupt-parent = <&gpio2>;
> > > > +        interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> > > > +      };
> > > > +    };
> > > > +
> > > > +    i2c1 {
> > > > +      #address-cells = <1>;
> > > > +      #size-cells = <0>;
> > > > +
> > > > +      rtc@6f {
> > > > +        compatible = "nxp,pcf85053";
> > > > +        reg = <0x6f>;
> > > > +        nxp,interface = "secondary";
> > >
> > > Maybe a silly question, but if you have a system that wants to have
> > > two pairs of RTCs, how would you determine which primary a secondary
> > > belongs to? I notice you have no link between these devices in dt so
> > > I am curious. Would it be better to eschew nxp,interface and have a
> > > phandle from the secondary to the primary?
> > >
> > > I don't know anything about your use case or features, so maybe
> > > knowing the relationship just is not relevant at all, or it can be determined
> at runtime.
> >
> > This device can connect to two independent hosts via separate I²C buses.
> > Each host sees the same hardware instance through its own I²C address.
> > The nxp,interface property simply declares the host's role, so the
> > driver knows whether to attempt write access or not.
> 
> That doesn't really answer what I was looking for. Is knowing the relationships
> either unimportant or determinable at runtime?

Primary and secondary are independent hosts connected to separate I²C buses,
so, no relationship needs to be described between them.

Thanks
Lakshay

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ