linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] pinctrl: spacemit: enable config option
@ 2025-02-18  0:31 Yixun Lan
  2025-02-18  8:47 ` Javier Martinez Canillas
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Yixun Lan @ 2025-02-18  0:31 UTC (permalink / raw)
  To: Linus Walleij, Paul Walmsley, Palmer Dabbelt
  Cc: Alex Elder, linux-riscv, linux-kernel, linux-gpio, spacemit,
	Conor Dooley, Alex Elder, Yixun Lan

Pinctrl is an essential driver for SpacemiT's SoC,
The uart driver requires it, same as sd card driver,
so let's enable it by default for this SoC.

The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
'make defconfig' to select kernel configuration options.
This result in a broken uart driver where fail at probe()
stage due to no pins found.

Fixes: a83c29e1d145 ("pinctrl: spacemit: add support for SpacemiT K1 SoC")
Reported-by: Alex Elder <elder@kernel.org>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Tested-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
This should fix problem that CONFIG_PINCTRL_SPACEMIT_K1 is not enabled
when using make defconfig, thus fail to initilize uart driver which requst
pins during probe stage.
---
Changes in v3:
- switch PINCTRL_SPACEMIT_K1 from tristate to bool
- Link to v2: https://lore.kernel.org/r/20250212-k1-pinctrl-option-v2-1-bde7da0bc0d9@gentoo.org

Changes in v2:
- set default as y
- Link to v1: https://lore.kernel.org/r/20250207-k1-pinctrl-option-v1-1-e8a7e4d8404f@gentoo.org
---
 arch/riscv/Kconfig.socs               | 1 +
 drivers/pinctrl/spacemit/Kconfig      | 3 ++-
 drivers/pinctrl/spacemit/pinctrl-k1.c | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index 1916cf7ba450ec9958265de2ca41dc504d4d2f7c..17606940bb5239d0fdfc6b5aefb50eeb982d14aa 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -26,6 +26,7 @@ config ARCH_SOPHGO
 
 config ARCH_SPACEMIT
 	bool "SpacemiT SoCs"
+	select PINCTRL
 	help
 	  This enables support for SpacemiT SoC platform hardware.
 
diff --git a/drivers/pinctrl/spacemit/Kconfig b/drivers/pinctrl/spacemit/Kconfig
index 168f8a5ffbb952cbeae3e3401c11149558e0a84b..a2f98b3f8a75580d2d157008997cc48f42a89368 100644
--- a/drivers/pinctrl/spacemit/Kconfig
+++ b/drivers/pinctrl/spacemit/Kconfig
@@ -4,9 +4,10 @@
 #
 
 config PINCTRL_SPACEMIT_K1
-	tristate "SpacemiT K1 SoC Pinctrl driver"
+	bool "SpacemiT K1 SoC Pinctrl driver"
 	depends on ARCH_SPACEMIT || COMPILE_TEST
 	depends on OF
+	default y
 	select GENERIC_PINCTRL_GROUPS
 	select GENERIC_PINMUX_FUNCTIONS
 	select GENERIC_PINCONF
diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c b/drivers/pinctrl/spacemit/pinctrl-k1.c
index a32579d736130c80bd12f0f9d8b3b2f69c428b3d..59fd555ff38d4453f446263a8fdb4a61faf63cfc 100644
--- a/drivers/pinctrl/spacemit/pinctrl-k1.c
+++ b/drivers/pinctrl/spacemit/pinctrl-k1.c
@@ -1044,7 +1044,7 @@ static struct platform_driver k1_pinctrl_driver = {
 		.of_match_table		= k1_pinctrl_ids,
 	},
 };
-module_platform_driver(k1_pinctrl_driver);
+builtin_platform_driver(k1_pinctrl_driver);
 
 MODULE_AUTHOR("Yixun Lan <dlan@gentoo.org>");
 MODULE_DESCRIPTION("Pinctrl driver for the SpacemiT K1 SoC");

---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20250207-k1-pinctrl-option-de5bdfd6b42e

Best regards,
-- 
Yixun Lan


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] pinctrl: spacemit: enable config option
  2025-02-18  0:31 [PATCH v3] pinctrl: spacemit: enable config option Yixun Lan
@ 2025-02-18  8:47 ` Javier Martinez Canillas
  2025-02-23 12:06 ` Yixun Lan
  2025-03-17  8:18 ` Geert Uytterhoeven
  2 siblings, 0 replies; 10+ messages in thread
