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: <ff596664-1062-92ff-a1fe-3b644925aeae@intel.com>
Date:   Fri, 30 Jun 2023 11:39:22 +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 03/10] acpi/ac: 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-4-michal.wilczynski@intel.com>
 <CAJZ5v0ippMo1Haa-YFszyWZNgUE_pPUtkFngQWjUyjJe4tm94g@mail.gmail.com>
From:   "Wilczynski, Michal" <michal.wilczynski@intel.com>
In-Reply-To: <CAJZ5v0ippMo1Haa-YFszyWZNgUE_pPUtkFngQWjUyjJe4tm94g@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?bkZabWpPWGpqTkJIWWhsWk8xMmY0c0o5TUF3TnY1NnR4SjRxa3ZocUFnNnFr?=
 =?utf-8?B?UDUvR3dFeGVIWitFOFRvTThFeUxMSSs4MTBEUkRqS083bWh5Vzl4RTVQWGg4?=
 =?utf-8?B?U3dKdzdyZlN5RktKZFBURjJ3YUUzQXg1bVQwWURaeEo2Rmx4K1V0Z0pMSHR2?=
 =?utf-8?B?YW1FSzFYdXdHWEE0ejgxRWpNNTJWbXU2eFB1dlN0bWVGYzRrbWNzZTRFNlZE?=
 =?utf-8?B?b1ZTSnNncHpiQ0FjUWlhTi83WGhiaXVWcDg1QjRBZXFMclR5b1hJVXhndmYx?=
 =?utf-8?B?cFRmUGRGNVhrRDlFMmovWjB4d3RKSHprRmwzRWpDaUxHRGlMRSs3RXI5SHFG?=
 =?utf-8?B?a3owZDNDUDR6dW1Ka2VKWTVCQUNSa0ZLWldJOGthQ1pFT0wzUTF2ZWViSDJi?=
 =?utf-8?B?ZDMxczVzeVJPSitiRW5sUlRRS3ZuZnhheldMSmRoNXVReU0rcERtaTdmZjg2?=
 =?utf-8?B?b2lpVG85c21XY2syOUJlaWg5WXF5MWNmaXh3WEEwRVpVU1BwR0g4NFk5Yi9u?=
 =?utf-8?B?dGFGOFFmTGdGSGhwMktXS0lJbHlvdWRQVTRCb0JzR1dlWXFsNmJZR0syTEx6?=
 =?utf-8?B?Q2hHeHo4MEhzRGlseEVmdDVKQlFoOEt2KysrQmg0M0pjeGsrWkxndGducWJ4?=
 =?utf-8?B?VktiNWVuUU93cVR1VkxvQ1hhTWV5KzB0SElxcXpyQTNUeTE1SmZ0NXlyQVJ0?=
 =?utf-8?B?TURkeUhhWHFyd2trV092eXR0VmJ5Zmx4SExlOHQxaHFnSTc3VytET2tHNXRH?=
 =?utf-8?B?azBaVTdRZnZyaXdlQkkyd3VQc05LMW5Ldi9SRHVIZ3JMVllNd0lDNko2Szlt?=
 =?utf-8?B?TmxmZHI3bjRvYjhyRXBiNXgxWEdVdkhEUGtmT3VlQ3VGYzMrdnNHS3ZuV0tr?=
 =?utf-8?B?S3BkNkx0ekViTUZ5Zk4yWEx0Z0picXFkeS91VHR5WlBwVmlxci9oWG0rd1pn?=
 =?utf-8?B?Sys4Nm54QjNLMG9qWWhqd1lJWTVIYlFwOGVqbUZYZGdORk9YQzJDNkd5SEFo?=
 =?utf-8?B?K0g3VXZqUW00SXBJb0VNRThEZm1oZ2tDRzhIRUFTaFJIN2RYVTNSNW03aTBq?=
 =?utf-8?B?V3dQUlk4L3BDMkkrZGlBenRyQ3hmMlhwYUR4b2EvTkNuVWtlTEF4NTgzY1NP?=
 =?utf-8?B?SXBtVFo2RUxadUI5c0NwS1U0YUEzYUQ1QXdnN3ZVdStNMmRCMU5tdVA2bUE2?=
 =?utf-8?B?RkMxa2hGaHNCVlpaQ1pDMThPa3dEb3RiN0U5cU5OcHY4cHh4eXhsREZsMWxu?=
 =?utf-8?B?Y2cwSWpSektpanJiRTBRYSs0R2NyS2NmRzh6TDB2Y3ZibllmTmFELzFpU2Ro?=
 =?utf-8?B?YzAxKzI4WWdMQk9PNmZydmN1UFFnM21aWHp5V20rSnRwRnZ2L1hZU2NJK1Mz?=
 =?utf-8?B?cEs0ZEswYytQL2FUeWhOWFNwTVpwdXkvb0swd1NEV01tek1YV1BRdXNpYXo2?=
 =?utf-8?B?V0N1UXpTa0tURnEvelpHemVGckJtY2l1Y0p6RWlEOHhRWHlwdGhacmwvZGRW?=
 =?utf-8?B?ZnlaeHVCNXhCMzRRVzlGc2dMV01taVh3UE1iMDFJaUJ2VHIvaTBJVWFiZnBP?=
 =?utf-8?B?bXpKc2NYSEhoenRjc3ZjWXRmMlIwQmtYTlhBa2UrQVZ2bEgwRkZnbDdSa2hV?=
 =?utf-8?B?U1ZtZk81Q0NLNDM5b2dabi8vTlVoMTRoa05SL0xWY0UzVjhTaHEvbnFtVXlB?=
 =?utf-8?B?TUV4SGVqZGxLYjJIUVdFZW1MMlF4TS9kQWVYckFnRWdjRzZ6bjJIcXc4RCtP?=
 =?utf-8?B?b1hNY0x1bFhwUGwrcVpTRjJySnRqV3Mzd0JmYzJ5dzk3ODkrU0dicFp1VmZ1?=
 =?utf-8?B?ZjFldU54MDYwZDZ6TjBSbUk1WlcwRmRnZklkYWtOMkhmcHY1UmsyZmZld2tJ?=
 =?utf-8?B?dzM1Y1BXT1VSY3dJZ2VlM0lXZUM4OFdqaGVQaXdZTHpkSHJGWFdENzlkNWYw?=
 =?utf-8?B?bXdaSU80RExtbTFReEl2cytIUjJWMEN3b2Nzc2xacWIxbjNyeHhaS2w4TTlS?=
 =?utf-8?B?Um5xN3FqR1hhNDdhcC9sWFVmd3cwOHdKcE9JWXVnMDRuVlF3MnNJWVJ0cTBO?=
 =?utf-8?B?YzJ6T3BMd0k3MXhjYmpETFFrVU1MaFppcW0wZm5zM3c3NUtZbnowTmtPSzdM?=
 =?utf-8?B?clRzeHNoVGNNblZLTHdIRCtpTjRHWEdqK1k3bmtvaGdVUTlHcXhkUlpzME9X?=
 =?utf-8?B?OWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2f8322e0-e1fb-468b-2051-08db794de6c5
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:39:29.6729
 (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: mueMwFERzOFx8EEVAQadELm8EsPjCGtulUrVs68ZwV3S3EpvjubCf/xI6wXCFviS1MmVCw86bUUIO3BWJLhjOSYSLGcuvfxYFai+O2Wvdmg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6414
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 5:55 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/ac.c | 33 ++++++++++++++++++++++++---------
>>  1 file changed, 24 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
>> index 1ace70b831cd..207ee3c85bad 100644
>> --- a/drivers/acpi/ac.c
>> +++ b/drivers/acpi/ac.c
>> @@ -34,7 +34,7 @@ MODULE_LICENSE("GPL");
>>
>>  static int acpi_ac_add(struct acpi_device *device);
>>  static void acpi_ac_remove(struct acpi_device *device);
>> -static void acpi_ac_notify(struct acpi_device *device, u32 event);
>> +static void acpi_ac_notify(acpi_handle handle, u32 event, void *data);
>>
>>  static const struct acpi_device_id ac_device_ids[] = {
>>         {"ACPI0003", 0},
>> @@ -54,11 +54,9 @@ static struct acpi_driver acpi_ac_driver = {
>>         .name = "ac",
>>         .class = ACPI_AC_CLASS,
>>         .ids = ac_device_ids,
>> -       .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
>>         .ops = {
>>                 .add = acpi_ac_add,
>>                 .remove = acpi_ac_remove,
>> -               .notify = acpi_ac_notify,
>>                 },
>>         .drv.pm = &acpi_ac_pm,
>>  };
>> @@ -128,9 +126,12 @@ static enum power_supply_property ac_props[] = {
>>  };
>>
>>  /* Driver Model */
>> -static void acpi_ac_notify(struct acpi_device *device, u32 event)
>> +static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
>>  {
>> -       struct acpi_ac *ac = acpi_driver_data(device);
> This line doesn't need to be changed.  Just add the device variable
> definition above it.
>
> And the same pattern is present in the other patches in the series.

I like the Reverse Christmas Tree, but sure will change that

>
>> +       struct acpi_device *device = data;
>> +       struct acpi_ac *ac;
>> +
>> +       ac = acpi_driver_data(device);
>>
>>         if (!ac)
>>                 return;