[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Eugeniu Rosca]

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

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Marek Vasut]

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

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Dirk Behme]

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;
>>
> 
> 

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Marek Vasut]

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

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Eugeniu Rosca]

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.

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Marek Vasut]

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

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Eugeniu Rosca]

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.

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Marek Vasut]

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

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Dirk Behme]

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

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Marek Vasut]

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

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Marek Vasut]

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

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Eugeniu Rosca]

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.

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Marek Vasut]

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

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Dirk Behme]

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.
> 

[U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n

[Marek Vasut]

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