Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1621601 > unrolled thread

Re: [PATCH 2/6] regulator: anatop: only set supply regulator when it actually exists

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/6] regulator: anatop: only set supply regulator when it  actually exists Mark Brown <broonie@kernel.org> - 2017-04-11 22:40 +0200
    Re: [PATCH 2/6] regulator: anatop: only set supply regulator when it  actually exists Dong Aisheng <dongas86@gmail.com> - 2017-04-12 17:20 +0200
      Re: [PATCH 2/6] regulator: anatop: only set supply regulator when it  actually exists Mark Brown <broonie@kernel.org> - 2017-04-12 18:00 +0200
        Re: [PATCH 2/6] regulator: anatop: only set supply regulator when it  actually exists Mark Brown <broonie@kernel.org> - 2017-04-12 18:10 +0200
        Re: [PATCH 2/6] regulator: anatop: only set supply regulator when it  actually exists Dong Aisheng <dongas86@gmail.com> - 2017-04-12 18:10 +0200

#1621601 — Re: [PATCH 2/6] regulator: anatop: only set supply regulator when it actually exists

[Multipart message — attachments visible in raw view] — view raw

On Wed, Apr 12, 2017 at 09:58:43AM +0800, Dong Aisheng wrote:
> Mandatorily set the initdata->supply_regulator while it actually not
> exist will cause regulator core to resolve supply each time whenever
> a new regulator registered which is meaningless and waste CPU mips.
> 
> We can observe more than one hundred times of iteration of resolving
> during a MX6Q SDB board booting up.
> 
> This patch adds the condition check for vin-supply to avoid the issue.

This is an obvious abstraction failure - there is nothing magical about
your driver which means that we need special casing in it to handle
badly written DTs that don't specify supplies.  Exactly the same
argument applies to all other regulators so if this is worth fixing it's
worth fixing in the core so we substitute in a dummy regulator if the
supply is genuinely missing.  Which is something we in fact have code to
do already though for some reason I can't see we bypass it, I'll send a
patch just now...

[toc] | [next] | [standalone]

#1622236

Hi Mark,

On Tue, Apr 11, 2017 at 09:31:24PM +0100, Mark Brown wrote:
> On Wed, Apr 12, 2017 at 09:58:43AM +0800, Dong Aisheng wrote:
> > Mandatorily set the initdata->supply_regulator while it actually not
> > exist will cause regulator core to resolve supply each time whenever
> > a new regulator registered which is meaningless and waste CPU mips.
> > 
> > We can observe more than one hundred times of iteration of resolving
> > during a MX6Q SDB board booting up.
> > 
> > This patch adds the condition check for vin-supply to avoid the issue.
> 
> This is an obvious abstraction failure - there is nothing magical about
> your driver which means that we need special casing in it to handle
> badly written DTs that don't specify supplies.  Exactly the same
> argument applies to all other regulators so if this is worth fixing it's
> worth fixing in the core so we substitute in a dummy regulator if the
> supply is genuinely missing.  Which is something we in fact have code to
> do already though for some reason I can't see we bypass it, I'll send a
> patch just now...

You're absolutely right!
I did this because there're some where else did the same thing.
e.g. drivers/regulator/fixed.c.

But it's obviously none of any platform specific and is perfectly
to be handled in regulator core.

Regards
Dong Aisheng

[toc] | [prev] | [next] | [standalone]

#1622266

[Multipart message — attachments visible in raw view] — view raw

On Thu, Apr 13, 2017 at 03:11:01PM +0800, Dong Aisheng wrote:

> You're absolutely right!
> I did this because there're some where else did the same thing.
> e.g. drivers/regulator/fixed.c.

> But it's obviously none of any platform specific and is perfectly
> to be handled in regulator core.

Did my patch solve the problems you were seeing?  I just wrote it
quickly last thing before I finished for the evening.

[toc] | [prev] | [next] | [standalone]

#1622274

[Multipart message — attachments visible in raw view] — view raw

On Thu, Apr 13, 2017 at 12:00:36AM +0800, Dong Aisheng wrote:

> It can solve the problem.
> But it breaks some thing and need a further tiny fix.
> I just replied the mail in your patch thread.
> Please check it!

OK, I'll look out for your mail (I've not seen it yet, guess it's got
held up).  Thanks for testing.

[toc] | [prev] | [next] | [standalone]

#1622281

On Wed, Apr 12, 2017 at 11:53 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Apr 13, 2017 at 03:11:01PM +0800, Dong Aisheng wrote:
>
>> You're absolutely right!
>> I did this because there're some where else did the same thing.
>> e.g. drivers/regulator/fixed.c.
>
>> But it's obviously none of any platform specific and is perfectly
>> to be handled in regulator core.
>
> Did my patch solve the problems you were seeing?  I just wrote it
> quickly last thing before I finished for the evening.

It can solve the problem.
But it breaks some thing and need a further tiny fix.
I just replied the mail in your patch thread.
Please check it!

Regards
Dong Aisheng

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web