[PATCH 0/9] Allwinner V3 SL631 Action Camera Support and Related Fixes

[PATCH 0/9] Allwinner V3 SL631 Action Camera Support and Related Fixes

[Paul Kocialkowski]

This series adds support for the Allwinner V3-based SL631 family of
Action Cameras, starting with the IMX179 fashion.

A few fixes to V3 support are added along the way, most notably support
for the NMI IRQ controller which is necessary for the AXP209 IRQ.

Note that some patches in this series may have already been submitted
(but not yet merged) by others and are included for the series to build.

Happy reviewing!

Paul Kocialkowski (9):
  ARM: sunxi: Add machine match for the Allwinner V3 SoC
  ARM: dts: sun8i-v3: Add UART1 PG pins description
  ARM: dts: sun8i-v3s: Add I2C1 PB pins description
  dt-bindings: irq: sun7i-nmi: Add binding for the V3s NMI
  irqchip/sunxi-nmi: Add support for the V3s NMI
  ARM: dts: sun8i-v3s: Add the V3s NMI IRQ controller
  ARM: dts: sun8i: Cleanup the Pinecube AXP209 node
  dt-bindings: arm: sunxi: Add SL631 with IMX179 bindings
  ARM: dts: sun8i-v3: Add support for the SL631 Action Camera with
    IMX179

 .../devicetree/bindings/arm/sunxi.yaml        |   6 +
 .../allwinner,sun7i-a20-sc-nmi.yaml           |   1 +
 arch/arm/boot/dts/Makefile                    |   1 +
 arch/arm/boot/dts/sun8i-s3-pinecube.dts       |   8 +-
 arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts   |  12 ++
 arch/arm/boot/dts/sun8i-v3-sl631.dtsi         | 145 ++++++++++++++++++
 arch/arm/boot/dts/sun8i-v3.dtsi               |   6 +
 arch/arm/boot/dts/sun8i-v3s.dtsi              |  16 +-
 arch/arm/mach-sunxi/sunxi.c                   |   1 +
 drivers/irqchip/irq-sunxi-nmi.c               |  18 ++-
 10 files changed, 206 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
 create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631.dtsi

-- 
2.28.0

[PATCH 1/9] ARM: sunxi: Add machine match for the Allwinner V3 SoC

[Paul Kocialkowski]

The Allwinner V3 SoC shares the same base as the V3s but comes with
extra pins and features available. As a result, it has its dedicated
compatible string (already used in device trees), which is added here.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/mach-sunxi/sunxi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
index 06da2747a90b..19635721013d 100644
--- a/arch/arm/mach-sunxi/sunxi.c
+++ b/arch/arm/mach-sunxi/sunxi.c
@@ -66,6 +66,7 @@ static const char * const sun8i_board_dt_compat[] = {
 	"allwinner,sun8i-h2-plus",
 	"allwinner,sun8i-h3",
 	"allwinner,sun8i-r40",
+	"allwinner,sun8i-v3",
 	"allwinner,sun8i-v3s",
 	NULL,
 };
-- 
2.28.0

[PATCH 2/9] ARM: dts: sun8i-v3: Add UART1 PG pins description

[Paul Kocialkowski]

UART1 is often exposed through the PG pins (usually for the debug
console) on the V3. They are not available on V3s.

