[PATCH 0/3] Add data hold delay support

[PATCH 0/3] Add data hold delay support

[Przemyslaw Gaj]

Add THD_DEL (Data Hold Delay) support. After testing different scenarios,
on various systems, sometimes there is a need to delay SDA_OUT propagation.

Adding support which allows to configure that delay using the device
tree parameter.

Przemyslaw Gaj (3):
  i3c: master: add data hold delay support
  dt-bindings: i3c: Document data hold delay feature
  MAINTAINERS: add myself as maintainer of Cadence I3C master controller
    driver

 Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt | 6 ++++++
 MAINTAINERS                                               | 6 ++++++
 drivers/i3c/master/i3c-master-cdns.c                      | 5 ++++-
 3 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.14.0


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

[PATCH 1/3] i3c: master: add data hold delay support

[Przemyslaw Gaj]

This patch adds support for THD_DEL (Data Hold Delay) to Cadence
I3C master constoller driver.

Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
---
 drivers/i3c/master/i3c-master-cdns.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index 10db0bf0655a..90ea98eef905 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -60,6 +60,7 @@
 #define CTRL_HALT_EN			BIT(30)
 #define CTRL_MCS			BIT(29)
 #define CTRL_MCS_EN			BIT(28)
+#define CTRL_THD_DEL(x) 		(((x) << 24) & GENMASK(25, 24))
 #define CTRL_HJ_DISEC			BIT(8)
 #define CTRL_MST_ACK			BIT(7)
 #define CTRL_HJ_ACK			BIT(6)
@@ -1186,7 +1187,7 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
 	struct cdns_i3c_master *master = to_cdns_i3c_master(m);
 	unsigned long pres_step, sysclk_rate, max_i2cfreq;
 	struct i3c_bus *bus = i3c_master_get_bus(m);
-	u32 ctrl, prescl0, prescl1, pres, low;
+	u32 ctrl, prescl0, prescl1, pres, low, thd_del;
 	struct i3c_device_info info = { };
 	int ret, ncycles;
 
@@ -1264,6 +1265,8 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
 	 * We will issue ENTDAA afterwards from the threaded IRQ handler.
 	 */
 	ctrl |= CTRL_HJ_ACK | CTRL_HJ_DISEC | CTRL_HALT_EN | CTRL_MCS_EN;
+	if (!of_property_read_u32(m->dev.of_node, "thd_del", &thd_del))
+		ctrl |= CTRL_THD_DEL(thd_del);
 	writel(ctrl, master->regs + CTRL);
 
 	cdns_i3c_master_enable(master);
-- 
2.14.0


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

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

[Przemyslaw Gaj]

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).
+
 I3C device connected on the bus follow the generic description (see
 Documentation/devicetree/bindings/i3c/i3c.txt for more details).
 
-- 
2.14.0


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

[PATCH 3/3] MAINTAINERS: add myself as maintainer of Cadence I3C master controller driver

[Przemyslaw Gaj]

As discussed with Boris Brezillon - I'm adding myself as the maintainer.

Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c4c532c70b86..afdce16d2be5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7821,6 +7821,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt
 F:	drivers/i3c/master/dw*
 
+I3C DRIVER FOR CADENCE I3C MASTER IP
+M:      Przemysław Gaj <pgaj@cadence.com>
+S:      Maintained
+F:      Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
+F:      drivers/i3c/master/i3c-master-cdns.c
+
 IA64 (Itanium) PLATFORM
 M:	Tony Luck <tony.luck@intel.com>
 M:	Fenghua Yu <fenghua.yu@intel.com>
-- 
2.14.0


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

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

[Boris Brezillon]

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

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.

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


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

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

[Boris Brezillon]

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

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.

> 
> 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.
> 
> > +
> >  I3C device connected on the bus follow the generic description (see
> >  Documentation/devicetree/bindings/i3c/i3c.txt for more details).
> >    
> 


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

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

[Przemyslaw Gaj]

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

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

[Boris Brezillon]

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

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

[Przemyslaw Gaj]

The 11/14/2019 12:09, Boris Brezillon wrote:
> EXTERNAL MAIL
> 
> 
> 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)?

