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

From: Masahiro Yamada <yamada.masahiro@socionext.com>
To: Wolfram Sang <wsa@the-dreams.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>,
	Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>,
	Michal Marek <michal.lkml@markovi.net>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Lee Jones <lee.jones@linaro.org>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH RFC] modpost: Support I2C Aliases from OF tables
Date: Thu, 1 Aug 2019 11:17:32 +0900	[thread overview]
Message-ID: <CAK7LNAQ6siWHU+N2c+6gqh7hHEJ_aDrVoiWnrTq1jiXQWSYYBA@mail.gmail.com> (raw)
In-Reply-To: <20190731194419.GB4084@kunai>

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