of_link_is_valid() can be static since it's not used anywhere else.
of_link_property() return type should have been int instead of bool.
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Saravana Kannan <[email protected]>
---
drivers/of/platform.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 21838226d68a..f68de5c4aeff 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -506,7 +506,7 @@ int of_platform_default_populate(struct device_node *root,
}
EXPORT_SYMBOL_GPL(of_platform_default_populate);
-bool of_link_is_valid(struct device_node *con, struct device_node *sup)
+static bool of_link_is_valid(struct device_node *con, struct device_node *sup)
{
of_node_get(sup);
/*
@@ -625,7 +625,7 @@ static const struct supplier_bindings bindings[] = {
{ },
};
-static bool of_link_property(struct device *dev, struct device_node *con_np,
+static int of_link_property(struct device *dev, struct device_node *con_np,
const char *prop)
{
struct device_node *phandle;
--
2.22.0.770.g0f2c4a37fd-goog
PowerPC platforms don't use the generic of/platform code to populate the
devices from DT. Therefore the generic device linking code is never used
in PowerPC. Compile it out to avoid warning about unused functions.
If a specific PowerPC platform wants to use this code in the future,
bringing this back for PowerPC would be trivial. We'll just need to export
of_link_to_suppliers() and then let the machine specific files do the
linking as they populate the devices from DT.
Signed-off-by: Saravana Kannan <[email protected]>
---
drivers/of/platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index f68de5c4aeff..a2a4e4b79d43 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -506,6 +506,7 @@ int of_platform_default_populate(struct device_node *root,
}
EXPORT_SYMBOL_GPL(of_platform_default_populate);
+#ifndef CONFIG_PPC
static bool of_link_is_valid(struct device_node *con, struct device_node *sup)
{
of_node_get(sup);
@@ -683,7 +684,6 @@ static int of_link_to_suppliers(struct device *dev)
return __of_link_to_suppliers(dev, dev->of_node);
}
-#ifndef CONFIG_PPC
static const struct of_device_id reserved_mem_matches[] = {
{ .compatible = "qcom,rmtfs-mem" },
{ .compatible = "qcom,cmd-db" },
--
2.22.0.770.g0f2c4a37fd-goog
On Tue, Aug 6, 2019 at 1:27 PM Saravana Kannan <[email protected]> wrote:
>
> of_link_is_valid() can be static since it's not used anywhere else.
>
> of_link_property() return type should have been int instead of bool.
>
> Reported-by: Stephen Rothwell <[email protected]>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> drivers/of/platform.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Acked-by: Rob Herring <[email protected]>
On Tue, Aug 6, 2019 at 1:27 PM Saravana Kannan <[email protected]> wrote:
>
> PowerPC platforms don't use the generic of/platform code to populate the
> devices from DT.
Yes, they do.
> Therefore the generic device linking code is never used
> in PowerPC. Compile it out to avoid warning about unused functions.
I'd prefer this get disabled on PPC using 'if (IS_ENABLED(CONFIG_PPC))
return' rather than #ifdefs.
>
> If a specific PowerPC platform wants to use this code in the future,
> bringing this back for PowerPC would be trivial. We'll just need to export
> of_link_to_suppliers() and then let the machine specific files do the
> linking as they populate the devices from DT.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> drivers/of/platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index f68de5c4aeff..a2a4e4b79d43 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -506,6 +506,7 @@ int of_platform_default_populate(struct device_node *root,
> }
> EXPORT_SYMBOL_GPL(of_platform_default_populate);
>
> +#ifndef CONFIG_PPC
> static bool of_link_is_valid(struct device_node *con, struct device_node *sup)
> {
> of_node_get(sup);
> @@ -683,7 +684,6 @@ static int of_link_to_suppliers(struct device *dev)
> return __of_link_to_suppliers(dev, dev->of_node);
> }
>
> -#ifndef CONFIG_PPC
> static const struct of_device_id reserved_mem_matches[] = {
> { .compatible = "qcom,rmtfs-mem" },
> { .compatible = "qcom,cmd-db" },
> --
> 2.22.0.770.g0f2c4a37fd-goog
>
On Tue, Aug 6, 2019 at 2:27 PM Rob Herring <[email protected]> wrote:
>
> On Tue, Aug 6, 2019 at 1:27 PM Saravana Kannan <[email protected]> wrote:
> >
> > PowerPC platforms don't use the generic of/platform code to populate the
> > devices from DT.
>
> Yes, they do.
No they don't. My wording could be better, but they don't use
of_platform_default_populate_init()
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/platform.c#n511
>
> > Therefore the generic device linking code is never used
> > in PowerPC. Compile it out to avoid warning about unused functions.
>
> I'd prefer this get disabled on PPC using 'if (IS_ENABLED(CONFIG_PPC))
> return' rather than #ifdefs.
I'm just moving the existing ifndef some lines above. I don't want to
go change existing #ifndef in this patch. Maybe that should be a
separate patch series that goes and fixes all such code in drivers/of/
or driver/
-Saravana
>
> >
> > If a specific PowerPC platform wants to use this code in the future,
> > bringing this back for PowerPC would be trivial. We'll just need to export
> > of_link_to_suppliers() and then let the machine specific files do the
> > linking as they populate the devices from DT.
> >
> > Signed-off-by: Saravana Kannan <[email protected]>
> > ---
> > drivers/of/platform.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index f68de5c4aeff..a2a4e4b79d43 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -506,6 +506,7 @@ int of_platform_default_populate(struct device_node *root,
> > }
> > EXPORT_SYMBOL_GPL(of_platform_default_populate);
> >
> > +#ifndef CONFIG_PPC
> > static bool of_link_is_valid(struct device_node *con, struct device_node *sup)
> > {
> > of_node_get(sup);
> > @@ -683,7 +684,6 @@ static int of_link_to_suppliers(struct device *dev)
> > return __of_link_to_suppliers(dev, dev->of_node);
> > }
> >
> > -#ifndef CONFIG_PPC
> > static const struct of_device_id reserved_mem_matches[] = {
> > { .compatible = "qcom,rmtfs-mem" },
> > { .compatible = "qcom,cmd-db" },
> > --
> > 2.22.0.770.g0f2c4a37fd-goog
> >
On Tue, Aug 6, 2019 at 3:04 PM Saravana Kannan <[email protected]> wrote:
>
> On Tue, Aug 6, 2019 at 2:27 PM Rob Herring <[email protected]> wrote:
> >
> > On Tue, Aug 6, 2019 at 1:27 PM Saravana Kannan <[email protected]> wrote:
> > >
> > > PowerPC platforms don't use the generic of/platform code to populate the
> > > devices from DT.
> >
> > Yes, they do.
>
> No they don't. My wording could be better, but they don't use
> of_platform_default_populate_init()
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/platform.c#n511
>
> >
> > > Therefore the generic device linking code is never used
> > > in PowerPC. Compile it out to avoid warning about unused functions.
> >
> > I'd prefer this get disabled on PPC using 'if (IS_ENABLED(CONFIG_PPC))
> > return' rather than #ifdefs.
>
> I'm just moving the existing ifndef some lines above. I don't want to
> go change existing #ifndef in this patch. Maybe that should be a
> separate patch series that goes and fixes all such code in drivers/of/
> or driver/
Bump. Thoughts? I don't think changing the existing if(n)defs should
be part of this patch series.
-Saravana
On Tue, Aug 6, 2019 at 4:04 PM Saravana Kannan <[email protected]> wrote:
>
> On Tue, Aug 6, 2019 at 2:27 PM Rob Herring <[email protected]> wrote:
> >
> > On Tue, Aug 6, 2019 at 1:27 PM Saravana Kannan <[email protected]> wrote:
> > >
> > > PowerPC platforms don't use the generic of/platform code to populate the
> > > devices from DT.
> >
> > Yes, they do.
>
> No they don't. My wording could be better, but they don't use
> of_platform_default_populate_init()
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/platform.c#n511
Right, but the rest of the of/platform code is used (guess where it
got moved here from?).
> > > Therefore the generic device linking code is never used
> > > in PowerPC. Compile it out to avoid warning about unused functions.
> >
> > I'd prefer this get disabled on PPC using 'if (IS_ENABLED(CONFIG_PPC))
> > return' rather than #ifdefs.
>
> I'm just moving the existing ifndef some lines above. I don't want to
> go change existing #ifndef in this patch. Maybe that should be a
> separate patch series that goes and fixes all such code in drivers/of/
> or driver/
So the initcall was originally just supposed to call
of_platform_default_populate(), but it's grown beyond that. That could
make things fragile as it is possible for platforms to call
of_platform_populate() (directly or indirectly) before
of_platform_default_populate_init(). That was supposed to work, but
now I think it's getting more fragile.
Anyways, I guess this patch is fine for now.
Rob
On Wed, Aug 14, 2019 at 4:41 PM Rob Herring <[email protected]> wrote:
>
> On Tue, Aug 6, 2019 at 4:04 PM Saravana Kannan <[email protected]> wrote:
> >
> > On Tue, Aug 6, 2019 at 2:27 PM Rob Herring <[email protected]> wrote:
> > >
> > > On Tue, Aug 6, 2019 at 1:27 PM Saravana Kannan <[email protected]> wrote:
> > > >
> > > > PowerPC platforms don't use the generic of/platform code to populate the
> > > > devices from DT.
> > >
> > > Yes, they do.
> >
> > No they don't. My wording could be better, but they don't use
> > of_platform_default_populate_init()
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/platform.c#n511
>
> Right, but the rest of the of/platform code is used (guess where it
> got moved here from?).
>
> > > > Therefore the generic device linking code is never used
> > > > in PowerPC. Compile it out to avoid warning about unused functions.
> > >
> > > I'd prefer this get disabled on PPC using 'if (IS_ENABLED(CONFIG_PPC))
> > > return' rather than #ifdefs.
> >
> > I'm just moving the existing ifndef some lines above. I don't want to
> > go change existing #ifndef in this patch. Maybe that should be a
> > separate patch series that goes and fixes all such code in drivers/of/
> > or driver/
>
> So the initcall was originally just supposed to call
> of_platform_default_populate(), but it's grown beyond that. That could
> make things fragile as it is possible for platforms to call
> of_platform_populate() (directly or indirectly) before
> of_platform_default_populate_init(). That was supposed to work, but
> now I think it's getting more fragile.
Can you clarify what's wrong with of_platfrom_populate() being called
before of_platform_default_populate_init()? If that's what a platform
wants to do, they can do it? I have some thoughts of my own, but I
want to hear yours.
In any case, I'd be happy to help clean up this initcall if you can
give me a direction to take it in.
> Anyways, I guess this patch is fine for now.
Thanks.
-Saravana
On Thu, Aug 15, 2019 at 9:04 PM Saravana Kannan <[email protected]> wrote:
>
> On Wed, Aug 14, 2019 at 4:41 PM Rob Herring <[email protected]> wrote:
> >
> > On Tue, Aug 6, 2019 at 4:04 PM Saravana Kannan <[email protected]> wrote:
> > >
> > > On Tue, Aug 6, 2019 at 2:27 PM Rob Herring <[email protected]> wrote:
> > > >
> > > > On Tue, Aug 6, 2019 at 1:27 PM Saravana Kannan <[email protected]> wrote:
> > > > >
> > > > > PowerPC platforms don't use the generic of/platform code to populate the
> > > > > devices from DT.
> > > >
> > > > Yes, they do.
> > >
> > > No they don't. My wording could be better, but they don't use
> > > of_platform_default_populate_init()
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/platform.c#n511
> >
> > Right, but the rest of the of/platform code is used (guess where it
> > got moved here from?).
> >
> > > > > Therefore the generic device linking code is never used
> > > > > in PowerPC. Compile it out to avoid warning about unused functions.
> > > >
> > > > I'd prefer this get disabled on PPC using 'if (IS_ENABLED(CONFIG_PPC))
> > > > return' rather than #ifdefs.
> > >
> > > I'm just moving the existing ifndef some lines above. I don't want to
> > > go change existing #ifndef in this patch. Maybe that should be a
> > > separate patch series that goes and fixes all such code in drivers/of/
> > > or driver/
> >
> > So the initcall was originally just supposed to call
> > of_platform_default_populate(), but it's grown beyond that. That could
> > make things fragile as it is possible for platforms to call
> > of_platform_populate() (directly or indirectly) before
> > of_platform_default_populate_init(). That was supposed to work, but
> > now I think it's getting more fragile.
>
> Can you clarify what's wrong with of_platfrom_populate() being called
> before of_platform_default_populate_init()? If that's what a platform
> wants to do, they can do it? I have some thoughts of my own, but I
> want to hear yours.
Really, I'd like to get rid of platforms doing their own calls. That's
mostly an arm32 issue. Most of what's left are either platforms using
auxdata which was supposed to be a transition thing or ones that set a
parent device (soc_device). The former takes work to finish converting
platforms to DT and I don't know what to do for the latter other than
always or never set a parent device. Also, I know there's an issue on
atmel where we can't remove their of_platform_populate call because it
changes the probe order and breaks their pinctrl and gpio driver (I
started a patch for that...).
Rob