* [PATCH 1/2] power:reset: add driver for LinkStation power off
@ 2020-04-25 1:00 Daniel González Cabanelas
2020-04-29 22:53 ` Sebastian Reichel
0 siblings, 1 reply; 3+ messages in thread
From: Daniel González Cabanelas @ 2020-04-25 1:00 UTC (permalink / raw)
To: linux-pm
Some Buffalo LinkStations perform the power off operation, at restart
time, depending on the state of an output pin (LED) at the ethernet 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.
Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
Reviewed-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
drivers/power/reset/Kconfig | 10 ++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/linkstation-poweroff.c | 117 +++++++++++++++++++++
3 files changed, 128 insertions(+)
create mode 100644 drivers/power/reset/linkstation-poweroff.c
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 8903803020..de948835f8 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -203,6 +203,16 @@ config POWER_RESET_KEYSTONE
help
Reboot support for the KEYSTONE SoCs.
+config POWER_RESET_LINKSTATION
+ tristate "Buffalo LinkStation power-off driver"
+ depends on ARCH_MVEBU || COMPILE_TEST
+ depends on OF_MDIO && PHYLIB
+ help
+ Some Buffalo LinkStations use a LED output at the ethernet PHY
+ to inform the board that a power off operation must be performed
+ after restarting. This driver sets the LED to the proper state
+ for restarting or powering off.
+
config POWER_RESET_SYSCON
bool "Generic SYSCON regmap reset driver"
depends on OF
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index da37f8b851..a8c83d2470 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
obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
obj-$(CONFIG_POWER_RESET_MT6323) += mt6323-poweroff.o
obj-$(CONFIG_POWER_RESET_QCOM_PON) += qcom-pon.o
diff --git a/drivers/power/reset/linkstation-poweroff.c b/drivers/power/reset/linkstation-poweroff.c
new file mode 100644
index 0000000000..b5a4cc2c2b
--- /dev/null
+++ b/drivers/power/reset/linkstation-poweroff.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LinkStation power off restart driver
+ * Copyright (C) 2020 Daniel González Cabanelas <dgcbueu@gmail.com>
+ */
+
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of.h>
+#include <linux/of_mdio.h>
+#include <linux/of_platform.h>
+#include <linux/reboot.h>
+#include <linux/phy.h>
+
+#define MII_MARVELL_LED_PAGE 3
+#define MII_PHY_LED_CTRL 16
+
+#define LED2_OFF (0x8 << 8)
+#define LED2_ON (0x9 << 8)
+#define LEDMASK GENMASK(11,8)
+
+static struct phy_device *phydev;
+
+static void mvphy_reg_led(u16 data)
+{
+ int rc;
+ rc = phy_modify_paged(phydev, MII_MARVELL_LED_PAGE,
+ MII_PHY_LED_CTRL, LEDMASK, data);
+ if (rc < 0)
+ dev_err(&phydev->mdio.dev,
+ "LED2 write register failed, %d\n", rc);
+}
+
+static int linkstation_reboot_notifier(struct notifier_block *nb,
+ unsigned long action, void *unused)
+{
+ if (action == SYS_RESTART)
+ mvphy_reg_led(LED2_ON);
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block linkstation_reboot_nb = {
+ .notifier_call = linkstation_reboot_notifier,
+};
+
+static void linkstation_poweroff(void)
+{
+ unregister_reboot_notifier(&linkstation_reboot_nb);
+ mvphy_reg_led(LED2_OFF);
+
+ kernel_restart("Power off");
+}
+
+static int linkstation_poweroff_probe(struct platform_device *pdev)
+{
+ struct mii_bus *bus;
+ struct device_node *dn;
+
+ dn = of_parse_phandle(pdev->dev.of_node, "phy-handle,led", 0);
+
+ if (dn) {
+ phydev = of_phy_find_device(dn);
+ of_node_put(dn);
+ } else {
+ dn = of_find_node_by_name(NULL, "mdio");
+ if (!dn)
+ return -ENODEV;
+
+ bus = of_mdio_find_bus(dn);
+ of_node_put(dn);
+ if (!bus)
+ return -EPROBE_DEFER;
+
+ phydev = phy_find_first(bus);
+ }
+
+ if (!phydev)
+ return -ENODEV;
+
+ register_reboot_notifier(&linkstation_reboot_nb);
+ pm_power_off = linkstation_poweroff;
+
+ dev_info(&pdev->dev, "PHY [%s]\n", phydev_name(phydev));
+
+ return 0;
+}
+
+static int linkstation_poweroff_remove(struct platform_device *pdev)
+{
+ pm_power_off = NULL;
+ unregister_reboot_notifier(&linkstation_reboot_nb);
+
+ return 0;
+}
+
+static const struct of_device_id ls_poweroff_of_match[] = {
+ { .compatible = "linkstation,power-off", },
+ { },
+};
+
+MODULE_DEVICE_TABLE(of, ls_poweroff_of_match);
+
+static struct platform_driver linkstation_poweroff_driver = {
+ .probe = linkstation_poweroff_probe,
+ .remove = linkstation_poweroff_remove,
+ .driver = {
+ .name = "linkstation_power_off",
+ .of_match_table = ls_poweroff_of_match,
+ },
+};
+
+module_platform_driver(linkstation_poweroff_driver);
+
+MODULE_AUTHOR("Daniel González Cabanelas <dgcbueu@gmail.com>");
+MODULE_DESCRIPTION("LinkStation power off driver");
+MODULE_LICENSE("GPL v2");
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] power:reset: add driver for LinkStation power off
2020-04-25 1:00 [PATCH 1/2] power:reset: add driver for LinkStation power off Daniel González Cabanelas
@ 2020-04-29 22:53 ` Sebastian Reichel
2020-04-30 17:27 ` Daniel González Cabanelas
0 siblings, 1 reply; 3+ messages in thread
From: Sebastian Reichel @ 2020-04-29 22:53 UTC (permalink / raw)
To: Daniel González Cabanelas; +Cc: linux-pm
[-- Attachment #1: Type: text/plain, Size: 6198 bytes --]
[please always add the maintainer to Cc or To, in this case sre@kernel.org]
Hi,
On Sat, Apr 25, 2020 at 03:00:43AM +0200, Daniel González Cabanelas wrote:
> Some Buffalo LinkStations perform the power off operation, at restart
> time, depending on the state of an output pin (LED) at the ethernet 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.
>
> Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> Reviewed-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
Thanks for the patch. Looks like that hardware was designed by
creative hardware engineers trying to impress software engineers
with weird designs :)
> drivers/power/reset/Kconfig | 10 ++
> drivers/power/reset/Makefile | 1 +
> drivers/power/reset/linkstation-poweroff.c | 117 +++++++++++++++++++++
> 3 files changed, 128 insertions(+)
> create mode 100644 drivers/power/reset/linkstation-poweroff.c
>
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index 8903803020..de948835f8 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -203,6 +203,16 @@ config POWER_RESET_KEYSTONE
> help
> Reboot support for the KEYSTONE SoCs.
>
> +config POWER_RESET_LINKSTATION
> + tristate "Buffalo LinkStation power-off driver"
> + depends on ARCH_MVEBU || COMPILE_TEST
> + depends on OF_MDIO && PHYLIB
> + help
> + Some Buffalo LinkStations use a LED output at the ethernet PHY
> + to inform the board that a power off operation must be performed
> + after restarting. This driver sets the LED to the proper state
> + for restarting or powering off.
> +
> config POWER_RESET_SYSCON
> bool "Generic SYSCON regmap reset driver"
> depends on OF
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index da37f8b851..a8c83d2470 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
> obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
> obj-$(CONFIG_POWER_RESET_MT6323) += mt6323-poweroff.o
> obj-$(CONFIG_POWER_RESET_QCOM_PON) += qcom-pon.o
> diff --git a/drivers/power/reset/linkstation-poweroff.c b/drivers/power/reset/linkstation-poweroff.c
> new file mode 100644
> index 0000000000..b5a4cc2c2b
> --- /dev/null
> +++ b/drivers/power/reset/linkstation-poweroff.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LinkStation power off restart driver
> + * Copyright (C) 2020 Daniel González Cabanelas <dgcbueu@gmail.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/of.h>
> +#include <linux/of_mdio.h>
> +#include <linux/of_platform.h>
> +#include <linux/reboot.h>
> +#include <linux/phy.h>
> +
> +#define MII_MARVELL_LED_PAGE 3
> +#define MII_PHY_LED_CTRL 16
> +
> +#define LED2_OFF (0x8 << 8)
> +#define LED2_ON (0x9 << 8)
> +#define LEDMASK GENMASK(11,8)
> +
> +static struct phy_device *phydev;
> +
> +static void mvphy_reg_led(u16 data)
> +{
> + int rc;
> + rc = phy_modify_paged(phydev, MII_MARVELL_LED_PAGE,
> + MII_PHY_LED_CTRL, LEDMASK, data);
> + if (rc < 0)
> + dev_err(&phydev->mdio.dev,
> + "LED2 write register failed, %d\n", rc);
> +}
> +
> +static int linkstation_reboot_notifier(struct notifier_block *nb,
> + unsigned long action, void *unused)
> +{
> + if (action == SYS_RESTART)
> + mvphy_reg_led(LED2_ON);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block linkstation_reboot_nb = {
> + .notifier_call = linkstation_reboot_notifier,
> +};
> +
> +static void linkstation_poweroff(void)
> +{
> + unregister_reboot_notifier(&linkstation_reboot_nb);
> + mvphy_reg_led(LED2_OFF);
> +
> + kernel_restart("Power off");
> +}
> +
> +static int linkstation_poweroff_probe(struct platform_device *pdev)
> +{
> + struct mii_bus *bus;
> + struct device_node *dn;
> +
> + dn = of_parse_phandle(pdev->dev.of_node, "phy-handle,led", 0);
> +
> + if (dn) {
> + phydev = of_phy_find_device(dn);
> + of_node_put(dn);
> + } else {
> + dn = of_find_node_by_name(NULL, "mdio");
> + if (!dn)
> + return -ENODEV;
> +
> + bus = of_mdio_find_bus(dn);
> + of_node_put(dn);
> + if (!bus)
> + return -EPROBE_DEFER;
> +
> + phydev = phy_find_first(bus);
do we need this code, or can we just make phy-handle,led mandatory
and error out otherwise?
> + }
> +
> + if (!phydev)
> + return -ENODEV;
Should this return -EPROBE_DEFER?
> + register_reboot_notifier(&linkstation_reboot_nb);
> + pm_power_off = linkstation_poweroff;
> +
> + dev_info(&pdev->dev, "PHY [%s]\n", phydev_name(phydev));
I don't think this deserves more than dev_dbg()
> + return 0;
> +}
> +
> +static int linkstation_poweroff_remove(struct platform_device *pdev)
> +{
> + pm_power_off = NULL;
> + unregister_reboot_notifier(&linkstation_reboot_nb);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ls_poweroff_of_match[] = {
> + { .compatible = "linkstation,power-off", },
> + { },
> +};
> +
> +MODULE_DEVICE_TABLE(of, ls_poweroff_of_match);
> +
> +static struct platform_driver linkstation_poweroff_driver = {
> + .probe = linkstation_poweroff_probe,
> + .remove = linkstation_poweroff_remove,
> + .driver = {
> + .name = "linkstation_power_off",
> + .of_match_table = ls_poweroff_of_match,
> + },
> +};
> +
> +module_platform_driver(linkstation_poweroff_driver);
> +
> +MODULE_AUTHOR("Daniel González Cabanelas <dgcbueu@gmail.com>");
> +MODULE_DESCRIPTION("LinkStation power off driver");
> +MODULE_LICENSE("GPL v2");
Otherwise looks good to me,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] power:reset: add driver for LinkStation power off
2020-04-29 22:53 ` Sebastian Reichel
@ 2020-04-30 17:27 ` Daniel González Cabanelas
0 siblings, 0 replies; 3+ messages in thread
From: Daniel González Cabanelas @ 2020-04-30 17:27 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm
Hi Sebastian. I''ll apply your suggestions.
Thank you
Daniel
El jue., 30 abr. 2020 a las 0:53, Sebastian Reichel
(<sebastian.reichel@collabora.com>) escribió:
>
> [please always add the maintainer to Cc or To, in this case sre@kernel.org]
>
> Hi,
>
> On Sat, Apr 25, 2020 at 03:00:43AM +0200, Daniel González Cabanelas wrote:
> > Some Buffalo LinkStations perform the power off operation, at restart
> > time, depending on the state of an output pin (LED) at the ethernet 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.
> >
> > Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> > Reviewed-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > ---
>
> Thanks for the patch. Looks like that hardware was designed by
> creative hardware engineers trying to impress software engineers
> with weird designs :)
>
> > drivers/power/reset/Kconfig | 10 ++
> > drivers/power/reset/Makefile | 1 +
> > drivers/power/reset/linkstation-poweroff.c | 117 +++++++++++++++++++++
> > 3 files changed, 128 insertions(+)
> > create mode 100644 drivers/power/reset/linkstation-poweroff.c
> >
> > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> > index 8903803020..de948835f8 100644
> > --- a/drivers/power/reset/Kconfig
> > +++ b/drivers/power/reset/Kconfig
> > @@ -203,6 +203,16 @@ config POWER_RESET_KEYSTONE
> > help
> > Reboot support for the KEYSTONE SoCs.
> >
> > +config POWER_RESET_LINKSTATION
> > + tristate "Buffalo LinkStation power-off driver"
> > + depends on ARCH_MVEBU || COMPILE_TEST
> > + depends on OF_MDIO && PHYLIB
> > + help
> > + Some Buffalo LinkStations use a LED output at the ethernet PHY
> > + to inform the board that a power off operation must be performed
> > + after restarting. This driver sets the LED to the proper state
> > + for restarting or powering off.
> > +
> > config POWER_RESET_SYSCON
> > bool "Generic SYSCON regmap reset driver"
> > depends on OF
> > diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> > index da37f8b851..a8c83d2470 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
> > obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
> > obj-$(CONFIG_POWER_RESET_MT6323) += mt6323-poweroff.o
> > obj-$(CONFIG_POWER_RESET_QCOM_PON) += qcom-pon.o
> > diff --git a/drivers/power/reset/linkstation-poweroff.c b/drivers/power/reset/linkstation-poweroff.c
> > new file mode 100644
> > index 0000000000..b5a4cc2c2b
> > --- /dev/null
> > +++ b/drivers/power/reset/linkstation-poweroff.c
> > @@ -0,0 +1,117 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * LinkStation power off restart driver
> > + * Copyright (C) 2020 Daniel González Cabanelas <dgcbueu@gmail.com>
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/notifier.h>
> > +#include <linux/of.h>
> > +#include <linux/of_mdio.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/reboot.h>
> > +#include <linux/phy.h>
> > +
> > +#define MII_MARVELL_LED_PAGE 3
> > +#define MII_PHY_LED_CTRL 16
> > +
> > +#define LED2_OFF (0x8 << 8)
> > +#define LED2_ON (0x9 << 8)
> > +#define LEDMASK GENMASK(11,8)
> > +
> > +static struct phy_device *phydev;
> > +
> > +static void mvphy_reg_led(u16 data)
> > +{
> > + int rc;
> > + rc = phy_modify_paged(phydev, MII_MARVELL_LED_PAGE,
> > + MII_PHY_LED_CTRL, LEDMASK, data);
> > + if (rc < 0)
> > + dev_err(&phydev->mdio.dev,
> > + "LED2 write register failed, %d\n", rc);
> > +}
> > +
> > +static int linkstation_reboot_notifier(struct notifier_block *nb,
> > + unsigned long action, void *unused)
> > +{
> > + if (action == SYS_RESTART)
> > + mvphy_reg_led(LED2_ON);
> > +
> > + return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block linkstation_reboot_nb = {
> > + .notifier_call = linkstation_reboot_notifier,
> > +};
> > +
> > +static void linkstation_poweroff(void)
> > +{
> > + unregister_reboot_notifier(&linkstation_reboot_nb);
> > + mvphy_reg_led(LED2_OFF);
> > +
> > + kernel_restart("Power off");
> > +}
> > +
> > +static int linkstation_poweroff_probe(struct platform_device *pdev)
> > +{
> > + struct mii_bus *bus;
> > + struct device_node *dn;
> > +
> > + dn = of_parse_phandle(pdev->dev.of_node, "phy-handle,led", 0);
> > +
> > + if (dn) {
> > + phydev = of_phy_find_device(dn);
> > + of_node_put(dn);
> > + } else {
> > + dn = of_find_node_by_name(NULL, "mdio");
> > + if (!dn)
> > + return -ENODEV;
> > +
> > + bus = of_mdio_find_bus(dn);
> > + of_node_put(dn);
> > + if (!bus)
> > + return -EPROBE_DEFER;
> > +
> > + phydev = phy_find_first(bus);
>
> do we need this code, or can we just make phy-handle,led mandatory
> and error out otherwise?
>
> > + }
> > +
> > + if (!phydev)
> > + return -ENODEV;
>
> Should this return -EPROBE_DEFER?
>
> > + register_reboot_notifier(&linkstation_reboot_nb);
> > + pm_power_off = linkstation_poweroff;
> > +
> > + dev_info(&pdev->dev, "PHY [%s]\n", phydev_name(phydev));
>
> I don't think this deserves more than dev_dbg()
>
> > + return 0;
> > +}
> > +
> > +static int linkstation_poweroff_remove(struct platform_device *pdev)
> > +{
> > + pm_power_off = NULL;
> > + unregister_reboot_notifier(&linkstation_reboot_nb);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id ls_poweroff_of_match[] = {
> > + { .compatible = "linkstation,power-off", },
> > + { },
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, ls_poweroff_of_match);
> > +
> > +static struct platform_driver linkstation_poweroff_driver = {
> > + .probe = linkstation_poweroff_probe,
> > + .remove = linkstation_poweroff_remove,
> > + .driver = {
> > + .name = "linkstation_power_off",
> > + .of_match_table = ls_poweroff_of_match,
> > + },
> > +};
> > +
> > +module_platform_driver(linkstation_poweroff_driver);
> > +
> > +MODULE_AUTHOR("Daniel González Cabanelas <dgcbueu@gmail.com>");
> > +MODULE_DESCRIPTION("LinkStation power off driver");
> > +MODULE_LICENSE("GPL v2");
>
> Otherwise looks good to me,
>
> -- Sebastian
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-04-30 17:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-25 1:00 [PATCH 1/2] power:reset: add driver for LinkStation power off Daniel González Cabanelas
2020-04-29 22:53 ` Sebastian Reichel
2020-04-30 17:27 ` Daniel González Cabanelas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).