lkml.org 
[lkml]   [2021]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/2] mfd: rk808: add reboot support to rk808 pmic
From
On 2021-12-15 21:32, Frank Wunderlich wrote:
> From: Peter Geis <pgwipeout@gmail.com>
>
> This adds reboot support to the rk808 pmic. This only enables if the
> rockchip,system-power-controller flag is set.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
> drivers/mfd/rk808.c | 48 +++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/rk808.h | 2 ++
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> index b181fe401330..afbd7e01df50 100644
> --- a/drivers/mfd/rk808.c
> +++ b/drivers/mfd/rk808.c
> @@ -19,6 +19,7 @@
> #include <linux/module.h>
> #include <linux/of_device.h>
> #include <linux/regmap.h>
> +#include <linux/reboot.h>
>
> struct rk808_reg_data {
> int addr;
> @@ -533,6 +534,7 @@ static void rk808_pm_power_off(void)
> int ret;
> unsigned int reg, bit;
> struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
> + dev_err(&rk808_i2c_client->dev, "poweroff device!\n");

This is not an error.

>
> switch (rk808->variant) {
> case RK805_ID:
> @@ -552,6 +554,7 @@ static void rk808_pm_power_off(void)
> bit = DEV_OFF;
> break;
> default:
> + dev_err(&rk808_i2c_client->dev, "poweroff device not supported!\n");

If we really don't support power off for the present PMIC then we should
avoid registering the power off handler in the first place. These
default cases should mostly just serve to keep static checkers happy.

> return;
> }
> ret = regmap_update_bits(rk808->regmap, reg, bit, bit);
> @@ -559,6 +562,44 @@ static void rk808_pm_power_off(void)
> dev_err(&rk808_i2c_client->dev, "Failed to shutdown device!\n");
> }
>
> +static int rk808_restart_notify(struct notifier_block *this, unsigned long mode, void *cmd)
> +{
> + int ret;
> + unsigned int reg, bit;
> + struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
> +
> + switch (rk808->variant) {
> + case RK805_ID:
> + reg = RK805_DEV_CTRL_REG;
> + bit = DEV_OFF_RST;
> + break;
> + case RK808_ID:
> + reg = RK808_DEVCTRL_REG,
> + bit = DEV_OFF;

Is that right? The RK808 datasheet does say that this bit resets itself
once the OFF state is reached, but there doesn't seem to be any obvious
guarantee of a power-on condition being present to automatically
transition back to ACTIVE.

> + break;
> + case RK817_ID:
> + reg = RK817_SYS_CFG(3);
> + bit = DEV_RST;
> + break;
> + case RK818_ID:
> + reg = RK818_DEVCTRL_REG;
> + bit = DEV_OFF_RST;
> + break;
> + default:
> + return NOTIFY_DONE;
> + }
> + ret = regmap_update_bits(rk808->regmap, reg, bit, bit);
> + if (ret)
> + dev_err(&rk808_i2c_client->dev, "Failed to restart device!\n");
> +
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block rk808_restart_handler = {
> + .notifier_call = rk808_restart_notify,
> + .priority = 255,
> +};
> +
> static void rk8xx_shutdown(struct i2c_client *client)
> {
> struct rk808 *rk808 = i2c_get_clientdata(client);
> @@ -727,6 +768,13 @@ static int rk808_probe(struct i2c_client *client,
> if (of_property_read_bool(np, "rockchip,system-power-controller")) {
> rk808_i2c_client = client;
> pm_power_off = rk808_pm_power_off;
> + rk808->nb = &rk808_restart_handler;

The handler just relies on the global rk808_i2c_client anyway, so does
this serve any purpose?

> +
> + dev_warn(&client->dev, "register restart handler\n");

Don't scream a warning about normal and expected operation.

Thanks,
Robin.

> +
> + ret = register_restart_handler(rk808->nb);
> + if (ret)
> + dev_err(&client->dev, "failed to register restart handler, %d\n", ret);
> }
>
> return 0;
> diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
> index a96e6d43ca06..5dfe0c4ceab1 100644
> --- a/include/linux/mfd/rk808.h
> +++ b/include/linux/mfd/rk808.h
> @@ -373,6 +373,7 @@ enum rk805_reg {
> #define SWITCH2_EN BIT(6)
> #define SWITCH1_EN BIT(5)
> #define DEV_OFF_RST BIT(3)
> +#define DEV_RST BIT(2)
> #define DEV_OFF BIT(0)
> #define RTC_STOP BIT(0)
>
> @@ -701,5 +702,6 @@ struct rk808 {
> long variant;
> const struct regmap_config *regmap_cfg;
> const struct regmap_irq_chip *regmap_irq_chip;
> + struct notifier_block *nb;
> };
> #endif /* __LINUX_REGULATOR_RK808_H */

\
 
 \ /
  Last update: 2021-12-16 14:18    [W:0.076 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site