From: Javier Martinez Canillas @ 2025-02-18  8:47 UTC (permalink / raw)
  To: Yixun Lan, Linus Walleij, Paul Walmsley, Palmer Dabbelt
  Cc: Alex Elder, linux-riscv, linux-kernel, linux-gpio, spacemit,
	Conor Dooley, Alex Elder, Yixun Lan

Yixun Lan <dlan@gentoo.org> writes:

> Pinctrl is an essential driver for SpacemiT's SoC,
> The uart driver requires it, same as sd card driver,
> so let's enable it by default for this SoC.
>
> The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
> 'make defconfig' to select kernel configuration options.
> This result in a broken uart driver where fail at probe()
> stage due to no pins found.
>
> Fixes: a83c29e1d145 ("pinctrl: spacemit: add support for SpacemiT K1 SoC")
> Reported-by: Alex Elder <elder@kernel.org>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Tested-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
> This should fix problem that CONFIG_PINCTRL_SPACEMIT_K1 is not enabled
> when using make defconfig, thus fail to initilize uart driver which requst
> pins during probe stage.
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] pinctrl: spacemit: enable config option
  2025-02-18  0:31 [PATCH v3] pinctrl: spacemit: enable config option Yixun Lan
  2025-02-18  8:47 ` Javier Martinez Canillas
@ 2025-02-23 12:06 ` Yixun Lan
  2025-02-25 16:23   ` Linus Walleij
  2025-03-17  8:18 ` Geert Uytterhoeven
  2 siblings, 1 reply; 10+ messages in thread
From: Yixun Lan @ 2025-02-23 12:06 UTC (permalink / raw)
  To: Linus Walleij, Paul Walmsley, Palmer Dabbelt
  Cc: Alex Elder, linux-riscv, linux-kernel, linux-gpio, spacemit,
	Conor Dooley, Alex Elder

Hi Linus Walleij:

On 08:31 Tue 18 Feb     , Yixun Lan wrote:
> Pinctrl is an essential driver for SpacemiT's SoC,
> The uart driver requires it, same as sd card driver,
> so let's enable it by default for this SoC.
> 
> The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
> 'make defconfig' to select kernel configuration options.
> This result in a broken uart driver where fail at probe()
> stage due to no pins found.
> 

Can you take this patch via pinctrl fixes tree? if possible in this cycle

