linux-kernel - Re: [PATCH] nokia N9: Add support for magnetometer and touchscreen

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180103115342.wnrvbzvxc4dtixdm@earth.universe>
Date:   Wed, 3 Jan 2018 12:53:42 +0100
From:   Sebastian Reichel <sre@...nel.org>
To:     Pavel Machek <pavel@....cz>
Cc:     pali.rohar@...il.com, kernel list <linux-kernel@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-omap@...r.kernel.org, tony@...mide.com, khilman@...nel.org,
        aaro.koskinen@....fi, ivo.g.dimitrov.75@...il.com,
        patrikbachan@...il.com, serge@...lyn.com, abcloriens@...il.com,
        clayton@...ftyguy.net, martijn@...xit.nl,
        sakari.ailus@...ux.intel.com,
        Filip Matijević <filip.matijevic.pz@...il.com>,
        bcousson@...libre.com, devicetree@...r.kernel.org
Subject: Re: [PATCH] nokia N9: Add support for magnetometer and touchscreen

Hi,

On Wed, Jan 03, 2018 at 11:25:53AM +0100, Pavel Machek wrote:
> On Tue 2018-01-02 18:27:20, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Tue, Jan 02, 2018 at 02:17:22PM +0100, Pavel Machek wrote:
> > > This adds dts support for magnetometer and touchscreen on Nokia N9.
> > 
> > I think it makes sense to have this splitted.
> 
> Creating more work for everyone for little gain? Meh.

More work for everyone? Could you please read Documentation/process
and actually follow it? It has a section about patches containing a
single logical change and you add two completly unrelated devices to
the device tree.

Also I suggest, that you rework your git workflow if this makes
more work than writing two sentences.

> > > diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts
> > > index 39e35f8..57a6679 100644
> > > --- a/arch/arm/boot/dts/omap3-n9.dts
> > > +++ b/arch/arm/boot/dts/omap3-n9.dts
> > > @@ -36,6 +57,22 @@
> > >  			};
> > >  		};
> > >  	};
> > > +
> > > +	touch@4b {
> > 
> > touchscreen@
> 
> Ok.
> 
> > > +		compatible = "atmel,maxtouch";
> > > +		reg = <0x4b>;
> > > +		interrupt-parent = <&gpio2>;
> > > +		interrupts = <29 2>; /* gpio_61, IRQF_TRIGGER_FALLING*/
> > 
> > reset-gpios = <&gpio3 17 GPIO_ACTIVE_SOMETHING>;
> > 
> > > +		vdd-supply = <&vio>;
> > > +		avdd-supply = <&vaux1>;
> > 
> > Those two are not mentioned in the binding and not supported by the
> > driver as far as I can see?
> 
> Driver will need to be fixed, AFAICT :-(.

Obviously. DTS is only updated when binding has been acknowledged,
though. To get this working ASAP you can add "always-enabled;" to
the regulator and remove it once the driver gained support for
enabling the regulators itself.

> > Touchscreen with the same settings is required for n950, so it
> > should be in the shared n950 + n9 file.
> 
> In future, settings will be different for n9/n950: calibration matrix
> is different as panel is rotated in different way. Still it probably
> makes sense to share. Ok.

The differences can be specified in the specific files by doing
something like the following:

n9-n950.dtsi:
ts: touchscreen {
    shared-properties;
};

n9.dts:
&ts {
    only-for-n9;
};

n950.dts:
&ts {
    only-for-n950;
};

> > > +&i2c3 {
> > > +	ak8975@0f {
> > > +		compatible = "asahi-kasei,ak8975";
> > > +		reg = <0x0f>;
> > > +	};
> > >  };
> > 
> > Looking at the N9 board file this is missing a rotation matrix. This
> > is supported by the binding:
> > 
> > Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
> 
> Do you have an idea how the rotation matrix should look like? I don't
> currently have an userland software that could calibrate and test the
> sensor, so I'd prefer to merge basic binding now and do calibration
> later.

Well you take the values from sysfs, multiply it with the rotation
matrix and check if the values look ok. No special userspace tools
required except mount (for sysfs), ls & cat.

-- Sebastian

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