| Message ID | 20230130144324.206208-17-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Kconfig: More cleanup of CONFIG options | expand |
> From: Simon Glass <sjg@chromium.org> > Date: Mon, 30 Jan 2023 07:40:49 -0700 > > This is implicitly used in the source and seems useful, so add it. Not sure how this ended up with an "iommu" tag, but that seems wrong. > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > drivers/pinctrl/mvebu/Kconfig | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig > index 7c51d138c8b..0b2be88e3a1 100644 > --- a/drivers/pinctrl/mvebu/Kconfig > +++ b/drivers/pinctrl/mvebu/Kconfig > @@ -7,6 +7,10 @@ config PINCTRL_ARMADA_38X > Support pin multiplexing and pin configuration control on > Marvell's Armada-38x SoC. > > +config SPL_PINCTRL_ARMADA_38X > + def_bool n # Armada 38x pin control driver (SPL) > + depends on SPL && ARMADA_38X && SPL_PINCTRL_FULL > + > config PINCTRL_ARMADA_37XX > depends on ARMADA_3700 && PINCTRL_FULL > bool "Armada 37xx pin control driver" > -- > 2.39.1.456.gfc5497dd1b-goog > >
On Mon, Jan 30, 2023 at 05:19:01PM +0100, Mark Kettenis wrote: > > From: Simon Glass <sjg@chromium.org> > > Date: Mon, 30 Jan 2023 07:40:49 -0700 > > > > This is implicitly used in the source and seems useful, so add it. > > Not sure how this ended up with an "iommu" tag, but that seems wrong. > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > drivers/pinctrl/mvebu/Kconfig | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig > > index 7c51d138c8b..0b2be88e3a1 100644 > > --- a/drivers/pinctrl/mvebu/Kconfig > > +++ b/drivers/pinctrl/mvebu/Kconfig > > @@ -7,6 +7,10 @@ config PINCTRL_ARMADA_38X > > Support pin multiplexing and pin configuration control on > > Marvell's Armada-38x SoC. > > > > +config SPL_PINCTRL_ARMADA_38X > > + def_bool n # Armada 38x pin control driver (SPL) > > + depends on SPL && ARMADA_38X && SPL_PINCTRL_FULL > > + > > config PINCTRL_ARMADA_37XX > > depends on ARMADA_3700 && PINCTRL_FULL > > bool "Armada 37xx pin control driver" Looking at the code in question, it's because in drivers/gpio/mvebu_gpio.c we conditionally not set request / rfree dm_gpio_ops because turris_omnia (only platform in question) does not set pinctrl in SPL, but does set SPL_DM_GPIO. So the question I have, is all of that intentional and used today, in SPL, on the platform? I guess some set/get directions / values, to check board revs or something?
On Monday 30 January 2023 11:29:27 Tom Rini wrote: > On Mon, Jan 30, 2023 at 05:19:01PM +0100, Mark Kettenis wrote: > > > From: Simon Glass <sjg@chromium.org> > > > Date: Mon, 30 Jan 2023 07:40:49 -0700 > > > > > > This is implicitly used in the source and seems useful, so add it. > > > > Not sure how this ended up with an "iommu" tag, but that seems wrong. > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > --- > > > > > > drivers/pinctrl/mvebu/Kconfig | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig > > > index 7c51d138c8b..0b2be88e3a1 100644 > > > --- a/drivers/pinctrl/mvebu/Kconfig > > > +++ b/drivers/pinctrl/mvebu/Kconfig > > > @@ -7,6 +7,10 @@ config PINCTRL_ARMADA_38X > > > Support pin multiplexing and pin configuration control on > > > Marvell's Armada-38x SoC. > > > > > > +config SPL_PINCTRL_ARMADA_38X > > > + def_bool n # Armada 38x pin control driver (SPL) > > > + depends on SPL && ARMADA_38X && SPL_PINCTRL_FULL > > > + > > > config PINCTRL_ARMADA_37XX > > > depends on ARMADA_3700 && PINCTRL_FULL > > > bool "Armada 37xx pin control driver" > > Looking at the code in question, it's because in > drivers/gpio/mvebu_gpio.c we conditionally not set request / rfree > dm_gpio_ops because turris_omnia (only platform in question) does not > set pinctrl in SPL, but does set SPL_DM_GPIO. So the question I have, is > all of that intentional and used today, in SPL, on the platform? I guess > some set/get directions / values, to check board revs or something? > > -- > Tom I think this is because no A38x board use DM pinctrl framework for configuring pin muxing. All boards set pin muxing in open coded board function which directly touch HW registers. DM pinctrl driver for A38x is relatively new and I think nobody converted any board to use it for initialization.
On Mon, Jan 30, 2023 at 07:16:26PM +0100, Pali Rohár wrote: > On Monday 30 January 2023 11:29:27 Tom Rini wrote: > > On Mon, Jan 30, 2023 at 05:19:01PM +0100, Mark Kettenis wrote: > > > > From: Simon Glass <sjg@chromium.org> > > > > Date: Mon, 30 Jan 2023 07:40:49 -0700 > > > > > > > > This is implicitly used in the source and seems useful, so add it. > > > > > > Not sure how this ended up with an "iommu" tag, but that seems wrong. > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > --- > > > > > > > > drivers/pinctrl/mvebu/Kconfig | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig > > > > index 7c51d138c8b..0b2be88e3a1 100644 > > > > --- a/drivers/pinctrl/mvebu/Kconfig > > > > +++ b/drivers/pinctrl/mvebu/Kconfig > > > > @@ -7,6 +7,10 @@ config PINCTRL_ARMADA_38X > > > > Support pin multiplexing and pin configuration control on > > > > Marvell's Armada-38x SoC. > > > > > > > > +config SPL_PINCTRL_ARMADA_38X > > > > + def_bool n # Armada 38x pin control driver (SPL) > > > > + depends on SPL && ARMADA_38X && SPL_PINCTRL_FULL > > > > + > > > > config PINCTRL_ARMADA_37XX > > > > depends on ARMADA_3700 && PINCTRL_FULL > > > > bool "Armada 37xx pin control driver" > > > > Looking at the code in question, it's because in > > drivers/gpio/mvebu_gpio.c we conditionally not set request / rfree > > dm_gpio_ops because turris_omnia (only platform in question) does not > > set pinctrl in SPL, but does set SPL_DM_GPIO. So the question I have, is > > all of that intentional and used today, in SPL, on the platform? I guess > > some set/get directions / values, to check board revs or something? > > I think this is because no A38x board use DM pinctrl framework for > configuring pin muxing. All boards set pin muxing in open coded board > function which directly touch HW registers. DM pinctrl driver for A38x > is relatively new and I think nobody converted any board to use it for > initialization. I do see that turris_omnia builds the pinctrl driver for full U-Boot today. But, does that mean that at some point in the future it should be converted to use SPL_PINCTRL ? As that would imply that we do not need / want SPL_PINCTRL_ARMADA_38X added (I suspect SPL_PINCTRL_STMFX and SPL_PINCTRL_ROCKCHIP are wrong) as it will be handled like the majority of other platforms in this area.
On Monday 30 January 2023 13:25:25 Tom Rini wrote: > On Mon, Jan 30, 2023 at 07:16:26PM +0100, Pali Rohár wrote: > > On Monday 30 January 2023 11:29:27 Tom Rini wrote: > > > On Mon, Jan 30, 2023 at 05:19:01PM +0100, Mark Kettenis wrote: > > > > > From: Simon Glass <sjg@chromium.org> > > > > > Date: Mon, 30 Jan 2023 07:40:49 -0700 > > > > > > > > > > This is implicitly used in the source and seems useful, so add it. > > > > > > > > Not sure how this ended up with an "iommu" tag, but that seems wrong. > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > --- > > > > > > > > > > drivers/pinctrl/mvebu/Kconfig | 4 ++++ > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig > > > > > index 7c51d138c8b..0b2be88e3a1 100644 > > > > > --- a/drivers/pinctrl/mvebu/Kconfig > > > > > +++ b/drivers/pinctrl/mvebu/Kconfig > > > > > @@ -7,6 +7,10 @@ config PINCTRL_ARMADA_38X > > > > > Support pin multiplexing and pin configuration control on > > > > > Marvell's Armada-38x SoC. > > > > > > > > > > +config SPL_PINCTRL_ARMADA_38X > > > > > + def_bool n # Armada 38x pin control driver (SPL) > > > > > + depends on SPL && ARMADA_38X && SPL_PINCTRL_FULL > > > > > + > > > > > config PINCTRL_ARMADA_37XX > > > > > depends on ARMADA_3700 && PINCTRL_FULL > > > > > bool "Armada 37xx pin control driver" > > > > > > Looking at the code in question, it's because in > > > drivers/gpio/mvebu_gpio.c we conditionally not set request / rfree > > > dm_gpio_ops because turris_omnia (only platform in question) does not > > > set pinctrl in SPL, but does set SPL_DM_GPIO. So the question I have, is > > > all of that intentional and used today, in SPL, on the platform? I guess > > > some set/get directions / values, to check board revs or something? > > > > I think this is because no A38x board use DM pinctrl framework for > > configuring pin muxing. All boards set pin muxing in open coded board > > function which directly touch HW registers. DM pinctrl driver for A38x > > is relatively new and I think nobody converted any board to use it for > > initialization. > > I do see that turris_omnia builds the pinctrl driver for full U-Boot > today. Yes, it allows u-boot pinmux command to print current pinctrl configuration and also allows gpio command to work correctly for gpios which are not in gpio mode by default. > But, does that mean that at some point in the future it should > be converted to use SPL_PINCTRL ? Every a38x board could be converted. But personally I'm not planning to do it. > As that would imply that we do not > need / want SPL_PINCTRL_ARMADA_38X added (I suspect SPL_PINCTRL_STMFX > and SPL_PINCTRL_ROCKCHIP are wrong) as it will be handled like the > majority of other platforms in this area. > > -- > Tom
On Mon, Jan 30, 2023 at 07:28:56PM +0100, Pali Rohár wrote: > On Monday 30 January 2023 13:25:25 Tom Rini wrote: > > On Mon, Jan 30, 2023 at 07:16:26PM +0100, Pali Rohár wrote: > > > On Monday 30 January 2023 11:29:27 Tom Rini wrote: > > > > On Mon, Jan 30, 2023 at 05:19:01PM +0100, Mark Kettenis wrote: > > > > > > From: Simon Glass <sjg@chromium.org> > > > > > > Date: Mon, 30 Jan 2023 07:40:49 -0700 > > > > > > > > > > > > This is implicitly used in the source and seems useful, so add it. > > > > > > > > > > Not sure how this ended up with an "iommu" tag, but that seems wrong. > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > --- > > > > > > > > > > > > drivers/pinctrl/mvebu/Kconfig | 4 ++++ > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig > > > > > > index 7c51d138c8b..0b2be88e3a1 100644 > > > > > > --- a/drivers/pinctrl/mvebu/Kconfig > > > > > > +++ b/drivers/pinctrl/mvebu/Kconfig > > > > > > @@ -7,6 +7,10 @@ config PINCTRL_ARMADA_38X > > > > > > Support pin multiplexing and pin configuration control on > > > > > > Marvell's Armada-38x SoC. > > > > > > > > > > > > +config SPL_PINCTRL_ARMADA_38X > > > > > > + def_bool n # Armada 38x pin control driver (SPL) > > > > > > + depends on SPL && ARMADA_38X && SPL_PINCTRL_FULL > > > > > > + > > > > > > config PINCTRL_ARMADA_37XX > > > > > > depends on ARMADA_3700 && PINCTRL_FULL > > > > > > bool "Armada 37xx pin control driver" > > > > > > > > Looking at the code in question, it's because in > > > > drivers/gpio/mvebu_gpio.c we conditionally not set request / rfree > > > > dm_gpio_ops because turris_omnia (only platform in question) does not > > > > set pinctrl in SPL, but does set SPL_DM_GPIO. So the question I have, is > > > > all of that intentional and used today, in SPL, on the platform? I guess > > > > some set/get directions / values, to check board revs or something? > > > > > > I think this is because no A38x board use DM pinctrl framework for > > > configuring pin muxing. All boards set pin muxing in open coded board > > > function which directly touch HW registers. DM pinctrl driver for A38x > > > is relatively new and I think nobody converted any board to use it for > > > initialization. > > > > I do see that turris_omnia builds the pinctrl driver for full U-Boot > > today. > > Yes, it allows u-boot pinmux command to print current pinctrl > configuration and also allows gpio command to work correctly for gpios > which are not in gpio mode by default. > > > But, does that mean that at some point in the future it should > > be converted to use SPL_PINCTRL ? > > Every a38x board could be converted. But personally I'm not planning to > do it. OK, but if someone were to do it, it would then be the case that PINCTRL_ARMADA_38X would be the driver used, yes? > > As that would imply that we do not > > need / want SPL_PINCTRL_ARMADA_38X added (I suspect SPL_PINCTRL_STMFX > > and SPL_PINCTRL_ROCKCHIP are wrong) as it will be handled like the > > majority of other platforms in this area. As I look at this more, I see that SPL_PINCTRL_STMFX _is_ one of the special cases as it seems in SPL they use the PINCTRL_STM32 driver and do _not_ want PINCTRL_STMFX so SPL_PINCTRL_STMFX is introduced as a way to keep the driver from being built in SPL.
On Monday 30 January 2023 13:59:21 Tom Rini wrote: > On Mon, Jan 30, 2023 at 07:28:56PM +0100, Pali Rohár wrote: > > On Monday 30 January 2023 13:25:25 Tom Rini wrote: > > > On Mon, Jan 30, 2023 at 07:16:26PM +0100, Pali Rohár wrote: > > > > On Monday 30 January 2023 11:29:27 Tom Rini wrote: > > > > > On Mon, Jan 30, 2023 at 05:19:01PM +0100, Mark Kettenis wrote: > > > > > > > From: Simon Glass <sjg@chromium.org> > > > > > > > Date: Mon, 30 Jan 2023 07:40:49 -0700 > > > > > > > > > > > > > > This is implicitly used in the source and seems useful, so add it. > > > > > > > > > > > > Not sure how this ended up with an "iommu" tag, but that seems wrong. > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > --- > > > > > > > > > > > > > > drivers/pinctrl/mvebu/Kconfig | 4 ++++ > > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig > > > > > > > index 7c51d138c8b..0b2be88e3a1 100644 > > > > > > > --- a/drivers/pinctrl/mvebu/Kconfig > > > > > > > +++ b/drivers/pinctrl/mvebu/Kconfig > > > > > > > @@ -7,6 +7,10 @@ config PINCTRL_ARMADA_38X > > > > > > > Support pin multiplexing and pin configuration control on > > > > > > > Marvell's Armada-38x SoC. > > > > > > > > > > > > > > +config SPL_PINCTRL_ARMADA_38X > > > > > > > + def_bool n # Armada 38x pin control driver (SPL) > > > > > > > + depends on SPL && ARMADA_38X && SPL_PINCTRL_FULL > > > > > > > + > > > > > > > config PINCTRL_ARMADA_37XX > > > > > > > depends on ARMADA_3700 && PINCTRL_FULL > > > > > > > bool "Armada 37xx pin control driver" > > > > > > > > > > Looking at the code in question, it's because in > > > > > drivers/gpio/mvebu_gpio.c we conditionally not set request / rfree > > > > > dm_gpio_ops because turris_omnia (only platform in question) does not > > > > > set pinctrl in SPL, but does set SPL_DM_GPIO. So the question I have, is > > > > > all of that intentional and used today, in SPL, on the platform? I guess > > > > > some set/get directions / values, to check board revs or something? > > > > > > > > I think this is because no A38x board use DM pinctrl framework for > > > > configuring pin muxing. All boards set pin muxing in open coded board > > > > function which directly touch HW registers. DM pinctrl driver for A38x > > > > is relatively new and I think nobody converted any board to use it for > > > > initialization. > > > > > > I do see that turris_omnia builds the pinctrl driver for full U-Boot > > > today. > > > > Yes, it allows u-boot pinmux command to print current pinctrl > > configuration and also allows gpio command to work correctly for gpios > > which are not in gpio mode by default. > > > > > But, does that mean that at some point in the future it should > > > be converted to use SPL_PINCTRL ? > > > > Every a38x board could be converted. But personally I'm not planning to > > do it. > > OK, but if someone were to do it, it would then be the case that > PINCTRL_ARMADA_38X would be the driver used, yes? Then it would be used in SPL. In proper U-Boot it is already used by commands like 'pinmux' or 'gpio'. > > > As that would imply that we do not > > > need / want SPL_PINCTRL_ARMADA_38X added (I suspect SPL_PINCTRL_STMFX > > > and SPL_PINCTRL_ROCKCHIP are wrong) as it will be handled like the > > > majority of other platforms in this area. > > As I look at this more, I see that SPL_PINCTRL_STMFX _is_ one of the > special cases as it seems in SPL they use the PINCTRL_STM32 driver and > do _not_ want PINCTRL_STMFX so SPL_PINCTRL_STMFX is introduced as a way > to keep the driver from being built in SPL. > > -- > Tom
diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig index 7c51d138c8b..0b2be88e3a1 100644 --- a/drivers/pinctrl/mvebu/Kconfig +++ b/drivers/pinctrl/mvebu/Kconfig @@ -7,6 +7,10 @@ config PINCTRL_ARMADA_38X Support pin multiplexing and pin configuration control on Marvell's Armada-38x SoC. +config SPL_PINCTRL_ARMADA_38X + def_bool n # Armada 38x pin control driver (SPL) + depends on SPL && ARMADA_38X && SPL_PINCTRL_FULL + config PINCTRL_ARMADA_37XX depends on ARMADA_3700 && PINCTRL_FULL bool "Armada 37xx pin control driver"
This is implicitly used in the source and seems useful, so add it. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/pinctrl/mvebu/Kconfig | 4 ++++ 1 file changed, 4 insertions(+)