linux-kernel - Re: [PATCH] mtd: rawnand: brcmnand: fix PM resume warning

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] [day] [month] [year] [list]
Message-ID: <874j0fczna.fsf@bootlin.com>
Date: Thu, 27 Feb 2025 13:58:49 +0100
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Kamal Dasu <kamal.dasu@...adcom.com>
Cc: florian.fainelli@...adcom.com,  Brian Norris
 <computersforpeace@...il.com>,  Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>,  Richard Weinberger
 <richard@....at>,  Vignesh Raghavendra <vigneshr@...com>,
  linux-mtd@...ts.infradead.org,  linux-kernel@...r.kernel.org,  Kamal Dasu
 <kdasu@...adcom.com>
Subject: Re: [PATCH] mtd: rawnand: brcmnand: fix PM resume warning

Hello Kamal,

On 26/02/2025 at 17:53:09 -05, Kamal Dasu <kamal.dasu@...adcom.com> wrote:

> Fixed warning due to unintialized struct nand_operation op->cs
> WARN_ON(op->cs >= nanddev_ntargets(&chip->base)
>
> [   14.588522] ------------[ cut here ]------------
> [   14.588529] WARNING: CPU: 0 PID: 1392 at drivers/mtd/nand/raw/internals.h:139 nand_reset_op+0x1e0/0x1f8
> [   14.588553] Modules linked in: bdc udc_core
> [   14.588579] CPU: 0 UID: 0 PID: 1392 Comm: rtcwake Tainted: G        W          6.14.0-rc4-g5394eea10651 #16
> [   14.588590] Tainted: [W]=WARN
> [   14.588593] Hardware name: Broadcom STB (Flattened Device Tree)
> [   14.588598] Call trace:
> [   14.588604]  dump_backtrace from show_stack+0x18/0x1c
> [   14.588622]  r7:00000009 r6:0000008b r5:60000153 r4:c0fa558c
> [   14.588625]  show_stack from dump_stack_lvl+0x70/0x7c
> [   14.588639]  dump_stack_lvl from dump_stack+0x18/0x1c
> [   14.588653]  r5:c08d40b0 r4:c1003cb0
> [   14.588656]  dump_stack from __warn+0x84/0xe4
> [   14.588668]  __warn from warn_slowpath_fmt+0x18c/0x194
> [   14.588678]  r7:c08d40b0 r6:c1003cb0 r5:00000000 r4:00000000
> [   14.588681]  warn_slowpath_fmt from nand_reset_op+0x1e0/0x1f8
> [   14.588695]  r8:70c40dff r7:89705f41 r6:36b4a597 r5:c26c9444 r4:c26b0048
> [   14.588697]  nand_reset_op from brcmnand_resume+0x13c/0x150
> [   14.588714]  r9:00000000 r8:00000000 r7:c24f8010 r6:c228a3f8 r5:c26c94bc r4:c26b0040
> [   14.588717]  brcmnand_resume from platform_pm_resume+0x34/0x54
> [   14.588735]  r5:00000010 r4:c0840a50
> [   14.588738]  platform_pm_resume from dpm_run_callback+0x5c/0x14c
> [   14.588757]  dpm_run_callback from device_resume+0xc0/0x324
> [   14.588776]  r9:c24f8054 r8:c24f80a0 r7:00000000 r6:00000000 r5:00000010 r4:c24f8010
> [   14.588779]  device_resume from dpm_resume+0x130/0x160
> [   14.588799]  r9:c22539e4 r8:00000010 r7:c22bebb0 r6:c24f8010 r5:c22539dc r4:c22539b0
> [   14.588802]  dpm_resume from dpm_resume_end+0x14/0x20
> [   14.588822]  r10:c2204e40 r9:00000000 r8:c228a3fc r7:00000000 r6:00000003 r5:c228a414
> [   14.588826]  r4:00000010
> [   14.588828]  dpm_resume_end from suspend_devices_and_enter+0x274/0x6f8
> [   14.588848]  r5:c228a414 r4:00000000
> [   14.588851]  suspend_devices_and_enter from pm_suspend+0x228/0x2bc
> [   14.588868]  r10:c3502910 r9:c3501f40 r8:00000004 r7:c228a438 r6:c0f95e18 r5:00000000
> [   14.588871]  r4:00000003
> [   14.588874]  pm_suspend from state_store+0x74/0xd0
> [   14.588889]  r7:c228a438 r6:c0f934c8 r5:00000003 r4:00000003
> [   14.588892]  state_store from kobj_attr_store+0x1c/0x28
> [   14.588913]  r9:00000000 r8:00000000 r7:f09f9f08 r6:00000004 r5:c3502900 r4:c0283250
> [   14.588916]  kobj_attr_store from sysfs_kf_write+0x40/0x4c
> [   14.588936]  r5:c3502900 r4:c0d92a48
> [   14.588939]  sysfs_kf_write from kernfs_fop_write_iter+0x104/0x1f0
> [   14.588956]  r5:c3502900 r4:c3501f40
> [   14.588960]  kernfs_fop_write_iter from vfs_write+0x250/0x420
> [   14.588980]  r10:c0e14b48 r9:00000000 r8:c25f5780 r7:00443398 r6:f09f9f68 r5:c34f7f00
> [   14.588983]  r4:c042a88c
> [   14.588987]  vfs_write from ksys_write+0x74/0xe4
> [   14.589005]  r10:00000004 r9:c25f5780 r8:c02002f0 r7:00000000 r6:00000000 r5:c34f7f00
> [   14.589008]  r4:c34f7f00
> [   14.589011]  ksys_write from sys_write+0x10/0x14
> [   14.589029]  r7:00000004 r6:004421c0 r5:00443398 r4:00000004
> [   14.589032]  sys_write from ret_fast_syscall+0x0/0x5c
> [   14.589044] Exception stack(0xf09f9fa8 to 0xf09f9ff0)
> [   14.589050] 9fa0:                   00000004 00443398 00000004 00443398 00000004 00000001
> [   14.589056] 9fc0: 00000004 00443398 004421c0 00000004 b6ecbd58 00000008 bebfbc38 0043eb78
> [   14.589062] 9fe0: 00440eb0 bebfbaf8 b6de18a0 b6e579e8
> [   14.589065] ---[ end trace 0000000000000000 ]---
>
> The fix uses the higher level nand_reset(chip, chipnr); where chipnr = 0, when
> doing PM resume operation in compliance with the controller support for single die
> nand chip. This ensures that the op->cs is initialized before calling the
> nand_reset_op().

While I believe the fix is correct, the explanation is partially
satisfying because you focus on the Warning reported above (which indeed
needs fixing) whereas the bigger problem you have is the fact that your
NAND chip is probably no longer in sync timing wise with the
host. Switching from nand_reset_op() to nand_reset() implies more than
just setting the cs field, it also reconfigures the data interface
(ie. the timings). I believe this is important enough to be mentioned.

> Fixes : 97d90da8a886 ("mtd: nand: provide several helpers to do common NAND operations")

Please add Cc: stable@ here.

> Signed-off-by: Kamal Dasu <kdasu@...adcom.com>

Thanks,
Miquèl