| Message ID | 20201031182137.1879521-1-contact@paulk.fr |
|---|---|
| Headers | show |
| Series | Allwinner V3 SL631 Action Camera Support and Related Fixes | expand |
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
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
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
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
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 = <®_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 = <®_dcdc3>; > + status = "okay"; > +}; > + > +&pio { > + vcc-pd-supply = <®_dcdc3>; > + vcc-pe-supply = <®_dcdc3>; > +}; > + > +#include "axp209.dtsi" > + > +&ac_power_supply { > + status = "okay"; > +}; > + > +&battery_power_supply { > + status = "okay"; > +}; > + > +®_dcdc2 { > + regulator-always-on; > + regulator-min-microvolt = <1250000>; > + regulator-max-microvolt = <1250000>; > + regulator-name = "vdd-sys-cpu"; > +}; > + > +®_dcdc3 { > + regulator-always-on; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-name = "vdd-3v3"; > +}; > + > +®_ldo1 { > + regulator-name = "vdd-rtc"; > +}; > + > +®_ldo2 { > + regulator-always-on; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3000000>; > + regulator-name = "avcc"; > +}; > + > +®_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
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
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 = <®_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 = <®_dcdc3>; > > + status = "okay"; > > +}; > > + > > +&pio { > > + vcc-pd-supply = <®_dcdc3>; > > + vcc-pe-supply = <®_dcdc3>; > > +}; > > + > > +#include "axp209.dtsi" > > + > > +&ac_power_supply { > > + status = "okay"; > > +}; > > + > > +&battery_power_supply { > > + status = "okay"; > > +}; > > + > > +®_dcdc2 { > > + regulator-always-on; > > + regulator-min-microvolt = <1250000>; > > + regulator-max-microvolt = <1250000>; > > + regulator-name = "vdd-sys-cpu"; > > +}; > > + > > +®_dcdc3 { > > + regulator-always-on; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-name = "vdd-3v3"; > > +}; > > + > > +®_ldo1 { > > + regulator-name = "vdd-rtc"; > > +}; > > + > > +®_ldo2 { > > + regulator-always-on; > > + regulator-min-microvolt = <3000000>; > > + regulator-max-microvolt = <3000000>; > > + regulator-name = "avcc"; > > +}; > > + > > +®_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
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
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
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