All of lore.kernel.org
 help / color / mirror / Atom feed
* small oddity in commit "power: reset: add driver for LinkStation power off"
@ 2021-02-18 17:39 ` Daniel Golle
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Golle @ 2021-02-18 17:39 UTC (permalink / raw)
  To: Daniel González Cabanelas; +Cc: linux-pm, linux-arm-kernel

Hi Daniel,

I stumbled upon a slight oddity in acommit you have contributed.
Please see my comment below.

> commit a7f79f99541eff4e6bcae0014eb08d3019337565
> Author: Daniel González Cabanelas <dgcbueu@gmail.com>
> Date:   Wed Jul 15 15:35:14 2020 +0200
> 
>     power: reset: add driver for LinkStation power off
>     
>     Some Buffalo LinkStations perform the power off operation, at restart
>     time, depending on the state of an output pin (LED2/INTn) at the ethernet
>     PHY. This pin is also used to wake the machine when a WoL packet is
>     received by the PHY.
>     
>     The driver is required by the Buffalo LinkStation LS421DE (ARM MVEBU),
>     and other models. Without it, the board remains forever halted if a
>     power off command is executed, unless the PSU is disconnected and
>     connected again.
>     
>     Add the driver to provide the power off function and also make the WoL
>     feature to be available.
>     
>     Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
>     Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ...
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 5710ca4695170..c51eceba9ea39 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_POWER_RESET_GEMINI_POWEROFF) += gemini-poweroff.o
>  obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
>  obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
>  obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
> +obj-${CONFIG_POWER_RESET_LINKSTATION} += linkstation-poweroff.o

Why are you using curly brackets (ie. shell variable) here instead of
normal parentheses (ie. Make variable)? It might work, but if there is
no special reason for this, we should just be consistent with the rest
of the file.


Cheers


Daniel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* small oddity in commit "power: reset: add driver for LinkStation power off"
@ 2021-02-18 17:39 ` Daniel Golle
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Golle @ 2021-02-18 17:39 UTC (permalink / raw)
  To: Daniel González Cabanelas; +Cc: linux-arm-kernel, linux-pm

Hi Daniel,

I stumbled upon a slight oddity in acommit you have contributed.
Please see my comment below.

> commit a7f79f99541eff4e6bcae0014eb08d3019337565
> Author: Daniel González Cabanelas <dgcbueu@gmail.com>
> Date:   Wed Jul 15 15:35:14 2020 +0200
> 
>     power: reset: add driver for LinkStation power off
>     
>     Some Buffalo LinkStations perform the power off operation, at restart
>     time, depending on the state of an output pin (LED2/INTn) at the ethernet
>     PHY. This pin is also used to wake the machine when a WoL packet is
>     received by the PHY.
>     
>     The driver is required by the Buffalo LinkStation LS421DE (ARM MVEBU),
>     and other models. Without it, the board remains forever halted if a
>     power off command is executed, unless the PSU is disconnected and
>     connected again.
>     
>     Add the driver to provide the power off function and also make the WoL
>     feature to be available.
>     
>     Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
>     Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ...
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 5710ca4695170..c51eceba9ea39 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_POWER_RESET_GEMINI_POWEROFF) += gemini-poweroff.o
>  obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
>  obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
>  obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
> +obj-${CONFIG_POWER_RESET_LINKSTATION} += linkstation-poweroff.o

Why are you using curly brackets (ie. shell variable) here instead of
normal parentheses (ie. Make variable)? It might work, but if there is
no special reason for this, we should just be consistent with the rest
of the file.


Cheers


Daniel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: small oddity in commit "power: reset: add driver for LinkStation power off"
  2021-02-18 17:39 ` Daniel Golle
@ 2021-02-18 17:52   ` Daniel González Cabanelas
  -1 siblings, 0 replies; 6+ messages in thread
From: Daniel González Cabanelas @ 2021-02-18 17:52 UTC (permalink / raw)
  To: Daniel Golle; +Cc: linux-pm, linux-arm-kernel

El jue, 18 feb 2021 a las 18:39, Daniel Golle
(<daniel@makrotopia.org>) escribió:
>
> Hi Daniel,
>
> I stumbled upon a slight oddity in acommit you have contributed.
> Please see my comment below.
>
> > commit a7f79f99541eff4e6bcae0014eb08d3019337565
> > Author: Daniel González Cabanelas <dgcbueu@gmail.com>
> > Date:   Wed Jul 15 15:35:14 2020 +0200
> >
> >     power: reset: add driver for LinkStation power off
> >
> >     Some Buffalo LinkStations perform the power off operation, at restart
> >     time, depending on the state of an output pin (LED2/INTn) at the ethernet
> >     PHY. This pin is also used to wake the machine when a WoL packet is
> >     received by the PHY.
> >
> >     The driver is required by the Buffalo LinkStation LS421DE (ARM MVEBU),
> >     and other models. Without it, the board remains forever halted if a
> >     power off command is executed, unless the PSU is disconnected and
> >     connected again.
> >
> >     Add the driver to provide the power off function and also make the WoL
> >     feature to be available.
> >
> >     Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> >     Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ...
> > diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> > index 5710ca4695170..c51eceba9ea39 100644
> > --- a/drivers/power/reset/Makefile
> > +++ b/drivers/power/reset/Makefile
> > @@ -10,6 +10,7 @@ obj-$(CONFIG_POWER_RESET_GEMINI_POWEROFF) += gemini-poweroff.o
> >  obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
> >  obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
> >  obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
> > +obj-${CONFIG_POWER_RESET_LINKSTATION} += linkstation-poweroff.o
>
> Why are you using curly brackets (ie. shell variable) here instead of
> normal parentheses (ie. Make variable)? It might work, but if there is
> no special reason for this, we should just be consistent with the rest
> of the file.

