Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Message-ID: <ec5029b0-553c-4a6c-b2a9-ef9943e553dc@intel.com>
Date:   Mon, 2 Oct 2023 22:27:02 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use
 acpi_evaluate_dsm_typed()
Content-Language: en-US
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        <nvdimm@lists.linux.dev>, <linux-acpi@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
CC:     Dan Williams <dan.j.williams@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        Ira Weiny <ira.weiny@intel.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Len Brown <lenb@kernel.org>
References: <20231002135458.2603293-1-andriy.shevchenko@linux.intel.com>
From:   "Wilczynski, Michal" <michal.wilczynski@intel.com>
In-Reply-To: <20231002135458.2603293-1-andriy.shevchenko@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QVBYUlFrKzYrS2Nic253QU9xQ2s4K2Z0U2VSaEhQYTRsVjBoWllYQldGZ3hj?=
 =?utf-8?B?eVl3aHMvU2FyU29hdk9mcFhuNVlwUDZOcHVFdHVlNUtRNEpTb0JRWUFXaFBF?=
 =?utf-8?B?MEdZMlV3eVRHQ0NkWXVsUWZhbXFIYkNmMTZ4OUthQ1AwWHFYN1M3cU12empS?=
 =?utf-8?B?bjdWeTF4RFllMk1tMS9IZWt6cGxJZTcxMWkxWmdITUh2U3NOMlM3UVVqTk9F?=
 =?utf-8?B?YTdLSzRKcy9JeEptV1FSUVB5cUhMQ1VYUFViZmlpdzRac1JRbFF6WFZNa3B6?=
 =?utf-8?B?NEs0MTh0Wm5sUlh0Y3h0dW5zeTFGcmtYTHMxZnJNL3dSMjhRVzdKb3RWNS9q?=
 =?utf-8?B?Rk91R29IdUFrMHc1eGdwSEEwd0dTUjRpQjRWVGNobHRsdlBybVBzb1VIVXc1?=
 =?utf-8?B?YzFzaVJSVTVpQ2NSWnBzTFpCK0JUL1dCVGRLZjNaZUxNSnlyYUlNcTZaeEpH?=
 =?utf-8?B?VlcySlUyNGpJWnkrUEFlQkk1S0xPcStYTjEwc2xJYWZrcUpnUFI5Y0cxcDJN?=
 =?utf-8?B?alNzUHFVRTFsaWMrK0FaVktqR3AyQ3I0TG9NMXBuQjk1N04rM09DMkZFT01T?=
 =?utf-8?B?N1pLdG5tSThHTUFNS0ovb2IyVk9zUTdkV3o4T3ZBeXdZRnJjMlpxT1FwamVu?=
 =?utf-8?B?WUVkVHpzN0VmWWNGMEVJMXA2ZkVnU2JjNG1WRVVoWXpCRTRYaVloNnczTzRr?=
 =?utf-8?B?S1FEMUM0dHEyVjRaQjZrRTJ1Nms4KzlvSzlFbGtPMmNCcTd4SDlFdUFQektK?=
 =?utf-8?B?UFpYVDFOOUptTDU3eWFERFpoOVRJa2M0M2NPblhSM1VOSkRhZTUvTGhBUlVv?=
 =?utf-8?B?akd2UGh3YmlGVC9XRG5qMy9OQWR5clp2b1VLQ29nR2hDRzFKeVVIZzEvTUEv?=
 =?utf-8?B?WHNnaW5MbDVNRW5lR0JzSXRtTnpoQVZYek1uNjdPRDYyYXhwR1Q0T2VENnpX?=
 =?utf-8?B?RlZPT1pDcmh4V1ZoaWQyNFJZUitFRnNUcVdXbkhOblpTUUNCay9aODVWdm80?=
 =?utf-8?B?YzVGelhwMS9MMWZmT2ZpcndudC9hVEpaa0JRNDhvd3MvLzF1b0ZXUmp3TWE5?=
 =?utf-8?B?eStVbGJ3ak9jVHkyd0tDSzZ0b1dLZ0R3eENoY3pUNXYzbFVNUlFBdzZjYTZi?=
 =?utf-8?B?ZitDNUdUbE9xOGF3SEd0WnpPN0MrUVZ4ZmRLYlZDMUdJLzVGRFpKQ1Z1UXlZ?=
 =?utf-8?B?Q2ZraVRkdUtYODc2eTFjc1JUVDk3M2hVNmI2UDdXSG93OVQ0bEV0VjRhcHdS?=
 =?utf-8?B?RzRQRTQ0dS9BbDJkT1pkc2pmMHZ2UVJUbitVeEhKdHNBWFIrekdnY0gyWHFI?=
 =?utf-8?B?MDR1emJLNTVUQURPalp1TGZJOVN4RmRlbFNxT1FkTEJaQTVRSGxOSHV0dkRQ?=
 =?utf-8?B?c28xY3VvUHhlakRUTTRoVUkvS0x4elRoTEI2ZXQwZnM3bzZrU1Vkakl4L01S?=
 =?utf-8?B?bW9VV0FNSkJvSVg4TE1USWJHYlVPR3F2dGdGTVNiTUF6T005bWhWbG5EWnV4?=
 =?utf-8?B?UWFKejFZYU9JajltK28rWWIyVUJWeDcxTnNBak44eGp5R1UzSDVxVXhrRzFx?=
 =?utf-8?B?UzJTVE5tZnU0cXBXaHM4M1pPVTBwTWl3SVpxRURKU0V4bjBaQlJuZGczdHZw?=
 =?utf-8?B?UE02NzNESXBGMXJtOTFoL0hHN2VvSTNQTW1BczJqOHBOcWZ2N2RSS2orOXlI?=
 =?utf-8?B?SGphbzNqTUxIM1VwWHFMZmh4Wm4ybWtEeWJwV1NBcVk1dG1WOHhWTmJGaFFL?=
 =?utf-8?B?VHEvR2FyOG40b0lFMTVaYnFDeE5SZUdsV2JBcFVFU2Q2QzJaVnBZSmlxOTlC?=
 =?utf-8?B?enJpOHJZSkVneUxyQmVDZGlIMHMyWW5DcStNVXA1THJRSVpxR1hFK05ITFhG?=
 =?utf-8?B?Y0gwZEZ2RGE3Ym01VUt3Z1FURlFpVTJEMklSSk0xOCtOWW1nTnJ0bHlLc1Er?=
 =?utf-8?B?V1Bob0JlZmRQMCtrdHVFT2pnV3JXcUc1dVdlMk12cFplcUhGTDFwMWFXMjVN?=
 =?utf-8?B?Z0F0Um55SGJkcERaQXFVS2VuVVQxelhRM241RW5OaUt2TkhpTU5DcXp4NGV1?=
 =?utf-8?B?QTFHQXZYOW93eHNKQVhQUXpLRk10Y1FJZkdLNzRTRHhwSW1Oak0xb0w3Unla?=
 =?utf-8?B?TTQ1djhpWmNicVJJbjQ0SHhGTnpKNC9vbUg4R2o5ZDBmeXBXUUtpd0xjVjN0?=
 =?utf-8?B?cGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 1e8237e9-e605-45ce-f788-08dbc385f432
