* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n @ 2019-04-02 13:18 Eugeniu Rosca 2019-04-02 13:34 ` Marek Vasut 0 siblings, 1 reply; 15+ messages in thread From: Eugeniu Rosca @ 2019-04-02 13:18 UTC (permalink / raw) To: u-boot 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<----- Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. Some recent Linux commits sharing the same approach: - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce ("acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle") - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e ("spi: make OF helper available for others") - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as GPIO") Reported-by: Dirk Behme <dirk.behme@de.bosch.com> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> --- drivers/pinctrl/renesas/sh_pfc.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/pinctrl/renesas/sh_pfc.h b/drivers/pinctrl/renesas/sh_pfc.h index b98c2f185d26..3d95d3c725cf 100644 --- a/drivers/pinctrl/renesas/sh_pfc.h +++ b/drivers/pinctrl/renesas/sh_pfc.h @@ -261,7 +261,15 @@ void sh_pfc_write(struct sh_pfc *pfc, u32 reg, u32 data); const struct pinmux_bias_reg * sh_pfc_pin_to_bias_reg(const struct sh_pfc *pfc, unsigned int pin, unsigned int *bit); +#if IS_ENABLED(CONFIG_PINCTRL_PFC) int sh_pfc_config_mux_for_gpio(struct udevice *dev, unsigned pin_selector); +#else +static inline +int sh_pfc_config_mux_for_gpio(struct udevice *dev, unsigned pin_selector) +{ + return -ENODEV; +} +#endif extern const struct sh_pfc_soc_info r8a7790_pinmux_info; extern const struct sh_pfc_soc_info r8a7791_pinmux_info; -- 2.21.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-02 13:18 [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n Eugeniu Rosca @ 2019-04-02 13:34 ` Marek Vasut 2019-04-02 15:17 ` Dirk Behme 0 siblings, 1 reply; 15+ messages in thread From: Marek Vasut @ 2019-04-02 13:34 UTC (permalink / raw) To: u-boot 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<----- > > Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. > > Some recent Linux commits sharing the same approach: > - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce > ("acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle") > - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e > ("spi: make OF helper available for others") > - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 > ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") > > Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as GPIO") > Reported-by: Dirk Behme <dirk.behme@de.bosch.com> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> Does CONFIG_PINCTRL_PFC=n produce a bootable binary ? What is the usecase of CONFIG_PINCTRL_PFC=n ? I suspect we should rather make sure CONFIG_PINCTRL_PFC=y . > --- > drivers/pinctrl/renesas/sh_pfc.h | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/pinctrl/renesas/sh_pfc.h b/drivers/pinctrl/renesas/sh_pfc.h > index b98c2f185d26..3d95d3c725cf 100644 > --- a/drivers/pinctrl/renesas/sh_pfc.h > +++ b/drivers/pinctrl/renesas/sh_pfc.h > @@ -261,7 +261,15 @@ void sh_pfc_write(struct sh_pfc *pfc, u32 reg, u32 data); > const struct pinmux_bias_reg * > sh_pfc_pin_to_bias_reg(const struct sh_pfc *pfc, unsigned int pin, > unsigned int *bit); > +#if IS_ENABLED(CONFIG_PINCTRL_PFC) > int sh_pfc_config_mux_for_gpio(struct udevice *dev, unsigned pin_selector); > +#else > +static inline > +int sh_pfc_config_mux_for_gpio(struct udevice *dev, unsigned pin_selector) > +{ > + return -ENODEV; > +} > +#endif > > extern const struct sh_pfc_soc_info r8a7790_pinmux_info; > extern const struct sh_pfc_soc_info r8a7791_pinmux_info; > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-02 13:34 ` Marek Vasut @ 2019-04-02 15:17 ` Dirk Behme 2019-04-02 15:28 ` Marek Vasut 0 siblings, 1 reply; 15+ messages in thread From: Dirk Behme @ 2019-04-02 15:17 UTC (permalink / raw) To: u-boot 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<----- >> >> Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. >> >> Some recent Linux commits sharing the same approach: >> - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce >> ("acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle") >> - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e >> ("spi: make OF helper available for others") >> - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 >> ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") >> >> Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as GPIO") >> Reported-by: Dirk Behme <dirk.behme@de.bosch.com> >> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > Does CONFIG_PINCTRL_PFC=n produce a bootable binary ? Why not? Main memory, boot device and UART are configured before U-Boot, no? Best regards Dirk > What is the usecase of CONFIG_PINCTRL_PFC=n ? > > I suspect we should rather make sure CONFIG_PINCTRL_PFC=y . > >> --- >> drivers/pinctrl/renesas/sh_pfc.h | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/pinctrl/renesas/sh_pfc.h b/drivers/pinctrl/renesas/sh_pfc.h >> index b98c2f185d26..3d95d3c725cf 100644 >> --- a/drivers/pinctrl/renesas/sh_pfc.h >> +++ b/drivers/pinctrl/renesas/sh_pfc.h >> @@ -261,7 +261,15 @@ void sh_pfc_write(struct sh_pfc *pfc, u32 reg, u32 data); >> const struct pinmux_bias_reg * >> sh_pfc_pin_to_bias_reg(const struct sh_pfc *pfc, unsigned int pin, >> unsigned int *bit); >> +#if IS_ENABLED(CONFIG_PINCTRL_PFC) >> int sh_pfc_config_mux_for_gpio(struct udevice *dev, unsigned pin_selector); >> +#else >> +static inline >> +int sh_pfc_config_mux_for_gpio(struct udevice *dev, unsigned pin_selector) >> +{ >> + return -ENODEV; >> +} >> +#endif >> >> extern const struct sh_pfc_soc_info r8a7790_pinmux_info; >> extern const struct sh_pfc_soc_info r8a7791_pinmux_info; >> > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-02 15:17 ` Dirk Behme @ 2019-04-02 15:28 ` Marek Vasut 2019-04-02 15:40 ` Eugeniu Rosca 0 siblings, 1 reply; 15+ messages in thread From: Marek Vasut @ 2019-04-02 15:28 UTC (permalink / raw) To: u-boot 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<----- >>> >>> Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. >>> >>> Some recent Linux commits sharing the same approach: >>> - >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce >>> >>> ("acpi, nfit: Add function to look up nvdimm device and provide >>> SMBIOS handle") >>> - >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e >>> >>> ("spi: make OF helper available for others") >>> - >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 >>> >>> ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") >>> >>> Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as GPIO") >>> Reported-by: Dirk Behme <dirk.behme@de.bosch.com> >>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >> >> 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. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-02 15:28 ` Marek Vasut @ 2019-04-02 15:40 ` Eugeniu Rosca 2019-04-02 16:02 ` Marek Vasut 2019-04-02 17:58 ` Dirk Behme 0 siblings, 2 replies; 15+ messages in thread From: Eugeniu Rosca @ 2019-04-02 15:40 UTC (permalink / raw) To: u-boot 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<----- > >>> > >>> Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. > >>> > >>> Some recent Linux commits sharing the same approach: > >>> - > >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce > >>> > >>> ("acpi, nfit: Add function to look up nvdimm device and provide > >>> SMBIOS handle") > >>> - > >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e > >>> > >>> ("spi: make OF helper available for others") > >>> - > >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 > >>> > >>> ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") > >>> > >>> Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as GPIO") > >>> Reported-by: Dirk Behme <dirk.behme@de.bosch.com> > >>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > >> > >> 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. 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. [1] U-Boot 2019.04-rc4-00100-g03ece61db8 (Apr 02 2019 - 17:23:57 +0200) CPU: Renesas Electronics R8A7795 rev 2.0 Model: Renesas H3ULCB board based on r8a7795 ES2.0+ DRAM: 3.9 GiB MMC: gpio at e6055000: set_value: error: gpio gpio at e60550001 not reserved Can't set regulator : regulator-vccq-sdhi0 gpio to: 1 sd at ee100000: 0, sd at ee140000: 1 Loading Environment from MMC... OK In: serial at e6e88000 Out: serial at e6e88000 Err: serial at e6e88000 Net: gpio at e6052000: set_value: error: gpio gpio at e605200010 not reserved gpio at e6052000: set_value: error: gpio gpio at e605200010 not reserved eth0: ethernet at e6800000 Hit any key to stop autoboot: 0 > -- > Best regards, > Marek Vasut Best regards, Eugeniu. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-02 15:40 ` Eugeniu Rosca @ 2019-04-02 16:02 ` Marek Vasut 2019-04-02 17:02 ` Eugeniu Rosca 2019-04-02 17:58 ` Dirk Behme 1 sibling, 1 reply; 15+ messages in thread From: Marek Vasut @ 2019-04-02 16:02 UTC (permalink / raw) To: u-boot 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<----- >>>>> >>>>> Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. >>>>> >>>>> Some recent Linux commits sharing the same approach: >>>>> - >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce >>>>> >>>>> ("acpi, nfit: Add function to look up nvdimm device and provide >>>>> SMBIOS handle") >>>>> - >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e >>>>> >>>>> ("spi: make OF helper available for others") >>>>> - >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 >>>>> >>>>> ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") >>>>> >>>>> Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as GPIO") >>>>> Reported-by: Dirk Behme <dirk.behme@de.bosch.com> >>>>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >>>> >>>> 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 ? > [1] U-Boot 2019.04-rc4-00100-g03ece61db8 (Apr 02 2019 - 17:23:57 +0200) > > CPU: Renesas Electronics R8A7795 rev 2.0 > Model: Renesas H3ULCB board based on r8a7795 ES2.0+ > DRAM: 3.9 GiB > MMC: gpio at e6055000: set_value: error: gpio gpio at e60550001 not reserved > Can't set regulator : regulator-vccq-sdhi0 gpio to: 1 > sd at ee100000: 0, sd at ee140000: 1 > Loading Environment from MMC... OK > In: serial at e6e88000 > Out: serial at e6e88000 > Err: serial at e6e88000 > Net: gpio at e6052000: set_value: error: gpio gpio at e605200010 not reserved > gpio at e6052000: set_value: error: gpio gpio at e605200010 not reserved > eth0: ethernet at e6800000 > Hit any key to stop autoboot: 0 > >> -- >> Best regards, >> Marek Vasut > > Best regards, > Eugeniu. > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-02 16:02 ` Marek Vasut @ 2019-04-02 17:02 ` Eugeniu Rosca 2019-04-03 12:11 ` Marek Vasut 0 siblings, 1 reply; 15+ messages in thread From: Eugeniu Rosca @ 2019-04-02 17:02 UTC (permalink / raw) To: u-boot 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. 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. > > [1] U-Boot 2019.04-rc4-00100-g03ece61db8 (Apr 02 2019 - 17:23:57 +0200) [..] Best regards, Eugeniu. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-02 17:02 ` Eugeniu Rosca @ 2019-04-03 12:11 ` Marek Vasut 2019-04-03 12:30 ` Dirk Behme 0 siblings, 1 reply; 15+ messages in thread From: Marek Vasut @ 2019-04-03 12:11 UTC (permalink / raw) To: u-boot 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 ? >>> [1] U-Boot 2019.04-rc4-00100-g03ece61db8 (Apr 02 2019 - 17:23:57 +0200) > [..] > > Best regards, > Eugeniu. > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-03 12:11 ` Marek Vasut @ 2019-04-03 12:30 ` Dirk Behme 2019-04-03 14:01 ` Marek Vasut 0 siblings, 1 reply; 15+ messages in thread From: Dirk Behme @ 2019-04-03 12:30 UTC (permalink / raw) To: u-boot 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: 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 ... Best regards Dirk ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-03 12:30 ` Dirk Behme @ 2019-04-03 14:01 ` Marek Vasut 2019-04-04 1:36 ` Marek Vasut 0 siblings, 1 reply; 15+ messages in thread From: Marek Vasut @ 2019-04-03 14:01 UTC (permalink / raw) To: u-boot 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) . -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-03 14:01 ` Marek Vasut @ 2019-04-04 1:36 ` Marek Vasut 2019-04-04 12:42 ` Eugeniu Rosca 0 siblings, 1 reply; 15+ messages in thread From: Marek Vasut @ 2019-04-04 1:36 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-04 1:36 ` Marek Vasut @ 2019-04-04 12:42 ` Eugeniu Rosca 2019-04-21 22:23 ` Marek Vasut 0 siblings, 1 reply; 15+ messages in thread From: Eugeniu Rosca @ 2019-04-04 12:42 UTC (permalink / raw) To: u-boot Hi Marek, On Thu, Apr 04, 2019 at 03:36:45AM +0200, Marek Vasut wrote: [..] > 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. I don't think anybody is going to contradict you, as long as your solution resolves/prevents the build error reported in this thread. Since you have the best vision of it, I assume you take the ownership of the change and CC the relevant people upon patch submission? Best regards, Eugeniu. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-04 12:42 ` Eugeniu Rosca @ 2019-04-21 22:23 ` Marek Vasut 0 siblings, 0 replies; 15+ messages in thread From: Marek Vasut @ 2019-04-21 22:23 UTC (permalink / raw) To: u-boot On 4/4/19 2:42 PM, Eugeniu Rosca wrote: > Hi Marek, Hi, > On Thu, Apr 04, 2019 at 03:36:45AM +0200, Marek Vasut wrote: > [..] >> 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. > > I don't think anybody is going to contradict you, as long as your > solution resolves/prevents the build error reported in this thread. > > Since you have the best vision of it, I assume you take the ownership > of the change and CC the relevant people upon patch submission? Try this series: http://patchwork.ozlabs.org/project/uboot/list/?series=103861 -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-02 15:40 ` Eugeniu Rosca 2019-04-02 16:02 ` Marek Vasut @ 2019-04-02 17:58 ` Dirk Behme 2019-04-03 12:08 ` Marek Vasut 1 sibling, 1 reply; 15+ messages in thread From: Dirk Behme @ 2019-04-02 17:58 UTC (permalink / raw) To: u-boot On 02.04.19 17:40, 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<----- >>>>> >>>>> Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. >>>>> >>>>> Some recent Linux commits sharing the same approach: >>>>> - >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce >>>>> >>>>> ("acpi, nfit: Add function to look up nvdimm device and provide >>>>> SMBIOS handle") >>>>> - >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e >>>>> >>>>> ("spi: make OF helper available for others") >>>>> - >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 >>>>> >>>>> ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") >>>>> >>>>> Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as GPIO") >>>>> Reported-by: Dirk Behme <dirk.behme@de.bosch.com> >>>>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >>>> >>>> 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. What's about people needing to do products based on these reference designs which have boot time and by this size requirements? And having functions which are build time encapsulated with CONFIG_* but not their callers I simply would consider as a bug which needs to be fixed. Like you have done here, citing several kernel examples for this :) Best regards Dirk > 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. > > [1] U-Boot 2019.04-rc4-00100-g03ece61db8 (Apr 02 2019 - 17:23:57 +0200) > > CPU: Renesas Electronics R8A7795 rev 2.0 > Model: Renesas H3ULCB board based on r8a7795 ES2.0+ > DRAM: 3.9 GiB > MMC: gpio at e6055000: set_value: error: gpio gpio at e60550001 not reserved > Can't set regulator : regulator-vccq-sdhi0 gpio to: 1 > sd at ee100000: 0, sd at ee140000: 1 > Loading Environment from MMC... OK > In: serial at e6e88000 > Out: serial at e6e88000 > Err: serial at e6e88000 > Net: gpio at e6052000: set_value: error: gpio gpio at e605200010 not reserved > gpio at e6052000: set_value: error: gpio gpio at e605200010 not reserved > eth0: ethernet at e6800000 > Hit any key to stop autoboot: 0 > >> -- >> Best regards, >> Marek Vasut > > Best regards, > Eugeniu. > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n 2019-04-02 17:58 ` Dirk Behme @ 2019-04-03 12:08 ` Marek Vasut 0 siblings, 0 replies; 15+ messages in thread From: Marek Vasut @ 2019-04-03 12:08 UTC (permalink / raw) To: u-boot On 4/2/19 7:58 PM, Dirk Behme wrote: > On 02.04.19 17:40, 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<----- >>>>>> >>>>>> Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. >>>>>> >>>>>> Some recent Linux commits sharing the same approach: >>>>>> - >>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce >>>>>> >>>>>> >>>>>> ("acpi, nfit: Add function to look up nvdimm device and provide >>>>>> SMBIOS handle") >>>>>> - >>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e >>>>>> >>>>>> >>>>>> ("spi: make OF helper available for others") >>>>>> - >>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 >>>>>> >>>>>> >>>>>> ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") >>>>>> >>>>>> Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as >>>>>> GPIO") >>>>>> Reported-by: Dirk Behme <dirk.behme@de.bosch.com> >>>>>> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >>>>> >>>>> 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. > > > What's about people needing to do products based on these reference > designs which have boot time and by this size requirements? You can boot Linux or other RTOS from ATF directly if boot time or size is the requirement. That's basically what happens with OpTee OS already today. Does that cover such requirements ? But pulling out vital functionality out of U-Boot, which results in a semi-broken build, is not a good way to optimize things. Can we agree on that ? > And having functions which are build time encapsulated with CONFIG_* but > not their callers I simply would consider as a bug which needs to be > fixed. Like you have done here, citing several kernel examples for this :) I wonder if the fix shouldn't be in Kconfig, make the GPIO driver depend on the PFC driver, because the GPIO driver cannot work properly without the PFC driver ; and ultimately the system cannot work properly without the PFC driver either. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-04-21 22:23 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-02 13:18 [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n Eugeniu Rosca 2019-04-02 13:34 ` Marek Vasut 2019-04-02 15:17 ` Dirk Behme 2019-04-02 15:28 ` Marek Vasut 2019-04-02 15:40 ` Eugeniu Rosca 2019-04-02 16:02 ` Marek Vasut 2019-04-02 17:02 ` Eugeniu Rosca 2019-04-03 12:11 ` Marek Vasut 2019-04-03 12:30 ` Dirk Behme 2019-04-03 14:01 ` Marek Vasut 2019-04-04 1:36 ` Marek Vasut 2019-04-04 12:42 ` Eugeniu Rosca 2019-04-21 22:23 ` Marek Vasut 2019-04-02 17:58 ` Dirk Behme 2019-04-03 12:08 ` Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.