diff mbox series

[v2,03/11] soc: renesas: Move away from using OF_POPULATED for fw_devlink

Message ID 20230127001141.407071-4-saravanak@google.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series fw_devlink improvements | expand

Commit Message

Saravana Kannan Jan. 27, 2023, 12:11 a.m. UTC
The OF_POPULATED flag was set to let fw_devlink know that the device
tree node will not have a struct device created for it. This information
is used by fw_devlink to avoid deferring the probe of consumers of this
device tree node.

Let's use fwnode_dev_initialized() instead because it achieves the same
effect without using OF specific flags. This allows more generic code to
be written in driver core.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/soc/renesas/rcar-sysc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Geert Uytterhoeven Jan. 27, 2023, 8:11 a.m. UTC | #1
Hi Saravana,

On Fri, Jan 27, 2023 at 1:11 AM Saravana Kannan <saravanak@google.com> wrote:
> The OF_POPULATED flag was set to let fw_devlink know that the device
> tree node will not have a struct device created for it. This information
> is used by fw_devlink to avoid deferring the probe of consumers of this
> device tree node.
>
> Let's use fwnode_dev_initialized() instead because it achieves the same
> effect without using OF specific flags. This allows more generic code to
> be written in driver core.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Thanks for your patch!

> --- a/drivers/soc/renesas/rcar-sysc.c
> +++ b/drivers/soc/renesas/rcar-sysc.c
> @@ -437,7 +437,7 @@ static int __init rcar_sysc_pd_init(void)
>
>         error = of_genpd_add_provider_onecell(np, &domains->onecell_data);
>         if (!error)
> -               of_node_set_flag(np, OF_POPULATED);
> +               fwnode_dev_initialized(&np->fwnode, true);

As drivers/soc/renesas/rmobile-sysc.c is already using this method,
it should work fine.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v6.4.

>
>  out_put:
>         of_node_put(np);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Andy Shevchenko Jan. 27, 2023, 9:25 a.m. UTC | #2
On Thu, Jan 26, 2023 at 04:11:30PM -0800, Saravana Kannan wrote:
> The OF_POPULATED flag was set to let fw_devlink know that the device
> tree node will not have a struct device created for it. This information
> is used by fw_devlink to avoid deferring the probe of consumers of this
> device tree node.
> 
> Let's use fwnode_dev_initialized() instead because it achieves the same
> effect without using OF specific flags. This allows more generic code to
> be written in driver core.

...

> -		of_node_set_flag(np, OF_POPULATED);
> +		fwnode_dev_initialized(&np->fwnode, true);

of_fwnode_handle(np) ?
Geert Uytterhoeven Jan. 27, 2023, 9:30 a.m. UTC | #3
Hi Andy,

On Fri, Jan 27, 2023 at 10:25 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Thu, Jan 26, 2023 at 04:11:30PM -0800, Saravana Kannan wrote:
> > The OF_POPULATED flag was set to let fw_devlink know that the device
> > tree node will not have a struct device created for it. This information
> > is used by fw_devlink to avoid deferring the probe of consumers of this
> > device tree node.
> >
> > Let's use fwnode_dev_initialized() instead because it achieves the same
> > effect without using OF specific flags. This allows more generic code to
> > be written in driver core.
>
> ...
>
> > -             of_node_set_flag(np, OF_POPULATED);
> > +             fwnode_dev_initialized(&np->fwnode, true);
>
> of_fwnode_handle(np) ?

Or of_node_to_fwnode(). Looks like we have (at least) two of them...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Andy Shevchenko Jan. 27, 2023, 9:44 a.m. UTC | #4
On Fri, Jan 27, 2023 at 10:30:35AM +0100, Geert Uytterhoeven wrote:
> On Fri, Jan 27, 2023 at 10:25 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Jan 26, 2023 at 04:11:30PM -0800, Saravana Kannan wrote:

...

> > > -             of_node_set_flag(np, OF_POPULATED);
> > > +             fwnode_dev_initialized(&np->fwnode, true);
> >
> > of_fwnode_handle(np) ?
> 
> Or of_node_to_fwnode().

Not really.

> Looks like we have (at least) two of them...

