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]

Message-ID: <576E41FC.1080102@baylibre.com>
Date:	Sat, 25 Jun 2016 10:34:04 +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 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.

Rob Herring was very clear with me with this policy, and it will simplify further driver
architecture change since it will not imply devicetree changes anymore.

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.

Neil
> 
>> ---
>>  Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt | 1 +
>>  drivers/input/misc/pmic8xxx-pwrkey.c                           | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
>> index 588536c..9e530e2 100644
>> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
>> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
>> @@ -8,6 +8,7 @@ PROPERTIES
>>  	Definition: must be one of:
>>  		    "qcom,pm8058-pwrkey"
>>  		    "qcom,pm8921-pwrkey"
>> +		    "qcom,pm8018-pwrkey"
>>  
>>  - reg:
>>  	Usage: required
>> diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c
>> index 67aab86..f6f9b87 100644
>> --- a/drivers/input/misc/pmic8xxx-pwrkey.c
>> +++ b/drivers/input/misc/pmic8xxx-pwrkey.c
>> @@ -446,6 +446,7 @@ static int pmic8xxx_pwrkey_remove(struct platform_device *pdev)
>>  }
>>  
>>  static const struct of_device_id pm8xxx_pwr_key_id_table[] = {
>> +	{ .compatible = "qcom,pm8018-pwrkey", .data = &pm8921_pwrkey_shutdown },
>>  	{ .compatible = "qcom,pm8058-pwrkey", .data = &pm8058_pwrkey_shutdown },
>>  	{ .compatible = "qcom,pm8921-pwrkey", .data = &pm8921_pwrkey_shutdown },
>>  	{ }
>> -- 
>> 1.9.1
>>
>