X-MS-Exchange-CrossTenant-AuthSource: CO6PR11MB5603.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Oct 2023 20:27:10.1021
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: tIpVapP7ft/h04x3ydNsrnQPEdpJO5DApFePnYIvB438YrvmJ+2HInXfSRSnPJOS4E/iBmDRccV9G+HBQp7R9inArRxobmHil2h6znJbdC4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8433
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Mon, 02 Oct 2023 13:27:55 -0700 (PDT)


On 10/2/2023 3:54 PM, Andy Shevchenko wrote:
> The acpi_evaluate_dsm_typed() provides a way to check the type of the
> object evaluated by _DSM call. Use it instead of open coded variant.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/acpi/nfit/core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index f96bf32cd368..280da408c02c 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1737,9 +1737,8 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem)
>  	if ((nfit_mem->dsm_mask & (1 << func)) == 0)
>  		return;
>  
> -	out_obj = acpi_evaluate_dsm(handle, guid, revid, func, &in_obj);
> -	if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER
> -			|| out_obj->buffer.length < sizeof(smart)) {
> +	out_obj = acpi_evaluate_dsm_typed(handle, guid, revid, func, &in_obj, ACPI_TYPE_BUFFER);

This line is 90 characters long, wouldn't it be better to split it ?

> +	if (!out_obj || out_obj->buffer.length < sizeof(smart)) {
>  		dev_dbg(dev->parent, "%s: failed to retrieve initial health\n",
>  				dev_name(dev));

While at it maybe fix alignment ? :-)

>  		ACPI_FREE(out_obj);

Just nitpicks, functionally code seems correct to me.
Reviewed-by: Michal Wilczynski <michal.wilczynski@intel.com>