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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANAwSgTtzAZJqpsD7uVKskTnDmrT1bs=JuHxnPrkpQKtnZLhvQ@mail.gmail.com>
Date:   Mon, 16 Dec 2019 00:21:50 +0530
From:   Anand Moon <linux.amoon@...il.com>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Lee Jones <lee.jones@...aro.org>,
        Linux Kernel <linux-kernel@...r.kernel.org>,
        Heiko Stuebner <heiko@...ech.de>, Soeren Moch <smoch@....de>,
        linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH 4/4] mfd: rk808: Convert RK805 to syscore/PM ops

Hi Robin

On Tue, 10 Dec 2019 at 18:54, Robin Murphy <robin.murphy@....com> wrote:
>
> RK805 has the same kind of dual-role sleep/shutdown pin as RK809/RK817,
> so it makes little sense for the driver to have to have two completely
> different mechanisms to handle essentially the same thing. Bring RK805
> in line with the RK809/RK817 flow to clean things up.
>
> Signed-off-by: Robin Murphy <robin.murphy@....com>
> ---
>  drivers/mfd/rk808.c       | 58 +++++++++++++++++----------------------
>  include/linux/mfd/rk808.h |  1 -
>  2 files changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> index 657b8baa3b8a..e88bdb889d3a 100644
> --- a/drivers/mfd/rk808.c
> +++ b/drivers/mfd/rk808.c
> @@ -186,7 +186,6 @@ static const struct rk808_reg_data rk805_pre_init_reg[] = {
>         {RK805_BUCK4_CONFIG_REG, RK805_BUCK3_4_ILMAX_MASK,
>                                  RK805_BUCK4_ILMAX_3500MA},
>         {RK805_BUCK4_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_400MA},
> -       {RK805_GPIO_IO_POL_REG, SLP_SD_MSK, SLEEP_FUN},
>         {RK805_THERMAL_REG, TEMP_HOTDIE_MSK, TEMP115C},
>  };
>
> @@ -449,21 +448,6 @@ static const struct regmap_irq_chip rk818_irq_chip = {
>
>  static struct i2c_client *rk808_i2c_client;
>
> -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)
>  {
>         int ret;
> @@ -499,17 +483,29 @@ static void rk8xx_syscore_shutdown(void)
>         struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
>         int ret;
>
> -       if (system_state == SYSTEM_POWER_OFF &&
> -           (rk808->variant == RK809_ID || rk808->variant == RK817_ID)) {
> +       if (system_state != SYSTEM_POWER_OFF)
> +              return;
> +
> +       switch (rk808->variant) {
> +       case RK805_ID:
> +               ret = regmap_update_bits(rk808->regmap,
> +                                        RK805_GPIO_IO_POL_REG,
> +                                        SLP_SD_MSK,
> +                                        SHUTDOWN_FUN);
> +               break;
> +       case RK809_ID:
> +       case RK817_ID:
>                 ret = regmap_update_bits(rk808->regmap,
>                                          RK817_SYS_CFG(3),
>                                          RK817_SLPPIN_FUNC_MSK,
>                                          SLPPIN_DN_FUN);
> -               if (ret) {
> -                       dev_warn(&rk808_i2c_client->dev,
> -                                "Cannot switch to power down function\n");
> -               }
> +               break;
> +       default:
> +               return;
>         }
> +       if (ret)
> +               dev_warn(&rk808_i2c_client->dev,
> +                        "Cannot switch to power down function\n");
>  }
>
>  static struct syscore_ops rk808_syscore_ops = {
> @@ -579,7 +575,6 @@ 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);
> -               rk808->pm_pwroff_prep_fn = rk805_device_shutdown_prepare;
>                 break;
>         case RK808_ID:
>                 rk808->regmap_cfg = &rk808_regmap_config;
> @@ -658,10 +653,8 @@ static int rk808_probe(struct i2c_client *client,
>                 goto err_irq;
>         }
>
> -       if (of_property_read_bool(np, "rockchip,system-power-controller")) {
> +       if (of_property_read_bool(np, "rockchip,system-power-controller"))
>                 pm_power_off = rk808_device_shutdown;
> -               pm_power_off_prepare = rk808->pm_pwroff_prep_fn;
> -       }
>
>         return 0;
>
> @@ -686,13 +679,6 @@ static int rk808_remove(struct i2c_client *client)
>         if (pm_power_off == rk808_device_shutdown)
>                 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;
>  }
>
> @@ -702,6 +688,12 @@ static int __maybe_unused rk8xx_suspend(struct device *dev)
>         int ret = 0;
>
>         switch (rk808->variant) {
> +       case RK805_ID:
> +               ret = regmap_update_bits(rk808->regmap,
> +                                        RK805_GPIO_IO_POL_REG,
> +                                        SLP_SD_MSK,
> +                                        SLEEP_FUN);
> +               break;
>         case RK809_ID:
>         case RK817_ID:
>                 ret = regmap_update_bits(rk808->regmap,
> diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
> index b038653fa87e..e07f6e61cd38 100644
> --- a/include/linux/mfd/rk808.h
> +++ b/include/linux/mfd/rk808.h
> @@ -620,6 +620,5 @@ struct rk808 {
>         long                            variant;
>         const struct regmap_config      *regmap_cfg;
>         const struct regmap_irq_chip    *regmap_irq_chip;
> -       void                            (*pm_pwroff_prep_fn)(void);
>  };
>  #endif /* __LINUX_REGULATOR_RK808_H */
> --
> 2.17.1
>

I am sill getting the kernel warning on issue poweroff see below.
on my Rock960 Model A
I feel the reason for this is we now have two poweroff callback
1  pm_power_off = rk808_device_shutdown
2  rk8xx_syscore_shutdown

In my investigation earlier common function for shutdown solve
the issue of clean shutdown.

for *rockchip,system-power-controller* dts property
we can used flags if check if this property support clean shutdown
for that device.

[  565.009291] xhci-hcd xhci-hcd.0.auto: USB bus 5 deregistered
[  565.010179] reboot: Power down
[  565.010536] ------------[ cut here ]------------
[  565.010940] No atomic I2C transfer handler for 'i2c-0'
[  565.011437] WARNING: CPU: 0 PID: 1 at drivers/i2c/i2c-core.h:40
i2c_transfer+0xe4/0xf8
[  565.012126] Modules linked in: snd_soc_hdmi_codec dw_hdmi_i2s_audio
rockchipdrm nvme analogix_dp nvme_core brcmfmac hci_uart dw_mipi_dsi
dw_hdmi btbcm cec panfrost bluetooth drm_kms_helper brcmutil gpu_sched
cfg80211 crct10dif_ce snd_soc_rockchip_i2s snd_soc_simple_card drm
ecdh_generic snd_soc_rockchip_pcm snd_soc_simple_card_utils
phy_rockchip_pcie ecc rtc_rk808 rfkill rockchip_thermal
pcie_rockchip_host ip_tables x_tables ipv6 nf_defrag_ipv6
[  565.015578] CPU: 0 PID: 1 Comm: shutdown Not tainted
5.5.0-rc1-00292-gd46dd6369c55 #7
[  565.016260] Hardware name: 96boards Rock960 (DT)
[  565.016666] pstate: 60000085 (nZCv daIf -PAN -UAO)
[  565.017087] pc : i2c_transfer+0xe4/0xf8
[  565.017425] lr : i2c_transfer+0xe4/0xf8
[  565.017762] sp : ffff80001004baf0
[  565.018052] x29: ffff80001004baf0 x28: ffff00007d208000
[  565.018517] x27: 0000000000000000 x26: 0000000000000000
[  565.018982] x25: 0000000000000008 x24: 0000000000000000
[  565.019447] x23: ffff00007d208000 x22: ffff80001004bc64
[  565.019912] x21: ffff80001004bb48 x20: 0000000000000002
[  565.020377] x19: ffff000078502080 x18: 0000000000000010
[  565.020842] x17: 0000000000000001 x16: 0000000000000019
[  565.021307] x15: ffff00007d208470 x14: ffffffffffffffff
[  565.021772] x13: ffff80009004b857 x12: ffff80001004b860
[  565.022237] x11: ffff800011841000 x10: ffff800011a10658
[  565.022702] x9 : 0000000000000000 x8 : ffff800011a11000
[  565.023167] x7 : ffff800010697c78 x6 : 0000000000000262
[  565.023632] x5 : 0000000000000000 x4 : 0000000000000000
[  565.024096] x3 : 00000000ffffffff x2 : ffff800011841ab8
[  565.024561] x1 : 7b11701b0ae78800 x0 : 0000000000000000
[  565.025027] Call trace:
[  565.025246]  i2c_transfer+0xe4/0xf8
[  565.025556]  regmap_i2c_read+0x5c/0xa0
[  565.025886]  _regmap_raw_read+0xcc/0x138
[  565.026230]  _regmap_bus_read+0x3c/0x70
[  565.026568]  _regmap_read+0x60/0xe0
[  565.026875]  _regmap_update_bits+0xc8/0x108
[  565.027241]  regmap_update_bits_base+0x60/0x90
[  565.027633]  rk808_device_shutdown+0x6c/0x88
[  565.028010]  machine_power_off+0x24/0x30
[  565.028356]  kernel_power_off+0x64/0x70
[  565.028693]  __do_sys_reboot+0x15c/0x240
[  565.029038]  __arm64_sys_reboot+0x20/0x28
[  565.029390]  el0_svc_common.constprop.0+0x68/0x160
[  565.029811]  el0_svc_handler+0x20/0x80
[  565.030141]  el0_sync_handler+0x10c/0x180
[  565.030493]  el0_sync+0x140/0x180
[  565.030785] ---[ end trace 5167e842ce15f686 ]---

-Anand

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