* [PATCH v3 1/2] mfd: rk808: Check pm_power_off pointer
2019-06-07 12:42 [PATCH v3 0/2] mfd: rk808: Fix pointers and poweroff Stefan Mavrodiev
@ 2019-06-07 12:42 ` Stefan Mavrodiev
2019-06-12 9:41 ` Lee Jones
2019-06-07 12:42 ` [PATCH v3 2/2] mfd: rk808: Prepare rk805 for poweroff Stefan Mavrodiev
1 sibling, 1 reply; 5+ messages in thread
From: Stefan Mavrodiev @ 2019-06-07 12:42 UTC (permalink / raw)
To: Heiko Stuebner, Lee Jones, linux-kernel; +Cc: Stefan Mavrodiev
The function pointer pm_power_off may point to function from other
module (PSCI for example). If rk808 is removed, pm_power_off is
overwritten to NULL and the system cannot be powered off.
This patch checks if pm_power_off points to a module function.
Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
Changes is v3:
- Add explanation comments
Changes in v2:
- Initial release actually
drivers/mfd/rk808.c | 17 +++++++++++------
include/linux/mfd/rk808.h | 1 +
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index 94377782d208..46d26e141cc4 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -438,7 +438,6 @@ static int rk808_probe(struct i2c_client *client,
struct rk808 *rk808;
const struct rk808_reg_data *pre_init_reg;
const struct mfd_cell *cells;
- void (*pm_pwroff_fn)(void);
int nr_pre_init_regs;
int nr_cells;
int pm_off = 0, msb, lsb;
@@ -475,7 +474,7 @@ static int rk808_probe(struct i2c_client *client,
nr_pre_init_regs = ARRAY_SIZE(rk805_pre_init_reg);
cells = rk805s;
nr_cells = ARRAY_SIZE(rk805s);
- pm_pwroff_fn = rk805_device_shutdown;
+ rk808->pm_pwroff_fn = rk805_device_shutdown;
break;
case RK808_ID:
rk808->regmap_cfg = &rk808_regmap_config;
@@ -484,7 +483,7 @@ static int rk808_probe(struct i2c_client *client,
nr_pre_init_regs = ARRAY_SIZE(rk808_pre_init_reg);
cells = rk808s;
nr_cells = ARRAY_SIZE(rk808s);
- pm_pwroff_fn = rk808_device_shutdown;
+ rk808->pm_pwroff_fn = rk808_device_shutdown;
break;
case RK818_ID:
rk808->regmap_cfg = &rk818_regmap_config;
@@ -493,7 +492,7 @@ static int rk808_probe(struct i2c_client *client,
nr_pre_init_regs = ARRAY_SIZE(rk818_pre_init_reg);
cells = rk818s;
nr_cells = ARRAY_SIZE(rk818s);
- pm_pwroff_fn = rk818_device_shutdown;
+ rk808->pm_pwroff_fn = rk818_device_shutdown;
break;
default:
dev_err(&client->dev, "Unsupported RK8XX ID %lu\n",
@@ -548,7 +547,7 @@ static int rk808_probe(struct i2c_client *client,
"rockchip,system-power-controller");
if (pm_off && !pm_power_off) {
rk808_i2c_client = client;
- pm_power_off = pm_pwroff_fn;
+ pm_power_off = rk808->pm_pwroff_fn;
}
return 0;
@@ -563,7 +562,13 @@ static int rk808_remove(struct i2c_client *client)
struct rk808 *rk808 = i2c_get_clientdata(client);
regmap_del_irq_chip(client->irq, rk808->irq_data);
- pm_power_off = NULL;
+
+ /**
+ * pm_power_off may points to a function from another module.
+ * Check if the pointer is set by us and only then overwrite it.
+ */
+ if (rk808->pm_pwroff_fn && pm_power_off == rk808->pm_pwroff_fn)
+ pm_power_off = NULL;
return 0;
}
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index d3156594674c..8b5d68a7bb9c 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -453,5 +453,6 @@ struct rk808 {
long variant;
const struct regmap_config *regmap_cfg;
const struct regmap_irq_chip *regmap_irq_chip;
+ void (*pm_pwroff_fn)(void);
};
#endif /* __LINUX_REGULATOR_RK808_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v3 2/2] mfd: rk808: Prepare rk805 for poweroff
2019-06-07 12:42 [PATCH v3 0/2] mfd: rk808: Fix pointers and poweroff Stefan Mavrodiev
2019-06-07 12:42 ` [PATCH v3 1/2] mfd: rk808: Check pm_power_off pointer Stefan Mavrodiev
@ 2019-06-07 12:42 ` Stefan Mavrodiev
2019-06-12 9:41 ` Lee Jones
1 sibling, 1 reply; 5+ messages in thread
From: Stefan Mavrodiev @ 2019-06-07 12:42 UTC (permalink / raw)
To: Heiko Stuebner, Lee Jones, linux-kernel; +Cc: Stefan Mavrodiev
RK805 has SLEEP signal, which can put the device into SLEEP or OFF
mode. The default is SLEEP mode.
However, when the kernel performs power-off (actually the ATF) the
device will not go fully off and this will result in higher power
consumption and inability to wake the device with RTC alarm.
The solution is to enable pm_power_off_prepare function, which will
configure SLEEP pin for OFF function.
Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
Change for v3:
- Remove useless warning messages
- Change poweroff error messages
- Add explanation comments
Changes for v2:
- Move pm_pwroff_prep_fn to header
- Check pm_power_off_prepare before make it NULL
drivers/mfd/rk808.c | 50 +++++++++++++++++++++++++++------------
include/linux/mfd/rk808.h | 1 +
2 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index 46d26e141cc4..416575343b4d 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -374,17 +374,29 @@ static void rk805_device_shutdown(void)
int ret;
struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
- if (!rk808) {
- dev_warn(&rk808_i2c_client->dev,
- "have no rk805, so do nothing here\n");
+ if (!rk808)
return;
- }
ret = regmap_update_bits(rk808->regmap,
RK805_DEV_CTRL_REG,
DEV_OFF, DEV_OFF);
if (ret)
- dev_err(&rk808_i2c_client->dev, "power off error!\n");
+ dev_err(&rk808_i2c_client->dev, "Failed to shutdown device!\n");
+}
+
+static void rk805_device_shutdown_prepare(void)
+{
+ int ret;
+ struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
+
+ if (!rk808)
+ return;
+
+ ret = regmap_update_bits(rk808->regmap,
+ RK805_GPIO_IO_POL_REG,
+ SLP_SD_MSK, SHUTDOWN_FUN);
+ if (ret)
+ dev_err(&rk808_i2c_client->dev, "Failed to shutdown device!\n");
}
static void rk808_device_shutdown(void)
@@ -392,17 +404,14 @@ static void rk808_device_shutdown(void)
int ret;
struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
- if (!rk808) {
- dev_warn(&rk808_i2c_client->dev,
- "have no rk808, so do nothing here\n");
+ if (!rk808)
return;
- }
ret = regmap_update_bits(rk808->regmap,
RK808_DEVCTRL_REG,
DEV_OFF_RST, DEV_OFF_RST);
if (ret)
- dev_err(&rk808_i2c_client->dev, "power off error!\n");
+ dev_err(&rk808_i2c_client->dev, "Failed to shutdown device!\n");
}
static void rk818_device_shutdown(void)
@@ -410,17 +419,14 @@ static void rk818_device_shutdown(void)
int ret;
struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
- if (!rk808) {
- dev_warn(&rk808_i2c_client->dev,
- "have no rk818, so do nothing here\n");
+ if (!rk808)
return;
- }
ret = regmap_update_bits(rk808->regmap,
RK818_DEVCTRL_REG,
DEV_OFF, DEV_OFF);
if (ret)
- dev_err(&rk808_i2c_client->dev, "power off error!\n");
+ dev_err(&rk808_i2c_client->dev, "Failed to shutdown device!\n");
}
static const struct of_device_id rk808_of_match[] = {
@@ -475,6 +481,7 @@ static int rk808_probe(struct i2c_client *client,
cells = rk805s;
nr_cells = ARRAY_SIZE(rk805s);
rk808->pm_pwroff_fn = rk805_device_shutdown;
+ rk808->pm_pwroff_prep_fn = rk805_device_shutdown_prepare;
break;
case RK808_ID:
rk808->regmap_cfg = &rk808_regmap_config;
@@ -550,6 +557,12 @@ static int rk808_probe(struct i2c_client *client,
pm_power_off = rk808->pm_pwroff_fn;
}
+ if (pm_off && !pm_power_off_prepare) {
+ if (!rk808_i2c_client)
+ rk808_i2c_client = client;
+ pm_power_off_prepare = rk808->pm_pwroff_prep_fn;
+ }
+
return 0;
err_irq:
@@ -570,6 +583,13 @@ static int rk808_remove(struct i2c_client *client)
if (rk808->pm_pwroff_fn && pm_power_off == rk808->pm_pwroff_fn)
pm_power_off = NULL;
+ /**
+ * As above, check if the pointer is set by us before overwrite.
+ */
+ if (rk808->pm_pwroff_prep_fn &&
+ pm_power_off_prepare == rk808->pm_pwroff_prep_fn)
+ pm_power_off_prepare = NULL;
+
return 0;
}
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index 8b5d68a7bb9c..ec928173e507 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -454,5 +454,6 @@ struct rk808 {
const struct regmap_config *regmap_cfg;
const struct regmap_irq_chip *regmap_irq_chip;
void (*pm_pwroff_fn)(void);
+ void (*pm_pwroff_prep_fn)(void);
};
#endif /* __LINUX_REGULATOR_RK808_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread