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: <d4ebf8ba-6f95-d20c-d7fb-e97b6535f71f@intel.com>
Date:   Fri, 30 Jun 2023 11:55:01 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH v5 09/10] acpi/nfit: Move handler installing logic to
 driver
Content-Language: en-US
To:     "Rafael J. Wysocki" <rafael@kernel.org>
CC:     <linux-acpi@vger.kernel.org>, <dan.j.williams@intel.com>,
        <vishal.l.verma@intel.com>, <lenb@kernel.org>,
        <dave.jiang@intel.com>, <ira.weiny@intel.com>,
        <rui.zhang@intel.com>, <linux-kernel@vger.kernel.org>,
        <nvdimm@lists.linux.dev>,
        "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>
References: <20230616165034.3630141-1-michal.wilczynski@intel.com>
 <20230616165034.3630141-10-michal.wilczynski@intel.com>
 <CAJZ5v0gcokw72q5uX-3pbBEZtJdCaWHN1vat8yPNQ3SXMgeD4g@mail.gmail.com>
From:   "Wilczynski, Michal" <michal.wilczynski@intel.com>
In-Reply-To: <CAJZ5v0gcokw72q5uX-3pbBEZtJdCaWHN1vat8yPNQ3SXMgeD4g@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cFIvMWtQZ3hJTlc3bnRiWE1QOS92UU5ESnBMZmFQOHMxRXE0ZFQxZzJBbTAz?=
 =?utf-8?B?ZFZ0b1pJeW5lYlI0VXNuMXF2ckpRM1dxTXFzaFJSdndHNGdja1o1aGt1ZEJF?=
 =?utf-8?B?Q3BXdWhoQkJWTjQxVDEvdzRFNGlQSTdOL0lxYkFWZG5pZ1BjOWJSbkNrV21E?=
 =?utf-8?B?RGcvVWlHN0UyV2JhRThxUUZGQ1JVMHd4TWM3ZGJSeDBmM3p5dWd5bXdTZzZV?=
 =?utf-8?B?NDJReHhLVGVQcFUzbVN2S1kzazZUTmh6SEtPUHFiSlErUEc1T1p0NVQ3VVlr?=
 =?utf-8?B?WEdKWWNUdXpZSldNQUdPekRZbGQ3dENDQzBGVU9JUklrZFFFTXUvL3dJbkVP?=
 =?utf-8?B?Tk9MbUFnZjJFbWE0ekNEckw3V21GMjFLbTNZVGs0N0llL3JDRHFOTDJpVWZO?=
 =?utf-8?B?SEZIMmZtNnBGcjJGcHNCTC90emJaNG1PclBSL3U0OFBPTExRK3YyeFlNNkRw?=
 =?utf-8?B?NGZXa2lFSmpub0VNN3R0VTl1VjhKN3gzeFZ3NjhrNVNFK3ZRTTAyYVkxNFl2?=
 =?utf-8?B?c2xVVjhrVGlIcGUrdDBYSnY1R1RIc2xQamxDMFpBK01RTVdpMVdmbnU4L0Vq?=
 =?utf-8?B?RkY0UW5HUHlSeHl0Wk05bDFYUGdpeE5PYUdYT2hyRnlyYjQ3dkZ5blN1M2dQ?=
 =?utf-8?B?V3hzeUI1em1OYmdKNTgyVXNaV2NhVXN4QkpTYllkZHBaelZvQ2ZIenIxT1Z0?=
 =?utf-8?B?K2I5ZVFhTi8rZmkzVUxsNTc0bThCakRWcFZjUFI4NUl3UjFucDE0dmV4TjlE?=
 =?utf-8?B?Tks4Z3JRRStTdTgxblhDNEMrQlFxbm9aVE9IencxbVVKNC9qMS9oUW9OKzZR?=
 =?utf-8?B?MmdMT0xxYnJzU0lvemx0Z2RFcnhacTUvbyswRDNXZFpyVWJZZnpleitsdDlh?=
 =?utf-8?B?Q3FZUi9qOHRBaHRNRkhmYy9WWVVQQ296U1phY3R2VWZ1QVFPWldUTjJOU0pK?=
 =?utf-8?B?eXNTVGdBYVZBdkZnWThENm5heUJtR29UVlBMemhNTmpRRTN1emdCWk53S0hC?=
 =?utf-8?B?a2V3ajd6RGJYeVZKV2hYdU5xaDJ2SGs4d2RDcXV2anVDZGhoNW11bnRKVTRm?=
 =?utf-8?B?aTFteU5VZmNROVVncy8xQW9TdGhNY1hIOFJyUTczSDlJS0piMVpZamc0UDVF?=
 =?utf-8?B?eHUrd1p2RHNxbjdSZzBkcHdIc1pQbmQ5cFcxaU0wZmQ5bVI1UU55Qm9KYlha?=
 =?utf-8?B?cXBDbVdYeTNSU3RLWVZaeUFoVjFHRkhWdEtuWHppcm9kT3A4b3ZzMTNlYktQ?=
 =?utf-8?B?R3VnNEttQzdoQzUxRUQvMll4bXcyT0lxNm5MeUxFd2lpTFpGOVJpR1l1VTRE?=
 =?utf-8?B?TS9DQUxXamtrbDgvV1hGbVVIYnVwZVBOT0F2NisxSWRMVjNrTTVOWEFWWWxm?=
 =?utf-8?B?QkpRRks2ZDBhdjZMTFl6U2NtTC9kRTdIamJJTlJmVG94bURzaG5qZ3dKNk5H?=
 =?utf-8?B?empKRnk1M2lFeDA4ZmR6aE1iV3FGekx6M1hpMExlSHp6ekJMd2xBS3RiV0dw?=
 =?utf-8?B?WW1iMUhDSnp4dzhQWEJmbDBhNVdBYldzQlduTVhzbFI2aFhWNW1XcWdNNTZY?=
 =?utf-8?B?dTQvMytiRURLQXVpS3ZLSkVmKzRJWnhZNTE1dUxOWi9QTEVhWHpuZ3Q4T2dy?=
 =?utf-8?B?MWl6VmVPYW9XeUhTcjRDL3B5WHArQ2JlMWhJd1AvYm1MZk1pWHlXMGdOZTdD?=
 =?utf-8?B?bUhTTjFsOVFMRnpIdFRQQ2liUGxQd3RBcHhpcnRKcnNIaUdEb2tvV2FvV1BL?=
 =?utf-8?B?cUU3dnhCZTNiaUNmOFJ0bHM4RlVRSmEvNU9lVTVBcGppSGJmVjN2clFmYzRw?=
 =?utf-8?B?M2tkSUl6UlFTb200eE5MYjdkd1FUMFBVd1JLQ2RjeTRVZlJaUWFsT2c4dnN5?=
 =?utf-8?B?d1JSZ0F3RnRwdXArVjNiRS9iay9yNi9jRWhyOEVhZkdGM2U4RXNzOWFZZ3N4?=
 =?utf-8?B?UTh5ZWdEV0F4eGdIMExTS1hqNGlLd2UwL0lQMWFOZG1XQ2xhY3B5ZGlpME55?=
 =?utf-8?B?UlJqZmFtWDZOUG9WM09yZGVQR2hONW5aQ2FoL0JYeVZKUFJ3ZnNIaFFxUTRx?=
 =?utf-8?B?UEJZVVdQSTZTRithQjdQbDhJVWcvalF5a001ZXJvaWRtTEpCem4vRlhheklE?=
 =?utf-8?B?d0lVVURyRjMvdkJXT2dzUVhlZWpFN1dhemVpVFBlcWIyZ0FDeURzNFAvT2Rx?=
 =?utf-8?B?bEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 29f2a3bf-ac1a-43bd-57b8-08db795016c6
