* [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
@ 2020-08-03 21:25 Heinrich Schuchardt
2020-08-04 1:46 ` Bin Meng
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Heinrich Schuchardt @ 2020-08-03 21:25 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
2020-08-03 21:25 [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n Heinrich Schuchardt
@ 2020-08-04 1:46 ` Bin Meng
2020-08-04 11:01 ` Heinrich Schuchardt
2020-08-04 2:00 ` Simon Glass
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA4730259@ATCPCS16.andestech.com>
2 siblings, 1 reply; 9+ messages in thread
From: Bin Meng @ 2020-08-04 1:46 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
2020-08-03 21:25 [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n Heinrich Schuchardt
2020-08-04 1:46 ` Bin Meng
@ 2020-08-04 2:00 ` Simon Glass
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA4730259@ATCPCS16.andestech.com>
2 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2020-08-04 2:00 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA4730259@ATCPCS16.andestech.com>
@ 2020-08-04 7:53 ` Rick Chen
0 siblings, 0 replies; 9+ messages in thread
From: Rick Chen @ 2020-08-04 7:53 UTC (permalink / raw)
To: u-boot
> From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de]
> Sent: Tuesday, August 04, 2020 5:26 AM
> To: Rick Jian-Zhi Chen(???)
> Cc: Simon Glass; Anup Patel; Lukas Auer; Bin Meng; Daniel Schwierzeck; u-boot at lists.denx.de; Heinrich Schuchardt
> Subject: [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(-)
>
Reviewed-by: Rick Chen <rick@andestech.com>
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
2020-08-04 1:46 ` Bin Meng
@ 2020-08-04 11:01 ` Heinrich Schuchardt
2020-08-04 13:15 ` Bin Meng
0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2020-08-04 11:01 UTC (permalink / raw)
To: u-boot
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>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
2020-08-04 11:01 ` Heinrich Schuchardt
@ 2020-08-04 13:15 ` Bin Meng
2020-08-04 14:58 ` Heinrich Schuchardt
0 siblings, 1 reply; 9+ messages in thread
From: Bin Meng @ 2020-08-04 13:15 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
2020-08-04 13:15 ` Bin Meng
@ 2020-08-04 14:58 ` Heinrich Schuchardt
2020-08-05 0:10 ` Bin Meng
0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2020-08-04 14:58 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
2020-08-04 14:58 ` Heinrich Schuchardt
@ 2020-08-05 0:10 ` Bin Meng
2020-08-05 7:17 ` Andy Shevchenko
0 siblings, 1 reply; 9+ messages in thread
From: Bin Meng @ 2020-08-05 0:10 UTC (permalink / raw)
To: u-boot
+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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n
2020-08-05 0:10 ` Bin Meng
@ 2020-08-05 7:17 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2020-08-05 7:17 UTC (permalink / raw)
To: u-boot
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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-08-05 7:17 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-03 21:25 [PATCH 1/1] riscv: fix building with CONFIG_SPL_SMP=n Heinrich Schuchardt
2020-08-04 1:46 ` Bin Meng
2020-08-04 11:01 ` Heinrich Schuchardt
2020-08-04 13:15 ` Bin Meng
2020-08-04 14:58 ` Heinrich Schuchardt
2020-08-05 0:10 ` Bin Meng
2020-08-05 7:17 ` Andy Shevchenko
2020-08-04 2:00 ` Simon Glass
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA4730259@ATCPCS16.andestech.com>
2020-08-04 7:53 ` Rick Chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.