2021-05-24 14:46:38

by Chun-Jie Chen

[permalink] [raw]
Subject: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller

This patch adds the new binding documentation of imp i2c wrapper controller
for Mediatek MT8192.

Signed-off-by: Weiyi Lu <[email protected]>
Signed-off-by: chun-jie.chen <[email protected]>
---
.../arm/mediatek/mediatek,imp_iic_wrap.yaml | 80 +++++++++++++++++++
1 file changed, 80 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wrap.yaml

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wrap.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wrap.yaml
new file mode 100644
index 000000000000..fb6cb9e60ee2
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wrap.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/mediatek/mediatek,imp_iic_wrap.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek IMP I2C Wrapper Controller
+
+maintainers:
+ - Chun-Jie Chen <[email protected]>
+
+description:
+ The Mediatek imp i2c wrapper controller provides functional configurations and clocks to the system.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - mediatek,mt8192-imp_iic_wrap_c
+ - mediatek,mt8192-imp_iic_wrap_e
+ - mediatek,mt8192-imp_iic_wrap_s
+ - mediatek,mt8192-imp_iic_wrap_ws
+ - mediatek,mt8192-imp_iic_wrap_w
+ - mediatek,mt8192-imp_iic_wrap_n
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ imp_iic_wrap_c: syscon@11007000 {
+ compatible = "mediatek,mt8192-imp_iic_wrap_c", "syscon";
+ reg = <0x11007000 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ - |
+ imp_iic_wrap_e: syscon@11cb1000 {
+ compatible = "mediatek,mt8192-imp_iic_wrap_e", "syscon";
+ reg = <0x11cb1000 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ - |
+ imp_iic_wrap_s: syscon@11d03000 {
+ compatible = "mediatek,mt8192-imp_iic_wrap_s", "syscon";
+ reg = <0x11d03000 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ - |
+ imp_iic_wrap_ws: syscon@11d23000 {
+ compatible = "mediatek,mt8192-imp_iic_wrap_ws", "syscon";
+ reg = <0x11d23000 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ - |
+ imp_iic_wrap_w: syscon@11e01000 {
+ compatible = "mediatek,mt8192-imp_iic_wrap_w", "syscon";
+ reg = <0x11e01000 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ - |
+ imp_iic_wrap_n: syscon@11f02000 {
+ compatible = "mediatek,mt8192-imp_iic_wrap_n", "syscon";
+ reg = <0x11f02000 0x1000>;
+ #clock-cells = <1>;
+ };
--
2.18.0


2021-06-02 17:14:30

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller

On Mon, May 24, 2021 at 08:20:32PM +0800, Chun-Jie Chen wrote:
> This patch adds the new binding documentation of imp i2c wrapper controller
> for Mediatek MT8192.
>
> Signed-off-by: Weiyi Lu <[email protected]>
> Signed-off-by: chun-jie.chen <[email protected]>
> ---
> .../arm/mediatek/mediatek,imp_iic_wrap.yaml | 80 +++++++++++++++++++
> 1 file changed, 80 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wrap.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wrap.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wrap.yaml
> new file mode 100644
> index 000000000000..fb6cb9e60ee2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wrap.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/mediatek/mediatek,imp_iic_wrap.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek IMP I2C Wrapper Controller
> +
> +maintainers:
> + - Chun-Jie Chen <[email protected]>
> +
> +description:
> + The Mediatek imp i2c wrapper controller provides functional configurations and clocks to the system.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - mediatek,mt8192-imp_iic_wrap_c
> + - mediatek,mt8192-imp_iic_wrap_e
> + - mediatek,mt8192-imp_iic_wrap_s
> + - mediatek,mt8192-imp_iic_wrap_ws
> + - mediatek,mt8192-imp_iic_wrap_w
> + - mediatek,mt8192-imp_iic_wrap_n

Looks to me like these are all the same h/w, but just have differing
sets of clocks. That's not really a reason to have different
compatibles.

If you need to know what clocks are present, you can walk the DT for
all 'clocks' properties matching this clock controller instance. Or use
'clock-indices' to define which ones are present.

Rob

2021-06-07 05:22:12

by Chun-Jie Chen

[permalink] [raw]
Subject: Re: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller

On Wed, 2021-06-02 at 12:12 -0500, Rob Herring wrote:
> On Mon, May 24, 2021 at 08:20:32PM +0800, Chun-Jie Chen wrote:
> > This patch adds the new binding documentation of imp i2c wrapper
> > controller
> > for Mediatek MT8192.
> >
> > Signed-off-by: Weiyi Lu <[email protected]>
> > Signed-off-by: chun-jie.chen <[email protected]>
> > ---
> > .../arm/mediatek/mediatek,imp_iic_wrap.yaml | 80
> > +++++++++++++++++++
> > 1 file changed, 80 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wra
> > p.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_w
> > rap.yaml
> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_w
> > rap.yaml
> > new file mode 100644
> > index 000000000000..fb6cb9e60ee2
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_w
> > rap.yaml
> > @@ -0,0 +1,80 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > http://devicetree.org/schemas/arm/mediatek/mediatek,imp_iic_wrap.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek IMP I2C Wrapper Controller
> > +
> > +maintainers:
> > + - Chun-Jie Chen <[email protected]>
> > +
> > +description:
> > + The Mediatek imp i2c wrapper controller provides functional
> > configurations and clocks to the system.
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - mediatek,mt8192-imp_iic_wrap_c
> > + - mediatek,mt8192-imp_iic_wrap_e
> > + - mediatek,mt8192-imp_iic_wrap_s
> > + - mediatek,mt8192-imp_iic_wrap_ws
> > + - mediatek,mt8192-imp_iic_wrap_w
> > + - mediatek,mt8192-imp_iic_wrap_n
>
> Looks to me like these are all the same h/w, but just have differing
> sets of clocks. That's not really a reason to have different
> compatibles.
>
> If you need to know what clocks are present, you can walk the DT for
> all 'clocks' properties matching this clock controller instance. Or
> use
> 'clock-indices' to define which ones are present.
>
> Rob

Some module is divided to sub-modules which are designed in different
h/w blocks for different usage, and if we want to use the same
compatible to present these h/w blocks, we need to move the clock data
provided by these h/w blocks to dts, but we usually use different
compatible to get the h/w blocks data in
Mediatek's clock driver, so do you suggest to register clock provided
by different h/w blocks using same compatible?

Best Regards,
Chun-Jie

2021-06-09 03:14:04

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller



On 07/06/2021 07:20, Chun-Jie Chen wrote:
> On Wed, 2021-06-02 at 12:12 -0500, Rob Herring wrote:
>> On Mon, May 24, 2021 at 08:20:32PM +0800, Chun-Jie Chen wrote:
>>> This patch adds the new binding documentation of imp i2c wrapper
>>> controller
>>> for Mediatek MT8192.
>>>
>>> Signed-off-by: Weiyi Lu <[email protected]>
>>> Signed-off-by: chun-jie.chen <[email protected]>
>>> ---
>>> .../arm/mediatek/mediatek,imp_iic_wrap.yaml | 80
>>> +++++++++++++++++++
>>> 1 file changed, 80 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_wra
>>> p.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_w
>>> rap.yaml
>>> b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_w
>>> rap.yaml
>>> new file mode 100644
>>> index 000000000000..fb6cb9e60ee2
>>> --- /dev/null
>>> +++
>>> b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imp_iic_w
>>> rap.yaml
>>> @@ -0,0 +1,80 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id:
>>> http://devicetree.org/schemas/arm/mediatek/mediatek,imp_iic_wrap.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MediaTek IMP I2C Wrapper Controller
>>> +
>>> +maintainers:
>>> + - Chun-Jie Chen <[email protected]>
>>> +
>>> +description:
>>> + The Mediatek imp i2c wrapper controller provides functional
>>> configurations and clocks to the system.
>>> +
>>> +properties:
>>> + compatible:
>>> + items:
>>> + - enum:
>>> + - mediatek,mt8192-imp_iic_wrap_c
>>> + - mediatek,mt8192-imp_iic_wrap_e
>>> + - mediatek,mt8192-imp_iic_wrap_s
>>> + - mediatek,mt8192-imp_iic_wrap_ws
>>> + - mediatek,mt8192-imp_iic_wrap_w
>>> + - mediatek,mt8192-imp_iic_wrap_n
>>
>> Looks to me like these are all the same h/w, but just have differing
>> sets of clocks. That's not really a reason to have different
>> compatibles.
>>
>> If you need to know what clocks are present, you can walk the DT for
>> all 'clocks' properties matching this clock controller instance. Or
>> use
>> 'clock-indices' to define which ones are present.
>>
>> Rob
>
> Some module is divided to sub-modules which are designed in different
> h/w blocks for different usage, and if we want to use the same
> compatible to present these h/w blocks, we need to move the clock data
> provided by these h/w blocks to dts, but we usually use different
> compatible to get the h/w blocks data in
> Mediatek's clock driver, so do you suggest to register clock provided
> by different h/w blocks using same compatible?
>

The mapping of them is as following:
imp_iic_wrap_c: 11007000
imp_iic_wrap_e: 11cb1000
imp_iic_wrap_s: 11d03000
imp_iic_wrap_ws: 11d23000
imp_iic_wrap_w: 11e01000
imp_iic_wrap_n: 11f02000

Regards,
Matthias

2021-06-10 17:43:09

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller

Quoting Matthias Brugger (2021-06-08 07:45:49)
>
>
> On 07/06/2021 07:20, Chun-Jie Chen wrote:
> > On Wed, 2021-06-02 at 12:12 -0500, Rob Herring wrote:
> >>> +
> >>> +description:
> >>> + The Mediatek imp i2c wrapper controller provides functional
> >>> configurations and clocks to the system.
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + items:
> >>> + - enum:
> >>> + - mediatek,mt8192-imp_iic_wrap_c
> >>> + - mediatek,mt8192-imp_iic_wrap_e
> >>> + - mediatek,mt8192-imp_iic_wrap_s
> >>> + - mediatek,mt8192-imp_iic_wrap_ws
> >>> + - mediatek,mt8192-imp_iic_wrap_w
> >>> + - mediatek,mt8192-imp_iic_wrap_n
> >>
> >> Looks to me like these are all the same h/w, but just have differing
> >> sets of clocks. That's not really a reason to have different
> >> compatibles.
> >>
> >> If you need to know what clocks are present, you can walk the DT for
> >> all 'clocks' properties matching this clock controller instance. Or
> >> use
> >> 'clock-indices' to define which ones are present.

Is the idea to use clock-indices and then list all the clock ids in
there and match them up at driver probe time to register the clocks
provided by the IO region? Feels like we'll do a lot of parsing at each
boot to match up structures and register clks with the clk framework.

If it's like other SoCs then the clk id maps to a hard macro for a type
of clk, and those hard macros have been glued together with other clks
and then partitioned into different IO regions to make up a clock
controller. Or maybe in this case, those clk hard macros have been
scattered into each IP block like SPI, i2c, uart, etc. so that the clock
controller doesn't really exist and merely the gates and rate control
(mux/divider) for the clk that's clocking some particular IP block all
live inside the IP wrapper. If it's this case then I hope there are a
bunch of PLLs that are fixed rate so that the i2c clk doesn't have to go
outside the wrapper to change frequency (of which there should be two
"standard" frequencies anyway).

> >>
> >> Rob
> >
> > Some module is divided to sub-modules which are designed in different
> > h/w blocks for different usage, and if we want to use the same
> > compatible to present these h/w blocks, we need to move the clock data
> > provided by these h/w blocks to dts, but we usually use different
> > compatible to get the h/w blocks data in
> > Mediatek's clock driver, so do you suggest to register clock provided
> > by different h/w blocks using same compatible?
> >
>
> The mapping of them is as following:
> imp_iic_wrap_c: 11007000
> imp_iic_wrap_e: 11cb1000
> imp_iic_wrap_s: 11d03000
> imp_iic_wrap_ws: 11d23000
> imp_iic_wrap_w: 11e01000
> imp_iic_wrap_n: 11f02000
>

Sure. What is their purpose though? Are they simply a bunch of different
i2c clks?

2021-06-11 10:00:13

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller



On 10/06/2021 19:41, Stephen Boyd wrote:
> Quoting Matthias Brugger (2021-06-08 07:45:49)
>>
>>
>> On 07/06/2021 07:20, Chun-Jie Chen wrote:
>>> On Wed, 2021-06-02 at 12:12 -0500, Rob Herring wrote:
>>>>> +
>>>>> +description:
>>>>> + The Mediatek imp i2c wrapper controller provides functional
>>>>> configurations and clocks to the system.
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + items:
>>>>> + - enum:
>>>>> + - mediatek,mt8192-imp_iic_wrap_c
>>>>> + - mediatek,mt8192-imp_iic_wrap_e
>>>>> + - mediatek,mt8192-imp_iic_wrap_s
>>>>> + - mediatek,mt8192-imp_iic_wrap_ws
>>>>> + - mediatek,mt8192-imp_iic_wrap_w
>>>>> + - mediatek,mt8192-imp_iic_wrap_n
>>>>
>>>> Looks to me like these are all the same h/w, but just have differing
>>>> sets of clocks. That's not really a reason to have different
>>>> compatibles.
>>>>
>>>> If you need to know what clocks are present, you can walk the DT for
>>>> all 'clocks' properties matching this clock controller instance. Or
>>>> use
>>>> 'clock-indices' to define which ones are present.
>
> Is the idea to use clock-indices and then list all the clock ids in
> there and match them up at driver probe time to register the clocks
> provided by the IO region? Feels like we'll do a lot of parsing at each
> boot to match up structures and register clks with the clk framework.
>
> If it's like other SoCs then the clk id maps to a hard macro for a type
> of clk, and those hard macros have been glued together with other clks
> and then partitioned into different IO regions to make up a clock
> controller. Or maybe in this case, those clk hard macros have been
> scattered into each IP block like SPI, i2c, uart, etc. so that the clock
> controller doesn't really exist and merely the gates and rate control
> (mux/divider) for the clk that's clocking some particular IP block all
> live inside the IP wrapper. If it's this case then I hope there are a
> bunch of PLLs that are fixed rate so that the i2c clk doesn't have to go
> outside the wrapper to change frequency (of which there should be two
> "standard" frequencies anyway).
>
>>>>
>>>> Rob
>>>
>>> Some module is divided to sub-modules which are designed in different
>>> h/w blocks for different usage, and if we want to use the same
>>> compatible to present these h/w blocks, we need to move the clock data
>>> provided by these h/w blocks to dts, but we usually use different
>>> compatible to get the h/w blocks data in
>>> Mediatek's clock driver, so do you suggest to register clock provided
>>> by different h/w blocks using same compatible?
>>>
>>
>> The mapping of them is as following:
>> imp_iic_wrap_c: 11007000
>> imp_iic_wrap_e: 11cb1000
>> imp_iic_wrap_s: 11d03000
>> imp_iic_wrap_ws: 11d23000
>> imp_iic_wrap_w: 11e01000
>> imp_iic_wrap_n: 11f02000
>>
>
> Sure. What is their purpose though? Are they simply a bunch of different
> i2c clks?
>

That would be need to be answered by MediaTek as I don't have access to any
documentation.

Regards,
Matthias

2021-06-15 02:36:39

by Chun-Jie Chen

[permalink] [raw]
Subject: Re: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller

On Fri, 2021-06-11 at 11:56 +0200, Matthias Brugger wrote:
>
> On 10/06/2021 19:41, Stephen Boyd wrote:
> > Quoting Matthias Brugger (2021-06-08 07:45:49)
> > >
> > >
> > > On 07/06/2021 07:20, Chun-Jie Chen wrote:
> > > > On Wed, 2021-06-02 at 12:12 -0500, Rob Herring wrote:
> > > > > > +
> > > > > > +description:
> > > > > > + The Mediatek imp i2c wrapper controller provides
> > > > > > functional
> > > > > > configurations and clocks to the system.
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + items:
> > > > > > + - enum:
> > > > > > + - mediatek,mt8192-imp_iic_wrap_c
> > > > > > + - mediatek,mt8192-imp_iic_wrap_e
> > > > > > + - mediatek,mt8192-imp_iic_wrap_s
> > > > > > + - mediatek,mt8192-imp_iic_wrap_ws
> > > > > > + - mediatek,mt8192-imp_iic_wrap_w
> > > > > > + - mediatek,mt8192-imp_iic_wrap_n
> > > > >
> > > > > Looks to me like these are all the same h/w, but just have
> > > > > differing
> > > > > sets of clocks. That's not really a reason to have different
> > > > > compatibles.
> > > > >
> > > > > If you need to know what clocks are present, you can walk the
> > > > > DT for
> > > > > all 'clocks' properties matching this clock controller
> > > > > instance. Or
> > > > > use
> > > > > 'clock-indices' to define which ones are present.
> >
> > Is the idea to use clock-indices and then list all the clock ids in
> > there and match them up at driver probe time to register the clocks
> > provided by the IO region? Feels like we'll do a lot of parsing at
> > each
> > boot to match up structures and register clks with the clk
> > framework.
> >
> > If it's like other SoCs then the clk id maps to a hard macro for a
> > type
> > of clk, and those hard macros have been glued together with other
> > clks
> > and then partitioned into different IO regions to make up a clock
> > controller. Or maybe in this case, those clk hard macros have been
> > scattered into each IP block like SPI, i2c, uart, etc. so that the
> > clock
> > controller doesn't really exist and merely the gates and rate
> > control
> > (mux/divider) for the clk that's clocking some particular IP block
> > all
> > live inside the IP wrapper. If it's this case then I hope there are
> > a
> > bunch of PLLs that are fixed rate so that the i2c clk doesn't have
> > to go
> > outside the wrapper to change frequency (of which there should be
> > two
> > "standard" frequencies anyway).
> >
> > > > >
> > > > > Rob
> > > >
> > > > Some module is divided to sub-modules which are designed in
> > > > different
> > > > h/w blocks for different usage, and if we want to use the same
> > > > compatible to present these h/w blocks, we need to move the
> > > > clock data
> > > > provided by these h/w blocks to dts, but we usually use
> > > > different
> > > > compatible to get the h/w blocks data in
> > > > Mediatek's clock driver, so do you suggest to register clock
> > > > provided
> > > > by different h/w blocks using same compatible?
> > > >
> > >
> > > The mapping of them is as following:
> > > imp_iic_wrap_c: 11007000
> > > imp_iic_wrap_e: 11cb1000
> > > imp_iic_wrap_s: 11d03000
> > > imp_iic_wrap_ws: 11d23000
> > > imp_iic_wrap_w: 11e01000
> > > imp_iic_wrap_n: 11f02000
> > >
> >
> > Sure. What is their purpose though? Are they simply a bunch of
> > different
> > i2c clks?
> >
>
> That would be need to be answered by MediaTek as I don't have access
> to any
> documentation.
>
> Regards,
> Matthias

We describe which clock controllers are exist in dts and
get the clock data provided by clock controller in driver data
by matching device compatible.

The clock data contains several clocks which includes the clock index,
parent clock source and the details of reg control inside the IP block
of clock controller.

In MT8192 platform, some IP block is divide to several sub-blocks and
each sub-block provides clock control by itself.

Best Regards,
Chun-Jie

2021-06-18 06:48:11

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller

On Wed, Jun 16, 2021 at 2:34 AM Chun-Jie Chen
<[email protected]> wrote:
>
> On Fri, 2021-06-11 at 11:56 +0200, Matthias Brugger wrote:
> >
> > On 10/06/2021 19:41, Stephen Boyd wrote:
> > > Quoting Matthias Brugger (2021-06-08 07:45:49)
> > > >
> > > >
> > > > On 07/06/2021 07:20, Chun-Jie Chen wrote:
> > > > > On Wed, 2021-06-02 at 12:12 -0500, Rob Herring wrote:
> > > > > > > +
> > > > > > > +description:
> > > > > > > + The Mediatek imp i2c wrapper controller provides
> > > > > > > functional
> > > > > > > configurations and clocks to the system.
> > > > > > > +
> > > > > > > +properties:
> > > > > > > + compatible:
> > > > > > > + items:
> > > > > > > + - enum:
> > > > > > > + - mediatek,mt8192-imp_iic_wrap_c
> > > > > > > + - mediatek,mt8192-imp_iic_wrap_e
> > > > > > > + - mediatek,mt8192-imp_iic_wrap_s
> > > > > > > + - mediatek,mt8192-imp_iic_wrap_ws
> > > > > > > + - mediatek,mt8192-imp_iic_wrap_w
> > > > > > > + - mediatek,mt8192-imp_iic_wrap_n
> > > > > >
> > > > > > Looks to me like these are all the same h/w, but just have
> > > > > > differing
> > > > > > sets of clocks. That's not really a reason to have different
> > > > > > compatibles.
> > > > > >
> > > > > > If you need to know what clocks are present, you can walk the
> > > > > > DT for
> > > > > > all 'clocks' properties matching this clock controller
> > > > > > instance. Or
> > > > > > use
> > > > > > 'clock-indices' to define which ones are present.
> > >
> > > Is the idea to use clock-indices and then list all the clock ids in
> > > there and match them up at driver probe time to register the clocks
> > > provided by the IO region? Feels like we'll do a lot of parsing at
> > > each
> > > boot to match up structures and register clks with the clk
> > > framework.
> > >
> > > If it's like other SoCs then the clk id maps to a hard macro for a
> > > type
> > > of clk, and those hard macros have been glued together with other
> > > clks
> > > and then partitioned into different IO regions to make up a clock
> > > controller. Or maybe in this case, those clk hard macros have been
> > > scattered into each IP block like SPI, i2c, uart, etc. so that the
> > > clock
> > > controller doesn't really exist and merely the gates and rate
> > > control
> > > (mux/divider) for the clk that's clocking some particular IP block
> > > all
> > > live inside the IP wrapper. If it's this case then I hope there are
> > > a
> > > bunch of PLLs that are fixed rate so that the i2c clk doesn't have
> > > to go
> > > outside the wrapper to change frequency (of which there should be
> > > two
> > > "standard" frequencies anyway).
> > >
> > > > > >
> > > > > > Rob
> > > > >
> > > > > Some module is divided to sub-modules which are designed in
> > > > > different
> > > > > h/w blocks for different usage, and if we want to use the same
> > > > > compatible to present these h/w blocks, we need to move the
> > > > > clock data
> > > > > provided by these h/w blocks to dts, but we usually use
> > > > > different
> > > > > compatible to get the h/w blocks data in
> > > > > Mediatek's clock driver, so do you suggest to register clock
> > > > > provided
> > > > > by different h/w blocks using same compatible?
> > > > >
> > > >
> > > > The mapping of them is as following:
> > > > imp_iic_wrap_c: 11007000
> > > > imp_iic_wrap_e: 11cb1000
> > > > imp_iic_wrap_s: 11d03000
> > > > imp_iic_wrap_ws: 11d23000
> > > > imp_iic_wrap_w: 11e01000
> > > > imp_iic_wrap_n: 11f02000
> > > >
> > >
> > > Sure. What is their purpose though? Are they simply a bunch of
> > > different
> > > i2c clks?
> > >
> >
> > That would be need to be answered by MediaTek as I don't have access
> > to any
> > documentation.
> >
> > Regards,
> > Matthias
>
> We describe which clock controllers are exist in dts and
> get the clock data provided by clock controller in driver data
> by matching device compatible.
>
> The clock data contains several clocks which includes the clock index,
> parent clock source and the details of reg control inside the IP block
> of clock controller.
>
> In MT8192 platform, some IP block is divide to several sub-blocks and
> each sub-block provides clock control by itself.

Some more information:

Based on what I read in the datasheets, I'm guessing that MediaTek groups
the I2C controllers into several groups and places them in different parts
of the die. The suffix of imp_iic_wrap_XXX is likely pointing to the
placement of the group. And the imp_iic_wrap_XXX is what the name suggests
a wrapper around the group of I2C controllers. The wrapper contains clock
and reset controls, as well as other things that I can't make out.


ChenYu

2021-06-18 17:19:09

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller



On 18/06/2021 08:32, Chen-Yu Tsai wrote:
> On Wed, Jun 16, 2021 at 2:34 AM Chun-Jie Chen
> <[email protected]> wrote:
>>
>> On Fri, 2021-06-11 at 11:56 +0200, Matthias Brugger wrote:
>>>
>>> On 10/06/2021 19:41, Stephen Boyd wrote:
>>>> Quoting Matthias Brugger (2021-06-08 07:45:49)
>>>>>
>>>>>
>>>>> On 07/06/2021 07:20, Chun-Jie Chen wrote:
>>>>>> On Wed, 2021-06-02 at 12:12 -0500, Rob Herring wrote:
>>>>>>>> +
>>>>>>>> +description:
>>>>>>>> + The Mediatek imp i2c wrapper controller provides
>>>>>>>> functional
>>>>>>>> configurations and clocks to the system.
>>>>>>>> +
>>>>>>>> +properties:
>>>>>>>> + compatible:
>>>>>>>> + items:
>>>>>>>> + - enum:
>>>>>>>> + - mediatek,mt8192-imp_iic_wrap_c
>>>>>>>> + - mediatek,mt8192-imp_iic_wrap_e
>>>>>>>> + - mediatek,mt8192-imp_iic_wrap_s
>>>>>>>> + - mediatek,mt8192-imp_iic_wrap_ws
>>>>>>>> + - mediatek,mt8192-imp_iic_wrap_w
>>>>>>>> + - mediatek,mt8192-imp_iic_wrap_n
>>>>>>>
>>>>>>> Looks to me like these are all the same h/w, but just have
>>>>>>> differing
>>>>>>> sets of clocks. That's not really a reason to have different
>>>>>>> compatibles.
>>>>>>>
>>>>>>> If you need to know what clocks are present, you can walk the
>>>>>>> DT for
>>>>>>> all 'clocks' properties matching this clock controller
>>>>>>> instance. Or
>>>>>>> use
>>>>>>> 'clock-indices' to define which ones are present.
>>>>
>>>> Is the idea to use clock-indices and then list all the clock ids in
>>>> there and match them up at driver probe time to register the clocks
>>>> provided by the IO region? Feels like we'll do a lot of parsing at
>>>> each
>>>> boot to match up structures and register clks with the clk
>>>> framework.
>>>>
>>>> If it's like other SoCs then the clk id maps to a hard macro for a
>>>> type
>>>> of clk, and those hard macros have been glued together with other
>>>> clks
>>>> and then partitioned into different IO regions to make up a clock
>>>> controller. Or maybe in this case, those clk hard macros have been
>>>> scattered into each IP block like SPI, i2c, uart, etc. so that the
>>>> clock
>>>> controller doesn't really exist and merely the gates and rate
>>>> control
>>>> (mux/divider) for the clk that's clocking some particular IP block
>>>> all
>>>> live inside the IP wrapper. If it's this case then I hope there are
>>>> a
>>>> bunch of PLLs that are fixed rate so that the i2c clk doesn't have
>>>> to go
>>>> outside the wrapper to change frequency (of which there should be
>>>> two
>>>> "standard" frequencies anyway).
>>>>
>>>>>>>
>>>>>>> Rob
>>>>>>
>>>>>> Some module is divided to sub-modules which are designed in
>>>>>> different
>>>>>> h/w blocks for different usage, and if we want to use the same
>>>>>> compatible to present these h/w blocks, we need to move the
>>>>>> clock data
>>>>>> provided by these h/w blocks to dts, but we usually use
>>>>>> different
>>>>>> compatible to get the h/w blocks data in
>>>>>> Mediatek's clock driver, so do you suggest to register clock
>>>>>> provided
>>>>>> by different h/w blocks using same compatible?
>>>>>>
>>>>>
>>>>> The mapping of them is as following:
>>>>> imp_iic_wrap_c: 11007000
>>>>> imp_iic_wrap_e: 11cb1000
>>>>> imp_iic_wrap_s: 11d03000
>>>>> imp_iic_wrap_ws: 11d23000
>>>>> imp_iic_wrap_w: 11e01000
>>>>> imp_iic_wrap_n: 11f02000
>>>>>
>>>>
>>>> Sure. What is their purpose though? Are they simply a bunch of
>>>> different
>>>> i2c clks?
>>>>
>>>
>>> That would be need to be answered by MediaTek as I don't have access
>>> to any
>>> documentation.
>>>
>>> Regards,
>>> Matthias
>>
>> We describe which clock controllers are exist in dts and
>> get the clock data provided by clock controller in driver data
>> by matching device compatible.
>>
>> The clock data contains several clocks which includes the clock index,
>> parent clock source and the details of reg control inside the IP block
>> of clock controller.
>>
>> In MT8192 platform, some IP block is divide to several sub-blocks and
>> each sub-block provides clock control by itself.
>
> Some more information:
>
> Based on what I read in the datasheets, I'm guessing that MediaTek groups
> the I2C controllers into several groups and places them in different parts
> of the die. The suffix of imp_iic_wrap_XXX is likely pointing to the
> placement of the group. And the imp_iic_wrap_XXX is what the name suggests
> a wrapper around the group of I2C controllers. The wrapper contains clock
> and reset controls, as well as other things that I can't make out.
>

Thanks for the clarification. If the wrapper contains more then just clocks,
then probably we will need a solution as done by MMSYS subsystem. Would be good
if you could work with MediaTek to find out what exactly this wrappers contain,
to get a better picture of if we need an additional driver or not.

Regards,
Matthias

2021-06-28 13:00:29

by Chun-Jie Chen

[permalink] [raw]
Subject: Re: [PATCH v9 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of imp i2c wrapper controller

On Fri, 2021-06-18 at 15:50 +0200, Matthias Brugger wrote:
>
> On 18/06/2021 08:32, Chen-Yu Tsai wrote:
> > On Wed, Jun 16, 2021 at 2:34 AM Chun-Jie Chen
> > <[email protected]> wrote:
> > >
> > > On Fri, 2021-06-11 at 11:56 +0200, Matthias Brugger wrote:
> > > >
> > > > On 10/06/2021 19:41, Stephen Boyd wrote:
> > > > > Quoting Matthias Brugger (2021-06-08 07:45:49)
> > > > > >
> > > > > >
> > > > > > On 07/06/2021 07:20, Chun-Jie Chen wrote:
> > > > > > > On Wed, 2021-06-02 at 12:12 -0500, Rob Herring wrote:
> > > > > > > > > +
> > > > > > > > > +description:
> > > > > > > > > + The Mediatek imp i2c wrapper controller provides
> > > > > > > > > functional
> > > > > > > > > configurations and clocks to the system.
> > > > > > > > > +
> > > > > > > > > +properties:
> > > > > > > > > + compatible:
> > > > > > > > > + items:
> > > > > > > > > + - enum:
> > > > > > > > > + - mediatek,mt8192-imp_iic_wrap_c
> > > > > > > > > + - mediatek,mt8192-imp_iic_wrap_e
> > > > > > > > > + - mediatek,mt8192-imp_iic_wrap_s
> > > > > > > > > + - mediatek,mt8192-imp_iic_wrap_ws
> > > > > > > > > + - mediatek,mt8192-imp_iic_wrap_w
> > > > > > > > > + - mediatek,mt8192-imp_iic_wrap_n
> > > > > > > >
> > > > > > > > Looks to me like these are all the same h/w, but just
> > > > > > > > have
> > > > > > > > differing
> > > > > > > > sets of clocks. That's not really a reason to have
> > > > > > > > different
> > > > > > > > compatibles.
> > > > > > > >
> > > > > > > > If you need to know what clocks are present, you can
> > > > > > > > walk the
> > > > > > > > DT for
> > > > > > > > all 'clocks' properties matching this clock controller
> > > > > > > > instance. Or
> > > > > > > > use
> > > > > > > > 'clock-indices' to define which ones are present.
> > > > >
> > > > > Is the idea to use clock-indices and then list all the clock
> > > > > ids in
> > > > > there and match them up at driver probe time to register the
> > > > > clocks
> > > > > provided by the IO region? Feels like we'll do a lot of
> > > > > parsing at
> > > > > each
> > > > > boot to match up structures and register clks with the clk
> > > > > framework.
> > > > >
> > > > > If it's like other SoCs then the clk id maps to a hard macro
> > > > > for a
> > > > > type
> > > > > of clk, and those hard macros have been glued together with
> > > > > other
> > > > > clks
> > > > > and then partitioned into different IO regions to make up a
> > > > > clock
> > > > > controller. Or maybe in this case, those clk hard macros have
> > > > > been
> > > > > scattered into each IP block like SPI, i2c, uart, etc. so
> > > > > that the
> > > > > clock
> > > > > controller doesn't really exist and merely the gates and rate
> > > > > control
> > > > > (mux/divider) for the clk that's clocking some particular IP
> > > > > block
> > > > > all
> > > > > live inside the IP wrapper. If it's this case then I hope
> > > > > there are
> > > > > a
> > > > > bunch of PLLs that are fixed rate so that the i2c clk doesn't
> > > > > have
> > > > > to go
> > > > > outside the wrapper to change frequency (of which there
> > > > > should be
> > > > > two
> > > > > "standard" frequencies anyway).
> > > > >
> > > > > > > >
> > > > > > > > Rob
> > > > > > >
> > > > > > > Some module is divided to sub-modules which are designed
> > > > > > > in
> > > > > > > different
> > > > > > > h/w blocks for different usage, and if we want to use the
> > > > > > > same
> > > > > > > compatible to present these h/w blocks, we need to move
> > > > > > > the
> > > > > > > clock data
> > > > > > > provided by these h/w blocks to dts, but we usually use
> > > > > > > different
> > > > > > > compatible to get the h/w blocks data in
> > > > > > > Mediatek's clock driver, so do you suggest to register
> > > > > > > clock
> > > > > > > provided
> > > > > > > by different h/w blocks using same compatible?
> > > > > > >
> > > > > >
> > > > > > The mapping of them is as following:
> > > > > > imp_iic_wrap_c: 11007000
> > > > > > imp_iic_wrap_e: 11cb1000
> > > > > > imp_iic_wrap_s: 11d03000
> > > > > > imp_iic_wrap_ws: 11d23000
> > > > > > imp_iic_wrap_w: 11e01000
> > > > > > imp_iic_wrap_n: 11f02000
> > > > > >
> > > > >
> > > > > Sure. What is their purpose though? Are they simply a bunch
> > > > > of
> > > > > different
> > > > > i2c clks?
> > > > >
> > > >
> > > > That would be need to be answered by MediaTek as I don't have
> > > > access
> > > > to any
> > > > documentation.
> > > >
> > > > Regards,
> > > > Matthias
> > >
> > > We describe which clock controllers are exist in dts and
> > > get the clock data provided by clock controller in driver data
> > > by matching device compatible.
> > >
> > > The clock data contains several clocks which includes the clock
> > > index,
> > > parent clock source and the details of reg control inside the IP
> > > block
> > > of clock controller.
> > >
> > > In MT8192 platform, some IP block is divide to several sub-blocks
> > > and
> > > each sub-block provides clock control by itself.
> >
> > Some more information:
> >
> > Based on what I read in the datasheets, I'm guessing that MediaTek
> > groups
> > the I2C controllers into several groups and places them in
> > different parts
> > of the die. The suffix of imp_iic_wrap_XXX is likely pointing to
> > the
> > placement of the group. And the imp_iic_wrap_XXX is what the name
> > suggests
> > a wrapper around the group of I2C controllers. The wrapper contains
> > clock
> > and reset controls, as well as other things that I can't make out.
> >
>
> Thanks for the clarification. If the wrapper contains more then just
> clocks,
> then probably we will need a solution as done by MMSYS subsystem.
> Would be good
> if you could work with MediaTek to find out what exactly this
> wrappers contain,
> to get a better picture of if we need an additional driver or not.
>
> Regards,
> Matthias

+ Kewei

Hi Matthias,

After discuss with i2c owenr(Kewei) in mediatek, although the HW block
of imp_iic_wrap_XXX contains more than just clock, but we only use the
clock part in this HW block and just consider that imp_iic_wrap_XXX is
a pure clock provider.

Thanks!
Best Regards,
Chun-Jie