X-MS-Exchange-CrossTenant-AuthSource: CO6PR11MB5603.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jun 2023 09:55:09.1727
 (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: ffg3bojvhTDTInUU0QUNWJL8v0ZIlogR+Ml6dx8fGbuJ7RLTDxHS0BAJCqPwOfdjCTrYCiuIfbb7jYgN7/2y3TgXMX40ET4Fw/1tYdCvchs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB7711
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED 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 6/29/2023 6:18 PM, Rafael J. Wysocki wrote:
> On Fri, Jun 16, 2023 at 6:51 PM Michal Wilczynski
> <michal.wilczynski@intel.com> wrote:
>> Currently logic for installing notifications from ACPI devices is
>> implemented using notify callback in struct acpi_driver. Preparations
>> are being made to replace acpi_driver with more generic struct
>> platform_driver, which doesn't contain notify callback. Furthermore
>> as of now handlers are being called indirectly through
>> acpi_notify_device(), which decreases performance.
>>
>> Call acpi_dev_install_notify_handler() at the end of .add() callback.
>> Call acpi_dev_remove_notify_handler() at the beginning of .remove()
>> callback. Change arguments passed to the notify function to match with
>> what's required by acpi_install_notify_handler(). Remove .notify
>> callback initialization in acpi_driver.
>>
>> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
>> ---
>>  drivers/acpi/nfit/core.c | 24 ++++++++++++++++++------
>>  1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
>> index 95930e9d776c..a281bdfee8a0 100644
>> --- a/drivers/acpi/nfit/core.c
>> +++ b/drivers/acpi/nfit/core.c
>> @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data)
>>  }
>>  EXPORT_SYMBOL_GPL(acpi_nfit_shutdown);
>>
>> -static void acpi_nfit_notify(struct acpi_device *adev, u32 event)
>> +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
>>  {
>> -       device_lock(&adev->dev);
>> -       __acpi_nfit_notify(&adev->dev, adev->handle, event);
>> -       device_unlock(&adev->dev);
> It's totally not necessary to rename the ACPI device variable here.
>
> Just add
>
> struct acpi_device *adev = data;
>
> to this function.

Sure, is adev a preferred name for acpi_device ? I've seen a mix of different naming
in drivers, some use device, adev, acpi_dev and so on. I suppose it's not a big deal, but
it would be good to know.

>
>> +       struct acpi_device *device = data;
>> +
>> +       device_lock(&device->dev);
>> +       __acpi_nfit_notify(&device->dev, handle, event);
>> +       device_unlock(&device->dev);
>>  }
>>
>>  static int acpi_nfit_add(struct acpi_device *adev)
>> @@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev)
>>
>>         if (rc)
>>                 return rc;
>> -       return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
>> +
>> +       rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
>> +       if (rc)
>> +               return rc;
>> +
>> +       return acpi_dev_install_notify_handler(adev,
>> +                                              ACPI_DEVICE_NOTIFY,
>> +                                              acpi_nfit_notify);
>>  }
>>
>>  static void acpi_nfit_remove(struct acpi_device *adev)
>>  {
>>         /* see acpi_nfit_unregister */
>> +
>> +       acpi_dev_remove_notify_handler(adev,
>> +                                      ACPI_DEVICE_NOTIFY,
>> +                                      acpi_nfit_notify);
>>  }
>>
>>  static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
>> @@ -3465,7 +3478,6 @@ static struct acpi_driver acpi_nfit_driver = {
>>         .ops = {
>>                 .add = acpi_nfit_add,
>>                 .remove = acpi_nfit_remove,
>> -               .notify = acpi_nfit_notify,
>>         },
>>  };
>>
>> --