Yes, and the latter one is IRQ subsystem invention. Should gone in favour of
the generic helper.
Saravana Kannan Jan. 28, 2023, 7:18 a.m. UTC | #5
On Fri, Jan 27, 2023 at 12:11 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Saravana,
>
> On Fri, Jan 27, 2023 at 1:11 AM Saravana Kannan <saravanak@google.com> wrote:
> > The OF_POPULATED flag was set to let fw_devlink know that the device
> > tree node will not have a struct device created for it. This information
> > is used by fw_devlink to avoid deferring the probe of consumers of this
> > device tree node.
> >
> > Let's use fwnode_dev_initialized() instead because it achieves the same
> > effect without using OF specific flags. This allows more generic code to
> > be written in driver core.
> >
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
>
> Thanks for your patch!
>
> > --- a/drivers/soc/renesas/rcar-sysc.c
> > +++ b/drivers/soc/renesas/rcar-sysc.c
> > @@ -437,7 +437,7 @@ static int __init rcar_sysc_pd_init(void)
> >
> >         error = of_genpd_add_provider_onecell(np, &domains->onecell_data);
> >         if (!error)
> > -               of_node_set_flag(np, OF_POPULATED);
> > +               fwnode_dev_initialized(&np->fwnode, true);
>
> As drivers/soc/renesas/rmobile-sysc.c is already using this method,
> it should work fine.
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-devel for v6.4.

Thanks! Does that mean I should drop this from this series? If two
maintainers pick the same patch up, will it cause problems? I'm
eventually expecting this series to be picked up by Greg into
driver-core-next.

-Saravana

>
> >
> >  out_put:
> >         of_node_put(np);
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Geert Uytterhoeven Jan. 30, 2023, 8:42 a.m. UTC | #6
Hi Saravana,

On Sat, Jan 28, 2023 at 8:19 AM Saravana Kannan <saravanak@google.com> wrote:
> On Fri, Jan 27, 2023 at 12:11 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Fri, Jan 27, 2023 at 1:11 AM Saravana Kannan <saravanak@google.com> wrote:
> > > The OF_POPULATED flag was set to let fw_devlink know that the device
> > > tree node will not have a struct device created for it. This information
> > > is used by fw_devlink to avoid deferring the probe of consumers of this
> > > device tree node.
> > >
> > > Let's use fwnode_dev_initialized() instead because it achieves the same
> > > effect without using OF specific flags. This allows more generic code to
> > > be written in driver core.
> > >
> > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/soc/renesas/rcar-sysc.c
> > > +++ b/drivers/soc/renesas/rcar-sysc.c
> > > @@ -437,7 +437,7 @@ static int __init rcar_sysc_pd_init(void)
> > >
> > >         error = of_genpd_add_provider_onecell(np, &domains->onecell_data);
> > >         if (!error)
> > > -               of_node_set_flag(np, OF_POPULATED);
> > > +               fwnode_dev_initialized(&np->fwnode, true);
> >
> > As drivers/soc/renesas/rmobile-sysc.c is already using this method,
> > it should work fine.
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > i.e. will queue in renesas-devel for v6.4.
>
> Thanks! Does that mean I should drop this from this series? If two
> maintainers pick the same patch up, will it cause problems? I'm
> eventually expecting this series to be picked up by Greg into
> driver-core-next.

Indeed. Patches for drivers/soc/renesas/ are supposed to go upstream
through the renesas-devel and soc trees. This patch has no dependencies
on anything else in the series (or vice versa), so there is no reason
to deviate from that, and possibly cause conflicts later.

BTW, I will convert to of_node_to_fwnode() while applying.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Saravana Kannan Jan. 30, 2023, 8 p.m. UTC | #7
On Mon, Jan 30, 2023 at 12:43 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Saravana,
>
> On Sat, Jan 28, 2023 at 8:19 AM Saravana Kannan <saravanak@google.com> wrote:
> > On Fri, Jan 27, 2023 at 12:11 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Fri, Jan 27, 2023 at 1:11 AM Saravana Kannan <saravanak@google.com> wrote:
> > > > The OF_POPULATED flag was set to let fw_devlink know that the device
> > > > tree node will not have a struct device created for it. This information
> > > > is used by fw_devlink to avoid deferring the probe of consumers of this
> > > > device tree node.
> > > >
> > > > Let's use fwnode_dev_initialized() instead because it achieves the same
> > > > effect without using OF specific flags. This allows more generic code to
> > > > be written in driver core.
> > > >
> > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/soc/renesas/rcar-sysc.c
> > > > +++ b/drivers/soc/renesas/rcar-sysc.c
> > > > @@ -437,7 +437,7 @@ static int __init rcar_sysc_pd_init(void)
> > > >
> > > >         error = of_genpd_add_provider_onecell(np, &domains->onecell_data);
> > > >         if (!error)
> > > > -               of_node_set_flag(np, OF_POPULATED);
> > > > +               fwnode_dev_initialized(&np->fwnode, true);
> > >
> > > As drivers/soc/renesas/rmobile-sysc.c is already using this method,
> > > it should work fine.
> > >
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > i.e. will queue in renesas-devel for v6.4.