> Fixes: a83c29e1d145 ("pinctrl: spacemit: add support for SpacemiT K1 SoC")
> Reported-by: Alex Elder <elder@kernel.org>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Tested-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
> This should fix problem that CONFIG_PINCTRL_SPACEMIT_K1 is not enabled
> when using make defconfig, thus fail to initilize uart driver which requst
> pins during probe stage.
> ---
> Changes in v3:
> - switch PINCTRL_SPACEMIT_K1 from tristate to bool
> - Link to v2: https://lore.kernel.org/r/20250212-k1-pinctrl-option-v2-1-bde7da0bc0d9@gentoo.org
> 
> Changes in v2:
> - set default as y
> - Link to v1: https://lore.kernel.org/r/20250207-k1-pinctrl-option-v1-1-e8a7e4d8404f@gentoo.org
> ---
>  arch/riscv/Kconfig.socs               | 1 +
>  drivers/pinctrl/spacemit/Kconfig      | 3 ++-
>  drivers/pinctrl/spacemit/pinctrl-k1.c | 2 +-
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 1916cf7ba450ec9958265de2ca41dc504d4d2f7c..17606940bb5239d0fdfc6b5aefb50eeb982d14aa 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -26,6 +26,7 @@ config ARCH_SOPHGO
>  
>  config ARCH_SPACEMIT
>  	bool "SpacemiT SoCs"
> +	select PINCTRL
>  	help
>  	  This enables support for SpacemiT SoC platform hardware.
>  
> diff --git a/drivers/pinctrl/spacemit/Kconfig b/drivers/pinctrl/spacemit/Kconfig
> index 168f8a5ffbb952cbeae3e3401c11149558e0a84b..a2f98b3f8a75580d2d157008997cc48f42a89368 100644
> --- a/drivers/pinctrl/spacemit/Kconfig
> +++ b/drivers/pinctrl/spacemit/Kconfig
> @@ -4,9 +4,10 @@
>  #
>  
>  config PINCTRL_SPACEMIT_K1
> -	tristate "SpacemiT K1 SoC Pinctrl driver"
> +	bool "SpacemiT K1 SoC Pinctrl driver"
>  	depends on ARCH_SPACEMIT || COMPILE_TEST
>  	depends on OF
> +	default y
>  	select GENERIC_PINCTRL_GROUPS
>  	select GENERIC_PINMUX_FUNCTIONS
>  	select GENERIC_PINCONF
> diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c b/drivers/pinctrl/spacemit/pinctrl-k1.c
> index a32579d736130c80bd12f0f9d8b3b2f69c428b3d..59fd555ff38d4453f446263a8fdb4a61faf63cfc 100644
> --- a/drivers/pinctrl/spacemit/pinctrl-k1.c
> +++ b/drivers/pinctrl/spacemit/pinctrl-k1.c
> @@ -1044,7 +1044,7 @@ static struct platform_driver k1_pinctrl_driver = {
>  		.of_match_table		= k1_pinctrl_ids,
>  	},
>  };
> -module_platform_driver(k1_pinctrl_driver);
> +builtin_platform_driver(k1_pinctrl_driver);
>  
>  MODULE_AUTHOR("Yixun Lan <dlan@gentoo.org>");
>  MODULE_DESCRIPTION("Pinctrl driver for the SpacemiT K1 SoC");
> 
> ---
> base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
> change-id: 20250207-k1-pinctrl-option-de5bdfd6b42e
> 
> Best regards,
> -- 
> Yixun Lan
> 
> 

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] pinctrl: spacemit: enable config option
  2025-02-23 12:06 ` Yixun Lan
@ 2025-02-25 16:23   ` Linus Walleij
  0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2025-02-25 16:23 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Paul Walmsley, Palmer Dabbelt, Alex Elder, linux-riscv,
	linux-kernel, linux-gpio, spacemit, Conor Dooley, Alex Elder

On Sun, Feb 23, 2025 at 1:06 PM Yixun Lan <dlan@gentoo.org> wrote:

> Hi Linus Walleij:
>
> On 08:31 Tue 18 Feb     , Yixun Lan wrote:
> > Pinctrl is an essential driver for SpacemiT's SoC,
> > The uart driver requires it, same as sd card driver,
> > so let's enable it by default for this SoC.
> >
> > The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
> > 'make defconfig' to select kernel configuration options.
> > This result in a broken uart driver where fail at probe()
> > stage due to no pins found.
> >
>
> Can you take this patch via pinctrl fixes tree? if possible in this cycle

OK!

Patch applied for fixes.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] pinctrl: spacemit: enable config option
  2025-02-18  0:31 [PATCH v3] pinctrl: spacemit: enable config option Yixun Lan
  2025-02-18  8:47 ` Javier Martinez Canillas
  2025-02-23 12:06 ` Yixun Lan