Describe these pins in device-tree.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/sun8i-v3.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-v3.dtsi b/arch/arm/boot/dts/sun8i-v3.dtsi
index ca4672ed2e02..edf48e0471ad 100644
--- a/arch/arm/boot/dts/sun8i-v3.dtsi
+++ b/arch/arm/boot/dts/sun8i-v3.dtsi
@@ -24,4 +24,10 @@ external_mdio: mdio@2 {
 
 &pio {
 	compatible = "allwinner,sun8i-v3-pinctrl";
+
+	/omit-if-no-ref/
+	uart1_pg_pins: uart1-pg-pins {
+		pins = "PG6", "PG7";
+		function = "uart1";
+	};
 };
-- 
2.28.0

[PATCH 3/9] ARM: dts: sun8i-v3s: Add I2C1 PB pins description

[Paul Kocialkowski]

I2C1 can be exposed through PB pins in addition to PE pins on the V3s.
Add the device-tree description for these pins.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/sun8i-v3s.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi b/arch/arm/boot/dts/sun8i-v3s.dtsi
index 0c7341676921..7b2d684aeb97 100644
--- a/arch/arm/boot/dts/sun8i-v3s.dtsi
+++ b/arch/arm/boot/dts/sun8i-v3s.dtsi
@@ -347,6 +347,12 @@ i2c0_pins: i2c0-pins {
 				function = "i2c0";
 			};
 
+			/omit-if-no-ref/
+			i2c1_pb_pins: i2c1-pb-pins {
+				pins = "PB8", "PB9";
+				function = "i2c1";
+			};
+
 			/omit-if-no-ref/
 			i2c1_pe_pins: i2c1-pe-pins {
 				pins = "PE21", "PE22";
-- 
2.28.0

[PATCH 4/9] dt-bindings: irq: sun7i-nmi: Add binding for the V3s NMI

[Paul Kocialkowski]

The NMI interrupt controller takes a specific compatible for the V3s.
Add it to the device-tree bindings documentation.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 .../interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml         | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
index 8acca0ae3129..52364c2200ef 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
@@ -26,6 +26,7 @@ properties:
       - const: allwinner,sun6i-a31-sc-nmi
         deprecated: true
       - const: allwinner,sun7i-a20-sc-nmi
+      - const: allwinner,sun8i-v3s-sc-nmi
       - items:
           - const: allwinner,sun8i-a83t-r-intc
           - const: allwinner,sun6i-a31-r-intc
-- 
2.28.0

[PATCH 5/9] irqchip/sunxi-nmi: Add support for the V3s NMI

[Paul Kocialkowski]

The V3s/V3 has a NMI IRQ controller, which is mainly used for the AXP209
interrupt. In great wisdom, Allwinner decided to invert the enable and
pending register offsets, compared to the A20.

As a result, a specific compatible and register description is required
for the V3s. This was tested with an AXP209 on a V3 board.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/irqchip/irq-sunxi-nmi.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-sunxi-nmi.c b/drivers/irqchip/irq-sunxi-nmi.c
index a412b5d5d0fa..59e0e4612ef7 100644
--- a/drivers/irqchip/irq-sunxi-nmi.c
+++ b/drivers/irqchip/irq-sunxi-nmi.c
@@ -44,6 +44,10 @@
 #define SUN7I_NMI_PENDING	0x04
 #define SUN7I_NMI_ENABLE	0x08
 
+#define SUN8I_V3S_NMI_CTRL	0x00
+#define SUN8I_V3S_NMI_ENABLE	0x04
+#define SUN8I_V3S_NMI_PENDING	0x08
+
 #define SUN9I_NMI_CTRL		0x00
 #define SUN9I_NMI_ENABLE	0x04
 #define SUN9I_NMI_PENDING	0x08
@@ -79,6 +83,12 @@ static const struct sunxi_sc_nmi_reg_offs sun7i_reg_offs __initconst = {
 	.enable	= SUN7I_NMI_ENABLE,
 };
 
+static const struct sunxi_sc_nmi_reg_offs sun8i_v3s_reg_offs __initconst = {
+	.ctrl	= SUN8I_V3S_NMI_CTRL,
+	.pend	= SUN8I_V3S_NMI_PENDING,
+	.enable	= SUN8I_V3S_NMI_ENABLE,
+};
+
 static const struct sunxi_sc_nmi_reg_offs sun9i_reg_offs __initconst = {
 	.ctrl	= SUN9I_NMI_CTRL,
 	.pend	= SUN9I_NMI_PENDING,
@@ -165,7 +175,6 @@ static int __init sunxi_sc_nmi_irq_init(struct device_node *node,
 	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
 	int ret;
 
-
 	domain = irq_domain_add_linear(node, 1, &irq_generic_chip_ops, NULL);
 	if (!domain) {
 		pr_err("Could not register interrupt domain.\n");
@@ -254,6 +263,13 @@ static int __init sun7i_sc_nmi_irq_init(struct device_node *node,
 }
 IRQCHIP_DECLARE(sun7i_sc_nmi, "allwinner,sun7i-a20-sc-nmi", sun7i_sc_nmi_irq_init);
 
+static int __init sun8i_v3s_sc_nmi_irq_init(struct device_node *node,
+					    struct device_node *parent)
+{
+	return sunxi_sc_nmi_irq_init(node, &sun8i_v3s_reg_offs);
+}
+IRQCHIP_DECLARE(sun8i_v3s_sc_nmi, "allwinner,sun8i-v3s-sc-nmi", sun8i_v3s_sc_nmi_irq_init);
+
 static int __init sun9i_nmi_irq_init(struct device_node *node,
 				     struct device_node *parent)
 {
-- 
2.28.0

[PATCH 6/9] ARM: dts: sun8i-v3s: Add the V3s NMI IRQ controller

[Paul Kocialkowski]

The V3s/V3 has a NMI interrupt controller, mainly used for the AXP209.
Its address follows the sytsem controller block, which was previously
incorrectly described as spanning over 0x1000 address bytes.

This reduces the system controller address span up to the NMI
controller and adds a node for the controller, with its dedicated
compatible.

While the interrupt number was found in Allwinner's documentation,
the address for the controller is specified in any Allwinner SDK
supporting sun8iw8 (V3/V3s) at:
drivers/power/axp_power/axp20/axp20-board.c

It was tested to work on a V3 board with an AXP209 connected to the
NMI interrupt line.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/sun8i-v3s.dtsi | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi b/arch/arm/boot/dts/sun8i-v3s.dtsi
index 7b2d684aeb97..d2e2abd3869a 100644
--- a/arch/arm/boot/dts/sun8i-v3s.dtsi
+++ b/arch/arm/boot/dts/sun8i-v3s.dtsi
@@ -157,12 +157,20 @@ mixer0_out_tcon0: endpoint {
 		syscon: system-control@1c00000 {
 			compatible = "allwinner,sun8i-v3s-system-control",
 				     "allwinner,sun8i-h3-system-control";
-			reg = <0x01c00000 0x1000>;
+			reg = <0x01c00000 0xd0>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
 		};
 
+		nmi_intc: interrupt-controller@1c000d0 {
+			compatible = "allwinner,sun8i-v3s-sc-nmi";
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			reg = <0x01c000d0 0x0c>;
+			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
 		tcon0: lcd-controller@1c0c000 {
 			compatible = "allwinner,sun8i-v3s-tcon";
 			reg = <0x01c0c000 0x1000>;
-- 
2.28.0

[PATCH 7/9] ARM: dts: sun8i: Cleanup the Pinecube AXP209 node

[Paul Kocialkowski]

This fixes a few things with the Pinecube AXP209 node:
- No compatible is required since it is using an AXP209 (not AXP203)
  according to the schematics and this is what the included axp209.dtsi
  already has;
- The interrupt-controller and #interrupt-cells properties are already
  described in the included axp209.dtsi;
- The interrupt comes through the NMI controller, not directly through
  the GIC.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/sun8i-s3-pinecube.dts | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-s3-pinecube.dts b/arch/arm/boot/dts/sun8i-s3-pinecube.dts
index 9bab6b7f4014..ed6b4e94088d 100644
--- a/arch/arm/boot/dts/sun8i-s3-pinecube.dts
+++ b/arch/arm/boot/dts/sun8i-s3-pinecube.dts
@@ -88,13 +88,9 @@ &i2c0 {
 	status = "okay";
 
 	axp209: pmic@34 {
-		compatible = "x-powers,axp203",
-			     "x-powers,axp209";
 		reg = <0x34>;
-		interrupt-parent = <&gic>;
-		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-controller;
-		#interrupt-cells = <1>;
+		interrupt-parent = <&nmi_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
 	};
 };
 
-- 
2.28.0

[PATCH 8/9] dt-bindings: arm: sunxi: Add SL631 with IMX179 bindings

[Paul Kocialkowski]

Document the compatible strings for the SL631 Action Camera with IMX179.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 Documentation/devicetree/bindings/arm/sunxi.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
index afa00268c7db..0fa0c0b5d89f 100644
--- a/Documentation/devicetree/bindings/arm/sunxi.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
@@ -754,6 +754,12 @@ properties:
           - const: sinlinx,sina33
           - const: allwinner,sun8i-a33
 
+      - description: SL631 Action Camera with IMX179
+        items:
+          - const: unknown,sl631-imx179
+          - const: unknown,sl631
+          - const: allwinner,sun8i-v3
+
       - description: Tanix TX6
         items:
           - const: oranth,tanix-tx6
-- 
2.28.0

[PATCH 9/9] ARM: dts: sun8i-v3: Add support for the SL631 Action Camera with IMX179

[Paul Kocialkowski]

The SL631 is a family of Allwinner V3 action cameras sold under
various names, such as SJCAM SJ4000 Air or F60 Action Camera.

Devices in this family share a common board design but can be found
with different image sensors, including the IMX179 and the OV4689.

This adds support for a common dtsi for the SL631 family as well as
a specific dts for the IMX179 fashion, which will later be populated
with an IMX179 node when a driver is available.

Features that were tested on the device include:
- UART debug
- MMC
- USB peripheral (e.g. g_ether)
- Buttons
- SPI NOR flash

Note that the exact designer/vendor of these boards is unknown.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/Makefile                  |   1 +
 arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts |  12 ++
 arch/arm/boot/dts/sun8i-v3-sl631.dtsi       | 145 ++++++++++++++++++++
 3 files changed, 158 insertions(+)
 create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
 create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 4363ba564bb4..b76bcda9a9df 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1196,6 +1196,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
 	sun8i-s3-lichee-zero-plus.dtb \
 	sun8i-s3-pinecube.dtb \
 	sun8i-t3-cqa3t-bv3.dtb \
+	sun8i-v3-sl631-imx179.dtb \
 	sun8i-v3s-licheepi-zero.dtb \
 	sun8i-v3s-licheepi-zero-dock.dtb \
 	sun8i-v40-bananapi-m2-berry.dtb
diff --git a/arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts b/arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
new file mode 100644
index 000000000000..9e3b78000bdb
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR X11)
+/*
+ * Copyright 2020 Paul Kocialkowski <contact@paulk.fr>
+ */
+
+#include "sun8i-v3-sl631.dtsi"
+
+/ {
+	model = "SL631 Action Camera with IMX179";
+	compatible = "unknown,sl631-imx179", "unknown,sl631",
+		     "allwinner,sun8i-v3";
+};
diff --git a/arch/arm/boot/dts/sun8i-v3-sl631.dtsi b/arch/arm/boot/dts/sun8i-v3-sl631.dtsi
new file mode 100644
index 000000000000..9bc84d2812a6
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-v3-sl631.dtsi
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR X11)
+/*
+ * Copyright 2020 Paul Kocialkowski <contact@paulk.fr>
+ */
+
+/dts-v1/;
+
+#include "sun8i-v3.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "SL631 Action Camera";
+	compatible = "unknown,sl631", "allwinner,sun8i-v3";
+
+	aliases {
+		serial0 = &uart1;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&i2c0 {
+	status = "okay";
+
+	axp209: pmic@34 {
+		reg = <0x34>;
+		interrupt-parent = <&nmi_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	};
+};
+
+&i2c1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pb_pins>;
+	status = "okay";
+};
+
+&lradc {
+	vref-supply = <&reg_ldo2>;
+	status = "okay";
+
+	button@174 {
+		label = "Volume Down";
+		linux,code = <KEY_VOLUMEDOWN>;
+		channel = <0>;
+		voltage = <174603>;
+	};
+
+	button@384 {
+		label = "Volume Up";
+		linux,code = <KEY_VOLUMEUP>;
+		channel = <0>;
+		voltage = <384126>;
+	};
+
+	button@593 {
+		label = "Home";
+		linux,code = <KEY_HOME>;
+		channel = <0>;
+		voltage = <593650>;
+	};
+};
+
+&mmc0 {
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
+	bus-width = <4>;
+	vmmc-supply = <&reg_dcdc3>;
+	status = "okay";
+};
+
+&pio {
+	vcc-pd-supply = <&reg_dcdc3>;
+	vcc-pe-supply = <&reg_dcdc3>;
+};
+
+#include "axp209.dtsi"
+
+&ac_power_supply {
+	status = "okay";
+};
+
+&battery_power_supply {
+	status = "okay";
+};
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1250000>;
+	regulator-max-microvolt = <1250000>;
+	regulator-name = "vdd-sys-cpu";
+};
+
+&reg_dcdc3 {
+	regulator-always-on;
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vdd-3v3";
+};
+
+&reg_ldo1 {
+	regulator-name = "vdd-rtc";
+};
+
+&reg_ldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+	regulator-name = "avcc";
+};
+
+&reg_ldo4 {
+	regulator-always-on;
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-ep952";
+};
+
+&spi0 {
+	status = "okay";
+
+	spi-flash@0 {
+		reg = <0>;
+		compatible = "macronix,mx25l6436f", "jedec,spi-nor";
+		spi-max-frequency = <50000000>;
+	};
+};
+
+&uart1 {
+	pinctrl-0 = <&uart1_pg_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&usb_otg {
+	dr_mode = "peripheral";
+	status = "okay";
+};
+
+&usbphy {
+	status = "okay";
+};
-- 
2.28.0

Re: [PATCH 1/9] ARM: sunxi: Add machine match for the Allwinner V3 SoC

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

On Sat, Oct 31, 2020 at 07:21:29PM +0100, Paul Kocialkowski wrote:
> The Allwinner V3 SoC shares the same base as the V3s but comes with
> extra pins and features available. As a result, it has its dedicated
> compatible string (already used in device trees), which is added here.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>

Applied, thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/9] ARM: dts: sun8i-v3s: Add I2C1 PB pins description

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 280 bytes --]

On Sat, Oct 31, 2020 at 07:21:31PM +0100, Paul Kocialkowski wrote:
> I2C1 can be exposed through PB pins in addition to PE pins on the V3s.
> Add the device-tree description for these pins.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>

Applied, thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/9] dt-bindings: irq: sun7i-nmi: Add binding for the V3s NMI

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

On Sat, Oct 31, 2020 at 07:21:32PM +0100, Paul Kocialkowski wrote:
> The NMI interrupt controller takes a specific compatible for the V3s.

Why?

> Add it to the device-tree bindings documentation.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  .../interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml         | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
> index 8acca0ae3129..52364c2200ef 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
> @@ -26,6 +26,7 @@ properties:
>        - const: allwinner,sun6i-a31-sc-nmi
>          deprecated: true
>        - const: allwinner,sun7i-a20-sc-nmi
> +      - const: allwinner,sun8i-v3s-sc-nmi

This should be sorted

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 5/9] irqchip/sunxi-nmi: Add support for the V3s NMI

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 526 bytes --]

On Sat, Oct 31, 2020 at 07:21:33PM +0100, Paul Kocialkowski wrote:
> The V3s/V3 has a NMI IRQ controller, which is mainly used for the AXP209
> interrupt. In great wisdom, Allwinner decided to invert the enable and
> pending register offsets, compared to the A20.
> 
> As a result, a specific compatible and register description is required
> for the V3s. This was tested with an AXP209 on a V3 board.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>

Acked-by: Maxime Ripard <mripard@kernel.org>

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 6/9] ARM: dts: sun8i-v3s: Add the V3s NMI IRQ controller

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

