Re: [linux-sunxi] [PATCH v2 2/6] pinctrl: sunxi: add support for the Allwinner H6 main pin controller

2018-02-12 Thread André Przywara 
Hi,

On 03/02/18 15:49, Icenowy Zheng wrote:
> The Allwinner H6 SoC has two pin controllers, one main controller
> (called CPUX-PORT in user manual) and one controller in CPUs power
> domain (called CPUS-PORT in user manual).

Leaving aside that I don't like this approach of stashing yet another
pile of data in the kernel ...

> This commit introduces support for the main pin controller on H6.

So I went through *every* single pin of port C, D, F, G and H and
compared your entries with the manual. Quite impressively, I couldn't
find any issues. Well done!

> The pin bank A and B are not wired out and hidden from the SoC's
> documents, however it's shown that the "ATE" (an AC200 chip
> co-packaged with the H6 die) is connected to the main SoC die via these
> pin banks. The information about these banks is just copied from the BSP
> pinctrl driver, but re-formatted to fit the mainline pinctrl driver
> format.

See below ...

> Signed-off-by: Icenowy Zheng 
> ---
> Changes in v2:
> - Dropped without_bus_gate description.
> - Switched to SPDX license identifier.
> 
>  .../bindings/pinctrl/allwinner,sunxi-pinctrl.txt   |   1 +
>  drivers/pinctrl/sunxi/Kconfig  |   4 +
>  drivers/pinctrl/sunxi/Makefile |   1 +
>  drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c  | 676 
> +
>  4 files changed, 682 insertions(+)
>  create mode 100644 drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> 
> diff --git 
> a/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt 
> b/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt
> index 09789fdfa749..ed5eb547afc8 100644
> --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt
> @@ -27,6 +27,7 @@ Required properties:
>"allwinner,sun50i-a64-pinctrl"
>"allwinner,sun50i-a64-r-pinctrl"
>"allwinner,sun50i-h5-pinctrl"
> +  "allwinner,sun50i-h6-pinctrl"
>"nextthing,gr8-pinctrl"
>  
>  - reg: Should contain the register physical address and length for the
> diff --git a/drivers/pinctrl/sunxi/Kconfig b/drivers/pinctrl/sunxi/Kconfig
> index bfce99d86dfc..5de1f63b07bb 100644
> --- a/drivers/pinctrl/sunxi/Kconfig
> +++ b/drivers/pinctrl/sunxi/Kconfig
> @@ -77,4 +77,8 @@ config PINCTRL_SUN50I_H5
>   def_bool ARM64 && ARCH_SUNXI
>   select PINCTRL_SUNXI
>  
> +config PINCTRL_SUN50I_H6
> + def_bool ARM64 && ARCH_SUNXI
> + select PINCTRL_SUNXI
> +
>  endif
> diff --git a/drivers/pinctrl/sunxi/Makefile b/drivers/pinctrl/sunxi/Makefile
> index 12a752e836ef..3c4aec6611e9 100644
> --- a/drivers/pinctrl/sunxi/Makefile
> +++ b/drivers/pinctrl/sunxi/Makefile
> @@ -18,5 +18,6 @@ obj-$(CONFIG_PINCTRL_SUN8I_H3)  += 
> pinctrl-sun8i-h3.o
>  obj-$(CONFIG_PINCTRL_SUN8I_H3_R) += pinctrl-sun8i-h3-r.o
>  obj-$(CONFIG_PINCTRL_SUN8I_V3S)  += pinctrl-sun8i-v3s.o
>  obj-$(CONFIG_PINCTRL_SUN50I_H5)  += pinctrl-sun50i-h5.o
> +obj-$(CONFIG_PINCTRL_SUN50I_H6)  += pinctrl-sun50i-h6.o
>  obj-$(CONFIG_PINCTRL_SUN9I_A80)  += pinctrl-sun9i-a80.o
>  obj-$(CONFIG_PINCTRL_SUN9I_A80_R)+= pinctrl-sun9i-a80-r.o
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c 
> b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> new file mode 100644
> index ..f1f728e2f6d3
> --- /dev/null
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
> @@ -0,0 +1,676 @@
> +/*
> + * Allwinner H6 SoC pinctrl driver.
> + *
> + * Copyright (C) 2017 Icenowy Zheng 
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pinctrl-sunxi.h"
> +
> +static const struct sunxi_desc_pin h6_pins[] = {
> + SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 0),
> +   SUNXI_FUNCTION(0x0, "gpio_in"),
> +   SUNXI_FUNCTION(0x1, "gpio_out"),

So why is this? If those pins are hardwired to the ATE's EMAC port, why
do we have GPIOs here? Is that actually possible to configure these as
an input/output, then bitbanging those pins?
And even if that would be possible, what would be the reason for doing
so? Doesn't naming gpio_in and gpio_out here make them appear as
exportable GPIOs in sysfs (and other GPIO interfaces)? I don't think
this is desirable?
>From dumping the MMIO registers in U-Boot I see that those pins reset to
0x7 (disabled), so we need to expose function 0x2 here to wire them
through to the ATE. But I don't believe that having function 0x0 and 0x1
here is useful.

Cheers,
Andre.

> +   SUNXI_FUNCTION(0x2, "emac")), /* ERXD1 */
> + SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 1),
> +   SUNXI_FUNCTION(0x0, "gpio_in"),
> +   SUNXI_FUNCTION(0x1, "gpio_out"),
> +   SUNXI_FUNCTION(0x2, "emac")), /* ERXD0 */
> + SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 2),
> +   SUNXI_FUNCTION(0x0, "gpio_in"),
> +

[linux-sunxi] [PATCH v2 2/6] pinctrl: sunxi: add support for the Allwinner H6 main pin controller

2018-02-03 Thread Icenowy Zheng 
The Allwinner H6 SoC has two pin controllers, one main controller
(called CPUX-PORT in user manual) and one controller in CPUs power
domain (called CPUS-PORT in user manual).

This commit introduces support for the main pin controller on H6.

The pin bank A and B are not wired out and hidden from the SoC's
documents, however it's shown that the "ATE" (an AC200 chip
co-packaged with the H6 die) is connected to the main SoC die via these
pin banks. The information about these banks is just copied from the BSP
pinctrl driver, but re-formatted to fit the mainline pinctrl driver
format.

Signed-off-by: Icenowy Zheng 
---
Changes in v2:
- Dropped without_bus_gate description.
- Switched to SPDX license identifier.

 .../bindings/pinctrl/allwinner,sunxi-pinctrl.txt   |   1 +
 drivers/pinctrl/sunxi/Kconfig  |   4 +
 drivers/pinctrl/sunxi/Makefile |   1 +
 drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c  | 676 +
 4 files changed, 682 insertions(+)
 create mode 100644 drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c

diff --git 
a/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt 
b/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt
index 09789fdfa749..ed5eb547afc8 100644
--- a/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sunxi-pinctrl.txt
@@ -27,6 +27,7 @@ Required properties:
   "allwinner,sun50i-a64-pinctrl"
   "allwinner,sun50i-a64-r-pinctrl"
   "allwinner,sun50i-h5-pinctrl"
+  "allwinner,sun50i-h6-pinctrl"
   "nextthing,gr8-pinctrl"
 
 - reg: Should contain the register physical address and length for the
diff --git a/drivers/pinctrl/sunxi/Kconfig b/drivers/pinctrl/sunxi/Kconfig
index bfce99d86dfc..5de1f63b07bb 100644
--- a/drivers/pinctrl/sunxi/Kconfig
+++ b/drivers/pinctrl/sunxi/Kconfig
@@ -77,4 +77,8 @@ config PINCTRL_SUN50I_H5
def_bool ARM64 && ARCH_SUNXI
select PINCTRL_SUNXI
 
