| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 13 Mar 2018 10:48:19 +0100
From: Daniel Schultz <d.schultz@...tec.de>
To: Joseph Chen <chenjh@...k-chips.com>,
Lee Jones <lee.jones@...aro.org>
Cc: zyw@...k-chips.com, zhangqing@...k-chips.com,
linux-kernel@...r.kernel.org, w.egorov@...tec.de,
"xsf@...k-chips.com" <xsf@...k-chips.com>,
谢修鑫 <tony.xie@...k-chips.com>
Subject: Re: [PATCH v2 2/2] mfd: rk808: Add restart functionality
Hi,
I can add a DTS property to enable the PMIC restart functions? So this
feature can be used if its desired.
My problem with the CRU reset is a strange behaviour in the ROM code.
During a reset from MMC1 (eMMC), the ROM tries to load the SPL from MMC0
(SD).
Daniel
On 03/13/2018 04:32 AM, Joseph Chen wrote:
> Hi, Daniel：
>
> On Rockchip platforms, we always use CRU to restart system.
> From your patch, I think you would like to support restart by CRU or
> PMIC, right ?
>
> I think restart by PMIC is not accepted, because PMIC restart
> means all regualtors are reset (voltage drops to 0mv and then power on
> again), including vdd_arm, vdd_logic, vcc_ddr, etc. This is like a
> cold boot for machine.
>
> We use CRU reset, because we want to keep status of last log,
> GRF, CRU or other registers, but PMIC restart can make them lost.
>
>
> 在 2018/3/12 22:10, Lee Jones 写道:
>> Rockchip guys,
>>
>> I'd really appreciate your input on these two patches please.
>>
>> Please provide Reviewed-by/Tested-by tags.
>>
>> On Wed, 07 Mar 2018, Daniel Schultz wrote:
>>> When using Rockchip SoCs with rk805/808/818 PMICs, restarts are
>>> realized by
>>> setting the reset registers in the "Clock and Reset Unit".
>>>
>>> Since all three shutdown functions have almost the same code, all logic
>>> from the shutdown functions can be refactored to a new function
>>> "rk808_update_bits", which can update a register by a given address and
>>> bitmask.
>>>
>>> After that, notifier blocks and the restart functions were added to the
>>> driver. Like the shutdown function, the restart is bound to the
>>> "rockchip,system-power-controller" device tree property and can easily
>>> revoked to the software restart by removing the property.
>>>
>>> Signed-off-by: Daniel Schultz <d.schultz@...tec.de>
>>> ---
>>> Changes:
>>> v2: Re-submit with recipients from Rockchip.
>>>
>>> drivers/mfd/rk808.c | 97
>>> ++++++++++++++++++++++++++++++-----------------
>>> include/linux/mfd/rk808.h | 1 +
>>> 2 files changed, 63 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
>>> index d138721..2c68f8c 100644
>>> --- a/drivers/mfd/rk808.c
>>> +++ b/drivers/mfd/rk808.c
>>> @@ -27,6 +27,7 @@
>>> #include <linux/module.h>
>>> #include <linux/of_device.h>
>>> #include <linux/regmap.h>
>>> +#include <linux/reboot.h>
>>> struct rk808_reg_data {
>>> int addr;
>>> @@ -369,59 +370,73 @@ static const struct regmap_irq_chip
>>> rk818_irq_chip = {
>>> static struct i2c_client *rk808_i2c_client;
>>> -static void rk805_device_shutdown(void)
>>> +static void rk808_update_bits(unsigned int reg, unsigned int bit_mask)
>>> {
>>> 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");
>>> + "have no %s, so do nothing here\n",
>>> + rk808->regmap_irq_chip->name);
>>> return;
>>> }
>>> ret = regmap_update_bits(rk808->regmap,
>>> - RK805_DEV_CTRL_REG,
>>> - DEV_OFF, DEV_OFF);
>>> + reg,
>>> + bit_mask, bit_mask);
>>> if (ret)
>>> - dev_err(&rk808_i2c_client->dev, "power off error!\n");
>>> + dev_err(&rk808_i2c_client->dev, "can't write to DEVCTRL:
>>> %x!\n",
>>> + ret);
>>> }
>>> -static void rk808_device_shutdown(void)
>>> +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 rk808, so do nothing here\n");
>>> - return;
>>> - }
>>> + rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF);
>>> +}
>>> +static int rk805_restart_notify(struct notifier_block *this,
>>> + unsigned long mode, void *cmd)
>>> +{
>>> + rk808_update_bits(RK805_DEV_CTRL_REG, DEV_OFF_RST);
>>> + return NOTIFY_DONE;
>>> +}
>>> - 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");
>>> +static void rk808_device_shutdown(void)
>>> +{
>>> + rk808_update_bits(RK808_DEVCTRL_REG, DEV_OFF_RST);
>>> +}
>>> +static int rk808_restart_notify(struct notifier_block *this,
>>> + unsigned long mode, void *cmd)
>>> +{
>>> + rk808_update_bits(RK808_DEVCTRL_REG, DEV_OFF);
>>> + return NOTIFY_DONE;
>>> }
>>> static void rk818_device_shutdown(void)
>>> {
>>> - int ret;
>>> - struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
>>> + rk808_update_bits(RK818_DEVCTRL_REG, DEV_OFF_RST);
>>> +}
>>> +static int rk818_restart_notify(struct notifier_block *this,
>>> + unsigned long mode, void *cmd)
>>> +{
>>> + rk808_update_bits(RK818_DEVCTRL_REG, DEV_OFF);
>>> + return NOTIFY_DONE;
>>> +}
>>> - if (!rk808) {
>>> - dev_warn(&rk808_i2c_client->dev,
>>> - "have no rk818, so do nothing here\n");
>>> - return;
>>> - }
>>> +static struct notifier_block rk805_restart_handler = {
>>> + .notifier_call = rk805_restart_notify,
>>> + .priority = 196,
>>> +};
>>> - ret = regmap_update_bits(rk808->regmap,
>>> - RK818_DEVCTRL_REG,
>>> - DEV_OFF_RST, DEV_OFF_RST);
>>> - if (ret)
>>> - dev_err(&rk808_i2c_client->dev, "power off error!\n");
>>> -}
>>> +static struct notifier_block rk808_restart_handler = {
>>> + .notifier_call = rk808_restart_notify,
>>> + .priority = 196,
>>> +};
>>> +
>>> +static struct notifier_block rk818_restart_handler = {
>>> + .notifier_call = rk818_restart_notify,
>>> + .priority = 196,
>>> +};
>>> static const struct of_device_id rk808_of_match[] = {
>>> { .compatible = "rockchip,rk805" },
>>> @@ -476,6 +491,7 @@ static int rk808_probe(struct i2c_client *client,
>>> cells = rk805s;
>>> nr_cells = ARRAY_SIZE(rk805s);
>>> pm_pwroff_fn = rk805_device_shutdown;
>>> + rk808->nb = &rk805_restart_handler;
>>> break;
>>> case RK808_ID:
>>> rk808->regmap_cfg = &rk808_regmap_config;
>>> @@ -485,6 +501,7 @@ static int rk808_probe(struct i2c_client *client,
>>> cells = rk808s;
>>> nr_cells = ARRAY_SIZE(rk808s);
>>> pm_pwroff_fn = rk808_device_shutdown;
>>> + rk808->nb = &rk808_restart_handler;
>>> break;
>>> case RK818_ID:
>>> rk808->regmap_cfg = &rk818_regmap_config;
>>> @@ -494,6 +511,7 @@ static int rk808_probe(struct i2c_client *client,
>>> cells = rk818s;
>>> nr_cells = ARRAY_SIZE(rk818s);
>>> pm_pwroff_fn = rk818_device_shutdown;
>>> + rk808->nb = &rk818_restart_handler;
>>> break;
>>> default:
>>> dev_err(&client->dev, "Unsupported RK8XX ID %lu\n",
>>> @@ -546,11 +564,18 @@ static int rk808_probe(struct i2c_client *client,
>>> pm_off = of_property_read_bool(np,
>>> "rockchip,system-power-controller");
>>> - if (pm_off && !pm_power_off) {
>>> + if (pm_off) {
>>> rk808_i2c_client = client;
>>> - pm_power_off = pm_pwroff_fn;
>>> + if (!pm_power_off)
>>> + pm_power_off = pm_pwroff_fn;
>>> + if (rk808->nb) {
>>> + ret = register_restart_handler(rk808->nb);
>>> + if (ret)
>>> + dev_err(&client->dev,
>>> + "cannot register restart handler, %d\n",
>>> + ret);
>>> + }
>>> }
>>> -
>>> return 0;
>>> err_irq:
>>> @@ -564,6 +589,8 @@ static int rk808_remove(struct i2c_client *client)
>>> regmap_del_irq_chip(client->irq, rk808->irq_data);
>>> pm_power_off = NULL;
>>> + if (rk808->nb)
>>> + unregister_restart_handler(rk808->nb);
>>> return 0;
>>> }
>>> diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
>>> index 338e0f6..9c26ca2 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;
>>> + struct notifier_block *nb;
>>> };
>>> #endif /* __LINUX_REGULATOR_RK808_H */
>
Powered by blists - more mailing lists