Re: [PATCH 1/5] pinctrl: qcom: msm8998: Fix MPM mappings

[Konrad Dybcio]

On 10.08.2023 09:14, Stephan Gerhold wrote:
> On Wed, Aug 09, 2023 at 09:38:54PM +0200, Konrad Dybcio wrote:
>> Commit 29f6e7e379fd ("pinctrl: qcom: msm8998: Add MPM pin mappings")
>> added a map of pins <-> wakeirqs. The values in each tuple were swapped
>> and the last one was missing. Fix that.
>>
>> Fixes: 29f6e7e379fd ("pinctrl: qcom: msm8998: Add MPM pin mappings")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
>> ---
>>  drivers/pinctrl/qcom/pinctrl-msm8998.c | 19 ++++++++++---------
>>  1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-msm8998.c b/drivers/pinctrl/qcom/pinctrl-msm8998.c
>> index b7cbf32b3125..08d6e555652a 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-msm8998.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-msm8998.c
>> @@ -1496,15 +1496,16 @@ static const struct msm_pingroup msm8998_groups[] = {
>>  };
>>  
>>  static const struct msm_gpio_wakeirq_map msm8998_mpm_map[] = {
>> -	{ 1, 3 }, { 5, 4 }, { 9, 5 }, { 11, 6 }, { 22, 8 }, { 24, 9 }, { 26, 10 },
>> -	{ 34, 11 }, { 36, 12 }, { 37, 13 }, { 38, 14 }, { 40, 15 }, { 42, 16 }, { 46, 17 },
>> -	{ 50, 18 }, { 53, 19 }, { 54, 20 }, { 56, 21 }, { 57, 22 }, { 58, 23 }, { 59, 24 },
>> -	{ 60, 25 }, { 61, 26 }, { 62, 27 }, { 63, 28 }, { 64, 29 }, { 66, 7 }, { 71, 30 },
>> -	{ 73, 31 }, { 77, 32 }, { 78, 33 }, { 79, 34 }, { 80, 35 }, { 82, 36 }, { 86, 37 },
>> -	{ 91, 38 }, { 92, 39 }, { 95, 40 }, { 97, 41 }, { 101, 42 }, { 104, 43 }, { 106, 44 },
>> -	{ 108, 45 }, { 110, 48 }, { 112, 46 }, { 113, 47 }, { 115, 51 }, { 116, 54 }, { 117, 55 },
>> -	{ 118, 56 }, { 119, 57 }, { 120, 58 }, { 121, 59 }, { 122, 60 }, { 123, 61 }, { 124, 62 },
>> -	{ 125, 63 }, { 126, 64 }, { 127, 50 }, { 129, 65 }, { 131, 66 }, { 132, 67 }, { 133, 68 },
>> +	{ 3, 1 }, { 4, 5 }, { 5, 9 }, { 6, 11 }, { 8, 22 }, { 9, 24 }, { 10, 26 },
>> +	{ 11, 34 }, { 12, 36 }, { 13, 37 }, { 14, 38 }, { 15, 40 }, { 16, 42 }, { 17, 46 },
>> +	{ 18, 50 }, { 19, 53 }, { 20, 54 }, { 21, 56 }, { 22, 57 }, { 23, 58 }, { 24, 59 },
>> +	{ 25, 60 }, { 26, 61 }, { 27, 62 }, { 28, 63 }, { 29, 64 }, { 7, 66 }, { 30, 71 },
>> +	{ 31, 73 }, { 32, 77 }, { 33, 78 }, { 34, 79 }, { 35, 80 }, { 36, 82 }, { 37, 86 },
>> +	{ 38, 91 }, { 39, 92 }, { 40, 95 }, { 41, 97 }, { 42, 101 }, { 43, 104 }, { 44, 106 },
>> +	{ 45, 108 }, { 48, 110 }, { 46, 112 }, { 47, 113 }, { 51, 115 }, { 54, 116 }, { 55, 117 },
>> +	{ 56, 118 }, { 57, 119 }, { 58, 120 }, { 59, 121 }, { 60, 122 }, { 61, 123 }, { 62, 124 },
>> +	{ 63, 125 }, { 64, 126 }, { 50, 127 }, { 65, 129 }, { 66, 131 }, { 67, 132 }, { 68, 133 },
>> +	{ 69, 145 },
>>  };
> 
> Are you sure this is correct?
> 
> /**
>  * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins
>  * @gpio:          The GPIOs that are wakeup capable
>  * @wakeirq:       The interrupt at the always-on interrupt controller
>  */
> struct msm_gpio_wakeirq_map {
> 	unsigned int gpio;
> 	unsigned int wakeirq;
> };
> 
> MSM8998 has 150 GPIOs and 96 MPM IRQs. The tuple { 69, 145 } can't be
> right because 145 is not a valid MPM pin. It's a valid GPIO though so
> the original order was correct. Maybe replace "Fixes:" with "Breaks:"? :D
Went over this again, this is totally a "Breaks"

Konrad