2021-10-12 14:09:33

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH 0/2] i2c: at91: Add support for programmable clock source

Add support to be able to set BRSRCCLK. This feature is support on lan966x

Horatiu Vultur (2):
dt-bindings: i2c: at91: Extend compatible list for lan966x
i2c: at91: add support for brsrcclk

.../devicetree/bindings/i2c/i2c-at91.txt | 6 +++--
drivers/i2c/busses/i2c-at91-core.c | 16 +++++++++++++
drivers/i2c/busses/i2c-at91-master.c | 23 +++++++++++++++++--
drivers/i2c/busses/i2c-at91.h | 1 +
4 files changed, 42 insertions(+), 4 deletions(-)

--
2.33.0


2021-10-12 14:13:03

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: i2c: at91: Extend compatible list for lan966x

Extend compatible list and the i2c-sda-hold-time-ns property
with 'microchip,lan966x-i2c'

Signed-off-by: Horatiu Vultur <[email protected]>
---
Documentation/devicetree/bindings/i2c/i2c-at91.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-at91.txt b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
index 2015f50aed0f..d3b5ed081597 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-at91.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-at91.txt
@@ -10,7 +10,8 @@ Required properties :
"atmel,at91sam9x5-i2c",
"atmel,sama5d4-i2c",
"atmel,sama5d2-i2c",
- "microchip,sam9x60-i2c".
+ "microchip,sam9x60-i2c",
+ "microchip,lan966x-i2c".
- reg: physical base address of the controller and length of memory mapped
region.
- interrupts: interrupt number to the cpu.
@@ -27,7 +28,8 @@ Optional properties:
- i2c-sda-hold-time-ns: TWD hold time, only available for:
"atmel,sama5d4-i2c",
"atmel,sama5d2-i2c",
- "microchip,sam9x60-i2c".
+ "microchip,sam9x60-i2c",
+ "microchip,lan966x-i2c".
- scl-gpios: specify the gpio related to SCL pin
- sda-gpios: specify the gpio related to SDA pin
- pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
--
2.33.0

2021-10-13 08:51:03

by Codrin Ciubotariu

[permalink] [raw]
Subject: Re: [PATCH 0/2] i2c: at91: Add support for programmable clock source

On 12.10.2021 17:07, Horatiu Vultur wrote:
> Add support to be able to set BRSRCCLK. This feature is support on lan966x
>
> Horatiu Vultur (2):
> dt-bindings: i2c: at91: Extend compatible list for lan966x
> i2c: at91: add support for brsrcclk
>
> .../devicetree/bindings/i2c/i2c-at91.txt | 6 +++--
> drivers/i2c/busses/i2c-at91-core.c | 16 +++++++++++++
> drivers/i2c/busses/i2c-at91-master.c | 23 +++++++++++++++++--
> drivers/i2c/busses/i2c-at91.h | 1 +
> 4 files changed, 42 insertions(+), 4 deletions(-)
>

Hi Horatiu,

From what I understand, on your DTS, you replaced the peripheral clock
with the GCLK in the I2C node. This means that you are forcing all the
variants that support clk_brsrcclk to treat the current clock as GCLK.
This is not necessarily correct, since this newer variants can also work
fine with only the peripheral clock and we should keep these option
available.

I would add an optional GCLK clock binding in the I2C node. This way
GCLK will be used only if it is present in DT and clk_brsrcclk set.

Best regards,
Codrin

2021-10-13 11:44:28

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH 0/2] i2c: at91: Add support for programmable clock source

The 10/13/2021 08:49, Codrin Ciubotariu - M19940 wrote:
> On 12.10.2021 17:07, Horatiu Vultur wrote:
> > Add support to be able to set BRSRCCLK. This feature is support on lan966x
> >
> > Horatiu Vultur (2):
> > dt-bindings: i2c: at91: Extend compatible list for lan966x
> > i2c: at91: add support for brsrcclk
> >
> > .../devicetree/bindings/i2c/i2c-at91.txt | 6 +++--
> > drivers/i2c/busses/i2c-at91-core.c | 16 +++++++++++++
> > drivers/i2c/busses/i2c-at91-master.c | 23 +++++++++++++++++--
> > drivers/i2c/busses/i2c-at91.h | 1 +
> > 4 files changed, 42 insertions(+), 4 deletions(-)
> >
>
> Hi Horatiu,

