Re: [PATCH v2] ASoC: rcar: adg: correct TIMSEL setting for SSI9

[Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>]

Hi Eugeniu

> Timing select registers for SRC and CMD are by default
> referring to the corresponding SSI word select.
> The calculation rule from HW spec skips SSI8, which has
> no clock connection.
> 
> From section 43.2.18 CMD Output Timing Select Register (CMDOUT_TIMSEL),
> of R-Car Series, 3rd Generation Hardware Userʼs Manual Rev.2.20:
> 
> CMD0_OUT_DIVCLK_	Output Timing
> SEL [4:0]		Signal Select
> B'0 0110: 		ssi_ws0
> B'0 0111: 		ssi_ws1
> B'0 1000: 		ssi_ws2
> B'0 1001: 		ssi_ws3
> B'0 1010: 		ssi_ws4
> B'0 1011: 		ssi_ws5
> B'0 1100: 		ssi_ws6
> B'0 1101: 		ssi_ws7
> 	<GAP>
> B'0 1110: 		ssi_ws9
> B'0 1111: 		Setting prohibited
> 
> Fix the erroneous prohibited setting of timsel value 1111 (0xf) for SSI9
> by using timsel value 1110 (0xe) instead. This is possible because SSI8
> is not connected as shown by <GAP> in the table above.
> 
> [21.695055] rcar_sound ec500000.sound: b adg[0]-CMDOUT_TIMSEL (32):00000f00/00000f1f
> 
> Correct the timsel assignment.
> 
> Fixes: 629509c5bc478c ("ASoC: rsnd: add Gen2 SRC and DMAEngine support")
> Suggested-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Signed-off-by: Andreas Pape <Andreas.Pape4@xxxxxxxxx>
> Signed-off-by: Yeswanth Rayapati <yeswanth.rayapati@xxxxxxxxxxxx>
> Tested-by: Yeswanth Rayapati <yeswanth.rayapati@xxxxxxxxxxxx>
> [erosca: massage commit description]
> Signed-off-by: Eugeniu Rosca <eugeniu.rosca@xxxxxxxxx>
> ---
> 
> Changes v1->v2:
>  - Link v1: https://lore.kernel.org/linux-sound/20240223163502.11619-1-erosca@xxxxxxxxxxxxxx/
>  - Employed the proposal from Morimoto-san
>  - Kept in mind below mapping/calculation rule:
> 
> 	W/o pinsharing
> 	SSI0 -> base+0
> 	SSI1 -> base+1
> 	SSI2 -> base+2
> 	SSI3 -> base+3
> 	SSI4 -> base+4
> 	SSI5 -> base+5
> 	SSI6 -> base+6
> 	SSI7 -> base+7
> 	SSI8 ->
> 	SSI9 -> base+8 =
> 	--> "SSI8 not connected, so SSI9 uses '8' "
> 
> 	W/ pinsharing (0,1,2,9 can be combined, 3,4 can be combined, 7,8 can be combined)
> 	SSI0 -> base+0
> 	SSI1 -> base+0
> 	SSI2 -> base+0
> 	SSI3 -> base+3
> 	SSI4 -> base+3
> 	SSI5 -> base+5
> 	SSI6 -> base+6
> 	SSI7 -> base+7
> 	SSI8 -> base+7
> 	SSI9 -> base+0
> 
>  sound/soc/sh/rcar/adg.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c
> index 230c48648af359..afd69c6eb6544c 100644
> --- a/sound/soc/sh/rcar/adg.c
> +++ b/sound/soc/sh/rcar/adg.c
> @@ -111,6 +111,13 @@ static u32 rsnd_adg_ssi_ws_timing_gen2(struct rsnd_dai_stream *io)
>  			ws = 7;
>  			break;
>  		}
> +	} else {
> +		/*
> +		 * SSI8 is not connected to ADG.
> +		 * Thus SSI9 is using ws = 8
> +		 */
> +		if (id == 9)
> +			ws = 8;
>  	}
>  
>  	return (0x6 + ws) << 8;
> -- 
> 2.43.2
> 

Looks good to me ! Thanks

Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>


Thank you for your help !!

Best regards
---
Renesas Electronics
Ph.D. Kuninori Morimoto