On Sat, Oct 31, 2020 at 07:21:34PM +0100, Paul Kocialkowski wrote:
> The V3s/V3 has a NMI interrupt controller, mainly used for the AXP209.
> Its address follows the sytsem controller block, which was previously
> incorrectly described as spanning over 0x1000 address bytes.

Is it after, or right in the middle of it?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 8/9] dt-bindings: arm: sunxi: Add SL631 with IMX179 bindings

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

On Sat, Oct 31, 2020 at 07:21:36PM +0100, Paul Kocialkowski wrote:
> Document the compatible strings for the SL631 Action Camera with IMX179.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  Documentation/devicetree/bindings/arm/sunxi.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
> index afa00268c7db..0fa0c0b5d89f 100644
> --- a/Documentation/devicetree/bindings/arm/sunxi.yaml
> +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
> @@ -754,6 +754,12 @@ properties:
>            - const: sinlinx,sina33
>            - const: allwinner,sun8i-a33
>  
> +      - description: SL631 Action Camera with IMX179
> +        items:
> +          - const: unknown,sl631-imx179
> +          - const: unknown,sl631
> +          - const: allwinner,sun8i-v3
> +

unknown is not a valid vendor (and you should explain why you picked
that vendor name in the first place).

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 9/9] ARM: dts: sun8i-v3: Add support for the SL631 Action Camera with IMX179

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5601 bytes --]

