Re: [PATCH 1/2] pwm: lpss: Move namespace import into a header

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Andy,

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.

> 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.

> 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.

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux