lkml.org 
[lkml]   [2016]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH] regulator: introduce boot protection flag
From
Date
Hi, Mark

On Thursday, June 09, 2016 01:16 AM, Mark Brown wrote:
> On Mon, May 09, 2016 at 03:05:08PM +0800, WEN Pingbo wrote:
>
>> And regulator core will postpone all operations until all consumers
>> have taked their place.
>
> It doesn't, it postpones them until late_initacall(). This is both
> after the consumers have loaded if they are built in and before any
> consumers built as modules come up.

Yes, this patch only protects a regulator from regulator
registration(built in) to late_initcall(). But, IMO, if a regulator is
critical, it's weird to build as a module. Maybe I was thoughtless here.

If we take modules under consideration, and to make this patch more
universal, I think what we really need is adding a flag to protect a
regulator from registration to a specific consumer(not the first
consumer). The regulator driver gives the initial state, and the
specific consumer need to clear this flag while finishing regulator
setting(by calling a function like regulator_clear_protect()). And what
the regulator core need to do is staging all operations during
protection. And that will cover all consumers probing order, whenever
the regulator is registered.

Any idea?

>
>> The boot_protection flag only work before late_initicall. And as other
>> constraints liked, you can specify this flag in a board file, or in
>> dts file.
>
> Anything added to the DT ABI needs a binding.
>

I will add bindings in next version.

>> + /* constraints check has already done */
>> + if (rdev->boot_mode)
>> + rdev->desc->ops->set_mode(rdev, rdev->boot_mode);
>
> This whole sequence of code ignores errors - that's not great. We
> should at least log them.
>

OK.

>> + mutex_unlock(&rdev->mutex);
>> +
>> + if (regulator)
>> + regulator_set_voltage(regulator, regulator->min_uV,
>> + regulator->max_uV);
>
> That's... exciting. There's a couple of issues here. One is that
> this is not operating on the rdev but rather on a consumer regulator
> device, the other is that we drop out of the lock before doing the
> update which tends to be a warning sign that something fun is going on
> and at least an internal function should be used. These two most likely
> come down to the same issue.
>

OK, some bugs here. I will use a unlock version.

Pingbo

\
 
 \ /
  Last update: 2016-06-15 14:41    [W:0.085 / U:4.580 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site