Received-SPF: pass (google.com: domain of linux-kernel+bounces-48224-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <e73426f6-6d50-4ed7-8613-1ec42fa3f991@amd.com>
Date: Thu, 1 Feb 2024 18:50:33 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] platform/x86/amd/hsmp: switch to use device_add_groups()
To: =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>,
 Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: naveenkrishna.chatradhi@amd.com, LKML <linux-kernel@vger.kernel.org>,
 Carlos Bilbao <carlos.bilbao@amd.com>, Hans de Goede <hdegoede@redhat.com>,
 platform-driver-x86@vger.kernel.org
References: <2024012822-exalted-fidgeting-f180@gregkh>
 <0d110d2a-da0c-017a-0e5a-fc6bef7b066a@linux.intel.com>
Content-Language: en-US
From: "Hegde, Suma" <Suma.Hegde@amd.com>
In-Reply-To: <0d110d2a-da0c-017a-0e5a-fc6bef7b066a@linux.intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?blY1cWRybkl0cDNpMC9aa1VPd0RTMnJrWlU3VHI2QTgrMkV6ank3V3I2YjVp?=
 =?utf-8?B?WDFCNjBHQWNDL05pYnBPYnRMT09iM3FwNUppazRVN2pQYndIU3RaSHRRdkVx?=
 =?utf-8?B?ZUxMSG9EN0RqcDBmZVhWSExNd01xOHdYNDlnUm5WM28wdkVpZExyREtkbFJE?=
 =?utf-8?B?SWxuQ0FOTHhNVHpIaEJpNzk1U0NQbzk0ZWpTdUJxWGovU1RTcFAyWVdGeHRP?=
 =?utf-8?B?YklYMlNZOWhDR2FOaytGaVd2QVB2UnByMmRJTTF6Y3RJcEx6UE5mV3NmcitO?=
 =?utf-8?B?bVF6VGNzMlhGc0dqYjRFQWFBSFB4UGRSckZDaFdSdU1LYjI3anh1eVZXRnQz?=
 =?utf-8?B?WXJ6c3BueFRpRXdkcTdjcGJsalRDNWY0dGVja1U2RWYwRU1md3ZKbGJUVkJB?=
 =?utf-8?B?R0tBWlF4cS9CRC9IWkF6aVlsYWhkTGcyWVMvbXdtb1BKUngxVzNlZGxxb2lK?=
 =?utf-8?B?SUszSk55bmVmbCszaUk5bmtTZ3RWaW55SWl6VCs5VC8rRHJ1ZkFUajZzU2xS?=
 =?utf-8?B?dFFhQXdYYlRuUTdvTVpnZjhKZEtVdlNBZEZaMlkxTTA0OXFSNTBSK0lYczNw?=
 =?utf-8?B?TkM0aVl1bTdJcTRqMkozUkcreEpKZEZCZTIvMldZVWU4a3hBcnArZ3htR1p2?=
 =?utf-8?B?R2FKL2JHNTgxSUJ5V0FmbzFJdjhzMmp1VXc1WEpJM1BIM2JoYlVvNWg1TXEx?=
 =?utf-8?B?RlZjVEpmRnlFeUdwOFN6MTBGSWdtZHhpWVhZZS8ydkpIV0pxTTBuUWMxdWxx?=
 =?utf-8?B?N2JuMEw0eGdXU0J5WkpiT0dwMU1WdTMyeWdWbXo5aW9qOGloa0gyZG4zTWQ5?=
 =?utf-8?B?SjFMSnRSN1pHRGhZQWp4SElEQlFuOHJrSmg3Mlp4SXR2dzl5UGx0MGRMemsw?=
 =?utf-8?B?Rm56N3NSdHQ5SUpnNThWTXJWaFJQeml4RHArbEE2Ynlpa2czVEFHK3RTRUxH?=
 =?utf-8?B?VnZFNTBTa0ltSVJadUVGSlkwZWYvN3lRWEZHNTN5VWZSS0g3RGdHYXlyQ3lt?=
 =?utf-8?B?RGZVUjFpejJmTmIyZGRqZTFYU2FDaFBEYjkyaGhCWUlWdDRnb2t3cXpvM1g2?=
 =?utf-8?B?QVVPN0ZIYVFWKy8wSUtTL2hDYkJuMmR4dmxMTkNqWGxIak1GZFpDVVQ4MFgr?=
 =?utf-8?B?cVIwbjE3THFhWXBrQVFCR1pYM1NWdlRpMU1XMzBZaHNVemFaMWJSRHQ2Tyt4?=
 =?utf-8?B?MVQrOHR5bWxUd0gveWxvSHJ2cmlWS1hPOEVHcTVOUFU5NzcxMlhTNkpIZXpR?=
 =?utf-8?B?Uy9nMG5xVHQ3ZTVYS3JRTmxHWlNPajZEd3BrdVdQVG1oZDlaTDdnWnowQUpq?=
 =?utf-8?B?TEZLRWJMVFFhcFBVM25ONytXWDR0S3MzOXNvdy92OHdMVmZrS3pNNTNWcUwx?=
 =?utf-8?B?d1Z6RFo4c3dvYkdhU2dobGNHakg3djE5K0ZhQ2lobjlsSmVZT2szU1ErQ3Ju?=
 =?utf-8?B?WFNwNkFQQytLOC9FbWRZU1RvcmpReEowNFNBczJYbVdrRXFQV1czeVlsUGJN?=
 =?utf-8?B?RnlqSFVZdUU2UnRZakhoRkl6MEQxWkpxSXcxazBIMFBNNzJwNm8xdXNNUHpj?=
 =?utf-8?B?Zmh3RXNySTFrKzdwTDNlZXlvZTZlZkt1TGNLVkliQ2xhVHpNeDRBZ2w5a0Iz?=
 =?utf-8?B?a3crVnd1VFB6bWViNnpZTFd1ZmdVMWQ5M2VRTEs3UDBNSFRNSXFMTHYzTXdU?=
 =?utf-8?B?Rkd4dDRoTzdzRmxGdHBpVG45ME5TY2ZVSEJ4clZUVFd6SzNxYkE1MU05eG1u?=
 =?utf-8?B?eVBmMEoyZkJvYnhBTmxpS1pheGlqZFVHUXVFMU9Sd1lOMkMwNE1BRHJTZXkz?=
 =?utf-8?B?eHppNGhQTXFKeHZpaDRIcXgvaG1CMU9MaUdVM01ERXo2eW9BdU5nWFhxbmtL?=
 =?utf-8?B?MEJPdGdhM211UzBDNGZxOTJvRXcxQ3hoTFQ3R1RLSHlNWmJ2ejduOG5CcG1C?=
 =?utf-8?B?bUloeGZYck4raTRveW1yR2ZzUDFIdGVsMWxzWG9BWG82eTFmZ0hHTFlZTFhp?=
 =?utf-8?B?SEtrRWZIdWZ5N1hvUlM3dC95MWp5WGNlVEo3cHJ6RFpLVjhnRytYZnV5bVh3?=
 =?utf-8?B?Skt2bzl2RWRCVndRcnkvcnJvTGNoUEFiS0x5YmRhSU9seUg1THBRNzNXWSsr?=
 =?utf-8?Q?QL3XBoOukOdh0van94GueXt/U?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 12d6034f-d4ea-428e-4255-08dc23289650
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5966.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Feb 2024 13:20:41.0426
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: mb1M0wGQXsHkYsE5n2ZorIMUsoGnT2CF8ykxOGtS6UH5+gJg6GaRKdPgkFZLsVSid8wak5T0GimRXy5aJ78QaA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB8173

