Re: [PATCH v2 4/4] pinctrl: ingenic: jz4730: add pinmux for I2S interface

[Paul Cercueil <paul@xxxxxxxxxxxxxxx>]

Hi Nikolaus,

Le vendredi 28 février 2025 à 14:33 +0100, H. Nikolaus Schaller a
écrit :
> I2S is used for the sound codec of the Alpha400.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
> ---
>  drivers/pinctrl/pinctrl-ingenic.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-ingenic.c
> b/drivers/pinctrl/pinctrl-ingenic.c
> index 08e082e84f5c6..6d7dc077c373e 100644
> --- a/drivers/pinctrl/pinctrl-ingenic.c
> +++ b/drivers/pinctrl/pinctrl-ingenic.c
> @@ -209,10 +209,14 @@ static int jz4730_nand_cs4_pins[] = { 0x56, };
>  static int jz4730_nand_cs5_pins[] = { 0x57, };
>  static int jz4730_pwm_pwm0_pins[] = { 0x5e, };
>  static int jz4730_pwm_pwm1_pins[] = { 0x5f, };
> -

Just a nit - but you remove a blank line in patch 4/4 that was added in
3/4, better not add it in the first place :)

That's the only comment I have on the whole patchset, so either fix it
in a v3 while adding my review tag, or maybe Linus can do it when
applying the patchset.

Cheers,
-Paul

>  static int jz4730_mii_pins[] = { 0x70, 0x71, 0x72, 0x73, 0x74, 0x75,
> 0x76,
>  				 0x77, 0x78, 0x19, 0x7a, 0x1b, 0x7c,
> };
>  
> +static int jz4730_i2s_mclk_pins[] = { 0x44, };
> +static int jz4730_i2s_acreset_pins[] = { 0x45, };
> +static int jz4730_i2s_data_pins[] = { 0x46, 0x47, };
> +static int jz4730_i2s_clock_pins[] = { 0x4d, 0x4e, };
> +
>  static u8 jz4730_lcd_8bit_funcs[] = { 1, 1, 1, 1, 1, 1, 1, 1, 2, 2,
> 2, };
>  
>  static const struct group_desc jz4730_groups[] = {
> @@ -235,6 +239,11 @@ static const struct group_desc jz4730_groups[] =
> {
>  	INGENIC_PIN_GROUP("pwm0", jz4730_pwm_pwm0, 1),
>  	INGENIC_PIN_GROUP("pwm1", jz4730_pwm_pwm1, 1),
>  	INGENIC_PIN_GROUP("mii", jz4730_mii, 1),
> +	INGENIC_PIN_GROUP("i2s-mclk-out", jz4730_i2s_mclk, 1),
> +	INGENIC_PIN_GROUP("i2s-acreset", jz4730_i2s_acreset, 1),
> +	INGENIC_PIN_GROUP("i2s-data", jz4730_i2s_data, 1),
> +	INGENIC_PIN_GROUP("i2s-master", jz4730_i2s_clock, 1),
> +	INGENIC_PIN_GROUP("i2s-slave", jz4730_i2s_clock, 2),
>  };
>  
>  static const char *jz4730_mmc_groups[] = { "mmc-1bit", "mmc-4bit",
> };
> @@ -251,6 +260,7 @@ static const char *jz4730_nand_groups[] = {
>  static const char *jz4730_pwm0_groups[] = { "pwm0", };
>  static const char *jz4730_pwm1_groups[] = { "pwm1", };
>  static const char *jz4730_mii_groups[] = { "mii", };
> +static const char *jz4730_i2s_groups[] = { "i2s-data", "i2s-master",
> "i2s-slave", };
>  
>  static const struct function_desc jz4730_functions[] = {
>  	INGENIC_PIN_FUNCTION("mmc", jz4730_mmc),
> @@ -263,6 +273,7 @@ static const struct function_desc
> jz4730_functions[] = {
>  	INGENIC_PIN_FUNCTION("pwm0", jz4730_pwm0),
>  	INGENIC_PIN_FUNCTION("pwm1", jz4730_pwm1),
>  	INGENIC_PIN_FUNCTION("mii", jz4730_mii),
> +	INGENIC_PIN_FUNCTION("i2s", jz4730_i2s),
>  };
>  
>  static const struct ingenic_chip_info jz4730_chip_info = {