Re: [alsa-devel] ASoC: rsnd: fix sound route path when using SRC6/SRC9
Hi Morimoto-san,
On Fri, Mar 31, 2017 at 9:32 PM, Linux Kernel Mailing List linux-kernel@vger.kernel.org wrote:
Web: https://git.kernel.org/torvalds/c/a1c2ff53726907aff5feb37e4cfd45c1ff626431 Commit: a1c2ff53726907aff5feb37e4cfd45c1ff626431 Parent: 4b30eebfc35c67771b5f58d9274d3e321b72d7a8 Refname: refs/heads/master Author: Hiroyuki Yokoyama hiroyuki.yokoyama.vx@renesas.com AuthorDate: Wed Mar 1 03:51:00 2017 +0000 Committer: Mark Brown broonie@kernel.org CommitDate: Mon Mar 13 12:58:07 2017 +0000
ASoC: rsnd: fix sound route path when using SRC6/SRC9 This patch fixes the problem that the missing value of the route path setting table and incorrect values are set in the CMD_ROUTE_SELECT register. Signed-off-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com> [Kuninori: shared data on MIX and non-MIX case] Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Signed-off-by: Mark Brown <broonie@kernel.org>
sound/soc/sh/rcar/cmd.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/sound/soc/sh/rcar/cmd.c b/sound/soc/sh/rcar/cmd.c index abb5eaac854a..7d92a24b7cfa 100644 --- a/sound/soc/sh/rcar/cmd.c +++ b/sound/soc/sh/rcar/cmd.c @@ -31,23 +31,24 @@ static int rsnd_cmd_init(struct rsnd_mod *mod, struct rsnd_mod *mix = rsnd_io_to_mod_mix(io); struct device *dev = rsnd_priv_to_dev(priv); u32 data;
u32 path[] = {[1] = 1 << 0,[5] = 1 << 8,[6] = 1 << 12,[9] = 1 << 15,}; if (!mix && !dvc) return 0;if (ARRAY_SIZE(path) < rsnd_mod_id(mod) + 1)return -ENXIO;if (mix) { struct rsnd_dai *rdai; struct rsnd_mod *src; struct rsnd_dai_stream *tio; int i;
u32 path[] = {[0] = 0,[1] = 1 << 0,[2] = 0,[3] = 0,[4] = 0,[5] = 1 << 8}; /* * it is assuming that integrater is well understanding about@@ -70,16 +71,19 @@ static int rsnd_cmd_init(struct rsnd_mod *mod, } else { struct rsnd_mod *src = rsnd_io_to_mod_src(io);
u32 path[] = {[0] = 0x30000,[1] = 0x30001,[2] = 0x40000,[3] = 0x10000,[4] = 0x20000,[5] = 0x40100
u8 cmd_case[] = {[0] = 0x3,[1] = 0x3,[2] = 0x4,[3] = 0x1,[4] = 0x2,[5] = 0x4,[6] = 0x1,[9] = 0x2, };
data = path[rsnd_mod_id(src)];
data = path[rsnd_mod_id(src)] |
With gcc 4.9.0:
sound/soc/sh/rcar/cmd.c: In function 'rsnd_cmd_init': sound/soc/sh/rcar/cmd.c:85:14: warning: array subscript is below array bounds [-Warray-bounds] data = path[rsnd_mod_id(src)] |
cmd_case[rsnd_mod_id(src)] << 16; } dev_dbg(dev, "ctu/mix path = 0x%08x", data);
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert
@@ -70,16 +71,19 @@ static int rsnd_cmd_init(struct rsnd_mod *mod, } else { struct rsnd_mod *src = rsnd_io_to_mod_src(io);
u32 path[] = {[0] = 0x30000,[1] = 0x30001,[2] = 0x40000,[3] = 0x10000,[4] = 0x20000,[5] = 0x40100
u8 cmd_case[] = {[0] = 0x3,[1] = 0x3,[2] = 0x4,[3] = 0x1,[4] = 0x2,[5] = 0x4,[6] = 0x1,[9] = 0x2, };
data = path[rsnd_mod_id(src)];
data = path[rsnd_mod_id(src)] |With gcc 4.9.0:
sound/soc/sh/rcar/cmd.c: In function 'rsnd_cmd_init': sound/soc/sh/rcar/cmd.c:85:14: warning: array subscript is belowarray bounds [-Warray-bounds] data = path[rsnd_mod_id(src)] |
Hmm.... I tried random code/compile, but gcc behavior seems strange. Fore example if I removed next line, this warning disappeard
- data = path[rsnd_mod_id(src)] | - cmd_case[rsnd_mod_id(src)] << 16; + data = path[rsnd_mod_id(src)];
Is this related to this ? https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59124
Best regards --- Kuninori Morimoto
Hi Morimoto-san,
On Wed, May 17, 2017 at 3:32 AM, Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
@@ -70,16 +71,19 @@ static int rsnd_cmd_init(struct rsnd_mod *mod, } else { struct rsnd_mod *src = rsnd_io_to_mod_src(io);
u32 path[] = {[0] = 0x30000,[1] = 0x30001,[2] = 0x40000,[3] = 0x10000,[4] = 0x20000,[5] = 0x40100
u8 cmd_case[] = {[0] = 0x3,[1] = 0x3,[2] = 0x4,[3] = 0x1,[4] = 0x2,[5] = 0x4,[6] = 0x1,[9] = 0x2, };
data = path[rsnd_mod_id(src)];
data = path[rsnd_mod_id(src)] |With gcc 4.9.0:
sound/soc/sh/rcar/cmd.c: In function 'rsnd_cmd_init': sound/soc/sh/rcar/cmd.c:85:14: warning: array subscript is belowarray bounds [-Warray-bounds] data = path[rsnd_mod_id(src)] |
Hmm.... I tried random code/compile, but gcc behavior seems strange. Fore example if I removed next line, this warning disappeard
data = path[rsnd_mod_id(src)] |cmd_case[rsnd_mod_id(src)] << 16;
data = path[rsnd_mod_id(src)];Is this related to this ? https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59124
Perhaps.
I guess the main question is: can src be NULL here? If yes, rsnd_mod_id(src) will be -1.
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert
Hmm.... I tried random code/compile, but gcc behavior seems strange. Fore example if I removed next line, this warning disappeard
data = path[rsnd_mod_id(src)] |cmd_case[rsnd_mod_id(src)] << 16;
data = path[rsnd_mod_id(src)];Is this related to this ? https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59124
Perhaps.
I guess the main question is: can src be NULL here? If yes, rsnd_mod_id(src) will be -1.
No. cmd itself will not be used if src was NULL on this HW. But, yes, it seems gcc is checking -1 value here. It is OK, but why it warns only here ??
Best regards --- Kuninori Morimoto
Hi Morimoto-san,
On Wed, May 17, 2017 at 9:38 AM, Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
Hmm.... I tried random code/compile, but gcc behavior seems strange. Fore example if I removed next line, this warning disappeard
data = path[rsnd_mod_id(src)] |cmd_case[rsnd_mod_id(src)] << 16;
data = path[rsnd_mod_id(src)];Is this related to this ? https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59124
Perhaps.
I guess the main question is: can src be NULL here? If yes, rsnd_mod_id(src) will be -1.
No. cmd itself will not be used if src was NULL on this HW.
But gcc cannot know that, as the function is called through a function pointer from another source file.
But, yes, it seems gcc is checking -1 value here. It is OK, but why it warns only here ??
Yes, I'm also wondering why we see the warning only here. I guess we should be lucky that we get it at least in one place ;-)
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert
I guess the main question is: can src be NULL here? If yes, rsnd_mod_id(src) will be -1.
No. cmd itself will not be used if src was NULL on this HW.
But gcc cannot know that, as the function is called through a function pointer from another source file.
Yes, I agree
But, yes, it seems gcc is checking -1 value here. It is OK, but why it warns only here ??
Yes, I'm also wondering why we see the warning only here. I guess we should be lucky that we get it at least in one place ;-)
I think it is gcc's something, and latest (?) gcc doesn't have such behavior. Anyway, I posted fixup patch to ML and you
Best regards --- Kuninori Morimoto
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Without this patch, gcc 4.9.x says
sound/soc/sh/rcar/cmd.c: In function 'rsnd_cmd_init': sound/soc/sh/rcar/cmd.c:85:14: warning: array subscript is below array\ bounds [-Warray-bounds] data = path[rsnd_mod_id(src)] |
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/rcar/cmd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/sh/rcar/cmd.c b/sound/soc/sh/rcar/cmd.c index d879c01..9a136d8 100644 --- a/sound/soc/sh/rcar/cmd.c +++ b/sound/soc/sh/rcar/cmd.c @@ -82,6 +82,9 @@ static int rsnd_cmd_init(struct rsnd_mod *mod, [9] = 0x2, };
+ if (unlikely(!src)) + return -EIO; + data = path[rsnd_mod_id(src)] | cmd_case[rsnd_mod_id(src)] << 16; }
The patch
ASoC: rsnd: check src mod pointer for rsnd_mod_id()
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 374503c6109e60f48fa9b11341b14466f07bd3f4 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 17 May 2017 06:50:32 +0000 Subject: [PATCH] ASoC: rsnd: check src mod pointer for rsnd_mod_id()
Without this patch, gcc 4.9.x says
sound/soc/sh/rcar/cmd.c: In function 'rsnd_cmd_init': sound/soc/sh/rcar/cmd.c:85:14: warning: array subscript is below array\ bounds [-Warray-bounds] data = path[rsnd_mod_id(src)] |
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sh/rcar/cmd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/sh/rcar/cmd.c b/sound/soc/sh/rcar/cmd.c index 7d92a24b7cfa..98835e9d1d7d 100644 --- a/sound/soc/sh/rcar/cmd.c +++ b/sound/soc/sh/rcar/cmd.c @@ -82,6 +82,9 @@ static int rsnd_cmd_init(struct rsnd_mod *mod, [9] = 0x2, };
+ if (unlikely(!src)) + return -EIO; + data = path[rsnd_mod_id(src)] | cmd_case[rsnd_mod_id(src)] << 16; }
participants (3)
-
Geert Uytterhoeven -
Kuninori Morimoto -
Mark Brown