Re: [PATCH v11 02/10] i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to i2c-core-smbus

[Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]

Hey Phil,

(CCing Jean for some additional expertise)

> > > In preparation to adding of_i2c_setup_smbus_alert() move
> > > i2c_setup_smbus_alert() to core module. of_i2c_setup_smbus_alert()
> > > will call i2c_setup_smbus_alert() and this avoid module dependecy issues.
> > 
> > I am not very happy with this but don't want to cause another delay. So,
> > I hope we can discuss and fix it incrementally.
> > 
> > From what it does, I really think both setup_alert functions belong
> > to i2c-smbus.c. Three possibilities come to my mind (untested,
> > though):
> > 
> > a) use try_then_request_module somehow
> > 
> > b) add to CONFIG_I2C something like: select I2C_SMBUS if OF
> > 
> > c) get rid of i2c-smbus.c entirely and move it all into the core
> > 
> > Dunno if a) or b) have been tried in the course of this series
> > already?
> > 
> 
> Nope hadn't tried either a) or b). a) is not something I was aware of.
> Had a brief play and not sure how to make this work. This gives me the
> same linking problem.

Which one? I2C core is built-in and the driver is a module? And
i2c-smbus is a module then, too?

> A variant of b) was tried by selecting I2C_SMBUS from driver modules
> having smbalert support, but that caused problems if the i2c core was
> builtin and the driver was a module. But this looks to work for me.

It should work(tm). Yet, it will cause overhead, because OF is big these
days and I2C_SMBUS is still rare.

> A variant of c) with option to exclude the i2c-smbus code still.
> Add i2c-smbus to the i2c-core module by:
> Change Kconfig I2C_SMBUS to a bool
> add i2c-smbus to i2c-core object.
> fixup duplicate init_module calls etc.

I see. From my side, why not. But I'd really like Jean's opinion here on
why i2c-smbus.c was a seperate module and not included in the core.
Because I split the I2C core into multiple source files recently, we
could have a seperate source file for i2c-smbus-alert without ifdeffery
but with some Makefile magic.

> Possibly also rename i2c-smbus to i2c-smbus-alert?

Why the rename? I thought we put all the code into the core?

> Let me know your preference and I'll put something together.

Thanks for the assistance. I am afraid we need a bit more discussion
first, though.

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]