From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5BD4FC433EF for ; Tue, 15 Mar 2022 01:01:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241219AbiCOBCT (ORCPT ); Mon, 14 Mar 2022 21:02:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37284 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233143AbiCOBCS (ORCPT ); Mon, 14 Mar 2022 21:02:18 -0400 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6FE4931911; Mon, 14 Mar 2022 18:01:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1647306067; x=1678842067; h=message-id:subject:from:reply-to:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=NWusIOIo+vyDfRBYbXa+JW6qvU98GxBj6Z5YtR9TtG0=; b=NlJjVTYHnBuGLse/ZPfYj2Zd799XgktYx4q4hv9lg7lW6k3PG5nZLqVG eXcU/UGAGoeonXZtDxhAqkdRwNADS3mOA22J5amppe4SS6RwaKhI84MGO SqVqyPdxJYV5lC541hpa4tQSDkOrf6sr3AcYtmnwndycC5/JGdWkdWxie /Or2zUhmIkoDpAKmQFSi4KrB6Mb3kzhbc4/QURqYA02wktseBmAUc+I7d 98z/ysbVn0WouiBdwHs5YxYW4nGdCZHRLXe+Xmr0PgMlBcPH1VJONU9NK OnTT9ohC/95b3+QO+hmN9/7pns5OQ0FJs79ZwNGBFcMx/XzqH0MTWFMAo w==; X-IronPort-AV: E=McAfee;i="6200,9189,10286"; a="280949601" X-IronPort-AV: E=Sophos;i="5.90,181,1643702400"; d="scan'208";a="280949601" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Mar 2022 18:01:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.90,181,1643702400"; d="scan'208";a="634413805" Received: from linux.intel.com ([10.54.29.200]) by FMSMGA003.fm.intel.com with ESMTP; 14 Mar 2022 18:01:06 -0700 Received: from zehall-mobl.amr.corp.intel.com (unknown [10.209.53.245]) by linux.intel.com (Postfix) with ESMTP id 9E357580690; Mon, 14 Mar 2022 18:01:06 -0700 (PDT) Message-ID: <4121875ff3ee2999e50c687ed9f808d1dc98a68f.camel@linux.intel.com> Subject: Re: [PATCH v3 1/5] ACPI / x86: Add support for LPS0 callback handler From: "David E. Box" Reply-To: david.e.box@linux.intel.com To: "Limonciello, Mario" , Hans de Goede , Mark Gross , "Rafael J . Wysocki" Cc: "open list:X86 PLATFORM DRIVERS" , "linux-acpi@vger.kernel.org" , "S-k, Shyam-sundar" , "Goswami, Sanket" Date: Mon, 14 Mar 2022 18:01:06 -0700 In-Reply-To: References: <20220314050340.1176-1-mario.limonciello@amd.com> <0101142d-3ea6-a47b-be26-76aaaac46b0a@redhat.com> Organization: David E. Box Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Mon, 2022-03-14 at 13:32 +0000, Limonciello, Mario wrote: > [Public] > > > > +int acpi_register_lps0_callbacks(struct lps0_callback_handler *arg) > > > +{ > > > + struct lps0_callback_handler *handler; > > > + > > > + if (!lps0_device_handle || sleep_no_lps0) > > > + return -ENODEV; > > > + > > > + handler = kmalloc(sizeof(*handler), GFP_KERNEL); > > > + if (!handler) > > > + return -ENOMEM; > > > + handler->prepare_late_callback = arg->prepare_late_callback; > > > + handler->restore_early_callback = arg->restore_early_callback; > > > + handler->context = arg->context; > > > + > > > + mutex_lock(&lps0_callback_handler_mutex); > > > + list_add(&handler->list_node, &lps0_callback_handler_head); > > > + mutex_unlock(&lps0_callback_handler_mutex); > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL_GPL(acpi_register_lps0_callbacks); > > > > Typically with calls like these we simply let the caller own the struct > > lps0_callback_handler > > and only make the list_add() call here. Typically the struct > > lps0_callback_handler will > > be embedded in the driver_data of the driver registering the handler and it > > will > > call the unregister function before free-ing its driver_data. > > > > When I put this together I was modeling it off of `struct acpi_wakeup_handler` > which the handling and the use in the kernel doesn't seem to follow the design > pattern > you describe. > > Rafael - can you please confirm which direction you want to see here for this? > > > > + > > > +void acpi_unregister_lps0_callbacks(struct lps0_callback_handler *arg) > > > +{ > > > + struct lps0_callback_handler *handler; > > > + > > > + mutex_lock(&lps0_callback_handler_mutex); > > > + list_for_each_entry(handler, &lps0_callback_handler_head, > > list_node) { > > > + if (handler->prepare_late_callback == arg- > > > prepare_late_callback && > > > + handler->restore_early_callback == arg- > > > restore_early_callback && > > > + handler->context == arg->context) { > > > + list_del(&handler->list_node); > > > + kfree(handler); > > > + break; > > > + } > > > + } > > > + mutex_unlock(&lps0_callback_handler_mutex); > > > +} > > > +EXPORT_SYMBOL_GPL(acpi_unregister_lps0_callbacks); > > > > And this then becomes just lock, list_del, unlock. > > > > Regards, > > > > Hans If you keep v3, Reviewed-by: David E. Box