From: Andy Yan <andy.yan@rock-chips.com>
To: Heiko Stuebner <heiko@sntech.de>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rockchip@lists.infradead.org, robh+dt@kernel.org,
mark.rutland@arm.com, linux@armlinux.org.uk,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/4] include: dt-bindings: Add more GPIO bank and index definition for rockchip pinctrl
Date: Mon, 5 Sep 2016 18:22:46 +0800 [thread overview]
Message-ID: <0dbac4cd-7f67-5052-16e6-463896bf5929@rock-chips.com> (raw)
In-Reply-To: <3544284.90s5nXbktR@phil>
Hi Heiko:
On 2016年09月05日 17:33, Heiko Stuebner wrote:
> Hi Andy,
>
> Am Sonntag, 4. September 2016, 16:35:30 CEST schrieb Andy Yan:
>> There are 8 gpio banks on RK3288, so add the missing
>> RK_GPIO7 and RK_GPIO8. Also add gpio index definition
>> to make it easier to description GPIO in dts.
>>
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> I tend to disagree here.
>
> I consider the bank defines RK_GPIOx to be deprecated and highly discourage
> them being used in new boards. They only encode the same number again (2 ->
> RK_GPIO2 etc) and therefore don't bring any useful addition over using the
> bank number directly.
>
> Slightly similar argument for the per-pin defines. While the external pins are
> described in the A/B/C/Dx notation for pinmux purposes, the gpio controllers
> on top use a regular 0-31 numbering. Also you cannot name constants generic
> GPIO_x, simply because that may conflict with other overly generic constant
> names.
>
> So I'm not yet convinced that these improve readability, but they would
> definitly need a RK_* prefix to make them specific.
>
>
> Heiko
I consider for a people who doesn't familiar with rockchip
pinctrl, He
may don't know what does these number 2/4/17....stands for in the dts
like
rockchip,pins = <2 17 RK_FUNC_GPIO &pcfg_pull_up>.
But if we use meaningful macro here like : rockchip,pins =
<RK_GPIO2 RK_GPIO_C0 RK_FUNC_GPIO &pcfg_pull_up>, I think it easier to
tell people this GPIO is GPIO_C0 of BNAK2.
Especially all the GPIOs in rockchip based schematic are described as
GPIOn_A/B/C/Dx. And it is also easier for people to directly use a macro
stands for 0~32 than translating A/B/C/Dx from the schematic to 0~32.
>> ---
>>
>> include/dt-bindings/pinctrl/rockchip.h | 35
>> ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
>>
>> diff --git a/include/dt-bindings/pinctrl/rockchip.h
>> b/include/dt-bindings/pinctrl/rockchip.h index 743e66a..fd35350 100644
>> --- a/include/dt-bindings/pinctrl/rockchip.h
>> +++ b/include/dt-bindings/pinctrl/rockchip.h
>> @@ -24,6 +24,41 @@
>> #define RK_GPIO3 3
>> #define RK_GPIO4 4
>> #define RK_GPIO6 6
>> +#define RK_GPIO7 7
>> +#define RK_GPIO8 8
>> +
>> +#define GPIO_A0 0
>> +#define GPIO_A1 1
>> +#define GPIO_A2 2
>> +#define GPIO_A3 3
>> +#define GPIO_A4 4
>> +#define GPIO_A5 5
>> +#define GPIO_A6 6
>> +#define GPIO_A7 7
>> +#define GPIO_B0 8
>> +#define GPIO_B1 9
>> +#define GPIO_B2 10
>> +#define GPIO_B3 11
>> +#define GPIO_B4 12
>> +#define GPIO_B5 13
>> +#define GPIO_B6 14
>> +#define GPIO_B7 15
>> +#define GPIO_C0 16
>> +#define GPIO_C1 17
>> +#define GPIO_C2 18
>> +#define GPIO_C3 19
>> +#define GPIO_C4 20
>> +#define GPIO_C5 21
>> +#define GPIO_C6 22
>> +#define GPIO_C7 23
>> +#define GPIO_D0 24
>> +#define GPIO_D1 25
>> +#define GPIO_D2 26
>> +#define GPIO_D3 27
>> +#define GPIO_D4 28
>> +#define GPIO_D5 29
>> +#define GPIO_D6 30
>> +#define GPIO_D7 31
>>
>> #define RK_FUNC_GPIO 0
>> #define RK_FUNC_1 1
>
>
>
>
WARNING: multiple messages have this Message-ID (diff)
From: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
To: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 3/4] include: dt-bindings: Add more GPIO bank and index definition for rockchip pinctrl
Date: Mon, 5 Sep 2016 18:22:46 +0800 [thread overview]
Message-ID: <0dbac4cd-7f67-5052-16e6-463896bf5929@rock-chips.com> (raw)
In-Reply-To: <3544284.90s5nXbktR@phil>
Hi Heiko:
On 2016年09月05日 17:33, Heiko Stuebner wrote:
> Hi Andy,
>
> Am Sonntag, 4. September 2016, 16:35:30 CEST schrieb Andy Yan:
>> There are 8 gpio banks on RK3288, so add the missing
>> RK_GPIO7 and RK_GPIO8. Also add gpio index definition
>> to make it easier to description GPIO in dts.
>>
>> Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> I tend to disagree here.
>
> I consider the bank defines RK_GPIOx to be deprecated and highly discourage
> them being used in new boards. They only encode the same number again (2 ->
> RK_GPIO2 etc) and therefore don't bring any useful addition over using the
> bank number directly.
>
> Slightly similar argument for the per-pin defines. While the external pins are
> described in the A/B/C/Dx notation for pinmux purposes, the gpio controllers
> on top use a regular 0-31 numbering. Also you cannot name constants generic
> GPIO_x, simply because that may conflict with other overly generic constant
> names.
>
> So I'm not yet convinced that these improve readability, but they would
> definitly need a RK_* prefix to make them specific.
>
>
> Heiko
I consider for a people who doesn't familiar with rockchip
pinctrl, He
may don't know what does these number 2/4/17....stands for in the dts
like
rockchip,pins = <2 17 RK_FUNC_GPIO &pcfg_pull_up>.
But if we use meaningful macro here like : rockchip,pins =
<RK_GPIO2 RK_GPIO_C0 RK_FUNC_GPIO &pcfg_pull_up>, I think it easier to
tell people this GPIO is GPIO_C0 of BNAK2.
Especially all the GPIOs in rockchip based schematic are described as
GPIOn_A/B/C/Dx. And it is also easier for people to directly use a macro
stands for 0~32 than translating A/B/C/Dx from the schematic to 0~32.
>> ---
>>
>> include/dt-bindings/pinctrl/rockchip.h | 35
>> ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
>>
>> diff --git a/include/dt-bindings/pinctrl/rockchip.h
>> b/include/dt-bindings/pinctrl/rockchip.h index 743e66a..fd35350 100644
>> --- a/include/dt-bindings/pinctrl/rockchip.h
>> +++ b/include/dt-bindings/pinctrl/rockchip.h
>> @@ -24,6 +24,41 @@
>> #define RK_GPIO3 3
>> #define RK_GPIO4 4
>> #define RK_GPIO6 6
>> +#define RK_GPIO7 7
>> +#define RK_GPIO8 8
>> +
>> +#define GPIO_A0 0
>> +#define GPIO_A1 1
>> +#define GPIO_A2 2
>> +#define GPIO_A3 3
>> +#define GPIO_A4 4
>> +#define GPIO_A5 5
>> +#define GPIO_A6 6
>> +#define GPIO_A7 7
>> +#define GPIO_B0 8
>> +#define GPIO_B1 9
>> +#define GPIO_B2 10
>> +#define GPIO_B3 11
>> +#define GPIO_B4 12
>> +#define GPIO_B5 13
>> +#define GPIO_B6 14
>> +#define GPIO_B7 15
>> +#define GPIO_C0 16
>> +#define GPIO_C1 17
>> +#define GPIO_C2 18
>> +#define GPIO_C3 19
>> +#define GPIO_C4 20
>> +#define GPIO_C5 21
>> +#define GPIO_C6 22
>> +#define GPIO_C7 23
>> +#define GPIO_D0 24
>> +#define GPIO_D1 25
>> +#define GPIO_D2 26
>> +#define GPIO_D3 27
>> +#define GPIO_D4 28
>> +#define GPIO_D5 29
>> +#define GPIO_D6 30
>> +#define GPIO_D7 31
>>
>> #define RK_FUNC_GPIO 0
>> #define RK_FUNC_1 1
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: andy.yan@rock-chips.com (Andy Yan)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/4] include: dt-bindings: Add more GPIO bank and index definition for rockchip pinctrl
Date: Mon, 5 Sep 2016 18:22:46 +0800 [thread overview]
Message-ID: <0dbac4cd-7f67-5052-16e6-463896bf5929@rock-chips.com> (raw)
In-Reply-To: <3544284.90s5nXbktR@phil>
Hi Heiko:
On 2016?09?05? 17:33, Heiko Stuebner wrote:
> Hi Andy,
>
> Am Sonntag, 4. September 2016, 16:35:30 CEST schrieb Andy Yan:
>> There are 8 gpio banks on RK3288, so add the missing
>> RK_GPIO7 and RK_GPIO8. Also add gpio index definition
>> to make it easier to description GPIO in dts.
>>
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> I tend to disagree here.
>
> I consider the bank defines RK_GPIOx to be deprecated and highly discourage
> them being used in new boards. They only encode the same number again (2 ->
> RK_GPIO2 etc) and therefore don't bring any useful addition over using the
> bank number directly.
>
> Slightly similar argument for the per-pin defines. While the external pins are
> described in the A/B/C/Dx notation for pinmux purposes, the gpio controllers
> on top use a regular 0-31 numbering. Also you cannot name constants generic
> GPIO_x, simply because that may conflict with other overly generic constant
> names.
>
> So I'm not yet convinced that these improve readability, but they would
> definitly need a RK_* prefix to make them specific.
>
>
> Heiko
I consider for a people who doesn't familiar with rockchip
pinctrl, He
may don't know what does these number 2/4/17....stands for in the dts
like
rockchip,pins = <2 17 RK_FUNC_GPIO &pcfg_pull_up>.
But if we use meaningful macro here like : rockchip,pins =
<RK_GPIO2 RK_GPIO_C0 RK_FUNC_GPIO &pcfg_pull_up>, I think it easier to
tell people this GPIO is GPIO_C0 of BNAK2.
Especially all the GPIOs in rockchip based schematic are described as
GPIOn_A/B/C/Dx. And it is also easier for people to directly use a macro
stands for 0~32 than translating A/B/C/Dx from the schematic to 0~32.
>> ---
>>
>> include/dt-bindings/pinctrl/rockchip.h | 35
>> ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
>>
>> diff --git a/include/dt-bindings/pinctrl/rockchip.h
>> b/include/dt-bindings/pinctrl/rockchip.h index 743e66a..fd35350 100644
>> --- a/include/dt-bindings/pinctrl/rockchip.h
>> +++ b/include/dt-bindings/pinctrl/rockchip.h
>> @@ -24,6 +24,41 @@
>> #define RK_GPIO3 3
>> #define RK_GPIO4 4
>> #define RK_GPIO6 6
>> +#define RK_GPIO7 7
>> +#define RK_GPIO8 8
>> +
>> +#define GPIO_A0 0
>> +#define GPIO_A1 1
>> +#define GPIO_A2 2
>> +#define GPIO_A3 3
>> +#define GPIO_A4 4
>> +#define GPIO_A5 5
>> +#define GPIO_A6 6
>> +#define GPIO_A7 7
>> +#define GPIO_B0 8
>> +#define GPIO_B1 9
>> +#define GPIO_B2 10
>> +#define GPIO_B3 11
>> +#define GPIO_B4 12
>> +#define GPIO_B5 13
>> +#define GPIO_B6 14
>> +#define GPIO_B7 15
>> +#define GPIO_C0 16
>> +#define GPIO_C1 17
>> +#define GPIO_C2 18
>> +#define GPIO_C3 19
>> +#define GPIO_C4 20
>> +#define GPIO_C5 21
>> +#define GPIO_C6 22
>> +#define GPIO_C7 23
>> +#define GPIO_D0 24
>> +#define GPIO_D1 25
>> +#define GPIO_D2 26
>> +#define GPIO_D3 27
>> +#define GPIO_D4 28
>> +#define GPIO_D5 29
>> +#define GPIO_D6 30
>> +#define GPIO_D7 31
>>
>> #define RK_FUNC_GPIO 0
>> #define RK_FUNC_1 1
>
>
>
>
next prev parent reply other threads:[~2016-09-05 10:22 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-04 8:32 [PATCH 0/4] One fix and some improvements for RK3288 Popmetal board Andy Yan
2016-09-04 8:32 ` Andy Yan
2016-09-04 8:32 ` Andy Yan
2016-09-04 8:33 ` [PATCH 1/4] ARM: dts: rockchip: enable usbotg for Popemtal-rk3288 board Andy Yan
2016-09-04 8:33 ` Andy Yan
2016-09-04 8:33 ` Andy Yan
2016-09-05 8:52 ` Heiko Stuebner
2016-09-05 8:52 ` Heiko Stuebner
2016-09-04 8:34 ` [PATCH 2/4] ARM: dts: rockchip: fix i2c address L3G4200D on PopMetal-RK3288 board Andy Yan
2016-09-04 8:34 ` Andy Yan
2016-09-04 8:34 ` Andy Yan
2016-09-05 9:12 ` Heiko Stuebner
2016-09-05 9:12 ` Heiko Stuebner
2016-09-05 9:12 ` Heiko Stuebner
2016-09-05 9:26 ` Andy Yan
2016-09-05 9:26 ` Andy Yan
2016-09-05 9:26 ` Andy Yan
2016-09-04 8:35 ` [PATCH 3/4] include: dt-bindings: Add more GPIO bank and index definition for rockchip pinctrl Andy Yan
2016-09-04 8:35 ` Andy Yan
2016-09-04 8:35 ` Andy Yan
2016-09-05 9:33 ` Heiko Stuebner
2016-09-05 9:33 ` Heiko Stuebner
2016-09-05 10:22 ` Andy Yan [this message]
2016-09-05 10:22 ` Andy Yan
2016-09-05 10:22 ` Andy Yan
2016-09-05 22:10 ` Heiko Stuebner
2016-09-05 22:10 ` Heiko Stuebner
2016-09-06 9:33 ` Andy Yan
2016-09-06 9:33 ` Andy Yan
2016-09-04 8:36 ` [PATCH 4/4] ARM: dts: use definition in rockchip pinctrl header to describe gpios on Popmetal-RK3288 Andy Yan
2016-09-04 8:36 ` Andy Yan
2016-09-04 8:36 ` Andy Yan
2016-09-04 12:04 ` kbuild test robot
2016-09-04 12:04 ` kbuild test robot
2016-09-04 12:04 ` kbuild test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0dbac4cd-7f67-5052-16e6-463896bf5929@rock-chips.com \
--to=andy.yan@rock-chips.com \
--cc=devicetree@vger.kernel.org \
--cc=heiko@sntech.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux@armlinux.org.uk \
--cc=mark.rutland@arm.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.