| Message ID | 20170704064947.10792-1-pbrobinson@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 04, 2017 at 07:49:47AM +0100, Peter Robinson wrote: > The Intel pin control drivers are architecture specific so add an if arch > to check for X86 or compile test to ensure continued test coverage. > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > --- > drivers/pinctrl/intel/Kconfig | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pinctrl/intel/Kconfig b/drivers/pinctrl/intel/Kconfig > index 396830a41127..0c7edc321415 100644 > --- a/drivers/pinctrl/intel/Kconfig > +++ b/drivers/pinctrl/intel/Kconfig > @@ -1,6 +1,7 @@ > # > # Intel pin control drivers > # > +if (X86 || COMPILE_TEST) > > config PINCTRL_BAYTRAIL > bool "Intel Baytrail GPIO pin control" > @@ -72,3 +73,5 @@ config PINCTRL_SUNRISEPOINT > Sunrisepoint is the PCH of Intel Skylake. This pinctrl driver > provides an interface that allows configuring of PCH pins and > using them as GPIOs. > + > +endif OK by me: Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Thanks,
On Tue, 2017-07-04 at 12:54 +0300, Heikki Krogerus wrote: > On Tue, Jul 04, 2017 at 07:49:47AM +0100, Peter Robinson wrote: > > The Intel pin control drivers are architecture specific so add an if > > arch > > to check for X86 or compile test to ensure continued test coverage. > > Sorry, have not seen the original mail. > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > > --- > > drivers/pinctrl/intel/Kconfig | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/pinctrl/intel/Kconfig > > b/drivers/pinctrl/intel/Kconfig > > index 396830a41127..0c7edc321415 100644 > > --- a/drivers/pinctrl/intel/Kconfig > > +++ b/drivers/pinctrl/intel/Kconfig > > @@ -1,6 +1,7 @@ > > # > > # Intel pin control drivers > > # > > +if (X86 || COMPILE_TEST) And what about ARM et al. architectures? Instead I would propose to reorganize parent Kconfig to have something like if (ARM || COMPILE_TEST) ...ARM stuff... endif if (X86 || COMPILE_TEST) ...X86 stuff... endif But personally I don't like any of the above. So, what's the issue this patch is targeting against? > > > > config PINCTRL_BAYTRAIL > > bool "Intel Baytrail GPIO pin control" > > @@ -72,3 +73,5 @@ config PINCTRL_SUNRISEPOINT > > Sunrisepoint is the PCH of Intel Skylake. This pinctrl > > driver > > provides an interface that allows configuring of PCH pins > > and > > using them as GPIOs. > > + > > +endif > > OK by me: > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > > Thanks, >
On Tue, Jul 4, 2017 at 11:21 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Tue, 2017-07-04 at 12:54 +0300, Heikki Krogerus wrote: >> On Tue, Jul 04, 2017 at 07:49:47AM +0100, Peter Robinson wrote: >> > The Intel pin control drivers are architecture specific so add an if >> > arch >> > to check for X86 or compile test to ensure continued test coverage. >> > > > Sorry, have not seen the original mail. https://marc.info/?l=linux-gpio&m=149915099506284&w=3 >> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> >> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> >> > --- >> > drivers/pinctrl/intel/Kconfig | 3 +++ >> > 1 file changed, 3 insertions(+) >> > >> > diff --git a/drivers/pinctrl/intel/Kconfig >> > b/drivers/pinctrl/intel/Kconfig >> > index 396830a41127..0c7edc321415 100644 >> > --- a/drivers/pinctrl/intel/Kconfig >> > +++ b/drivers/pinctrl/intel/Kconfig >> > @@ -1,6 +1,7 @@ >> > # >> > # Intel pin control drivers >> > # >> > +if (X86 || COMPILE_TEST) > > And what about ARM et al. architectures? If you look in the various sub directories you'll see that the various arch sub directories have similar for their SoC eg ARCH_SUNXI or explicit depends on the SoC achieving the same outcome. > Instead I would propose to reorganize parent Kconfig to have something > like Well they're done in alphabetical and there's appropriate depends ARCH_ or SOC_ etc so that those architectures don't randomly pop up in configs for other unrelated things, the intel/ one is one of the only ones that doesn't do this (hence this patch) > if (ARM || COMPILE_TEST) > ...ARM stuff... > endif > > if (X86 || COMPILE_TEST) > ...X86 stuff... > endif > > But personally I don't like any of the above. So, what's the issue this > patch is targeting against? So that every time (in my case a distro) they don't have to explicitly have unrelated "# CONFIG_PINCTRL_CHERRYVIEW is not set" style bits through all their configs because it's completely unrelated to the platform. >> > >> > config PINCTRL_BAYTRAIL >> > bool "Intel Baytrail GPIO pin control" >> > @@ -72,3 +73,5 @@ config PINCTRL_SUNRISEPOINT >> > Sunrisepoint is the PCH of Intel Skylake. This pinctrl >> > driver >> > provides an interface that allows configuring of PCH pins >> > and >> > using them as GPIOs. >> > + >> > +endif >> >> OK by me: >> >> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> >> >> Thanks, >> > > -- > Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 4, 2017 at 8:49 AM, Peter Robinson <pbrobinson@gmail.com> wrote: > The Intel pin control drivers are architecture specific so add an if arch > to check for X86 or compile test to ensure continued test coverage. > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> I'm waiting for Mika to say what he thinks about this. He's the overall Intel pin control maintainer. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 31, 2017 at 03:41:16PM +0200, Linus Walleij wrote: > On Tue, Jul 4, 2017 at 8:49 AM, Peter Robinson <pbrobinson@gmail.com> wrote: > > > The Intel pin control drivers are architecture specific so add an if arch > > to check for X86 or compile test to ensure continued test coverage. > > > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > > I'm waiting for Mika to say what he thinks about this. He's the overall > Intel pin control maintainer. Looks fine to me, Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 4, 2017 at 8:49 AM, Peter Robinson <pbrobinson@gmail.com> wrote: > The Intel pin control drivers are architecture specific so add an if arch > to check for X86 or compile test to ensure continued test coverage. > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> Patch applied with the ACKs. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2017-07-05 at 09:01 +0100, Peter Robinson wrote: > On Tue, Jul 4, 2017 at 11:21 AM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, 2017-07-04 at 12:54 +0300, Heikki Krogerus wrote: > > > On Tue, Jul 04, 2017 at 07:49:47AM +0100, Peter Robinson wrote: > > > > --- a/drivers/pinctrl/intel/Kconfig > > > > +++ b/drivers/pinctrl/intel/Kconfig > > > > @@ -1,6 +1,7 @@ > > > > # > > > > # Intel pin control drivers > > > > # > > > > +if (X86 || COMPILE_TEST) > > > > And what about ARM et al. architectures? > > If you look in the various sub directories you'll see that the various > arch sub directories have similar for their SoC eg ARCH_SUNXI or > explicit depends on the SoC achieving the same outcome. ARM world is too fragmented. I can't take it as a good example. Most of distros would like to maintain less kernels (ideally one per architecture). In the above example it's a sub-arch. Yes, I know that x86 has 3 let's say "sub-arches" which require different settings to kernel. (None of them makes difference to pin control case though) > > Instead I would propose to reorganize parent Kconfig to have > > something > > like > > Well they're done in alphabetical and there's appropriate depends > ARCH_ or SOC_ etc so that those architectures don't randomly pop up in > configs for other unrelated things, the intel/ one is one of the only > ones that doesn't do this (hence this patch) > > > if (ARM || COMPILE_TEST) > > ...ARM stuff... > > endif > > > > if (X86 || COMPILE_TEST) > > ...X86 stuff... > > endif > > > > But personally I don't like any of the above. So, what's the issue > > this > > patch is targeting against? > > So that every time (in my case a distro) they don't have to explicitly > have unrelated "# CONFIG_PINCTRL_CHERRYVIEW is not set" style bits > through all their configs because it's completely unrelated to the > platform. Okay, so, what's wrong with defining big blocks on per ARCH basis as I pointed above? ARM, ARM64, X86, etc.
diff --git a/drivers/pinctrl/intel/Kconfig b/drivers/pinctrl/intel/Kconfig index 396830a41127..0c7edc321415 100644 --- a/drivers/pinctrl/intel/Kconfig +++ b/drivers/pinctrl/intel/Kconfig @@ -1,6 +1,7 @@ # # Intel pin control drivers # +if (X86 || COMPILE_TEST) config PINCTRL_BAYTRAIL bool "Intel Baytrail GPIO pin control" @@ -72,3 +73,5 @@ config PINCTRL_SUNRISEPOINT Sunrisepoint is the PCH of Intel Skylake. This pinctrl driver provides an interface that allows configuring of PCH pins and using them as GPIOs. + +endif
The Intel pin control drivers are architecture specific so add an if arch to check for X86 or compile test to ensure continued test coverage. Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> Signed-off-by: Peter Robinson <pbrobinson@gmail.com> --- drivers/pinctrl/intel/Kconfig | 3 +++ 1 file changed, 3 insertions(+)