@ 2025-03-17  8:18 ` Geert Uytterhoeven
  2025-03-17 12:11   ` Alex Elder
  2025-03-17 12:41   ` Yixun Lan
  2 siblings, 2 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2025-03-17  8:18 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Linus Walleij, Paul Walmsley, Palmer Dabbelt, Alex Elder,
	linux-riscv, linux-kernel, linux-gpio, spacemit, Conor Dooley,
	Alex Elder

Hi Yixun,

Thanks for your patch, which is now commit 7ff4faba63571c51
("pinctrl: spacemit: enable config option") in v6.14-rc7.

On Tue, 18 Feb 2025 at 01:32, Yixun Lan <dlan@gentoo.org> wrote:
> Pinctrl is an essential driver for SpacemiT's SoC,
> The uart driver requires it, same as sd card driver,
> so let's enable it by default for this SoC.
>
> The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
> 'make defconfig' to select kernel configuration options.
> This result in a broken uart driver where fail at probe()
> stage due to no pins found.

Perhaps this is an issue with the uart driver?
I just disabled CONFIG_PINCTRL_RZA2 on RZA2MEVB (which is one of the
few Renesas platforms where the pin control driver is not enabled by
default, for saving memory), and the system booted fine into a Debian
nfsroot.  Probe order of some devices did change, and "Trying to
probe devices needed for running init" was printed.

> Fixes: a83c29e1d145 ("pinctrl: spacemit: add support for SpacemiT K1 SoC")
> Reported-by: Alex Elder <elder@kernel.org>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Tested-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Yixun Lan <dlan@gentoo.org>

> --- a/drivers/pinctrl/spacemit/Kconfig
> +++ b/drivers/pinctrl/spacemit/Kconfig
> @@ -4,9 +4,10 @@
>  #
>
>  config PINCTRL_SPACEMIT_K1
> -       tristate "SpacemiT K1 SoC Pinctrl driver"
> +       bool "SpacemiT K1 SoC Pinctrl driver"
>         depends on ARCH_SPACEMIT || COMPILE_TEST
>         depends on OF
> +       default y

Ouch, fix sent...
"[PATCH] pinctrl: spacemit: PINCTRL_SPACEMIT_K1 should not default to
y unconditionally"
https://lore.kernel.org/6881b8d1ad74ac780af8a974e604b5ef3f5d4aad.1742198691.git.geert+renesas@glider.be

>         select GENERIC_PINCTRL_GROUPS
>         select GENERIC_PINMUX_FUNCTIONS
>         select GENERIC_PINCONF

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] pinctrl: spacemit: enable config option
  2025-03-17  8:18 ` Geert Uytterhoeven
@ 2025-03-17 12:11   ` Alex Elder
  2025-03-17 12:41   ` Yixun Lan
  1 sibling, 0 replies; 10+ messages in thread
From: Alex Elder @ 2025-03-17 12:11 UTC (permalink / raw)
  To: Geert Uytterhoeven, Yixun Lan
  Cc: Linus Walleij, Paul Walmsley, Palmer Dabbelt, Alex Elder,
	linux-riscv, linux-kernel, linux-gpio, spacemit, Conor Dooley

On 3/17/25 3:18 AM, Geert Uytterhoeven wrote:
> Hi Yixun,
> 
> Thanks for your patch, which is now commit 7ff4faba63571c51
> ("pinctrl: spacemit: enable config option") in v6.14-rc7.
> 
> On Tue, 18 Feb 2025 at 01:32, Yixun Lan <dlan@gentoo.org> wrote:
>> Pinctrl is an essential driver for SpacemiT's SoC,
>> The uart driver requires it, same as sd card driver,
>> so let's enable it by default for this SoC.
>>
>> The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
>> 'make defconfig' to select kernel configuration options.
>> This result in a broken uart driver where fail at probe()
>> stage due to no pins found.
> 
> Perhaps this is an issue with the uart driver?
> I just disabled CONFIG_PINCTRL_RZA2 on RZA2MEVB (which is one of the
> few Renesas platforms where the pin control driver is not enabled by
> default, for saving memory), and the system booted fine into a Debian
> nfsroot.  Probe order of some devices did change, and "Trying to
> probe devices needed for running init" was printed.
> 
>> Fixes: a83c29e1d145 ("pinctrl: spacemit: add support for SpacemiT K1 SoC")
>> Reported-by: Alex Elder <elder@kernel.org>
>> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>> Tested-by: Alex Elder <elder@riscstar.com>
>> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> 
>> --- a/drivers/pinctrl/spacemit/Kconfig
>> +++ b/drivers/pinctrl/spacemit/Kconfig
>> @@ -4,9 +4,10 @@
>>   #
>>
>>   config PINCTRL_SPACEMIT_K1
>> -       tristate "SpacemiT K1 SoC Pinctrl driver"
>> +       bool "SpacemiT K1 SoC Pinctrl driver"
>>          depends on ARCH_SPACEMIT || COMPILE_TEST
>>          depends on OF
>> +       default y
> 
> Ouch, fix sent...
> "[PATCH] pinctrl: spacemit: PINCTRL_SPACEMIT_K1 should not default to
> y unconditionally"
> https://lore.kernel.org/6881b8d1ad74ac780af8a974e604b5ef3f5d4aad.1742198691.git.geert+renesas@glider.be