Hi Daniel. Indeed there is no reason to use curly brackets.

I have no idea why I commited this small mistake, probably I need to
graduate my glasses. Feel free to send a patch to fix it.

Thank you
Daniel
>
>
> Cheers
>
>
> Daniel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: small oddity in commit "power: reset: add driver for LinkStation power off"
@ 2021-02-18 17:52   ` Daniel González Cabanelas
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel González Cabanelas @ 2021-02-18 17:52 UTC (permalink / raw)
  To: Daniel Golle; +Cc: linux-arm-kernel, linux-pm

El jue, 18 feb 2021 a las 18:39, Daniel Golle
(<daniel@makrotopia.org>) escribió:
>
> Hi Daniel,
>
> I stumbled upon a slight oddity in acommit you have contributed.
> Please see my comment below.
>
> > commit a7f79f99541eff4e6bcae0014eb08d3019337565
> > Author: Daniel González Cabanelas <dgcbueu@gmail.com>
> > Date:   Wed Jul 15 15:35:14 2020 +0200
> >
> >     power: reset: add driver for LinkStation power off
> >
> >     Some Buffalo LinkStations perform the power off operation, at restart
> >     time, depending on the state of an output pin (LED2/INTn) at the ethernet
> >     PHY. This pin is also used to wake the machine when a WoL packet is
> >     received by the PHY.
> >
> >     The driver is required by the Buffalo LinkStation LS421DE (ARM MVEBU),
> >     and other models. Without it, the board remains forever halted if a
> >     power off command is executed, unless the PSU is disconnected and
> >     connected again.
> >
> >     Add the driver to provide the power off function and also make the WoL
> >     feature to be available.
> >
> >     Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> >     Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ...
> > diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> > index 5710ca4695170..c51eceba9ea39 100644
> > --- a/drivers/power/reset/Makefile
> > +++ b/drivers/power/reset/Makefile
> > @@ -10,6 +10,7 @@ obj-$(CONFIG_POWER_RESET_GEMINI_POWEROFF) += gemini-poweroff.o
> >  obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
> >  obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
> >  obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
> > +obj-${CONFIG_POWER_RESET_LINKSTATION} += linkstation-poweroff.o
>
> Why are you using curly brackets (ie. shell variable) here instead of
> normal parentheses (ie. Make variable)? It might work, but if there is
> no special reason for this, we should just be consistent with the rest
> of the file.

Hi Daniel. Indeed there is no reason to use curly brackets.

I have no idea why I commited this small mistake, probably I need to
graduate my glasses. Feel free to send a patch to fix it.

Thank you
Daniel
>
>
> Cheers
>
>
> Daniel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] power: reset: replace curly brackets in Makefile
  2021-02-18 17:52   ` Daniel González Cabanelas
@ 2021-02-18 18:18     ` Daniel Golle
  -1 siblings, 0 replies; 6+ messages in thread
From: Daniel Golle @ 2021-02-18 18:18 UTC (permalink / raw)
  To: linux-pm, linux-arm-kernel
  Cc: Daniel González Cabanelas, Sebastian Reichel

Normal parentheses should be used when referring to config variables
in Makefile. Replace the accidentally introduced curly brackets by
regular parentheses.

Fixes: a7f79f99541ef ("power: reset: add driver for LinkStation power off")
Acked-by: Daniel González Cabanelas <dgcbueu@gmail.com>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/power/reset/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 4f959b6976066..cf3f4d02d8a54 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -11,7 +11,7 @@ obj-$(CONFIG_POWER_RESET_GEMINI_POWEROFF) += gemini-poweroff.o
 obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
 obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
 obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
-obj-${CONFIG_POWER_RESET_LINKSTATION} += linkstation-poweroff.o
+obj-$(CONFIG_POWER_RESET_LINKSTATION) += linkstation-poweroff.o
 obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
 obj-$(CONFIG_POWER_RESET_MT6323) += mt6323-poweroff.o
 obj-$(CONFIG_POWER_RESET_OXNAS) += oxnas-restart.o
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] power: reset: replace curly brackets in Makefile
@ 2021-02-18 18:18     ` Daniel Golle
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Golle @ 2021-02-18 18:18 UTC (permalink / raw)
  To: linux-pm, linux-arm-kernel
  Cc: Daniel González Cabanelas, Sebastian Reichel

Normal parentheses should be used when referring to config variables
in Makefile. Replace the accidentally introduced curly brackets by
regular parentheses.

Fixes: a7f79f99541ef ("power: reset: add driver for LinkStation power off")
Acked-by: Daniel González Cabanelas <dgcbueu@gmail.com>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/power/reset/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index 4f959b6976066..cf3f4d02d8a54 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -11,7 +11,7 @@ obj-$(CONFIG_POWER_RESET_GEMINI_POWEROFF) += gemini-poweroff.o
 obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
 obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
 obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
-obj-${CONFIG_POWER_RESET_LINKSTATION} += linkstation-poweroff.o
+obj-$(CONFIG_POWER_RESET_LINKSTATION) += linkstation-poweroff.o
 obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
 obj-$(CONFIG_POWER_RESET_MT6323) += mt6323-poweroff.o
 obj-$(CONFIG_POWER_RESET_OXNAS) += oxnas-restart.o
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-18 19:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-18 17:39 small oddity in commit "power: reset: add driver for LinkStation power off" Daniel Golle
2021-02-18 17:39 ` Daniel Golle
2021-02-18 17:52 ` Daniel González Cabanelas
2021-02-18 17:52   ` Daniel González Cabanelas
2021-02-18 18:18   ` [PATCH] power: reset: replace curly brackets in Makefile Daniel Golle
2021-02-18 18:18     ` Daniel Golle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.