On 1/29/2024 6:16 PM, Ilpo Järvinen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> + Cc Suma Hegde.
>
> On Sun, 28 Jan 2024, Greg Kroah-Hartman wrote:
>
>> The use of devm_*() functions works properly for when the device
>> structure itself is dynamic, but the hsmp driver is attempting to have a
>> local, static, struct device and then calls devm_() functions attaching
>> memory to the device that will never be freed.
>>
>> The logic of having a static struct device is almost never a wise
>> choice, but for now, just remove the use of devm_device_add_groups() in
>> this driver as it obviously is not needed.

Hi Greg,

Could you please hold on merging this patch for a week? I will push a 
patch for converting platform specific structure's memory allocation 
from static to a dynamic

allocation.

>>
>> Cc: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
>> Cc: Carlos Bilbao <carlos.bilbao@amd.com>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
>> Cc: platform-driver-x86@vger.kernel.org
>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> ---
>>   drivers/platform/x86/amd/hsmp.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
>> index b55d80e29139..49ef0b1d6df0 100644
>> --- a/drivers/platform/x86/amd/hsmp.c
>> +++ b/drivers/platform/x86/amd/hsmp.c
>> @@ -471,7 +471,7 @@ static int hsmp_create_sysfs_interface(void)
>>                if (ret)
>>                        return ret;
>>        }
>> -     return devm_device_add_groups(plat_dev.dev, hsmp_attr_grps);
>> +     return device_add_groups(plat_dev.dev, hsmp_attr_grps);
>>   }
> Thanks Greg for bringing this up. I've added Suma who has some patches
> which change code around this area.
Thanks Ilpo for adding me. I will push patch to address above issue on 
top of "Add ACPI probing support for HSMP" patch series.
> --
>   i.

Thanks and Regards,

Suma