Yes, this is the clock driving I3C master logic.

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

Ok. In that case I don't need dt binding.

-- 
-- 
Przemyslaw Gaj

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

RE: [PATCH 0/3] Add data hold delay support

[Vitor Soares]

Hi,

From: Przemyslaw Gaj <pgaj@cadence.com>
Date: Thu, Nov 14, 2019 at 05:51:52

> Add THD_DEL (Data Hold Delay) support. After testing different scenarios,
> on various systems, sometimes there is a need to delay SDA_OUT propagation.
> 
> Adding support which allows to configure that delay using the device
> tree parameter.
> 
> Przemyslaw Gaj (3):
>   i3c: master: add data hold delay support
>   dt-bindings: i3c: Document data hold delay feature
>   MAINTAINERS: add myself as maintainer of Cadence I3C master controller
>     driver
> 
>  Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt | 6 ++++++
>  MAINTAINERS                                               | 6 ++++++
>  drivers/i3c/master/i3c-master-cdns.c                      | 5 ++++-
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> -- 
> 2.14.0

I'm trying to understand this implementation and the use case but I'm not 
understanding.

Could you please elaborate?

Best regards,
Vitor Soares

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

Re: [PATCH 0/3] Add data hold delay support

[Przemyslaw Gaj]

Hi Vitor,

The 11/14/2019 11:58, Vitor Soares wrote:
> 
> Hi,
> 
> From: Przemyslaw Gaj <pgaj@cadence.com>
> Date: Thu, Nov 14, 2019 at 05:51:52
> 
> > Add THD_DEL (Data Hold Delay) support. After testing different scenarios,
> > on various systems, sometimes there is a need to delay SDA_OUT propagation.
> > 
> > Adding support which allows to configure that delay using the device
> > tree parameter.
> > 
> > Przemyslaw Gaj (3):
> >   i3c: master: add data hold delay support
> >   dt-bindings: i3c: Document data hold delay feature
> >   MAINTAINERS: add myself as maintainer of Cadence I3C master controller
> >     driver
> > 
> >  Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt | 6 ++++++
> >  MAINTAINERS                                               | 6 ++++++
> >  drivers/i3c/master/i3c-master-cdns.c                      | 5 ++++-
> >  3 files changed, 16 insertions(+), 1 deletion(-)
> > 
> > -- 
> > 2.14.0
> 
> I'm trying to understand this implementation and the use case but I'm not 
> understanding.
> 
> Could you please elaborate?

There is new controller functionality which allows to meet SDA to SCL
propagation time. Baisically, you can delay SDA propagation using this
functionality to meet I3C MIPI spec timing. This is very useful for some
SoC designs.

From Spec:
Table 75 of MIPI I3C Specification 1.0 (page 142) defines non-zero
minimal tHD_PP timing on master output (Fig 65). This setting allows to
meet this timing on master's soc outputs, regardless of PCB balancing.

> 
> Best regards,
> Vitor Soares

-- 
-- 
Przemyslaw Gaj

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

RE: [PATCH 0/3] Add data hold delay support

[Vitor Soares]

Hi,

From: Przemyslaw Gaj <pgaj@cadence.com>
Date: Thu, Nov 14, 2019 at 12:48:56
> 
> There is new controller functionality which allows to meet SDA to SCL
> propagation time. Baisically, you can delay SDA propagation using this
> functionality to meet I3C MIPI spec timing. This is very useful for some
> SoC designs.
> 
> From Spec:
> Table 75 of MIPI I3C Specification 1.0 (page 142) defines non-zero
> minimal tHD_PP timing on master output (Fig 65). This setting allows to
> meet this timing on master's soc outputs, regardless of PCB balancing.
> 
> > 
> > Best regards,
> > Vitor Soares
> 
> -- 
> -- 
> Przemyslaw Gaj

Thanks for pointing me the right place to read about it.
I would suggest you to put that info in the patch as well.

Best regards,
Vitor Soares
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH 1/3] i3c: master: add data hold delay support

[Vitor Soares]

From: Przemyslaw Gaj <pgaj@cadence.com>
Date: Thu, Nov 14, 2019 at 05:51:53

