From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH 1/5] pinctrl: sunxi: refactor pinctrl choice selecting for ARM64 Date: Tue, 28 Feb 2017 18:17:30 +0000 Message-ID: <488811a1-b64f-c435-e919-7f83dc107697@arm.com> References: <20170228172444.59655-1-icenowy@aosc.xyz> Reply-To: andre.przywara-5wv7dgnIgG8@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Sender: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org In-Reply-To: <20170228172444.59655-1-icenowy-ymACFijhrKM@public.gmane.org> List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , To: icenowy-ymACFijhrKM@public.gmane.org, Linus Walleij , Rob Herring , Maxime Ripard , Chen-Yu Tsai , Catalin Marinas , Will Deacon Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org List-Id: linux-gpio@vger.kernel.org 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 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. But as the rest of the patch is fine, if you remove this line: Reviewed-by: Andre Przywara 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 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751488AbdB1STD (ORCPT ); Tue, 28 Feb 2017 13:19:03 -0500 Received: from foss.arm.com ([217.140.101.70]:40704 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751392AbdB1STC (ORCPT ); Tue, 28 Feb 2017 13:19:02 -0500 Subject: Re: [linux-sunxi] [PATCH 1/5] pinctrl: sunxi: refactor pinctrl choice selecting for ARM64 To: icenowy@aosc.xyz, Linus Walleij , Rob Herring , Maxime Ripard , Chen-Yu Tsai , Catalin Marinas , Will Deacon References: <20170228172444.59655-1-icenowy@aosc.xyz> Cc: linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com From: Andre Przywara Message-ID: <488811a1-b64f-c435-e919-7f83dc107697@arm.com> Date: Tue, 28 Feb 2017 18:17:30 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <20170228172444.59655-1-icenowy@aosc.xyz> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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. But as the rest of the patch is fine, if you remove this line: Reviewed-by: Andre Przywara 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 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: andre.przywara@arm.com (Andre Przywara) Date: Tue, 28 Feb 2017 18:17:30 +0000 Subject: [linux-sunxi] [PATCH 1/5] pinctrl: sunxi: refactor pinctrl choice selecting for ARM64 In-Reply-To: <20170228172444.59655-1-icenowy@aosc.xyz> References: <20170228172444.59655-1-icenowy@aosc.xyz> Message-ID: <488811a1-b64f-c435-e919-7f83dc107697@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 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. But as the rest of the patch is fine, if you remove this line: Reviewed-by: Andre Przywara 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 >