On Sat, Oct 31, 2020 at 07:21:37PM +0100, Paul Kocialkowski wrote:
> The SL631 is a family of Allwinner V3 action cameras sold under
> various names, such as SJCAM SJ4000 Air or F60 Action Camera.
> 
> Devices in this family share a common board design but can be found
> with different image sensors, including the IMX179 and the OV4689.
> 
> This adds support for a common dtsi for the SL631 family as well as
> a specific dts for the IMX179 fashion, which will later be populated
> with an IMX179 node when a driver is available.
> 
> Features that were tested on the device include:
> - UART debug
> - MMC
> - USB peripheral (e.g. g_ether)
> - Buttons
> - SPI NOR flash
> 
> Note that the exact designer/vendor of these boards is unknown.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  arch/arm/boot/dts/Makefile                  |   1 +
>  arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts |  12 ++
>  arch/arm/boot/dts/sun8i-v3-sl631.dtsi       | 145 ++++++++++++++++++++
>  3 files changed, 158 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
>  create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 4363ba564bb4..b76bcda9a9df 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1196,6 +1196,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>  	sun8i-s3-lichee-zero-plus.dtb \
>  	sun8i-s3-pinecube.dtb \
>  	sun8i-t3-cqa3t-bv3.dtb \
> +	sun8i-v3-sl631-imx179.dtb \
>  	sun8i-v3s-licheepi-zero.dtb \
>  	sun8i-v3s-licheepi-zero-dock.dtb \
>  	sun8i-v40-bananapi-m2-berry.dtb
> diff --git a/arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts b/arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
> new file mode 100644
> index 000000000000..9e3b78000bdb
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR X11)
> +/*
> + * Copyright 2020 Paul Kocialkowski <contact@paulk.fr>
> + */
> +
> +#include "sun8i-v3-sl631.dtsi"
> +
> +/ {
> +	model = "SL631 Action Camera with IMX179";
> +	compatible = "unknown,sl631-imx179", "unknown,sl631",
> +		     "allwinner,sun8i-v3";
> +};
> diff --git a/arch/arm/boot/dts/sun8i-v3-sl631.dtsi b/arch/arm/boot/dts/sun8i-v3-sl631.dtsi
> new file mode 100644
> index 000000000000..9bc84d2812a6
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-v3-sl631.dtsi
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR X11)
> +/*
> + * Copyright 2020 Paul Kocialkowski <contact@paulk.fr>
> + */
> +
> +/dts-v1/;
> +
> +#include "sun8i-v3.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +
> +/ {
> +	model = "SL631 Action Camera";
> +	compatible = "unknown,sl631", "allwinner,sun8i-v3";
> +
> +	aliases {
> +		serial0 = &uart1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +
> +&i2c0 {
> +	status = "okay";
> +
> +	axp209: pmic@34 {
> +		reg = <0x34>;
> +		interrupt-parent = <&nmi_intc>;
> +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +};
> +
> +&i2c1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c1_pb_pins>;
> +	status = "okay";
> +};
> +
> +&lradc {
> +	vref-supply = <&reg_ldo2>;
> +	status = "okay";
> +
> +	button@174 {
> +		label = "Volume Down";
> +		linux,code = <KEY_VOLUMEDOWN>;
> +		channel = <0>;
> +		voltage = <174603>;
> +	};
> +
> +	button@384 {
> +		label = "Volume Up";
> +		linux,code = <KEY_VOLUMEUP>;
> +		channel = <0>;
> +		voltage = <384126>;
> +	};
> +
> +	button@593 {
> +		label = "Home";
> +		linux,code = <KEY_HOME>;
> +		channel = <0>;
> +		voltage = <593650>;
> +	};
> +};

The buttons are not valid node names, since you can't use a unit-address
without reg.

> +&mmc0 {
> +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> +	bus-width = <4>;
> +	vmmc-supply = <&reg_dcdc3>;
> +	status = "okay";
> +};
> +
> +&pio {
> +	vcc-pd-supply = <&reg_dcdc3>;
> +	vcc-pe-supply = <&reg_dcdc3>;
> +};
> +
> +#include "axp209.dtsi"
> +
> +&ac_power_supply {
> +	status = "okay";
> +};
> +
> +&battery_power_supply {
> +	status = "okay";
> +};
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1250000>;
> +	regulator-max-microvolt = <1250000>;
> +	regulator-name = "vdd-sys-cpu";
> +};
> +
> +&reg_dcdc3 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3300000>;
> +	regulator-max-microvolt = <3300000>;
> +	regulator-name = "vdd-3v3";
> +};
> +
> +&reg_ldo1 {
> +	regulator-name = "vdd-rtc";
> +};
> +
> +&reg_ldo2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3000000>;
> +	regulator-max-microvolt = <3000000>;
> +	regulator-name = "avcc";
> +};
> +
> +&reg_ldo4 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3300000>;
> +	regulator-max-microvolt = <3300000>;
> +	regulator-name = "vcc-ep952";
> +};

AFAIK we don't have a driver for the ep952, why would we need to leave
that regulator on?

> +
> +&spi0 {
> +	status = "okay";
> +
> +	spi-flash@0 {
> +		reg = <0>;
> +		compatible = "macronix,mx25l6436f", "jedec,spi-nor";
> +		spi-max-frequency = <50000000>;
> +	};
> +};
> +
> +&uart1 {
> +	pinctrl-0 = <&uart1_pg_pins>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +};
> +
> +&usb_otg {
> +	dr_mode = "peripheral";
> +	status = "okay";
> +};

Is it a peripheral because you didn't test the host mode, or because the
hardware doesn't have it?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/9] dt-bindings: irq: sun7i-nmi: Add binding for the V3s NMI

[Paul Kocialkowski]

Hi,

On Mon 02 Nov 20, 11:09, Maxime Ripard wrote:
> On Sat, Oct 31, 2020 at 07:21:32PM +0100, Paul Kocialkowski wrote:
> > The NMI interrupt controller takes a specific compatible for the V3s.
> 
> Why?

Well, it's explained in details in the patch adding support in the driver
and in less details when adding the dt nodes. But okay I'll add a short
explanation here as well :)

> > Add it to the device-tree bindings documentation.
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  .../interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml         | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
> > index 8acca0ae3129..52364c2200ef 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
> > @@ -26,6 +26,7 @@ properties:
> >        - const: allwinner,sun6i-a31-sc-nmi
> >          deprecated: true
> >        - const: allwinner,sun7i-a20-sc-nmi
> > +      - const: allwinner,sun8i-v3s-sc-nmi
> 
> This should be sorted

