Rob Herring [off-list ref] writes:

On Thu, May 31, 2018 at 9:05 PM, Levin [off-list ref] 
wrote:

quoted

Hi Rob,


On 2018-05-31 10:45 PM, Rob Herring wrote:

quoted

On Wed, May 30, 2018 at 10:27 PM,  [off-list ref] wrote:

quoted

From: Levin Du <redacted>

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 <redacted>

---

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