This series introduces a restart handler for the pca9450. This is an
optional feature and can be activated by assigning the handler priority
in the device tree.
Best regards,
Holger
Holger Assmann (2):
regulator: pca9450: enable restart handler for I2C operation
dt-bindings: regulator: pca9450: add restart handler priority
.../regulator/nxp,pca9450-regulator.yaml | 3 ++
drivers/regulator/pca9450-regulator.c | 54 +++++++++++++++++++
include/linux/regulator/pca9450.h | 7 +++
3 files changed, 64 insertions(+)
--
2.39.2
The NXP PCA9450 can perform various kinds of power cycles when triggered
by I2C-command.
We therefore make this functionality accessible by introducing a
respective restart handler. It will be used after a priority has been
defined within the devicetree.
Signed-off-by: Holger Assmann <[email protected]>
---
drivers/regulator/pca9450-regulator.c | 54 +++++++++++++++++++++++++++
include/linux/regulator/pca9450.h | 7 ++++
2 files changed, 61 insertions(+)
diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
index 2ab365d2749f9..e623323599964 100644
--- a/drivers/regulator/pca9450-regulator.c
+++ b/drivers/regulator/pca9450-regulator.c
@@ -16,6 +16,7 @@
#include <linux/regulator/machine.h>
#include <linux/regulator/of_regulator.h>
#include <linux/regulator/pca9450.h>
+#include <linux/reboot.h>
struct pc9450_dvs_config {
unsigned int run_reg; /* dvs0 */
@@ -33,6 +34,7 @@ struct pca9450 {
struct device *dev;
struct regmap *regmap;
struct gpio_desc *sd_vsel_gpio;
+ struct notifier_block restart_handler;
enum pca9450_chip_type type;
unsigned int rcnt;
int irq;
@@ -700,6 +702,34 @@ static irqreturn_t pca9450_irq_handler(int irq, void *data)
return IRQ_HANDLED;
}
+static int pca9450_restart(struct notifier_block *nb,
+ unsigned long mode, void *cmd)
+{
+ struct pca9450 *pmic = container_of(nb, struct pca9450,
+ restart_handler);
+ struct i2c_client *client = container_of(pmic->dev,
+ struct i2c_client, dev);
+ int ret = 0;
+ u32 rtype;
+
+ switch (mode) {
+ case REBOOT_WARM:
+ /* Warm reset (Toggle POR_B for 20 ms) */
+ rtype = 0x35;
+ break;
+ default:
+ /* Cold reset (Power recycle all regulators) */
+ rtype = 0x64;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, PCA9450_REG_SWRST, rtype);
+ if (ret < 0)
+ dev_alert(pmic->dev, "Failed to shutdown (err = %d)\n", ret);
+
+ mdelay(500);
+ return NOTIFY_DONE;
+}
+
static int pca9450_i2c_probe(struct i2c_client *i2c)
{
enum pca9450_chip_type type = (unsigned int)(uintptr_t)
@@ -845,12 +875,35 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
return PTR_ERR(pca9450->sd_vsel_gpio);
}
+ /* Register I2C restart handler if one is defined by device tree */
+ if (!of_property_read_u32(i2c->dev.of_node, "priority",
+ &pca9450->restart_handler.priority)) {
+ pca9450->restart_handler.notifier_call = pca9450_restart;
+
+ ret = register_restart_handler(&pca9450->restart_handler);
+ if (ret)
+ return dev_err_probe(&i2c->dev, ret,
+ "cannot register restart handler.\n");
+ else
+ dev_info(&i2c->dev,
+ "registered restart handler with priority %d.\n",
+ pca9450->restart_handler.priority);
+ }
+
dev_info(&i2c->dev, "%s probed.\n",
type == PCA9450_TYPE_PCA9450A ? "pca9450a" : "pca9450bc");
return 0;
}
+static void pca9450_i2c_remove(struct i2c_client *i2c)
+{
+ struct pca9450 *pca9450 = dev_get_drvdata(&i2c->dev);
+
+ if (pca9450->restart_handler.notifier_call)
+ unregister_restart_handler(&pca9450->restart_handler);
+}
+
static const struct of_device_id pca9450_of_match[] = {
{
.compatible = "nxp,pca9450a",
@@ -875,6 +928,7 @@ static struct i2c_driver pca9450_i2c_driver = {
.of_match_table = pca9450_of_match,
},
.probe = pca9450_i2c_probe,
+ .remove = pca9450_i2c_remove,
};
module_i2c_driver(pca9450_i2c_driver);
diff --git a/include/linux/regulator/pca9450.h b/include/linux/regulator/pca9450.h
index 505c908dbb817..2ee38bab23402 100644
--- a/include/linux/regulator/pca9450.h
+++ b/include/linux/regulator/pca9450.h
@@ -226,6 +226,13 @@ enum {
#define WDOG_B_CFG_COLD_LDO12 0x80
#define WDOG_B_CFG_COLD 0xC0
+/* PCA9450_REG_SWRST bits */
+#define SW_RST_NONE 0x00
+#define SW_RST_DEFAULTS 0x05
+#define SW_RST_COLD_LDO12_CLK32 0x14
+#define SW_RST_WARM 0x35
+#define SW_RST_COLD 0x64
+
/* PCA9450_REG_CONFIG2 bits */
#define I2C_LT_MASK 0x03
#define I2C_LT_FORCE_DISABLE 0x00
--
2.39.2
This is an optional property. If set, the pca9450 will be registered as
a reset device with the chosen priority level.
Signed-off-by: Holger Assmann <[email protected]>
---
.../devicetree/bindings/regulator/nxp,pca9450-regulator.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
index 3d469b8e97748..7cc2d6636cf52 100644
--- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
@@ -35,6 +35,9 @@ properties:
interrupts:
maxItems: 1
+ priority:
+ $ref: /schemas/power/reset/restart-handler.yaml#
+
regulators:
type: object
description: |
--
2.39.2
On 19/04/2024 10:31, Holger Assmann wrote:
> This is an optional property. If set, the pca9450 will be registered as
> a reset device with the chosen priority level.
>
> Signed-off-by: Holger Assmann <[email protected]>
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.
> ---
> .../devicetree/bindings/regulator/nxp,pca9450-regulator.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> index 3d469b8e97748..7cc2d6636cf52 100644
> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> @@ -35,6 +35,9 @@ properties:
> interrupts:
> maxItems: 1
>
> + priority:
> + $ref: /schemas/power/reset/restart-handler.yaml#
You defined object, which is not explained in commit msg. This code does
not look correct or it does not implement what you said.
Please look at existing code - do you see anything like this? No, there
is no such code and this should raise question.
You probably want to annotate that device is a restart handler?
Best regards,
Krzysztof
On 19/04/2024 10:31, Holger Assmann wrote:
> The NXP PCA9450 can perform various kinds of power cycles when triggered
> by I2C-command.
> We therefore make this functionality accessible by introducing a
> respective restart handler. It will be used after a priority has been
> defined within the devicetree.
>
..
> +
> static int pca9450_i2c_probe(struct i2c_client *i2c)
> {
> enum pca9450_chip_type type = (unsigned int)(uintptr_t)
> @@ -845,12 +875,35 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> return PTR_ERR(pca9450->sd_vsel_gpio);
> }
>
> + /* Register I2C restart handler if one is defined by device tree */
> + if (!of_property_read_u32(i2c->dev.of_node, "priority",
> + &pca9450->restart_handler.priority)) {
Priority property does not define whether this is or is not restart
handler. In case of missing priority, you should use just default:
SYS_OFF_PRIO_DEFAULT. Not skip the registering.
Best regards,
Krzysztof
Hello Krzysztof,
Am 19.04.24 um 15:41 schrieb Krzysztof Kozlowski:
>
> Priority property does not define whether this is or is not restart
> handler. In case of missing priority, you should use just default:
> SYS_OFF_PRIO_DEFAULT. Not skip the registering.
>
Thank you for your fast response and the feedback. I will work it in and
send a v2 later.
Kind regards,
Holger
--
Pengutronix e.K. | Holger Assmann |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hello Krzysztof,
also thanks for the feedback on this one.
Am 19.04.24 um 15:39 schrieb Krzysztof Kozlowski:
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.
Short note: I did that prior submitting, but I did it directly for the
yaml-file and not for the directory - Those do not look the same
regarding their prefix scheme.
I will change it for my v2 and use a subject like for the directory.
>
>> ---
>> .../devicetree/bindings/regulator/nxp,pca9450-regulator.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> index 3d469b8e97748..7cc2d6636cf52 100644
>> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> @@ -35,6 +35,9 @@ properties:
>> interrupts:
>> maxItems: 1
>>
>> + priority:
>> + $ref: /schemas/power/reset/restart-handler.yaml#
>
> You defined object, which is not explained in commit msg. This code does
> not look correct or it does not implement what you said.
>
> Please look at existing code - do you see anything like this? No, there
> is no such code and this should raise question.
I am a bit lost on that one to be honest.
The only other instances where a "priority" for restart handling is
described are "gpio-poweroff.yaml" and "syscon-reboot.yaml". These files
are dedicated documentation for the reset bindings, so I tried to
transfer the respective entry over for my commit.
Do you suggest I should replace
+ priority:
+ $ref: /schemas/power/reset/restart-handler.yaml#
with
+allOf:
+ - $ref: /schemas/power/reset/restart-handler.yaml#
in order to properly include the context for the restart handling?
Running dt_binding_check does not indicate an issue with any of those two.
>
> You probably want to annotate that device is a restart handler?
You mean by adding to the "description" part of the file?
Kind regards,
Holger
--
Pengutronix e.K. | Holger Assmann |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 22/04/2024 10:24, Holger Assmann wrote:
> Hello Krzysztof,
>
> also thanks for the feedback on this one.
>
> Am 19.04.24 um 15:39 schrieb Krzysztof Kozlowski:
>>
>> Please use subject prefixes matching the subsystem. You can get them for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching.
>
> Short note: I did that prior submitting, but I did it directly for the
> yaml-file and not for the directory - Those do not look the same
> regarding their prefix scheme.
No, if you run it per file or per directory, prefix is the same. Starts with regulator, doesn't it?
>
> I will change it for my v2 and use a subject like for the directory.
regulator: dt-bindings: nxp,pca9450: add foo bar
>
>
>>
>>> ---
>>> .../devicetree/bindings/regulator/nxp,pca9450-regulator.yaml | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>> index 3d469b8e97748..7cc2d6636cf52 100644
>>> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>> @@ -35,6 +35,9 @@ properties:
>>> interrupts:
>>> maxItems: 1
>>>
>>> + priority:
>>> + $ref: /schemas/power/reset/restart-handler.yaml#
>>
>> You defined object, which is not explained in commit msg. This code does
>> not look correct or it does not implement what you said.
>>
>> Please look at existing code - do you see anything like this? No, there
>> is no such code and this should raise question.
>
> I am a bit lost on that one to be honest.
>
> The only other instances where a "priority" for restart handling is
> described are "gpio-poweroff.yaml" and "syscon-reboot.yaml". These files
> are dedicated documentation for the reset bindings, so I tried to
> transfer the respective entry over for my commit.
Where do you see such syntax?
>
> Do you suggest I should replace
>
> + priority:
> + $ref: /schemas/power/reset/restart-handler.yaml#
I don't understand what you want to express with such syntax. That's why I suggested you to look for existing examples. There is no such code as above, so you are the first one to add it. And then the obvious question: why are you doing things quite different than others? That's pretty often a hint that something is odd.
>
> with
>
> +allOf:
> + - $ref: /schemas/power/reset/restart-handler.yaml#
>
> in order to properly include the context for the restart handling?
> Running dt_binding_check does not indicate an issue with any of those two.
Again, what do you want to achieve?
>
>
>>
>> You probably want to annotate that device is a restart handler?
>
> You mean by adding to the "description" part of the file?
No. What do you want to achieve?
Best regards,
Krzysztof