Ah sorry. So this should be between allwinner,sun8i-a83t-r-intc
and allwinner,sun9i-a80-nmi then.

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

Re: [PATCH 6/9] ARM: dts: sun8i-v3s: Add the V3s NMI IRQ controller

[Paul Kocialkowski]

Hi,

On Mon 02 Nov 20, 11:12, Maxime Ripard wrote:
> On Sat, Oct 31, 2020 at 07:21:34PM +0100, Paul Kocialkowski wrote:
> > The V3s/V3 has a NMI interrupt controller, mainly used for the AXP209.
> > Its address follows the sytsem controller block, which was previously
> > incorrectly described as spanning over 0x1000 address bytes.
> 
> Is it after, or right in the middle of it?

That's up for interpretation actually:
- The V3 datasheet mentions that System Control is 0x01C00000 --- 0x01C00FFF;
- In practice, sunxi_sram.c uses a regmap with max_reg set to 0x30 for the
  V3s/H3 so this gives us some room.

Looking at other SoCs with the same setup (take sun8i-r40 for instance),
system-control is limited to 0x30 and the NMI controller follows it.
In the case of R40, the SRAM controlled is also said to be 4K-long in the
Allwinner docs.

So all in all, this leads me to believe that the system-controller instance
stops well before 0x1c000d0 on the V3s as well. Otherwise, we should also
make the R40 consistent.

Cheers,

Paul

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

Re: [PATCH 9/9] ARM: dts: sun8i-v3: Add support for the SL631 Action Camera with IMX179

[Paul Kocialkowski]

Hi,