> This patch adds support for THD_DEL (Data Hold Delay) to Cadence
> I3C master constoller driver.
> 
> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
> ---
>  drivers/i3c/master/i3c-master-cdns.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
> index 10db0bf0655a..90ea98eef905 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -60,6 +60,7 @@
>  #define CTRL_HALT_EN			BIT(30)
>  #define CTRL_MCS			BIT(29)
>  #define CTRL_MCS_EN			BIT(28)
> +#define CTRL_THD_DEL(x) 		(((x) << 24) & GENMASK(25, 24))
>  #define CTRL_HJ_DISEC			BIT(8)
>  #define CTRL_MST_ACK			BIT(7)
>  #define CTRL_HJ_ACK			BIT(6)
> @@ -1186,7 +1187,7 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
>  	struct cdns_i3c_master *master = to_cdns_i3c_master(m);
>  	unsigned long pres_step, sysclk_rate, max_i2cfreq;
>  	struct i3c_bus *bus = i3c_master_get_bus(m);
> -	u32 ctrl, prescl0, prescl1, pres, low;
> +	u32 ctrl, prescl0, prescl1, pres, low, thd_del;
>  	struct i3c_device_info info = { };
>  	int ret, ncycles;
>  
> @@ -1264,6 +1265,8 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
>  	 * We will issue ENTDAA afterwards from the threaded IRQ handler.
>  	 */
>  	ctrl |= CTRL_HJ_ACK | CTRL_HJ_DISEC | CTRL_HALT_EN | CTRL_MCS_EN;
> +	if (!of_property_read_u32(m->dev.of_node, "thd_del", &thd_del))
> +		ctrl |= CTRL_THD_DEL(thd_del);
>  	writel(ctrl, master->regs + CTRL);
>  
>  	cdns_i3c_master_enable(master);
> -- 
> 2.14.0

Please change the commit message so it reflects Cadence driver.

Best regards,
Vitor Soares

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

Re: [PATCH 0/3] Add data hold delay support

[Boris Brezillon]

On Thu, 14 Nov 2019 13:31:33 +0000
Vitor Soares <Vitor.Soares@synopsys.com> wrote:

> Hi,
> 
> From: Przemyslaw Gaj <pgaj@cadence.com>
> Date: Thu, Nov 14, 2019 at 12:48:56
> > 
> > There is new controller functionality which allows to meet SDA to SCL
> > propagation time. Baisically, you can delay SDA propagation using this
> > functionality to meet I3C MIPI spec timing. This is very useful for some
> > SoC designs.
> > 
> > From Spec:
> > Table 75 of MIPI I3C Specification 1.0 (page 142) defines non-zero
> > minimal tHD_PP timing on master output (Fig 65). This setting allows to
> > meet this timing on master's soc outputs, regardless of PCB balancing.
> >   
> > > 
> > > Best regards,
> > > Vitor Soares  
> > 
> > -- 
> > -- 
> > Przemyslaw Gaj  
> 
> Thanks for pointing me the right place to read about it.
> I would suggest you to put that info in the patch as well.

I agree. The spec details should be mentioned in the commit message and
in the code as well (in a comment).

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

Re: [PATCH 3/3] MAINTAINERS: add myself as maintainer of Cadence I3C master controller driver

[Boris Brezillon]

On Thu, 14 Nov 2019 06:51:55 +0100
Przemyslaw Gaj <pgaj@cadence.com> wrote:

> As discussed with Boris Brezillon - I'm adding myself as the maintainer.

Queued to i3c/next.

> 
> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
> ---
>  MAINTAINERS | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c4c532c70b86..afdce16d2be5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7821,6 +7821,12 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt
>  F:	drivers/i3c/master/dw*
>  
> +I3C DRIVER FOR CADENCE I3C MASTER IP
> +M:      Przemysław Gaj <pgaj@cadence.com>
> +S:      Maintained
> +F:      Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
> +F:      drivers/i3c/master/i3c-master-cdns.c
> +
>  IA64 (Itanium) PLATFORM
>  M:	Tony Luck <tony.luck@intel.com>
>  M:	Fenghua Yu <fenghua.yu@intel.com>


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