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

[Kim, Cheol Yong]

On 9/6/2019 4:47 AM, Martin Blumenstingl wrote:
> Hi Rahul,
>
> On Wed, Sep 4, 2019 at 10:04 AM Tanwar, Rahul
> <rahul.tanwar@xxxxxxxxxxxxxxx> wrote:
> > 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.
> this is very sad news
> as far as I can tell many IP cores are similar/identical on
> GRX350/GRX550, LGM and even VRX200
>
> I already know that VRX200 is a legacy product and you won't be supporting it
> once LGM support lands upstream you could add support for
> GRX350/GRX550 with small to medium effort
> that is a big win (in my opinion) because it means happier end-users
> (see XWAY and VRX200 support in OpenWrt for example: while support
> from Intel/Lantiq has died long ago these devices can still run a
> recent LTS kernel and get security updates. without OpenWrt these
> devices would probably end up as electronic waste)

I'm sorry to say that we don't plan to support legacy SOCs. As you might 
know, we tried to upstream some of drivers last year but found a lot of 
problems to support both legacy SOCs and LGM.

It was a real pain to support all of them with limited resources/time 
and we couldn't make it. Our new plan was to reattempt to upstream LGM 
drivers only. This can be more realistic target for us.

> maybe implementing a re-usable regmap clock driver (for mux, gate and
> divider) means less effort (compared to converting everything to
> standard clock ops) for you.
> (we did the switch from standard clock ops to regmap for the Amlogic
> Meson clock drivers when we discovered that there were some non-clock
> registers that belong to other IP blocks in it and it was a lot of
> effort)
> this will allow you to add support for GRX350/GRX550 in the future if
> demand for upstream drivers rises.

I've discussed internally the amount of efforts to create a reusable 
regmap clock driver which might be reused by other companies too.

It seems it requires significant efforts for implementation/tests. As we 
don't plan to support our old SOCs for now, I'm not sure if we need to 
put such a big efforts.

Stephan,

It seems you don't like both meson/qcom regmap clock implementation.

What is your opinion for our current CGU clock driver implementation?


> Martin