2020-05-09 10:46:26

by Hui Song

[permalink] [raw]
Subject: [PATCH v1 3/3] dm: armv8: gpio: include <asm/arch/gpio.h> for fsl-layerscape

From: "hui.song" <[email protected]>

Enable the gpio feature on fsl-layerscape platform.

Signed-off-by: hui.song <[email protected]>
---
arch/arm/include/asm/gpio.h | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
index 333e407b66..7715a01706 100644
--- a/arch/arm/include/asm/gpio.h
+++ b/arch/arm/include/asm/gpio.h
@@ -1,12 +1,8 @@
#if !defined(CONFIG_ARCH_UNIPHIER) && !defined(CONFIG_ARCH_STI) && \
!defined(CONFIG_ARCH_K3) && !defined(CONFIG_ARCH_BCM68360) && \
!defined(CONFIG_ARCH_BCM6858) && !defined(CONFIG_ARCH_BCM63158) && \
- !defined(CONFIG_ARCH_ROCKCHIP) && !defined(CONFIG_ARCH_LX2160A) && \
- !defined(CONFIG_ARCH_LS1028A) && !defined(CONFIG_ARCH_LS2080A) && \
- !defined(CONFIG_ARCH_LS1088A) && !defined(CONFIG_ARCH_ASPEED) && \
- !defined(CONFIG_ARCH_LS1012A) && !defined(CONFIG_ARCH_LS1043A) && \
- !defined(CONFIG_ARCH_LS1046A) && !defined(CONFIG_ARCH_U8500) && \
- !defined(CONFIG_CORTINA_PLATFORM)
+ !defined(CONFIG_ARCH_ROCKCHIP) && !defined(CONFIG_ARCH_ASPEED) && \
+ !defined(CONFIG_ARCH_U8500) && !defined(CONFIG_CORTINA_PLATFORM)
#include <asm/arch/gpio.h>
#endif
#include <asm-generic/gpio.h>
--
2.17.1


2020-05-09 15:35:31

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] armv8: gpio: add gpio feature

On Sat, May 09, 2020 at 06:39:55PM +0800, Hui Song wrote:
> From: "hui.song" <[email protected]>
>
> add one struct mpc8xxx_gpio_plat to enable gpio feature.
>
> Signed-off-by: hui.song <[email protected]>
> ---
> .../include/asm/arch-fsl-layerscape/gpio.h | 22 +++++++++++++++++++
> 1 file changed, 22 insertions(+)
> create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/gpio.h
>
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/gpio.h b/arch/arm/include/asm/arch-fsl-layerscape/gpio.h
> new file mode 100644
> index 0000000000..d8dd750a72
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/gpio.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + */
> +
> +/*
> + * Dummy header file to enable CONFIG_OF_CONTROL.
> + * If CONFIG_OF_CONTROL is enabled, lib/fdtdec.c is compiled.
> + * It includes <asm/arch/gpio.h> via <asm/gpio.h>, so those SoCs that enable
> + * OF_CONTROL must have arch/gpio.h.
> + */

This does not seem right. You would expect each sub arch to have a
subdirectory in arch/arm/include/asm/ when in fact none do.

Andrew

2020-05-09 17:30:16

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] armv8: gpio: add gpio feature

On Sat, May 09, 2020 at 05:33:15PM +0200, Andrew Lunn wrote:
> On Sat, May 09, 2020 at 06:39:55PM +0800, Hui Song wrote:
> > From: "hui.song" <[email protected]>
> >
> > add one struct mpc8xxx_gpio_plat to enable gpio feature.
> >
> > Signed-off-by: hui.song <[email protected]>
> > ---
> > .../include/asm/arch-fsl-layerscape/gpio.h | 22 +++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> > create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/gpio.h
> >
> > diff --git a/arch/arm/include/asm/arch-fsl-layerscape/gpio.h b/arch/arm/include/asm/arch-fsl-layerscape/gpio.h
> > new file mode 100644
> > index 0000000000..d8dd750a72
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-fsl-layerscape/gpio.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright 2014 Freescale Semiconductor, Inc.
> > + */
> > +
> > +/*
> > + * Dummy header file to enable CONFIG_OF_CONTROL.
> > + * If CONFIG_OF_CONTROL is enabled, lib/fdtdec.c is compiled.
> > + * It includes <asm/arch/gpio.h> via <asm/gpio.h>, so those SoCs that enable
> > + * OF_CONTROL must have arch/gpio.h.
> > + */
>
> This does not seem right. You would expect each sub arch to have a
> subdirectory in arch/arm/include/asm/ when in fact none do.

