| Message ID | 20240106022532.1746932-10-suma.hegde@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v5,01/11] platform/x86/amd/hsmp: Move hsmp_test to probe | expand |
On Sat, 6 Jan 2024, Suma Hegde wrote: > Use the standard array allocation variant of devm memory allocation > APIs. > > Signed-off-by: Suma Hegde <suma.hegde@amd.com> > Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> > --- > Changes since v4: > 1. Change sizeof(struct bin_attribute *) to sizeof(*hsmp_bin_attrs) > 2. Change sizeof(struct attribute_group *) to sizeof(*hsmp_attr_grps) > 3. Split some of the changes to 11th patch in this v5 series > > Changes since v3: > New patch, based on Ilpos review comments and additional cosmetic changes. > drivers/platform/x86/amd/hsmp.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c > index 99bebb0ca5a9..ccf7cd8f98f6 100644 > --- a/drivers/platform/x86/amd/hsmp.c > +++ b/drivers/platform/x86/amd/hsmp.c > @@ -657,8 +657,9 @@ static int hsmp_create_attr_list(struct attribute_group *attr_grp, > struct bin_attribute **hsmp_bin_attrs; > > /* Null terminated list of attributes */ > - hsmp_bin_attrs = devm_kzalloc(dev, sizeof(struct bin_attribute *) * > - (NUM_HSMP_ATTRS + 1), GFP_KERNEL); > + hsmp_bin_attrs = devm_kcalloc(dev, NUM_HSMP_ATTRS + 1, > + sizeof(*hsmp_bin_attrs), > + GFP_KERNEL); > if (!hsmp_bin_attrs) > return -ENOMEM; > > @@ -673,8 +674,9 @@ static int hsmp_create_non_acpi_sysfs_if(struct device *dev) > struct attribute_group *attr_grp; > u16 i; > > - hsmp_attr_grps = devm_kzalloc(dev, sizeof(struct attribute_group *) * > - (plat_dev.num_sockets + 1), GFP_KERNEL); > + hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1, > + sizeof(*hsmp_attr_grps), > + GFP_KERNEL); > if (!hsmp_attr_grps) > return -ENOMEM; > > @@ -804,8 +806,8 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev) > * on each probe. > */ > if (!plat_dev.is_probed) { > - plat_dev.sock = devm_kzalloc(&pdev->dev, > - (plat_dev.num_sockets * sizeof(struct hsmp_socket)), > + plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets, > + sizeof(struct hsmp_socket), I wonder why the sizeof() of the target isn't used in this case like in the other cases?
On Sat, 6 Jan 2024, Suma Hegde wrote: > Use the standard array allocation variant of devm memory allocation > APIs. > > Signed-off-by: Suma Hegde <suma.hegde@amd.com> > Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> I decided to apply all but this patch 10/11 into review-ilpo. I fixed the parenthesis issue I mentioned in one of the patches while applying. Please check the comment I made against this patch and respin this one. I also noticed while applying one other extra parenthesis case in patch 5 but since it was not added, I didn't go to tweak it now myself, but just you know.
On Thu, 25 Jan 2024, Ilpo Järvinen wrote: > On Sat, 6 Jan 2024, Suma Hegde wrote: > > > Use the standard array allocation variant of devm memory allocation > > APIs. > > > > Signed-off-by: Suma Hegde <suma.hegde@amd.com> > > Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> > > I decided to apply all but this patch 10/11 into review-ilpo. I fixed the > parenthesis issue I mentioned in one of the patches while applying. > > Please check the comment I made against this patch and respin this one. > > I also noticed while applying one other extra parenthesis case in patch 5 > but since it was not added, I didn't go to tweak it now myself, but just > you know. Hi Suma, There are number of issues and warnings due to these patches including one build failure due to lack of ACPI in the config (I think), can you please take a look at them.
On 1/29/2024 6:14 PM, Ilpo Järvinen wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On Thu, 25 Jan 2024, Ilpo Järvinen wrote: > >> On Sat, 6 Jan 2024, Suma Hegde wrote: >> >>> Use the standard array allocation variant of devm memory allocation >>> APIs. >>> >>> Signed-off-by: Suma Hegde <suma.hegde@amd.com> >>> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> >> I decided to apply all but this patch 10/11 into review-ilpo. I fixed the >> parenthesis issue I mentioned in one of the patches while applying. >> >> Please check the comment I made against this patch and respin this one. >> >> I also noticed while applying one other extra parenthesis case in patch 5 >> but since it was not added, I didn't go to tweak it now myself, but just >> you know. > Hi Suma, > > There are number of issues and warnings due to these patches including > one build failure due to lack of ACPI in the config (I think), can you > please take a look at them. Hi Ilpo, I have pushed patch with fixes for smatch error and warnings. For the CONFIG_ACPI=n build failure, I have added "depends on ACPI" for hsmp driver and pushed patch for that. But we support NON-ACPI probing also, there may be x86 platforms with ACPI disabled, is there a previous reference of how this can be handled without making it dependent on ACPI in Kconfig? Thanks and Regards, Suma > -- > i.
On Mon, 29 Jan 2024, Hegde, Suma wrote: > > On 1/29/2024 6:14 PM, Ilpo Järvinen wrote: > > Caution: This message originated from an External Source. Use proper caution > > when opening attachments, clicking links, or responding. > > > > > > On Thu, 25 Jan 2024, Ilpo Järvinen wrote: > > > > > On Sat, 6 Jan 2024, Suma Hegde wrote: > > > > > > > Use the standard array allocation variant of devm memory allocation > > > > APIs. > > > > > > > > Signed-off-by: Suma Hegde <suma.hegde@amd.com> > > > > Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> > > > I decided to apply all but this patch 10/11 into review-ilpo. I fixed the > > > parenthesis issue I mentioned in one of the patches while applying. > > > > > > Please check the comment I made against this patch and respin this one. > > > > > > I also noticed while applying one other extra parenthesis case in patch 5 > > > but since it was not added, I didn't go to tweak it now myself, but just > > > you know. > > Hi Suma, > > > > There are number of issues and warnings due to these patches including > > one build failure due to lack of ACPI in the config (I think), can you > > please take a look at them. > > Hi Ilpo, > > I have pushed patch with fixes for smatch error and warnings. > > For the CONFIG_ACPI=n build failure, I have added "depends on ACPI" for hsmp > driver and pushed patch for that. Hi, I've folded your fixes into the relevant patches now. > But we support NON-ACPI probing also, there may be x86 platforms with ACPI > disabled, is there a previous reference of how this can be handled > > without making it dependent on ACPI in Kconfig? Given you have quite much code that relates to ACPI case, perhaps creating hsmp-acpi.c wouldn't be a bad idea so you can make that file depend on ACPI without polluting the hsmp.c code with #ifdefs.
On 1/31/2024 4:02 PM, Ilpo Järvinen wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On Mon, 29 Jan 2024, Hegde, Suma wrote: > >> On 1/29/2024 6:14 PM, Ilpo Järvinen wrote: >>> Caution: This message originated from an External Source. Use proper caution >>> when opening attachments, clicking links, or responding. >>> >>> >>> On Thu, 25 Jan 2024, Ilpo Järvinen wrote: >>> >>>> On Sat, 6 Jan 2024, Suma Hegde wrote: >>>> >>>>> Use the standard array allocation variant of devm memory allocation >>>>> APIs. >>>>> >>>>> Signed-off-by: Suma Hegde <suma.hegde@amd.com> >>>>> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> >>>> I decided to apply all but this patch 10/11 into review-ilpo. I fixed the >>>> parenthesis issue I mentioned in one of the patches while applying. >>>> >>>> Please check the comment I made against this patch and respin this one. >>>> >>>> I also noticed while applying one other extra parenthesis case in patch 5 >>>> but since it was not added, I didn't go to tweak it now myself, but just >>>> you know. >>> Hi Suma, >>> >>> There are number of issues and warnings due to these patches including >>> one build failure due to lack of ACPI in the config (I think), can you >>> please take a look at them. >> Hi Ilpo, >> >> I have pushed patch with fixes for smatch error and warnings. >> >> For the CONFIG_ACPI=n build failure, I have added "depends on ACPI" for hsmp >> driver and pushed patch for that. > Hi, > > I've folded your fixes into the relevant patches now. > >> But we support NON-ACPI probing also, there may be x86 platforms with ACPI >> disabled, is there a previous reference of how this can be handled >> >> without making it dependent on ACPI in Kconfig? > Given you have quite much code that relates to ACPI case, perhaps > creating hsmp-acpi.c wouldn't be a bad idea so you can make that file > depend on ACPI without polluting the hsmp.c code with #ifdefs. Thanks Ilpo for the suggestion. I will address Han's and Greg's comments and later will work on splitting the ACPI code into separate file. Regards, Suma > -- > i.
On Tue, 6 Feb 2024, Hegde, Suma wrote: > > On 1/31/2024 4:02 PM, Ilpo Järvinen wrote: > > Caution: This message originated from an External Source. Use proper caution > > when opening attachments, clicking links, or responding. > > > > > > On Mon, 29 Jan 2024, Hegde, Suma wrote: > > > > > On 1/29/2024 6:14 PM, Ilpo Järvinen wrote: > > > > Caution: This message originated from an External Source. Use proper > > > > caution > > > > when opening attachments, clicking links, or responding. > > > > > > > > > > > > On Thu, 25 Jan 2024, Ilpo Järvinen wrote: > > > > > > > > > On Sat, 6 Jan 2024, Suma Hegde wrote: > > > > > > > > > > > Use the standard array allocation variant of devm memory allocation > > > > > > APIs. > > > > > > > > > > > > Signed-off-by: Suma Hegde <suma.hegde@amd.com> > > > > > > Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> > > > > > I decided to apply all but this patch 10/11 into review-ilpo. I fixed > > > > > the > > > > > parenthesis issue I mentioned in one of the patches while applying. > > > > > > > > > > Please check the comment I made against this patch and respin this > > > > > one. > > > > > > > > > > I also noticed while applying one other extra parenthesis case in > > > > > patch 5 > > > > > but since it was not added, I didn't go to tweak it now myself, but > > > > > just > > > > > you know. > > > > Hi Suma, > > > > > > > > There are number of issues and warnings due to these patches including > > > > one build failure due to lack of ACPI in the config (I think), can you > > > > please take a look at them. > > > Hi Ilpo, > > > > > > I have pushed patch with fixes for smatch error and warnings. > > > > > > For the CONFIG_ACPI=n build failure, I have added "depends on ACPI" for > > > hsmp > > > driver and pushed patch for that. > > Hi, > > > > I've folded your fixes into the relevant patches now. > > > > > But we support NON-ACPI probing also, there may be x86 platforms with ACPI > > > disabled, is there a previous reference of how this can be handled > > > > > > without making it dependent on ACPI in Kconfig? > > Given you have quite much code that relates to ACPI case, perhaps > > creating hsmp-acpi.c wouldn't be a bad idea so you can make that file > > depend on ACPI without polluting the hsmp.c code with #ifdefs. > > Thanks Ilpo for the suggestion. I will address Han's and Greg's comments and > later will work on splitting the ACPI code into separate file. Okay, thanks. Mario also raised concerns besides those from Hans and Greg so could you also take a look at them as well.
On 2/6/2024 2:34 PM, Ilpo Järvinen wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On Tue, 6 Feb 2024, Hegde, Suma wrote: > >> On 1/31/2024 4:02 PM, Ilpo Järvinen wrote: >>> Caution: This message originated from an External Source. Use proper caution >>> when opening attachments, clicking links, or responding. >>> >>> >>> On Mon, 29 Jan 2024, Hegde, Suma wrote: >>> >>>> On 1/29/2024 6:14 PM, Ilpo Järvinen wrote: >>>>> Caution: This message originated from an External Source. Use proper >>>>> caution >>>>> when opening attachments, clicking links, or responding. >>>>> >>>>> >>>>> On Thu, 25 Jan 2024, Ilpo Järvinen wrote: >>>>> >>>>>> On Sat, 6 Jan 2024, Suma Hegde wrote: >>>>>> >>>>>>> Use the standard array allocation variant of devm memory allocation >>>>>>> APIs. >>>>>>> >>>>>>> Signed-off-by: Suma Hegde <suma.hegde@amd.com> >>>>>>> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> >>>>>> I decided to apply all but this patch 10/11 into review-ilpo. I fixed >>>>>> the >>>>>> parenthesis issue I mentioned in one of the patches while applying. >>>>>> >>>>>> Please check the comment I made against this patch and respin this >>>>>> one. >>>>>> >>>>>> I also noticed while applying one other extra parenthesis case in >>>>>> patch 5 >>>>>> but since it was not added, I didn't go to tweak it now myself, but >>>>>> just >>>>>> you know. >>>>> Hi Suma, >>>>> >>>>> There are number of issues and warnings due to these patches including >>>>> one build failure due to lack of ACPI in the config (I think), can you >>>>> please take a look at them. >>>> Hi Ilpo, >>>> >>>> I have pushed patch with fixes for smatch error and warnings. >>>> >>>> For the CONFIG_ACPI=n build failure, I have added "depends on ACPI" for >>>> hsmp >>>> driver and pushed patch for that. >>> Hi, >>> >>> I've folded your fixes into the relevant patches now. >>> >>>> But we support NON-ACPI probing also, there may be x86 platforms with ACPI >>>> disabled, is there a previous reference of how this can be handled >>>> >>>> without making it dependent on ACPI in Kconfig? >>> Given you have quite much code that relates to ACPI case, perhaps >>> creating hsmp-acpi.c wouldn't be a bad idea so you can make that file >>> depend on ACPI without polluting the hsmp.c code with #ifdefs. >> Thanks Ilpo for the suggestion. I will address Han's and Greg's comments and >> later will work on splitting the ACPI code into separate file. > Okay, thanks. > > Mario also raised concerns besides those from Hans and Greg so could you > also take a look at them as well. Yes Ilpo, we are working on addressing Mario's comments as well. Thanks and Regards, Suma > > -- > i.
diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c index 99bebb0ca5a9..ccf7cd8f98f6 100644 --- a/drivers/platform/x86/amd/hsmp.c +++ b/drivers/platform/x86/amd/hsmp.c @@ -657,8 +657,9 @@ static int hsmp_create_attr_list(struct attribute_group *attr_grp, struct bin_attribute **hsmp_bin_attrs; /* Null terminated list of attributes */ - hsmp_bin_attrs = devm_kzalloc(dev, sizeof(struct bin_attribute *) * - (NUM_HSMP_ATTRS + 1), GFP_KERNEL); + hsmp_bin_attrs = devm_kcalloc(dev, NUM_HSMP_ATTRS + 1, + sizeof(*hsmp_bin_attrs), + GFP_KERNEL); if (!hsmp_bin_attrs) return -ENOMEM; @@ -673,8 +674,9 @@ static int hsmp_create_non_acpi_sysfs_if(struct device *dev) struct attribute_group *attr_grp; u16 i; - hsmp_attr_grps = devm_kzalloc(dev, sizeof(struct attribute_group *) * - (plat_dev.num_sockets + 1), GFP_KERNEL); + hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1, + sizeof(*hsmp_attr_grps), + GFP_KERNEL); if (!hsmp_attr_grps) return -ENOMEM; @@ -804,8 +806,8 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev) * on each probe. */ if (!plat_dev.is_probed) { - plat_dev.sock = devm_kzalloc(&pdev->dev, - (plat_dev.num_sockets * sizeof(struct hsmp_socket)), + plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets, + sizeof(struct hsmp_socket), GFP_KERNEL); if (!plat_dev.sock) return -ENOMEM;