Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1469420 > unrolled thread

[PATCH 01/10] reset: ath79: add driver Kconfig option

Back to article view | Back to linux.kernel

Contents

  [PATCH 01/10] reset: ath79: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:30 +0200
    [PATCH 10/10] reset: hi6220: allow to compile test driver on other architectures Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:30 +0200
      Re: [PATCH 10/10] reset: hi6220: allow to compile test driver on  other architectures Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-08-24 20:10 +0200
        Re: [PATCH 10/10] reset: hi6220: allow to compile test driver on  other architectures Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-25 09:30 +0200
    [PATCH 04/10] reset: meson: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:30 +0200
      Re: [PATCH 04/10] reset: meson: add driver Kconfig option Neil Armstrong <narmstrong@baylibre.com> - 2016-08-24 15:40 +0200
    [PATCH 07/10] reset: stm32: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:30 +0200
    [PATCH 09/10] reset: zynq: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:30 +0200
      Re: [PATCH 09/10] reset: zynq: add driver Kconfig option Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-08-24 20:00 +0200
        Re: [PATCH 09/10] reset: zynq: add driver Kconfig option Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-08-25 06:30 +0200
          Re: [PATCH 09/10] reset: zynq: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-25 09:30 +0200
            Re: [PATCH 09/10] reset: zynq: add driver Kconfig option Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-08-25 09:50 +0200
              Re: [PATCH 09/10] reset: zynq: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-25 10:30 +0200
    [PATCH 05/10] reset: pistachio: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:40 +0200
      RE: [PATCH 05/10] reset: pistachio: add driver Kconfig option James Hartley <James.Hartley@imgtec.com> - 2016-08-29 22:50 +0200
    [PATCH 02/10] reset: berlin: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:40 +0200
    [PATCH 06/10] reset: socfpga: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:40 +0200
      Re: [PATCH 06/10] reset: socfpga: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-25 09:30 +0200
    [PATCH 08/10] reset: sunxi: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:40 +0200
      Re: [PATCH 08/10] reset: sunxi: add driver Kconfig option Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-08-24 20:00 +0200
    [PATCH 03/10] reset: lpc18xx: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-24 15:40 +0200
      Re: [PATCH 03/10] reset: lpc18xx: add driver Kconfig option Joachim Eastwood <manabian@gmail.com> - 2016-08-24 22:50 +0200
        Re: [PATCH 03/10] reset: lpc18xx: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-25 09:30 +0200
    Re: [PATCH 01/10] reset: ath79: add driver Kconfig option Arnd Bergmann <arnd@arndb.de> - 2016-08-24 18:00 +0200
      Re: [PATCH 01/10] reset: ath79: add driver Kconfig option Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-08-24 20:30 +0200
        Re: [PATCH 01/10] reset: ath79: add driver Kconfig option Arnd Bergmann <arnd@arndb.de> - 2016-08-24 22:10 +0200
          Re: [PATCH 01/10] reset: ath79: add driver Kconfig option Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-08-25 06:40 +0200
          Re: [PATCH 01/10] reset: ath79: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-25 09:30 +0200
            Re: [PATCH 01/10] reset: ath79: add driver Kconfig option Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-08-25 09:50 +0200
              Re: [PATCH 01/10] reset: ath79: add driver Kconfig option Philipp Zabel <p.zabel@pengutronix.de> - 2016-08-25 10:00 +0200
    Re: [PATCH 01/10] reset: ath79: add driver Kconfig option Alban <albeu@free.fr> - 2016-08-25 13:20 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1469452 — [PATCH 03/10] reset: lpc18xx: add driver Kconfig option

Visible only if COMPILE_TEST is enabled, this allows to include the
driver in build tests.

Cc: Joachim Eastwood <manabian@gmail.com>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/reset/Kconfig  | 7 +++++++
 drivers/reset/Makefile | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 1194cbe..8e33de2 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -27,6 +27,13 @@ config RESET_BERLIN
 	help
 	  This enables the reset controller driver for Marvell Berlin SoCs.
 
+config RESET_LPC18XX
+	bool "LPC18xx/43xx Reset Driver" if COMPILE_TEST
+	default ARCH_LPC18XX
+	help
+	  This enables the LPC18xx/43 reset driver that supports the reset
+	  controllers on AR71xx SoCs.
+
 config RESET_OXNAS
 	bool
 
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 34c0b23..25aa05a 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,5 +1,4 @@
 obj-y += core.o
-obj-$(CONFIG_ARCH_LPC18XX) += reset-lpc18xx.o
 obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_MACH_PISTACHIO) += reset-pistachio.o
 obj-$(CONFIG_ARCH_MESON) += reset-meson.o
