Re: [PATCH 0/2] cpufreq/opp: rework regulator initialization

[Viresh Kumar]

On 11-02-19, 13:22, Marek Szyprowski wrote:
> Hi Viresh,
> 
> On 2019-02-11 10:55, Viresh Kumar wrote:
> > On 11-02-19, 10:52, Marek Szyprowski wrote:
> >> On 2019-02-11 09:44, Viresh Kumar wrote:
> >>> On 07-02-19, 13:22, Marek Szyprowski wrote:
> >>>> Dear All,
> >>>>
> >>>> Recent commit 9ac6cb5fbb17 ("i2c: add suspended flag and accessors for
> >>>> i2c adapters") added a visible warning for an attempt to do i2c transfer
> >>>> over a suspended i2c bus. This revealed a long standing issue in the
> >>>> cpufreq-dt driver, which gives a following warning during system
> >>>> suspend/resume cycle:
> >>> Marek,
> >>>
> >>> I have sent a patchset which is not directly related to the problem
> >>> you are facing, but it may solve your problem as a side effect. Can
> >>> you please see if that works to solve this issue as well ?
> >>>
> >>> https://lore.kernel.org/lkml/cover.1549874368.git.viresh.kumar@xxxxxxxxxx/T/#u
> >> Thanks for the patch. It indeed solves the problem of the i2c transfer
> >> in cpu hotplug procedure during system resume, although my resources
> >> management rewrite is still valid as a way to fix remaining 'todos' in
> >> the cpufreq-dt driver.
> > Which remaining todos are you talking about ? Sorry I am not able to
> > recall :(
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpufreq/cpufreq-dt.c#n347

Ah, okay. Thanks for the pointer.

Ideally we shouldn't be doing anything in probe, like
resources_available(), but we are forced to do it as
subsys_interface_register() doesn't return the errors returned from
cpufreq_add_dev() currently. I tried to fix it once but there were
some complications I believe and then left it.

The main problem is that if cpufreq_online() doesn't find the required
resources and returns -EPROBE_DEFER, it never comes back to the
probe() routine which registers the cpufreq driver. And so we have to
add the duplicate checks in probe() itself before it registers the
cpufreq driver.

Now all we need to do there is to guarantee that the resources are
available when the cpufreq driver registers and so we do it only for
CPU0 currently. Logically speaking, if the resources are available for
CPU0, it will normally be available for any other CPU as well and so
there never was a requirement to test it for other CPUs. And so I left
a FIXME there, so that we know what's going on there in case a
platform comes up for which it doesn't work.

I won't fix it with something like what this patch series tried to do,
rather I would try to make sure that EPROBE_DEFER gets returned from
cpufreq_driver_register() instead.

Thanks.

-- 
viresh