diff mbox

mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading

Message ID 20161005155112.13774-2-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hans de Goede Oct. 5, 2016, 3:51 p.m. UTC
The i2c subsys does not load modules by compatible, only by
i2c-id, with e.g. a modalias of: "i2c:axp209".

Populate the axp20x_i2c_id[] table with supported ids, so that
module auto-loading will work.

Reported-by: Dennis Gilmore <dennis@ausil.us>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mfd/axp20x-i2c.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Chen-Yu Tsai Oct. 18, 2016, 5:25 a.m. UTC | #1
On Wed, Oct 5, 2016 at 11:51 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> The i2c subsys does not load modules by compatible, only by
> i2c-id, with e.g. a modalias of: "i2c:axp209".
>
> Populate the axp20x_i2c_id[] table with supported ids, so that
> module auto-loading will work.
>
> Reported-by: Dennis Gilmore <dennis@ausil.us>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

Even though axp20x-i2c seems to be the only "DT only" i2c client,
would it make sense to add DT-based module autoloading to the i2c
core?

This also made me realize we probably don't have module auto-loading
for sunxi-rsb either.

Both should be doable with Stephen Boyd's "of: device: Export
of_device_{get_modalias, uvent_modalias} to modules" patch.

Regards
ChenYu
Hans de Goede Oct. 18, 2016, 10:25 a.m. UTC | #2
Hi,

On 18-10-16 07:25, Chen-Yu Tsai wrote:
> On Wed, Oct 5, 2016 at 11:51 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The i2c subsys does not load modules by compatible, only by
>> i2c-id, with e.g. a modalias of: "i2c:axp209".
>>
>> Populate the axp20x_i2c_id[] table with supported ids, so that
>> module auto-loading will work.
>>
>> Reported-by: Dennis Gilmore <dennis@ausil.us>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> Acked-by: Chen-Yu Tsai <wens@csie.org>
>
> Even though axp20x-i2c seems to be the only "DT only" i2c client,
> would it make sense to add DT-based module autoloading to the i2c
> core?

If it is not too invasive, then yes that would be a sensible addition IMHO.

Regards,

Hans
Sebastian Reichel Oct. 19, 2016, 2:51 a.m. UTC | #3
Hi,

