LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Paul Kocialkowski <contact@paulk.fr>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org, Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>, Chen-Yu Tsai <wens@csie.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v3 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet
Date: Mon, 07 May 2018 22:15:46 +0200	[thread overview]
Message-ID: <d8de7d2237a74388ac8c8152eded3477e7fd56a4.camel@paulk.fr> (raw)
In-Reply-To: <20180507071931.tebv45yxrojxxkeb@flea>

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

Hi,

Le lundi 07 mai 2018 à 09:19 +0200, Maxime Ripard a écrit :
> Hi,
> 
> On Sun, May 06, 2018 at 11:49:01PM +0200, Paul Kocialkowski wrote:
> > This adds support for the Ainol AW1, an A20-based 7" tablet from Ainol.
> > 
> > The following board-specific features are supported:
> > * LCD panel
> > * Backlight
> > * USB OTG
> > * Buttons
> > * Touchscreen (doesn't work without non-free firmware)
> > * Accelerometer
> > * Battery
> > 
> > The following are untested:
> > * Audio output
> > * Audio speakers
> > * USB via SPCI connector
> > 
> > The following are not supported:
> > * Wi-Fi
> > * Bluetooth
> > * NAND
> > * Audio via SPCI connector
> > * Audio via Bluetooth I2S
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  arch/arm/boot/dts/Makefile                |   1 +
> >  arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts | 275 ++++++++++++++++++++++
> >  2 files changed, 276 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
> > 
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 7e2424957809..4a80971f2bc9 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -946,6 +946,7 @@ dtb-$(CONFIG_MACH_SUN6I) += \
> >  	sun6i-a31s-sinovoip-bpi-m2.dtb \
> >  	sun6i-a31s-yones-toptech-bs1078-v2.dtb
> >  dtb-$(CONFIG_MACH_SUN7I) += \
> > +	sun7i-a20-ainol-aw1.dtb \
> >  	sun7i-a20-bananapi.dtb \
> >  	sun7i-a20-bananapi-m1-plus.dtb \
> >  	sun7i-a20-bananapro.dtb \
> > diff --git a/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
> > new file mode 100644
> > index 000000000000..9a1d54a9f9a0
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
> > @@ -0,0 +1,275 @@
> > +/*
> > + * Copyright (C) 2018 Paul Kocialkowski <contact@paulk.fr>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0+
> 
> This should be your first line.

Alright then. For reference, I based this off sun8i-h3-libretech-all-h3-
cc.dts, that has the copyright first.

> Also, we usually license our DT under
> a dual license (GPL and MIT) so that other projects (like FreeBSD) can
> use them as well, instead of duplicating them. It would be great if
> you'd consider it.

Ah, I didn't think of BSD-licensed systems like FreeBSD off-hand.

Since they are apparently working on ARM support, it's probably worth
publishing this under a non-copyleft license.

> > + */
> > +
> > +/dts-v1/;
> > +#include "sun7i-a20.dtsi"
> > +#include "sunxi-common-regulators.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/pwm/pwm.h>
> > +
> > +/ {
> > +	model = "Ainol AW1";
> > +	compatible = "ainol,ainol-aw1", "allwinner,sun7i-a20";
> > +
> > +	aliases {
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +
> > +	backlight: backlight {
> > +		compatible = "pwm-backlight";
> > +		pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
> > +		brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
> 
> The increase in perceived brightness should be linear. Usually, for
> PWMs backed backlight, an exponential list is a much better
> approximation.

Thanks for the hint, it never occurred to me that pwm duty cycle was not
linear with brightness, but that makes sense. I'll give that a try (on
255 values instead of 10 to keep some level of precision in low
brightness). The way to go here is probably use a base-255 logarithm
such as: duty cycle = range * log(i+1)/log(255) with i the linear
brightness value (0 to 255) and range the amplitude of our values (that
gets divided by the maximum brightness, so it's really up to hardware
precision at this point). I'll go with a range of 255 as well.

> > +		default-brightness-level = <5>;
> > +		enable-gpios = <&pio 7 7 GPIO_ACTIVE_HIGH>; /* PH7 */
> > +	};
> > +
> > +	panel: panel {
> > +		compatible = "innolux,at070tn90";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		power-supply = <&panel_power>;
> > +		backlight = <&backlight>;
> > +
> > +		port@0 {
> > +			reg = <0>;
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			panel_input: endpoint@0 {
> > +				reg = <0>;
> > +				remote-endpoint = <&tcon0_out_panel>;
> > +			};
> > +		};
> > +	};
> > +
> > +	panel_power: panel_power {
> > +		compatible = "regulator-fixed";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&panel_power_pin>;
> > +		regulator-name = "panel-power";
> > +		regulator-min-microvolt = <10400000>;
> > +		regulator-max-microvolt = <10400000>;
> > +		gpio = <&pio 7 8 GPIO_ACTIVE_HIGH>; /* PH8 */
> > +		enable-active-high;
> > +		regulator-boot-on;
> > +	};
> > +};
> > +
> > +&codec {
> > +	allwinner,pa-gpios = <&pio 7 15 GPIO_ACTIVE_HIGH>; /* PH15 */
> > +	status = "okay";
> > +};
> > +
> > +&cpu0 {
> > +	cpu-supply = <&reg_dcdc2>;
> > +};
> > +
> > +&de {
> > +	status = "okay";
> > +};
> > +
> > +&ehci0 {
> > +	status = "okay";
> > +};
> > +
> > +&ehci1 {
> > +	status = "okay";
> > +};
> > +
> > +&i2c0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&i2c0_pins_a>;
> > +	status = "okay";
> > +
> > +	axp209: pmic@34 {
> > +		reg = <0x34>;
> > +		interrupt-parent = <&nmi_intc>;
> > +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > +	};
> > +};
> > +
> > +&i2c1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&i2c1_pins_a>;
> > +	status = "okay";
> > +
> > +	lis3dh: accelerometer@18 {
> > +		compatible = "st,lis3dh-accel";
> > +		reg = <0x18>;
> > +		vdd-supply = <&reg_vcc3v3>;
> > +		vddio-supply = <&reg_vcc3v3>;
> > +		st,drdy-int-pin = <1>;
> > +	};
> > +};
> > +
> > +&i2c2 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&i2c2_pins_a>;
> > +	status = "okay";
> > +	clock-frequency = <400000>;
> > +
> > +	gsl1680: touchscreen@40 {
> > +		compatible = "silead,gsl1680";
> > +		reg = <0x40>;
> > +		interrupt-parent = <&pio>;
> > +		interrupts = <7 21 IRQ_TYPE_EDGE_FALLING>; /* EINT21 (PH21) */
> > +		power-gpios = <&pio 7 20 GPIO_ACTIVE_HIGH>; /* PH20 */
> > +		firmware-name = "gsl1680-ainol-aw1.fw";
> > +		touchscreen-size-x = <480>;
> > +		touchscreen-size-y = <800>;
> > +		touchscreen-swapped-x-y;
> > +		touchscreen-inverted-y;
> > +		silead,max-fingers = <5>;
> > +	};
> > +};
> > +
> > +&lradc {
> > +	vref-supply = <&reg_ldo2>;
> > +	status = "okay";
> > +
> > +	button@571 {
> > +		label = "Volume Up";
> > +		linux,code = <KEY_VOLUMEUP>;
> > +		channel = <0>;
> > +		voltage = <571428>;
> > +	};
> > +
> > +	button@761 {
> > +		label = "Volume Down";
> > +		linux,code = <KEY_VOLUMEDOWN>;
> > +		channel = <0>;
> > +		voltage = <761904>;
> > +	};
> > +
> > +	button@952 {
> > +		label = "Home";
> > +		linux,code = <KEY_HOME>;
> > +		channel = <0>;
> > +		voltage = <952380>;
> > +	};
> > +};
> > +
> > +&mmc0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&mmc0_pins_a>;
> > +	vmmc-supply = <&reg_vcc3v3>;
> 
> You have the regulators described in your DT, you'd better use them
> instead of the one coming from sunxi-common-regulators.dtsi.

