2019-06-12 07:19:00

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH next] of/fdt: Fix defined but not used compiler warning

When CONFIG_OF_EARLY_FLATTREE is disabled, there is a compiler warning,

drivers/of/fdt.c:129:19: warning: ‘of_fdt_match’ defined but not used [-Wunused-function]
static int __init of_fdt_match(const void *blob, unsigned long node,

Move of_fdt_match() and of_fdt_is_compatible() under CONFIG_OF_EARLY_FLATTREE
to fix it.

Cc: Stephen Boyd <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Frank Rowand <[email protected]>
Signed-off-by: Kefeng Wang <[email protected]>
---
drivers/of/fdt.c | 106 +++++++++++++++++++++++------------------------
1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 3d36b5afd9bd..d6afd5b22940 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -78,38 +78,6 @@ void __init of_fdt_limit_memory(int limit)
}
}

-/**
- * of_fdt_is_compatible - Return true if given node from the given blob has
- * compat in its compatible list
- * @blob: A device tree blob
- * @node: node to test
- * @compat: compatible string to compare with compatible list.
- *
- * On match, returns a non-zero value with smaller values returned for more
- * specific compatible values.
- */
-static int of_fdt_is_compatible(const void *blob,
- unsigned long node, const char *compat)
-{
- const char *cp;
- int cplen;
- unsigned long l, score = 0;
-
- cp = fdt_getprop(blob, node, "compatible", &cplen);
- if (cp == NULL)
- return 0;
- while (cplen > 0) {
- score++;
- if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
- return score;
- l = strlen(cp) + 1;
- cp += l;
- cplen -= l;
- }
-
- return 0;
-}
-
static bool of_fdt_device_is_available(const void *blob, unsigned long node)
{
const char *status = fdt_getprop(blob, node, "status", NULL);
@@ -123,27 +91,6 @@ static bool of_fdt_device_is_available(const void *blob, unsigned long node)
return false;
}

-/**
- * of_fdt_match - Return true if node matches a list of compatible values
- */
-static int __init of_fdt_match(const void *blob, unsigned long node,
- const char *const *compat)
-{
- unsigned int tmp, score = 0;
-
- if (!compat)
- return 0;
-
- while (*compat) {
- tmp = of_fdt_is_compatible(blob, node, *compat);
- if (tmp && (score == 0 || (tmp < score)))
- score = tmp;
- compat++;
- }
-
- return score;
-}
-
static void *unflatten_dt_alloc(void **mem, unsigned long size,
unsigned long align)
{
@@ -764,6 +711,59 @@ const void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
return fdt_getprop(initial_boot_params, node, name, size);
}

+/**
+ * of_fdt_is_compatible - Return true if given node from the given blob has
+ * compat in its compatible list
+ * @blob: A device tree blob
+ * @node: node to test
+ * @compat: compatible string to compare with compatible list.
+ *
+ * On match, returns a non-zero value with smaller values returned for more
+ * specific compatible values.
+ */
+static int of_fdt_is_compatible(const void *blob,
+ unsigned long node, const char *compat)
+{
+ const char *cp;
+ int cplen;
+ unsigned long l, score = 0;
+
+ cp = fdt_getprop(blob, node, "compatible", &cplen);
+ if (cp == NULL)
+ return 0;
+ while (cplen > 0) {
+ score++;
+ if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
+ return score;
+ l = strlen(cp) + 1;
+ cp += l;
+ cplen -= l;
+ }
+
+ return 0;
+}
+
+/**
+ * of_fdt_match - Return true if node matches a list of compatible values
+ */
+static int __init of_fdt_match(const void *blob, unsigned long node,
+ const char *const *compat)
+{
+ unsigned int tmp, score = 0;
+
+ if (!compat)
+ return 0;
+
+ while (*compat) {
+ tmp = of_fdt_is_compatible(blob, node, *compat);
+ if (tmp && (score == 0 || (tmp < score)))
+ score = tmp;
+ compat++;
+ }
+
+ return score;
+}
+
/**
* of_flat_dt_is_compatible - Return true if given node has compat in compatible list
* @node: node to test
--
2.20.1


2019-06-12 18:03:42

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH next] of/fdt: Fix defined but not used compiler warning

Quoting Kefeng Wang (2019-06-11 18:00:11)
> When CONFIG_OF_EARLY_FLATTREE is disabled, there is a compiler warning,
>
> drivers/of/fdt.c:129:19: warning: ‘of_fdt_match’ defined but not used [-Wunused-function]
> static int __init of_fdt_match(const void *blob, unsigned long node,
>
> Move of_fdt_match() and of_fdt_is_compatible() under CONFIG_OF_EARLY_FLATTREE
> to fix it.
>
> Cc: Stephen Boyd <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Frank Rowand <[email protected]>
> Signed-off-by: Kefeng Wang <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2019-06-12 18:07:33

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH next] of/fdt: Fix defined but not used compiler warning

Hi Kefeng,

If Rob agrees, I'd like to see one more change in this patch.

Since the only caller of of_fdt_match() is of_flat_dt_match(),
can you move the body of of_fdt_match() into of_flat_dt_match()
and eliminate of_fdt_match()?

(Noting that of_flat_dt_match() consists only of the call to
of_fdt_match().)

-Frank


On 6/11/19 6:00 PM, Kefeng Wang wrote:
> When CONFIG_OF_EARLY_FLATTREE is disabled, there is a compiler warning,
>
> drivers/of/fdt.c:129:19: warning: ‘of_fdt_match’ defined but not used [-Wunused-function]
> static int __init of_fdt_match(const void *blob, unsigned long node,
>
> Move of_fdt_match() and of_fdt_is_compatible() under CONFIG_OF_EARLY_FLATTREE
> to fix it.
>
> Cc: Stephen Boyd <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Frank Rowand <[email protected]>
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
> drivers/of/fdt.c | 106 +++++++++++++++++++++++------------------------
> 1 file changed, 53 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 3d36b5afd9bd..d6afd5b22940 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -78,38 +78,6 @@ void __init of_fdt_limit_memory(int limit)
> }
> }
>
> -/**
> - * of_fdt_is_compatible - Return true if given node from the given blob has
> - * compat in its compatible list
> - * @blob: A device tree blob
> - * @node: node to test
> - * @compat: compatible string to compare with compatible list.
> - *
> - * On match, returns a non-zero value with smaller values returned for more
> - * specific compatible values.
> - */
> -static int of_fdt_is_compatible(const void *blob,
> - unsigned long node, const char *compat)
> -{
> - const char *cp;
> - int cplen;
> - unsigned long l, score = 0;
> -
> - cp = fdt_getprop(blob, node, "compatible", &cplen);
> - if (cp == NULL)
> - return 0;
> - while (cplen > 0) {
> - score++;
> - if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> - return score;
> - l = strlen(cp) + 1;
> - cp += l;
> - cplen -= l;
> - }
> -
> - return 0;
> -}
> -
> static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> {
> const char *status = fdt_getprop(blob, node, "status", NULL);
> @@ -123,27 +91,6 @@ static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> return false;
> }
>
> -/**
> - * of_fdt_match - Return true if node matches a list of compatible values
> - */
> -static int __init of_fdt_match(const void *blob, unsigned long node,> - const char *const *compat)
> -{
> - unsigned int tmp, score = 0;
> -
> - if (!compat)
> - return 0;
> -
> - while (*compat) {
> - tmp = of_fdt_is_compatible(blob, node, *compat);
> - if (tmp && (score == 0 || (tmp < score)))
> - score = tmp;
> - compat++;
> - }
> -
> - return score;
> -}
> -
> static void *unflatten_dt_alloc(void **mem, unsigned long size,
> unsigned long align)
> {
> @@ -764,6 +711,59 @@ const void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
> return fdt_getprop(initial_boot_params, node, name, size);
> }
>
> +/**
> + * of_fdt_is_compatible - Return true if given node from the given blob has
> + * compat in its compatible list
> + * @blob: A device tree blob
> + * @node: node to test
> + * @compat: compatible string to compare with compatible list.
> + *
> + * On match, returns a non-zero value with smaller values returned for more
> + * specific compatible values.
> + */
> +static int of_fdt_is_compatible(const void *blob,
> + unsigned long node, const char *compat)
> +{
> + const char *cp;
> + int cplen;
> + unsigned long l, score = 0;
> +
> + cp = fdt_getprop(blob, node, "compatible", &cplen);
> + if (cp == NULL)
> + return 0;
> + while (cplen > 0) {
> + score++;
> + if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> + return score;
> + l = strlen(cp) + 1;
> + cp += l;
> + cplen -= l;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * of_fdt_match - Return true if node matches a list of compatible values
> + */
> +static int __init of_fdt_match(const void *blob, unsigned long node,
> + const char *const *compat)
> +{
> + unsigned int tmp, score = 0;
> +
> + if (!compat)
> + return 0;
> +
> + while (*compat) {
> + tmp = of_fdt_is_compatible(blob, node, *compat);
> + if (tmp && (score == 0 || (tmp < score)))
> + score = tmp;
> + compat++;
> + }
> +
> + return score;
> +}
> +
> /**
> * of_flat_dt_is_compatible - Return true if given node has compat in compatible list
> * @node: node to test
>

2019-06-12 18:09:20

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH next] of/fdt: Fix defined but not used compiler warning

On Wed, Jun 12, 2019 at 10:45 AM Frank Rowand <[email protected]> wrote:
>
> Hi Kefeng,
>
> If Rob agrees, I'd like to see one more change in this patch.
>
> Since the only caller of of_fdt_match() is of_flat_dt_match(),
> can you move the body of of_fdt_match() into of_flat_dt_match()
> and eliminate of_fdt_match()?

That's fine as long as we think there's never any use for of_fdt_match
after init? Fixup of nodes in an overlay for example.

Rob

>
> (Noting that of_flat_dt_match() consists only of the call to
> of_fdt_match().)
>
> -Frank
>
>
> On 6/11/19 6:00 PM, Kefeng Wang wrote:
> > When CONFIG_OF_EARLY_FLATTREE is disabled, there is a compiler warning,
> >
> > drivers/of/fdt.c:129:19: warning: ‘of_fdt_match’ defined but not used [-Wunused-function]
> > static int __init of_fdt_match(const void *blob, unsigned long node,
> >
> > Move of_fdt_match() and of_fdt_is_compatible() under CONFIG_OF_EARLY_FLATTREE
> > to fix it.
> >
> > Cc: Stephen Boyd <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Frank Rowand <[email protected]>
> > Signed-off-by: Kefeng Wang <[email protected]>
> > ---
> > drivers/of/fdt.c | 106 +++++++++++++++++++++++------------------------
> > 1 file changed, 53 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index 3d36b5afd9bd..d6afd5b22940 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -78,38 +78,6 @@ void __init of_fdt_limit_memory(int limit)
> > }
> > }
> >
> > -/**
> > - * of_fdt_is_compatible - Return true if given node from the given blob has
> > - * compat in its compatible list
> > - * @blob: A device tree blob
> > - * @node: node to test
> > - * @compat: compatible string to compare with compatible list.
> > - *
> > - * On match, returns a non-zero value with smaller values returned for more
> > - * specific compatible values.
> > - */
> > -static int of_fdt_is_compatible(const void *blob,
> > - unsigned long node, const char *compat)
> > -{
> > - const char *cp;
> > - int cplen;
> > - unsigned long l, score = 0;
> > -
> > - cp = fdt_getprop(blob, node, "compatible", &cplen);
> > - if (cp == NULL)
> > - return 0;
> > - while (cplen > 0) {
> > - score++;
> > - if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> > - return score;
> > - l = strlen(cp) + 1;
> > - cp += l;
> > - cplen -= l;
> > - }
> > -
> > - return 0;
> > -}
> > -
> > static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> > {
> > const char *status = fdt_getprop(blob, node, "status", NULL);
> > @@ -123,27 +91,6 @@ static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> > return false;
> > }
> >
> > -/**
> > - * of_fdt_match - Return true if node matches a list of compatible values
> > - */
> > -static int __init of_fdt_match(const void *blob, unsigned long node,> - const char *const *compat)
> > -{
> > - unsigned int tmp, score = 0;
> > -
> > - if (!compat)
> > - return 0;
> > -
> > - while (*compat) {
> > - tmp = of_fdt_is_compatible(blob, node, *compat);
> > - if (tmp && (score == 0 || (tmp < score)))
> > - score = tmp;
> > - compat++;
> > - }
> > -
> > - return score;
> > -}
> > -
> > static void *unflatten_dt_alloc(void **mem, unsigned long size,
> > unsigned long align)
> > {
> > @@ -764,6 +711,59 @@ const void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
> > return fdt_getprop(initial_boot_params, node, name, size);
> > }
> >
> > +/**
> > + * of_fdt_is_compatible - Return true if given node from the given blob has
> > + * compat in its compatible list
> > + * @blob: A device tree blob
> > + * @node: node to test
> > + * @compat: compatible string to compare with compatible list.
> > + *
> > + * On match, returns a non-zero value with smaller values returned for more
> > + * specific compatible values.
> > + */
> > +static int of_fdt_is_compatible(const void *blob,
> > + unsigned long node, const char *compat)
> > +{
> > + const char *cp;
> > + int cplen;
> > + unsigned long l, score = 0;
> > +
> > + cp = fdt_getprop(blob, node, "compatible", &cplen);
> > + if (cp == NULL)
> > + return 0;
> > + while (cplen > 0) {
> > + score++;
> > + if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> > + return score;
> > + l = strlen(cp) + 1;
> > + cp += l;
> > + cplen -= l;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * of_fdt_match - Return true if node matches a list of compatible values
> > + */
> > +static int __init of_fdt_match(const void *blob, unsigned long node,
> > + const char *const *compat)
> > +{
> > + unsigned int tmp, score = 0;
> > +
> > + if (!compat)
> > + return 0;
> > +
> > + while (*compat) {
> > + tmp = of_fdt_is_compatible(blob, node, *compat);
> > + if (tmp && (score == 0 || (tmp < score)))
> > + score = tmp;
> > + compat++;
> > + }
> > +
> > + return score;
> > +}
> > +
> > /**
> > * of_flat_dt_is_compatible - Return true if given node has compat in compatible list
> > * @node: node to test
> >
>

2019-06-12 18:30:53

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH next] of/fdt: Fix defined but not used compiler warning

On 6/12/19 10:00 AM, Rob Herring wrote:
> On Wed, Jun 12, 2019 at 10:45 AM Frank Rowand <[email protected]> wrote:
>>
>> Hi Kefeng,
>>
>> If Rob agrees, I'd like to see one more change in this patch.
>>
>> Since the only caller of of_fdt_match() is of_flat_dt_match(),
>> can you move the body of of_fdt_match() into of_flat_dt_match()
>> and eliminate of_fdt_match()?
>
> That's fine as long as we think there's never any use for of_fdt_match
> after init? Fixup of nodes in an overlay for example.

We can always re-expose the functionality as of_fdt_match() in the future
if the need arises. But Stephen's recent patch was moving in the opposite
direction, removing of_fdt_match() from the header file and making it
static.

-Frank

>
> Rob
>
>>
>> (Noting that of_flat_dt_match() consists only of the call to
>> of_fdt_match().)
>>
>> -Frank
>>
>>
>> On 6/11/19 6:00 PM, Kefeng Wang wrote:
>>> When CONFIG_OF_EARLY_FLATTREE is disabled, there is a compiler warning,
>>>
>>> drivers/of/fdt.c:129:19: warning: ‘of_fdt_match’ defined but not used [-Wunused-function]
>>> static int __init of_fdt_match(const void *blob, unsigned long node,
>>>
>>> Move of_fdt_match() and of_fdt_is_compatible() under CONFIG_OF_EARLY_FLATTREE
>>> to fix it.
>>>
>>> Cc: Stephen Boyd <[email protected]>
>>> Cc: Rob Herring <[email protected]>
>>> Cc: Frank Rowand <[email protected]>
>>> Signed-off-by: Kefeng Wang <[email protected]>
>>> ---
>>> drivers/of/fdt.c | 106 +++++++++++++++++++++++------------------------
>>> 1 file changed, 53 insertions(+), 53 deletions(-)
>>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 3d36b5afd9bd..d6afd5b22940 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -78,38 +78,6 @@ void __init of_fdt_limit_memory(int limit)
>>> }
>>> }
>>>
>>> -/**
>>> - * of_fdt_is_compatible - Return true if given node from the given blob has
>>> - * compat in its compatible list
>>> - * @blob: A device tree blob
>>> - * @node: node to test
>>> - * @compat: compatible string to compare with compatible list.
>>> - *
>>> - * On match, returns a non-zero value with smaller values returned for more
>>> - * specific compatible values.
>>> - */
>>> -static int of_fdt_is_compatible(const void *blob,
>>> - unsigned long node, const char *compat)
>>> -{
>>> - const char *cp;
>>> - int cplen;
>>> - unsigned long l, score = 0;
>>> -
>>> - cp = fdt_getprop(blob, node, "compatible", &cplen);
>>> - if (cp == NULL)
>>> - return 0;
>>> - while (cplen > 0) {
>>> - score++;
>>> - if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
>>> - return score;
>>> - l = strlen(cp) + 1;
>>> - cp += l;
>>> - cplen -= l;
>>> - }
>>> -
>>> - return 0;
>>> -}
>>> -
>>> static bool of_fdt_device_is_available(const void *blob, unsigned long node)
>>> {
>>> const char *status = fdt_getprop(blob, node, "status", NULL);
>>> @@ -123,27 +91,6 @@ static bool of_fdt_device_is_available(const void *blob, unsigned long node)
>>> return false;
>>> }
>>>
>>> -/**
>>> - * of_fdt_match - Return true if node matches a list of compatible values
>>> - */
>>> -static int __init of_fdt_match(const void *blob, unsigned long node,> - const char *const *compat)
>>> -{
>>> - unsigned int tmp, score = 0;
>>> -
>>> - if (!compat)
>>> - return 0;
>>> -
>>> - while (*compat) {
>>> - tmp = of_fdt_is_compatible(blob, node, *compat);
>>> - if (tmp && (score == 0 || (tmp < score)))
>>> - score = tmp;
>>> - compat++;
>>> - }
>>> -
>>> - return score;
>>> -}
>>> -
>>> static void *unflatten_dt_alloc(void **mem, unsigned long size,
>>> unsigned long align)
>>> {
>>> @@ -764,6 +711,59 @@ const void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
>>> return fdt_getprop(initial_boot_params, node, name, size);
>>> }
>>>
>>> +/**
>>> + * of_fdt_is_compatible - Return true if given node from the given blob has
>>> + * compat in its compatible list
>>> + * @blob: A device tree blob
>>> + * @node: node to test
>>> + * @compat: compatible string to compare with compatible list.
>>> + *
>>> + * On match, returns a non-zero value with smaller values returned for more
>>> + * specific compatible values.
>>> + */
>>> +static int of_fdt_is_compatible(const void *blob,
>>> + unsigned long node, const char *compat)
>>> +{
>>> + const char *cp;
>>> + int cplen;
>>> + unsigned long l, score = 0;
>>> +
>>> + cp = fdt_getprop(blob, node, "compatible", &cplen);
>>> + if (cp == NULL)
>>> + return 0;
>>> + while (cplen > 0) {
>>> + score++;
>>> + if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
>>> + return score;
>>> + l = strlen(cp) + 1;
>>> + cp += l;
>>> + cplen -= l;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * of_fdt_match - Return true if node matches a list of compatible values
>>> + */
>>> +static int __init of_fdt_match(const void *blob, unsigned long node,
>>> + const char *const *compat)
>>> +{
>>> + unsigned int tmp, score = 0;
>>> +
>>> + if (!compat)
>>> + return 0;
>>> +
>>> + while (*compat) {
>>> + tmp = of_fdt_is_compatible(blob, node, *compat);
>>> + if (tmp && (score == 0 || (tmp < score)))
>>> + score = tmp;
>>> + compat++;
>>> + }
>>> +
>>> + return score;
>>> +}
>>> +
>>> /**
>>> * of_flat_dt_is_compatible - Return true if given node has compat in compatible list
>>> * @node: node to test
>>>
>>
>

2019-06-14 13:54:03

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH next] of/fdt: Fix defined but not used compiler warning

On Wed, Jun 12, 2019 at 12:29 PM Frank Rowand <[email protected]> wrote:
>
> On 6/12/19 10:00 AM, Rob Herring wrote:
> > On Wed, Jun 12, 2019 at 10:45 AM Frank Rowand <[email protected]> wrote:
> >>
> >> Hi Kefeng,
> >>
> >> If Rob agrees, I'd like to see one more change in this patch.
> >>
> >> Since the only caller of of_fdt_match() is of_flat_dt_match(),
> >> can you move the body of of_fdt_match() into of_flat_dt_match()
> >> and eliminate of_fdt_match()?
> >
> > That's fine as long as we think there's never any use for of_fdt_match
> > after init? Fixup of nodes in an overlay for example.
>
> We can always re-expose the functionality as of_fdt_match() in the future
> if the need arises. But Stephen's recent patch was moving in the opposite
> direction, removing of_fdt_match() from the header file and making it
> static.

Yes, we can, but it is just churn if we think it is likely needed.

OTOH, we probably want users to just use libfdt API directly and
should add this to libfdt if needed.

So yes, please implement Frank's suggestion.

Rob

2019-06-15 02:56:50

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH next v2] of/fdt: Fix defined but not used compiler warning

When CONFIG_OF_EARLY_FLATTREE is disabled, there is a compiler
warning,

drivers/of/fdt.c:129:19: warning: ‘of_fdt_match’ defined but not used [-Wunused-function]
static int __init of_fdt_match(const void *blob, unsigned long node,

Since the only caller of of_fdt_match() is of_flat_dt_match(),
let's move the body of of_fdt_match() into of_flat_dt_match()
and eliminate of_fdt_match().

Meanwhile, move of_fdt_is_compatible() under CONFIG_OF_EARLY_FLATTREE,
as all callers are over there.

Cc: Stephen Boyd <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Frank Rowand <[email protected]>
Signed-off-by: Kefeng Wang <[email protected]>
---
v2:
-Move the body of of_fdt_match() into of_flat_dt_match(), suggested by Frank.

drivers/of/fdt.c | 99 ++++++++++++++++++++++--------------------------
1 file changed, 45 insertions(+), 54 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 3d36b5afd9bd..cd17dc62a719 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -78,38 +78,6 @@ void __init of_fdt_limit_memory(int limit)
}
}

-/**
- * of_fdt_is_compatible - Return true if given node from the given blob has
- * compat in its compatible list
- * @blob: A device tree blob
- * @node: node to test
- * @compat: compatible string to compare with compatible list.
- *
- * On match, returns a non-zero value with smaller values returned for more
- * specific compatible values.
- */
-static int of_fdt_is_compatible(const void *blob,
- unsigned long node, const char *compat)
-{
- const char *cp;
- int cplen;
- unsigned long l, score = 0;
-
- cp = fdt_getprop(blob, node, "compatible", &cplen);
- if (cp == NULL)
- return 0;
- while (cplen > 0) {
- score++;
- if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
- return score;
- l = strlen(cp) + 1;
- cp += l;
- cplen -= l;
- }
-
- return 0;
-}
-
static bool of_fdt_device_is_available(const void *blob, unsigned long node)
{
const char *status = fdt_getprop(blob, node, "status", NULL);
@@ -123,27 +91,6 @@ static bool of_fdt_device_is_available(const void *blob, unsigned long node)
return false;
}

-/**
- * of_fdt_match - Return true if node matches a list of compatible values
- */
-static int __init of_fdt_match(const void *blob, unsigned long node,
- const char *const *compat)
-{
- unsigned int tmp, score = 0;
-
- if (!compat)
- return 0;
-
- while (*compat) {
- tmp = of_fdt_is_compatible(blob, node, *compat);
- if (tmp && (score == 0 || (tmp < score)))
- score = tmp;
- compat++;
- }
-
- return score;
-}
-
static void *unflatten_dt_alloc(void **mem, unsigned long size,
unsigned long align)
{
@@ -764,6 +711,38 @@ const void *__init of_get_flat_dt_prop(unsigned long node, const char *name,
return fdt_getprop(initial_boot_params, node, name, size);
}

+/**
+ * of_fdt_is_compatible - Return true if given node from the given blob has
+ * compat in its compatible list
+ * @blob: A device tree blob
+ * @node: node to test
+ * @compat: compatible string to compare with compatible list.
+ *
+ * On match, returns a non-zero value with smaller values returned for more
+ * specific compatible values.
+ */
+static int of_fdt_is_compatible(const void *blob,
+ unsigned long node, const char *compat)
+{
+ const char *cp;
+ int cplen;
+ unsigned long l, score = 0;
+
+ cp = fdt_getprop(blob, node, "compatible", &cplen);
+ if (cp == NULL)
+ return 0;
+ while (cplen > 0) {
+ score++;
+ if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
+ return score;
+ l = strlen(cp) + 1;
+ cp += l;
+ cplen -= l;
+ }
+
+ return 0;
+}
+
/**
* of_flat_dt_is_compatible - Return true if given node has compat in compatible list
* @node: node to test
@@ -779,7 +758,19 @@ int __init of_flat_dt_is_compatible(unsigned long node, const char *compat)
*/
static int __init of_flat_dt_match(unsigned long node, const char *const *compat)
{
- return of_fdt_match(initial_boot_params, node, compat);
+ unsigned int tmp, score = 0;
+
+ if (!compat)
+ return 0;
+
+ while (*compat) {
+ tmp = of_fdt_is_compatible(initial_boot_params, node, *compat);
+ if (tmp && (score == 0 || (tmp < score)))
+ score = tmp;
+ compat++;
+ }
+
+ return score;
}

/**
--
2.20.1

2019-06-15 02:58:08

by Kefeng Wang

[permalink] [raw]
Subject: Re: [PATCH next] of/fdt: Fix defined but not used compiler warning



On 2019/6/14 21:53, Rob Herring wrote:
> On Wed, Jun 12, 2019 at 12:29 PM Frank Rowand <[email protected]> wrote:
>>
>> On 6/12/19 10:00 AM, Rob Herring wrote:
>>> On Wed, Jun 12, 2019 at 10:45 AM Frank Rowand <[email protected]> wrote:
>>>>
>>>> Hi Kefeng,
>>>>
>>>> If Rob agrees, I'd like to see one more change in this patch.
>>>>
>>>> Since the only caller of of_fdt_match() is of_flat_dt_match(),
>>>> can you move the body of of_fdt_match() into of_flat_dt_match()
>>>> and eliminate of_fdt_match()?
>>>
>>> That's fine as long as we think there's never any use for of_fdt_match
>>> after init? Fixup of nodes in an overlay for example.
>>
>> We can always re-expose the functionality as of_fdt_match() in the future
>> if the need arises. But Stephen's recent patch was moving in the opposite
>> direction, removing of_fdt_match() from the header file and making it
>> static.
>
> Yes, we can, but it is just churn if we think it is likely needed.
>
> OTOH, we probably want users to just use libfdt API directly and
> should add this to libfdt if needed.
>
> So yes, please implement Frank's suggestion.

OK,done in patch v2.

>
> Rob
>
> .
>

2019-06-17 15:11:40

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH next v2] of/fdt: Fix defined but not used compiler warning

Quoting Kefeng Wang (2019-06-14 20:03:43)
> When CONFIG_OF_EARLY_FLATTREE is disabled, there is a compiler
> warning,
>
> drivers/of/fdt.c:129:19: warning: ‘of_fdt_match’ defined but not used [-Wunused-function]
> static int __init of_fdt_match(const void *blob, unsigned long node,
>
> Since the only caller of of_fdt_match() is of_flat_dt_match(),
> let's move the body of of_fdt_match() into of_flat_dt_match()
> and eliminate of_fdt_match().
>
> Meanwhile, move of_fdt_is_compatible() under CONFIG_OF_EARLY_FLATTREE,
> as all callers are over there.
>
> Cc: Stephen Boyd <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Frank Rowand <[email protected]>
> Signed-off-by: Kefeng Wang <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

Arnd sent something similar now too.

2019-06-18 14:10:46

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH next v2] of/fdt: Fix defined but not used compiler warning

On Sat, 15 Jun 2019 11:03:43 +0800, Kefeng Wang wrote:
> When CONFIG_OF_EARLY_FLATTREE is disabled, there is a compiler
> warning,
>
> drivers/of/fdt.c:129:19: warning: ‘of_fdt_match’ defined but not used [-Wunused-function]
> static int __init of_fdt_match(const void *blob, unsigned long node,
>
> Since the only caller of of_fdt_match() is of_flat_dt_match(),
> let's move the body of of_fdt_match() into of_flat_dt_match()
> and eliminate of_fdt_match().
>
> Meanwhile, move of_fdt_is_compatible() under CONFIG_OF_EARLY_FLATTREE,
> as all callers are over there.
>
> Cc: Stephen Boyd <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Frank Rowand <[email protected]>
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
> v2:
> -Move the body of of_fdt_match() into of_flat_dt_match(), suggested by Frank.
>
> drivers/of/fdt.c | 99 ++++++++++++++++++++++--------------------------
> 1 file changed, 45 insertions(+), 54 deletions(-)
>

Applied, thanks.

Rob