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

From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To: "Tanwar, Rahul" <rahul.tanwar@linux.intel.com>
Cc: andriy.shevchenko@intel.com, cheol.yong.kim@intel.com,
	linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
	mark.rutland@arm.com, mturquette@baylibre.com,
	qi-ming.wu@intel.com, rahul.tanwar@intel.com, robh+dt@kernel.org,
	robh@kernel.org, sboyd@kernel.org, yixin.zhu@linux.intel.com
Subject: Re: [PATCH v1 1/2] clk: intel: Add CGU clock driver for a new SoC
Date: Thu, 5 Sep 2019 22:47:23 +0200	[thread overview]
Message-ID: <CAFBinCA+J-HnXfRnquqviXvX0Jo84hoLC9=_uHbyWKZycwyAFw@mail.gmail.com> (raw)
In-Reply-To: <b7920723-1df2-62df-61c7-98c3a1665aa1@linux.intel.com>

Hi Rahul,

On Wed, Sep 4, 2019 at 10:04 AM Tanwar, Rahul
<rahul.tanwar@linux.intel.com> 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)

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.

Martin