LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Levin Du <djw@t-chip.com.cn>
To: Rob Herring <robh+dt@kernel.org>
Cc: "open list\:ARM\/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	Wayne Chou <zxf@t-chip.com.cn>, Heiko Stuebner <heiko@sntech.de>,
	devicetree@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"open list\:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	"moderated list\:ARM\/FREESCALE IMX \/ MXC ARM ARCHITECTURE"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 2/5] gpio: syscon: rockchip: add GPIO_MUTE support for rk3328
Date: Sat, 02 Jun 2018 16:40:09 +0800	[thread overview]
Message-ID: <87zi0d8tue.fsf@t-chip.com.cn> (raw)
In-Reply-To: <CAL_JsqKSy4q5dfVfXbRMLa7koTWufpAt3yNdE-9mwyJ-VzVQRQ@mail.gmail.com>


Rob Herring <robh+dt@kernel.org> writes:

> On Thu, May 31, 2018 at 9:05 PM, Levin <djw@t-chip.com.cn> 
> wrote:
>> Hi Rob,
>>
>>
>> On 2018-05-31 10:45 PM, Rob Herring wrote:
>>>
>>> On Wed, May 30, 2018 at 10:27 PM,  <djw@t-chip.com.cn> wrote:
>>>>
>>>> From: Levin Du <djw@t-chip.com.cn>
>>>>
>>>> In Rockchip RK3328, the output only GPIO_MUTE pin, originally 
>>>> for codec
>>>> mute control, can also be used for general purpose. It is 
>>>> manipulated by
>>>> the GRF_SOC_CON10 register.
>>>>
>>>> Signed-off-by: Levin Du <djw@t-chip.com.cn>
>>>>
>>>> ---
>>>>
>>>> Changes in v3:
>>>> - Change from general gpio-syscon to specific 
>>>> rk3328-gpio-mute
>>>>
>>>> Changes in v2:
>>>> - Rename gpio_syscon10 to gpio_mute in doc
>>>>
>>>> Changes in v1:
>>>> - Refactured for general gpio-syscon usage for Rockchip SoCs.
>>>> - Add doc rockchip,gpio-syscon.txt
>>>>
>>>>   .../bindings/gpio/rockchip,rk3328-gpio-mute.txt    | 28
>>>> +++++++++++++++++++
>>>>   drivers/gpio/gpio-syscon.c                         | 31
>>>> ++++++++++++++++++++++
>>>>   2 files changed, 59 insertions(+)
>>>>   create mode 100644
>>>> Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
>>>> b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
>>>> new file mode 100644
>>>> index 0000000..10bc632
>>>> --- /dev/null
>>>> +++
>>>> b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
>>>> @@ -0,0 +1,28 @@
>>>> +Rockchip RK3328 GPIO controller dedicated for the GPIO_MUTE 
>>>> pin.
>>>> +
>>>> +In Rockchip RK3328, the output only GPIO_MUTE pin, 
>>>> originally for codec
>>>> mute
>>>> +control, can also be used for general purpose. It is 
>>>> manipulated by the
>>>> +GRF_SOC_CON10 register.
>>>> +
>>>> +Required properties:
>>>> +- compatible: Should contain "rockchip,rk3328-gpio-mute".
>>>> +- gpio-controller: Marks the device node as a gpio 
>>>> controller.
>>>> +- #gpio-cells: Should be 2. The first cell is the pin number 
>>>> and
>>>> +  the second cell is used to specify the gpio polarity:
>>>> +    0 = Active high,
>>>> +    1 = Active low.
>>>> +
>>>> +Example:
>>>> +
>>>> +       grf: syscon@ff100000 {
>>>> +               compatible = "rockchip,rk3328-grf", "syscon",
>>>> "simple-mfd";
>>>> +
>>>> +               gpio_mute: gpio-mute {
>>>
>>> Node names should be generic:
>>>
>>> gpio {
>>>
>>> This also means you can't add another GPIO node in the future 
>>> and
>>> you'll have to live with "rockchip,rk3328-gpio-mute" covering 
>>> more
>>> than 1 GPIO if you do need to add more GPIOs.
>>
>>
>> As the first line describes, this GPIO controller is dedicated 
>> for the
>> GPIO_MUTE pin.
>> There's only one GPIO pin in the GRF_SOC_CON10 register. 
>> Therefore the
>> gpio_mute
>> name is proper IMHO.
>
> It's how many GPIOs in the GRF, not this register. What I'm 
> saying is
> when you come along later to add another GPIO in the GRF, you 
> had
> better just add it to this same node. I'm not going to accept 
> another
> GPIO controller node within the GRF. You have the cells to 
> support
> more than 1, so it would only be a driver change. The compatible
> string would then not be ideally named at that point. But 
> compatible
> strings are just unique identifiers, so it doesn't really matter 
> what
> the string is.
>

I'll try my best to introduce the situation here. The GRF, 
GPIO0~GPIO3
are register blocks in the RK3328 Soc. The GPIO0~GPIO3 contain 
registers
for GPIO operations like reading/writing data, setting direction,
interruption etc, which corresponds to the GPIO banks 
(gpio0~gpio3)
defined in rk3328.dtsi:

	pinctrl: pinctrl {
		compatible = "rockchip,rk3328-pinctrl";
		rockchip,grf = <&grf>;
		#address-cells = <2>;
		#size-cells = <2>;
		ranges;

		gpio0: gpio0@ff210000 {
			compatible = "rockchip,gpio-bank";
			reg = <0x0 0xff210000 0x0 0x100>;
			interrupts = <GIC_SPI 51 
			IRQ_TYPE_LEVEL_HIGH>;
			clocks = <&cru PCLK_GPIO0>;

			gpio-controller;
			#gpio-cells = <2>;

			interrupt-controller;
			#interrupt-cells = <2>;
		};

		gpio1: gpio1@ff220000 {
                //...
		};

		gpio2: gpio2@ff230000 {
                //...
		};

		gpio3: gpio3@ff240000 {
                //...
		};
         }

However, these general GPIO pins has multiplexed functions and 
their
pull up/down and driving strength can also be configured. These 
settings
are manipulated by the GRF registers in pinctrl driver. Quoted 
from the
TRM, the GRF has the following function:

 - IOMUX control
 - Control the state of GPIO in power-down mode
 - GPIO PAD pull down and pull up control
 - Used for common system control
 - Used to record the system state

Therefore the functions of the GRF are messy and scattered in 
different
nodes. The so-called GPIO_MUTE does not belong to GPIO0~GPIO3. It 
is
manipulated by the GRF_SOC_CON10 register in the GRF block.

> I'm being told both "this is the only GPIO" and "the GRF has too 
> many
> different functions for us to tell you what they all are". So 
> which is
> it?
>
> Rob

They are both true, but lack of context. See the above 
description.

Thanks,
Levin

  reply	other threads:[~2018-06-02  8:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-31  3:27 [PATCH v3 0/5] Add sdmmc UHS support to ROC-RK3328-CC board djw
2018-05-31  3:27 ` [PATCH v3 1/5] gpio: syscon: allow fetching syscon from parent node djw
2018-06-08  7:54   ` Linus Walleij
2018-05-31  3:27 ` [PATCH v3 2/5] gpio: syscon: rockchip: add GPIO_MUTE support for rk3328 djw
2018-05-31 14:45   ` Rob Herring
2018-06-01  2:05     ` Levin
2018-06-01 17:03       ` Rob Herring
2018-06-02  8:40         ` Levin Du [this message]
2018-06-05 19:58           ` Rob Herring
2018-06-07  1:32             ` Levin
2018-06-28  7:22               ` djw
2018-07-06 21:10                 ` Rob Herring
2018-05-31  3:27 ` [PATCH v3 3/5] arm64: dts: rockchip: Add GPIO_MUTE pin support to rk3328 djw
2018-05-31  3:27 ` [PATCH v3 4/5] arm64: dts: rockchip: Add io-domain to roc-rk3328-cc djw
2018-05-31  5:03 ` [PATCH v3 5/5] arm64: dts: rockchip: Add sdmmc UHS support for roc-rk3328-cc djw

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=87zi0d8tue.fsf@t-chip.com.cn \
    --to=djw@t-chip.com.cn \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=zxf@t-chip.com.cn \
    --subject='Re: [PATCH v3 2/5] gpio: syscon: rockchip: add GPIO_MUTE support for rk3328' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).