Re: [PATCH 2/2] [I2C] add PCA9543 chip driver

[Riku Voipio <riku.voipio-WgUW+8SLYMv1KXRcyAk9cg@public.gmane.org>]

Bryan Wu wrote:
> From: Michael Hennerich 
> --- /dev/null
> +++ b/drivers/i2c/chips/pca9543.c
> @@ -0,0 +1,212 @@
> +/*
> +    pca9543.c - Part of lm_sensors, Linux kernel modules for hardware
> +             monitoring
>   
Not a sensor chip..
> +/* A few notes about the PCA9543:
> +
> +*   The PCA9543 is a bi-directional translating switch, controlled by the
> +    I2C bus. The SCL/SDA upstream pair fans out to two downstream
> +    pairs, or channels. Any individual SCx/SDx channels or combination
> +    of channels can be selected, determined by the contents of the
> +    programmable control register. Two interrupt inputs, INT0 to INT3,
> +    one for each of the downstream pairs, are provided. One interrupt
> +    output INT, which acts as an AND of the two interrupt inputs, is
> +    provided.
> +
> +*/
>   
You might want to add a url to the datasheet, rather than
a copypaste from it. Any description in the driver sould
> +
> +/* This is the driver that will be inserted */
> +static struct i2c_driver pca9543_driver = {
> +	.driver = {
> +		.name	= "pca9543",
> +	},
> +	.id		= I2C_DRIVERID_PCF8574,
> +	.attach_adapter	= pca9543_attach_adapter,
> +	.detach_client	= pca9543_detach_client,
> +};
>   
Since the chip is undetectable, and used on embedded platform, I think you
should consider a "new-style" driver with .probe/remove instead.
See i2c/writing-clients
> +	struct i2c_client *new_client;
>
> +	new_client = &data->client;
> +	i2c_set_clientdata(new_client, data);
> +	new_client->addr = address;
> +	new_client->adapter = adapter;
> +	new_client->driver = &pca9543_driver;
> +	new_client->flags = 0;
>
>   
s/new_client/client/ please.
> +	/* Now, we would do the remaining detection. But the PCF8574 is plainly
> +	   impossible to detect! Stupid chip. */
> +
>   
copypaste from PCF8574 driver? :) Again, consider if the new style drivers
would me more useful for your platform.



_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c