From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Thu, 4 Apr 2019 03:36:45 +0200 Subject: [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n In-Reply-To: <5e965baf-c7f6-1ea1-2117-a18964a464e0@gmail.com> References: <20190402131823.15841-1-erosca@de.adit-jv.com> <1a61e450-6c59-56b2-2efa-ff2bce58d369@gmail.com> <47a77b99-711a-fde1-220f-057bdb08ca6a@gmail.com> <2e9e3d57-2690-4c64-f4cf-c0832f3e74b4@gmail.com> <20190402154020.GA21698@vmlxhi-102.adit-jv.com> <22ce7959-7615-012b-73dd-b3aee3d21220@gmail.com> <20190402170207.GA23017@vmlxhi-102.adit-jv.com> <5e965baf-c7f6-1ea1-2117-a18964a464e0@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On 4/3/19 4:01 PM, Marek Vasut wrote: > On 4/3/19 2:30 PM, Dirk Behme wrote: >> On 03.04.2019 14:11, Marek Vasut wrote: >>> On 4/2/19 7:02 PM, Eugeniu Rosca wrote: >>>> On Tue, Apr 02, 2019 at 06:02:46PM +0200, Marek Vasut wrote: >>>>> On 4/2/19 5:40 PM, Eugeniu Rosca wrote: >>>>>> On Tue, Apr 02, 2019 at 05:28:43PM +0200, Marek Vasut wrote: >>>>>>> On 4/2/19 5:17 PM, Dirk Behme wrote: >>>>>>>> On 02.04.19 15:34, Marek Vasut wrote: >>>>>>>>> On 4/2/19 3:18 PM, Eugeniu Rosca wrote: >>>>>>>>>> With CONFIG_PINCTRL_PFC=n, aarch64-linux-gnu-ld reports: >>>>>>>>>> >>>>>>>>>> -----8<----- >>>>>>>>>>     LD      u-boot >>>>>>>>>> drivers/gpio/built-in.o: In function `rcar_gpio_request': >>>>>>>>>> drivers/gpio/gpio-rcar.c:128: undefined reference to >>>>>>>>>> `sh_pfc_config_mux_for_gpio' >>>>>>>>>> -----8<----- >>>> [..] >>>>>>>>> Does CONFIG_PINCTRL_PFC=n produce a bootable binary ? >>>>>>>> >>>>>>>> Why not? Main memory, boot device and UART are configured before >>>>>>>> U-Boot, >>>>>>>> no? >>>>>>> >>>>>>> It depends on what is running before U-Boot, so not necessarily. >>>>>>> >>>>>>> And speaking of boot device, consider the case where the system runs >>>>>>> from eMMC and uses the HS200/HS400 modes, which need to switch bus >>>>>>> mode >>>>>>> using the pinmux driver. >>>>>>> >>>>>>> Is there a real-world use case where you would want to disable the >>>>>>> pinmux driver ? And what is the benefit of that, except that it would >>>>>>> cause all kinds of weird problems. >>>>>> >>>>>> My H3ULCB-KF boots just fine [1] with CONFIG_PINCTRL_PFC=n, but I >>>>>> personally don't have any use-case which I need to fulfill on a >>>>>> Renesas reference design by disabling PFC. >>>>> >>>>> And the eMMC and SDHI both work fine too in HS400/SDR104 modes ? >>>>> They cannot, since you cannot switch the pinmux properties of the bus. >>>>> What about the errors in the log below, they don't look quite fine. >>>>> >>>>>> Rather, the motivation here is to ensure U-Boot builds fine with as >>>>>> many randconfig results as possible, which is a standard practice in >>>>>> Linux. I personally favor my solution, but I am also open minded if >>>>>> the linker error is avoided by introducing a direct/reverse dependency >>>>>> between PFC and another relevant R-Car3 Kconfig symbol. >>>>> >>>>> I am fine with fixing randconfig build errors. My question here is >>>>> whether it makes sense to allow U-Boot build without PFC support, >>>>> since that would cause all kinds of problems. I am banking toward >>>>> playing it safe and not allowing such an option at all. Thoughts ? >>>> >>>> It looks like in Linux, PINCTRL is a fundamental feature selected >>>> (i.e. *cannot* be disabled by users) by ARCH_RENESAS since v4.5 commit >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=26a7e06dfee9 >>>> >>>> ("arm64: renesas: r8a7795: Add Renesas R8A7795 SoC support"). >>>> >>>> So, demanding a PFC-free U-Boot doesn't look reasonable to me. >>> >>> That's sensible. >>> >>>> Should PINCTRL be selected by CONFIG_RCAR_GEN3 as it is done in Linux? >>>> One caveat is that PINCTRL currently depends on DM, so R-Car3 U-Boot >>>> would become dependent on DM too, i.e. users won't have the option of >>>> a legacy U-Boot anymore. >>> >>> Non-DM operation is not supported anyway, the direction is toward DM/DT >>> support. Ultimately, it should be possible to have a single U-Boot >>> binary and just exchange the DT to support different boards. >>> >>> My concern is with the size of the PFC tables, they are massive, sparse >>> and keep growing, but that's a different topic. >>> >>> That said, what about making the GPIO driver depend on PFC driver and >>> then have Gen3 select PFC by default in Kconfig ? >> >> >> Of course, you can add such a dependency in Kconfig. But that's not the >> question here and won't fix the issue: > > What is the question then ? > >> It won't fix the issue that we have code encapsulated with a CONFIG_* >> option and a caller which is not encapsulated with this. >> >> To fix this with your proposal, you need to merge CONFIG_PINCTRL_PFC and >> CONFIG_RCAR_GPIO to *one* CONFIG_RCAR_PINCTRL_PFC_GPIO (or whatever) to >> ensure that both, the function definition *and* the caller are >> encapsulated by the *same* CONFIG switch. But this sounds somehow quite >> strange to me ... > > I don't think I understand this part. If the GPIO driver depends on the > PFC driver in Kconfig, then you can either have > - both compiled in > - neither PFC nor GPIO driver > - only the PFC driver > and all three options provide working result. Did I miss something ? > > We can add this patch too, but I'd like to see the Kconfig fix alongside > it. Note that the patch should use #if CONFIG_IS_ENABLED(PINCTRL_PFC) . I was thinking about this patch further and I think the best way forward would be to extend the pinmux/pinctrl API with a callback to set a pin as GPIO and then just call that API from the GPIO driver. That would be the generic solution and would make this whole sh_pfc_config_mux_for_gpio() go away altogether. -- Best regards, Marek Vasut