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:

quoted

On Wed, Jun 16, 2021 at 2:34 AM Chun-Jie Chen
[off-list ref] wrote:

quoted

On Fri, 2021-06-11 at 11:56 +0200, Matthias Brugger wrote:

quoted

On 10/06/2021 19:41, Stephen Boyd wrote:

quoted

Quoting Matthias Brugger (2021-06-08 07:45:49)

quoted

On 07/06/2021 07:20, Chun-Jie Chen wrote:

quoted

On Wed, 2021-06-02 at 12:12 -0500, Rob Herring wrote:

quoted

quoted

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

quoted

quoted

quoted

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