[v4,1/4] gpio: vf610: allow disabling the vf610 driver

Message ID 20240124205900.14791-2-martin@kaiser.cx
State New
Headers
Series gpio: vf610: allow disabling the vf610 driver |

Commit Message

Martin Kaiser Jan. 24, 2024, 8:58 p.m. UTC
  The vf610 gpio driver is enabled by default for all i.MX machines,
without any option to disable it in a board-specific config file.

Most i.MX chipsets have no hardware for this driver. Change the default
to enable GPIO_VF610 for SOC_VF610 and disable it otherwise.

Add a text description after the bool type, this makes the driver
selectable by make config etc.

Fixes: 30a35c07d9e9 ("gpio: vf610: drop the SOC_VF610 dependency for GPIO_VF610")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v4:
 - add a new patch to enable COMPILE_TEST

v3:
 - split the changes into three patches

v2:
 - enable the vf610 gpio driver in the defconfig files for arm_v7
   (i.MX7ULP) and arm64 (i.MX8QM, DXL, ULP and i.MX93)

 drivers/gpio/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Francesco Dolcini Jan. 26, 2024, 12:27 p.m. UTC | #1
On Wed, Jan 24, 2024 at 09:58:57PM +0100, Martin Kaiser wrote:
> The vf610 gpio driver is enabled by default for all i.MX machines,
> without any option to disable it in a board-specific config file.
> 
> Most i.MX chipsets have no hardware for this driver. Change the default
> to enable GPIO_VF610 for SOC_VF610 and disable it otherwise.
> 
> Add a text description after the bool type, this makes the driver
> selectable by make config etc.
> 
> Fixes: 30a35c07d9e9 ("gpio: vf610: drop the SOC_VF610 dependency for GPIO_VF610")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> v4:
>  - add a new patch to enable COMPILE_TEST
> 
> v3:
>  - split the changes into three patches
> 
> v2:
>  - enable the vf610 gpio driver in the defconfig files for arm_v7
>    (i.MX7ULP) and arm64 (i.MX8QM, DXL, ULP and i.MX93)
> 
>  drivers/gpio/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 1301cec94f12..353af1a4d0ac 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -711,7 +711,8 @@ config GPIO_UNIPHIER
>  	  Say yes here to support UniPhier GPIOs.
>  
>  config GPIO_VF610
> -	def_bool y
> +	bool "VF610 GPIO support"
> +	default y if SOC_VF610

any reason for having this default y for SOC_VF610, but not for the
other SOC that uses the same variant (i.MX7ULP, ... ?).

Francesco
  
Martin Kaiser Jan. 29, 2024, 9:26 p.m. UTC | #2
Thus wrote Francesco Dolcini (francesco@dolcini.it):

> On Wed, Jan 24, 2024 at 09:58:57PM +0100, Martin Kaiser wrote:
> > The vf610 gpio driver is enabled by default for all i.MX machines,
> > without any option to disable it in a board-specific config file.

> > Most i.MX chipsets have no hardware for this driver. Change the default
> > to enable GPIO_VF610 for SOC_VF610 and disable it otherwise.

> > Add a text description after the bool type, this makes the driver
> > selectable by make config etc.

> > Fixes: 30a35c07d9e9 ("gpio: vf610: drop the SOC_VF610 dependency for GPIO_VF610")
> > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> > ---
> > v4:
> >  - add a new patch to enable COMPILE_TEST

> > v3:
> >  - split the changes into three patches

> > v2:
> >  - enable the vf610 gpio driver in the defconfig files for arm_v7
> >    (i.MX7ULP) and arm64 (i.MX8QM, DXL, ULP and i.MX93)

> >  drivers/gpio/Kconfig | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)

> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index 1301cec94f12..353af1a4d0ac 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -711,7 +711,8 @@ config GPIO_UNIPHIER
> >  	  Say yes here to support UniPhier GPIOs.

> >  config GPIO_VF610
> > -	def_bool y
> > +	bool "VF610 GPIO support"
> > +	default y if SOC_VF610

> any reason for having this default y for SOC_VF610, but not for the
> other SOC that uses the same variant (i.MX7ULP, ... ?).

Ok, it's probably not as consistent as it could be.

It seems that there are three categories

* Vybrid SoCs

According to the reference manual, they all have the gpio-vf610 hardware.
Defaulting to y for SOC_VF610 makes sense. It's now possible to disable the
driver if a board doesn't need it.

There's a bunch of defconfigs, not sure which ones would have to enable
gpio-vf610 if it weren't on by default.

* imx7ulp

The devicetrees show that all imx7ulp have gpio-vf610 hardware. You're right,
we should use the same approach, i.e.
   default y if SOC_IMX7ULP