Well, according to the reference A20 design, the mmc pins and the card
are powered by the 3.3V power rail, that comes from a regular step-down
regulator sourcing from IPSOUT, so I don't see what regulator I should
better use. Do you have a suggestion?

> > +	bus-width = <4>;
> > +	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
> > +	cd-inverted;
> > +	status = "okay";
> > +};
> > +
> > +&ohci0 {
> > +	status = "okay";
> > +};
> > +
> > +&ohci1 {
> > +	status = "okay";
> > +};
> > +
> > +&otg_sram {
> > +	status = "okay";
> > +};
> > +
> > +&pio {
> > +	panel_power_pin: panel_power_pin@0 {
> > +		pins = "PH8";
> > +		function = "gpio_out";
> > +	};
> > +};
> 
> You don't need that pinctrl node.

I'll get rid of it then. You mentioned that regulator-simple uses the
old GPIO API, so I assumed it meant that a pinctrl node is still needed.
For reference, it uses of_get_named_gpio (not the devm-managed fashion).

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: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2018-05-07 20:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-06 21:48 [PATCH v3 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90 Paul Kocialkowski
2018-05-06 21:49 ` [PATCH v3 2/3] ARM: dts: sun7i: Add RGB666 pins definition Paul Kocialkowski
2018-05-06 21:49 ` [PATCH v3 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet Paul Kocialkowski
2018-05-07  7:19   ` Maxime Ripard
2018-05-07 20:15     ` Paul Kocialkowski [this message]
2018-05-08 13:28       ` Maxime Ripard
2018-05-07  7:08 ` [PATCH v3 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90 Maxime Ripard
2018-05-07 18:50   ` Paul Kocialkowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d8de7d2237a74388ac8c8152eded3477e7fd56a4.camel@paulk.fr \
    --to=contact@paulk.fr \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=maxime.ripard@bootlin.com \
    --cc=robh+dt@kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=wens@csie.org \
    --subject='Re: [PATCH v3 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).