Re: [PATCH v1 1/2] clk: intel: Add CGU clock driver for a new SoC

From: Tanwar, Rahul
Date: Wed Sep 04 2019 - 04:04:07 EST

Hi Martin,

On 4/9/2019 2:53 AM, Martin Blumenstingl wrote:
My understanding is that if we do not use syscon, then there is no
point in using regmap because this driver uses simple 32 bit register
access. Can directly read/write registers using readl() & writel().

Would you agree ?
if there was only the LGM SoC then I would say: drop regmap

however, last year a driver for the GRX350/GRX550 SoCs was proposed: [0]
this was never updated but it seems to use the same "framework" as the
LGM driver
with this in mind I am for keeping regmap support because.
I think it will be easier to add support for old SoCs like
GRX350/GRX550 (but also VRX200), because the PLL sub-driver (I am
assuming that it is similar on all SoCs) or some other helpers can be
re-used across various SoCs instead of "duplicating" code (where one
variant would use regmap and the other readl/writel).

Earlier, we had discussed about it in our team. There are no plans to

upstream mips based platform code, past up-streaming efforts for mips

platforms were also dropped. GRX350/GRX550/VRX200 are all mips

based platforms. Plan is to upstream only x86 based platforms. In-fact,

i had removed GRX & other older SoCs support from this driver before

sending for review. So we can consider only x86 based LGM family of

SoCs for this driver & all of them will be reusing same IP.

there's not a single other "select OF_EARLY_FLATTREE" in driver/clk
I'm not saying this is wrong but it makes me curious why you need this

We need OF_EARLY_FLATTREE for LGM. But adding a new x86
platform for LGM is discouraged because that would lead to too
many platforms. Only differentiating factor for LGM is CPU model
ID but it can differentiate only at run time. So i had no option
other then enabling it with some LGM specific core system module
driver and CGU seemed perfect for this purpose.
so when my x86 kernel maintainer enables CONFIG_INTEL_LGM_CGU_CLK then
OF_EARLY_FLATTREE is enabled as well.
does this hurt any existing x86 platform? if not: why can't we enable
it for x86 unconditionally?

IMHO, it will not hurt any other existing x86 platform but enabling it for

x86 unconditionally also doesn't sound like a good idea. I now get your

point that enabling OF_EARLY_FLATTREE here is a bit odd. I will remove

it in next patch.



I went through meson & qcom regmap clock code. Agree, it can be
reused for mux, divider and gate. But as mentioned above, i am now
considering to move away from using regmap.
thank you for evaluating them. let's continue the discussion above
whether regmap should be used - after that we decide (if needed) which
regmap implementation to use