mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Andrej Picej <andrej.picej@norik.com>
To: Sascha Hauer <sha@pengutronix.de>
Cc: barebox@lists.infradead.org, Y.Bas@phytec.de, s.riedmueller@phytec.de
Subject: Re: [PATCH 3/5] regulator: allow use of dummy regulator
Date: Wed, 17 Nov 2021 10:29:14 +0100	[thread overview]
Message-ID: <f68a9f57-9e88-1d9c-0389-f6377168d0a0@norik.com> (raw)
In-Reply-To: <bc395f93-7380-cc0c-9aad-78fd81d1dba6@norik.com>

On 17. 11. 21 09:11, Andrej Picej wrote:
> 
> 
> On 17. 11. 21 08:21, Sascha Hauer wrote:
>> On Mon, Nov 15, 2021 at 02:02:06PM +0100, Andrej Picej wrote:
>>> It is quite common for users to delete power supply nodes of regulators
>>> which aren't yet supported.
>>> The idea of a function call or devicetree property which allows use of
>>> dummy regulator is not new. This implementation uses barebox specific
>>> devicetree property "barebox,allow-dummy-supply" to allow switching to
>>> dummy power regulator.
>>> Basically just catch the regulators ensure_probed error, if this
>>> property is set.
>>>
>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>> ---
>>>   drivers/regulator/core.c | 9 ++++++++-
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>>> index 097f7d779..1c58932e1 100644
>>> --- a/drivers/regulator/core.c
>>> +++ b/drivers/regulator/core.c
>>> @@ -231,8 +231,15 @@ static struct regulator_internal 
>>> *of_regulator_get(struct device_d *dev, const c
>>>       }
>>>       ret = of_device_ensure_probed(node);
>>> -    if (ret)
>>> +    if (ret) {
>>> +        if (of_get_property(dev->device_node, 
>>> "barebox,allow-dummy-supply", NULL)) {
>>> +            dev_dbg(dev, "Allow use of dummy regulator for " \
>>> +                "%s-supply\n", supply);
>>> +            ri = NULL;
>>> +            goto out;
>>> +        }
>>>           return ERR_PTR(ret);
>>
>> I wonder if we should rather add a property on the producer side than on
>> the consumer side, i.e. Add a barebox,status = "disabled" property to
>> the regulator node. We had the same discussion with phys recently, maybe
>> we can use the same approach for both issues.
>>
> 
> I was wandering that too. But decided to go with consumer side so users 
> which might want to use this have to enable this for every consumers 
> which can use dummy regulators. IMO this would mean more thought would 
> go into this and would be consequently more error prone.
> 
> So for producer side did you have in mind that this setting will be set 
> for every regulator, like this?
> 
>>                 regulators {
>>                         vddcore_reg: bcore1 {
>> +                               barebox,allow-dummy-supply;
>>                                 regulator-min-microvolt = <730000>;
>>                                 regulator-max-microvolt = <1380000>;
>>                                 regulator-always-on;
>>                         };
> 
> Or should this setting be set for all regulators provided by the same 
> IC, like this?
> 
>>                 regulators {
>> +                       barebox,allow-dummy-supply;
>>                         vddcore_reg: bcore1 {
>>                                 regulator-min-microvolt = <730000>;
>>                                 regulator-max-microvolt = <1380000>;
>>                                 regulator-always-on;
>>                         };
> 
> I guess we could make both cases work, first check if 
> "barebox,allow-dummy-supply" is present in the regulator node and then 
> also check parent node (regulators)? What do you think?
> 

Like this:

> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 097f7d779..4ff7e1c2f 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -197,7 +197,7 @@ static struct regulator_internal *of_regulator_get(struct device_d *dev, const c
>  {
>         char *propname;
>         struct regulator_internal *ri;
> -       struct device_node *node;
> +       struct device_node *node, *node_parent;
>         int ret;
>  
>         propname = basprintf("%s-supply", supply);
> @@ -231,8 +231,23 @@ static struct regulator_internal *of_regulator_get(struct device_d *dev, const c
>         }
>  
>         ret = of_device_ensure_probed(node);
> -       if (ret)
> +       if (ret) {
> +               /* If "barebox,allow-dummy-supply" property is set for regulator
> +                * provider, allow use of dummy regulator -> NULL is returned.
> +                * Check regulator node and its parent, if this setting is set
> +                * PMIC wide.
> +                */
> +               node_parent = of_get_parent(node);
> +               if (of_get_property(node, "barebox,allow-dummy-supply", NULL) ||
> +                   of_get_property(node_parent, "barebox,allow-dummy-supply", NULL)) {
> +                       dev_dbg(dev, "Allow use of dummy regulator for " \
> +                               "%s-supply\n", supply);
> +                       ri = NULL;
> +                       goto out;
> +               }
> +
>                 return ERR_PTR(ret);
> +       }
>  
>         list_for_each_entry(ri, &regulator_list, list) {
>                 if (ri->node == node) {

BR,
Andrej

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2021-11-17  9:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-15 13:02 [PATCH 0/5] PHYTEC patches to be mainlined Andrej Picej
2021-11-15 13:02 ` [PATCH 1/5] flash-header-phytec-pcl063: Set SOC voltage to 1.25 V during boot Andrej Picej
2021-11-15 13:02 ` [PATCH 2/5] ARM: configs: phytec-som-imx6.config Andrej Picej
2021-11-17  7:03   ` Sascha Hauer
2021-11-17  9:59     ` Stefan Riedmüller
2021-11-17 10:08       ` Andrej Picej
2021-11-15 13:02 ` [PATCH 3/5] regulator: allow use of dummy regulator Andrej Picej
2021-11-17  7:21   ` Sascha Hauer
2021-11-17  8:11     ` Andrej Picej
2021-11-17  9:29       ` Andrej Picej [this message]
2021-11-15 13:02 ` [PATCH 4/5] ARM: dts: imx6qdl: pfla02: allow use of dummy regulators Andrej Picej
2021-11-15 13:02 ` [PATCH 5/5] documentation: regulator: add dummy-supply Andrej Picej

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f68a9f57-9e88-1d9c-0389-f6377168d0a0@norik.com \
    --to=andrej.picej@norik.com \
    --cc=Y.Bas@phytec.de \
    --cc=barebox@lists.infradead.org \
    --cc=s.riedmueller@phytec.de \
    --cc=sha@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox