diff mbox series

[U-Boot,v3,05/16] include: kernel.h: include printk.h

Message ID 20191113004502.29986-6-takahiro.akashi@linaro.org
State Accepted
Commit 29852bfc2296152baf31958092eb1f7384359185
Delegated to: Tom Rini
Headers show
Series import x509/pkcs7 parsers from linux | expand

Commit Message

Takahiro Akashi Nov. 13, 2019, 12:44 a.m. UTC
Adding "printk.h" will help improve portability from linux kernel
code (in my case, lib/asn1_decoder.c and others) where printf and
pr_* variant functions are used.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/linux/kernel.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Heinrich Schuchardt Nov. 26, 2019, 3:35 a.m. UTC | #1
On 11/13/19 1:44 AM, AKASHI Takahiro wrote:
> Adding "printk.h" will help improve portability from linux kernel
> code (in my case, lib/asn1_decoder.c and others) where printf and

nits:

%s/printf/printk/g

You anyway change the includes in lib/crypto/public_key.c in patch 12/16
and lib/asn1_decoder in patch 09/16. So why not add linux/printk.h there?

Otherwise I would expect this patch to remove #include <linux/printk.h>
in all files where it becomes obsolete due to this patch, e.g.

board/synopsys/hsdk/hsdk.c
arch/arm/mach-uniphier/fdt-fixup.c
arch/arm/mach-uniphier/dram_init.c
arch/arm/mach-uniphier/arm32/psci.c
arch/arm/mach-uniphier/dram/ddrphy-training.c

Best regards

Heinrich

> pr_* variant functions are used.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   include/linux/kernel.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 5c7e5f635b1a..564819a1c0a7 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -1,8 +1,8 @@
>   #ifndef _LINUX_KERNEL_H
>   #define _LINUX_KERNEL_H
>
> -
>   #include <linux/types.h>
> +#include <linux/printk.h> /* for printf/pr_* utilities */
>
>   #define USHRT_MAX	((u16)(~0U))
>   #define SHRT_MAX	((s16)(USHRT_MAX>>1))
>
Takahiro Akashi Nov. 27, 2019, 1:02 a.m. UTC | #2
Heinrich,

On Tue, Nov 26, 2019 at 04:35:00AM +0100, Heinrich Schuchardt wrote:
> On 11/13/19 1:44 AM, AKASHI Takahiro wrote:
> >Adding "printk.h" will help improve portability from linux kernel
> >code (in my case, lib/asn1_decoder.c and others) where printf and
> 
> nits:
> 
> %s/printf/printk/g

Okay.

> You anyway change the includes in lib/crypto/public_key.c in patch 12/16
> and lib/asn1_decoder in patch 09/16. So why not add linux/printk.h there?

I would rather disagree here as I have good reasons.
First, adding "printk.h" to "linux/kernel.h" makes sense simply as
Linux's original "linux/kernel.h" also has it. So when you will import
any files from linux in the future, you will probably get *less* bothered
with missing include files in .c file.
Second, I would like to change linux code at the very minimum if possible.
In lib/crypto/public_key.c, for example, I simply deleted not-used and
non-existing include files or just replaced any with its counterpart
in U-Boot (module.h -> compat.h).

> Otherwise I would expect this patch to remove #include <linux/printk.h>
> in all files where it becomes obsolete due to this patch, e.g.
> 
> board/synopsys/hsdk/hsdk.c
> arch/arm/mach-uniphier/fdt-fixup.c
> arch/arm/mach-uniphier/dram_init.c
> arch/arm/mach-uniphier/arm32/psci.c
> arch/arm/mach-uniphier/dram/ddrphy-training.c

I don't fully understand that why those files include "linux/kernel.h".
Different source files may include "linux/kernel.h" for different reasons.
Take board/synopsys/hsdk/hsdk.c, for example.
It uses ARRAY_SIZE macro for its convenience, *not* for printk.
I believe that including any include files *directly & explicitly* for
its own use of any definitions or functions is a good practice
even if such include files may be included *indirectly* via another include
file.

So I want to keep this patch unchanged.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >pr_* variant functions are used.
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >---
> >  include/linux/kernel.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> >index 5c7e5f635b1a..564819a1c0a7 100644
> >--- a/include/linux/kernel.h
> >+++ b/include/linux/kernel.h
> >@@ -1,8 +1,8 @@
> >  #ifndef _LINUX_KERNEL_H
> >  #define _LINUX_KERNEL_H
> >
> >-
> >  #include <linux/types.h>
> >+#include <linux/printk.h> /* for printf/pr_* utilities */
> >
> >  #define USHRT_MAX	((u16)(~0U))
> >  #define SHRT_MAX	((s16)(USHRT_MAX>>1))
> >
>
Tom Rini Dec. 6, 2019, 9:49 p.m. UTC | #3
On Wed, Nov 13, 2019 at 09:44:51AM +0900, AKASHI Takahiro wrote:

> Adding "printk.h" will help improve portability from linux kernel
> code (in my case, lib/asn1_decoder.c and others) where printf and
> pr_* variant functions are used.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Applied to u-boot/master, thanks!
Takahiro Akashi Dec. 9, 2019, 1:21 a.m. UTC | #4
On Fri, Dec 06, 2019 at 04:49:10PM -0500, Tom Rini wrote:
> On Wed, Nov 13, 2019 at 09:44:51AM +0900, AKASHI Takahiro wrote:
> 
> > Adding "printk.h" will help improve portability from linux kernel
> > code (in my case, lib/asn1_decoder.c and others) where printf and
> > pr_* variant functions are used.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> Applied to u-boot/master, thanks!

Thank you for the merge.
Heinrich pointed out a typo in the commit message (not in the code):
 %s/printf/printk/g

Please correct it if you have a chance.

Thanks,
-Takahiro Akashi

> -- 
> Tom
diff mbox series

Patch

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 5c7e5f635b1a..564819a1c0a7 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -1,8 +1,8 @@ 
 #ifndef _LINUX_KERNEL_H
 #define _LINUX_KERNEL_H
 
-
 #include <linux/types.h>
+#include <linux/printk.h> /* for printf/pr_* utilities */
 
 #define USHRT_MAX	((u16)(~0U))
 #define SHRT_MAX	((s16)(USHRT_MAX>>1))