+config PINCTRL_SUN50I_H6
+   def_bool ARM64 && ARCH_SUNXI
+   select PINCTRL_SUNXI
+
 endif
diff --git a/drivers/pinctrl/sunxi/Makefile b/drivers/pinctrl/sunxi/Makefile
index 12a752e836ef..3c4aec6611e9 100644
--- a/drivers/pinctrl/sunxi/Makefile
+++ b/drivers/pinctrl/sunxi/Makefile
@@ -18,5 +18,6 @@ obj-$(CONFIG_PINCTRL_SUN8I_H3)+= 
pinctrl-sun8i-h3.o
 obj-$(CONFIG_PINCTRL_SUN8I_H3_R)   += pinctrl-sun8i-h3-r.o
 obj-$(CONFIG_PINCTRL_SUN8I_V3S)+= pinctrl-sun8i-v3s.o
 obj-$(CONFIG_PINCTRL_SUN50I_H5)+= pinctrl-sun50i-h5.o
+obj-$(CONFIG_PINCTRL_SUN50I_H6)+= pinctrl-sun50i-h6.o
 obj-$(CONFIG_PINCTRL_SUN9I_A80)+= pinctrl-sun9i-a80.o
 obj-$(CONFIG_PINCTRL_SUN9I_A80_R)  += pinctrl-sun9i-a80-r.o
diff --git a/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c 
b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
new file mode 100644
index ..f1f728e2f6d3
--- /dev/null
+++ b/drivers/pinctrl/sunxi/pinctrl-sun50i-h6.c
@@ -0,0 +1,676 @@
+/*
+ * Allwinner H6 SoC pinctrl driver.
+ *
+ * Copyright (C) 2017 Icenowy Zheng 
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "pinctrl-sunxi.h"
+
+static const struct sunxi_desc_pin h6_pins[] = {
+   SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 0),
+ SUNXI_FUNCTION(0x0, "gpio_in"),
+ SUNXI_FUNCTION(0x1, "gpio_out"),
+ SUNXI_FUNCTION(0x2, "emac")), /* ERXD1 */
+   SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 1),
+ SUNXI_FUNCTION(0x0, "gpio_in"),
+ SUNXI_FUNCTION(0x1, "gpio_out"),
+ SUNXI_FUNCTION(0x2, "emac")), /* ERXD0 */
+   SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 2),
+ SUNXI_FUNCTION(0x0, "gpio_in"),
+ SUNXI_FUNCTION(0x1, "gpio_out"),
+ SUNXI_FUNCTION(0x2, "emac")), /* ECRS_DV */
+   SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 3),
+ SUNXI_FUNCTION(0x0, "gpio_in"),
+ SUNXI_FUNCTION(0x1, "gpio_out"),
+ SUNXI_FUNCTION(0x2, "emac")), /* ERXERR */
+   SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 4),
+ SUNXI_FUNCTION(0x0, "gpio_in"),
+ SUNXI_FUNCTION(0x1, "gpio_out"),
+ SUNXI_FUNCTION(0x2, "emac")), /* ETXD1 */
+   SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 5),
+ SUNXI_FUNCTION(0x0, "gpio_in"),
+ SUNXI_FUNCTION(0x1, "gpio_out"),
+ SUNXI_FUNCTION(0x2, "emac")), /* ETXD0 */
+   SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 6),
+ SUNXI_FUNCTION(0x0, "gpio_in"),
+ SUNXI_FUNCTION(0x1, "gpio_out"),
+ SUNXI_FUNCTION(0x2, "emac")), /* ETXCK */
+   SUNXI_PIN(SUNXI_PINCTRL_PIN(A, 7),
+ SUNXI_FUNCTION(0x0, "gpio_in"),
+ SUNXI_FUNCTION(0x1, "gpio_out"),
+ SUNXI_FUNCTION(0x2,