@@ -10,6 +9,7 @@ obj-$(CONFIG_ARCH_HISI) += hisilicon/
 obj-$(CONFIG_ARCH_ZYNQ) += reset-zynq.o
 obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
 obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
+obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
 obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
 obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
 obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]

#1469706 — Re: [PATCH 03/10] reset: lpc18xx: add driver Kconfig option

Hi Philipp,

On 24 August 2016 at 15:28, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Visible only if COMPILE_TEST is enabled, this allows to include the
> driver in build tests.
>
> Cc: Joachim Eastwood <manabian@gmail.com>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/reset/Kconfig  | 7 +++++++
>  drivers/reset/Makefile | 2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 1194cbe..8e33de2 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -27,6 +27,13 @@ config RESET_BERLIN
>         help
>           This enables the reset controller driver for Marvell Berlin SoCs.
>
> +config RESET_LPC18XX
> +       bool "LPC18xx/43xx Reset Driver" if COMPILE_TEST
> +       default ARCH_LPC18XX
> +       help
> +         This enables the LPC18xx/43 reset driver that supports the reset
> +         controllers on AR71xx SoCs.

Don't know where you got the "AR71xx SoCs" from. This reset controller
is found on NXP LPC18xx/43xx SoCs.

Other than that it looks fine to me.

Acked-by: Joachim Eastwood <manabian@gmail.com>


> +
>  config RESET_OXNAS
>         bool
>
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 34c0b23..25aa05a 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,5 +1,4 @@
>  obj-y += core.o
> -obj-$(CONFIG_ARCH_LPC18XX) += reset-lpc18xx.o
>  obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_MACH_PISTACHIO) += reset-pistachio.o
>  obj-$(CONFIG_ARCH_MESON) += reset-meson.o
> @@ -10,6 +9,7 @@ obj-$(CONFIG_ARCH_HISI) += hisilicon/
>  obj-$(CONFIG_ARCH_ZYNQ) += reset-zynq.o
>  obj-$(CONFIG_RESET_ATH79) += reset-ath79.o
>  obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
> +obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
>  obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
>  obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
>  obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> --
> 2.8.1


regards,
Joachim Eastwood

[toc] | [prev] | [next] | [standalone]

#1469910 — Re: [PATCH 03/10] reset: lpc18xx: add driver Kconfig option

Am Mittwoch, den 24.08.2016, 22:32 +0200 schrieb Joachim Eastwood:
> Hi Philipp,
> 
> On 24 August 2016 at 15:28, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Visible only if COMPILE_TEST is enabled, this allows to include the
> > driver in build tests.
> >
> > Cc: Joachim Eastwood <manabian@gmail.com>
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  drivers/reset/Kconfig  | 7 +++++++
> >  drivers/reset/Makefile | 2 +-
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > index 1194cbe..8e33de2 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -27,6 +27,13 @@ config RESET_BERLIN
> >         help
> >           This enables the reset controller driver for Marvell Berlin SoCs.
> >
> > +config RESET_LPC18XX
> > +       bool "LPC18xx/43xx Reset Driver" if COMPILE_TEST
> > +       default ARCH_LPC18XX
> > +       help
> > +         This enables the LPC18xx/43 reset driver that supports the reset
> > +         controllers on AR71xx SoCs.
> 
> Don't know where you got the "AR71xx SoCs" from. This reset controller
> is found on NXP LPC18xx/43xx SoCs.
> 
> Other than that it looks fine to me.
> 
> Acked-by: Joachim Eastwood <manabian@gmail.com>

Thanks, copy and paste error. I'll fix it.

regards
Philipp

[toc] | [prev] | [next] | [standalone]

#1469557

On Wednesday, August 24, 2016 3:28:53 PM CEST Philipp Zabel wrote:
>  if RESET_CONTROLLER
>  
> +config RESET_ATH79
> +       bool "AR71xx Reset Driver" if COMPILE_TEST
> +       default ATH79
> +       help
> +         This enables the ATH79 reset controller driver that supports the
> +         AR71xx SoC reset controller.
> +
> 

Nice series!

Just note that there is one possible problem with COMPILE_TEST
when the platforms are enabled, as you can then disable a driver
that is normally there, and that can in turn cause problems in
rare cases, e.g. when the driver has a global function that is
called from platform code. I don't know if any of the drivers
do that, but if they do, you'd have to use

config RESET_ATH79
       bool "AR71xx Reset Driver" if COMPILE_TEST && !ATH79
       default ATH79

to ensure that it's impossible to disable the driver on platforms
that require it.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1469634

Hi Arnd,


