* [PATCH v2 0/2] mfd: rk808: Fix pointers and poweroff @ 2019-05-21 6:24 Stefan Mavrodiev 2019-05-21 6:24 ` [PATCH v2 1/2] mfd: rk808: Check pm_power_off pointer Stefan Mavrodiev 2019-05-21 6:24 ` [PATCH v2 2/2] mfd: rk808: Prepare rk805 for poweroff Stefan Mavrodiev 0 siblings, 2 replies; 5+ messages in thread From: Stefan Mavrodiev @ 2019-05-21 6:24 UTC (permalink / raw) To: Heiko Stuebner, Lee Jones, linux-kernel; +Cc: Stefan Mavrodiev This patch is actually follow-up to: [PATCH 1/1] mfd: rk808: Prepare rk8085 for poweroff The patchset fixes poweroff function for boards with RK8085 PMU. During the preparation of v2 possible wrong pointer access was spot (pm_power_off), so one more patch was introduced in the series. Stefan Mavrodiev (2): mfd: rk808: Check pm_power_off pointer mfd: rk808: Prepare rk805 for poweroff drivers/mfd/rk808.c | 42 +++++++++++++++++++++++++++++++++------ include/linux/mfd/rk808.h | 2 ++ 2 files changed, 38 insertions(+), 6 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] mfd: rk808: Check pm_power_off pointer 2019-05-21 6:24 [PATCH v2 0/2] mfd: rk808: Fix pointers and poweroff Stefan Mavrodiev @ 2019-05-21 6:24 ` Stefan Mavrodiev 2019-06-03 12:45 ` Lee Jones 2019-05-21 6:24 ` [PATCH v2 2/2] mfd: rk808: Prepare rk805 for poweroff Stefan Mavrodiev 1 sibling, 1 reply; 5+ messages in thread From: Stefan Mavrodiev @ 2019-05-21 6:24 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 in v2: - Initial release actually drivers/mfd/rk808.c | 13 +++++++------ include/linux/mfd/rk808.h | 1 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c index 94377782d208..c0b179792bbf 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,9 @@ 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; + + 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
* Re: [PATCH v2 1/2] mfd: rk808: Check pm_power_off pointer 2019-05-21 6:24 ` [PATCH v2 1/2] mfd: rk808: Check pm_power_off pointer Stefan Mavrodiev @ 2019-06-03 12:45 ` Lee Jones 0 siblings, 0 replies; 5+ messages in thread From: Lee Jones @ 2019-06-03 12:45 UTC (permalink / raw) To: Stefan Mavrodiev; +Cc: Heiko Stuebner, linux-kernel On Tue, 21 May 2019, Stefan Mavrodiev wrote: > 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 in v2: > - Initial release actually > > drivers/mfd/rk808.c | 13 +++++++------ > include/linux/mfd/rk808.h | 1 + > 2 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c > index 94377782d208..c0b179792bbf 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,9 @@ 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; > + > + if (rk808->pm_pwroff_fn && pm_power_off == rk808->pm_pwroff_fn) > + pm_power_off = NULL; The idea seems sound, but I think you should comment this statement. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] mfd: rk808: Prepare rk805 for poweroff 2019-05-21 6:24 [PATCH v2 0/2] mfd: rk808: Fix pointers and poweroff Stefan Mavrodiev 2019-05-21 6:24 ` [PATCH v2 1/2] mfd: rk808: Check pm_power_off pointer Stefan Mavrodiev @ 2019-05-21 6:24 ` Stefan Mavrodiev 2019-06-03 12:44 ` Lee Jones 1 sibling, 1 reply; 5+ messages in thread From: Stefan Mavrodiev @ 2019-05-21 6:24 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> --- Changes for v2: - Move pm_pwroff_prep_fn to header - Check pm_power_off_prepare before make it NULL drivers/mfd/rk808.c | 29 +++++++++++++++++++++++++++++ include/linux/mfd/rk808.h | 1 + 2 files changed, 30 insertions(+) diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c index c0b179792bbf..fb6cdf900899 100644 --- a/drivers/mfd/rk808.c +++ b/drivers/mfd/rk808.c @@ -387,6 +387,24 @@ static void rk805_device_shutdown(void) dev_err(&rk808_i2c_client->dev, "power off error!\n"); } +static void rk805_device_shutdown_prepare(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"); + 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, "power off error!\n"); +} + static void rk808_device_shutdown(void) { int ret; @@ -475,6 +493,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 +569,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: @@ -566,6 +591,10 @@ static int rk808_remove(struct i2c_client *client) if (rk808->pm_pwroff_fn && pm_power_off == rk808->pm_pwroff_fn) pm_power_off = NULL; + 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
* Re: [PATCH v2 2/2] mfd: rk808: Prepare rk805 for poweroff 2019-05-21 6:24 ` [PATCH v2 2/2] mfd: rk808: Prepare rk805 for poweroff Stefan Mavrodiev @ 2019-06-03 12:44 ` Lee Jones 0 siblings, 0 replies; 5+ messages in thread From: Lee Jones @ 2019-06-03 12:44 UTC (permalink / raw) To: Stefan Mavrodiev; +Cc: Heiko Stuebner, linux-kernel On Tue, 21 May 2019, Stefan Mavrodiev wrote: > 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> > --- > Changes for v2: > - Move pm_pwroff_prep_fn to header > - Check pm_power_off_prepare before make it NULL > > drivers/mfd/rk808.c | 29 +++++++++++++++++++++++++++++ > include/linux/mfd/rk808.h | 1 + > 2 files changed, 30 insertions(+) > > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c > index c0b179792bbf..fb6cdf900899 100644 > --- a/drivers/mfd/rk808.c > +++ b/drivers/mfd/rk808.c > @@ -387,6 +387,24 @@ static void rk805_device_shutdown(void) > dev_err(&rk808_i2c_client->dev, "power off error!\n"); > } > > +static void rk805_device_shutdown_prepare(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"); No need for this message I think. If it's not available, just return. > + 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, "power off error!\n"); > +} "Failed to shutdown device" > static void rk808_device_shutdown(void) > { > int ret; > @@ -475,6 +493,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 +569,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: > @@ -566,6 +591,10 @@ static int rk808_remove(struct i2c_client *client) > if (rk808->pm_pwroff_fn && pm_power_off == rk808->pm_pwroff_fn) > pm_power_off = NULL; > > + if (rk808->pm_pwroff_prep_fn && > + pm_power_off_prepare == rk808->pm_pwroff_prep_fn) > + pm_power_off_prepare = NULL; I think this block would benefit from a comment. > 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 */ -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-03 12:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-21 6:24 [PATCH v2 0/2] mfd: rk808: Fix pointers and poweroff Stefan Mavrodiev 2019-05-21 6:24 ` [PATCH v2 1/2] mfd: rk808: Check pm_power_off pointer Stefan Mavrodiev 2019-06-03 12:45 ` Lee Jones 2019-05-21 6:24 ` [PATCH v2 2/2] mfd: rk808: Prepare rk805 for poweroff Stefan Mavrodiev 2019-06-03 12:44 ` Lee Jones
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.