On Mon 02 Nov 20, 11:16, Maxime Ripard wrote:
> On Sat, Oct 31, 2020 at 07:21:37PM +0100, Paul Kocialkowski wrote:
> > The SL631 is a family of Allwinner V3 action cameras sold under
> > various names, such as SJCAM SJ4000 Air or F60 Action Camera.
> > 
> > Devices in this family share a common board design but can be found
> > with different image sensors, including the IMX179 and the OV4689.
> > 
> > This adds support for a common dtsi for the SL631 family as well as
> > a specific dts for the IMX179 fashion, which will later be populated
> > with an IMX179 node when a driver is available.
> > 
> > Features that were tested on the device include:
> > - UART debug
> > - MMC
> > - USB peripheral (e.g. g_ether)
> > - Buttons
> > - SPI NOR flash
> > 
> > Note that the exact designer/vendor of these boards is unknown.
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  arch/arm/boot/dts/Makefile                  |   1 +
> >  arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts |  12 ++
> >  arch/arm/boot/dts/sun8i-v3-sl631.dtsi       | 145 ++++++++++++++++++++
> >  3 files changed, 158 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
> >  create mode 100644 arch/arm/boot/dts/sun8i-v3-sl631.dtsi
> > 
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 4363ba564bb4..b76bcda9a9df 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1196,6 +1196,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> >  	sun8i-s3-lichee-zero-plus.dtb \
> >  	sun8i-s3-pinecube.dtb \
> >  	sun8i-t3-cqa3t-bv3.dtb \
> > +	sun8i-v3-sl631-imx179.dtb \
> >  	sun8i-v3s-licheepi-zero.dtb \
> >  	sun8i-v3s-licheepi-zero-dock.dtb \
> >  	sun8i-v40-bananapi-m2-berry.dtb
> > diff --git a/arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts b/arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
> > new file mode 100644
> > index 000000000000..9e3b78000bdb
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun8i-v3-sl631-imx179.dts
> > @@ -0,0 +1,12 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR X11)
> > +/*
> > + * Copyright 2020 Paul Kocialkowski <contact@paulk.fr>
> > + */
> > +
> > +#include "sun8i-v3-sl631.dtsi"
> > +
> > +/ {
> > +	model = "SL631 Action Camera with IMX179";
> > +	compatible = "unknown,sl631-imx179", "unknown,sl631",
> > +		     "allwinner,sun8i-v3";
> > +};
> > diff --git a/arch/arm/boot/dts/sun8i-v3-sl631.dtsi b/arch/arm/boot/dts/sun8i-v3-sl631.dtsi
> > new file mode 100644
> > index 000000000000..9bc84d2812a6
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun8i-v3-sl631.dtsi
> > @@ -0,0 +1,145 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR X11)
> > +/*
> > + * Copyright 2020 Paul Kocialkowski <contact@paulk.fr>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "sun8i-v3.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +
> > +/ {
> > +	model = "SL631 Action Camera";
> > +	compatible = "unknown,sl631", "allwinner,sun8i-v3";
> > +
> > +	aliases {
> > +		serial0 = &uart1;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +};
> > +
> > +&i2c0 {
> > +	status = "okay";
> > +
> > +	axp209: pmic@34 {
> > +		reg = <0x34>;
> > +		interrupt-parent = <&nmi_intc>;
> > +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > +	};
> > +};
> > +
> > +&i2c1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&i2c1_pb_pins>;
> > +	status = "okay";
> > +};
> > +
> > +&lradc {
> > +	vref-supply = <&reg_ldo2>;
> > +	status = "okay";
> > +
> > +	button@174 {
> > +		label = "Volume Down";
> > +		linux,code = <KEY_VOLUMEDOWN>;
> > +		channel = <0>;
> > +		voltage = <174603>;
> > +	};
> > +
> > +	button@384 {
> > +		label = "Volume Up";
> > +		linux,code = <KEY_VOLUMEUP>;
> > +		channel = <0>;
> > +		voltage = <384126>;
> > +	};
> > +
> > +	button@593 {
> > +		label = "Home";
> > +		linux,code = <KEY_HOME>;
> > +		channel = <0>;
> > +		voltage = <593650>;
> > +	};
> > +};
> 
> The buttons are not valid node names, since you can't use a unit-address
> without reg.

Ah sorry, I see that nowadays the form is e.g. button-174.
This was probably a copy-paste from older dts. And indeed there's no reg around
to justify it (and a voltage is not an address anyway).

> > +&mmc0 {
> > +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> > +	bus-width = <4>;
> > +	vmmc-supply = <&reg_dcdc3>;
> > +	status = "okay";
> > +};
> > +
> > +&pio {
> > +	vcc-pd-supply = <&reg_dcdc3>;
> > +	vcc-pe-supply = <&reg_dcdc3>;
> > +};
> > +
> > +#include "axp209.dtsi"
> > +
> > +&ac_power_supply {
> > +	status = "okay";
> > +};
> > +
> > +&battery_power_supply {
> > +	status = "okay";
> > +};
> > +
> > +&reg_dcdc2 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <1250000>;
> > +	regulator-max-microvolt = <1250000>;
> > +	regulator-name = "vdd-sys-cpu";
> > +};
> > +
> > +&reg_dcdc3 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <3300000>;
> > +	regulator-max-microvolt = <3300000>;
> > +	regulator-name = "vdd-3v3";
> > +};
> > +
> > +&reg_ldo1 {
> > +	regulator-name = "vdd-rtc";
> > +};
> > +
> > +&reg_ldo2 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <3000000>;
> > +	regulator-max-microvolt = <3000000>;
> > +	regulator-name = "avcc";
> > +};
> > +
> > +&reg_ldo4 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <3300000>;
> > +	regulator-max-microvolt = <3300000>;
> > +	regulator-name = "vcc-ep952";
> > +};
> 
> AFAIK we don't have a driver for the ep952, why would we need to leave
> that regulator on?

Right, I don't think it needs to be. I'll test without it and remove if it is
indeed not needed.

Keep in mind that all of this comes from the fex and I don't have schematics
so this is trial and error.

> > +&spi0 {
> > +	status = "okay";
> > +
> > +	spi-flash@0 {
> > +		reg = <0>;
> > +		compatible = "macronix,mx25l6436f", "jedec,spi-nor";
> > +		spi-max-frequency = <50000000>;
> > +	};
> > +};
> > +
> > +&uart1 {
> > +	pinctrl-0 = <&uart1_pg_pins>;
> > +	pinctrl-names = "default";
> > +	status = "okay";
> > +};
> > +
> > +&usb_otg {
> > +	dr_mode = "peripheral";
> > +	status = "okay";
> > +};
> 
> Is it a peripheral because you didn't test the host mode, or because the
> hardware doesn't have it?

According to the fex and trial and error, there's no way to drive VBUS on the
hardware design so it's only peripheral.

Cheers and thanks for the review,

Paul

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

Re: [PATCH 8/9] dt-bindings: arm: sunxi: Add SL631 with IMX179 bindings

[Paul Kocialkowski]

Hi,

On Mon 02 Nov 20, 11:13, Maxime Ripard wrote:
> On Sat, Oct 31, 2020 at 07:21:36PM +0100, Paul Kocialkowski wrote:
> > Document the compatible strings for the SL631 Action Camera with IMX179.
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  Documentation/devicetree/bindings/arm/sunxi.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > index afa00268c7db..0fa0c0b5d89f 100644
> > --- a/Documentation/devicetree/bindings/arm/sunxi.yaml
> > +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > @@ -754,6 +754,12 @@ properties:
> >            - const: sinlinx,sina33
> >            - const: allwinner,sun8i-a33
> >  
> > +      - description: SL631 Action Camera with IMX179
> > +        items:
> > +          - const: unknown,sl631-imx179
> > +          - const: unknown,sl631
> > +          - const: allwinner,sun8i-v3
> > +
> 
> unknown is not a valid vendor (and you should explain why you picked
> that vendor name in the first place).

Ah right, it's not in the vendor prefixes. The rationale is that there is no
indication of what the vendor might be on the PCB. Should I maybe use
allwinner here? It looks derived from a V3 reference design anyway
(at least the pin assignment matches e.g. Sochip's S3 EVB and other known S3
boards too apparently) so this could make sense.

Cheers,

Paul

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

Re: [PATCH 6/9] ARM: dts: sun8i-v3s: Add the V3s NMI IRQ controller

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

On Mon, Nov 02, 2020 at 11:25:22AM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon 02 Nov 20, 11:12, Maxime Ripard wrote:
> > On Sat, Oct 31, 2020 at 07:21:34PM +0100, Paul Kocialkowski wrote:
> > > The V3s/V3 has a NMI interrupt controller, mainly used for the AXP209.
> > > Its address follows the sytsem controller block, which was previously
> > > incorrectly described as spanning over 0x1000 address bytes.
> > 
> > Is it after, or right in the middle of it?
> 
> That's up for interpretation actually:
> - The V3 datasheet mentions that System Control is 0x01C00000 --- 0x01C00FFF;
> - In practice, sunxi_sram.c uses a regmap with max_reg set to 0x30 for the
>   V3s/H3 so this gives us some room.
> 
> Looking at other SoCs with the same setup (take sun8i-r40 for instance),
> system-control is limited to 0x30 and the NMI controller follows it.
> In the case of R40, the SRAM controlled is also said to be 4K-long in the
> Allwinner docs.
> 
> So all in all, this leads me to believe that the system-controller instance
> stops well before 0x1c000d0 on the V3s as well. Otherwise, we should also
> make the R40 consistent.

That's a bit unfortunate, but yeah, I guess we want to remain consistent here.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 8/9] dt-bindings: arm: sunxi: Add SL631 with IMX179 bindings

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]

On Mon, Nov 02, 2020 at 11:33:40AM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon 02 Nov 20, 11:13, Maxime Ripard wrote:
> > On Sat, Oct 31, 2020 at 07:21:36PM +0100, Paul Kocialkowski wrote:
> > > Document the compatible strings for the SL631 Action Camera with IMX179.
> > > 
> > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > ---
> > >  Documentation/devicetree/bindings/arm/sunxi.yaml | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > > index afa00268c7db..0fa0c0b5d89f 100644
> > > --- a/Documentation/devicetree/bindings/arm/sunxi.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > > @@ -754,6 +754,12 @@ properties:
> > >            - const: sinlinx,sina33
> > >            - const: allwinner,sun8i-a33
> > >  
> > > +      - description: SL631 Action Camera with IMX179
> > > +        items:
> > > +          - const: unknown,sl631-imx179
> > > +          - const: unknown,sl631
> > > +          - const: allwinner,sun8i-v3
> > > +
> > 
> > unknown is not a valid vendor (and you should explain why you picked
> > that vendor name in the first place).
> 
> Ah right, it's not in the vendor prefixes. The rationale is that there is no
> indication of what the vendor might be on the PCB. Should I maybe use
> allwinner here?

You should mention it in the commit log for a start :)

But yeah, we've used allwinner as a fallback for those cases so far.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 6/9] ARM: dts: sun8i-v3s: Add the V3s NMI IRQ controller

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

On Mon 02 Nov 20, 14:44, Maxime Ripard wrote:
> On Mon, Nov 02, 2020 at 11:25:22AM +0100, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Mon 02 Nov 20, 11:12, Maxime Ripard wrote:
> > > On Sat, Oct 31, 2020 at 07:21:34PM +0100, Paul Kocialkowski wrote:
> > > > The V3s/V3 has a NMI interrupt controller, mainly used for the AXP209.
> > > > Its address follows the sytsem controller block, which was previously
> > > > incorrectly described as spanning over 0x1000 address bytes.
> > > 
> > > Is it after, or right in the middle of it?
> > 
> > That's up for interpretation actually:
> > - The V3 datasheet mentions that System Control is 0x01C00000 --- 0x01C00FFF;
> > - In practice, sunxi_sram.c uses a regmap with max_reg set to 0x30 for the
> >   V3s/H3 so this gives us some room.
> > 
> > Looking at other SoCs with the same setup (take sun8i-r40 for instance),
> > system-control is limited to 0x30 and the NMI controller follows it.
> > In the case of R40, the SRAM controlled is also said to be 4K-long in the
> > Allwinner docs.
> > 
> > So all in all, this leads me to believe that the system-controller instance
> > stops well before 0x1c000d0 on the V3s as well. Otherwise, we should also
> > make the R40 consistent.
> 
> That's a bit unfortunate, but yeah, I guess we want to remain consistent here.

Honestly I think the Allwinner docs are plain wrong on this one.
IIRC they used to describe the NMI as a separate controller in the memory
map. I think they just overlook it now and copy/paste 4K size for each
controller regardless of the actual hardware, so I'm not very worried.

Cheers,

Pauls

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 8/9] dt-bindings: arm: sunxi: Add SL631 with IMX179 bindings

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 2021 bytes --]

On Mon 02 Nov 20, 15:27, Maxime Ripard wrote:
> On Mon, Nov 02, 2020 at 11:33:40AM +0100, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Mon 02 Nov 20, 11:13, Maxime Ripard wrote:
> > > On Sat, Oct 31, 2020 at 07:21:36PM +0100, Paul Kocialkowski wrote:
> > > > Document the compatible strings for the SL631 Action Camera with IMX179.
> > > > 
> > > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > > ---
> > > >  Documentation/devicetree/bindings/arm/sunxi.yaml | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > > > index afa00268c7db..0fa0c0b5d89f 100644
> > > > --- a/Documentation/devicetree/bindings/arm/sunxi.yaml
> > > > +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > > > @@ -754,6 +754,12 @@ properties:
> > > >            - const: sinlinx,sina33
> > > >            - const: allwinner,sun8i-a33
> > > >  
> > > > +      - description: SL631 Action Camera with IMX179
> > > > +        items:
> > > > +          - const: unknown,sl631-imx179
> > > > +          - const: unknown,sl631
> > > > +          - const: allwinner,sun8i-v3
> > > > +
> > > 
> > > unknown is not a valid vendor (and you should explain why you picked
> > > that vendor name in the first place).
> > 
> > Ah right, it's not in the vendor prefixes. The rationale is that there is no
> > indication of what the vendor might be on the PCB. Should I maybe use
> > allwinner here?
> 
> You should mention it in the commit log for a start :)

Definitely.

> But yeah, we've used allwinner as a fallback for those cases so far.

Okay then, I'll go with that and add some explanation in the commit log.

Thanks for the review!

Cheers,

Paul

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]