From what I can tell, these patches are not for the kernel. The
filenames don't match th kernel layout.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

2020-05-09 18:07:23

by Amit Tomer

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] armv8: gpio: add gpio feature

> From what I can tell, these patches are not for the kernel. The
> filenames don't match th kernel layout.

These files looks to be from U-boot, and must be intended for U-boot
as I see U-boot mailing
address in recipient's address?

2020-05-09 18:20:38

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] armv8: gpio: add gpio feature

On Sat, May 09, 2020 at 11:34:59PM +0530, Amit Tomer wrote:
> > From what I can tell, these patches are not for the kernel. The
> > filenames don't match th kernel layout.
>
> These files looks to be from U-boot, and must be intended for U-boot
> as I see U-boot mailing address in recipient's address?

So why is it copied to:

[email protected] - a kernel mailing list
[email protected] - the main kernel mailing list
[email protected] - the gpio driver kernel mailing list
[email protected] - the ARM kernel mailing list

Given that it includes four kernel mailing lists (ok, devicetree
may be argued to have a wider application), then I don't think the
conclusion that "it's for u-boot, because there's _one_ u-boot
mailing list in the recipients" is particularly obvious.

The author really needs to state that up front if they're sending
it to a wide audeience, rather than leaving people to guess, thereby
potentially wasting their time.

Not only did Andrew review the patch as if it were for the kernel,
but I also wasted time on this as well when I double-took the
ifdefs, and wanted to check the current driver in the kernel.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

2020-05-09 18:29:05

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] armv8: gpio: add gpio feature

On Sat, May 09, 2020 at 07:18:45PM +0100, Russell King - ARM Linux admin wrote:
> On Sat, May 09, 2020 at 11:34:59PM +0530, Amit Tomer wrote:
> > > From what I can tell, these patches are not for the kernel. The
> > > filenames don't match th kernel layout.
> >
> > These files looks to be from U-boot, and must be intended for U-boot
> > as I see U-boot mailing address in recipient's address?
>
> So why is it copied to:
>
> [email protected] - a kernel mailing list
> [email protected] - the main kernel mailing list
> [email protected] - the gpio driver kernel mailing list
> [email protected] - the ARM kernel mailing list
>
> Given that it includes four kernel mailing lists (ok, devicetree
> may be argued to have a wider application), then I don't think the
> conclusion that "it's for u-boot, because there's _one_ u-boot
> mailing list in the recipients" is particularly obvious.
>
> The author really needs to state that up front if they're sending
> it to a wide audeience, rather than leaving people to guess, thereby
> potentially wasting their time.
>
> Not only did Andrew review the patch as if it were for the kernel,
> but I also wasted time on this as well when I double-took the
> ifdefs, and wanted to check the current driver in the kernel.

Oh, and... [email protected] bounces because that domain is
not resolvable - I guess that is internal to NXP, and this patch
should have remained within NXP and not been posted publically.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

2020-05-09 18:37:22

by Amit Tomer

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] armv8: gpio: add gpio feature

> Oh, and... [email protected] bounces because that domain is
> not resolvable - I guess that is internal to NXP, and this patch
> should have remained within NXP and not been posted publically.

Yeah, realized it just after sending my reply. It is for internal NXP list.
Had it been for open source U-boot, Author would have chosen
"[email protected]" ?

But, I still believe Author did it unknowingly and we all should be
easy on him/her.

Thanks
-Amit