Linux Power Management development
 help / color / mirror / Atom feed
From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Adrian Hunter <adrian.hunter@intel.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>,
	Victor Shih <victor.shih@genesyslogic.com.tw>,
	<linux-mmc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Subject: Re: [PATCH v2 1/6] mmc: sdhci: Use EXPORT_PM_FN_NS_GPL() for exporting PM functions
Date: Thu, 16 Jan 2025 16:55:24 +0000	[thread overview]
Message-ID: <3968b172-01f7-4a75-a6f0-9be7ccc0ff5e@opensource.cirrus.com> (raw)
In-Reply-To: <Z4kiVLrPTDZr3J2K@smile.fi.intel.com>

On 16/01/2025 3:14 pm, Andy Shevchenko wrote:
> On Mon, Dec 09, 2024 at 07:11:41PM +0200, Adrian Hunter wrote:
>> On 9/12/24 18:36, Andy Shevchenko wrote:
>>> On Mon, Dec 09, 2024 at 12:38:59PM +0200, Adrian Hunter wrote:
>>>> On 1/11/24 12:11, Andy Shevchenko wrote:
>>>>> Switch from ugly ifdeffery to using EXPORT_PM_FN_NS_GPL()
>>>>> for exporting PM functions. This helps cleaning up the other
>>>>> SDHCI drivers in the future.
>>>>
>>>> It seems sdhci is the first code in the kernel to use
>>>> EXPORT_PM_FN_NS_GPL() but it was not asked for ;-)
>>>>
>>>> As such, can you fill in a little background.  I am not
>>>> sure what it achieves.  Why have CONFIG_PM if not to
>>>> #ifdef dependent code behind it?
>>>
>>> It makes sure that the code elimination happens at compile time and
>>
>> Does it eliminate the code?  Maybe I am missing something,
>> but it looks like it is still there:
> 
> Hmm... Indeed. My tests show the same. I believe these new macros were never
> tested (and we have no users in the kernel).
> 
> Richard?
> 

As I recall the intention wasn't to eliminate the code. It was to
eliminate the EXPORT for code that had already been eliminated by other
macros.

A bunch of ugly macros were added a while back by someone to create PM
callback functions so that the code would be eliminated if CONFIG_PM=n,
without having to use __maybe_unused or ifdefs in the .c, And together
with this, drivers were being zealously changed to use these macros to
eliminated unused PM callbacks.

But the macros assumed the PM functions were always local static.
Sometimes a family of drivers for similar hardware share common code and
the PM functions are exported so I added these macros to make the EXPORT
also disappear. It's not great but it was based on an existing similar
pattern.

I did also have patches that used the macros with the PM wrapper macros
to eliminate the code and exports in the cs35l56 codec driver. But they
got held up behind a large backlog of other work and are basically still
sitting in a dark corner somewhere. At this point it doesn't seem worth
the trouble to eliminate the PM callbacks, especially as we don't know
of any platform that is using codecs like cs35l56 without PM. So really
I don't mind if you remove these EXPORT macros.

>>> at the same time gives developer less uglified (by ifdeffery) code.
>>> It means there is less risk to miss anything of that which make become
>>> a compile-time warning of unused function, or even issues during linking
>>> with modules, etc.
>>>
>>> Should I update a commit message with that?
> 


      reply	other threads:[~2025-01-16 16:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20241101101441.3518612-1-andriy.shevchenko@linux.intel.com>
     [not found] ` <20241101101441.3518612-2-andriy.shevchenko@linux.intel.com>
2024-12-09 10:38   ` [PATCH v2 1/6] mmc: sdhci: Use EXPORT_PM_FN_NS_GPL() for exporting PM functions Adrian Hunter
2024-12-09 16:36     ` Andy Shevchenko
2024-12-09 17:11       ` Adrian Hunter
2025-01-16 15:14         ` Andy Shevchenko
2025-01-16 16:55           ` Richard Fitzgerald [this message]

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=3968b172-01f7-4a75-a6f0-9be7ccc0ff5e@opensource.cirrus.com \
    --to=rf@opensource.cirrus.com \
    --cc=adrian.hunter@intel.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=ulf.hansson@linaro.org \
    --cc=victor.shih@genesyslogic.com.tw \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox