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

[Boris Brezillon <boris.brezillon@collabora.com>]

On Thu, 14 Nov 2019 11:54:32 +0100
Przemyslaw Gaj <pgaj@cadence.com> wrote:

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

By clk you mean SCL or the clock driving the I3C master logic (which is
likely to run at a higher freq)?

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

Yes, exactly.

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