[PATCH RFC] modpost: Support I2C Aliases from OF tables

[PATCH RFC] modpost: Support I2C Aliases from OF tables

[Kieran Bingham]

I2C drivers match against an I2C ID table, an OF table, and an ACPI
table. It is now also possible to match against an OF table entry
without the vendor prefix to support backwards compatibility, and allow
simplification of the i2c probe functions.

As part of this matching, the probe function is being converted to
remove the need to specify the i2c_device_id table, but to support
module aliasing, we still require to have the MODULE_DEVICE_TABLE entry.

Facilitate generating the I2C aliases directly from the of_device_id
tables, by stripping the vendor prefix prefix from the compatible string
and using that as an alias just as the i2c-core supports.

Drivers which remove the i2c_device_id table can then register against
the of_device_id table by adding an extra MODULE_DEVICE_TABLE
registration as shown by the following example:

 /* si4713_i2c_driver - i2c driver interface */
-static const struct i2c_device_id si4713_id[] = {
-       { "si4713" , 0 },
-       { },
-};
-MODULE_DEVICE_TABLE(i2c, si4713_id);

 static const struct of_device_id si4713_of_match[] = {
        { .compatible = "silabs,si4713" },
        { },
 };
 MODULE_DEVICE_TABLE(of, si4713_of_match);
+MODULE_DEVICE_TABLE(i2c_of, si4713_of_match);

Several drivers have had their i2c_device_id tables removed entirely
which will lead to their module aliases being limited, during the
following patches:

0f21700ac40c ("rtc: pcf85063: switch to probe_new")
3a4f4f2963f4 ("ASoC: rt5677: Convert I2C driver to ->probe_new()")
511cb17448d9 ("mfd: tps65217: Introduce dependency on CONFIG_OF")
8597c0920d6f ("NFC: fdp: Convert I2C driver to ->probe_new()")
b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new() call-back type")

The following patches might require I2C aliases to be generated from
their ACPI tables:

e19c92059a70 ("media: staging: atomisp: Switch i2c drivers to use ->probe_new()")
f758eb2363ec ("media: dw9714: Remove ACPI match tables, convert to use probe_new")

Cc: Lee Jones <lee.jones@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mark Brown <broonie@kernel.org>

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 scripts/mod/file2alias.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index e17a29ae2e97..16776f624d3a 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -379,6 +379,35 @@ static void do_of_table(void *symval, unsigned long size,
 		do_of_entry_multi(symval + i, mod);
 }
 
+static int do_i2c_of_entry(void *symval, struct module *mod)
+{
+	const char *alias;
+	DEF_FIELD_ADDR(symval, of_device_id, compatible);
+
+	alias = strrchr(*compatible, ',');
+
+	buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s%s\");\n",
+		   I2C_MODULE_PREFIX, alias ? alias + 1 : *compatible);
+
+	return 1;
+}
+
+/* Reuse OF tables to generate I2C aliases */
+static void do_i2c_of_table(void *symval, unsigned long size,
+			    struct module *mod)
+{
+	unsigned int i;
+	const unsigned long id_size = SIZE_of_device_id;
+
+	device_id_check(mod->name, "i2c_of", size, id_size, symval);
+
+	/* Leave last one: it's the terminator. */
+	size -= id_size;
+
+	for (i = 0; i < size; i += id_size)
+		do_i2c_of_entry(symval + i, mod);
+}
+
 /* Looks like: hid:bNvNpN */
 static int do_hid_entry(const char *filename,
 			     void *symval, char *alias)
@@ -1452,6 +1481,8 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
 		do_usb_table(symval, sym->st_size, mod);
 	if (sym_is(name, namelen, "of"))
 		do_of_table(symval, sym->st_size, mod);
+	else if (sym_is(name, namelen, "i2c_of"))
+		do_i2c_of_table(symval, sym->st_size, mod);
 	else if (sym_is(name, namelen, "pnp"))
 		do_pnp_device_entry(symval, sym->st_size, mod);
 	else if (sym_is(name, namelen, "pnp_card"))
-- 
2.20.1

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Javier Martinez Canillas]

Hello Kieran,

