Re: [RESEND PATCH v2 1/2] mfd: retu: Add OF device ID table

[Javier Martinez Canillas]

Hello Rob,

On 03/30/2017 08:41 AM, Rob Herring wrote:
> On Wed, Mar 15, 2017 at 5:49 AM, Javier Martinez Canillas
> <javier@xxxxxxxxxxxxxxx> wrote:
>> The driver doesn't have a struct of_device_id table but supported devices
>> are registered via Device Trees. This is working on the assumption that a
>> I2C device registered via OF will always match a legacy I2C device ID and
>> that the MODALIAS reported will always be of the form i2c:<device>.
> 
> It should also have the OF MODALIAS.
> 

Yes, it should but currently it doesn't. This is the .uevent callback for I2C:

static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
{
	struct i2c_client *client = to_i2c_client(dev);
	int rc;

	rc = acpi_device_uevent_modalias(dev, env);
	if (rc != -ENODEV)
		return rc;

	return add_uevent_var(env, "MODALIAS=%s%s", I2C_MODULE_PREFIX, client->name);
}

>> But this could change in the future so the correct approach is to have a
>> OF device ID table if the devices are registered via OF.
> 
> I believe this is already the case.
>

No, in fact the purpose of these patches (and others that I've been posting in
all subsystems) is to make sure that adding proper OF MODALIAS support for I2C
won't cause regressions in existing drivers due not having OF device ID tables.

This is the RFC patch I posted some time ago for I2C core:

https://patchwork.ozlabs.org/patch/388200/
 
>> The compatible strings don't have a vendor prefix because that's how it's
>> used currently, and changing this will be a Device Tree ABI break.
> 
> How so? These "compatible" strings are not documented, so there is no
> ABI. You may have been lucky to get things working using the driver
> name as the compatible string.
>

Yes, it's not documented but I thought that having users meant that it was
already an ABI.

I'm more than happy to change the upstream DTS to add a vendor prefix and
use this also in the drivers.

> Rob
>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America