I hope you meant queue it up for 6.3 and not 6.4?

> >
> > Thanks! Does that mean I should drop this from this series? If two
> > maintainers pick the same patch up, will it cause problems? I'm
> > eventually expecting this series to be picked up by Greg into
> > driver-core-next.
>
> Indeed. Patches for drivers/soc/renesas/ are supposed to go upstream
> through the renesas-devel and soc trees. This patch has no dependencies
> on anything else in the series (or vice versa), so there is no reason
> to deviate from that, and possibly cause conflicts later.

This series is supposed to fix a bunch of issues and I vaguely think
the series depends on this patch to work correctly on some Renesas
systems. You are my main renesas person, so it's probably some issue
you hit. Is you pick it up outside of this series I need to keep
asking folks to pick up two different patch threads. I don't have a
strong opinion, just a FYI. If you can take this patch soon, I don't
have any concerns.

> BTW, I will convert to of_node_to_fwnode() while applying.

Sounds good.

-Saravana
Geert Uytterhoeven Jan. 31, 2023, 8:14 a.m. UTC | #8
Hi Saravana,

On Mon, Jan 30, 2023 at 9:00 PM Saravana Kannan <saravanak@google.com> wrote:
> On Mon, Jan 30, 2023 at 12:43 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Sat, Jan 28, 2023 at 8:19 AM Saravana Kannan <saravanak@google.com> wrote:
> > > On Fri, Jan 27, 2023 at 12:11 AM Geert Uytterhoeven
> > > <geert@linux-m68k.org> wrote:
> > > > On Fri, Jan 27, 2023 at 1:11 AM Saravana Kannan <saravanak@google.com> wrote:
> > > > > The OF_POPULATED flag was set to let fw_devlink know that the device
> > > > > tree node will not have a struct device created for it. This information
> > > > > is used by fw_devlink to avoid deferring the probe of consumers of this
> > > > > device tree node.
> > > > >
> > > > > Let's use fwnode_dev_initialized() instead because it achieves the same
> > > > > effect without using OF specific flags. This allows more generic code to
> > > > > be written in driver core.
> > > > >
> > > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > --- a/drivers/soc/renesas/rcar-sysc.c
> > > > > +++ b/drivers/soc/renesas/rcar-sysc.c
> > > > > @@ -437,7 +437,7 @@ static int __init rcar_sysc_pd_init(void)
> > > > >
> > > > >         error = of_genpd_add_provider_onecell(np, &domains->onecell_data);
> > > > >         if (!error)
> > > > > -               of_node_set_flag(np, OF_POPULATED);
> > > > > +               fwnode_dev_initialized(&np->fwnode, true);
> > > >
> > > > As drivers/soc/renesas/rmobile-sysc.c is already using this method,
> > > > it should work fine.
> > > >
> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > i.e. will queue in renesas-devel for v6.4.
>
> I hope you meant queue it up for 6.3 and not 6.4?

V6.4.
The deadline for submitting pull requests for the soc tree is rc6.
Sorry, your series was posted too late to make that.

> > > Thanks! Does that mean I should drop this from this series? If two
> > > maintainers pick the same patch up, will it cause problems? I'm
> > > eventually expecting this series to be picked up by Greg into
> > > driver-core-next.
> >
> > Indeed. Patches for drivers/soc/renesas/ are supposed to go upstream
> > through the renesas-devel and soc trees. This patch has no dependencies
> > on anything else in the series (or vice versa), so there is no reason
> > to deviate from that, and possibly cause conflicts later.
>
> This series is supposed to fix a bunch of issues and I vaguely think
> the series depends on this patch to work correctly on some Renesas
> systems. You are my main renesas person, so it's probably some issue
> you hit. Is you pick it up outside of this series I need to keep
> asking folks to pick up two different patch threads. I don't have a
> strong opinion, just a FYI. If you can take this patch soon, I don't
> have any concerns.

Oh right, you do remove OF_POPULATED handling in
"[PATCH v2 09/11] of: property: Simplify of_link_to_phandle()".
It might be wise to postpone that removal, as after your series,
there are stillseveral users left, some of them might be impacted.

I do plan to test your full series on all my boards, but probably that
won't happen this week.

