diff mbox series

[v3,07/25] ASoC: renesas: rz-ssi: Use only the proper amount of dividers

Message ID 20241113133540.2005850-8-claudiu.beznea.uj@bp.renesas.com
State New
Headers show
Series [v3,01/25] clk: renesas: r9a08g045-cpg: Add clocks, resets and power domains support for SSI | expand

Commit Message

Claudiu Beznea Nov. 13, 2024, 1:35 p.m. UTC 
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

There is no need to populate the ckdv[] with invalid dividers as that
part will not be indexed anyway. The ssi->audio_mck/bclk_rate should
always be >= 0.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v3:
- s/sh/renesas in patch title

Changes in v2:
- none

 sound/soc/renesas/rz-ssi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Geert Uytterhoeven Dec. 9, 2024, 1:22 p.m. UTC | #1 
Hi Claudiu,

On Wed, Nov 13, 2024 at 2:36 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> There is no need to populate the ckdv[] with invalid dividers as that
> part will not be indexed anyway. The ssi->audio_mck/bclk_rate should
> always be >= 0.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

> --- a/sound/soc/renesas/rz-ssi.c
> +++ b/sound/soc/renesas/rz-ssi.c
> @@ -258,8 +258,7 @@ static void rz_ssi_stream_quit(struct rz_ssi_priv *ssi,
>  static int rz_ssi_clk_setup(struct rz_ssi_priv *ssi, unsigned int rate,
>                             unsigned int channels)
>  {
> -       static s8 ckdv[16] = { 1,  2,  4,  8, 16, 32, 64, 128,
> -                              6, 12, 24, 48, 96, -1, -1, -1 };
> +       static s8 ckdv[] = { 1,  2,  4,  8, 16, 32, 64, 128, 6, 12, 24, 48, 96 };

"u8", as 128 doesn't fit in s8 (why doesn't the compiler complain?).

>         unsigned int channel_bits = 32; /* System Word Length */
>         unsigned long bclk_rate = rate * channels * channel_bits;
>         unsigned int div;

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert
Claudiu Beznea Dec. 9, 2024, 1:31 p.m. UTC | #2 
Hi, Geert,

On 09.12.2024 15:22, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Wed, Nov 13, 2024 at 2:36 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> There is no need to populate the ckdv[] with invalid dividers as that
>> part will not be indexed anyway. The ssi->audio_mck/bclk_rate should
>> always be >= 0.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Thanks for your patch!
> 
>> --- a/sound/soc/renesas/rz-ssi.c
>> +++ b/sound/soc/renesas/rz-ssi.c
>> @@ -258,8 +258,7 @@ static void rz_ssi_stream_quit(struct rz_ssi_priv *ssi,
>>  static int rz_ssi_clk_setup(struct rz_ssi_priv *ssi, unsigned int rate,
>>                             unsigned int channels)
>>  {
>> -       static s8 ckdv[16] = { 1,  2,  4,  8, 16, 32, 64, 128,
>> -                              6, 12, 24, 48, 96, -1, -1, -1 };
>> +       static s8 ckdv[] = { 1,  2,  4,  8, 16, 32, 64, 128, 6, 12, 24, 48, 96 };
> 
> "u8", as 128 doesn't fit in s8 (why doesn't the compiler complain?).

Failed to notice that. Thank you for pointing it! I saw no compiler
complains, though.

Thank you,
Claudiu

> 
>>         unsigned int channel_bits = 32; /* System Word Length */
>>         unsigned long bclk_rate = rate * channels * channel_bits;
>>         unsigned int div;
> 
> The rest LGTM.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
>
Geert Uytterhoeven Dec. 9, 2024, 2:28 p.m. UTC | #3 
Hi Claudiu,

On Mon, Dec 9, 2024 at 2:32 PM Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:
> On 09.12.2024 15:22, Geert Uytterhoeven wrote:
> > On Wed, Nov 13, 2024 at 2:36 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >> -       static s8 ckdv[16] = { 1,  2,  4,  8, 16, 32, 64, 128,
> >> -                              6, 12, 24, 48, 96, -1, -1, -1 };
> >> +       static s8 ckdv[] = { 1,  2,  4,  8, 16, 32, 64, 128, 6, 12, 24, 48, 96 };
> >
> > "u8", as 128 doesn't fit in s8 (why doesn't the compiler complain?).
>
> Failed to notice that. Thank you for pointing it! I saw no compiler
> complains, though.

Me neither. And the code has been storing 128 in s8 since the beginning...

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c
index 2d8721156099..b4439505929f 100644
--- a/sound/soc/renesas/rz-ssi.c
+++ b/sound/soc/renesas/rz-ssi.c
@@ -258,8 +258,7 @@  static void rz_ssi_stream_quit(struct rz_ssi_priv *ssi,
 static int rz_ssi_clk_setup(struct rz_ssi_priv *ssi, unsigned int rate,
 			    unsigned int channels)
 {
-	static s8 ckdv[16] = { 1,  2,  4,  8, 16, 32, 64, 128,
-			       6, 12, 24, 48, 96, -1, -1, -1 };
+	static s8 ckdv[] = { 1,  2,  4,  8, 16, 32, 64, 128, 6, 12, 24, 48, 96 };
 	unsigned int channel_bits = 32;	/* System Word Length */
 	unsigned long bclk_rate = rate * channels * channel_bits;
 	unsigned int div;