Rob Herring <robh+dt at kernel.org> writes:
> On Thu, May 31, 2018 at 9:05 PM, Levin <djw at 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 at t-chip.com.cn> wrote:
>>>>
>>>> From: Levin Du <djw at 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 at 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 at 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 at 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 at ff220000 {
//...
};
gpio2: gpio2 at ff230000 {
//...
};
gpio3: gpio3 at 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
