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

[Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>]

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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>
>>> 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.
> 

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.