| Message ID | 20240708093145.1398949-2-u.kleine-koenig@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: mvebu: Drop a write-only variable | expand |
On Mon, Jul 08, 2024 at 11:31:42AM +0200, Uwe Kleine-König wrote: > This fixes a W=1 compiler error: > > arch/arm/mach-mvebu/board-v7.c: In function ‘mvebu_scan_mem’: > arch/arm/mach-mvebu/board-v7.c:84:27: error: variable ‘size’ set but not used [-Werror=unused-but-set-variable] > 84 | u64 base, size; > | ^~~~ > > Fixes: 8da2b2f7ceee ("ARM: mvebu: reserve the first 10 KB of each memory bank for suspend/resume") > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > --- > Hello, > > not entirely sure about the correctness of this patch. Maybe you want to > error out if size < MVEBU_DDR_TRAINING_AREA_SZ? I think it is too late for that now, although it would of been correct. There could be DT blobs in use which have this wrong, and now enforcing it would cause a regression. The best we can do is print a warning. Andrew
On Mon, Jul 08, 2024 at 03:42:40PM +0200, Andrew Lunn wrote: > On Mon, Jul 08, 2024 at 11:31:42AM +0200, Uwe Kleine-König wrote: > > This fixes a W=1 compiler error: > > > > arch/arm/mach-mvebu/board-v7.c: In function ‘mvebu_scan_mem’: > > arch/arm/mach-mvebu/board-v7.c:84:27: error: variable ‘size’ set but not used [-Werror=unused-but-set-variable] > > 84 | u64 base, size; > > | ^~~~ > > > > Fixes: 8da2b2f7ceee ("ARM: mvebu: reserve the first 10 KB of each memory bank for suspend/resume") > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > > --- > > Hello, > > > > not entirely sure about the correctness of this patch. Maybe you want to > > error out if size < MVEBU_DDR_TRAINING_AREA_SZ? > > I think it is too late for that now, although it would of been > correct. There could be DT blobs in use which have this wrong, and now > enforcing it would cause a regression. The best we can do is print a > warning. What is the right incarnation to do so? I think this happens very early during boot, is pr_warn() suitable? Or better use WARN_ON? (Or was WARN_ON the one we're not supposed to use?) Best regards Uwe
On Tue, Jul 09, 2024 at 09:04:43AM +0200, Uwe Kleine-König wrote: > On Mon, Jul 08, 2024 at 03:42:40PM +0200, Andrew Lunn wrote: > > On Mon, Jul 08, 2024 at 11:31:42AM +0200, Uwe Kleine-König wrote: > > > This fixes a W=1 compiler error: > > > > > > arch/arm/mach-mvebu/board-v7.c: In function ‘mvebu_scan_mem’: > > > arch/arm/mach-mvebu/board-v7.c:84:27: error: variable ‘size’ set but not used [-Werror=unused-but-set-variable] > > > 84 | u64 base, size; > > > | ^~~~ > > > > > > Fixes: 8da2b2f7ceee ("ARM: mvebu: reserve the first 10 KB of each memory bank for suspend/resume") > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > > > --- > > > Hello, > > > > > > not entirely sure about the correctness of this patch. Maybe you want to > > > error out if size < MVEBU_DDR_TRAINING_AREA_SZ? > > > > I think it is too late for that now, although it would of been > > correct. There could be DT blobs in use which have this wrong, and now > > enforcing it would cause a regression. The best we can do is print a > > warning. > > What is the right incarnation to do so? I think this happens very early > during boot, is pr_warn() suitable? Or better use WARN_ON? (Or was WARN_ON > the one we're not supposed to use?) WARN_ON() is being pushed back on, since in some setups it causes a reboot. So pr_warn() would be better. It should work this early, but it is likely to be cached and output later, unless ernlyprintk is enabled. Andrew
diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c index fd5d0c8ff695..3fc4cce0225d 100644 --- a/arch/arm/mach-mvebu/board-v7.c +++ b/arch/arm/mach-mvebu/board-v7.c @@ -81,10 +81,11 @@ static int __init mvebu_scan_mem(unsigned long node, const char *uname, endp = reg + (l / sizeof(__be32)); while ((endp - reg) >= (dt_root_addr_cells + dt_root_size_cells)) { - u64 base, size; + u64 base; base = dt_mem_next_cell(dt_root_addr_cells, ®); - size = dt_mem_next_cell(dt_root_size_cells, ®); + /* skip over size */ + dt_mem_next_cell(dt_root_size_cells, ®); memblock_reserve(base, MVEBU_DDR_TRAINING_AREA_SZ); }
This fixes a W=1 compiler error: arch/arm/mach-mvebu/board-v7.c: In function ‘mvebu_scan_mem’: arch/arm/mach-mvebu/board-v7.c:84:27: error: variable ‘size’ set but not used [-Werror=unused-but-set-variable] 84 | u64 base, size; | ^~~~ Fixes: 8da2b2f7ceee ("ARM: mvebu: reserve the first 10 KB of each memory bank for suspend/resume") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> --- Hello, not entirely sure about the correctness of this patch. Maybe you want to error out if size < MVEBU_DDR_TRAINING_AREA_SZ? Best regards Uwe arch/arm/mach-mvebu/board-v7.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) base-commit: 0b58e108042b0ed28a71cd7edf5175999955b233