Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <111a5b7d-e8e8-7a08-dcac-cad654fb3486@nvidia.com>
Date:   Wed, 4 May 2022 14:05:30 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH] gpio: max77620: Make the irqchip immutable
Content-Language: en-US
To:     Marc Zyngier <maz@kernel.org>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-tegra@vger.kernel.org
References: <20220504111936.8412-1-jonathanh@nvidia.com>
 <877d718r95.wl-maz@kernel.org>
From:   Jon Hunter <jonathanh@nvidia.com>
In-Reply-To: <877d718r95.wl-maz@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VUQrUE1ycjVQSnU2M2VWbFM2WTNTeHBJL0c5WGl0eDZvY1B4aHp4Vm1kd3Zk?=
 =?utf-8?B?NC90WnFzQllyUUFYN3F3OWZMUlRoYmJPc04yODJPZkl5R1BGaHdiUlVxVnE0?=
 =?utf-8?B?ZFJKNGRXMXMrdWZuanVId0V5bkY5SXpiVytvMFFYYnpDUE5lZ1FLRmplRGlE?=
 =?utf-8?B?ZFpMQVJUc3ZsNXRqRnhWKzFTRkRUUnZmQW1LMXAxZHk2RHk2VjZ3Y1FQQy9J?=
 =?utf-8?B?VlZZOHlFR0dmVW00YXNMWENpOCtRRUREY29nWGZxTTd3aHNjSVhybC8wWXNV?=
 =?utf-8?B?RFNMUFAvanJrU0Npd3VQenZXOCt5OXJ0cjBjT2pVMWtWc0Zpb0JxendQU1lo?=
 =?utf-8?B?UTNnUmpwY0JKNjhRMlZCK3Y0bUZOeDJBTk5JaWlQU1oyT2JIbTFNQWRXREFs?=
 =?utf-8?B?SWxwYVk1YkJWbzVoL1B2bjg1ZTRQUzZxeGhzbDhDbkJTYVVub2Q2c0tBUGdB?=
 =?utf-8?B?T2toZ0tRb1QxeGs2RjJMZWpHMmwvZUxySVE2VFFJOTV2YWluSldHdXFuemph?=
 =?utf-8?B?T1B2dmgzZytUaWNQbVBoTEw5bURMSFFtRmVCZUQwT3ZxUVFvM1l2Qnd5TjdG?=
 =?utf-8?B?anhwSU5INEN2QUFBQkw4NzF3Q0ZMeXJJUDBHM3pienJUcmNhdHJXc2JuUHdy?=
 =?utf-8?B?VEt1NlFlWjdJY2c5OUoxMEdOZS9DU3B4RGdVR1FhSUhGVldnZU5tdnFDUFda?=
 =?utf-8?B?Wkxudk1FdnhkSXN4aHVsaGJ0Tzc0cG5YSTZubStJd0krVW1nRDVObFVyOWhy?=
 =?utf-8?B?blFVeFRWZW04WjhOTWZCV3NXR2w5aThDajQrczg5L3NoNGx3MW1nbzhmcDlG?=
 =?utf-8?B?clpMV21TUC9tRGxJN2hGWVZyeHMrUFMxZm9IRm5CZFkzNGVGK2dtNWdmM00w?=
 =?utf-8?B?b1dFbnluOEJnY2pDRzdLQlFyaWRlNzgxT3NreWxtelpJczVJdkgveGxXS2o4?=
 =?utf-8?B?dGtKaERIL2Jmd1h5Nlg0U0Y3TFNuQUtmYll1U2VncDhpdmZUNFk5K3I0THZV?=
 =?utf-8?B?Q0lRMklweXlkMjJjRU5QRXJ1MjN1OVpibHc5aitFVnVMcDNPUnhKOVN0T2Ra?=
 =?utf-8?B?dmI1dUs4S1RHZ01aUElUSmFUMlg3VDVVTUpONkV3dldhSDRPeEJzSFpFbTZj?=
 =?utf-8?B?Mndib3c0aURMT1BjZG1zN3BWUmpiYVVLWjI4TTVPdEpVYWRYZVhuU3RVZDdy?=
 =?utf-8?B?NmZocW5IdFo3enljRE9Dem9ORVppTUdUcWdkUGFKOFJoRXlXaFZNdW5tZW55?=
 =?utf-8?B?eEFzWUpERjhtTDdTRExjRjExeVBZeG5NMHc3YXJ3MmNMOUprNEg2SkFuTWtx?=
 =?utf-8?B?a1dvSXRHV1BzdlNkaFpWY3poV1FyTXFuN0RCS2huZDZIaXlhL0xNMlVvS0ZB?=
 =?utf-8?B?MkExSVd3TkIrTFdZdmsyRTlUUjBMdTZSa2dUMTd0aGYrLzEwMUVWQS9lOW5G?=
 =?utf-8?B?WmxYdmpuelErNWZqelUxTCtZYXB4Q29uR1orZGtVQUZNTmg3cDJ1Qzd4Z3ZX?=
 =?utf-8?B?YlE0ekUwYTA5dmd3YXEwRW91ajRxTnlCMjAyYkNQOWxKaG9xTE9WK3J4TXFI?=
 =?utf-8?B?QnBSMlN1eDViZEFCbkxJNmM3NE5odk9XOG1LT2hITVJNeEUvN3Q3WXVYY1hP?=
 =?utf-8?B?OG5oSC80Mk5EVzZBRDM5Y2xjS2NseEtkMEdQSnpLTzczbFVJMjBZYXRkS0Rp?=
 =?utf-8?B?VW1seDBTVEcyM09VeXNLbUZkb1VkU2YzdWRva3dFcWtMekxlWENORnNlT1cw?=
 =?utf-8?B?Rk40OStkMDJzWmtJOVNRc1EvTnZWcGJxejQxSHJXMlExTWE5SzFKMStKK2s5?=
 =?utf-8?B?cHEra21XSXg0VmNOaUtkZkw4b3NraWViRklOMXI1ZkduN2ZRWWpiNmtkazNI?=
 =?utf-8?B?UVV1aThWQjVtNjFPUlViaWxJdjdxS1Vpd1E2QWZlYTUwbTRQNHVVdU1COUNs?=
 =?utf-8?B?WHpWKzh0akdVajdkREYrOFpXOGRlcENTeFNtS0N1b2QzVWg0blFxZGNMakdp?=
 =?utf-8?B?Ym5xWGpneDNNRC9SeEJ6UjcwcHUrZkhGdEd2RlM1K0VQNHQzY2hzZWdpNU5X?=
 =?utf-8?B?VXVKVFhWNGFVa1M3QUlocFZYTVludWJqMWtvSS9ZSVMyU3B1NUxDajJ6M1c3?=
 =?utf-8?B?ajJkeEtYcHZtZEZHanh1d0FFWUt0aHNXTGx1ejNISjN1MU1pa0VQSjBxRk9i?=
 =?utf-8?B?eE1ub05pU0t4cmIzbU1VVkhHRnU0SXNJam8yRE9pS1l4dUZMNzNlVENFaSti?=
 =?utf-8?B?ektvc25XWlErQVBDdG13eVZEbytHamZNNlZKK2w5S25tamxpL3IzS0I5eUh2?=
 =?utf-8?B?dzllSGZRaHVWQUxuTWFGN1h1MElwVVovSGliV2FVRks3TVA1V2QvUT09?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8b621e70-48fe-49a7-e5e5-08da2dcec72a