2016-08-25 0:51 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> On Wednesday, August 24, 2016 3:28:53 PM CEST Philipp Zabel wrote:
>>  if RESET_CONTROLLER
>>
>> +config RESET_ATH79
>> +       bool "AR71xx Reset Driver" if COMPILE_TEST
>> +       default ATH79
>> +       help
>> +         This enables the ATH79 reset controller driver that supports the
>> +         AR71xx SoC reset controller.
>> +
>>
>
> Nice series!
>
> Just note that there is one possible problem with COMPILE_TEST
> when the platforms are enabled, as you can then disable a driver
> that is normally there, and that can in turn cause problems in
> rare cases, e.g. when the driver has a global function that is
> called from platform code. I don't know if any of the drivers
> do that, but if they do, you'd have to use
>
> config RESET_ATH79
>        bool "AR71xx Reset Driver" if COMPILE_TEST && !ATH79
>        default ATH79
>
> to ensure that it's impossible to disable the driver on platforms
> that require it.

Hmm,
Can we do this only when we really have to do so?
I think we should not care about such a rare case that may not happen.

Let's start with only "if COMPILE_TEST",
and take a look at it if a build error is detected.

Anyway, depending on platform code is a sign of weird implementation.

It might be better to find a potential issue rather than hide it.


-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1469686

On Thursday, August 25, 2016 3:18:55 AM CEST Masahiro Yamada wrote:
> Hi Arnd,
> 
> 
> 2016-08-25 0:51 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> > On Wednesday, August 24, 2016 3:28:53 PM CEST Philipp Zabel wrote:
> >>  if RESET_CONTROLLER
> >>
> >> +config RESET_ATH79
> >> +       bool "AR71xx Reset Driver" if COMPILE_TEST
> >> +       default ATH79
> >> +       help
> >> +         This enables the ATH79 reset controller driver that supports the
> >> +         AR71xx SoC reset controller.
> >> +
> >>
> >
> > Nice series!
> >
> > Just note that there is one possible problem with COMPILE_TEST
> > when the platforms are enabled, as you can then disable a driver
> > that is normally there, and that can in turn cause problems in
> > rare cases, e.g. when the driver has a global function that is
> > called from platform code. I don't know if any of the drivers
> > do that, but if they do, you'd have to use
> >
> > config RESET_ATH79
> >        bool "AR71xx Reset Driver" if COMPILE_TEST && !ATH79
> >        default ATH79
> >
> > to ensure that it's impossible to disable the driver on platforms
> > that require it.
> 
> Hmm,
> Can we do this only when we really have to do so?
> I think we should not care about such a rare case that may not happen.
> 
> Let's start with only "if COMPILE_TEST",
> and take a look at it if a build error is detected.
> 
> Anyway, depending on platform code is a sign of weird implementation.
> 
> It might be better to find a potential issue rather than hide it.
> 
> 
> 

I just checked the object files in an allyesconfig build and found
one instance:

arch/arm/mach-sunxi/sunxi.c:extern void __init sun6i_reset_init(void);
arch/arm/mach-sunxi/sunxi.c:            sun6i_reset_init();
drivers/reset/reset-sunxi.c:void __init sun6i_reset_init(void)

We should definitely make sure this one is handled right, and maybe
check the source code for other instances.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1469811

2016-08-25 5:06 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> On Thursday, August 25, 2016 3:18:55 AM CEST Masahiro Yamada wrote:
>> Hi Arnd,
>>
>>
>> 2016-08-25 0:51 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
>> > On Wednesday, August 24, 2016 3:28:53 PM CEST Philipp Zabel wrote:
>> >>  if RESET_CONTROLLER
>> >>
>> >> +config RESET_ATH79
>> >> +       bool "AR71xx Reset Driver" if COMPILE_TEST
>> >> +       default ATH79
>> >> +       help
>> >> +         This enables the ATH79 reset controller driver that supports the
>> >> +         AR71xx SoC reset controller.
>> >> +
>> >>
>> >
>> > Nice series!
>> >
>> > Just note that there is one possible problem with COMPILE_TEST
>> > when the platforms are enabled, as you can then disable a driver
>> > that is normally there, and that can in turn cause problems in
>> > rare cases, e.g. when the driver has a global function that is
>> > called from platform code. I don't know if any of the drivers
>> > do that, but if they do, you'd have to use
>> >
>> > config RESET_ATH79
>> >        bool "AR71xx Reset Driver" if COMPILE_TEST && !ATH79
>> >        default ATH79
>> >
>> > to ensure that it's impossible to disable the driver on platforms
>> > that require it.
>>
>> Hmm,
>> Can we do this only when we really have to do so?
>> I think we should not care about such a rare case that may not happen.
>>
>> Let's start with only "if COMPILE_TEST",
>> and take a look at it if a build error is detected.
>>
>> Anyway, depending on platform code is a sign of weird implementation.
>>
>> It might be better to find a potential issue rather than hide it.
>>
>>
>>
>
> I just checked the object files in an allyesconfig build and found
> one instance:
>
> arch/arm/mach-sunxi/sunxi.c:extern void __init sun6i_reset_init(void);
> arch/arm/mach-sunxi/sunxi.c:            sun6i_reset_init();
> drivers/reset/reset-sunxi.c:void __init sun6i_reset_init(void)
>
> We should definitely make sure this one is handled right, and maybe
> check the source code for other instances.