> > BTW, I will convert to of_node_to_fwnode() while applying.
>
> Sounds good.

If you still want this to land in v6,3 (with the of_node_to_fwnode()
conversion):
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Saravana Kannan Feb. 4, 2023, 10:30 p.m. UTC | #9
On Tue, Jan 31, 2023 at 12:14 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Saravana,
>
> On Mon, Jan 30, 2023 at 9:00 PM Saravana Kannan <saravanak@google.com> wrote:
> > On Mon, Jan 30, 2023 at 12:43 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Sat, Jan 28, 2023 at 8:19 AM Saravana Kannan <saravanak@google.com> wrote:
> > > > On Fri, Jan 27, 2023 at 12:11 AM Geert Uytterhoeven
> > > > <geert@linux-m68k.org> wrote:
> > > > > On Fri, Jan 27, 2023 at 1:11 AM Saravana Kannan <saravanak@google.com> wrote:
> > > > > > The OF_POPULATED flag was set to let fw_devlink know that the device
> > > > > > tree node will not have a struct device created for it. This information
> > > > > > is used by fw_devlink to avoid deferring the probe of consumers of this
> > > > > > device tree node.
> > > > > >
> > > > > > Let's use fwnode_dev_initialized() instead because it achieves the same
> > > > > > effect without using OF specific flags. This allows more generic code to
> > > > > > be written in driver core.
> > > > > >
> > > > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > > >
> > > > > Thanks for your patch!
> > > > >
> > > > > > --- a/drivers/soc/renesas/rcar-sysc.c
> > > > > > +++ b/drivers/soc/renesas/rcar-sysc.c
> > > > > > @@ -437,7 +437,7 @@ static int __init rcar_sysc_pd_init(void)
> > > > > >
> > > > > >         error = of_genpd_add_provider_onecell(np, &domains->onecell_data);
> > > > > >         if (!error)
> > > > > > -               of_node_set_flag(np, OF_POPULATED);
> > > > > > +               fwnode_dev_initialized(&np->fwnode, true);
> > > > >
> > > > > As drivers/soc/renesas/rmobile-sysc.c is already using this method,
> > > > > it should work fine.
> > > > >
> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > > i.e. will queue in renesas-devel for v6.4.
> >
> > I hope you meant queue it up for 6.3 and not 6.4?
>
> V6.4.
> The deadline for submitting pull requests for the soc tree is rc6.
> Sorry, your series was posted too late to make that.
>
> > > > Thanks! Does that mean I should drop this from this series? If two
> > > > maintainers pick the same patch up, will it cause problems? I'm
> > > > eventually expecting this series to be picked up by Greg into
> > > > driver-core-next.
> > >
> > > Indeed. Patches for drivers/soc/renesas/ are supposed to go upstream
> > > through the renesas-devel and soc trees. This patch has no dependencies
> > > on anything else in the series (or vice versa), so there is no reason
> > > to deviate from that, and possibly cause conflicts later.
> >
> > This series is supposed to fix a bunch of issues and I vaguely think
> > the series depends on this patch to work correctly on some Renesas
> > systems. You are my main renesas person, so it's probably some issue
> > you hit. Is you pick it up outside of this series I need to keep
> > asking folks to pick up two different patch threads. I don't have a
> > strong opinion, just a FYI. If you can take this patch soon, I don't
> > have any concerns.
>
> Oh right, you do remove OF_POPULATED handling in
> "[PATCH v2 09/11] of: property: Simplify of_link_to_phandle()".
> It might be wise to postpone that removal, as after your series,
> there are stillseveral users left, some of them might be impacted.
>
> I do plan to test your full series on all my boards, but probably that
> won't happen this week.
>
> > > BTW, I will convert to of_node_to_fwnode() while applying.
> >
> > Sounds good.
>
> If you still want this to land in v6,3 (with the of_node_to_fwnode()
> conversion):
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
>

Yeah, let me try to land this in 6.3 with the series.

-Saravana
diff mbox series

Patch

diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c
index b0a80de34c98..03246ed4a79e 100644
--- a/drivers/soc/renesas/rcar-sysc.c
+++ b/drivers/soc/renesas/rcar-sysc.c
@@ -437,7 +437,7 @@  static int __init rcar_sysc_pd_init(void)
 
 	error = of_genpd_add_provider_onecell(np, &domains->onecell_data);
 	if (!error)
-		of_node_set_flag(np, OF_POPULATED);
+		fwnode_dev_initialized(&np->fwnode, true);
 
 out_put:
 	of_node_put(np);