[PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
Building with CONFIG_SPL_SMP=n results in:
arch/riscv/lib/spl.c: In function ‘jump_to_image_no_args’: arch/riscv/lib/spl.c:33:6: error: unused variable ‘ret’ [-Werror=unused-variable] 33 | int ret; | ^~~
Define the variable ret as __maybe_unused.
Fixes: 191636e44898 ("riscv: Introduce SPL_SMP Kconfig option for U-Boot SPL") Fixes: 8c59f2023cc8 ("riscv: add SPL support") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- arch/riscv/lib/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/lib/spl.c b/arch/riscv/lib/spl.c index c47dcd46ce..ef00ec2bcc 100644 --- a/arch/riscv/lib/spl.c +++ b/arch/riscv/lib/spl.c @@ -30,7 +30,7 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) { typedef void __noreturn (*image_entry_riscv_t)(ulong hart, void *dtb); void *fdt_blob; - int ret; + __maybe_unused int ret;
#if CONFIG_IS_ENABLED(LOAD_FIT) || CONFIG_IS_ENABLED(LOAD_FIT_FULL) fdt_blob = spl_image->fdt_addr; -- 2.27.0
On Tue, Aug 4, 2020 at 5:26 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Building with CONFIG_SPL_SMP=n results in:
arch/riscv/lib/spl.c: In function ‘jump_to_image_no_args’: arch/riscv/lib/spl.c:33:6: error: unused variable ‘ret’ [-Werror=unused-variable] 33 | int ret; | ^~~
Define the variable ret as __maybe_unused.
Fixes: 191636e44898 ("riscv: Introduce SPL_SMP Kconfig option for U-Boot SPL")
This should be on the same line
Fixes: 8c59f2023cc8 ("riscv: add SPL support") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
arch/riscv/lib/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/lib/spl.c b/arch/riscv/lib/spl.c index c47dcd46ce..ef00ec2bcc 100644 --- a/arch/riscv/lib/spl.c +++ b/arch/riscv/lib/spl.c @@ -30,7 +30,7 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) { typedef void __noreturn (*image_entry_riscv_t)(ulong hart, void *dtb); void *fdt_blob;
int ret;
__maybe_unused int ret;#if CONFIG_IS_ENABLED(LOAD_FIT) || CONFIG_IS_ENABLED(LOAD_FIT_FULL) fdt_blob = spl_image->fdt_addr;
Reviewed-by: Bin Meng bin.meng@windriver.com
On 04.08.20 03:46, Bin Meng wrote:
On Tue, Aug 4, 2020 at 5:26 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Building with CONFIG_SPL_SMP=n results in:
arch/riscv/lib/spl.c: In function ‘jump_to_image_no_args’: arch/riscv/lib/spl.c:33:6: error: unused variable ‘ret’ [-Werror=unused-variable] 33 | int ret; | ^~~
Define the variable ret as __maybe_unused.
Fixes: 191636e44898 ("riscv: Introduce SPL_SMP Kconfig option for U-Boot SPL")
This should be on the same line
Commit messages should not exceed 75 characters. See scripts/checkpatch.pl:
WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
Best regards
Heinrich
Fixes: 8c59f2023cc8 ("riscv: add SPL support") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
arch/riscv/lib/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/lib/spl.c b/arch/riscv/lib/spl.c index c47dcd46ce..ef00ec2bcc 100644 --- a/arch/riscv/lib/spl.c +++ b/arch/riscv/lib/spl.c @@ -30,7 +30,7 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) { typedef void __noreturn (*image_entry_riscv_t)(ulong hart, void *dtb); void *fdt_blob;
int ret;
__maybe_unused int ret;#if CONFIG_IS_ENABLED(LOAD_FIT) || CONFIG_IS_ENABLED(LOAD_FIT_FULL) fdt_blob = spl_image->fdt_addr;
Reviewed-by: Bin Meng bin.meng@windriver.com
On Tue, Aug 4, 2020 at 7:02 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 04.08.20 03:46, Bin Meng wrote:
On Tue, Aug 4, 2020 at 5:26 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Building with CONFIG_SPL_SMP=n results in:
arch/riscv/lib/spl.c: In function ‘jump_to_image_no_args’: arch/riscv/lib/spl.c:33:6: error: unused variable ‘ret’ [-Werror=unused-variable] 33 | int ret; | ^~~
Define the variable ret as __maybe_unused.
Fixes: 191636e44898 ("riscv: Introduce SPL_SMP Kconfig option for U-Boot SPL")
This should be on the same line
Commit messages should not exceed 75 characters. See scripts/checkpatch.pl:
True, for normal commit messages.
WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
But this Fixes tag is special. I suspect 2 lines will break some scripts that is handling this "Fixes" tag.
Regards, Bin
On 04.08.20 15:15, Bin Meng wrote:
On Tue, Aug 4, 2020 at 7:02 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 04.08.20 03:46, Bin Meng wrote:
On Tue, Aug 4, 2020 at 5:26 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Building with CONFIG_SPL_SMP=n results in:
arch/riscv/lib/spl.c: In function ‘jump_to_image_no_args’: arch/riscv/lib/spl.c:33:6: error: unused variable ‘ret’ [-Werror=unused-variable] 33 | int ret; | ^~~
Define the variable ret as __maybe_unused.
Fixes: 191636e44898 ("riscv: Introduce SPL_SMP Kconfig option for U-Boot SPL")
This should be on the same line
Commit messages should not exceed 75 characters. See scripts/checkpatch.pl:
True, for normal commit messages.
WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
But this Fixes tag is special. I suspect 2 lines will break some scripts that is handling this "Fixes" tag.
checkpatch.pl and patchstream.py are the only U-Boot scripts containing the string "Fixes".
* checkpatch.pl does not complain. * I don't use patman. So I don't care if it has a bug.
We already have patches like this by other developers and nobody complained:
dcdea292d9f3 4fb2264b2848 00160cf32e6e
So why should I worry?
Best regards
Heinrich
Regards, Bin
+Andy Shevchenko
On Tue, Aug 4, 2020 at 10:58 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 04.08.20 15:15, Bin Meng wrote:
On Tue, Aug 4, 2020 at 7:02 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 04.08.20 03:46, Bin Meng wrote:
On Tue, Aug 4, 2020 at 5:26 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Building with CONFIG_SPL_SMP=n results in:
arch/riscv/lib/spl.c: In function ‘jump_to_image_no_args’: arch/riscv/lib/spl.c:33:6: error: unused variable ‘ret’ [-Werror=unused-variable] 33 | int ret; | ^~~
Define the variable ret as __maybe_unused.
Fixes: 191636e44898 ("riscv: Introduce SPL_SMP Kconfig option for U-Boot SPL")
This should be on the same line
Commit messages should not exceed 75 characters. See scripts/checkpatch.pl:
True, for normal commit messages.
WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
But this Fixes tag is special. I suspect 2 lines will break some scripts that is handling this "Fixes" tag.
checkpatch.pl and patchstream.py are the only U-Boot scripts containing the string "Fixes".
- checkpatch.pl does not complain.
- I don't use patman. So I don't care if it has a bug.
We already have patches like this by other developers and nobody complained:
IIRC, last time Andy raised the same concern.
Andy, would you share some examples or best practices?
dcdea292d9f3 4fb2264b2848 00160cf32e6e
So why should I worry?
Regards, Bin
On Wed, Aug 5, 2020 at 3:11 AM Bin Meng bmeng.cn@gmail.com wrote:
On Tue, Aug 4, 2020 at 10:58 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 04.08.20 15:15, Bin Meng wrote:
On Tue, Aug 4, 2020 at 7:02 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 04.08.20 03:46, Bin Meng wrote:
On Tue, Aug 4, 2020 at 5:26 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
...
Fixes: 191636e44898 ("riscv: Introduce SPL_SMP Kconfig option for U-Boot SPL")
This should be on the same line
The rule of thumb is one tag == one line. Otherwise it breaks a lot of scripting here and there. The reason why is that actually Unix tools are not designed (yes, I know it's possible to do in some cases) to handle multi-line processing (like multi-line grep). So, 100% Fixes should be one line. Also this is applicable to URLs.
Commit messages should not exceed 75 characters. See scripts/checkpatch.pl:
True, for normal commit messages.
WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
This warning is bogus. Fix your tools.
But this Fixes tag is special. I suspect 2 lines will break some scripts that is handling this "Fixes" tag.
Precisely!
checkpatch.pl and patchstream.py are the only U-Boot scripts containing the string "Fixes".
- checkpatch.pl does not complain.
- I don't use patman. So I don't care if it has a bug.
You don't but maintainers tell you what to do. And yes, tools sometimes wrong or false positive, feel free to complain about them.
We already have patches like this by other developers and nobody complained:
IIRC, last time Andy raised the same concern.
Andy, would you share some examples or best practices?
dcdea292d9f3 4fb2264b2848 00160cf32e6e
So why should I worry?
I hope above clarifies. And it's generally a weak argument to point to bad examples in the past.
On Mon, 3 Aug 2020 at 15:26, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Building with CONFIG_SPL_SMP=n results in:
arch/riscv/lib/spl.c: In function ‘jump_to_image_no_args’: arch/riscv/lib/spl.c:33:6: error: unused variable ‘ret’ [-Werror=unused-variable] 33 | int ret; | ^~~
Define the variable ret as __maybe_unused.
Fixes: 191636e44898 ("riscv: Introduce SPL_SMP Kconfig option for U-Boot SPL") Fixes: 8c59f2023cc8 ("riscv: add SPL support") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
arch/riscv/lib/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (4)
-
Andy Shevchenko -
Bin Meng -
Heinrich Schuchardt -
Simon Glass