Re: [PATCH] i2c: designware: Actually make use of the I2C_DW_COMMON and I2C_DW symbol namespaces

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Dec 03, 2024 at 11:46:07PM +0100, Uwe Kleine-König wrote:
> On Tue, Dec 03, 2024 at 09:27:35PM +0200, Andy Shevchenko wrote:
> > On Tue, Dec 03, 2024 at 06:36:40PM +0100, Uwe Kleine-König wrote:
> > > DEFAULT_SYMBOL_NAMESPACE must already be defined when <linux/export.h>
> > > is included. So move the define above the include block.
> > 
> > As in the other email I pointed out the doc says that we need to undef the
> > symbol. No need to move it around.
> >
> > The only requirement is to place that before any EXPORT_SYMBOL*() we want to
> > add it to.
> 
> Did you test your statement?

I speak the documentation.

> I did on top of v6.13-rc1:
> 
>  - Remove the MODULE_IMPORT_NS(I2C_DW_COMMON) statements from
>    drivers/i2c/busses/i2c-designware-master.c,
>    drivers/i2c/busses/i2c-designware-pcidrv.c,
>    drivers/i2c/busses/i2c-designware-platdrv.c and
>    drivers/i2c/busses/i2c-designware-slave.c
>    => Built fine, ergo the symbols are not in the I2C_DW_COMMON
>    namespace.
> 
>  - On top of the previous: Add an #undef for DEFAULT_SYMBOL_NAMESPACE
>    directly before the #define
>    => Built fine, ergo the #undef doesn't make the namespace define
>    work.
> 
>  - On top of the previous: Move #undef + #define above the #includes
>    => Several warnings like:
> 
> 	WARNING: modpost: module i2c-designware-platform uses symbol i2c_dw_prepare_clk from namespace I2C_DW_COMMON, but does not import it.
> 
>    Ergo the position of the definition is relevant.
> 
>  - On top of the previous: Drop the #undef
>    => same as before, ergo the #undef is not needed.
> 
> Independent of what is in the docs that matches my understandment of C.
> 
> I don't expect that DEFAULT_SYMBOL_NAMESPACE to be already defined. If
> there was a definition already, a #define without #undef results (under
> some likely conditions) in a compiler warning. With the #undef this
> warning would be suppressed. So the #undef is something like a --force
> switch that I prefer not to use without reason, because I want to get
> the warning if my expectations are not met.
> 
> So I still think the patch is fine as is.

Perhaps we need to update the documentation first.

-- 
With Best Regards,
Andy Shevchenko