[linux-sunxi] [PATCH 1/5] pinctrl: sunxi: refactor pinctrl choice selecting for ARM64

[icenowy@aosc.xyz (Icenowy Zheng)]

01.03.2017, 02:15, "Andre Przywara" <andre.przywara@arm.com>:
> Hi Icenowy,
>
> (first thing: could you create your series with --cover-letter and fill
> this in? There you could explain what this series is about and also
> state things like dependencies from other patches and the commit that
> you based that on.)
>
> On 28/02/17 17:24, Icenowy Zheng wrote:
>> ?ARM64 Allwinner SoCs used to have every pinctrl driver selected in
>> ?ARCH_SUNXI. Change this to make their default value to (ARM64 &&
>> ?ARCH_SUNXI).
>>
>> ?Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>
> Overall this is a nice solution. Thanks for posting this.
>
>> ?---
>> ??drivers/pinctrl/sunxi/Kconfig | 9 +++++----
>> ??1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> ?diff --git a/drivers/pinctrl/sunxi/Kconfig b/drivers/pinctrl/sunxi/Kconfig
>> ?index 816015cf7053..92325736d953 100644
>> ?--- a/drivers/pinctrl/sunxi/Kconfig
>> ?+++ b/drivers/pinctrl/sunxi/Kconfig
>> ?@@ -48,8 +48,9 @@ config PINCTRL_SUN8I_H3
>> ??????????select PINCTRL_SUNXI
>>
>> ??config PINCTRL_SUN8I_H3_R
>> ?- def_bool MACH_SUN8I
>> ?- select PINCTRL_SUNXI_COMMON
>> ?+ def_bool MACH_SUN8I || (ARM64 && ARCH_SUNXI)
>> ?+ depends on RESET_CONTROLLER
>
> So this looks a bit odd. I take it this is for the extra reset register
> in the PRCM block.
> 1) I don't think this belongs into this patch. If this has been
> forgotten in the past, please make an extra patch for this. It's cheap
> to do so ;-)
> 2) Is this really a "depends on"? Shouldn't this be a select? With
> sunxi_ng clocks we don't need the reset controller for the normal
> peripherals anymore, so this option might not be selected by default in
> the future. And having this "depends on" leads to the PINCTRL_ option
> being hidden if RESET_CONTROLLER isn't selected.
> I think this bites us already in arm64, where ARCH_HAS_RESET_CONTROLLER
> is not enabled for ARCH_SUNXI.

PINCTRL_ options are always hidden now, Maxime, do you
want to change that?

We should enable ARCH_HAS_RESET_CONTROLLER for
ARCH_SUNXI, as sunxi-ng ccu is a reset controller.

Without RESET_CONTROLLER enabled, errors will occur
when linking:
```
drivers/built-in.o: In function `sunxi_ccu_probe':
of_reserved_mem.c:(.text+0x16058): undefined reference to `reset_controller_register'
```

If you don't care, I will make it an additional patch.

And there's really a reset line for the R_PIO pinctrls.

But you're right, this line shouldn't occur in this patch.
I will make a more patch for it.

>
> But as the rest of the patch is fine, if you remove this line:
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
>
> Cheers,
> Andre.
>
>> ?+ select PINCTRL_SUNXI
>>
>> ??config PINCTRL_SUN8I_V3S
>> ??????????def_bool MACH_SUN8I
>> ?@@ -65,11 +66,11 @@ config PINCTRL_SUN9I_A80_R
>> ??????????select PINCTRL_SUNXI
>>
>> ??config PINCTRL_SUN50I_A64
>> ?- bool
>> ?+ def_bool ARM64 && ARCH_SUNXI
>> ??????????select PINCTRL_SUNXI
>>
>> ??config PINCTRL_SUN50I_H5
>> ?- bool
>> ?+ def_bool ARM64 && ARCH_SUNXI
>> ??????????select PINCTRL_SUNXI
>>
>> ??endif