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>,
	Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 2/2] mfd: da9063: ensure all gpio devices are probed before
Date: Mon, 21 Mar 2022 10:46:13 +0100	[thread overview]
Message-ID: <26ed18a0-1bec-001b-d766-e9d283ce2e90@norik.com> (raw)
In-Reply-To: <20220316104440.GR405@pengutronix.de>



On 16. 03. 22 11:44, Sascha Hauer wrote:
> On Tue, Mar 15, 2022 at 03:24:23PM +0100, Ahmad Fatoum wrote:
>> On 15.03.22 14:39, Andrej Picej wrote:
>>> GPIO lines in da9063 are assigned dynamically, while majority of SOC
>>> GPIO drivers assign their GPIOs in static manner (GPIO line numbers can
>>> be calculated).
>>>
>>> This introduces regression if deep probe support is used. If da9063
>>> GPIOs are registered before the SOCs GPIOs, there is a good chance that
>>> the SOCs statically computed GPIO line numbers will already be used by
>>> PMIC.
>>>
>>> Ensure all SOCs GPIO drivers and GPIO lines get registered before the
>>> da9063 registers its own gpiochip.
>>>
>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>
>> I don't think this is the proper place. Also I think you may have
>> introduced a recursion here if the PMIC happens to have an alias
>> itself?
>>
>> What I think we could do instead is to move this into gpiochip_add()
>> and opencode the alias iterator:
>>
>>
>> foreach (gpio_alias) {
>> 	if (gpio_alias_dev == chip->dev)
>> 		__gpiochip_add();
>> 	else
>> 		of_device_ensure_probed();
>> }
> 
> This of_device_ensure_probed() will bring you into the very same loop
> again. Better call this loop from outside gpiochip_add(). We could
> add a gpio_ensure_probed() which executes this loop. That would be
> called from some initcall, or maybe from board code when gpios are
> needed before this initcall.
> 
> Sascha
> 

That was my initial idea, but then I wanted to make it a bit more 
general, so it could be used also for other, "non gpio" devices.
Would that make sense?

I would expend the function (from PATCH 1/1) into something like this:
> int of_device_ensured_probed_by_alias_stem(struct device_node *np, const char *stem)
> {
> 	struct alias_prop *app;
> 	int id = -ENODEV;
> 	int ret;
> 
> 	if (!deep_probe_is_supported())
> 		return 0;
> 
> 	list_for_each_entry(app, &aliases_lookup, link) {
> 		if (of_node_cmp(app->stem, stem) != 0)
> 			continue;
> 
> 		/* If device node is given skip the probing of that device so we
> 		 * avoid recursion.
> 		 */
> 		if (np != NULL && np == app->np)
> 			continue;
> 
> 		ret = of_device_ensure_probed(app->np);
> 		if (ret)
> 			return ret;
> 	}
> 
> 	return 0;
> }
> EXPORT_SYMBOL_GPL(of_device_ensured_probed_by_alias_stem);

Where the user can specify which device should be skipped, so the method 
doesn't introduce recursion if device happens to have an alias.

We could then call this function from board file like this (example for 
PHYTEC):
> --- a/arch/arm/boards/phytec-som-imx6/board.c
> +++ b/arch/arm/boards/phytec-som-imx6/board.c
> @@ -111,6 +111,13 @@ static int phycore_da9062_setup_buck_mode(void)
>         if (!pmic_np)
>                 return -ENODEV;
>  
> +       of_device_ensured_probed_by_alias_stem(pmic_np, "gpio");
> +
>         ret = of_device_ensure_probed(pmic_np);
>         if (ret)
>                 return ret;
What do you think?



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


  reply	other threads:[~2022-03-21  9:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-15 13:39 [PATCH 1/2] of: implement new of_device_ensured_probed_by_alias_stem Andrej Picej
2022-03-15 13:39 ` [PATCH 2/2] mfd: da9063: ensure all gpio devices are probed before Andrej Picej
2022-03-15 14:24   ` Ahmad Fatoum
2022-03-16 10:44     ` Sascha Hauer
2022-03-21  9:46       ` Andrej Picej [this message]
2022-03-15 14:16 ` [PATCH 1/2] of: implement new of_device_ensured_probed_by_alias_stem Ahmad Fatoum
2022-03-15 14:29   ` Ahmad Fatoum
2022-03-16 10:01     ` Andrej Picej
2022-03-16  7:29   ` 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=26ed18a0-1bec-001b-d766-e9d283ce2e90@norik.com \
    --to=andrej.picej@norik.com \
    --cc=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --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