Re: [PATCH 1/2] i2c: add ACPI support for i2c-piix4

[Jean Delvare <jdelvare@suse.de>]

Hi Andrew,

On Thu, 23 Nov 2017 13:09:37 +1000, Andrew Cooks wrote:
> This enables the i2c-piix4 SMBus controller driver to enumerate I2C
> slave devices using ACPI. It builds on the related I2C mux device work
> in commit 8eb5c87a92c0 ("i2c: add ACPI support for I2C mux ports")
> 
> Signed-off-by: Andrew Cooks <andrew.cooks@opengear.com>
> ---
>  drivers/i2c/busses/i2c-piix4.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index 174579d..9260cfa 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -837,6 +837,12 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
>  	/* set up the sysfs linkage to our parent device */
>  	adap->dev.parent = &dev->dev;
>  
> +	if (has_acpi_companion(&dev->dev)) {
> +		acpi_preset_companion(&adap->dev,
> +				      ACPI_COMPANION(&dev->dev),
> +				      port);
> +	}
> +
>  	snprintf(adap->name, sizeof(adap->name),
>  		"SMBus PIIX4 adapter%s at %04x", name, smba);
>  

Looks good to me but I think you have the patches in the wrong order.
First we must get the port number right, and then you can instantiate
the devices from the ACPI data. If you do it the other way around then
you have a transient situation where you instantiate a device where it
does not exist.

-- 
Jean Delvare
SUSE L3 Support