On 7/10/19 9:39 PM, Kieran Bingham wrote:
> I2C drivers match against an I2C ID table, an OF table, and an ACPI
> table. It is now also possible to match against an OF table entry
> without the vendor prefix to support backwards compatibility, and allow
> simplification of the i2c probe functions.
> 
> As part of this matching, the probe function is being converted to
> remove the need to specify the i2c_device_id table, but to support
> module aliasing, we still require to have the MODULE_DEVICE_TABLE entry.
>

My opinion on this is that I2C drivers should register the tables of the
firmware interfaces that support. That is, if a driver is only used in a
platform that supports OF then it should only require an OF device table.

But if the driver supports devices that can also be present in platforms
that use ACPI, then should also require to have an ACPI device ID table.

So there should be consistency about what table is used for both matching
a device with a driver and reporting a modalias to user-space for module
auto-loading. If a I2C device was instantiated by OF, then the OF table
should be used for both reporting a modalias uevent and matching a driver.

Now, the i2c_of_match_device() function attempts to match by first calling
of_match_device() and if fails fallbacks to i2c_of_match_device_sysfs().

The latter attempts to match the I2C device by striping the vendor prefix
of the compatible strings on the OF device ID table. That means that you
could instantiate an I2C device ID through the sysfs interface and the OF
table would be used to match the driver.

But i2c_device_uevent() would had reported an I2C modalias and not an OF
modalias (since the registered device won't have an associated of_node) so
there's inconsistency in that case since a table is used to match but no
to report modaliases.

> Facilitate generating the I2C aliases directly from the of_device_id
> tables, by stripping the vendor prefix prefix from the compatible string
> and using that as an alias just as the i2c-core supports.
> 

I see two ways to solve this issue. One is with $SUBJECT since we can argue
that if a OF-only driver is able to match devices that were instantiated
through the sysfs interface that only have a device name, then a modalias
of the form i2c:<foo> is needed. Since the compatible strings without the
vendor prefix is used to match, then I think that makes sense to also use
them without the vendor prefix to populate I2C modaliases as $SUBJECT does.

The other option is to remove i2c_of_match_device() and don't make OF match
to fallback to i2c_of_match_device_sysfs(). This is what happens in the ACPI
case, since i2c_device_match() just calls acpi_driver_match_device() directly
and doesn't have a wrapper function that fallbacks to sysfs matching.

In this case an I2C device ID table would be required if the devices have to
be instantiated through sysfs. That way the I2C table would be used both for
auto-loading and also to match the device when it doesn't have an of_node.

If the former is the correct way to solve this then the patch looks good to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Wolfram Sang]

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

Hi Javier,

thank you for providing the extra information.

(And Kieran, thanks for the patch!)

> The other option is to remove i2c_of_match_device() and don't make OF match
> to fallback to i2c_of_match_device_sysfs(). This is what happens in the ACPI
> case, since i2c_device_match() just calls acpi_driver_match_device() directly
> and doesn't have a wrapper function that fallbacks to sysfs matching.
> 
> In this case an I2C device ID table would be required if the devices have to
> be instantiated through sysfs. That way the I2C table would be used both for
> auto-loading and also to match the device when it doesn't have an of_node.

That would probably mean that only a minority of drivers will not add an I2C
device ID table because it is easy to add an you get the sysfs feature?

Then we are back again with the situation that most drivers will have
multiple tables. With the minor change that the I2C device id table is
not required anymore by the core, but it will be just very useful to
have? Or?

> If the former is the correct way to solve this then the patch looks good to me.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

For this actual patch from Kieran, I'd like to hear an opinion from the
people maintaining modpost. The aproach looks okay to me, yet I can't
tell how "easy" we are with adding new types like 'i2c_of'.

Thanks everyone,

   Wolfram


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

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Masahiro Yamada]

Hi.

On Thu, Aug 1, 2019 at 4:44 AM Wolfram Sang <wsa@the-dreams.de> wrote:
>
> Hi Javier,
>
> thank you for providing the extra information.
>
> (And Kieran, thanks for the patch!)
>
> > The other option is to remove i2c_of_match_device() and don't make OF match
> > to fallback to i2c_of_match_device_sysfs(). This is what happens in the ACPI
> > case, since i2c_device_match() just calls acpi_driver_match_device() directly
> > and doesn't have a wrapper function that fallbacks to sysfs matching.
> >
> > In this case an I2C device ID table would be required if the devices have to
> > be instantiated through sysfs. That way the I2C table would be used both for
> > auto-loading and also to match the device when it doesn't have an of_node.
>
> That would probably mean that only a minority of drivers will not add an I2C
> device ID table because it is easy to add an you get the sysfs feature?
>
> Then we are back again with the situation that most drivers will have
> multiple tables. With the minor change that the I2C device id table is
> not required anymore by the core, but it will be just very useful to
> have? Or?
>
> > If the former is the correct way to solve this then the patch looks good to me.
> >
> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>
> For this actual patch from Kieran, I'd like to hear an opinion from the
> people maintaining modpost.


As you see 'git log scripts/mod/file2alias.c',
this file is touched by every subsystem.

So, the decision is up to you, Wolfram.
And, you can pick this to your tree if you like.


The implementation is really trivial.


As Javier pointed out, this discussion comes down to
"do we want to fall back to i2c_of_match_device_sysfs()?"

If a driver supports DT and devices are instantiated via DT,
in which situation is this useful?
Do legacy non-DT platforms need this?



> The aproach looks okay to me, yet I can't
> tell how "easy" we are with adding new types like 'i2c_of'.

As far as I understood, this patch provides a shorthand.
You can save one table, but still get the
same MODULE_ALIAS in the *.mod.c file.
You need to add two MODULE_DEVICE_TABLE() though.

MODULE_DEVICE_TABLE(of, si4713_of_match);
MODULE_DEVICE_TABLE(i2c_of, si4713_of_match);


-- 
Best Regards
Masahiro Yamada

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Javier Martinez Canillas]

Hello Wolfram,

On 7/31/19 9:44 PM, Wolfram Sang wrote:
> Hi Javier,
> 
> thank you for providing the extra information.
> 
> (And Kieran, thanks for the patch!)
> 
>> The other option is to remove i2c_of_match_device() and don't make OF match
>> to fallback to i2c_of_match_device_sysfs(). This is what happens in the ACPI
>> case, since i2c_device_match() just calls acpi_driver_match_device() directly
>> and doesn't have a wrapper function that fallbacks to sysfs matching.
>>
>> In this case an I2C device ID table would be required if the devices have to
>> be instantiated through sysfs. That way the I2C table would be used both for
>> auto-loading and also to match the device when it doesn't have an of_node.
> 
> That would probably mean that only a minority of drivers will not add an I2C
> device ID table because it is easy to add an you get the sysfs feature?
>

I believe so yes.
 
> Then we are back again with the situation that most drivers will have
> multiple tables. With the minor change that the I2C device id table is
> not required anymore by the core, but it will be just very useful to
> have? Or?
>

Yes, it won't be needed anymore if you are only instantiating all your devices
from your firmware interface (e.g: OF, ACPI).

>> If the former is the correct way to solve this then the patch looks good to me.
>>
>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> For this actual patch from Kieran, I'd like to hear an opinion from the
> people maintaining modpost. The aproach looks okay to me, yet I can't
> tell how "easy" we are with adding new types like 'i2c_of'.
>

As Masahiro-san mentioned, this approach will still require to add a new macro
MODULE_DEVICE_TABLE(i2c_of, bar_of_match) so the OF device table is used twice.

One to expose the "of:N*T*Cfoo,bar" and another one to expose it as "i2c:bar".

I expect that many developers would miss adding this macro for new drivers that
are DT-only and so sysfs instantiation would not work there. So whatever is the
approach taken we should clearly document all this so drivers authors are aware.

> Thanks everyone,
> 
>    Wolfram
> 

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Javier Martinez Canillas]

Hello Masahiro-san,

On 8/1/19 4:17 AM, Masahiro Yamada wrote:
> Hi.
> 
> On Thu, Aug 1, 2019 at 4:44 AM Wolfram Sang <wsa@the-dreams.de> wrote:
>>
>> Hi Javier,
>>
>> thank you for providing the extra information.
>>
>> (And Kieran, thanks for the patch!)
>>
>>> The other option is to remove i2c_of_match_device() and don't make OF match
>>> to fallback to i2c_of_match_device_sysfs(). This is what happens in the ACPI
>>> case, since i2c_device_match() just calls acpi_driver_match_device() directly
>>> and doesn't have a wrapper function that fallbacks to sysfs matching.
>>>
>>> In this case an I2C device ID table would be required if the devices have to
>>> be instantiated through sysfs. That way the I2C table would be used both for
>>> auto-loading and also to match the device when it doesn't have an of_node.
>>
>> That would probably mean that only a minority of drivers will not add an I2C
>> device ID table because it is easy to add an you get the sysfs feature?
>>
>> Then we are back again with the situation that most drivers will have
>> multiple tables. With the minor change that the I2C device id table is
>> not required anymore by the core, but it will be just very useful to
>> have? Or?
>>
>>> If the former is the correct way to solve this then the patch looks good to me.
>>>
>>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>>
>> For this actual patch from Kieran, I'd like to hear an opinion from the
>> people maintaining modpost.
> 
> 
> As you see 'git log scripts/mod/file2alias.c',
> this file is touched by every subsystem.
> 
> So, the decision is up to you, Wolfram.
> And, you can pick this to your tree if you like.
> 
> 
> The implementation is really trivial.
> 
> 
> As Javier pointed out, this discussion comes down to
> "do we want to fall back to i2c_of_match_device_sysfs()?"
> 

Yes, I think that's the crux of the matter. Basically the matching logic
should be consistent with the modalias uevent exposed to user-space to
auto-load modules.

So I think that we should either:

a) take Kieran's patch or b) remove the i2c_of_match_device_sysfs() fallback
for OF and require an I2C device table for sysfs instantiation and matching.

> If a driver supports DT and devices are instantiated via DT,
> in which situation is this useful?

Is useful if you don't have all the I2C devices described in the DT. For example
a daughterboard with an I2C device is connected to a board through an expansion
slot or an I2C device connected directly to I2C pins exposed in a machine.

In these cases your I2C devices won't be static so users might want to use the
sysfs user-space interface to instantiate the I2C devices, i.e:

 # echo eeprom 0x50 > /sys/bus/i2c/devices/i2c-3/new_device

as explained in https://github.com/torvalds/linux/blob/master/Documentation/i2c/instantiating-devices#L207

> Do legacy non-DT platforms need this?

Yes, can also be used by non-DT platforms. But in this case isn't a problem
because drivers for these platform will already have an I2C device ID table.

> 
> 
> 
>> The aproach looks okay to me, yet I can't
>> tell how "easy" we are with adding new types like 'i2c_of'.
> 
> As far as I understood, this patch provides a shorthand.
> You can save one table, but still get the
> same MODULE_ALIAS in the *.mod.c file.
> You need to add two MODULE_DEVICE_TABLE() though.
> 
> MODULE_DEVICE_TABLE(of, si4713_of_match);
> MODULE_DEVICE_TABLE(i2c_of, si4713_of_match);
>

That's my understanding as well.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Geert Uytterhoeven]

Hi Javier,

On Tue, Aug 6, 2019 at 12:25 AM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> On 7/31/19 9:44 PM, Wolfram Sang wrote:
> > Hi Javier,
> >> The other option is to remove i2c_of_match_device() and don't make OF match
> >> to fallback to i2c_of_match_device_sysfs(). This is what happens in the ACPI
> >> case, since i2c_device_match() just calls acpi_driver_match_device() directly
> >> and doesn't have a wrapper function that fallbacks to sysfs matching.
> >>
> >> In this case an I2C device ID table would be required if the devices have to
> >> be instantiated through sysfs. That way the I2C table would be used both for
> >> auto-loading and also to match the device when it doesn't have an of_node.
> >
> > That would probably mean that only a minority of drivers will not add an I2C
> > device ID table because it is easy to add an you get the sysfs feature?
> >
>
> I believe so yes.

> As Masahiro-san mentioned, this approach will still require to add a new macro
> MODULE_DEVICE_TABLE(i2c_of, bar_of_match) so the OF device table is used twice.
>
> One to expose the "of:N*T*Cfoo,bar" and another one to expose it as "i2c:bar".
>
> I expect that many developers would miss adding this macro for new drivers that
> are DT-only and so sysfs instantiation would not work there. So whatever is the
> approach taken we should clearly document all this so drivers authors are aware.

You could add a new I2C_MODULE_DEVICE_TABLE() that adds both, right?
Makes it a little bit easier to check/enforce this.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Geert Uytterhoeven]

On Tue, Aug 6, 2019 at 12:48 AM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> On 8/1/19 4:17 AM, Masahiro Yamada wrote:
> So I think that we should either:
>
> a) take Kieran's patch or b) remove the i2c_of_match_device_sysfs() fallback
> for OF and require an I2C device table for sysfs instantiation and matching.
>
> > If a driver supports DT and devices are instantiated via DT,
> > in which situation is this useful?
>
> Is useful if you don't have all the I2C devices described in the DT. For example
> a daughterboard with an I2C device is connected to a board through an expansion
> slot or an I2C device connected directly to I2C pins exposed in a machine.
>
> In these cases your I2C devices won't be static so users might want to use the
> sysfs user-space interface to instantiate the I2C devices, i.e:
>
>  # echo eeprom 0x50 > /sys/bus/i2c/devices/i2c-3/new_device
>
> as explained in https://github.com/torvalds/linux/blob/master/Documentation/i2c/instantiating-devices#L207

Does this actually work with DT names, too? E.g.

# echo atmel,24c02 > /sys/bus/i2c/devices/i2c-3/new_device

Still leaves us with legacy names for backwards compatibility.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Javier Martinez Canillas]

Hello Geert,

On 8/6/19 9:22 AM, Geert Uytterhoeven wrote:
> Hi Javier,
> 
> On Tue, Aug 6, 2019 at 12:25 AM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> On 7/31/19 9:44 PM, Wolfram Sang wrote:
>>> Hi Javier,
>>>> The other option is to remove i2c_of_match_device() and don't make OF match
>>>> to fallback to i2c_of_match_device_sysfs(). This is what happens in the ACPI
>>>> case, since i2c_device_match() just calls acpi_driver_match_device() directly
>>>> and doesn't have a wrapper function that fallbacks to sysfs matching.
>>>>
>>>> In this case an I2C device ID table would be required if the devices have to
>>>> be instantiated through sysfs. That way the I2C table would be used both for
>>>> auto-loading and also to match the device when it doesn't have an of_node.
>>>
>>> That would probably mean that only a minority of drivers will not add an I2C
>>> device ID table because it is easy to add an you get the sysfs feature?
>>>
>>
>> I believe so yes.
> 
>> As Masahiro-san mentioned, this approach will still require to add a new macro
>> MODULE_DEVICE_TABLE(i2c_of, bar_of_match) so the OF device table is used twice.
>>
>> One to expose the "of:N*T*Cfoo,bar" and another one to expose it as "i2c:bar".
>>
>> I expect that many developers would miss adding this macro for new drivers that
>> are DT-only and so sysfs instantiation would not work there. So whatever is the
>> approach taken we should clearly document all this so drivers authors are aware.
> 
> You could add a new I2C_MODULE_DEVICE_TABLE() that adds both, right?
> Makes it a little bit easier to check/enforce this.
>

Right, we could add a macro for that. Although it should probably be called
I2C_OF_MODULE_DEVICE_TABLE() or something like that since is specific to OF.

> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Javier Martinez Canillas]

Hello Geert,

On 8/6/19 9:30 AM, Geert Uytterhoeven wrote:
> On Tue, Aug 6, 2019 at 12:48 AM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> On 8/1/19 4:17 AM, Masahiro Yamada wrote:
>> So I think that we should either:
>>
>> a) take Kieran's patch or b) remove the i2c_of_match_device_sysfs() fallback
>> for OF and require an I2C device table for sysfs instantiation and matching.
>>
>>> If a driver supports DT and devices are instantiated via DT,
>>> in which situation is this useful?
>>
>> Is useful if you don't have all the I2C devices described in the DT. For example
>> a daughterboard with an I2C device is connected to a board through an expansion
>> slot or an I2C device connected directly to I2C pins exposed in a machine.
>>
>> In these cases your I2C devices won't be static so users might want to use the
>> sysfs user-space interface to instantiate the I2C devices, i.e:
>>
>>  # echo eeprom 0x50 > /sys/bus/i2c/devices/i2c-3/new_device
>>
>> as explained in https://github.com/torvalds/linux/blob/master/Documentation/i2c/instantiating-devices#L207
> 
> Does this actually work with DT names, too? E.g.
> 
> # echo atmel,24c02 > /sys/bus/i2c/devices/i2c-3/new_device
>

My understanding is that it does. If I'm reading the code correctly the
i2c_of_match_device_sysfs() function first attempts to match using both
the vendor and device part of the compatible string and if that fails,
it strips the vendor part and try to match only using the device part.

So you could use any of the following:

# echo 24c02 0x50 > /sys/bus/i2c/devices/i2c-3/new_device

# echo atmel,24c02 0x50 > /sys/bus/i2c/devices/i2c-3/new_device

Best regards, 
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Enrico Weigelt, metux IT consult]

On 06.08.19 19:12, Javier Martinez Canillas wrote:

> Right, we could add a macro for that. Although it should probably be called
> I2C_OF_MODULE_DEVICE_TABLE() or something like that since is specific to OF.

At that point it should be completely noop when OF is disabled, so we
also can get rid of many ifdef's.

I've got some patch somewhere for introducing a MODULE_OF_TABLE() macro
as replacement for many MODULE_DEVICE_TABLE(of, ...) cases, which noops
when CONFIG_OF is disabled. (and similar ones for other table types).

by the way: haven't followed whole discussion, just picked up some
proposed table changes ... does that require userland upgrades ?


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Andy Shevchenko]

On Thu, Aug 08, 2019 at 03:12:47PM +0200, Enrico Weigelt, metux IT consult wrote:
> On 06.08.19 19:12, Javier Martinez Canillas wrote:
> 
> > Right, we could add a macro for that. Although it should probably be called
> > I2C_OF_MODULE_DEVICE_TABLE() or something like that since is specific to OF.
> 
> At that point it should be completely noop when OF is disabled, so we
> also can get rid of many ifdef's.

Why?

> I've got some patch somewhere for introducing a MODULE_OF_TABLE() macro
> as replacement for many MODULE_DEVICE_TABLE(of, ...) cases, which noops
> when CONFIG_OF is disabled. (and similar ones for other table types).

It's simple wrong to have #ifdef CONFIG_OF without counterpart of_match_ptr().
And taking into consideration that ID table itself doesn't depend to OF at all,
why not simple drop that #ifdef and of_match_ptr() all together?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables

[Enrico Weigelt, metux IT consult]

On 08.08.19 15:24, Andy Shevchenko wrote:
> On Thu, Aug 08, 2019 at 03:12:47PM +0200, Enrico Weigelt, metux IT consult wrote:
>> On 06.08.19 19:12, Javier Martinez Canillas wrote:
>>
>>> Right, we could add a macro for that. Although it should probably be called
>>> I2C_OF_MODULE_DEVICE_TABLE() or something like that since is specific to OF.
>>
>> At that point it should be completely noop when OF is disabled, so we
>> also can get rid of many ifdef's.
> 
> Why?

For cases where drivers work w/ or w/o oftree. Not sure whether it
applies to i2c specifically, but there're other places where we still
need nasty ifdef's (eg. gpio-keyboard).

>> I've got some patch somewhere for introducing a MODULE_OF_TABLE() macro
>> as replacement for many MODULE_DEVICE_TABLE(of, ...) cases, which noops
>> when CONFIG_OF is disabled. (and similar ones for other table types).
> 
> It's simple wrong to have #ifdef CONFIG_OF without counterpart of_match_ptr().

Of course, but that's just a part of the story. (actually I'd prefer
using it everywhere, even if the driver only supports oftree).

> And taking into consideration that ID table itself doesn't depend to OF at all,
> why not simple drop that #ifdef and of_match_ptr() all together?

Consumes less space. Yes, it isn't much, but in some scenarios one needs
to heavily reduce the kernel size. And I wouldn't like to use
of_match_ptr() inside a MODULE_DEVICE_TABLE() call :o


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287