Re: [PATCH 5/7] cpufreq: qcom-hw: Use regmap for accessing hardware registers

[Amit Kucheria]

On Tue, Sep 8, 2020 at 5:18 PM Amit Kucheria <amitk@xxxxxxxxxx> wrote:
>
> On Tue, Sep 8, 2020 at 4:48 PM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> >
> > On 08-09-20, 16:41, Manivannan Sadhasivam wrote:
> > > On 0908, Viresh Kumar wrote:
> > > > On 08-09-20, 13:27, Manivannan Sadhasivam wrote:
> > > > > Use regmap for accessing cpufreq registers in hardware.
> > > >
> > > > Why ? Please mention why a change is required in the log.
> > > >
> > >
> > > Only because it is recommended to use regmap for abstracting the hw access.
> >
> > Yes it can be very useful in abstracting the hw access in case of
> > busses like SPI/I2C, others, but in this case there is only one way of
> > doing it with the exact same registers. I am not sure it is worth it
> > here. FWIW, I have never played with regmaps personally, and so every
> > chance I can be wrong here.
>
> One could handle the reg offsets through a struct initialisation, but
> then you end up with lots of #defines for bitmasks and bits for each
> version of the IP. And the core code becomes a bit convoluted IMO,
> trying to handle the differences.
>
> regmap hides the differences of the bit positions and register offsets
> between several IP versions.
>
> > > Moreover it handles the proper locking for us in the core (spinlock vs mutex).
> >
> > What locking do you need here ?
>
> Right, locking isn't the main reason here.

Having said this, perhaps this patch can be held back for now, since
we're not yet using some of the features of regmap to abstract away
bit fields and such.

We don't strictly need it for just different register offsets.

Regards,
Amit