This is interesting; what you say was what was proposed in v1
of the patch series.

I did not understand COMPILE_TEST to be used the way you say,
but I think that just means you understood it better than I do.

I think the rule is that "depends on FOO || COMPILE_TEST" alone
is fine.  But if you are going to specify a default, it should
be "default FOO" and not "default y".

Thanks for pointing this out.  I'll go respond to your patch.

					-Alex

>>          select GENERIC_PINCTRL_GROUPS
>>          select GENERIC_PINMUX_FUNCTIONS
>>          select GENERIC_PINCONF
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] pinctrl: spacemit: enable config option
  2025-03-17  8:18 ` Geert Uytterhoeven
  2025-03-17 12:11   ` Alex Elder
@ 2025-03-17 12:41   ` Yixun Lan
  2025-03-17 12:59     ` Geert Uytterhoeven
  1 sibling, 1 reply; 10+ messages in thread
From: Yixun Lan @ 2025-03-17 12:41 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linus Walleij, Paul Walmsley, Palmer Dabbelt, Alex Elder,
	linux-riscv, linux-kernel, linux-gpio, spacemit, Conor Dooley,
	Alex Elder

Hi Geert:

On 09:18 Mon 17 Mar     , Geert Uytterhoeven wrote:
> Hi Yixun,
> 
> Thanks for your patch, which is now commit 7ff4faba63571c51
> ("pinctrl: spacemit: enable config option") in v6.14-rc7.
> 
> On Tue, 18 Feb 2025 at 01:32, Yixun Lan <dlan@gentoo.org> wrote:
> > Pinctrl is an essential driver for SpacemiT's SoC,
> > The uart driver requires it, same as sd card driver,
> > so let's enable it by default for this SoC.
> >
> > The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
> > 'make defconfig' to select kernel configuration options.
> > This result in a broken uart driver where fail at probe()
> > stage due to no pins found.
> 
> Perhaps this is an issue with the uart driver?
> I just disabled CONFIG_PINCTRL_RZA2 on RZA2MEVB (which is one of the
> few Renesas platforms where the pin control driver is not enabled by
> default, for saving memory), and the system booted fine into a Debian
> nfsroot.  Probe order of some devices did change, and "Trying to
> probe devices needed for running init" was printed.
> 
my problem was CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled, result as
# CONFIG_PINCTRL_SPACEMIT_K1 is not set

for your case, is CONFIG_PINCTRL_RZA2 built as module? 
it should work for uart driver with deferred probe mechanism..

> > Fixes: a83c29e1d145 ("pinctrl: spacemit: add support for SpacemiT K1 SoC")
> > Reported-by: Alex Elder <elder@kernel.org>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > Tested-by: Alex Elder <elder@riscstar.com>
> > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> 
> > --- a/drivers/pinctrl/spacemit/Kconfig
> > +++ b/drivers/pinctrl/spacemit/Kconfig
> > @@ -4,9 +4,10 @@
> >  #
> >
> >  config PINCTRL_SPACEMIT_K1
> > -       tristate "SpacemiT K1 SoC Pinctrl driver"
> > +       bool "SpacemiT K1 SoC Pinctrl driver"
> >         depends on ARCH_SPACEMIT || COMPILE_TEST
> >         depends on OF
> > +       default y
> 
> Ouch, fix sent...
> "[PATCH] pinctrl: spacemit: PINCTRL_SPACEMIT_K1 should not default to
> y unconditionally"
> https://lore.kernel.org/6881b8d1ad74ac780af8a974e604b5ef3f5d4aad.1742198691.git.geert+renesas@glider.be
> 
I got suggestion in v1
https://lore.kernel.org/all/20250211-nature-kilt-9882e53e5a3f@spud/

