[PATCH] i2c-imx: Add missing preporcessor directives

[PATCH] i2c-imx: Add missing preporcessor directives

[Andrey Smirnov]

On non-PowerPC platforms call to i2c_fsl_set_clk() will try to obtain
I2C clock freqency from i2c_fsl->clk, however that field would not be
initialized if CONFIG_COMMON_CLK is not set. This patch add
conditional compilation directives to make sure that i2c_fsl_set_clk()
is not called if CONFIG_COMMON_CLK is not set.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/i2c/busses/i2c-imx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 4cd03e1..4abb710 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -557,11 +557,13 @@ static int __init i2c_fsl_probe(struct device_d *pdev)

 	i2c_fsl->dfsrr = -1;

+#if defined(CONFIG_COMMON_CLK) || defined(CONFIG_PPC)
 	/* Set up clock divider */
 	if (pdata && pdata->bitrate)
 		i2c_fsl_set_clk(i2c_fsl, pdata->bitrate);
 	else
 		i2c_fsl_set_clk(i2c_fsl, FSL_I2C_BIT_RATE);
+#endif

 	/* Set up chip registers to defaults */
 	writeb(0, i2c_fsl->base + FSL_I2C_I2CR);
--
2.1.4

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] i2c-imx: Add missing preporcessor directives

[Sam Ravnborg]

On Fri, Aug 14, 2015 at 07:45:51PM -0700, Andrey Smirnov wrote:
> On non-PowerPC platforms call to i2c_fsl_set_clk() will try to obtain
> I2C clock freqency from i2c_fsl->clk, however that field would not be
> initialized if CONFIG_COMMON_CLK is not set. This patch add
> conditional compilation directives to make sure that i2c_fsl_set_clk()
> is not called if CONFIG_COMMON_CLK is not set.

This driver is only built for ARCH_IMX (which select COMMON_CLK)
or two PowerPC variants.

See following snip from Kconfig:

config I2C_IMX
        bool "MPC85xx/MPC5200/i.MX I2C Master driver"
        depends on (ARCH_IMX && !ARCH_IMX1) || ARCH_MPC85XX || ARCH_MPC5200

How did you hit this bug in the first place?

If there is a valid reason to use this driver for non COMMON_CLK
case and not PowerPC then consider following approach.
Here the iffeddery is kept at the definition of the function.

	Sam

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 714e83c..9e2c810 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -337,7 +337,7 @@ static void i2c_fsl_set_clk(struct fsl_i2c_struct *i2c_fsl,
 	i2c_fsl->ifdr = fdr;
 	i2c_fsl->dfsrr = dfsr;
 }
-#else
+#elif defined (COMMON_CLK)
 static void i2c_fsl_set_clk(struct fsl_i2c_struct *i2c_fsl,
 			    unsigned int rate)
 {
@@ -374,6 +374,11 @@ static void i2c_fsl_set_clk(struct fsl_i2c_struct *i2c_fsl,
 	dev_dbg(&i2c_fsl->adapter.dev, "<%s> IFDR[IC]=0x%x, REAL DIV=%d\n",
 		__func__, i2c_clk_div[i][1], i2c_clk_div[i][0]);
 }
+#else
+static void i2c_fsl_set_clk(struct fsl_i2c_struct *i2c_fsl,
+			    unsigned int rate)
+{
+}
 #endif
 
 static int i2c_fsl_write(struct i2c_adapter *adapter, struct i2c_msg *msgs)

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] i2c-imx: Add missing preporcessor directives

[Andrey Smirnov]

>
> This driver is only built for ARCH_IMX (which select COMMON_CLK)
> or two PowerPC variants.
>
> See following snip from Kconfig:
>
> config I2C_IMX
>         bool "MPC85xx/MPC5200/i.MX I2C Master driver"
>         depends on (ARCH_IMX && !ARCH_IMX1) || ARCH_MPC85XX || ARCH_MPC5200
>
> How did you hit this bug in the first place?

I didn't really hit that bug. I was reading the source code of
i2c-imx.c to see an example of I2C controller driver and I noticed
inconsistency in how conditional compilation was applied.

>
> If there is a valid reason to use this driver for non COMMON_CLK
> case and not PowerPC then consider following approach.
> Here the iffeddery is kept at the definition of the function.

I like your approach better. I'll update the patch.

>
>         Sam
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 714e83c..9e2c810 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -337,7 +337,7 @@ static void i2c_fsl_set_clk(struct fsl_i2c_struct *i2c_fsl,
>         i2c_fsl->ifdr = fdr;
>         i2c_fsl->dfsrr = dfsr;
>  }
> -#else
> +#elif defined (COMMON_CLK)
>  static void i2c_fsl_set_clk(struct fsl_i2c_struct *i2c_fsl,
>                             unsigned int rate)
>  {
> @@ -374,6 +374,11 @@ static void i2c_fsl_set_clk(struct fsl_i2c_struct *i2c_fsl,
>         dev_dbg(&i2c_fsl->adapter.dev, "<%s> IFDR[IC]=0x%x, REAL DIV=%d\n",
>                 __func__, i2c_clk_div[i][1], i2c_clk_div[i][0]);
>  }
> +#else
> +static void i2c_fsl_set_clk(struct fsl_i2c_struct *i2c_fsl,
> +                           unsigned int rate)
> +{
> +}
>  #endif
>
>  static int i2c_fsl_write(struct i2c_adapter *adapter, struct i2c_msg *msgs)

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox