diff mbox

dm: include <dm/util.h> from drivers/core/util.c

Message ID 1498117801-19220-1-git-send-email-yamada.masahiro@socionext.com
State Accepted
Commit 766c28a548714a8944e9638a2727157191ff6cca
Headers show

Commit Message

Masahiro Yamada June 22, 2017, 7:50 a.m. UTC
Fix sparse warnings "... was not declared. Should it be static?"

Also, fix redefinition of dm_warn/dm_dbg.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/core/util.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Bin Meng June 22, 2017, 8:21 a.m. UTC | #1
On Thu, Jun 22, 2017 at 3:50 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Fix sparse warnings "... was not declared. Should it be static?"
>
> Also, fix redefinition of dm_warn/dm_dbg.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  drivers/core/util.c | 5 +++++
>  1 file changed, 5 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Simon Glass July 6, 2017, 4:49 a.m. UTC | #2
Hi Masahiro,

On 22 June 2017 at 01:50, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Fix sparse warnings "... was not declared. Should it be static?"
>
> Also, fix redefinition of dm_warn/dm_dbg.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  drivers/core/util.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/core/util.c b/drivers/core/util.c
> index 5ceac8bbb15b..2e232d57a14f 100644
> --- a/drivers/core/util.c
> +++ b/drivers/core/util.c
> @@ -5,9 +5,11 @@
>   */
>
>  #include <common.h>
> +#include <dm/util.h>
>  #include <libfdt.h>
>  #include <vsprintf.h>
>
> +#ifdef CONFIG_DM_WARN
>  void dm_warn(const char *fmt, ...)
>  {
>         va_list args;
> @@ -16,7 +18,9 @@ void dm_warn(const char *fmt, ...)
>         vprintf(fmt, args);
>         va_end(args);
>  }
> +#endif
>
> +#ifdef DEBUG

I don't think you can add this #ifdef, since DEBUG can be defined for
any one, not necessarily this one. If someone adds "#define DEBUG' to
(say) llists.c then this will cause a compile error.

>  void dm_dbg(const char *fmt, ...)
>  {
>         va_list args;
> @@ -25,6 +29,7 @@ void dm_dbg(const char *fmt, ...)
>         vprintf(fmt, args);
>         va_end(args);
>  }
> +#endif
>
>  int list_count_items(struct list_head *head)
>  {
> --
> 2.7.4
>

Regards,
Simon
Simon Glass July 6, 2017, 5:36 p.m. UTC | #3
Hi Masahiro,

On 22 June 2017 at 01:50, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Fix sparse warnings "... was not declared. Should it be static?"
>
> Also, fix redefinition of dm_warn/dm_dbg.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  drivers/core/util.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
Applied to u-boot-dm, thanks!
Masahiro Yamada July 7, 2017, 1:26 a.m. UTC | #4
Hi Simon,



2017-07-06 13:49 GMT+09:00 Simon Glass <sjg@chromium.org>:
> Hi Masahiro,
>
> On 22 June 2017 at 01:50, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>> Fix sparse warnings "... was not declared. Should it be static?"
>>
>> Also, fix redefinition of dm_warn/dm_dbg.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  drivers/core/util.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/core/util.c b/drivers/core/util.c
>> index 5ceac8bbb15b..2e232d57a14f 100644
>> --- a/drivers/core/util.c
>> +++ b/drivers/core/util.c
>> @@ -5,9 +5,11 @@
>>   */
>>
>>  #include <common.h>
>> +#include <dm/util.h>
>>  #include <libfdt.h>
>>  #include <vsprintf.h>
>>
>> +#ifdef CONFIG_DM_WARN
>>  void dm_warn(const char *fmt, ...)
>>  {
>>         va_list args;
>> @@ -16,7 +18,9 @@ void dm_warn(const char *fmt, ...)
>>         vprintf(fmt, args);
>>         va_end(args);
>>  }
>> +#endif
>>
>> +#ifdef DEBUG
>
> I don't think you can add this #ifdef, since DEBUG can be defined for
> any one, not necessarily this one. If someone adds "#define DEBUG' to
> (say) llists.c then this will cause a compile error.

You are right, (but you picked up this)

So, what shall we do?

Drop this patch from u-boot-dm, and respin it?


For example, we can use #ifdef CONFIG_DM_DEBUG
instead of #ifdef DEBUG.
Simon Glass July 7, 2017, 3:56 a.m. UTC | #5
Hi Masahiro,

On 6 July 2017 at 19:26, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Hi Simon,
>
>
>
> 2017-07-06 13:49 GMT+09:00 Simon Glass <sjg@chromium.org>:
>> Hi Masahiro,
>>
>> On 22 June 2017 at 01:50, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>>> Fix sparse warnings "... was not declared. Should it be static?"
>>>
>>> Also, fix redefinition of dm_warn/dm_dbg.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  drivers/core/util.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/core/util.c b/drivers/core/util.c
>>> index 5ceac8bbb15b..2e232d57a14f 100644
>>> --- a/drivers/core/util.c
>>> +++ b/drivers/core/util.c
>>> @@ -5,9 +5,11 @@
>>>   */
>>>
>>>  #include <common.h>
>>> +#include <dm/util.h>
>>>  #include <libfdt.h>
>>>  #include <vsprintf.h>
>>>
>>> +#ifdef CONFIG_DM_WARN
>>>  void dm_warn(const char *fmt, ...)
>>>  {
>>>         va_list args;
>>> @@ -16,7 +18,9 @@ void dm_warn(const char *fmt, ...)
>>>         vprintf(fmt, args);
>>>         va_end(args);
>>>  }
>>> +#endif
>>>
>>> +#ifdef DEBUG
>>
>> I don't think you can add this #ifdef, since DEBUG can be defined for
>> any one, not necessarily this one. If someone adds "#define DEBUG' to
>> (say) llists.c then this will cause a compile error.
>
> You are right, (but you picked up this)
>
> So, what shall we do?
>
> Drop this patch from u-boot-dm, and respin it?

Either that or do a fix-up patch. Whichever is easier for you.

Regards,
Simon
diff mbox

Patch

diff --git a/drivers/core/util.c b/drivers/core/util.c
index 5ceac8bbb15b..2e232d57a14f 100644
--- a/drivers/core/util.c
+++ b/drivers/core/util.c
@@ -5,9 +5,11 @@ 
  */
 
 #include <common.h>
+#include <dm/util.h>
 #include <libfdt.h>
 #include <vsprintf.h>
 
+#ifdef CONFIG_DM_WARN
 void dm_warn(const char *fmt, ...)
 {
 	va_list args;
@@ -16,7 +18,9 @@  void dm_warn(const char *fmt, ...)
 	vprintf(fmt, args);
 	va_end(args);
 }
+#endif
 
+#ifdef DEBUG
 void dm_dbg(const char *fmt, ...)
 {
 	va_list args;
@@ -25,6 +29,7 @@  void dm_dbg(const char *fmt, ...)
 	vprintf(fmt, args);
 	va_end(args);
 }
+#endif
 
 int list_count_items(struct list_head *head)
 {