On 12/11/2018 6:26 PM, Thierry Reding wrote:
> On Tue, Dec 11, 2018 at 10:18:24AM +0800, Mark Zhang wrote:
>> On 12/10/2018 6:29 PM, Thierry Reding wrote:
>>> On Mon, Dec 10, 2018 at 05:43:56PM +0800, Mark Zhang wrote:
>>>> Add gpio-keys nodes for the power button.
>>>>
>>>> Signed-off-by: Mark Zhang <markz@xxxxxxxxxx>
>>>> ---
>>>> arch/arm64/boot/dts/nvidia/tegra210-p2894.dtsi | 15 +++++++++++++++
>>>> 1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2894.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2894.dtsi
>>>> index ac52286ab9ab..1053f18cc4ae 100644
>>>> --- a/arch/arm64/boot/dts/nvidia/tegra210-p2894.dtsi
>>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210-p2894.dtsi
>>>> @@ -1,5 +1,6 @@
>>>> // SPDX-License-Identifier: GPL-2.0
>>>>
>>>> +#include <dt-bindings/input/input.h>
>>>> #include "tegra210.dtsi"
>>>>
>>>> / {
>>>> @@ -52,6 +53,20 @@
>>>> };
>>>> };
>>>>
>>>> + gpio-keys {
>>>> + compatible = "gpio-keys";
>>>> + gpio-keys,name = "gpio-keys";
>>>
>>> This property is not only undocumented, it's also unsupported. Let's
>>> just drop it. Bonus points for sending in a patch that removes the only
>>> other occurrence of this from:
>>
>> I saw this property is documented here:
>> Documentation/devicetree/bindings/input/gpio-keys.txt
>
> You must be looking at a different version than I. Where is the version
> of that file that you're looking at?
>
>> Also in drivers/input/keyboard/gpio_keys.c, there is driver which is
>> compatible with this property though I didn't read it in detail.
>
> There's no code in that driver to parse this property. There is a
> property which seems to do what you want, but it is called "label".
OH OH I see, you're talking about "gpio-keys,name" this property right?
I was thinking that you're suggesting to remove the whole "gpio-keys"
section. Yeah gpio-keys,name is not supported, and I will send a patch
to remove it in tegra210-smaug.dts as well.
Mark
>
> It's also not necessary since the gpio-keys driver will use the device
> name if no label was defined and the device name in this case should
> match what you're passing in as name ("gpio-keys"), so you're not
> gaining anything by adding that property.
>
>> Anyway could you explain more why this property is not supported?
>>
>>>
>>> arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
>>>
>>> Also, the Shield TV has an LED that's on when the device is powered on,
>>> does that happen automatically in hardware, or would that be done as a
>>> GPIO (or LED) as well?
>>
>> Yeah that "green" LED is actually a PWM LED and the brightness is
>> adjustable. So what this patch tries to introduce is the power button of
>> the Shield which is a GPIO key.
>
> Okay, good. Sounds like it should be easy to add the LED later on.
>
> Thierry
>
