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

["H. Nikolaus Schaller" <hns@xxxxxxxxxxxxx>]

Hi Paul and Conor,

> Am 28.02.2025 um 19:26 schrieb 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 :)

Ah, sometimes this gets missed.

> 
> 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.

I'll send a V3 tomorrow together with Conor's comment/fix for 1/4.

BR and thanks,
Nikolaus

> 
> 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 = {
>