* Re: [PATCH] spi: rockchip: avoid objtool warning
2021-02-25 12:55 [PATCH] spi: rockchip: avoid objtool warning Arnd Bergmann
@ 2021-02-25 14:09 ` Emil Renner Berthing
2021-02-25 21:16 ` Nick Desaulniers
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Emil Renner Berthing @ 2021-02-25 14:09 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mark Brown, Heiko Stuebner, Nathan Chancellor, Nick Desaulniers,
Arnd Bergmann, Jon Lin, Chris Ruehl, Alexander Kochetkov,
Johan Jonker, Vincent Pelletier, linux-spi, linux-arm-kernel,
open list:ARM/Rockchip SoC...,
Linux Kernel Mailing List, clang-built-linux
On Thu, 25 Feb 2021 at 13:55, Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> Building this file with clang leads to a an unreachable code path
> causing a warning from objtool:
>
> drivers/spi/spi-rockchip.o: warning: objtool: rockchip_spi_transfer_one()+0x2e0: sibling call from callable instruction with modified stack frame
>
> Use BUG() instead of unreachable() to avoid the undefined behavior
> if it does happen.
>
> Fixes: 65498c6ae241 ("spi: rockchip: support 4bit words")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Emil Renner Berthing <kernel@esmil.dk>
> ---
> drivers/spi/spi-rockchip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index 936ef54e0903..972beac1169a 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -521,7 +521,7 @@ static void rockchip_spi_config(struct rockchip_spi *rs,
> * ctlr->bits_per_word_mask, so this shouldn't
> * happen
> */
> - unreachable();
> + BUG();
> }
>
> if (use_dma) {
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: rockchip: avoid objtool warning
2021-02-25 12:55 [PATCH] spi: rockchip: avoid objtool warning Arnd Bergmann
2021-02-25 14:09 ` Emil Renner Berthing
@ 2021-02-25 21:16 ` Nick Desaulniers
2021-02-25 21:19 ` Heiko Stübner
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Nick Desaulniers @ 2021-02-25 21:16 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mark Brown, Heiko Stuebner, Nathan Chancellor,
Emil Renner Berthing, Arnd Bergmann, Jon Lin, Chris Ruehl,
Alexander Kochetkov, Johan Jonker, Vincent Pelletier, linux-spi,
Linux ARM, linux-rockchip, LKML, clang-built-linux
On Thu, Feb 25, 2021 at 4:55 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> Building this file with clang leads to a an unreachable code path
> causing a warning from objtool:
>
> drivers/spi/spi-rockchip.o: warning: objtool: rockchip_spi_transfer_one()+0x2e0: sibling call from callable instruction with modified stack frame
>
> Use BUG() instead of unreachable() to avoid the undefined behavior
> if it does happen.
Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Fixes: 65498c6ae241 ("spi: rockchip: support 4bit words")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/spi/spi-rockchip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index 936ef54e0903..972beac1169a 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -521,7 +521,7 @@ static void rockchip_spi_config(struct rockchip_spi *rs,
> * ctlr->bits_per_word_mask, so this shouldn't
> * happen
> */
> - unreachable();
> + BUG();
> }
>
> if (use_dma) {
> --
> 2.29.2
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: rockchip: avoid objtool warning
2021-02-25 12:55 [PATCH] spi: rockchip: avoid objtool warning Arnd Bergmann
2021-02-25 14:09 ` Emil Renner Berthing
2021-02-25 21:16 ` Nick Desaulniers
@ 2021-02-25 21:19 ` Heiko Stübner
2021-02-26 8:15 ` Pratyush Yadav
2021-03-01 23:37 ` Mark Brown
4 siblings, 0 replies; 9+ messages in thread
From: Heiko Stübner @ 2021-02-25 21:19 UTC (permalink / raw)
To: Mark Brown, Nathan Chancellor, Nick Desaulniers,
Emil Renner Berthing, Arnd Bergmann
Cc: Arnd Bergmann, Jon Lin, Chris Ruehl, Alexander Kochetkov,
Johan Jonker, Vincent Pelletier, linux-spi, linux-arm-kernel,
linux-rockchip, linux-kernel, clang-built-linux
Am Donnerstag, 25. Februar 2021, 13:55:34 CET schrieb Arnd Bergmann:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Building this file with clang leads to a an unreachable code path
> causing a warning from objtool:
>
> drivers/spi/spi-rockchip.o: warning: objtool: rockchip_spi_transfer_one()+0x2e0: sibling call from callable instruction with modified stack frame
>
> Use BUG() instead of unreachable() to avoid the undefined behavior
> if it does happen.
>
> Fixes: 65498c6ae241 ("spi: rockchip: support 4bit words")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> drivers/spi/spi-rockchip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index 936ef54e0903..972beac1169a 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -521,7 +521,7 @@ static void rockchip_spi_config(struct rockchip_spi *rs,
> * ctlr->bits_per_word_mask, so this shouldn't
> * happen
> */
> - unreachable();
> + BUG();
> }
>
> if (use_dma) {
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: rockchip: avoid objtool warning
2021-02-25 12:55 [PATCH] spi: rockchip: avoid objtool warning Arnd Bergmann
` (2 preceding siblings ...)
2021-02-25 21:19 ` Heiko Stübner
@ 2021-02-26 8:15 ` Pratyush Yadav
2021-02-26 9:49 ` Arnd Bergmann
2021-03-01 23:37 ` Mark Brown
4 siblings, 1 reply; 9+ messages in thread
From: Pratyush Yadav @ 2021-02-26 8:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mark Brown, Heiko Stuebner, Nathan Chancellor, Nick Desaulniers,
Emil Renner Berthing, Arnd Bergmann, Jon Lin, Chris Ruehl,
Alexander Kochetkov, Johan Jonker, Vincent Pelletier, linux-spi,
linux-arm-kernel, linux-rockchip, linux-kernel,
clang-built-linux
Hi,
On 25/02/21 01:55PM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Building this file with clang leads to a an unreachable code path
> causing a warning from objtool:
>
> drivers/spi/spi-rockchip.o: warning: objtool: rockchip_spi_transfer_one()+0x2e0: sibling call from callable instruction with modified stack frame
>
> Use BUG() instead of unreachable() to avoid the undefined behavior
> if it does happen.
>
> Fixes: 65498c6ae241 ("spi: rockchip: support 4bit words")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/spi/spi-rockchip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index 936ef54e0903..972beac1169a 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -521,7 +521,7 @@ static void rockchip_spi_config(struct rockchip_spi *rs,
> * ctlr->bits_per_word_mask, so this shouldn't
> * happen
> */
> - unreachable();
> + BUG();
checkpatch says:
Avoid crashing the kernel - try using WARN_ON & recovery code rather
than BUG() or BUG_ON()
Which makes sense to me. This is not something bad enough to justify
crashing the kernel.
> }
>
> if (use_dma) {
> --
> 2.29.2
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: rockchip: avoid objtool warning
2021-02-26 8:15 ` Pratyush Yadav
@ 2021-02-26 9:49 ` Arnd Bergmann
2021-02-26 11:04 ` Pratyush Yadav
0 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2021-02-26 9:49 UTC (permalink / raw)
To: Pratyush Yadav
Cc: Mark Brown, Heiko Stuebner, Nathan Chancellor, Nick Desaulniers,
Emil Renner Berthing, Arnd Bergmann, Jon Lin, Chris Ruehl,
Alexander Kochetkov, Johan Jonker, Vincent Pelletier, linux-spi,
Linux ARM, open list:ARM/Rockchip SoC support, linux-kernel,
clang-built-linux
On Fri, Feb 26, 2021 at 9:16 AM 'Pratyush Yadav' via Clang Built Linux
<clang-built-linux@googlegroups.com> wrote:
>
> Hi,
>
> On 25/02/21 01:55PM, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > Building this file with clang leads to a an unreachable code path
> > causing a warning from objtool:
> >
> > drivers/spi/spi-rockchip.o: warning: objtool: rockchip_spi_transfer_one()+0x2e0: sibling call from callable instruction with modified stack frame
> >
> > Use BUG() instead of unreachable() to avoid the undefined behavior
> > if it does happen.
> >
> > Fixes: 65498c6ae241 ("spi: rockchip: support 4bit words")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > drivers/spi/spi-rockchip.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> > index 936ef54e0903..972beac1169a 100644
> > --- a/drivers/spi/spi-rockchip.c
> > +++ b/drivers/spi/spi-rockchip.c
> > @@ -521,7 +521,7 @@ static void rockchip_spi_config(struct rockchip_spi *rs,
> > * ctlr->bits_per_word_mask, so this shouldn't
> > * happen
> > */
> > - unreachable();
> > + BUG();
>
> checkpatch says:
>
> Avoid crashing the kernel - try using WARN_ON & recovery code rather
> than BUG() or BUG_ON()
>
> Which makes sense to me. This is not something bad enough to justify
> crashing the kernel.
I thought about rewriting it more thoroughly when I wrote the patch, but
couldn't come up with a good alternative, so I did the simplest change
in this direction, replacing the silent crash with a loud one.
Should we just dev_warn() and return instead, hoping that it won't do
more harm?
The backtrace from WARN_ON() probably doesn't help here.
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: rockchip: avoid objtool warning
2021-02-26 9:49 ` Arnd Bergmann
@ 2021-02-26 11:04 ` Pratyush Yadav
2021-02-26 11:15 ` Arnd Bergmann
0 siblings, 1 reply; 9+ messages in thread
From: Pratyush Yadav @ 2021-02-26 11:04 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mark Brown, Heiko Stuebner, Nathan Chancellor, Nick Desaulniers,
Emil Renner Berthing, Arnd Bergmann, Jon Lin, Chris Ruehl,
Alexander Kochetkov, Johan Jonker, Vincent Pelletier, linux-spi,
Linux ARM, open list:ARM/Rockchip SoC support, linux-kernel,
clang-built-linux
On 26/02/21 10:49AM, Arnd Bergmann wrote:
> On Fri, Feb 26, 2021 at 9:16 AM 'Pratyush Yadav' via Clang Built Linux
> <clang-built-linux@googlegroups.com> wrote:
> >
> > Hi,
> >
> > On 25/02/21 01:55PM, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > >
> > > Building this file with clang leads to a an unreachable code path
> > > causing a warning from objtool:
> > >
> > > drivers/spi/spi-rockchip.o: warning: objtool: rockchip_spi_transfer_one()+0x2e0: sibling call from callable instruction with modified stack frame
> > >
> > > Use BUG() instead of unreachable() to avoid the undefined behavior
> > > if it does happen.
> > >
> > > Fixes: 65498c6ae241 ("spi: rockchip: support 4bit words")
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > drivers/spi/spi-rockchip.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> > > index 936ef54e0903..972beac1169a 100644
> > > --- a/drivers/spi/spi-rockchip.c
> > > +++ b/drivers/spi/spi-rockchip.c
> > > @@ -521,7 +521,7 @@ static void rockchip_spi_config(struct rockchip_spi *rs,
> > > * ctlr->bits_per_word_mask, so this shouldn't
> > > * happen
> > > */
> > > - unreachable();
> > > + BUG();
> >
> > checkpatch says:
> >
> > Avoid crashing the kernel - try using WARN_ON & recovery code rather
> > than BUG() or BUG_ON()
> >
> > Which makes sense to me. This is not something bad enough to justify
> > crashing the kernel.
>
> I thought about rewriting it more thoroughly when I wrote the patch, but
> couldn't come up with a good alternative, so I did the simplest change
> in this direction, replacing the silent crash with a loud one.
>
> Should we just dev_warn() and return instead, hoping that it won't do
> more harm?
Hmm... I'm not very familiar with this device or driver so take what I
say with a grain of salt. On the surface level it looks like it might
end up doing something wrong or unexpected.
Returning an error code from this function (along with the dev_warn() or
WARN_ON()) is the most sensible thing to do IMO. If the SPI layer sends
an invalid value then the driver should be well within its rights to
refuse the transaction. The function is currently void but making it
return int seems fairly straightforward.
>
> The backtrace from WARN_ON() probably doesn't help here.
>
> Arnd
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] spi: rockchip: avoid objtool warning
2021-02-26 11:04 ` Pratyush Yadav
@ 2021-02-26 11:15 ` Arnd Bergmann
0 siblings, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2021-02-26 11:15 UTC (permalink / raw)
To: Pratyush Yadav
Cc: Mark Brown, Heiko Stuebner, Nathan Chancellor, Nick Desaulniers,
Emil Renner Berthing, Arnd Bergmann, Jon Lin, Chris Ruehl,
Alexander Kochetkov, Johan Jonker, Vincent Pelletier, linux-spi,
Linux ARM, open list:ARM/Rockchip SoC support, linux-kernel,
clang-built-linux
On Fri, Feb 26, 2021 at 12:05 PM 'Pratyush Yadav' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
> On 26/02/21 10:49AM, Arnd Bergmann wrote:
> > On Fri, Feb 26, 2021 at 9:16 AM 'Pratyush Yadav' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote:
>
> Returning an error code from this function (along with the dev_warn() or
> WARN_ON()) is the most sensible thing to do IMO. If the SPI layer sends
> an invalid value then the driver should be well within its rights to
> refuse the transaction. The function is currently void but making it
> return int seems fairly straightforward.
Indeed, this seems like a clear -EINVAL case. I've updated my patch, will
send after build testing.
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] spi: rockchip: avoid objtool warning
2021-02-25 12:55 [PATCH] spi: rockchip: avoid objtool warning Arnd Bergmann
` (3 preceding siblings ...)
2021-02-26 8:15 ` Pratyush Yadav
@ 2021-03-01 23:37 ` Mark Brown
4 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2021-03-01 23:37 UTC (permalink / raw)
To: Nick Desaulniers, Heiko Stuebner, Arnd Bergmann,
Nathan Chancellor, Emil Renner Berthing
Cc: linux-rockchip, Alexander Kochetkov, Jon Lin, Chris Ruehl,
Johan Jonker, linux-kernel, clang-built-linux, linux-spi,
Arnd Bergmann, Vincent Pelletier, linux-arm-kernel
On Thu, 25 Feb 2021 13:55:34 +0100, Arnd Bergmann wrote:
> Building this file with clang leads to a an unreachable code path
> causing a warning from objtool:
>
> drivers/spi/spi-rockchip.o: warning: objtool: rockchip_spi_transfer_one()+0x2e0: sibling call from callable instruction with modified stack frame
>
> Use BUG() instead of unreachable() to avoid the undefined behavior
> if it does happen.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/1] spi: rockchip: avoid objtool warning
commit: d86e880f7a7c5b64a650146a1353f98750863f21
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 9+ messages in thread