[PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO

Raag Jadav posted 5 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Raag Jadav 11 months, 1 week ago
Now that we have Intel MFD driver for PSE GPIO, depend on it.

Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
 drivers/gpio/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 98b4d1633b25..b2efd2533630 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1372,7 +1372,7 @@ config HTC_EGPIO
 
 config GPIO_ELKHARTLAKE
 	tristate "Intel Elkhart Lake PSE GPIO support"
-	depends on X86 || COMPILE_TEST
+	depends on (X86 && MFD_INTEL_EHL_PSE_GPIO) || COMPILE_TEST
 	select GPIO_TANGIER
 	help
 	  Select this option to enable GPIO support for Intel Elkhart Lake
-- 
2.34.1
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Andy Shevchenko 11 months, 1 week ago
On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:
> Now that we have Intel MFD driver for PSE GPIO, depend on it.

...

>  config GPIO_ELKHARTLAKE
>  	tristate "Intel Elkhart Lake PSE GPIO support"
> -	depends on X86 || COMPILE_TEST
> +	depends on (X86 && MFD_INTEL_EHL_PSE_GPIO) || COMPILE_TEST
>  	select GPIO_TANGIER

Looking on how GPIO PMIC drivers are written, I would redo this as

	depends on (X86 || COMPILE_TEST) && MFD_INTEL_EHL_PSE_GPIO

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Raag Jadav 11 months, 1 week ago
On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:
> > Now that we have Intel MFD driver for PSE GPIO, depend on it.
> 
> ...
> 
> >  config GPIO_ELKHARTLAKE
> >  	tristate "Intel Elkhart Lake PSE GPIO support"
> > -	depends on X86 || COMPILE_TEST
> > +	depends on (X86 && MFD_INTEL_EHL_PSE_GPIO) || COMPILE_TEST
> >  	select GPIO_TANGIER
> 
> Looking on how GPIO PMIC drivers are written, I would redo this as
> 
> 	depends on (X86 || COMPILE_TEST) && MFD_INTEL_EHL_PSE_GPIO

True, but perhaps allow independent COMPILE_TEST where possible?

Raag
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Andy Shevchenko 11 months, 1 week ago
On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:

...

> > >  config GPIO_ELKHARTLAKE
> > >  	tristate "Intel Elkhart Lake PSE GPIO support"
> > > -	depends on X86 || COMPILE_TEST
> > > +	depends on (X86 && MFD_INTEL_EHL_PSE_GPIO) || COMPILE_TEST
> > >  	select GPIO_TANGIER
> > 
> > Looking on how GPIO PMIC drivers are written, I would redo this as
> > 
> > 	depends on (X86 || COMPILE_TEST) && MFD_INTEL_EHL_PSE_GPIO
> 
> True, but perhaps allow independent COMPILE_TEST where possible?

It will be tested in all-or-none way. Or you think it has to be tested
individually? If so, why is it needed?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Raag Jadav 11 months, 1 week ago
On Mon, Mar 03, 2025 at 01:44:52PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> > On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:
> 
> ...
> 
> > > >  config GPIO_ELKHARTLAKE
> > > >  	tristate "Intel Elkhart Lake PSE GPIO support"
> > > > -	depends on X86 || COMPILE_TEST
> > > > +	depends on (X86 && MFD_INTEL_EHL_PSE_GPIO) || COMPILE_TEST
> > > >  	select GPIO_TANGIER
> > > 
> > > Looking on how GPIO PMIC drivers are written, I would redo this as
> > > 
> > > 	depends on (X86 || COMPILE_TEST) && MFD_INTEL_EHL_PSE_GPIO
> > 
> > True, but perhaps allow independent COMPILE_TEST where possible?
> 
> It will be tested in all-or-none way. Or you think it has to be tested
> individually? If so, why is it needed?

Better CI coverage? We also have it for Intel pinctrl drivers.

Raag
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Andy Shevchenko 11 months, 1 week ago
On Mon, Mar 03, 2025 at 02:13:35PM +0200, Raag Jadav wrote:
> On Mon, Mar 03, 2025 at 01:44:52PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> > > On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > > > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:

...

> > > > >  config GPIO_ELKHARTLAKE
> > > > >  	tristate "Intel Elkhart Lake PSE GPIO support"
> > > > > -	depends on X86 || COMPILE_TEST
> > > > > +	depends on (X86 && MFD_INTEL_EHL_PSE_GPIO) || COMPILE_TEST
> > > > >  	select GPIO_TANGIER
> > > > 
> > > > Looking on how GPIO PMIC drivers are written, I would redo this as
> > > > 
> > > > 	depends on (X86 || COMPILE_TEST) && MFD_INTEL_EHL_PSE_GPIO
> > > 
> > > True, but perhaps allow independent COMPILE_TEST where possible?
> > 
> > It will be tested in all-or-none way. Or you think it has to be tested
> > individually? If so, why is it needed?
> 
> Better CI coverage?

How? I do not see the difference, can you elaborate?
(Assuming that CIs are using the merge_config.sh approach or alike)

> We also have it for Intel pinctrl drivers.

This is unrelated to Intel pin control drivers.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Raag Jadav 11 months, 1 week ago
On Mon, Mar 03, 2025 at 02:21:55PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 03, 2025 at 02:13:35PM +0200, Raag Jadav wrote:
> > On Mon, Mar 03, 2025 at 01:44:52PM +0200, Andy Shevchenko wrote:
> > > On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> > > > On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > > > > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:
> 
> ...
> 
> > > > > >  config GPIO_ELKHARTLAKE
> > > > > >  	tristate "Intel Elkhart Lake PSE GPIO support"
> > > > > > -	depends on X86 || COMPILE_TEST
> > > > > > +	depends on (X86 && MFD_INTEL_EHL_PSE_GPIO) || COMPILE_TEST
> > > > > >  	select GPIO_TANGIER
> > > > > 
> > > > > Looking on how GPIO PMIC drivers are written, I would redo this as
> > > > > 
> > > > > 	depends on (X86 || COMPILE_TEST) && MFD_INTEL_EHL_PSE_GPIO
> > > > 
> > > > True, but perhaps allow independent COMPILE_TEST where possible?
> > > 
> > > It will be tested in all-or-none way. Or you think it has to be tested
> > > individually? If so, why is it needed?
> > 
> > Better CI coverage?
> 
> How? I do not see the difference, can you elaborate?
> (Assuming that CIs are using the merge_config.sh approach or alike)

That is my understanding of it.

config COMPILE_TEST
        bool "Compile also drivers which will not load"
        depends on HAS_IOMEM
        help
          Some drivers can be compiled on a different platform than they are
          intended to be run on. Despite they cannot be loaded there (or even
          when they load they cannot be used due to missing HW support),
          developers still, opposing to distributors, might want to build such
          drivers to compile-test them.

Raag
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Andy Shevchenko 11 months, 1 week ago
On Mon, Mar 03, 2025 at 02:46:24PM +0200, Raag Jadav wrote:
> On Mon, Mar 03, 2025 at 02:21:55PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 03, 2025 at 02:13:35PM +0200, Raag Jadav wrote:
> > > On Mon, Mar 03, 2025 at 01:44:52PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> > > > > On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > > > > > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:

...

> > > > > > >  config GPIO_ELKHARTLAKE
> > > > > > >  	tristate "Intel Elkhart Lake PSE GPIO support"
> > > > > > > -	depends on X86 || COMPILE_TEST
> > > > > > > +	depends on (X86 && MFD_INTEL_EHL_PSE_GPIO) || COMPILE_TEST
> > > > > > >  	select GPIO_TANGIER
> > > > > > 
> > > > > > Looking on how GPIO PMIC drivers are written, I would redo this as
> > > > > > 
> > > > > > 	depends on (X86 || COMPILE_TEST) && MFD_INTEL_EHL_PSE_GPIO
> > > > > 
> > > > > True, but perhaps allow independent COMPILE_TEST where possible?
> > > > 
> > > > It will be tested in all-or-none way. Or you think it has to be tested
> > > > individually? If so, why is it needed?
> > > 
> > > Better CI coverage?
> > 
> > How? I do not see the difference, can you elaborate?
> > (Assuming that CIs are using the merge_config.sh approach or alike)
> 
> That is my understanding of it.
> 
> config COMPILE_TEST
>         bool "Compile also drivers which will not load"
>         depends on HAS_IOMEM
>         help
>           Some drivers can be compiled on a different platform than they are
>           intended to be run on. Despite they cannot be loaded there (or even
>           when they load they cannot be used due to missing HW support),
>           developers still, opposing to distributors, might want to build such
>           drivers to compile-test them.

Yes, and how does my suggestion prevent from this happening?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Raag Jadav 11 months, 1 week ago
On Mon, Mar 03, 2025 at 03:19:57PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 03, 2025 at 02:46:24PM +0200, Raag Jadav wrote:
> > On Mon, Mar 03, 2025 at 02:21:55PM +0200, Andy Shevchenko wrote:
> > > On Mon, Mar 03, 2025 at 02:13:35PM +0200, Raag Jadav wrote:
> > > > On Mon, Mar 03, 2025 at 01:44:52PM +0200, Andy Shevchenko wrote:
> > > > > On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> > > > > > On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > > > > > > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:

...

> > > > Better CI coverage?
> > > 
> > > How? I do not see the difference, can you elaborate?
> > > (Assuming that CIs are using the merge_config.sh approach or alike)
> > 
> > That is my understanding of it.
> > 
> > config COMPILE_TEST
> >         bool "Compile also drivers which will not load"
> >         depends on HAS_IOMEM
> >         help
> >           Some drivers can be compiled on a different platform than they are
> >           intended to be run on. Despite they cannot be loaded there (or even
> >           when they load they cannot be used due to missing HW support),
> >           developers still, opposing to distributors, might want to build such
> >           drivers to compile-test them.
> 
> Yes, and how does my suggestion prevent from this happening?

Nothing's preventing it, but since we have an opportunity to allow
a wider build test (even without arch or mfd dependency), shouldn't
we allow it?

Raag
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Andy Shevchenko 11 months, 1 week ago
On Mon, Mar 03, 2025 at 04:01:43PM +0200, Raag Jadav wrote:
> On Mon, Mar 03, 2025 at 03:19:57PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 03, 2025 at 02:46:24PM +0200, Raag Jadav wrote:
> > > On Mon, Mar 03, 2025 at 02:21:55PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Mar 03, 2025 at 02:13:35PM +0200, Raag Jadav wrote:
> > > > > On Mon, Mar 03, 2025 at 01:44:52PM +0200, Andy Shevchenko wrote:
> > > > > > On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> > > > > > > On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > > > > > > > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:

...

> > > > > Better CI coverage?
> > > > 
> > > > How? I do not see the difference, can you elaborate?
> > > > (Assuming that CIs are using the merge_config.sh approach or alike)
> > > 
> > > That is my understanding of it.
> > > 
> > > config COMPILE_TEST
> > >         bool "Compile also drivers which will not load"
> > >         depends on HAS_IOMEM
> > >         help
> > >           Some drivers can be compiled on a different platform than they are
> > >           intended to be run on. Despite they cannot be loaded there (or even
> > >           when they load they cannot be used due to missing HW support),
> > >           developers still, opposing to distributors, might want to build such
> > >           drivers to compile-test them.
> > 
> > Yes, and how does my suggestion prevent from this happening?
> 
> Nothing's preventing it, but since we have an opportunity to allow
> a wider build test (even without arch or mfd dependency), shouldn't
> we allow it?

We are going circles here. My point that there is a little sense to do that
without MFD as it's impractical. On top of that this is inconsistent to other
drivers with similar design.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Andy Shevchenko 11 months, 1 week ago
On Mon, Mar 03, 2025 at 04:01:43PM +0200, Raag Jadav wrote:
> On Mon, Mar 03, 2025 at 03:19:57PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 03, 2025 at 02:46:24PM +0200, Raag Jadav wrote:
> > > On Mon, Mar 03, 2025 at 02:21:55PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Mar 03, 2025 at 02:13:35PM +0200, Raag Jadav wrote:
> > > > > On Mon, Mar 03, 2025 at 01:44:52PM +0200, Andy Shevchenko wrote:
> > > > > > On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> > > > > > > On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > > > > > > > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:

...

> > > > > Better CI coverage?
> > > > 
> > > > How? I do not see the difference, can you elaborate?
> > > > (Assuming that CIs are using the merge_config.sh approach or alike)
> > > 
> > > That is my understanding of it.
> > > 
> > > config COMPILE_TEST
> > >         bool "Compile also drivers which will not load"
> > >         depends on HAS_IOMEM
> > >         help
> > >           Some drivers can be compiled on a different platform than they are
> > >           intended to be run on. Despite they cannot be loaded there (or even
> > >           when they load they cannot be used due to missing HW support),
> > >           developers still, opposing to distributors, might want to build such
> > >           drivers to compile-test them.
> > 
> > Yes, and how does my suggestion prevent from this happening?
> 
> Nothing's preventing it, but since we have an opportunity to allow
> a wider build test (even without arch or mfd dependency), shouldn't
> we allow it?

I don't see much benefit out of this. If MFD is not available, the other
drivers may be built, but it won't make any practical sense except build for
the sake of build. I think when they are all together, it makes real sense
to compile test. MFD driver here is like a subsubsystem dependecy, we don't
usually compile the drivers without subsystem being enabled.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Raag Jadav 11 months, 1 week ago
On Mon, Mar 03, 2025 at 04:20:08PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 03, 2025 at 04:01:43PM +0200, Raag Jadav wrote:
> > On Mon, Mar 03, 2025 at 03:19:57PM +0200, Andy Shevchenko wrote:
> > > On Mon, Mar 03, 2025 at 02:46:24PM +0200, Raag Jadav wrote:
> > > > On Mon, Mar 03, 2025 at 02:21:55PM +0200, Andy Shevchenko wrote:
> > > > > On Mon, Mar 03, 2025 at 02:13:35PM +0200, Raag Jadav wrote:
> > > > > > On Mon, Mar 03, 2025 at 01:44:52PM +0200, Andy Shevchenko wrote:
> > > > > > > On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> > > > > > > > On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > > > > > > > > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:
> 
> ...
> 
> > > > > > Better CI coverage?
> > > > > 
> > > > > How? I do not see the difference, can you elaborate?
> > > > > (Assuming that CIs are using the merge_config.sh approach or alike)
> > > > 
> > > > That is my understanding of it.
> > > > 
> > > > config COMPILE_TEST
> > > >         bool "Compile also drivers which will not load"
> > > >         depends on HAS_IOMEM
> > > >         help
> > > >           Some drivers can be compiled on a different platform than they are
> > > >           intended to be run on. Despite they cannot be loaded there (or even
> > > >           when they load they cannot be used due to missing HW support),
> > > >           developers still, opposing to distributors, might want to build such
> > > >           drivers to compile-test them.
> > > 
> > > Yes, and how does my suggestion prevent from this happening?
> > 
> > Nothing's preventing it, but since we have an opportunity to allow
> > a wider build test (even without arch or mfd dependency), shouldn't
> > we allow it?
> 
> I don't see much benefit out of this. If MFD is not available, the other
> drivers may be built, but it won't make any practical sense except build for
> the sake of build. I think when they are all together, it makes real sense
> to compile test. MFD driver here is like a subsubsystem dependecy, we don't
> usually compile the drivers without subsystem being enabled.

I thought the point of COMPILE_TEST is to do exactly that, but sure if
you insist.

Raag
Re: [PATCH v2 2/5] gpio: elkhartlake: depend on MFD_INTEL_EHL_PSE_GPIO
Posted by Andy Shevchenko 11 months, 1 week ago
On Tue, Mar 04, 2025 at 07:22:23AM +0200, Raag Jadav wrote:
> On Mon, Mar 03, 2025 at 04:20:08PM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 03, 2025 at 04:01:43PM +0200, Raag Jadav wrote:
> > > On Mon, Mar 03, 2025 at 03:19:57PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Mar 03, 2025 at 02:46:24PM +0200, Raag Jadav wrote:
> > > > > On Mon, Mar 03, 2025 at 02:21:55PM +0200, Andy Shevchenko wrote:
> > > > > > On Mon, Mar 03, 2025 at 02:13:35PM +0200, Raag Jadav wrote:
> > > > > > > On Mon, Mar 03, 2025 at 01:44:52PM +0200, Andy Shevchenko wrote:
> > > > > > > > On Mon, Mar 03, 2025 at 01:38:15PM +0200, Raag Jadav wrote:
> > > > > > > > > On Mon, Mar 03, 2025 at 10:21:13AM +0200, Andy Shevchenko wrote:
> > > > > > > > > > On Mon, Mar 03, 2025 at 10:17:42AM +0530, Raag Jadav wrote:

...

> > > > > > > Better CI coverage?
> > > > > > 
> > > > > > How? I do not see the difference, can you elaborate?
> > > > > > (Assuming that CIs are using the merge_config.sh approach or alike)
> > > > > 
> > > > > That is my understanding of it.
> > > > > 
> > > > > config COMPILE_TEST
> > > > >         bool "Compile also drivers which will not load"
> > > > >         depends on HAS_IOMEM
> > > > >         help
> > > > >           Some drivers can be compiled on a different platform than they are
> > > > >           intended to be run on. Despite they cannot be loaded there (or even
> > > > >           when they load they cannot be used due to missing HW support),
> > > > >           developers still, opposing to distributors, might want to build such
> > > > >           drivers to compile-test them.
> > > > 
> > > > Yes, and how does my suggestion prevent from this happening?
> > > 
> > > Nothing's preventing it, but since we have an opportunity to allow
> > > a wider build test (even without arch or mfd dependency), shouldn't
> > > we allow it?
> > 
> > I don't see much benefit out of this. If MFD is not available, the other
> > drivers may be built, but it won't make any practical sense except build for
> > the sake of build. I think when they are all together, it makes real sense
> > to compile test. MFD driver here is like a subsubsystem dependecy, we don't
> > usually compile the drivers without subsystem being enabled.
> 
> I thought the point of COMPILE_TEST is to do exactly that, but sure if
> you insist.

I think it's okay to compile test the whole batch of the related to each other
drivers, they are not many and again, the configuration that MFD is absent and
leaf drivers are built sounds to me impractical, so not really used in the real
life. If the leaf driver makes sense separately on some future HW or HW designs,
then we might reconsider the dependencies.

-- 
With Best Regards,
Andy Shevchenko