so for COMPILE_TEST case, ARCH_SPACEMIT config won't be enabled? then neither PINCTRL_SPACEMIT_K1
anyway, I'm fine with either way, thanks

> >         select GENERIC_PINCTRL_GROUPS
> >         select GENERIC_PINMUX_FUNCTIONS
> >         select GENERIC_PINCONF
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] pinctrl: spacemit: enable config option
  2025-03-17 12:41   ` Yixun Lan
@ 2025-03-17 12:59     ` Geert Uytterhoeven
  2025-03-17 13:29       ` Yixun Lan
  0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2025-03-17 12:59 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Linus Walleij, Paul Walmsley, Palmer Dabbelt, Alex Elder,
	linux-riscv, linux-kernel, linux-gpio, spacemit, Conor Dooley,
	Alex Elder

Hi Yixun,

On Mon, 17 Mar 2025 at 13:41, Yixun Lan <dlan@gentoo.org> wrote:
> On 09:18 Mon 17 Mar     , Geert Uytterhoeven wrote:
> > Thanks for your patch, which is now commit 7ff4faba63571c51
> > ("pinctrl: spacemit: enable config option") in v6.14-rc7.
> >
> > On Tue, 18 Feb 2025 at 01:32, Yixun Lan <dlan@gentoo.org> wrote:
> > > Pinctrl is an essential driver for SpacemiT's SoC,
> > > The uart driver requires it, same as sd card driver,
> > > so let's enable it by default for this SoC.
> > >
> > > The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
> > > 'make defconfig' to select kernel configuration options.
> > > This result in a broken uart driver where fail at probe()
> > > stage due to no pins found.
> >
> > Perhaps this is an issue with the uart driver?
> > I just disabled CONFIG_PINCTRL_RZA2 on RZA2MEVB (which is one of the
> > few Renesas platforms where the pin control driver is not enabled by
> > default, for saving memory), and the system booted fine into a Debian
> > nfsroot.  Probe order of some devices did change, and "Trying to
> > probe devices needed for running init" was printed.
> >
> my problem was CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled, result as
> # CONFIG_PINCTRL_SPACEMIT_K1 is not set
>
> for your case, is CONFIG_PINCTRL_RZA2 built as module?
> it should work for uart driver with deferred probe mechanism..

No, CONFIG_PINCTRL_RZA2 was disabled in my testing.

> > > --- a/drivers/pinctrl/spacemit/Kconfig
> > > +++ b/drivers/pinctrl/spacemit/Kconfig
> > > @@ -4,9 +4,10 @@
> > >  #
> > >
> > >  config PINCTRL_SPACEMIT_K1
> > > -       tristate "SpacemiT K1 SoC Pinctrl driver"
> > > +       bool "SpacemiT K1 SoC Pinctrl driver"
> > >         depends on ARCH_SPACEMIT || COMPILE_TEST
> > >         depends on OF
> > > +       default y
> >
> > Ouch, fix sent...
> > "[PATCH] pinctrl: spacemit: PINCTRL_SPACEMIT_K1 should not default to
> > y unconditionally"
> > https://lore.kernel.org/6881b8d1ad74ac780af8a974e604b5ef3f5d4aad.1742198691.git.geert+renesas@glider.be
> >
> I got suggestion in v1
> https://lore.kernel.org/all/20250211-nature-kilt-9882e53e5a3f@spud/

Yeah, I read that, but only after I noticed the issue in v6.14-rc7.

> so for COMPILE_TEST case, ARCH_SPACEMIT config won't be enabled? then neither PINCTRL_SPACEMIT_K1
> anyway, I'm fine with either way, thanks
>
> > >         select GENERIC_PINCTRL_GROUPS
> > >         select GENERIC_PINMUX_FUNCTIONS
> > >         select GENERIC_PINCONF

It depends. ARCH_SPACEMIT can be enabled only when building for
RISC-V, while COMPILE_TEST can be enabled everywhere.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] pinctrl: spacemit: enable config option
  2025-03-17 12:59     ` Geert Uytterhoeven
@ 2025-03-17 13:29       ` Yixun Lan
  2025-03-17 14:18         ` Geert Uytterhoeven
  0 siblings, 1 reply; 10+ messages in thread
From: Yixun Lan @ 2025-03-17 13:29 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linus Walleij, Paul Walmsley, Palmer Dabbelt, Alex Elder,
	linux-riscv, linux-kernel, linux-gpio, spacemit, Conor Dooley,
	Alex Elder

Hi Geert:

On 13:59 Mon 17 Mar     , Geert Uytterhoeven wrote:
> Hi Yixun,
> 
> On Mon, 17 Mar 2025 at 13:41, Yixun Lan <dlan@gentoo.org> wrote:
> > On 09:18 Mon 17 Mar     , Geert Uytterhoeven wrote:
> > > Thanks for your patch, which is now commit 7ff4faba63571c51
> > > ("pinctrl: spacemit: enable config option") in v6.14-rc7.
> > >
> > > On Tue, 18 Feb 2025 at 01:32, Yixun Lan <dlan@gentoo.org> wrote:
> > > > Pinctrl is an essential driver for SpacemiT's SoC,
> > > > The uart driver requires it, same as sd card driver,
> > > > so let's enable it by default for this SoC.
> > > >
> > > > The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
> > > > 'make defconfig' to select kernel configuration options.
> > > > This result in a broken uart driver where fail at probe()
> > > > stage due to no pins found.
> > >
> > > Perhaps this is an issue with the uart driver?
> > > I just disabled CONFIG_PINCTRL_RZA2 on RZA2MEVB (which is one of the
> > > few Renesas platforms where the pin control driver is not enabled by
> > > default, for saving memory), and the system booted fine into a Debian
> > > nfsroot.  Probe order of some devices did change, and "Trying to
> > > probe devices needed for running init" was printed.
> > >
> > my problem was CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled, result as
> > # CONFIG_PINCTRL_SPACEMIT_K1 is not set
> >
> > for your case, is CONFIG_PINCTRL_RZA2 built as module?
> > it should work for uart driver with deferred probe mechanism..
> 
> No, CONFIG_PINCTRL_RZA2 was disabled in my testing.
> 
emm, this is interesting, there might be problem that uart driver
fail to have correct pin settings without pre initialization..

which uart driver is used in RZA2MEVB platform? any pinctrl dts property?
different hardware may vary..

> > > > --- a/drivers/pinctrl/spacemit/Kconfig
> > > > +++ b/drivers/pinctrl/spacemit/Kconfig
> > > > @@ -4,9 +4,10 @@
> > > >  #
> > > >
> > > >  config PINCTRL_SPACEMIT_K1
> > > > -       tristate "SpacemiT K1 SoC Pinctrl driver"
> > > > +       bool "SpacemiT K1 SoC Pinctrl driver"
> > > >         depends on ARCH_SPACEMIT || COMPILE_TEST
> > > >         depends on OF
> > > > +       default y
> > >
> > > Ouch, fix sent...
> > > "[PATCH] pinctrl: spacemit: PINCTRL_SPACEMIT_K1 should not default to
> > > y unconditionally"
> > > https://lore.kernel.org/6881b8d1ad74ac780af8a974e604b5ef3f5d4aad.1742198691.git.geert+renesas@glider.be
> > >
> > I got suggestion in v1
> > https://lore.kernel.org/all/20250211-nature-kilt-9882e53e5a3f@spud/
> 
> Yeah, I read that, but only after I noticed the issue in v6.14-rc7.
> 
> > so for COMPILE_TEST case, ARCH_SPACEMIT config won't be enabled? then neither PINCTRL_SPACEMIT_K1
> > anyway, I'm fine with either way, thanks
> >
> > > >         select GENERIC_PINCTRL_GROUPS
> > > >         select GENERIC_PINMUX_FUNCTIONS
> > > >         select GENERIC_PINCONF
> 
> It depends. ARCH_SPACEMIT can be enabled only when building for
> RISC-V, while COMPILE_TEST can be enabled everywhere.
> 
Ok, good to know
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] pinctrl: spacemit: enable config option
  2025-03-17 13:29       ` Yixun Lan
@ 2025-03-17 14:18         ` Geert Uytterhoeven
  0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2025-03-17 14:18 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Linus Walleij, Paul Walmsley, Palmer Dabbelt, Alex Elder,
	linux-riscv, linux-kernel, linux-gpio, spacemit, Conor Dooley,
	Alex Elder

Hi Yixun,

On Mon, 17 Mar 2025 at 14:30, Yixun Lan <dlan@gentoo.org> wrote:
> On 13:59 Mon 17 Mar     , Geert Uytterhoeven wrote:
> > On Mon, 17 Mar 2025 at 13:41, Yixun Lan <dlan@gentoo.org> wrote:
> > > On 09:18 Mon 17 Mar     , Geert Uytterhoeven wrote:
> > > > Thanks for your patch, which is now commit 7ff4faba63571c51
> > > > ("pinctrl: spacemit: enable config option") in v6.14-rc7.
> > > >
> > > > On Tue, 18 Feb 2025 at 01:32, Yixun Lan <dlan@gentoo.org> wrote:
> > > > > Pinctrl is an essential driver for SpacemiT's SoC,
> > > > > The uart driver requires it, same as sd card driver,
> > > > > so let's enable it by default for this SoC.
> > > > >
> > > > > The CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled when using
> > > > > 'make defconfig' to select kernel configuration options.
> > > > > This result in a broken uart driver where fail at probe()
> > > > > stage due to no pins found.
> > > >
> > > > Perhaps this is an issue with the uart driver?
> > > > I just disabled CONFIG_PINCTRL_RZA2 on RZA2MEVB (which is one of the
> > > > few Renesas platforms where the pin control driver is not enabled by
> > > > default, for saving memory), and the system booted fine into a Debian
> > > > nfsroot.  Probe order of some devices did change, and "Trying to
> > > > probe devices needed for running init" was printed.
> > > >
> > > my problem was CONFIG_PINCTRL_SPACEMIT_K1 isn't enabled, result as
> > > # CONFIG_PINCTRL_SPACEMIT_K1 is not set
> > >
> > > for your case, is CONFIG_PINCTRL_RZA2 built as module?
> > > it should work for uart driver with deferred probe mechanism..
> >
> > No, CONFIG_PINCTRL_RZA2 was disabled in my testing.
> >
> emm, this is interesting, there might be problem that uart driver
> fail to have correct pin settings without pre initialization..
>
> which uart driver is used in RZA2MEVB platform? any pinctrl dts property?
> different hardware may vary..

It indeed depends on both hardware and firmware.
RZA2MEVB uses the sh-sci driver, and its serial console is set up
by the boot loader.

Does your serial console work with "earlycon"?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-03-17 14:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-18  0:31 [PATCH v3] pinctrl: spacemit: enable config option Yixun Lan
2025-02-18  8:47 ` Javier Martinez Canillas
2025-02-23 12:06 ` Yixun Lan
2025-02-25 16:23   ` Linus Walleij
2025-03-17  8:18 ` Geert Uytterhoeven
2025-03-17 12:11   ` Alex Elder
2025-03-17 12:41   ` Yixun Lan
2025-03-17 12:59     ` Geert Uytterhoeven
2025-03-17 13:29       ` Yixun Lan
2025-03-17 14:18         ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).