| Message ID | 1426914386-18386-1-git-send-email-fl.service@t-firefly.com |
|---|---|
| State | Accepted |
| Headers | show |
Hi On 21/03/2015 06:06, wengbj wrote: > Signed-off-by: wengbj <fl.service@t-firefly.com> > --- > target/linux/ramips/dts/FIREWRT.dts | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/target/linux/ramips/dts/FIREWRT.dts b/target/linux/ramips/dts/FIREWRT.dts > index 54f0e55..2b018e6 100644 > --- a/target/linux/ramips/dts/FIREWRT.dts > +++ b/target/linux/ramips/dts/FIREWRT.dts > @@ -103,6 +103,11 @@ > gpios = <&gpio0 18 1>; > linux,code = <0x211>; > }; > + power { > + label = "power"; > + gpios = <&gpio0 23 1>; > + linux,code = <116>; > + }; what kind of power button is this ? does the board have a gpio to toggle the supply voltage ? if so please consider also adding a "gpio-poweroff" node. John > }; > > pinctrl { >
On 21 March 2015 at 06:06, wengbj <fl.service@t-firefly.com> wrote: > Signed-off-by: wengbj <fl.service@t-firefly.com> > --- > target/linux/ramips/dts/FIREWRT.dts | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/target/linux/ramips/dts/FIREWRT.dts b/target/linux/ramips/dts/FIREWRT.dts > index 54f0e55..2b018e6 100644 > --- a/target/linux/ramips/dts/FIREWRT.dts > +++ b/target/linux/ramips/dts/FIREWRT.dts > @@ -103,6 +103,11 @@ > gpios = <&gpio0 18 1>; > linux,code = <0x211>; > }; > + power { > + label = "power"; > + gpios = <&gpio0 23 1>; > + linux,code = <116>; > + }; Could you break this bad habit and use KEY_* instead of magic numbers? Like a KEY_POWER.
On 21/03/2015 23:15, Rafał Miłecki wrote: > On 21 March 2015 at 06:06, wengbj <fl.service@t-firefly.com> wrote: >> Signed-off-by: wengbj <fl.service@t-firefly.com> >> --- >> target/linux/ramips/dts/FIREWRT.dts | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/target/linux/ramips/dts/FIREWRT.dts b/target/linux/ramips/dts/FIREWRT.dts >> index 54f0e55..2b018e6 100644 >> --- a/target/linux/ramips/dts/FIREWRT.dts >> +++ b/target/linux/ramips/dts/FIREWRT.dts >> @@ -103,6 +103,11 @@ >> gpios = <&gpio0 18 1>; >> linux,code = <0x211>; >> }; >> + power { >> + label = "power"; >> + gpios = <&gpio0 23 1>; >> + linux,code = <116>; >> + }; > > Could you break this bad habit and use KEY_* instead of magic numbers? > Like a KEY_POWER. > sure but this is out of scope of this patch and needs to be done globally for all dts files in the ralink target.
On 21 March 2015 at 23:18, John Crispin <blogic@openwrt.org> wrote: > > > On 21/03/2015 23:15, Rafał Miłecki wrote: >> On 21 March 2015 at 06:06, wengbj <fl.service@t-firefly.com> wrote: >>> Signed-off-by: wengbj <fl.service@t-firefly.com> >>> --- >>> target/linux/ramips/dts/FIREWRT.dts | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/target/linux/ramips/dts/FIREWRT.dts b/target/linux/ramips/dts/FIREWRT.dts >>> index 54f0e55..2b018e6 100644 >>> --- a/target/linux/ramips/dts/FIREWRT.dts >>> +++ b/target/linux/ramips/dts/FIREWRT.dts >>> @@ -103,6 +103,11 @@ >>> gpios = <&gpio0 18 1>; >>> linux,code = <0x211>; >>> }; >>> + power { >>> + label = "power"; >>> + gpios = <&gpio0 23 1>; >>> + linux,code = <116>; >>> + }; >> >> Could you break this bad habit and use KEY_* instead of magic numbers? >> Like a KEY_POWER. >> > > sure but this is out of scope of this patch and needs to be done > globally for all dts files in the ralink target. I don't mean changing any existing entry, just the one that was suggested to be added. I think it's OK to add it in a correct & nice form?
On 22/03/2015 00:31, Rafał Miłecki wrote: > On 21 March 2015 at 23:18, John Crispin <blogic@openwrt.org> wrote: >> >> >> On 21/03/2015 23:15, Rafał Miłecki wrote: >>> On 21 March 2015 at 06:06, wengbj <fl.service@t-firefly.com> wrote: >>>> Signed-off-by: wengbj <fl.service@t-firefly.com> >>>> --- >>>> target/linux/ramips/dts/FIREWRT.dts | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/target/linux/ramips/dts/FIREWRT.dts b/target/linux/ramips/dts/FIREWRT.dts >>>> index 54f0e55..2b018e6 100644 >>>> --- a/target/linux/ramips/dts/FIREWRT.dts >>>> +++ b/target/linux/ramips/dts/FIREWRT.dts >>>> @@ -103,6 +103,11 @@ >>>> gpios = <&gpio0 18 1>; >>>> linux,code = <0x211>; >>>> }; >>>> + power { >>>> + label = "power"; >>>> + gpios = <&gpio0 23 1>; >>>> + linux,code = <116>; >>>> + }; >>> >>> Could you break this bad habit and use KEY_* instead of magic numbers? >>> Like a KEY_POWER. >>> >> >> sure but this is out of scope of this patch and needs to be done >> globally for all dts files in the ralink target. > > I don't mean changing any existing entry, just the one that was > suggested to be added. > > I think it's OK to add it in a correct & nice form? > i think that is incosistent to the rest of the code. people add code based on what is there. expecting them to magically know what you wish is not consistent tot waht is there. therefore the change needs to be made globally for all targets and not just for one.
Dear John:
We are using "self-lock power switch" . When the switch is pressed, the power will turn on . Original design is a physical switch .
But we want to unmount the sd card , usb device and the harddisk before the power off. So we add a detect pin to check the switch status and add a script to shutdown the power .
The schematic like this :
Best Regards.
Jay Weng
Website: www.t-firefly.com
E-mail: fl.service@t-firefly.com
From: John Crispin
Date: 2015-03-22 04:36
To: wengbj
CC: openwrt-devel; linux.c; wbj; zxf; dxj
Subject: Re: [PATCH 1/2] ralink: add FireWRT power button
Hi
On 21/03/2015 06:06, wengbj wrote:
> Signed-off-by: wengbj <fl.service@t-firefly.com>
> ---
> target/linux/ramips/dts/FIREWRT.dts | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/target/linux/ramips/dts/FIREWRT.dts b/target/linux/ramips/dts/FIREWRT.dts
> index 54f0e55..2b018e6 100644
> --- a/target/linux/ramips/dts/FIREWRT.dts
> +++ b/target/linux/ramips/dts/FIREWRT.dts
> @@ -103,6 +103,11 @@
> gpios = <&gpio0 18 1>;
> linux,code = <0x211>;
> };
> + power {
> + label = "power";
> + gpios = <&gpio0 23 1>;
> + linux,code = <116>;
> + };
what kind of power button is this ? does the board have a gpio to toggle
the supply voltage ? if so please consider also adding a "gpio-poweroff"
node.
John
>
};
>
> pinctrl {
>
diff --git a/target/linux/ramips/dts/FIREWRT.dts b/target/linux/ramips/dts/FIREWRT.dts index 54f0e55..2b018e6 100644 --- a/target/linux/ramips/dts/FIREWRT.dts +++ b/target/linux/ramips/dts/FIREWRT.dts @@ -103,6 +103,11 @@ gpios = <&gpio0 18 1>; linux,code = <0x211>; }; + power { + label = "power"; + gpios = <&gpio0 23 1>; + linux,code = <116>; + }; }; pinctrl {
Signed-off-by: wengbj <fl.service@t-firefly.com> --- target/linux/ramips/dts/FIREWRT.dts | 5 +++++ 1 file changed, 5 insertions(+)