Messages in this thread | | | Date | Fri, 3 Mar 2023 17:17:39 +0300 | Subject | Re: [PATCH 05/20] pinctrl: ralink: move to mediatek as mtmips | From | Arınç ÜNAL <> |
| |
Heyo,
On 3.03.2023 13:57, Sergio Paracuellos wrote: > Hi Arınç, > > On Fri, Mar 3, 2023 at 9:16 AM Arınç ÜNAL <arinc.unal@arinc9.com> wrote: >> >> Hey Sergio, >> >> On 3.03.2023 09:34, Sergio Paracuellos wrote: >>> On Fri, Mar 3, 2023 at 7:17 AM Sergio Paracuellos >>> <sergio.paracuellos@gmail.com> wrote: >>>> >>>> Hi Arınç, >>>> >>>> On Fri, Mar 3, 2023 at 1:30 AM <arinc9.unal@gmail.com> wrote: >>>>> >>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com> >>>>> >>>>> This platform from Ralink was acquired by MediaTek in 2011. Then, MediaTek >>>>> introduced new SoCs which utilise this platform. Move the driver to >>>>> mediatek pinctrl directory. Rename the ralink core driver to mtmips. >>>>> >>>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >>>>> --- >>>>> drivers/pinctrl/Kconfig | 1 - >>>>> drivers/pinctrl/Makefile | 1 - >>>>> drivers/pinctrl/mediatek/Kconfig | 51 ++++++++++- >>>>> drivers/pinctrl/mediatek/Makefile | 63 +++++++------ >>>>> .../{ralink => mediatek}/pinctrl-mt7620.c | 34 +++---- >>>>> .../{ralink => mediatek}/pinctrl-mt7621.c | 30 +++---- >>>>> .../{ralink => mediatek}/pinctrl-mt76x8.c | 60 ++++++------- >>>>> .../pinctrl-mtmips.c} | 90 +++++++++---------- >>>>> .../pinctrl-mtmips.h} | 16 ++-- >>>>> .../{ralink => mediatek}/pinctrl-rt2880.c | 20 ++--- >>>>> .../{ralink => mediatek}/pinctrl-rt305x.c | 44 ++++----- >>>>> .../{ralink => mediatek}/pinctrl-rt3883.c | 28 +++--- >>>>> drivers/pinctrl/ralink/Kconfig | 40 --------- >>>>> drivers/pinctrl/ralink/Makefile | 9 -- >>>>> 14 files changed, 246 insertions(+), 241 deletions(-) >>>>> rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt7620.c (81%) >>>>> rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt7621.c (80%) >>>>> rename drivers/pinctrl/{ralink => mediatek}/pinctrl-mt76x8.c (81%) >>>>> rename drivers/pinctrl/{ralink/pinctrl-ralink.c => mediatek/pinctrl-mtmips.c} (74%) >>>>> rename drivers/pinctrl/{ralink/pinctrl-ralink.h => mediatek/pinctrl-mtmips.h} (75%) >>>>> rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt2880.c (71%) >>>>> rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt305x.c (75%) >>>>> rename drivers/pinctrl/{ralink => mediatek}/pinctrl-rt3883.c (80%) >>>>> delete mode 100644 drivers/pinctrl/ralink/Kconfig >>>>> delete mode 100644 drivers/pinctrl/ralink/Makefile >>>>> >>>>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig >>>>> index dcb53c4a9584..8a6012770640 100644 >>>>> --- a/drivers/pinctrl/Kconfig >>>>> +++ b/drivers/pinctrl/Kconfig >>>>> @@ -537,7 +537,6 @@ source "drivers/pinctrl/nomadik/Kconfig" >>>>> source "drivers/pinctrl/nuvoton/Kconfig" >>>>> source "drivers/pinctrl/pxa/Kconfig" >>>>> source "drivers/pinctrl/qcom/Kconfig" >>>>> -source "drivers/pinctrl/ralink/Kconfig" >>>>> source "drivers/pinctrl/renesas/Kconfig" >>>>> source "drivers/pinctrl/samsung/Kconfig" >>>>> source "drivers/pinctrl/spear/Kconfig" >>>>> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile >>>>> index d5939840bb2a..ada6ed1d4e91 100644 >>>>> --- a/drivers/pinctrl/Makefile >>>>> +++ b/drivers/pinctrl/Makefile >>>>> @@ -66,7 +66,6 @@ obj-y += nomadik/ >>>>> obj-y += nuvoton/ >>>>> obj-$(CONFIG_PINCTRL_PXA) += pxa/ >>>>> obj-$(CONFIG_ARCH_QCOM) += qcom/ >>>>> -obj-$(CONFIG_PINCTRL_RALINK) += ralink/ >>>>> obj-$(CONFIG_PINCTRL_RENESAS) += renesas/ >>>>> obj-$(CONFIG_PINCTRL_SAMSUNG) += samsung/ >>>>> obj-$(CONFIG_PINCTRL_SPEAR) += spear/ >>>>> diff --git a/drivers/pinctrl/mediatek/Kconfig b/drivers/pinctrl/mediatek/Kconfig >>>>> index a71874fed3d6..2eeb55010563 100644 >>>>> --- a/drivers/pinctrl/mediatek/Kconfig >>>>> +++ b/drivers/pinctrl/mediatek/Kconfig >>>>> @@ -1,6 +1,6 @@ >>>>> # SPDX-License-Identifier: GPL-2.0-only >>>>> menu "MediaTek pinctrl drivers" >>>>> - depends on ARCH_MEDIATEK || COMPILE_TEST >>>>> + depends on ARCH_MEDIATEK || RALINK || COMPILE_TEST >>>>> >>>>> config EINT_MTK >>>>> tristate "MediaTek External Interrupt Support" >>>>> @@ -22,6 +22,12 @@ config PINCTRL_MTK >>>>> config PINCTRL_MTK_V2 >>>>> tristate >>>>> >>>>> +config PINCTRL_MTK_MTMIPS >>>>> + bool >>>>> + depends on RALINK >>>>> + select PINMUX >>>>> + select GENERIC_PINCONF >>>>> + >>>>> config PINCTRL_MTK_MOORE >>>>> bool >>>>> depends on OF >>>>> @@ -43,6 +49,49 @@ config PINCTRL_MTK_PARIS >>>>> select OF_GPIO >>>>> select PINCTRL_MTK_V2 >>>>> >>>>> +# For MIPS SoCs >>>>> +config PINCTRL_MT7620 >>>>> + bool "MediaTek MT7620 pin control" >>>>> + depends on SOC_MT7620 || COMPILE_TEST >>>>> + depends on RALINK >>>>> + default SOC_MT7620 >>>>> + select PINCTRL_MTK_MTMIPS >>>>> + >>>>> +config PINCTRL_MT7621 >>>>> + bool "MediaTek MT7621 pin control" >>>>> + depends on SOC_MT7621 || COMPILE_TEST >>>>> + depends on RALINK >>>>> + default SOC_MT7621 >>>>> + select PINCTRL_MTK_MTMIPS >>>>> + >>>>> +config PINCTRL_MT76X8 >>>>> + bool "MediaTek MT76X8 pin control" >>>>> + depends on SOC_MT7620 || COMPILE_TEST >>>>> + depends on RALINK >>>>> + default SOC_MT7620 >>>>> + select PINCTRL_MTK_MTMIPS >>>>> + >>>>> +config PINCTRL_RT2880 >>>>> + bool "Ralink RT2880 pin control" >>>>> + depends on SOC_RT288X || COMPILE_TEST >>>>> + depends on RALINK >>>>> + default SOC_RT288X >>>>> + select PINCTRL_MTK_MTMIPS >>>>> + >>>>> +config PINCTRL_RT305X >>>>> + bool "Ralink RT305X pin control" >>>>> + depends on SOC_RT305X || COMPILE_TEST >>>>> + depends on RALINK >>>>> + default SOC_RT305X >>>>> + select PINCTRL_MTK_MTMIPS >>>>> + >>>>> +config PINCTRL_RT3883 >>>>> + bool "Ralink RT3883 pin control" >>>>> + depends on SOC_RT3883 || COMPILE_TEST >>>>> + depends on RALINK >>>>> + default SOC_RT3883 >>>>> + select PINCTRL_MTK_MTMIPS >>>>> + >>>> >>>> I am not a Kconfig expert at all but... >>>> >>>> Should not all of these be depends on SOC_XXX || (COMPILE_TEST && >>>> RALINK) and avoid the " depends on RALINK" next line in all of them? >> >> This seems to do the same thing but I'm following the "either change >> them all or fit into the crowd" ideology. >> >>>> >>>> Just asking since we have yet arch read and write register operations >>>> in pinctrl common ralink code. Having in this way, when we address >>>> this arch thing in the next series just removing the "&& RALINK" part >>>> makes the review pretty obvious. >> >> You'd have to change RALINK with OF since we're still depending on that. >> RALINK selects OF by default so it's currently a hidden dependency. >> >>>> >>>> Other than that, changes look good to me. >>> >>> I think "depends on SOC_XXX || (COMPILE_TEST && MIPS)" would work also >>> and might be more accurate for compile testing targets. > > Are you sure? SOC_XXX here is already being enabled only if RALINK is > already enabled, right? [0]
I'm not sure who's your reply to, or what it's about here.
> >> >> This is not OK in both cases. If the driver is dependent on Ralink >> architecture code, choosing any other MIPS platform will make the driver >> available to compile, which will fail. > > SOC_XXX is already dependent on RALINK for real uses but the driver is > going to be selected for other MIPS platforms only for COMPILE_TEST > targets. Ideally drivers should be arch agnostic so can be selected > for any single arch build. Now we have arch dependent read and write > calls in the code, so you need the right headers to be properly found > to be able to compile testing. I think MIPS is enough dependency here > to properly find them. But if not, this should be (COMPILE_TEST && > RALINK)
I expect below to work without requiring the MIPS option.
ifeq ($(CONFIG_COMPILE_TEST),y) CFLAGS_pinctrl-mtmips.o += -I$(srctree)/arch/mips/include endif
> >> >> If the driver is independent of Ralink architecture code, you're >> limiting the driver to be compiled only when a MIPS platform is selected. > > So... how are you planning to allow compile testing of the driver in > any single arch when we get rid of all the arch dependent code? If you > make everything dependent on RALINK you cannot.
I intend to make it dependent on OF, not RALINK.
Arınç
| |