linux-kernel - Re: [PATCH v2 2/5] input: pmic8xxx-pwrkey: Add support for pm8018 pwrkey

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]

Date:	Sun, 26 Jun 2016 15:06:30 +0200
From:	Neil Armstrong <narmstrong@...libre.com>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	andy.gross@...aro.org, david.brown@...aro.org,
	lee.jones@...aro.org, lgirdwood@...il.com, broonie@...nel.org,
	a.zummo@...ertech.it, alexandre.belloni@...e-electrons.com,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
	rtc-linux@...glegroups.com, Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v2 2/5] input: pmic8xxx-pwrkey: Add support for pm8018
 pwrkey

On 06/25/2016 05:53 PM, Dmitry Torokhov wrote:
> On Sat, Jun 25, 2016 at 10:34:04AM +0200, Neil Armstrong wrote:
>> On 06/25/2016 12:07 AM, Dmitry Torokhov wrote:
>>> On Fri, Jun 24, 2016 at 11:18:04AM +0200, Neil Armstrong wrote:
>>>> In order to support pwrkey for Qualcomm MDM9615 SoC, add support
>>>> for the pm8018 pwrkey in pmic8xxx-pwrkey.
>>>>
>>>> Reviewed-by: Bjorn Andersson <bjorn.andersson@...aro.org>
>>>> Signed-off-by: Neil Armstrong <narmstrong@...libre.com>
>>>
>>> NAK.
>>
>> Hi Dmitry,
>>
>> Actually, the new compatible string make sense, because the driver is compatible with the
>> "pm8018" pwrkey but from a system point of view, it's not a pm8921 pwrkey, hence the new
>> compatible string.
> 
> A lot of systems note this fact in DTS, but not require driver changes,
> by specifying several compatible strings:
> 
> 	compatible = "nvidia,tegra114-sdhci", "nvidia,tegra30-sdhci";
> 	compatible = "fsl,imx6q-i2c", "fsl,imx21-i2c";
> 	compatible = "rockchip,rk3036-timer", "rockchip,rk3288-timer";

Sure, your point is valid.
But here, the situation is quite different, the question is about confidence.
>From the system point of view, I'm 100% sure there is a pm8010-pwrkey variant here, but
I`m not convinced at all how it is similar from the 8921 version.
>From the software point of view, I'm 80% sure the *actual* driver in it current form
somehow works for the pm8018-pwrkey, not more.
If somehow the driver is updated to support a 8921 feature that is not supported by the
8018 version, it will rely on the compatible string to make this a smart move.
Since I do not have the pm8018 datasheet and the 8921 either, I cannot statue on this,
So the smartest move from my side is to actually have a different compatible string
to avoid future blocking situations.

>>
>> Rob Herring was very clear with me with this policy, and it will simplify further driver
> 
> Could I get a pointer to this discussion so I can educate myself better
> about DT policies?

I had quite a lot of comments on the OXNAS support push (started here https://lkml.org/lkml/2016/3/3/495) were
the policy was to narrow the new compatible strings to a SoC specific naming.
For the qcom driver, the strings the already compliant and why not continue with the pm8018 ?

>> architecture change since it will not imply devicetree changes anymore.
> 
> Would we need the driver changes? What are the differences in power key
> functionality between 8018 and 8921?

You raise the biggest question, I do not know, so why should we say the pm8018-pwrkey /is/ compatible with pm8921-pwronly only by looking existing driver ?

>>
>> My point of view is that the devicetree describes the hardware and need to have SoC specific
>> compatible string since it describes the actual silicon, and drivers must make sure to handle
>> all the SoC or family variants using the compatible string and the match data.
> 
> No, the compatible string means that the hardware is *compatible* with
> something. It does not mean that we need to adjust driver every time a
> company pumps out a new package including said hardware.

It was something that I questionned myself about, but it seems the maintainers agrees quite easily to accept these compatible adding patches
like the USB Ids or PCI ids patches.

Regards,
Neil

> Thanks.
>