Hi Codrin,

>
> From what I understand, on your DTS, you replaced the peripheral clock
> with the GCLK in the I2C node. This means that you are forcing all the
> variants that support clk_brsrcclk to treat the current clock as GCLK.
> This is not necessarily correct, since this newer variants can also work
> fine with only the peripheral clock and we should keep these option
> available.
>
> I would add an optional GCLK clock binding in the I2C node. This way
> GCLK will be used only if it is present in DT and clk_brsrcclk set.

Thanks for the explanation.

I think actually I will drop this patch series because apparently
lan966x works fine also with the peripheral clock. So then no changes
are required.

If you think is worth it, I can do another version with the proposed
changes.

>
> Best regards,
> Codrin

--
/Horatiu

2021-10-13 13:13:16

by Codrin Ciubotariu

[permalink] [raw]
Subject: Re: [PATCH 0/2] i2c: at91: Add support for programmable clock source

> I think actually I will drop this patch series because apparently
> lan966x works fine also with the peripheral clock. So then no changes
> are required.
>
> If you think is worth it, I can do another version with the proposed
> changes.

Probably not until we have an implemented good reason to use GCLK.

Best regards,
Codrin

2021-10-22 23:39:59

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: i2c: at91: Extend compatible list for lan966x

On Tue, 12 Oct 2021 16:07:17 +0200, Horatiu Vultur wrote:
> Extend compatible list and the i2c-sda-hold-time-ns property
> with 'microchip,lan966x-i2c'
>
> Signed-off-by: Horatiu Vultur <[email protected]>
> ---
> Documentation/devicetree/bindings/i2c/i2c-at91.txt | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>

Acked-by: Rob Herring <[email protected]>

2021-11-06 00:47:17

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 0/2] i2c: at91: Add support for programmable clock source


> I think actually I will drop this patch series because apparently
> lan966x works fine also with the peripheral clock. So then no changes
> are required.

Not even patch 1/2?


Attachments:
(No filename) (186.00 B)
signature.asc (849.00 B)
Download all attachments

2021-11-08 12:32:21

by Codrin Ciubotariu

[permalink] [raw]
Subject: Re: [PATCH 0/2] i2c: at91: Add support for programmable clock source

On 05.11.2021 23:47, Wolfram Sang wrote:
>
>> I think actually I will drop this patch series because apparently
>> lan966x works fine also with the peripheral clock. So then no changes
>> are required.
>
> Not even patch 1/2?
>

we can keep the new compatible, but patch 2/2 needs to be split.

Best regards,
Codrin

2021-11-08 12:46:11

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH 0/2] i2c: at91: Add support for programmable clock source

The 11/08/2021 08:35, Codrin Ciubotariu - M19940 wrote:
> On 05.11.2021 23:47, Wolfram Sang wrote:
> >
> >> I think actually I will drop this patch series because apparently
> >> lan966x works fine also with the peripheral clock. So then no changes
> >> are required.
> >
> > Not even patch 1/2?
> >
>
> we can keep the new compatible, but patch 2/2 needs to be split.

For me it is OK to use the compatible string 'microchip,sam9x60-i2c'

>
> Best regards,
> Codrin

--
/Horatiu

2021-11-29 10:42:45

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 0/2] i2c: at91: Add support for programmable clock source


> > > Not even patch 1/2?
> >
> > we can keep the new compatible, but patch 2/2 needs to be split.
>
> For me it is OK to use the compatible string 'microchip,sam9x60-i2c'

I'll drop this patch for now. If anyone is still interested in it,
please resend.


Attachments:
(No filename) (259.00 B)
signature.asc (833.00 B)
Download all attachments