regulator-core has_full_constraints set too late for dt using boards ?

regulator-core has_full_constraints set too late for dt using boards ?

[Hans de Goede]

Hi All,

While looking into making regulator_get_optional not print
an error when no regulator is found, I've noticed that
for dt enabled boards, has_full_constraints is not set
until regulator_init_complete() runs, which is a
late_initcall.

This means that it gets set after it has already been checked
from calls like regulator_get which are likely done by init
functions running earlier.

This seems wrong ...

Regards,

Hans

regulator-core has_full_constraints set too late for dt using boards ?

[Mark Brown]

On Wed, Dec 11, 2013 at 04:43:01PM +0100, Hans de Goede wrote:

> While looking into making regulator_get_optional not print
> an error when no regulator is found, I've noticed that
> for dt enabled boards, has_full_constraints is not set
> until regulator_init_complete() runs, which is a
> late_initcall.

> This means that it gets set after it has already been checked
> from calls like regulator_get which are likely done by init
> functions running earlier.

> This seems wrong ...

Please look at the current code, that's not how this works any more.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131211/55f674c2/attachment.sig>

regulator-core has_full_constraints set too late for dt using boards ?

[Hans de Goede]

Hi,

On 12/11/2013 05:02 PM, Mark Brown wrote:
> On Wed, Dec 11, 2013 at 04:43:01PM +0100, Hans de Goede wrote:
>
>> While looking into making regulator_get_optional not print
>> an error when no regulator is found, I've noticed that
>> for dt enabled boards, has_full_constraints is not set
>> until regulator_init_complete() runs, which is a
>> late_initcall.
>
>> This means that it gets set after it has already been checked
>> from calls like regulator_get which are likely done by init
>> functions running earlier.
>
>> This seems wrong ...
>
> Please look at the current code, that's not how this works any more.

More current then 3.13-rc3 ? Because that is what I've been looking at.

I could be completely wrong of course. Either way could you give a few
hints as to how my interpretation of how this works is wrong ?

Thanks & Regards,

Hans

regulator-core has_full_constraints set too late for dt using boards ?

[Mark Brown]

On Thu, Dec 12, 2013 at 11:14:10AM +0100, Hans de Goede wrote:
> On 12/11/2013 05:02 PM, Mark Brown wrote:

> >Please look at the current code, that's not how this works any more.

> More current then 3.13-rc3 ? Because that is what I've been looking at.

Yes, -next.  It'll be going to Linus probably this week.

> I could be completely wrong of course. Either way could you give a few
> hints as to how my interpretation of how this works is wrong ?

It's correct for the code you've been looking at but has since been
changed.  Please look at the changes in -next, you should generally
check -next for the current state of things.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131212/b8104725/attachment-0001.sig>

regulator-core has_full_constraints set too late for dt using boards ?

[Hans de Goede]

Hi,

On 12/12/2013 11:42 AM, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 11:14:10AM +0100, Hans de Goede wrote:
>> On 12/11/2013 05:02 PM, Mark Brown wrote:
>
>>> Please look at the current code, that's not how this works any more.
>
>> More current then 3.13-rc3 ? Because that is what I've been looking at.
>
> Yes, -next.  It'll be going to Linus probably this week.
>
>> I could be completely wrong of course. Either way could you give a few
>> hints as to how my interpretation of how this works is wrong ?
>
> It's correct for the code you've been looking at but has since been
> changed.  Please look at the changes in -next, you should generally
> check -next for the current state of things.

Ah yes, I see, thanks for clarifying this.

Note my patch to not do a dev_err when no regulator is found for
regulator_get_optional still is needed in -next.

Thanks & Regards,

Hans

regulator-core has_full_constraints set too late for dt using boards ?

[Mark Brown]

On Thu, Dec 12, 2013 at 01:19:39PM +0100, Hans de Goede wrote:

> Note my patch to not do a dev_err when no regulator is found for
> regulator_get_optional still is needed in -next.

It's sitting in my review queue, it's not very well described so it's
not clear that you've selected the correct criteria and I need to find
time to actually look at it properly.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131212/c792fad9/attachment.sig>

regulator-core has_full_constraints set too late for dt using boards ?

[Hans de Goede]

Hi,

On 12/12/2013 01:23 PM, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 01:19:39PM +0100, Hans de Goede wrote:
>
>> Note my patch to not do a dev_err when no regulator is found for
>> regulator_get_optional still is needed in -next.
>
> It's sitting in my review queue, it's not very well described so it's
> not clear that you've selected the correct criteria and I need to find
> time to actually look at it properly.

I did not want to add a third boolean parameter to indicate
_regulator_get() was being called from regulator_get_optional.

Since regulator_get_optional is the only one calling
_regulator_get() with both allow_dummy and exclusive set to
false I've added the following condition to printing the
error:

	if (!(!allow_dummy && !exclusive))

Simplified to:

	if (allow_dummy || exclusive)

Which means that it will get printed in all cases it would
get printed previously, except when called from
regulator_get_optional.

Alternatively a third "optional" boolean parameter could
get added to _regulator_get() and the test for printing the
error could become:

	if (!optional)

Let me know if you would prefer doing things that way and
I'll respin the patch.

Thanks & Regards,

Hans

regulator-core has_full_constraints set too late for dt using boards ?

[Mark Brown]

On Thu, Dec 12, 2013 at 02:05:14PM +0100, Hans de Goede wrote:
> On 12/12/2013 01:23 PM, Mark Brown wrote:

> >It's sitting in my review queue, it's not very well described so it's
> >not clear that you've selected the correct criteria and I need to find
> >time to actually look at it properly.

> Since regulator_get_optional is the only one calling
> _regulator_get() with both allow_dummy and exclusive set to
> false I've added the following condition to printing the
> error:

Put some of this analysis in the commit message and perhaps a comment in
the code please...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131212/a3e93479/attachment.sig>