Hi Wolfgang,
On Wed, 16 Jul 2008 12:47:25 +0200, Wolfgang Grandegger wrote:
> Currently, the I2C buses are probed for HWMON I2C devices, which might
> not be acceptable in same cases. This patch makes device probing
> configurable through the property "probe" of the FDT I2C device node:
>
> i2c at 3000 {
> ...
> compatible = "fsl-i2c";
> probe;
> ...
> };
>
> Assuming that systems using fsl-i2c should have proper platform data,
> probing is disabled by default. Unfortunately, that's not the case and
> various DTS files would require to be updated.
>
> There have already been some discussion on this subject with Jean
> under the following thread:
>
> http://ozlabs.org/pipermail/linuxppc-dev/2008-July/060008.html
>
> Please comment.
>
> Thanks,
>
> Wolfgang.
>
> Signed-off-by: Wolfgang Grandegger <wg at grandegger.com>
> ---
> arch/powerpc/sysdev/fsl_soc.c | 4 ++++
> drivers/i2c/busses/i2c-mpc.c | 17 ++++++++++++++---
> include/linux/fsl_devices.h | 1 +
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> Index: linux-2.6-denx/drivers/i2c/busses/i2c-mpc.c
> ===================================================================
> --- linux-2.6-denx.orig/drivers/i2c/busses/i2c-mpc.c
> +++ linux-2.6-denx/drivers/i2c/busses/i2c-mpc.c
> @@ -306,13 +306,21 @@ static const struct i2c_algorithm mpc_al
> .functionality = mpc_functionality,
> };
>
> -static struct i2c_adapter mpc_ops = {
> +static struct i2c_adapter mpc_probe_ops = {
> .owner = THIS_MODULE,
> .name = "MPC adapter",
> .id = I2C_HW_MPC107,
> .algo = &mpc_algo,
> .class = I2C_CLASS_HWMON,
> - .timeout = 1,
> + .timeout = 1,
Your patch is adding undesirable leading white space.
> +};
> +
> +static struct i2c_adapter mpc_ops = {
> + .owner = THIS_MODULE,
> + .name = "MPC adapter",
> + .id = I2C_HW_MPC107,
> + .algo = &mpc_algo,
> + .timeout = 1,
> };
I don't think you need 2 separate structures. Just set the class field
at runtime (see below.)
>
> static int fsl_i2c_probe(struct platform_device *pdev)
> @@ -354,7 +362,10 @@ static int fsl_i2c_probe(struct platform
> mpc_i2c_setclock(i2c);
> platform_set_drvdata(pdev, i2c);
>
> - i2c->adap = mpc_ops;
> + (i2c->flags & FSL_I2C_DEV_PROBE)
Missing if? :)
> + i2c->adap = mpc_probe_ops;
> + else
> + i2c->adap = mpc_ops;
Or just:
i2c->adap = mpc_ops;
if (i2c->flags & FSL_I2C_DEV_PROBE)
i2c->adap.flags = I2C_CLASS_HWMON;
> i2c->adap.nr = pdev->id;
> i2c_set_adapdata(&i2c->adap, i2c);
> i2c->adap.dev.parent = &pdev->dev;
> Index: linux-2.6-denx/arch/powerpc/sysdev/fsl_soc.c
> ===================================================================
> --- linux-2.6-denx.orig/arch/powerpc/sysdev/fsl_soc.c
> +++ linux-2.6-denx/arch/powerpc/sysdev/fsl_soc.c
> @@ -518,6 +518,10 @@ static int __init fsl_i2c_of_init(void)
> if (flags)
> i2c_data.device_flags |= FSL_I2C_DEV_CLOCK_5200;
>
> + flags = of_get_property(np, "probe", NULL);
> + if (flags)
> + i2c_data.device_flags |= FSL_I2C_DEV_PROBE;
> +
> ret =
> platform_device_add_data(i2c_dev, &i2c_data,
> sizeof(struct
> Index: linux-2.6-denx/include/linux/fsl_devices.h
> ===================================================================
> --- linux-2.6-denx.orig/include/linux/fsl_devices.h
> +++ linux-2.6-denx/include/linux/fsl_devices.h
> @@ -82,6 +82,7 @@ struct fsl_i2c_platform_data {
> /* Flags related to I2C device features */
> #define FSL_I2C_DEV_SEPARATE_DFSRR 0x00000001
> #define FSL_I2C_DEV_CLOCK_5200 0x00000002
> +#define FSL_I2C_DEV_PROBE 0x00000004
>
> enum fsl_usb2_operating_modes {
> FSL_USB2_MPH_HOST,
As a side note: in general it is not a good idea to use a template for
struct i2c_adapter. This structure is relatively large compared to the
few fields you need to set. If you insist on having a template, it
should at least be marked either const or __initdata.
--
Jean Delvare