Message-ID: <20201102101626.t4ox3rwbwp335i2m@gilmour.lan>
Date:   Mon, 2 Nov 2020 11:16:26 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Paul Kocialkowski <contact@...lk.fr>
Cc:     devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Matteo Scordino <matteo.scordino@...il.com>,
        Icenowy Zheng <icenowy@...c.io>
Subject: Re: [PATCH 9/9] ARM: dts: sun8i-v3: Add support for the SL631 Action
 Camera with IMX179

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@...lk.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@...lk.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@...lk.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

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists