Re: [PATCH v1 1/1] i2c: Add acpi support to enumerate i2c mux clients

[sathyanarayanan kuppuswamy <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]

Hi Sang

Thanks for your comments. Please find my reply inline.

On 06/02/2015 09:19 AM, Wolfram Sang wrote:
>> +#ifdef CONFIG_ACPI
>> +static void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
>> +					  struct device *mux_dev)
>> +{
>> +	acpi_handle handle;
>> +	acpi_status status;
>> +
>> +	handle = ACPI_HANDLE(mux_dev);
>> +	if (!handle)
>> +		return;
>> +
>> +	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
>> +				     acpi_i2c_add_device, NULL,
>> +				     adap, NULL);
>> +
>> +	if (ACPI_FAILURE(status))
>> +		dev_warn(mux_dev, "mux adapter slave enumeration fails\n");
>> +}
>> +#else /* !CONFIG_ACPI */
>> +static inline void acpi_i2c_mux_register_devices(struct i2c_adapter *adap,
>> +						 struct device *mux_dev) { }
>> +#endif /* CONFIG_ACPI */
> IMO, this shares too much code with acpi_i2c_register_devices(). And it
> pulls in ACPI into mux.c which is not really needed.
Even though mux is a just a virtual adapter without any ACPI ID, Its 
slaves are a still actual devices and needs to be enumerated by ACPI. So 
think its accptable to include ACPI in mux code. Don't you agree ?

Also ACPI handle code in this function is slightly different from 
acpi_i2c_register_devices() code.

>
> What about naming the above function
> acpi_i2c_register_devices_from_dev() and let acpi_i2c_register_devices()
If you call this function from i2c-core register device function, then 
how will you maintain the hierarchy ? We need some way to indicate that 
these devices are under mux adapter right ?
> then call it as a helper function, all this in i2c-core.c?
>
> Thanks for your patience BTW...
>

-- 
Sathyanarayanan Kuppuswamy
Android kernel developer