On Wed, Oct 05, 2016 at 05:51:12PM +0200, Hans de Goede wrote:
> The i2c subsys does not load modules by compatible, only by
> i2c-id, with e.g. a modalias of: "i2c:axp209".
> 
> Populate the axp20x_i2c_id[] table with supported ids, so that
> module auto-loading will work.
> 
> Reported-by: Dennis Gilmore <dennis@ausil.us>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mfd/axp20x-i2c.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index b1b8658..d35a5fe 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -69,10 +69,11 @@ static const struct of_device_id axp20x_i2c_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
>  
> -/*
> - * This is useless for OF-enabled devices, but it is needed by I2C subsystem
> - */
>  static const struct i2c_device_id axp20x_i2c_id[] = {
> +	{ "axp152", 0 },
> +	{ "axp202", 0 },
> +	{ "axp209", 0 },
> +	{ "axp221", 0 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);

Thanks, queued.

-- Sebastian
Sebastian Reichel Oct. 19, 2016, 2:56 a.m. UTC | #4
Hi,

On Wed, Oct 19, 2016 at 04:51:55AM +0200, Sebastian Reichel wrote:
> > diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> > index b1b8658..d35a5fe 100644
> > --- a/drivers/mfd/axp20x-i2c.c
> > +++ b/drivers/mfd/axp20x-i2c.c
> > @@ -69,10 +69,11 @@ static const struct of_device_id axp20x_i2c_of_match[] = {
> >  };
> >  MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
> >  
> > -/*
> > - * This is useless for OF-enabled devices, but it is needed by I2C subsystem
> > - */
> >  static const struct i2c_device_id axp20x_i2c_id[] = {
> > +	{ "axp152", 0 },
> > +	{ "axp202", 0 },
> > +	{ "axp209", 0 },
> > +	{ "axp221", 0 },
> >  	{ },
> >  };
> >  MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
> 
> Thanks, queued.

My mistake. It should obviously go through mfd. So take this instead:

Acked-By: Sebastian Reichel <sre@kernel.org>

-- Sebastian
Lee Jones Oct. 24, 2016, 10:09 a.m. UTC | #5
On Tue, 18 Oct 2016, Hans de Goede wrote:
> On 18-10-16 07:25, Chen-Yu Tsai wrote:
> > On Wed, Oct 5, 2016 at 11:51 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> > > The i2c subsys does not load modules by compatible, only by
> > > i2c-id, with e.g. a modalias of: "i2c:axp209".
> > > 
> > > Populate the axp20x_i2c_id[] table with supported ids, so that
> > > module auto-loading will work.
> > > 
> > > Reported-by: Dennis Gilmore <dennis@ausil.us>
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > 
> > Acked-by: Chen-Yu Tsai <wens@csie.org>
> > 
> > Even though axp20x-i2c seems to be the only "DT only" i2c client,
> > would it make sense to add DT-based module autoloading to the i2c
> > core?
> 
> If it is not too invasive, then yes that would be a sensible addition IMHO.

If I understand you correctly, I already have a patch-set on the ML
that does this.
Wolfram Sang Oct. 24, 2016, 10:17 a.m. UTC | #6
> > > Even though axp20x-i2c seems to be the only "DT only" i2c client,
> > > would it make sense to add DT-based module autoloading to the i2c
> > > core?
> > 
> > If it is not too invasive, then yes that would be a sensible addition IMHO.
> 
> If I understand you correctly, I already have a patch-set on the ML
> that does this.

Incomplete, though. But we will see if Kieran finds time somewhen...
Lee Jones Oct. 24, 2016, 10:34 a.m. UTC | #7
On Mon, 24 Oct 2016, Wolfram Sang wrote:

> 
> > > > Even though axp20x-i2c seems to be the only "DT only" i2c client,
> > > > would it make sense to add DT-based module autoloading to the i2c
> > > > core?
> > > 
> > > If it is not too invasive, then yes that would be a sensible addition IMHO.
> > 
> > If I understand you correctly, I already have a patch-set on the ML
> > that does this.
> 
> Incomplete, though. But we will see if Kieran finds time somewhen...

I was under the impression it was all but ready.

What are you waiting on?
Wolfram Sang Oct. 24, 2016, 10:47 a.m. UTC | #8
> I was under the impression it was all but ready.

Then, I would have applied it.

> What are you waiting on?

Lee, I don't want to explain it *again*. Please re-read Kieran's last
attempt.
Lee Jones Oct. 26, 2016, 2 p.m. UTC | #9
On Mon, 24 Oct 2016, Wolfram Sang wrote:

> 
> > I was under the impression it was all but ready.
> 
> Then, I would have applied it.
> 
> > What are you waiting on?
> 
> Lee, I don't want to explain it *again*. Please re-read Kieran's last
> attempt.

I met with Kieran.  He's submitted a new version.

Here's hoping! ;)
Lee Jones Oct. 26, 2016, 2:01 p.m. UTC | #10
On Wed, 05 Oct 2016, Hans de Goede wrote:

> The i2c subsys does not load modules by compatible, only by
> i2c-id, with e.g. a modalias of: "i2c:axp209".
> 
> Populate the axp20x_i2c_id[] table with supported ids, so that
> module auto-loading will work.
> 
> Reported-by: Dennis Gilmore <dennis@ausil.us>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mfd/axp20x-i2c.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Applied (for now), thanks.

> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index b1b8658..d35a5fe 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -69,10 +69,11 @@ static const struct of_device_id axp20x_i2c_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
>  
> -/*
> - * This is useless for OF-enabled devices, but it is needed by I2C subsystem
> - */
>  static const struct i2c_device_id axp20x_i2c_id[] = {
> +	{ "axp152", 0 },
> +	{ "axp202", 0 },
> +	{ "axp209", 0 },
> +	{ "axp221", 0 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
diff mbox

Patch

diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
index b1b8658..d35a5fe 100644
--- a/drivers/mfd/axp20x-i2c.c
+++ b/drivers/mfd/axp20x-i2c.c
@@ -69,10 +69,11 @@  static const struct of_device_id axp20x_i2c_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
 
-/*
- * This is useless for OF-enabled devices, but it is needed by I2C subsystem
- */
 static const struct i2c_device_id axp20x_i2c_id[] = {
+	{ "axp152", 0 },
+	{ "axp202", 0 },
+	{ "axp209", 0 },
+	{ "axp221", 0 },
 	{ },
 };
 MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);