diff mbox series

[v2,16/25] arch: arm: mach-at91: pm: Move prototypes to mutually included header

Message ID 20201112093918.GV2063125@dell
State Superseded
Headers show
Series None | expand

Commit Message

Lee Jones Nov. 12, 2020, 9:39 a.m. UTC
Both the caller and the supplier's source file should have access to
the include file containing the prototypes.

Fixes the following W=1 kernel build warning(s):

 drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype for ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes]
 1637 | void at91_pinctrl_gpio_suspend(void)
 | ^~~~~~~~~~~~~~~~~~~~~~~~~
 drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype for ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes]
 1661 | void at91_pinctrl_gpio_resume(void)
 | ^~~~~~~~~~~~~~~~~~~~~~~~

Cc: Russell King <linux@armlinux.org.uk>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>

---

v2: Moved shared header to: include/soc/at91/pm.h

 arch/arm/mach-at91/pm.c        | 19 ++++++++-----------
 drivers/pinctrl/pinctrl-at91.c |  2 ++
 include/soc/at91/pm.h          |  9 +++++++++
 3 files changed, 19 insertions(+), 11 deletions(-)
 create mode 100644 include/soc/at91/pm.h

-- 
2.25.1

Comments

Alexandre Belloni Nov. 12, 2020, 10:07 a.m. UTC | #1
Hi,

On 12/11/2020 09:39:18+0000, Lee Jones wrote:
> Both the caller and the supplier's source file should have access to

> the include file containing the prototypes.

> 

> Fixes the following W=1 kernel build warning(s):

> 

>  drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype for ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes]

>  1637 | void at91_pinctrl_gpio_suspend(void)

>  | ^~~~~~~~~~~~~~~~~~~~~~~~~

>  drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype for ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes]

>  1661 | void at91_pinctrl_gpio_resume(void)

>  | ^~~~~~~~~~~~~~~~~~~~~~~~

> 

> Cc: Russell King <linux@armlinux.org.uk>

> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>

> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>

> Cc: Ludovic Desroches <ludovic.desroches@microchip.com>

> Signed-off-by: Lee Jones <lee.jones@linaro.org>

> ---

> 

> v2: Moved shared header to: include/soc/at91/pm.h

> 

>  arch/arm/mach-at91/pm.c        | 19 ++++++++-----------

>  drivers/pinctrl/pinctrl-at91.c |  2 ++

>  include/soc/at91/pm.h          |  9 +++++++++

>  3 files changed, 19 insertions(+), 11 deletions(-)

>  create mode 100644 include/soc/at91/pm.h

> 

> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c

> index 120f9aa6fff32..90dcdfe3b3d0d 100644

> --- a/arch/arm/mach-at91/pm.c

> +++ b/arch/arm/mach-at91/pm.c

> @@ -17,6 +17,8 @@

>  #include <linux/clk/at91_pmc.h>

>  #include <linux/platform_data/atmel.h>

>  

> +#include <soc/at91/pm.h>

> +

>  #include <asm/cacheflush.h>

>  #include <asm/fncpy.h>

>  #include <asm/system_misc.h>

> @@ -25,17 +27,6 @@

>  #include "generic.h"

>  #include "pm.h"

>  

> -/*

> - * FIXME: this is needed to communicate between the pinctrl driver and

> - * the PM implementation in the machine. Possibly part of the PM

> - * implementation should be moved down into the pinctrl driver and get

> - * called as part of the generic suspend/resume path.

> - */

> -#ifdef CONFIG_PINCTRL_AT91

> -extern void at91_pinctrl_gpio_suspend(void);

> -extern void at91_pinctrl_gpio_resume(void);

> -#endif

> -

