Hi Michael, Am Montag, 11. Mai 2015, 22:54:11 schrieb Michael Niewöhner: > Rebooting radxa rock which uses act8846 results in kernel panic because the > bootloader doesn't readjust frequency or voltage for cpu correctly. This > workaround power cycles the act8846 at restart to reset the board.
Hmm, it might be nice to reword this, as this being a "workaround" for the radxarock is a bit "short sighted". The act8846 simply provides means to reset the system when used as main pmic [hence the reliance on the system-power-controller] and other socs/boards using it as pmic might also profit from exposing this function - for example if there is no other means of reset. > Signed-off-by: Michael Niewoehner <[email protected]> > --- > drivers/regulator/act8865-regulator.c | 43 > +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) > > diff --git a/drivers/regulator/act8865-regulator.c > b/drivers/regulator/act8865-regulator.c index 2ff73d7..f8cdff3 100644 > --- a/drivers/regulator/act8865-regulator.c > +++ b/drivers/regulator/act8865-regulator.c > @@ -27,6 +27,7 @@ > #include <linux/of_device.h> > #include <linux/regulator/of_regulator.h> > #include <linux/regmap.h> > +#include <linux/reboot.h> > > /* > * ACT8600 Global Register Map. > @@ -133,10 +134,14 @@ > #define ACT8865_VOLTAGE_NUM 64 > #define ACT8600_SUDCDC_VOLTAGE_NUM 255 > > +#define ACT8846_SIPC_MASK 0x01 > + > struct act8865 { > struct regmap *regmap; > int off_reg; > int off_mask; > + int sipc_reg; > + int sipc_mask; > }; > > static const struct regmap_config act8865_regmap_config = { > @@ -402,6 +407,17 @@ static void act8865_power_off(void) > while (1); > } > > +static int act8846_power_cycle(struct notifier_block *this, > + unsigned long code, void *unused) > +{ > + struct act8865 *act8846; > + > + act8846 = i2c_get_clientdata(act8865_i2c_client); > + regmap_write(act8846->regmap, act8846->sipc_reg, act8846->sipc_mask); The act8846 is currently the only one of the three types providing such functionality, so until the second chip variant surfaces that provides a reset capability, it might be less intrusive to simply do regmap_write(act8846->regmap, ACT8846_GLB_OFF_CTRL, ACT8846_SIPC_MASK); here and skip all the infrastructure like sipc_reg and sipc_mask assignment and handling? But I guess that is Mark's call what he likes better. > + > + return NOTIFY_DONE; > +} > + structs like the restart_handler normally live near the function they reference and not as static part of some function. And as the restart functionality is not rockchip-specific, it should also not be named rockchip_foo as below ;-) static struct notifier_block act8846_restart_handler = { .notifier_call = act8846_power_cycle, .priority = 129, }; > static int act8865_pmic_probe(struct i2c_client *client, > const struct i2c_device_id *i2c_id) > { > @@ -413,6 +429,7 @@ static int act8865_pmic_probe(struct i2c_client *client, > struct act8865 *act8865; > unsigned long type; > int off_reg, off_mask; > + int sipc_reg, sipc_mask; > > pdata = dev_get_platdata(dev); > > @@ -434,18 +451,24 @@ static int act8865_pmic_probe(struct i2c_client > *client, num_regulators = ARRAY_SIZE(act8600_regulators); > off_reg = -1; > off_mask = -1; > + sipc_reg = -1; > + sipc_mask = -1; > break; > case ACT8846: > regulators = act8846_regulators; > num_regulators = ARRAY_SIZE(act8846_regulators); > off_reg = ACT8846_GLB_OFF_CTRL; > off_mask = ACT8846_OFF_SYSMASK; > + sipc_reg = ACT8846_GLB_OFF_CTRL; > + sipc_mask = ACT8846_SIPC_MASK; > break; > case ACT8865: > regulators = act8865_regulators; > num_regulators = ARRAY_SIZE(act8865_regulators); > off_reg = ACT8865_SYS_CTRL; > off_mask = ACT8865_MSTROFF; > + sipc_reg = -1; > + sipc_mask = -1; > break; > default: > dev_err(dev, "invalid device id %lu\n", type); > @@ -494,6 +517,26 @@ static int act8865_pmic_probe(struct i2c_client > *client, } > } no need to add a second check for of_device_is_system_power_controller() as we already have a check for this property directly above, simply extend it with something like the following. if (of_device_is_system_power_controller(dev->of_node)) { int ret; /* already existing code to set poweroff handler */ if (type == ACT8846) { ret = register_restart_handler(&act8846_restart_handler); if (ret) pr_err("%s: cannot register restart handler, %d\n", __func__, ret); } } > > + /* Register restart handler */ > + if (of_device_is_system_power_controller(dev->of_node) > + && sipc_reg > 0) { > + static struct notifier_block rockchip_restart_handler = { > + .notifier_call = act8846_power_cycle, > + .priority = 129, > + }; > + > + int ret; > + > + act8865_i2c_client = client; > + act8865->sipc_reg = sipc_reg; > + act8865->sipc_mask = sipc_mask; > + > + ret = register_restart_handler(&rockchip_restart_handler); > + if (ret) > + pr_err("%s: cannot register restart handler, %d\n", > + __func__, ret); > + } > + > /* Finally register devices */ > for (i = 0; i < num_regulators; i++) { > const struct regulator_desc *desc = ®ulators[i]; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
