Linux PWM subsystem development
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Uwe Kleine-König" <u.kleine-koenig@baylibre.com>
Cc: linux-pwm@vger.kernel.org, "Hans de Goede" <hdegoede@redhat.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH 1/2] pwm: lpss: Move namespace import into a header
Date: Wed, 4 Dec 2024 00:28:23 +0200	[thread overview]
Message-ID: <Z0-GB4tQAz2gfmUN@smile.fi.intel.com> (raw)
In-Reply-To: <6fjduueg7r3nctg4ivevvk7kopax4ynm32prxacrieq5gpbcgx@zhrgpx2loulp>

On Tue, Dec 03, 2024 at 10:32:17PM +0100, Uwe Kleine-König wrote:
> On Tue, Dec 03, 2024 at 09:12:36PM +0200, Andy Shevchenko wrote:
> > On Tue, Dec 03, 2024 at 06:16:14PM +0100, Uwe Kleine-König wrote:
> > > Each user of the exported symbols related to the pwm-lpss driver needs
> > > to import the matching namespace. So this can just be done in the header
> > > together with the prototypes.
> > > 
> > > This fixes drivers/pinctrl/intel/pinctrl-intel.c which failed to import
> > > that namespace before. (However this didn't hurt because the pwm-lpss
> > > module namespace isn't used; see the next commit.)
> > 
> > I disagree on this change, I think it had been discussed already.
> 
> Who discussed this? If I was involved, I don't remember. So if you have
> a link, that would be great.

Sure, https://lore.kernel.org/linux-pwm/20220908135658.64463-1-andriy.shevchenko@linux.intel.com/
you were a participant there.

> > The header must not provide any module importing features as it effectively
> > diminishes the point of namespace. Any (ab)user can include just a header and
> > be okay with that.
> 
> Huh? Any abuser can also just do the IMPORT_NS statement? Module
> namespaces are not a safeguard against bad code? So I don't see why
> making it simple for the regular users should be the wrong choice.
> 
> Actually I think this is very elegant because this way all needs to use
> these symbols (i.e. prototype and namespace) are in a single place and
> users just do the #include and get all the preconditions.

Recent LWN https://lwn.net/Articles/998221/ article describes in more details
what I implied under abuser here.

> > Besides that, you should have based this on recent changes in the NS area of
> > the module symbols, i.e. module namespaces needs to be provided as string
> > literals.
> 
> I coordinated my patch set with the pwm maintainer and he is ok with it
> :-) That's why I wrote "This conflicts with
> ceb8bf2ceaa77fe222fe8fe32cb7789c9099ddf1 that is currently in Linus'
> tree. I'll take care about that." in the cover letter.

Other subsystems simply backmerged those changes. Note, at any time one may
consider origin/master as an immutable (at the point of a given change).
It might be better (if no urgent need) to wait for rc2 and merge it before
doing other changes.

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2024-12-03 22:28 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-03 17:16 [PATCH 0/2] pwm: lpss: module namespace fixes Uwe Kleine-König
2024-12-03 17:16 ` [PATCH 1/2] pwm: lpss: Move namespace import into a header Uwe Kleine-König
2024-12-03 19:12   ` Andy Shevchenko
2024-12-03 21:32     ` Uwe Kleine-König
2024-12-03 22:28       ` Andy Shevchenko [this message]
2024-12-03 23:38         ` Uwe Kleine-König
2024-12-04  1:50           ` Andy Shevchenko
2024-12-16  8:46     ` Uwe Kleine-König
2025-01-21 22:46       ` Linus Walleij
2024-12-03 17:16 ` [PATCH 2/2] pwm: lpss: Define DEFAULT_SYMBOL_NAMESPACE earlier Uwe Kleine-König
2024-12-03 19:19   ` Andy Shevchenko

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=Z0-GB4tQAz2gfmUN@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=hdegoede@redhat.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-pwm@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@baylibre.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: 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