Re: [PATCH 2/3] dt-bindings: i3c: Document data hold delay feature

From: Przemyslaw Gaj <pgaj@cadence.com>
To: Boris Brezillon <boris.brezillon@collabora.com>
Cc: linux-i3c@lists.infradead.org, vitor.soares@synopsys.com,
	rafalc@cadence.com, agolec@cadence.com, bbrezillon@kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: i3c: Document data hold delay feature
Date: Thu, 14 Nov 2019 11:54:32 +0100	[thread overview]
Message-ID: <20191114105431.GA27940@global.cadence.com> (raw)
In-Reply-To: <20191114102601.745ce03f@collabora.com>

The 11/14/2019 10:26, Boris Brezillon wrote:
> 
> On Thu, 14 Nov 2019 10:15:49 +0100
> Boris Brezillon <boris.brezillon@collabora.com> wrote:
> 
> > On Thu, 14 Nov 2019 06:51:54 +0100
> > Przemyslaw Gaj <pgaj@cadence.com> wrote:
> > 
> > > Documenting THD_DEL (Data Hold Delay) feature of Cadence I3C
> > > master controller.
> > > 
> > > Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
> > > ---
> > >  Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt b/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
> > > index 1cf6182f888c..7d6349354390 100644
> > > --- a/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
> > > +++ b/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
> > > @@ -21,6 +21,12 @@ Documentation/devicetree/bindings/i3c/i3c.txt for more details):
> > >  - i2c-scl-hz
> > >  - i3c-scl-hz
> > >  
> > > +Optional properties, Cadence I3C master controller specific:
> > > +
> > > +- thd_del: Data Hold Delay. Sets data hold delay (i.e. SDA_OUT data
> > > +	   line will change its value with extra delay that specified
> > > +	   in this register).  
> > 
> > If it's a Cadence specific property, it should be prefixed with 'cdns,':
> > 
> >  - cdns,thd-delay

Ok.

> 
> Oh, and you need to specify the unit. Given the code, I suspect it's in
> clk-cycles, which is not great, since you probably want the delay to
> always be the same no matter the frequency of the clk driving the I3C
> master block.
> 

Actually, this is encoded value. 3 means no delay, 2 - 1x clk delay, 
1 - 2x clk dealy, 0 - 3x clk delay. I should mention about that in the
documentation.

> > 
> > Quick question about this delay, is it related to the IP integration in
> > a SoC or is it board specific? In former case, I'd recommend attaching
> > this piece of information to the compatible and have one compatible per
> > SoC.
> > 

This is spec requirement, slave shouldn't see SDA change before SCL. It
is possible to achive this requirement during SoC physical design. If this
is not the case, you can achive this using thd_del functionality. For
now this is generic driver for our controller. So the question is, what
should I do? Should I add default value for existing compatible and wait
for different SoCs compatibility?

> > > +
> > >  I3C device connected on the bus follow the generic description (see
> > >  Documentation/devicetree/bindings/i3c/i3c.txt for more details).
> > >    
> > 
> 

-- 
-- 
Przemyslaw Gaj

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c