* [PATCH v3] power: reset: at91-reset: add sysfs interface to the power on reason
@ 2019-12-09 9:43 Kamel Bouhara
2019-12-09 11:33 ` Claudiu.Beznea
0 siblings, 1 reply; 5+ messages in thread
From: Kamel Bouhara @ 2019-12-09 9:43 UTC (permalink / raw)
To: Sebastian Reichel, Nicolas Ferre, Alexandre Belloni,
Ludovic Desroches, linux-arm-kernel
Cc: Kamel Bouhara, Thomas Petazzoni
This patch export the power on reason through the sysfs interface and
introduce some generic reset sources.
Update the ABI documentation to list current power on sources.
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
Changes in v2
=============
- Be less specific on the crystal oscillator value
- Add an Acked-by
Changes in v3
=============
- Really be less specific on the crystal oscillator value
.../sysfs-devices-platform-power-on-reason | 14 ++++++
drivers/power/reset/at91-reset.c | 44 +++++++++++++------
include/linux/power/power_on_reason.h | 19 ++++++++
3 files changed, 64 insertions(+), 13 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
create mode 100644 include/linux/power/power_on_reason.h
diff --git a/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason b/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
new file mode 100644
index 000000000000..83daeb9b1aa2
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
@@ -0,0 +1,14 @@
+What: /sys/devices/platform/.../power_on_reason
+
+Date: October 2019
+KernelVersion: 5.4
+Contact: Kamel Bouhara <kamel.bouhara@bootlin.com>
+Description: This file shows system power on reason.
+ The possible sources are:
+ General System Power-ON, RTC wakeup, Watchdog timeout,
+ Software Reset, User pressed reset button,
+ CPU Clock failure, 32.768kHz Oscillator Failure,
+ Low power mode exit, Unknown.
+
+ The file is read only.
+
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 44ca983a49a1..3cb2df40af37 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -17,7 +17,7 @@
#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/reboot.h>
-
+#include <linux/power/power_on_reason.h>
#include <soc/at91/at91sam9_ddrsdr.h>
#include <soc/at91/at91sam9_sdramc.h>
@@ -146,42 +146,42 @@ static int samx7_restart(struct notifier_block *this, unsigned long mode,
return NOTIFY_DONE;
}
-static void __init at91_reset_status(struct platform_device *pdev)
+static const char *at91_reset_reason(struct platform_device *pdev)
{
const char *reason;
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
switch ((reg & AT91_RSTC_RSTTYP) >> 8) {
case RESET_TYPE_GENERAL:
- reason = "general reset";
+ reason = POWER_ON_REASON_GENERAL;
break;
case RESET_TYPE_WAKEUP:
- reason = "wakeup";
+ reason = POWER_ON_REASON_RTC;
break;
case RESET_TYPE_WATCHDOG:
- reason = "watchdog reset";
+ reason = POWER_ON_REASON_WATCHDOG;
break;
case RESET_TYPE_SOFTWARE:
- reason = "software reset";
+ reason = POWER_ON_REASON_SOFTWARE;
break;
case RESET_TYPE_USER:
- reason = "user reset";
+ reason = POWER_ON_REASON_USER;
break;
case RESET_TYPE_CPU_FAIL:
- reason = "CPU clock failure detection";
+ reason = POWER_ON_REASON_CPU_FAIL;
break;
case RESET_TYPE_XTAL_FAIL:
- reason = "32.768 kHz crystal failure detection";
+ reason = POWER_ON_REASON_XTAL_FAIL;
break;
case RESET_TYPE_ULP2:
- reason = "ULP2 reset";
+ reason = POWER_ON_REASON_LOW_POWER;
break;
default:
- reason = "unknown reset";
+ reason = POWER_ON_REASON_UNKNOWN;
break;
}
- dev_info(&pdev->dev, "Starting after %s\n", reason);
+ return reason;
}
static const struct of_device_id at91_ramc_of_match[] = {
@@ -204,6 +204,17 @@ static struct notifier_block at91_restart_nb = {
.priority = 192,
};
+static ssize_t power_on_reason_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+
+ return sprintf(buf, "%s\n", at91_reset_reason(pdev));
+}
+
+static DEVICE_ATTR_RO(power_on_reason);
+
static int __init at91_reset_probe(struct platform_device *pdev)
{
const struct of_device_id *match;
@@ -248,7 +259,14 @@ static int __init at91_reset_probe(struct platform_device *pdev)
return ret;
}
- at91_reset_status(pdev);
+ ret = device_create_file(&pdev->dev, &dev_attr_power_on_reason);
+ if (ret) {
+ dev_err(&pdev->dev, "Could not create sysfs entry\n");
+ return ret;
+ }
+
+ dev_info(&pdev->dev, "Starting after %s reset\n",
+ at91_reset_reason(pdev));
return 0;
}
diff --git a/include/linux/power/power_on_reason.h b/include/linux/power/power_on_reason.h
new file mode 100644
index 000000000000..9c44baa52911
--- /dev/null
+++ b/include/linux/power/power_on_reason.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Author: Kamel Bouhra <kamel.bouhara@bootlin.com>
+ */
+
+#ifndef POWER_ON_REASON_H
+#define POWER_ON_REASON_H
+
+#define POWER_ON_REASON_GENERAL "General"
+#define POWER_ON_REASON_RTC "RTC wakeup"
+#define POWER_ON_REASON_WATCHDOG "Watchdog timeout"
+#define POWER_ON_REASON_SOFTWARE "Software"
+#define POWER_ON_REASON_USER "User"
+#define POWER_ON_REASON_CPU_FAIL "CPU clock Failure"
+#define POWER_ON_REASON_XTAL_FAIL "Crystal oscillator Failure"
+#define POWER_ON_REASON_LOW_POWER "Low power exit"
+#define POWER_ON_REASON_UNKNOWN "Unknown"
+
+#endif /* POWER_ON_REASON_H */
--
2.24.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] 5+ messages in thread
* Re: [PATCH v3] power: reset: at91-reset: add sysfs interface to the power on reason
2019-12-09 9:43 [PATCH v3] power: reset: at91-reset: add sysfs interface to the power on reason Kamel Bouhara
@ 2019-12-09 11:33 ` Claudiu.Beznea
2019-12-09 13:44 ` Kamel Bouhara
0 siblings, 1 reply; 5+ messages in thread
From: Claudiu.Beznea @ 2019-12-09 11:33 UTC (permalink / raw)
To: kamel.bouhara, sre, Nicolas.Ferre, alexandre.belloni,
Ludovic.Desroches, linux-arm-kernel
Cc: thomas.petazzoni
Hi Kamel,
On 09.12.2019 11:43, Kamel Bouhara wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> This patch export the power on reason through the sysfs interface and
> introduce some generic reset sources.
> Update the ABI documentation to list current power on sources.
>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> ---
> Changes in v2
> =============
> - Be less specific on the crystal oscillator value
> - Add an Acked-by
> Changes in v3
> =============
> - Really be less specific on the crystal oscillator value
>
> .../sysfs-devices-platform-power-on-reason | 14 ++++++
> drivers/power/reset/at91-reset.c | 44 +++++++++++++------
> include/linux/power/power_on_reason.h | 19 ++++++++
> 3 files changed, 64 insertions(+), 13 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
> create mode 100644 include/linux/power/power_on_reason.h
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason b/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
> new file mode 100644
> index 000000000000..83daeb9b1aa2
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
> @@ -0,0 +1,14 @@
> +What: /sys/devices/platform/.../power_on_reason
> +
> +Date: October 2019
> +KernelVersion: 5.4
> +Contact: Kamel Bouhara <kamel.bouhara@bootlin.com>
> +Description: This file shows system power on reason.
> + The possible sources are:
> + General System Power-ON, RTC wakeup, Watchdog timeout,
> + Software Reset, User pressed reset button,
> + CPU Clock failure, 32.768kHz Oscillator Failure,
Crystal oscillator value is still present here.
> + Low power mode exit, Unknown.
> +
> + The file is read only.
> +
> diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
> index 44ca983a49a1..3cb2df40af37 100644
> --- a/drivers/power/reset/at91-reset.c
> +++ b/drivers/power/reset/at91-reset.c
> @@ -17,7 +17,7 @@
> #include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/reboot.h>
> -
> +#include <linux/power/power_on_reason.h>
As far as I know, headers in include/linux are only visible in kernel.
Although you may use this header, in future, in other drivers (as Alexandre
specified in a previous email), at the moment it is only used by
at91-reset.c. So, why not keeping them in at91-reset.c or leave it as is
for the moment and introduce it when this will be necessary?
Moreover, you are doing 2 things on a patch:
1/ export the reset reasons through sysfs
2/ introduce the reset reason defines
Thank you,
Claudiu Beznea
> #include <soc/at91/at91sam9_ddrsdr.h>
> #include <soc/at91/at91sam9_sdramc.h>
>
> @@ -146,42 +146,42 @@ static int samx7_restart(struct notifier_block *this, unsigned long mode,
> return NOTIFY_DONE;
> }
>
> -static void __init at91_reset_status(struct platform_device *pdev)
> +static const char *at91_reset_reason(struct platform_device *pdev)
> {
> const char *reason;
> u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
>
> switch ((reg & AT91_RSTC_RSTTYP) >> 8) {
> case RESET_TYPE_GENERAL:
> - reason = "general reset";
> + reason = POWER_ON_REASON_GENERAL;
> break;
> case RESET_TYPE_WAKEUP:
> - reason = "wakeup";
> + reason = POWER_ON_REASON_RTC;
> break;
> case RESET_TYPE_WATCHDOG:
> - reason = "watchdog reset";
> + reason = POWER_ON_REASON_WATCHDOG;
> break;
> case RESET_TYPE_SOFTWARE:
> - reason = "software reset";
> + reason = POWER_ON_REASON_SOFTWARE;
> break;
> case RESET_TYPE_USER:
> - reason = "user reset";
> + reason = POWER_ON_REASON_USER;
> break;
> case RESET_TYPE_CPU_FAIL:
> - reason = "CPU clock failure detection";
> + reason = POWER_ON_REASON_CPU_FAIL;
> break;
> case RESET_TYPE_XTAL_FAIL:
> - reason = "32.768 kHz crystal failure detection";
> + reason = POWER_ON_REASON_XTAL_FAIL;
> break;
> case RESET_TYPE_ULP2:
> - reason = "ULP2 reset";
> + reason = POWER_ON_REASON_LOW_POWER;
> break;
> default:
> - reason = "unknown reset";
> + reason = POWER_ON_REASON_UNKNOWN;
> break;
> }
>
> - dev_info(&pdev->dev, "Starting after %s\n", reason);
> + return reason;
> }
>
> static const struct of_device_id at91_ramc_of_match[] = {
> @@ -204,6 +204,17 @@ static struct notifier_block at91_restart_nb = {
> .priority = 192,
> };
>
> +static ssize_t power_on_reason_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + return sprintf(buf, "%s\n", at91_reset_reason(pdev));
> +}
> +
> +static DEVICE_ATTR_RO(power_on_reason);
> +
> static int __init at91_reset_probe(struct platform_device *pdev)
> {
> const struct of_device_id *match;
> @@ -248,7 +259,14 @@ static int __init at91_reset_probe(struct platform_device *pdev)
> return ret;
> }
>
> - at91_reset_status(pdev);
> + ret = device_create_file(&pdev->dev, &dev_attr_power_on_reason);
> + if (ret) {
> + dev_err(&pdev->dev, "Could not create sysfs entry\n");
> + return ret;
> + }
> +
> + dev_info(&pdev->dev, "Starting after %s reset\n",
> + at91_reset_reason(pdev));
>
> return 0;
> }
> diff --git a/include/linux/power/power_on_reason.h b/include/linux/power/power_on_reason.h
> new file mode 100644
> index 000000000000..9c44baa52911
> --- /dev/null
> +++ b/include/linux/power/power_on_reason.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Author: Kamel Bouhra <kamel.bouhara@bootlin.com>
> + */
> +
> +#ifndef POWER_ON_REASON_H
> +#define POWER_ON_REASON_H
> +
> +#define POWER_ON_REASON_GENERAL "General"
> +#define POWER_ON_REASON_RTC "RTC wakeup"
> +#define POWER_ON_REASON_WATCHDOG "Watchdog timeout"
> +#define POWER_ON_REASON_SOFTWARE "Software"
> +#define POWER_ON_REASON_USER "User"
> +#define POWER_ON_REASON_CPU_FAIL "CPU clock Failure"
> +#define POWER_ON_REASON_XTAL_FAIL "Crystal oscillator Failure"
> +#define POWER_ON_REASON_LOW_POWER "Low power exit"
> +#define POWER_ON_REASON_UNKNOWN "Unknown"
> +
> +#endif /* POWER_ON_REASON_H */
> --
> 2.24.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
_______________________________________________
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] 5+ messages in thread
* Re: [PATCH v3] power: reset: at91-reset: add sysfs interface to the power on reason
2019-12-09 11:33 ` Claudiu.Beznea
@ 2019-12-09 13:44 ` Kamel Bouhara
2019-12-19 17:13 ` Sebastian Reichel
0 siblings, 1 reply; 5+ messages in thread
From: Kamel Bouhara @ 2019-12-09 13:44 UTC (permalink / raw)
To: Claudiu.Beznea
Cc: alexandre.belloni, sre, Ludovic.Desroches, thomas.petazzoni,
linux-arm-kernel
On Mon, Dec 09, 2019 at 11:33:23AM +0000, Claudiu.Beznea@microchip.com wrote:
> Hi Kamel,
>
> On 09.12.2019 11:43, Kamel Bouhara wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > This patch export the power on reason through the sysfs interface and
> > introduce some generic reset sources.
> > Update the ABI documentation to list current power on sources.
> >
> > Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> > Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> > ---
> > Changes in v2
> > =============
> > - Be less specific on the crystal oscillator value
> > - Add an Acked-by
> > Changes in v3
> > =============
> > - Really be less specific on the crystal oscillator value
> >
> > .../sysfs-devices-platform-power-on-reason | 14 ++++++
> > drivers/power/reset/at91-reset.c | 44 +++++++++++++------
> > include/linux/power/power_on_reason.h | 19 ++++++++
> > 3 files changed, 64 insertions(+), 13 deletions(-)
> > create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
> > create mode 100644 include/linux/power/power_on_reason.h
> >
> > diff --git a/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason b/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
> > new file mode 100644
> > index 000000000000..83daeb9b1aa2
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
> > @@ -0,0 +1,14 @@
> > +What: /sys/devices/platform/.../power_on_reason
> > +
> > +Date: October 2019
> > +KernelVersion: 5.4
> > +Contact: Kamel Bouhara <kamel.bouhara@bootlin.com>
> > +Description: This file shows system power on reason.
> > + The possible sources are:
> > + General System Power-ON, RTC wakeup, Watchdog timeout,
> > + Software Reset, User pressed reset button,
> > + CPU Clock failure, 32.768kHz Oscillator Failure,
>
> Crystal oscillator value is still present here.
>
Indeed, thanks.
> > + Low power mode exit, Unknown.
> > +
> > + The file is read only.
> > +
> > diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
> > index 44ca983a49a1..3cb2df40af37 100644
> > --- a/drivers/power/reset/at91-reset.c
> > +++ b/drivers/power/reset/at91-reset.c
> > @@ -17,7 +17,7 @@
> > #include <linux/of_address.h>
> > #include <linux/platform_device.h>
> > #include <linux/reboot.h>
> > -
> > +#include <linux/power/power_on_reason.h>
>
> As far as I know, headers in include/linux are only visible in kernel.
> Although you may use this header, in future, in other drivers (as Alexandre
> specified in a previous email), at the moment it is only used by
> at91-reset.c. So, why not keeping them in at91-reset.c or leave it as is
> for the moment and introduce it when this will be necessary?
>
Well, It's been a while now the idea was proposed, I've just submitted
it here.
> Moreover, you are doing 2 things on a patch:
> 1/ export the reset reasons through sysfs
> 2/ introduce the reset reason defines
>
Ok, I shall split it in two patches, if it could clarify things ?
> Thank you,
> Claudiu Beznea
>
> > #include <soc/at91/at91sam9_ddrsdr.h>
> > #include <soc/at91/at91sam9_sdramc.h>
> >
> > @@ -146,42 +146,42 @@ static int samx7_restart(struct notifier_block *this, unsigned long mode,
> > return NOTIFY_DONE;
> > }
> >
> > -static void __init at91_reset_status(struct platform_device *pdev)
> > +static const char *at91_reset_reason(struct platform_device *pdev)
> > {
> > const char *reason;
> > u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
> >
> > switch ((reg & AT91_RSTC_RSTTYP) >> 8) {
> > case RESET_TYPE_GENERAL:
> > - reason = "general reset";
> > + reason = POWER_ON_REASON_GENERAL;
> > break;
> > case RESET_TYPE_WAKEUP:
> > - reason = "wakeup";
> > + reason = POWER_ON_REASON_RTC;
> > break;
> > case RESET_TYPE_WATCHDOG:
> > - reason = "watchdog reset";
> > + reason = POWER_ON_REASON_WATCHDOG;
> > break;
> > case RESET_TYPE_SOFTWARE:
> > - reason = "software reset";
> > + reason = POWER_ON_REASON_SOFTWARE;
> > break;
> > case RESET_TYPE_USER:
> > - reason = "user reset";
> > + reason = POWER_ON_REASON_USER;
> > break;
> > case RESET_TYPE_CPU_FAIL:
> > - reason = "CPU clock failure detection";
> > + reason = POWER_ON_REASON_CPU_FAIL;
> > break;
> > case RESET_TYPE_XTAL_FAIL:
> > - reason = "32.768 kHz crystal failure detection";
> > + reason = POWER_ON_REASON_XTAL_FAIL;
> > break;
> > case RESET_TYPE_ULP2:
> > - reason = "ULP2 reset";
> > + reason = POWER_ON_REASON_LOW_POWER;
> > break;
> > default:
> > - reason = "unknown reset";
> > + reason = POWER_ON_REASON_UNKNOWN;
> > break;
> > }
> >
> > - dev_info(&pdev->dev, "Starting after %s\n", reason);
> > + return reason;
> > }
> >
> > static const struct of_device_id at91_ramc_of_match[] = {
> > @@ -204,6 +204,17 @@ static struct notifier_block at91_restart_nb = {
> > .priority = 192,
> > };
> >
> > +static ssize_t power_on_reason_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct platform_device *pdev = to_platform_device(dev);
> > +
> > + return sprintf(buf, "%s\n", at91_reset_reason(pdev));
> > +}
> > +
> > +static DEVICE_ATTR_RO(power_on_reason);
> > +
> > static int __init at91_reset_probe(struct platform_device *pdev)
> > {
> > const struct of_device_id *match;
> > @@ -248,7 +259,14 @@ static int __init at91_reset_probe(struct platform_device *pdev)
> > return ret;
> > }
> >
> > - at91_reset_status(pdev);
> > + ret = device_create_file(&pdev->dev, &dev_attr_power_on_reason);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Could not create sysfs entry\n");
> > + return ret;
> > + }
> > +
> > + dev_info(&pdev->dev, "Starting after %s reset\n",
> > + at91_reset_reason(pdev));
> >
> > return 0;
> > }
> > diff --git a/include/linux/power/power_on_reason.h b/include/linux/power/power_on_reason.h
> > new file mode 100644
> > index 000000000000..9c44baa52911
> > --- /dev/null
> > +++ b/include/linux/power/power_on_reason.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Author: Kamel Bouhra <kamel.bouhara@bootlin.com>
> > + */
> > +
> > +#ifndef POWER_ON_REASON_H
> > +#define POWER_ON_REASON_H
> > +
> > +#define POWER_ON_REASON_GENERAL "General"
> > +#define POWER_ON_REASON_RTC "RTC wakeup"
> > +#define POWER_ON_REASON_WATCHDOG "Watchdog timeout"
> > +#define POWER_ON_REASON_SOFTWARE "Software"
> > +#define POWER_ON_REASON_USER "User"
> > +#define POWER_ON_REASON_CPU_FAIL "CPU clock Failure"
> > +#define POWER_ON_REASON_XTAL_FAIL "Crystal oscillator Failure"
> > +#define POWER_ON_REASON_LOW_POWER "Low power exit"
> > +#define POWER_ON_REASON_UNKNOWN "Unknown"
> > +
> > +#endif /* POWER_ON_REASON_H */
> > --
> > 2.24.0
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
--
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
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] 5+ messages in thread
* Re: [PATCH v3] power: reset: at91-reset: add sysfs interface to the power on reason
2019-12-09 13:44 ` Kamel Bouhara
@ 2019-12-19 17:13 ` Sebastian Reichel
2019-12-20 10:07 ` Kamel Bouhara
0 siblings, 1 reply; 5+ messages in thread
From: Sebastian Reichel @ 2019-12-19 17:13 UTC (permalink / raw)
To: Kamel Bouhara
Cc: alexandre.belloni, Ludovic.Desroches, thomas.petazzoni,
Claudiu.Beznea, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 8846 bytes --]
Hi,
On Mon, Dec 09, 2019 at 02:44:58PM +0100, Kamel Bouhara wrote:
> On Mon, Dec 09, 2019 at 11:33:23AM +0000, Claudiu.Beznea@microchip.com wrote:
> > On 09.12.2019 11:43, Kamel Bouhara wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > >
> > > This patch export the power on reason through the sysfs interface and
> > > introduce some generic reset sources.
> > > Update the ABI documentation to list current power on sources.
> > >
> > > Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> > > Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> > > ---
> > > Changes in v2
> > > =============
> > > - Be less specific on the crystal oscillator value
> > > - Add an Acked-by
> > > Changes in v3
> > > =============
> > > - Really be less specific on the crystal oscillator value
> > >
> > > .../sysfs-devices-platform-power-on-reason | 14 ++++++
> > > drivers/power/reset/at91-reset.c | 44 +++++++++++++------
> > > include/linux/power/power_on_reason.h | 19 ++++++++
> > > 3 files changed, 64 insertions(+), 13 deletions(-)
> > > create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
> > > create mode 100644 include/linux/power/power_on_reason.h
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason b/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
> > > new file mode 100644
> > > index 000000000000..83daeb9b1aa2
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-devices-platform-power-on-reason
> > > @@ -0,0 +1,14 @@
> > > +What: /sys/devices/platform/.../power_on_reason
> > > +
> > > +Date: October 2019
> > > +KernelVersion: 5.4
> > > +Contact: Kamel Bouhara <kamel.bouhara@bootlin.com>
> > > +Description: This file shows system power on reason.
> > > + The possible sources are:
> > > + General System Power-ON, RTC wakeup, Watchdog timeout,
> > > + Software Reset, User pressed reset button,
> > > + CPU Clock failure, 32.768kHz Oscillator Failure,
> >
> > Crystal oscillator value is still present here.
> >
>
> Indeed, thanks.
>
> > > + Low power mode exit, Unknown.
> > > +
> > > + The file is read only.
> > > +
> > > diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
> > > index 44ca983a49a1..3cb2df40af37 100644
> > > --- a/drivers/power/reset/at91-reset.c
> > > +++ b/drivers/power/reset/at91-reset.c
> > > @@ -17,7 +17,7 @@
> > > #include <linux/of_address.h>
> > > #include <linux/platform_device.h>
> > > #include <linux/reboot.h>
> > > -
> > > +#include <linux/power/power_on_reason.h>
> >
> > As far as I know, headers in include/linux are only visible in kernel.
> > Although you may use this header, in future, in other drivers (as Alexandre
> > specified in a previous email), at the moment it is only used by
> > at91-reset.c. So, why not keeping them in at91-reset.c or leave it as is
> > for the moment and introduce it when this will be necessary?
> >
>
> Well, It's been a while now the idea was proposed, I've just submitted
> it here.
>
> > Moreover, you are doing 2 things on a patch:
> > 1/ export the reset reasons through sysfs
> > 2/ introduce the reset reason defines
>
> Ok, I shall split it in two patches, if it could clarify things ?
Yes, please split into two patches.
> > Thank you,
> > Claudiu Beznea
> >
> > > #include <soc/at91/at91sam9_ddrsdr.h>
> > > #include <soc/at91/at91sam9_sdramc.h>
> > >
> > > @@ -146,42 +146,42 @@ static int samx7_restart(struct notifier_block *this, unsigned long mode,
> > > return NOTIFY_DONE;
> > > }
> > >
> > > -static void __init at91_reset_status(struct platform_device *pdev)
> > > +static const char *at91_reset_reason(struct platform_device *pdev)
> > > {
> > > const char *reason;
> > > u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
> > >
> > > switch ((reg & AT91_RSTC_RSTTYP) >> 8) {
> > > case RESET_TYPE_GENERAL:
> > > - reason = "general reset";
> > > + reason = POWER_ON_REASON_GENERAL;
> > > break;
> > > case RESET_TYPE_WAKEUP:
> > > - reason = "wakeup";
> > > + reason = POWER_ON_REASON_RTC;
> > > break;
> > > case RESET_TYPE_WATCHDOG:
> > > - reason = "watchdog reset";
> > > + reason = POWER_ON_REASON_WATCHDOG;
> > > break;
> > > case RESET_TYPE_SOFTWARE:
> > > - reason = "software reset";
> > > + reason = POWER_ON_REASON_SOFTWARE;
> > > break;
> > > case RESET_TYPE_USER:
> > > - reason = "user reset";
> > > + reason = POWER_ON_REASON_USER;
> > > break;
> > > case RESET_TYPE_CPU_FAIL:
> > > - reason = "CPU clock failure detection";
> > > + reason = POWER_ON_REASON_CPU_FAIL;
> > > break;
> > > case RESET_TYPE_XTAL_FAIL:
> > > - reason = "32.768 kHz crystal failure detection";
> > > + reason = POWER_ON_REASON_XTAL_FAIL;
> > > break;
> > > case RESET_TYPE_ULP2:
> > > - reason = "ULP2 reset";
> > > + reason = POWER_ON_REASON_LOW_POWER;
> > > break;
> > > default:
> > > - reason = "unknown reset";
> > > + reason = POWER_ON_REASON_UNKNOWN;
> > > break;
> > > }
> > >
> > > - dev_info(&pdev->dev, "Starting after %s\n", reason);
> > > + return reason;
> > > }
> > >
> > > static const struct of_device_id at91_ramc_of_match[] = {
> > > @@ -204,6 +204,17 @@ static struct notifier_block at91_restart_nb = {
> > > .priority = 192,
> > > };
> > >
> > > +static ssize_t power_on_reason_show(struct device *dev,
> > > + struct device_attribute *attr,
> > > + char *buf)
> > > +{
> > > + struct platform_device *pdev = to_platform_device(dev);
> > > +
> > > + return sprintf(buf, "%s\n", at91_reset_reason(pdev));
> > > +}
> > > +
> > > +static DEVICE_ATTR_RO(power_on_reason);
> > > +
> > > static int __init at91_reset_probe(struct platform_device *pdev)
> > > {
> > > const struct of_device_id *match;
> > > @@ -248,7 +259,14 @@ static int __init at91_reset_probe(struct platform_device *pdev)
> > > return ret;
> > > }
> > >
> > > - at91_reset_status(pdev);
> > > + ret = device_create_file(&pdev->dev, &dev_attr_power_on_reason);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "Could not create sysfs entry\n");
> > > + return ret;
> > > + }
> > > +
> > > + dev_info(&pdev->dev, "Starting after %s reset\n",
> > > + at91_reset_reason(pdev));
> > >
> > > return 0;
> > > }
> > > diff --git a/include/linux/power/power_on_reason.h b/include/linux/power/power_on_reason.h
> > > new file mode 100644
> > > index 000000000000..9c44baa52911
> > > --- /dev/null
> > > +++ b/include/linux/power/power_on_reason.h
> > > @@ -0,0 +1,19 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Author: Kamel Bouhra <kamel.bouhara@bootlin.com>
> > > + */
> > > +
> > > +#ifndef POWER_ON_REASON_H
> > > +#define POWER_ON_REASON_H
> > > +
> > > +#define POWER_ON_REASON_GENERAL "General"
> > > +#define POWER_ON_REASON_RTC "RTC wakeup"
> > > +#define POWER_ON_REASON_WATCHDOG "Watchdog timeout"
> > > +#define POWER_ON_REASON_SOFTWARE "Software"
> > > +#define POWER_ON_REASON_USER "User"
Is this user reset button from the sysfs file? I think just "User"
is a bit too short. Also please make sure, that the list actually
matches the API described in the documentation :)
-- Sebastian
> > > +#define POWER_ON_REASON_CPU_FAIL "CPU clock Failure"
> > > +#define POWER_ON_REASON_XTAL_FAIL "Crystal oscillator Failure"
> > > +#define POWER_ON_REASON_LOW_POWER "Low power exit"
> > > +#define POWER_ON_REASON_UNKNOWN "Unknown"
> > > +
> > > +#endif /* POWER_ON_REASON_H */
> > > --
> > > 2.24.0
> > >
> > >
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > >
>
> --
> Kamel Bouhara, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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] 5+ messages in thread
* Re: [PATCH v3] power: reset: at91-reset: add sysfs interface to the power on reason
2019-12-19 17:13 ` Sebastian Reichel
@ 2019-12-20 10:07 ` Kamel Bouhara
0 siblings, 0 replies; 5+ messages in thread
From: Kamel Bouhara @ 2019-12-20 10:07 UTC (permalink / raw)
To: Sebastian Reichel
Cc: alexandre.belloni, Ludovic.Desroches, thomas.petazzoni,
Claudiu.Beznea, linux-arm-kernel
On Thu, Dec 19, 2019 at 06:13:14PM +0100, Sebastian Reichel wrote:
> Hi,
>
Hi,
[...]
> >
> > > Moreover, you are doing 2 things on a patch:
> > > 1/ export the reset reasons through sysfs
> > > 2/ introduce the reset reason defines
> >
> > Ok, I shall split it in two patches, if it could clarify things ?
>
> Yes, please split into two patches.
>
OK.
[...]
>
> Is this user reset button from the sysfs file? I think just "User"
> is a bit too short. Also please make sure, that the list actually
> matches the API described in the documentation :)
>
OK, fixed in v4, please check.
Cheers.
Kamel
--
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
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] 5+ messages in thread
end of thread, other threads:[~2019-12-20 10:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-09 9:43 [PATCH v3] power: reset: at91-reset: add sysfs interface to the power on reason Kamel Bouhara
2019-12-09 11:33 ` Claudiu.Beznea
2019-12-09 13:44 ` Kamel Bouhara
2019-12-19 17:13 ` Sebastian Reichel
2019-12-20 10:07 ` Kamel Bouhara
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).