Re: [PATCH v3] arm: dts: sun7i-a20-bananapi: name the GPIO lines

[Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3358 bytes --]

Hi Oleksij,

It looks much better, thanks!

On Sat, May 06, 2017 at 07:30:38AM +0200, Oleksij Rempel wrote:
> This names the GPIO lines on the Banana Pi board in accordance with
> the A20_Banana_Pi v1.4 Specification.
> 
> This will make these line names reflect through to user space
> so that they can easily be identified and used with the new
> character device ABI.
> 
> Signed-off-by: Oleksij Rempel <linux-YEK0n+YFykbzxQdaRaTXBw@public.gmane.org>
> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> Cc: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---

Usually, it's better here if you put a change log with the things that
changed from one version to another.

You probably remember all of the comments that were made on the
previous version of that patch, but I don't ;)

>  arch/arm/boot/dts/sun7i-a20-bananapi.dts | 51 ++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> index 91f2e5f..6b326b3 100644
> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> @@ -178,6 +178,57 @@
>  };
>  
>  &pio {
> +	gpio-line-names =
> +		/* PA */
> +		"ERXD3", "ERXD2", "ERXD1", "ERXD0", "ETXD3",
> +			"ETXD2", "ETXD1", "ETXD0",
> +		"ERXCK", "ERXERR", "ERXDV", "EMDC", "EMDIO",
> +			"ETXEN", "ETXCK", "ECRS",
> +		"ECOL", "ETXERR", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		/* PB */
> +		"PMU-SCK", "PMU-SDA", "", "", "", "", "", "",
> +		"", "USB0-DRV", "", "", "", "", "", "",
> +		"", "", "", "", "SCL", "SDA", "", "",

I'm not sure how it's called in the schematics / documentation, but
having a way to get which bus it's from would be helpful (like you did
for the PMIC I2C bus).

> +		"", "", "", "", "", "", "", "",
> +		/* PC */
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		/* PD */
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		/* PE */
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		/* PF */
> +		"SD0-D1", "SD0-D0", "SD0-CLK", "SD0-CMD", "SD0-D3",
> +			"SD0-D2", "", "",

Why did you change the wrapping and indentation on those two lines?

> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		/* PG */
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		"", "", "", "", "", "", "", "",
> +		/* PH */
> +		"TXD0", "RXD0", "IO-1", "PH3", "USB0-IDDET", "PH5", "", "",

What are PH3 and PH5 used for on the board? Can't we provide a more
explicit name?

Thanks!
Maxime
-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]