Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 10 Feb 2022 23:32:32 +0100
From:   Alexandre Belloni <alexandre.belloni@bootlin.com>
To:     Hugo Villeneuve <hugo@hugovil.com>
Cc:     Rob Herring <robh@kernel.org>,
        Alessandro Zummo <a.zummo@towertech.it>,
        Hugo Villeneuve <hvilleneuve@dimonoff.com>,
        linux-rtc@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 10/10] dt-bindings: rtc: pcf2127: add PCF2131 INT_A and
 INT_B support
Message-ID: <YgWSgGTKR63g+S9e@piout.net>
References: <20220125200009.900660-1-hugo@hugovil.com>
 <20220125200009.900660-11-hugo@hugovil.com>
 <YgMy/CYL8lmf6Y+J@robh.at.kernel.org>
 <20220210171234.4e317c8a5d5f91f358382b07@hugovil.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220210171234.4e317c8a5d5f91f358382b07@hugovil.com>
Precedence: bulk

On 10/02/2022 17:12:34-0500, Hugo Villeneuve wrote:
> On Tue, 8 Feb 2022 21:20:28 -0600
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Tue, Jan 25, 2022 at 03:00:09PM -0500, Hugo Villeneuve wrote:
> > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > 
> > > The PCF2131 has two output interrupt pins, named INT_A and INT_B.
> > > 
> > > Add properties to identify onto which pin we want the alarm interrupt
> > > to be routed. It can be either one, or both.
> > > 
> > > These properties are automatically set to false for variants other
> > > than PCF2131 (ex: PCF2127).
> > > 
> > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > ---
> > >  .../devicetree/bindings/rtc/nxp,pcf2127.yaml  | 23 +++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > index 57eb0a58afa3..83656dd2f97f 100644
> > > --- a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
> > > @@ -24,6 +24,16 @@ properties:
> > >    interrupts:
> > >      maxItems: 1
> > >  
> > > +  alarm-output-a:
> > 
> > nxp,alarm-output-a
> 
> Ok, this will be fixed for V2.
> 

Actually, this property has to be made more generic and thought out.
There are multiple RTCs that have multiple interrupt pins where one of
the pin can be used for different interrupt or clock output.

With your binding, there is no way to separate which interrupt is going
to which pin and so there is no way to get the alarm and BLF or the
watchdog on different pins and we certainly don't want to have a
property per interrupt type.

Also, the documentation is missing the fact that the driver makes having
one of the property mandatory.


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