X-MS-Exchange-CrossTenant-AuthSource: CO6PR12MB5444.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 May 2022 13:05:35.7148
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: m4hnExUzZeZXs+Y7USH0m64Nj9HQsNTmd+3J3iv4NRwx7EGDmXo961ItEuQVbtGNuOOO2peO0WAbb73e5o9utQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR1201MB2556
X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 04/05/2022 12:35, Marc Zyngier wrote:
> Hi Jon,
> 
> On Wed, 04 May 2022 12:19:36 +0100,
> Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>> Commit 6c846d026d49 ("gpio: Don't fiddle with irqchips marked as
>> immutable") added a warning to indicate if the gpiolib is altering the
>> internals of irqchips. Following this change the following warning is
>> now observed for the max77620 gpio driver ...
>>
>>   WARNING KERN gpio gpiochip0: (max77620-gpio): not an immutable chip,
>>   	please consider fixing it!
>>
>> Fix the above warning by making the max77620 gpio driver immutable.
> 
> Thanks for looking into this. Comments below.
> 
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>> This warning is observed with v5.18-rc5 and so it would be great if
>> we can fix for v5.18. This is triggering a kernel warning test failure
>> on one of our platforms.
> 
> I'm surprised. This is definitely *not* 5.18 material, and I can't see
> the patches in Linus' tree. Are you sure you're not running -next
> instead?

Ah, you are correct, it was -next and not v5.18-rc5!

>>
>>   drivers/gpio/gpio-max77620.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c
>> index ebf9dea6546b..aa92658780d6 100644
>> --- a/drivers/gpio/gpio-max77620.c
>> +++ b/drivers/gpio/gpio-max77620.c
>> @@ -119,14 +119,23 @@ static void max77620_gpio_bus_sync_unlock(struct irq_data *data)
>>   	mutex_unlock(&gpio->buslock);
>>   }
>>   
>> -static struct irq_chip max77620_gpio_irqchip = {
>> +static void max77620_gpio_irq_print_chip(struct irq_data *data, struct seq_file *p)
>> +{
>> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
>> +
>> +	seq_printf(p, dev_name(gc->parent));
>> +}
>> +
> 
> I'd rather you don't do this. This was added as a band-aid for drivers
> that display a device-specific string in /proc/interrupts, and that we
> cannot fix because the string is in effect an ABI.
> 
> Here, you're going in the opposite direction (making the name
> device-specific while it was constant so far). This has the same
> ABI-breaking effect, and we shouldn't do that. It is also rather
> pointless, as we already have all the required debugging information
> in debugfs (and I assume that this is the reason this is added).


OK, I will drop this.

>> +static const struct irq_chip max77620_gpio_irqchip = {
>>   	.name		= "max77620-gpio",
>>   	.irq_mask	= max77620_gpio_irq_mask,
>>   	.irq_unmask	= max77620_gpio_irq_unmask,
> 
> You seem to be missing the updates for these two functions. Please see
> the updated documentation in commit 5644b66a9c63 ("Documentation:
> Update the recommended pattern for GPIO irqchips").

Yes I see. OK, I will add that.

Thanks! Jon

-- 
nvpublic