Re: [PATCH v4 5/7] cpufreq: qcom-hw: Implement CPRh aware OSM programming

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Wed 14 Apr 20:31 CDT 2021, Taniya Das wrote:

> 
> On 4/13/2021 9:19 AM, Viresh Kumar wrote:
> > On 12-04-21, 15:01, Taniya Das wrote:
> > > Technically the HW we are trying to program here differs in terms of
> > > clocking, the LUT definitions and many more. It will definitely make
> > > debugging much more troublesome if we try to accommodate multiple versions of
> > > CPUFREQ-HW in the same code.
> > > 
> > > Thus to keep it simple, easy to read, debug, the suggestion is to keep it
> > > with "v1" tag as the OSM version we are trying to put here is from OSM1.0.
> > 
> > That is a valid point and is always a case with so many drivers. What
> > I am concerned about is how much code is common across versions, if it
> > is 5-70%, or more, then we should definitely share, arrange to have
> > callbacks or ops per version and call them in a generic fashion instead
> > of writing a new driver. This is what's done across
> > drivers/frameworks, etc.
> > 
> 
> The code sharing here between versions should be very minimal as most
> portion of the code here in V1 would focus on programming to prepare the LUT
> to be further read by the driver, the programming in itself is huge for v1.
> I am okay if you move the v1 in a different file and invoke based on
> version.
> 

The initialization of the hardware certainly is a large chunk of code,
but once initialized it's the same driver. Your argument that this new
code makes it harder to debug things doesn't seem relevant, as the old
support doesn't seem to affect the code paths used on the modern
version.

Creating a new driver and picking it by compatible is certainly possible
to do, but looking at Angelo's patch it seems like it would contain 100%
of what's in qcom-cpufreq-hw today.

As such, I like the approach suggested in this series.

Regards,
Bjorn