From: Andy Shevchenko <andy.shevchenko@gmail.com> To: "Rafael J. Wysocki" <rafael@kernel.org> Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>, "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>, ACPI Devel Maling List <linux-acpi@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, "open list:ACPI COMPONENT ARCHITECTURE (ACPICA)" <devel@acpica.org>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, Len Brown <lenb@kernel.org>, Robert Moore <robert.moore@intel.com>, Erik Kaneda <erik.kaneda@intel.com> Subject: Re: [PATCH v1 1/1] ACPI: bus: Introduce acpi_dev_get() and reuse it in ACPI code Date: Mon, 12 Apr 2021 21:09:53 +0300 [thread overview] Message-ID: <CAHp75VcdZq9afovEP1Di6ScoFvBMuV0Es-XcuP60-Ewr9mfX5w@mail.gmail.com> (raw) In-Reply-To: <CAJZ5v0jTQziFzc7pZ631=iigm6nv7xKAYyaA64pssAPKTA522g@mail.gmail.com> On Mon, Apr 12, 2021 at 9:05 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Mon, Apr 12, 2021 at 7:47 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > > > On Mon, Apr 12, 2021 at 8:32 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > > On Sat, Apr 10, 2021 at 3:47 PM Andy Shevchenko > > > <andy.shevchenko@gmail.com> wrote: > > > > ... > > > > > > static void get_acpi_device(void *dev) > > > > { > > > > - if (dev) > > > > - get_device(&((struct acpi_device *)dev)->dev); > > > > + acpi_dev_get(dev); > > > > > > I would do > > > > > > if (dev) > > > acpi_dev_get(dev); > > > > > > here. > > > > Hmm... I don't see a point. acpi_dev_get() guaranteed to perform this check. > > > > > > } > > > > > > > > +static inline void acpi_dev_get(struct acpi_device *adev) > > > > +{ > > > > + if (adev) > > > > + get_device(&adev->dev); > > > > > > And I would drop the adev check from here (because the code calling it > > > may be running with wrong assumptions if adev is NULL). Or it should > > > return adev and the caller should be held responsible for checking it > > > against NULL (if they care). > > > > But this follows the get_device() / put_device() logic. > > Not really. get_device() returns a pointer. > > > Personally I don't think this is a good idea to deviate. > > Well, exactly. :-) > > > Note the acpi_bus_get_acpi_device() > > This also returns a pointer. Is it okay to return a pointer in acpi_dev_get() then? I will do it that way if there are no objections. > > / acpi_bus_put_acpi_device() as well. -- With Best Regards, Andy Shevchenko
WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andy.shevchenko at gmail.com> To: devel@acpica.org Subject: [Devel] Re: [PATCH v1 1/1] ACPI: bus: Introduce acpi_dev_get() and reuse it in ACPI code Date: Mon, 12 Apr 2021 21:09:53 +0300 [thread overview] Message-ID: <CAHp75VcdZq9afovEP1Di6ScoFvBMuV0Es-XcuP60-Ewr9mfX5w@mail.gmail.com> (raw) In-Reply-To: CAJZ5v0jTQziFzc7pZ631=iigm6nv7xKAYyaA64pssAPKTA522g@mail.gmail.com [-- Attachment #1: Type: text/plain, Size: 1707 bytes --] On Mon, Apr 12, 2021 at 9:05 PM Rafael J. Wysocki <rafael(a)kernel.org> wrote: > > On Mon, Apr 12, 2021 at 7:47 PM Andy Shevchenko > <andy.shevchenko(a)gmail.com> wrote: > > > > On Mon, Apr 12, 2021 at 8:32 PM Rafael J. Wysocki <rafael(a)kernel.org> wrote: > > > On Sat, Apr 10, 2021 at 3:47 PM Andy Shevchenko > > > <andy.shevchenko(a)gmail.com> wrote: > > > > ... > > > > > > static void get_acpi_device(void *dev) > > > > { > > > > - if (dev) > > > > - get_device(&((struct acpi_device *)dev)->dev); > > > > + acpi_dev_get(dev); > > > > > > I would do > > > > > > if (dev) > > > acpi_dev_get(dev); > > > > > > here. > > > > Hmm... I don't see a point. acpi_dev_get() guaranteed to perform this check. > > > > > > } > > > > > > > > +static inline void acpi_dev_get(struct acpi_device *adev) > > > > +{ > > > > + if (adev) > > > > + get_device(&adev->dev); > > > > > > And I would drop the adev check from here (because the code calling it > > > may be running with wrong assumptions if adev is NULL). Or it should > > > return adev and the caller should be held responsible for checking it > > > against NULL (if they care). > > > > But this follows the get_device() / put_device() logic. > > Not really. get_device() returns a pointer. > > > Personally I don't think this is a good idea to deviate. > > Well, exactly. :-) > > > Note the acpi_bus_get_acpi_device() > > This also returns a pointer. Is it okay to return a pointer in acpi_dev_get() then? I will do it that way if there are no objections. > > / acpi_bus_put_acpi_device() as well. -- With Best Regards, Andy Shevchenko
next prev parent reply other threads:[~2021-04-12 18:10 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-10 13:47 [PATCH v1 1/1] ACPI: bus: Introduce acpi_dev_get() and reuse it in ACPI code Andy Shevchenko 2021-04-10 13:47 ` [Devel] " Andy Shevchenko 2021-04-12 17:31 ` Rafael J. Wysocki 2021-04-12 17:31 ` [Devel] " Rafael J. Wysocki 2021-04-12 17:47 ` Andy Shevchenko 2021-04-12 17:47 ` [Devel] " Andy Shevchenko 2021-04-12 18:05 ` Rafael J. Wysocki 2021-04-12 18:05 ` [Devel] " Rafael J. Wysocki 2021-04-12 18:09 ` Andy Shevchenko [this message] 2021-04-12 18:09 ` Andy Shevchenko 2021-04-12 18:12 ` Rafael J. Wysocki 2021-04-12 18:12 ` [Devel] " Rafael J. Wysocki
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAHp75VcdZq9afovEP1Di6ScoFvBMuV0Es-XcuP60-Ewr9mfX5w@mail.gmail.com \ --to=andy.shevchenko@gmail.com \ --cc=devel@acpica.org \ --cc=dwaipayanray1@gmail.com \ --cc=erik.kaneda@intel.com \ --cc=lenb@kernel.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=rafael.j.wysocki@intel.com \ --cc=rafael@kernel.org \ --cc=rjw@rjwysocki.net \ --cc=robert.moore@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.