and get rid of the imx_v6_v7_defconfig change.

* imx8, imx9

For arm64, there are no SOC_... defines and there's only one defconfig. The
devicetrees don't show clearly which chip has gpio-vf610. We're on the safe
side if we enable gpio-vf610 in defconfig.

Does this make sense?

   Martin
  
Francesco Dolcini Jan. 30, 2024, 6:45 a.m. UTC | #3
On Mon, Jan 29, 2024 at 10:26:16PM +0100, Martin Kaiser wrote:
> Thus wrote Francesco Dolcini (francesco@dolcini.it):
> 
> > On Wed, Jan 24, 2024 at 09:58:57PM +0100, Martin Kaiser wrote:
> > > The vf610 gpio driver is enabled by default for all i.MX machines,
> > > without any option to disable it in a board-specific config file.
> 
> > > Most i.MX chipsets have no hardware for this driver. Change the default
> > > to enable GPIO_VF610 for SOC_VF610 and disable it otherwise.
> 
> > > Add a text description after the bool type, this makes the driver
> > > selectable by make config etc.
> 
> > > Fixes: 30a35c07d9e9 ("gpio: vf610: drop the SOC_VF610 dependency for GPIO_VF610")
> > > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> > > ---
> 
> > >  config GPIO_VF610
> > > -	def_bool y
> > > +	bool "VF610 GPIO support"
> > > +	default y if SOC_VF610
> 
> > any reason for having this default y for SOC_VF610, but not for the
> > other SOC that uses the same variant (i.MX7ULP, ... ?).
> 
> Ok, it's probably not as consistent as it could be.

..

> Does this make sense?
sounds fair to me.

Francesco

> 
>    Martin
  
Bartosz Golaszewski Feb. 2, 2024, 1:32 p.m. UTC | #4
On Wed, Jan 24, 2024 at 9:59 PM Martin Kaiser <martin@kaiser.cx> wrote:
>
> The vf610 gpio driver is enabled by default for all i.MX machines,
> without any option to disable it in a board-specific config file.
>
> Most i.MX chipsets have no hardware for this driver. Change the default
> to enable GPIO_VF610 for SOC_VF610 and disable it otherwise.
>
> Add a text description after the bool type, this makes the driver
> selectable by make config etc.
>
> Fixes: 30a35c07d9e9 ("gpio: vf610: drop the SOC_VF610 dependency for GPIO_VF610")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---

Applied, thanks!

Bart
  
Martin Kaiser Feb. 6, 2024, 2:05 p.m. UTC | #5
Thus wrote Francesco Dolcini (francesco@dolcini.it):

> On Mon, Jan 29, 2024 at 10:26:16PM +0100, Martin Kaiser wrote:
> > Thus wrote Francesco Dolcini (francesco@dolcini.it):

> > > On Wed, Jan 24, 2024 at 09:58:57PM +0100, Martin Kaiser wrote:
> > > > The vf610 gpio driver is enabled by default for all i.MX machines,
> > > > without any option to disable it in a board-specific config file.

> > > > Most i.MX chipsets have no hardware for this driver. Change the default
> > > > to enable GPIO_VF610 for SOC_VF610 and disable it otherwise.

> > > > Add a text description after the bool type, this makes the driver
> > > > selectable by make config etc.

> > > > Fixes: 30a35c07d9e9 ("gpio: vf610: drop the SOC_VF610 dependency for GPIO_VF610")
> > > > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> > > > ---

> > > >  config GPIO_VF610
> > > > -	def_bool y
> > > > +	bool "VF610 GPIO support"
> > > > +	default y if SOC_VF610

> > > any reason for having this default y for SOC_VF610, but not for the
> > > other SOC that uses the same variant (i.MX7ULP, ... ?).

> > Ok, it's probably not as consistent as it could be.

> ...

> > Does this make sense?
> sounds fair to me.

Sorry for the delay, other tasks got in the way.

The maintainers have meanwhile merged v4.

Basically, the conclusion of my last mail was that imx7ulp and soc_vf610 could
be configured in the same way to make things a bit clearer. This wouldn't be
much of an improvement compared to the v4 set, I'd suggest keeping the current
state.

Thanks,
Martin
  

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1301cec94f12..353af1a4d0ac 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -711,7 +711,8 @@  config GPIO_UNIPHIER
 	  Say yes here to support UniPhier GPIOs.
 
 config GPIO_VF610
-	def_bool y
+	bool "VF610 GPIO support"
+	default y if SOC_VF610
 	depends on ARCH_MXC
 	select GPIOLIB_IRQCHIP
 	help