>  struct at91_soc_pm {

>  	int (*config_shdwc_ws)(void __iomem *shdwc, u32 *mode, u32 *polarity);

>  	int (*config_pmc_ws)(void __iomem *pmc, u32 mode, u32 polarity);

> @@ -326,6 +317,12 @@ static void at91_pm_suspend(suspend_state_t state)

>  static int at91_pm_enter(suspend_state_t state)

>  {

>  #ifdef CONFIG_PINCTRL_AT91

> +	/*

> +	 * FIXME: this is needed to communicate between the pinctrl driver and

> +	 * the PM implementation in the machine. Possibly part of the PM

> +	 * implementation should be moved down into the pinctrl driver and get

> +	 * called as part of the generic suspend/resume path.

> +	 */

>  	at91_pinctrl_gpio_suspend();

>  #endif

>  

> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c

> index 72edc675431ce..0a7e10d39505c 100644

> --- a/drivers/pinctrl/pinctrl-at91.c

> +++ b/drivers/pinctrl/pinctrl-at91.c

> @@ -23,6 +23,8 @@

>  /* Since we request GPIOs from ourself */

>  #include <linux/pinctrl/consumer.h>

>  

> +#include <soc/at91/pm.h>

> +

>  #include "pinctrl-at91.h"

>  #include "core.h"

>  

> diff --git a/include/soc/at91/pm.h b/include/soc/at91/pm.h

> new file mode 100644

> index 0000000000000..0fd5093f7f73a

> --- /dev/null

> +++ b/include/soc/at91/pm.h

> @@ -0,0 +1,9 @@

> +/* SPDX-License-Identifier: GPL-2.0-only */

> +/*

> + * atmel platform data

> + */

> +

> +#ifdef CONFIG_PINCTRL_AT91


Shouldn't that be a header guard instead of depending on PINCTRL_AT91 ?


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Lee Jones Nov. 12, 2020, 10:48 a.m. UTC | #2
On Thu, 12 Nov 2020, Alexandre Belloni wrote:

> Hi,

> 

> On 12/11/2020 09:39:18+0000, Lee Jones wrote:

> > Both the caller and the supplier's source file should have access to

> > the include file containing the prototypes.

> > 

> > Fixes the following W=1 kernel build warning(s):

> > 

> >  drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype for ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes]

> >  1637 | void at91_pinctrl_gpio_suspend(void)

> >  | ^~~~~~~~~~~~~~~~~~~~~~~~~

> >  drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype for ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes]

> >  1661 | void at91_pinctrl_gpio_resume(void)

> >  | ^~~~~~~~~~~~~~~~~~~~~~~~

> > 

> > Cc: Russell King <linux@armlinux.org.uk>

> > Cc: Nicolas Ferre <nicolas.ferre@microchip.com>

> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>

> > Cc: Ludovic Desroches <ludovic.desroches@microchip.com>

> > Signed-off-by: Lee Jones <lee.jones@linaro.org>

> > ---


[...]

> > diff --git a/include/soc/at91/pm.h b/include/soc/at91/pm.h

> > new file mode 100644

> > index 0000000000000..0fd5093f7f73a

> > --- /dev/null

> > +++ b/include/soc/at91/pm.h

> > @@ -0,0 +1,9 @@

> > +/* SPDX-License-Identifier: GPL-2.0-only */

> > +/*

> > + * atmel platform data

> > + */

> > +

> > +#ifdef CONFIG_PINCTRL_AT91

> 

> Shouldn't that be a header guard instead of depending on PINCTRL_AT91 ?


I copied the same semantics from the header it was taken from.

Happy to turn it into a proper header file too.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Alexandre Belloni Nov. 13, 2020, 9:39 a.m. UTC | #3
On 12/11/2020 10:48:10+0000, Lee Jones wrote:
> On Thu, 12 Nov 2020, Alexandre Belloni wrote:

> 

> > Hi,

> > 

> > On 12/11/2020 09:39:18+0000, Lee Jones wrote:

> > > Both the caller and the supplier's source file should have access to

> > > the include file containing the prototypes.

> > > 

> > > Fixes the following W=1 kernel build warning(s):

> > > 

> > >  drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype for ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes]

> > >  1637 | void at91_pinctrl_gpio_suspend(void)

> > >  | ^~~~~~~~~~~~~~~~~~~~~~~~~

> > >  drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype for ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes]

> > >  1661 | void at91_pinctrl_gpio_resume(void)

> > >  | ^~~~~~~~~~~~~~~~~~~~~~~~

> > > 

> > > Cc: Russell King <linux@armlinux.org.uk>

> > > Cc: Nicolas Ferre <nicolas.ferre@microchip.com>

> > > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>

> > > Cc: Ludovic Desroches <ludovic.desroches@microchip.com>

> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>

> > > ---

> 

> [...]

> 

> > > diff --git a/include/soc/at91/pm.h b/include/soc/at91/pm.h

> > > new file mode 100644

> > > index 0000000000000..0fd5093f7f73a

> > > --- /dev/null

> > > +++ b/include/soc/at91/pm.h

> > > @@ -0,0 +1,9 @@

> > > +/* SPDX-License-Identifier: GPL-2.0-only */

> > > +/*

> > > + * atmel platform data

> > > + */

> > > +

> > > +#ifdef CONFIG_PINCTRL_AT91

> > 

> > Shouldn't that be a header guard instead of depending on PINCTRL_AT91 ?

> 

> I copied the same semantics from the header it was taken from.

> 

> Happy to turn it into a proper header file too.


I guess that would be better. How do you expect this patch to be merged?

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Lee Jones Nov. 13, 2020, 10 a.m. UTC | #4
On Fri, 13 Nov 2020, Alexandre Belloni wrote:

> On 12/11/2020 10:48:10+0000, Lee Jones wrote:

> > On Thu, 12 Nov 2020, Alexandre Belloni wrote:

> > 

> > > Hi,

> > > 

> > > On 12/11/2020 09:39:18+0000, Lee Jones wrote:

> > > > Both the caller and the supplier's source file should have access to

> > > > the include file containing the prototypes.

> > > > 

> > > > Fixes the following W=1 kernel build warning(s):

> > > > 

> > > >  drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype for ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes]

> > > >  1637 | void at91_pinctrl_gpio_suspend(void)

> > > >  | ^~~~~~~~~~~~~~~~~~~~~~~~~

> > > >  drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype for ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes]

> > > >  1661 | void at91_pinctrl_gpio_resume(void)

> > > >  | ^~~~~~~~~~~~~~~~~~~~~~~~

> > > > 

> > > > Cc: Russell King <linux@armlinux.org.uk>

> > > > Cc: Nicolas Ferre <nicolas.ferre@microchip.com>

> > > > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>

> > > > Cc: Ludovic Desroches <ludovic.desroches@microchip.com>

> > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>

> > > > ---

> > 

> > [...]

> > 

> > > > diff --git a/include/soc/at91/pm.h b/include/soc/at91/pm.h

> > > > new file mode 100644

> > > > index 0000000000000..0fd5093f7f73a

> > > > --- /dev/null

> > > > +++ b/include/soc/at91/pm.h

> > > > @@ -0,0 +1,9 @@

> > > > +/* SPDX-License-Identifier: GPL-2.0-only */

> > > > +/*

> > > > + * atmel platform data

> > > > + */

> > > > +

> > > > +#ifdef CONFIG_PINCTRL_AT91

> > > 

> > > Shouldn't that be a header guard instead of depending on PINCTRL_AT91 ?

> > 

> > I copied the same semantics from the header it was taken from.

> > 

> > Happy to turn it into a proper header file too.

> 

> I guess that would be better. How do you expect this patch to be merged?


I'll fix it.

No expectations.  Whatever's easiest.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
diff mbox series

Patch

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 120f9aa6fff32..90dcdfe3b3d0d 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -17,6 +17,8 @@ 
 #include <linux/clk/at91_pmc.h>
 #include <linux/platform_data/atmel.h>
 
+#include <soc/at91/pm.h>
+
 #include <asm/cacheflush.h>
 #include <asm/fncpy.h>
 #include <asm/system_misc.h>
@@ -25,17 +27,6 @@ 
 #include "generic.h"
 #include "pm.h"
 
-/*
- * FIXME: this is needed to communicate between the pinctrl driver and
- * the PM implementation in the machine. Possibly part of the PM
- * implementation should be moved down into the pinctrl driver and get
- * called as part of the generic suspend/resume path.
- */
-#ifdef CONFIG_PINCTRL_AT91
-extern void at91_pinctrl_gpio_suspend(void);
-extern void at91_pinctrl_gpio_resume(void);
-#endif
-
 struct at91_soc_pm {
 	int (*config_shdwc_ws)(void __iomem *shdwc, u32 *mode, u32 *polarity);
 	int (*config_pmc_ws)(void __iomem *pmc, u32 mode, u32 polarity);
@@ -326,6 +317,12 @@  static void at91_pm_suspend(suspend_state_t state)
 static int at91_pm_enter(suspend_state_t state)
 {
 #ifdef CONFIG_PINCTRL_AT91
+	/*
+	 * FIXME: this is needed to communicate between the pinctrl driver and
+	 * the PM implementation in the machine. Possibly part of the PM
+	 * implementation should be moved down into the pinctrl driver and get
+	 * called as part of the generic suspend/resume path.
+	 */
 	at91_pinctrl_gpio_suspend();
 #endif
 
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 72edc675431ce..0a7e10d39505c 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -23,6 +23,8 @@ 
 /* Since we request GPIOs from ourself */
 #include <linux/pinctrl/consumer.h>
 
+#include <soc/at91/pm.h>
+
 #include "pinctrl-at91.h"
 #include "core.h"
 
diff --git a/include/soc/at91/pm.h b/include/soc/at91/pm.h
new file mode 100644
index 0000000000000..0fd5093f7f73a
--- /dev/null
+++ b/include/soc/at91/pm.h
@@ -0,0 +1,9 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * atmel platform data
+ */
+
+#ifdef CONFIG_PINCTRL_AT91
+void at91_pinctrl_gpio_suspend(void);
+void at91_pinctrl_gpio_resume(void);
+#endif