Hmm.

Is is solved with RESET_OF_DECLARE(),
like we have CLK_OF_DECLARE() ?

Or, use something like postcore_initcall() to probe it really early?
Not sure...



-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1469896

Am Mittwoch, den 24.08.2016, 22:06 +0200 schrieb Arnd Bergmann:
> On Thursday, August 25, 2016 3:18:55 AM CEST Masahiro Yamada wrote:
> > Hi Arnd,
> > 
> > 
> > 2016-08-25 0:51 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> > > On Wednesday, August 24, 2016 3:28:53 PM CEST Philipp Zabel wrote:
> > >>  if RESET_CONTROLLER
> > >>
> > >> +config RESET_ATH79
> > >> +       bool "AR71xx Reset Driver" if COMPILE_TEST
> > >> +       default ATH79
> > >> +       help
> > >> +         This enables the ATH79 reset controller driver that supports the
> > >> +         AR71xx SoC reset controller.
> > >> +
> > >>
> > >
> > > Nice series!
> > >
> > > Just note that there is one possible problem with COMPILE_TEST
> > > when the platforms are enabled, as you can then disable a driver
> > > that is normally there, and that can in turn cause problems in
> > > rare cases, e.g. when the driver has a global function that is
> > > called from platform code. I don't know if any of the drivers
> > > do that, but if they do, you'd have to use
> > >
> > > config RESET_ATH79
> > >        bool "AR71xx Reset Driver" if COMPILE_TEST && !ATH79
> > >        default ATH79
> > >
> > > to ensure that it's impossible to disable the driver on platforms
> > > that require it.
> > 
> > Hmm,
> > Can we do this only when we really have to do so?
> > I think we should not care about such a rare case that may not happen.
> > 
> > Let's start with only "if COMPILE_TEST",
> > and take a look at it if a build error is detected.
> > 
> > Anyway, depending on platform code is a sign of weird implementation.
> > 
> > It might be better to find a potential issue rather than hide it.
> > 
> > 
> > 
> 
> I just checked the object files in an allyesconfig build and found
> one instance:
> 
> arch/arm/mach-sunxi/sunxi.c:extern void __init sun6i_reset_init(void);
> arch/arm/mach-sunxi/sunxi.c:            sun6i_reset_init();
> drivers/reset/reset-sunxi.c:void __init sun6i_reset_init(void)
> 
> We should definitely make sure this one is handled right, and maybe
> check the source code for other instances.

I'll add "&& !ARCH_SUNXI" to the RESET_SUNXI symbol

The only other drivers that have __init symbols are the STiH4xx reset
drivers, which I haven't touched yet: STIH4xx_RESET symbols are already
selected by their respective SOC_STIH4xx under ARCH_STI. And the hi6220
driver, which doesn't have a user yet.

regards
Philipp

[toc] | [prev] | [next] | [standalone]

#1469934

2016-08-25 16:22 GMT+09:00 Philipp Zabel <p.zabel@pengutronix.de>:
> And the hi6220
> driver, which doesn't have a user yet.


It does.

See arch/arm64/configs/defconfig





-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#1469940

Am Donnerstag, den 25.08.2016, 16:27 +0900 schrieb Masahiro Yamada:
> 2016-08-25 16:22 GMT+09:00 Philipp Zabel <p.zabel@pengutronix.de>:
> > And the hi6220
> > driver, which doesn't have a user yet.
> 
> It does.
> 
> See arch/arm64/configs/defconfig

My mistake, I missed that hi6220_reset_init is a postcore_initcall, so
there is no compile time dependency on the reset driver.

regards
Philipp

[toc] | [prev] | [next] | [standalone]

#1470070

[Multipart message — attachments visible in raw view] — view raw

On Wed, 24 Aug 2016 15:28:53 +0200
Philipp Zabel <p.zabel@pengutronix.de> wrote:

> Visible only if COMPILE_TEST is enabled, this allows to include the
> driver in build tests.
> 
> Cc: Alban Bedel <albeu@free.fr>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Acked-by: Aban Bedel <albeu@free.fr>

Alban

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web