linux-kernel - Re: [PATCH 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180412145136.zp2i7u76oinphiiz@flea>
Date:   Thu, 12 Apr 2018 16:51:36 +0200
From:   Maxime Ripard <maxime.ripard@...tlin.com>
To:     Paul Kocialkowski <contact@...lk.fr>
Cc:     devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-sunxi@...glegroups.com, Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Russell King <linux@...linux.org.uk>,
        Chen-Yu Tsai <wens@...e.org>,
        Thierry Reding <thierry.reding@...il.com>,
        David Airlie <airlied@...ux.ie>
Subject: Re: [PATCH 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet

On Thu, Apr 12, 2018 at 01:08:51AM +0200, Paul Kocialkowski wrote:
> > > +	backlight: backlight {
> > > +		compatible = "pwm-backlight";
> > > +		pinctrl-names = "default";
> > > +		pinctrl-0 = <&backlight_enable_pin>;
> > 
> > You don't need any of the pinctrl nodes for the GPIOs
> 
> I tried without the pinctrl nodes and got issues on various controllers
> (e.g. i2c for the touchscreen) because of the missing pinctrl nodes on
> 4.16. Maybe I'm missing some patches here?

You don't need any patches. What was the error exactly?

> > > +&cpu0 {
> > > +	cpu-supply = <&reg_dcdc2>;
> > > +};
> > 
> > How was CPUfreq tested?
> 
> In fact, I haven't tried it at all, but I can definitely do that with
> e.g. ssvb's stress test for various cpufreq functioning points.

That would be great yes.

> > > +&i2c2 {
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&i2c2_pins_a>;
> > > +	status = "okay";
> > > +	clock-frequency = <400000>; /* 400 KHz required for
> > > GSL1680. */
> > 
> > I'm not sure that comment is worth it. The only device there is the
> > touchscreen, so it's kind of obvious that it's the device that needs
> > that frequency.
> 
> Well, I found a similar comment in the other dts using the same
> touchscreen controller. Since the information was rather valuable (it
> made it clear that I needed the same clock frequency for that specific
> touchscreen),

You can have the same kind of comment for pretty much all DT
lines. you could for example have on the pinctrl property just above
the comment that the I2C2 on that boards are tied to those pins. But
that's just redundant, and the SNR would be pretty bad if we were to
do it everywhere.

> it might help others in the future (even if only when grepping for
> gsl1680).
> 
> > > +
> > > +	gsl1680: touchscreen@40 {
> > > +		compatible = "silead,gsl1680";

You have the gsl1680 two times here, so grep would find it either way.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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