Re: [PATCH v7 2/4] i2c-acpi : exclude ARA address for smbus device

[CAPDEVILLE Marc]

Le dimanche 14 janvier 2018 Ã 11:28 +0000, Jonathan Cameron a ÃcritÂ:
> On Sat, 13 Jan 2018 14:37:03 +0100
> Marc CAPDEVILLE <m.capdeville@xxxxxxxxxx> wrote:
> 
> > Somme ACPI enumerated devices are known to support smbus alert protocol.
> > Theses devices may be miss-enumerated with the reserved smbus ARA address.
> > 
> > This is the case on Asus T100 tablet where cm3218 ambiant light sensor
> > expose two i2c serial bus connections, with the first one being the alert
> > response address.
> > 
> > This patch make a match on known ACPI ids for which devices are smbus ARA
> > capable, then skip the connection if it has the reserved 0x0c address and
> > mark it with I2C_CLIENT_ALERT flag. So device is enumerated with the
> > correct address.
> 
> I wonder if we are safe to always skip 0x0c address whether or not
> we know the device supports ARA.ÂÂThe exception may be for devices
> that do support ARA but are rolling their own support the hard way...

The device is only flagged with I2C_CLIENT_ALERT if its ID is in the
i2c_acpi_alert_device_ids. IF a driver want to be enumerated with the
reserved Ox0c address, It just have to not put its Id in this table. But
this may break with registering the smbus_alert device as the address will
be marked as busy for this adapter.

> I suppose it's possible there are i2c devices (not smbus where the
> spec says you must not use 0x0c for normal address IIRC) that use
> this address.ÂÂDoes anyone know of any?

I think this possible, but a such device can't share the bus with a smbus
alert capable device.
> 
> 
> Jonathan
> 
> > 
> > Signed-off-by: Marc CAPDEVILLE <m.capdeville@xxxxxxxxxx>
> > ---
> > Âdrivers/i2c/i2c-core-acpi.c | 23 +++++++++++++++++++++--
> > Âinclude/linux/i2c.hÂÂÂÂÂÂÂÂÂ|ÂÂ1 +
> > Â2 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> > index a9126b3cda61..5a8886f14329 100644
> > --- a/drivers/i2c/i2c-core-acpi.c
> > +++ b/drivers/i2c/i2c-core-acpi.c
> > @@ -59,8 +59,14 @@ static int i2c_acpi_fill_info(struct acpi_resource *ares,
> > void *data)
> > Â	if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
> > Â		return 1;
> > Â
> > -	if (lookup->index != -1 && lookup->n++ != lookup->index)
> > -		return 1;
> > +	if (lookup->index != -1) {
> > +		if (lookup->n++ != lookup->index)
> > +			return 1;
> > +	} else {
> > +		if (lookup->info->flags & I2C_CLIENT_ALERT &&
> > +		ÂÂÂÂsb->slave_address == 0x0c)
> > +			return 1;
> > +	}
> > Â
> > Â	status = acpi_get_handle(lookup->device_handle,
> > Â				Âsb->resource_source.string_ptr,
> > @@ -85,6 +91,15 @@ static const struct acpi_device_id
> > i2c_acpi_ignored_device_ids[] = {
> > Â	{}
> > Â};
> > Â
> > +static const struct acpi_device_id i2c_acpi_alert_device_ids[] = {
> > +	/*
> > +	Â* Smbus alert capable device which may have the reserved ARA
> > address
> > +	Â* in their serial bus resources list.
> > +	Â*/
> > +	{ "CPLM3218", 0 },
> > +	{}
> > +};
> > +
> > Âstatic int i2c_acpi_do_lookup(struct acpi_device *adev,
> > Â			ÂÂÂÂÂÂstruct i2c_acpi_lookup *lookup)
> > Â{
> > @@ -100,6 +115,10 @@ static int i2c_acpi_do_lookup(struct acpi_device *adev,
> > Â		return -ENODEV;
> > Â
> > Â	memset(info, 0, sizeof(*info));
> > +
> > +	if (acpi_match_device_ids(adev, i2c_acpi_alert_device_ids) == 0)
> > +		info->flags |= I2C_CLIENT_ALERT;
> > +
> > Â	lookup->device_handle = acpi_device_handle(adev);
> > Â
> > Â	/* Look up for I2cSerialBus resource */
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index 7592dce12923..b0d6f1333442 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -743,6 +743,7 @@ i2c_unlock_adapter(struct i2c_adapter *adapter)
> > Â#define I2C_CLIENT_SLAVE	0x20	/* we are the slave */
> > Â#define I2C_CLIENT_HOST_NOTIFY	0x40	/* We want to use I2C
> > host notify */
> > Â#define I2C_CLIENT_WAKE		0x80	/* for board_info; true
> > iff can wake */
> > +#define I2C_CLIENT_ALERT	0x100	/* Client use SMBUS alert
> > protocol */
> > Â#define I2C_CLIENT_SCCB		0x9000	/* Use Omnivision SCCB
> > protocol */
> > Â					/* Must match I2C_M_STOP|IGNORE_NAK
> > */
> > Â
> 
>