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. >> + compatible = "rockchip,rk3328-gpio-mute"; >> + gpio-controller; >> + #gpio-cells = <2>; >> + }; >> + }; >> + >> +Note: The gpio_mute node should be declared as the child of the GRF (General >> +Register File) node. The GPIO_MUTE pin is referred to as <&gpio_mute 0>. > This is wrong because you should have 2 cells. The phandle doesn't > count as a cell. > > Rob > Thanks for pointing that out. So it should be: ?? The GPIO_MUTE pin is referred to as <&gpio_mute 0 POLARITY>. Thanks, Levin