[patch 0/2] S3C24XX Updates for post 2.6.26

[patch 0/2] S3C24XX Updates for post 2.6.26

[Ben Dooks]

I2C updates for merging once 2.6.26 is out.

-- 
Ben

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

[patch 1/2] I2C: S3C2410: Pass the I2C bus number via drivers platform data

[Ben Dooks]

[-- Attachment #1: simtec/simtec-drivers-i2c-s3c2410-busnum.patch --]
[-- Type: text/plain, Size: 2756 bytes --]

Allow the platform data to specify the bus bumber that the
new I2C bus will be given. This is to allow the use of the
board registration mechanism to specify the new style of
I2C device registration which allows boards to provide a
list of attached devices.

Note, as discussed on the mailing list, we have dropped
backwards compatibility of adding an dynamic bus number
as it should not affect most boards to have the bus pinned
to 0 if they have either not specified platform data for
driver. Any board supplying platform data will automatically
have the bus_num field set to 0, and anyone who needs the
driver on a different bus number can supply platform data
to set bus_num.

Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>

Index: linux-2.6.26-rc4-quilt3/drivers/i2c/busses/i2c-s3c2410.c
===================================================================
--- linux-2.6.26-rc4-quilt3.orig/drivers/i2c/busses/i2c-s3c2410.c	2008-06-02 22:55:20.000000000 +0100
+++ linux-2.6.26-rc4-quilt3/drivers/i2c/busses/i2c-s3c2410.c	2008-06-02 22:55:29.000000000 +0100
@@ -752,9 +752,12 @@ static int s3c24xx_i2c_init(struct s3c24
 static int s3c24xx_i2c_probe(struct platform_device *pdev)
 {
 	struct s3c24xx_i2c *i2c = &s3c24xx_i2c;
+	struct s3c2410_platform_i2c *pdata;
 	struct resource *res;
 	int ret;
 
+	pdata = s3c24xx_i2c_get_platformdata(&pdev->dev);
+
 	/* find the clock and enable it */
 
 	i2c->dev = &pdev->dev;
@@ -832,7 +835,15 @@ static int s3c24xx_i2c_probe(struct plat
 	dev_dbg(&pdev->dev, "irq resource %p (%lu)\n", res,
 		(unsigned long)res->start);
 
-	ret = i2c_add_adapter(&i2c->adap);
+	/* Note, previous versions of the driver used i2c_add_adapter()
+	 * to add an bus at any number. We now pass the bus number via
+	 * the platform data, so if unset it will now default to always
+	 * being bus 0.
+	 */
+
+	i2c->adap.nr = pdata->bus_num;
+	ret = i2c_add_numbered_adapter(&i2c->adap);
+
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to add bus to i2c core\n");
 		goto err_irq;
Index: linux-2.6.26-rc4-quilt3/include/asm-arm/plat-s3c/iic.h
===================================================================
--- linux-2.6.26-rc4-quilt3.orig/include/asm-arm/plat-s3c/iic.h	2008-06-02 20:12:47.000000000 +0100
+++ linux-2.6.26-rc4-quilt3/include/asm-arm/plat-s3c/iic.h	2008-06-02 22:55:29.000000000 +0100
@@ -21,6 +21,7 @@
 */
 
 struct s3c2410_platform_i2c {
+	int		bus_num;	/* bus number to use */
 	unsigned int	flags;
 	unsigned int	slave_addr;	/* slave address for controller */
 	unsigned long	bus_freq;	/* standard bus frequency */

-- 

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

Re: [patch 1/2] I2C: S3C2410: Pass the I2C bus number via drivers platform data

[Jean Delvare]

Hi Ben,

On Wed, 09 Jul 2008 13:51:48 +0100, Ben Dooks wrote:
> Allow the platform data to specify the bus bumber that the
> new I2C bus will be given. This is to allow the use of the
> board registration mechanism to specify the new style of
> I2C device registration which allows boards to provide a
> list of attached devices.
> 
> Note, as discussed on the mailing list, we have dropped
> backwards compatibility of adding an dynamic bus number
> as it should not affect most boards to have the bus pinned
> to 0 if they have either not specified platform data for

either?

> driver. Any board supplying platform data will automatically
> have the bus_num field set to 0, and anyone who needs the
> driver on a different bus number can supply platform data
> to set bus_num.

Sounds OK to me.

> 
> Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> 
> Index: linux-2.6.26-rc4-quilt3/drivers/i2c/busses/i2c-s3c2410.c
> ===================================================================
> --- linux-2.6.26-rc4-quilt3.orig/drivers/i2c/busses/i2c-s3c2410.c	2008-06-02 22:55:20.000000000 +0100
> +++ linux-2.6.26-rc4-quilt3/drivers/i2c/busses/i2c-s3c2410.c	2008-06-02 22:55:29.000000000 +0100
> @@ -752,9 +752,12 @@ static int s3c24xx_i2c_init(struct s3c24
>  static int s3c24xx_i2c_probe(struct platform_device *pdev)
>  {
>  	struct s3c24xx_i2c *i2c = &s3c24xx_i2c;
> +	struct s3c2410_platform_i2c *pdata;
>  	struct resource *res;
>  	int ret;
>  
> +	pdata = s3c24xx_i2c_get_platformdata(&pdev->dev);
> +
>  	/* find the clock and enable it */
>  
>  	i2c->dev = &pdev->dev;
> @@ -832,7 +835,15 @@ static int s3c24xx_i2c_probe(struct plat
>  	dev_dbg(&pdev->dev, "irq resource %p (%lu)\n", res,
>  		(unsigned long)res->start);
>  
> -	ret = i2c_add_adapter(&i2c->adap);
> +	/* Note, previous versions of the driver used i2c_add_adapter()
> +	 * to add an bus at any number. We now pass the bus number via

"an bus" doesn't look correct.

> +	 * the platform data, so if unset it will now default to always
> +	 * being bus 0.
> +	 */
> +
> +	i2c->adap.nr = pdata->bus_num;
> +	ret = i2c_add_numbered_adapter(&i2c->adap);
> +
>  	if (ret < 0) {

Best practice is to not have a blank line between a function call and
the test of its return value.

>  		dev_err(&pdev->dev, "failed to add bus to i2c core\n");
>  		goto err_irq;
> Index: linux-2.6.26-rc4-quilt3/include/asm-arm/plat-s3c/iic.h
> ===================================================================
> --- linux-2.6.26-rc4-quilt3.orig/include/asm-arm/plat-s3c/iic.h	2008-06-02 20:12:47.000000000 +0100
> +++ linux-2.6.26-rc4-quilt3/include/asm-arm/plat-s3c/iic.h	2008-06-02 22:55:29.000000000 +0100
> @@ -21,6 +21,7 @@
>  */
>  
>  struct s3c2410_platform_i2c {
> +	int		bus_num;	/* bus number to use */
>  	unsigned int	flags;
>  	unsigned int	slave_addr;	/* slave address for controller */
>  	unsigned long	bus_freq;	/* standard bus frequency */
> 

Other than these details, the patch looks OK to me.

-- 
Jean Delvare

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

[patch 2/2] I2C: S3C24XX I2C frequency scaling support.

[Ben Dooks]

[-- Attachment #1: simtec/cpufreq/s3c24xx-cpufreq-drivers-i2c.patch --]
[-- Type: text/plain, Size: 6655 bytes --]

Add support for CPU frequency scaling to the S3C24XX I2C driver.

Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>

Index: linux-2.6.26-rc9/drivers/i2c/busses/i2c-s3c2410.c
===================================================================
--- linux-2.6.26-rc9.orig/drivers/i2c/busses/i2c-s3c2410.c	2008-07-09 13:46:08.000000000 +0100
+++ linux-2.6.26-rc9/drivers/i2c/busses/i2c-s3c2410.c	2008-07-09 13:48:46.000000000 +0100
@@ -33,6 +33,7 @@
 #include <linux/err.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
+#include <linux/cpufreq.h>
 
 #include <asm/hardware.h>
 #include <asm/irq.h>
@@ -64,6 +65,7 @@ struct s3c24xx_i2c {
 	unsigned int		tx_setup;
 
 	enum s3c24xx_i2c_state	state;
+	unsigned long		clkrate;
 
 	void __iomem		*regs;
 	struct clk		*clk;
@@ -71,6 +73,10 @@ struct s3c24xx_i2c {
 	struct resource		*irq;
 	struct resource		*ioarea;
 	struct i2c_adapter	adap;
+
+#ifdef CONFIG_CPU_FREQ
+	struct notifier_block	freq_transition;
+#endif
 };
 
 /* default platform data to use if not supplied in the platform_device
@@ -501,6 +507,9 @@ static int s3c24xx_i2c_doxfer(struct s3c
 	unsigned long timeout;
 	int ret;
 
+	if (!readl(i2c->regs + S3C2410_IICCON) & S3C2410_IICCON_IRQEN)
+		return -EIO;
+
 	ret = s3c24xx_i2c_set_master(i2c);
 	if (ret != 0) {
 		dev_err(i2c->dev, "cannot get bus (error %d)\n", ret);
@@ -636,27 +645,28 @@ static inline int freq_acceptable(unsign
 	return (diff >= -2 && diff <= 2);
 }
 
-/* s3c24xx_i2c_getdivisor
+/* s3c24xx_i2c_clockrate
  *
  * work out a divisor for the user requested frequency setting,
  * either by the requested frequency, or scanning the acceptable
  * range of frequencies until something is found
 */
 
-static int s3c24xx_i2c_getdivisor(struct s3c24xx_i2c *i2c,
-				  struct s3c2410_platform_i2c *pdata,
-				  unsigned long *iicon,
-				  unsigned int *got)
+static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
 {
+	struct s3c2410_platform_i2c *pdata;
 	unsigned long clkin = clk_get_rate(i2c->clk);
-	
 	unsigned int divs, div1;
+	u32 iiccon;
 	int freq;
 	int start, end;
 
+	i2c->clkrate = clkin;
+
+	pdata = s3c24xx_i2c_get_platformdata(i2c->adap.dev.parent);
 	clkin /= 1000;		/* clkin now in KHz */
      
-	dev_dbg(i2c->dev,  "pdata %p, freq %lu %lu..%lu\n",
+	dev_dbg(i2c->dev, "pdata %p, freq %lu %lu..%lu\n",
 		 pdata, pdata->bus_freq, pdata->min_freq, pdata->max_freq);
 
 	if (pdata->bus_freq != 0) {
@@ -688,11 +698,79 @@ static int s3c24xx_i2c_getdivisor(struct
 
  found:
 	*got = freq;
-	*iicon |= (divs-1);
-	*iicon |= (div1 == 512) ? S3C2410_IICCON_TXDIV_512 : 0;
+
+	iiccon = readl(i2c->regs + S3C2410_IICCON);
+	iiccon &= ~(S3C2410_IICCON_SCALEMASK | S3C2410_IICCON_TXDIV_512);
+	iiccon |= (divs-1);
+
+	if (div1 == 512)
+		iiccon |= S3C2410_IICCON_TXDIV_512;
+
+	writel(iiccon, i2c->regs + S3C2410_IICCON);
+
+	return 0;
+}
+
+#ifdef CONFIG_CPU_FREQ
+
+#define freq_to_i2c(_n) container_of(_n, struct s3c24xx_i2c, freq_transition)
+
+static int s3c24xx_i2c_cpufreq_transition(struct notifier_block *nb,
+					  unsigned long val, void *data)
+{
+	struct s3c24xx_i2c *i2c = freq_to_i2c(nb);
+	unsigned long clkrate;
+	unsigned long flags;
+	unsigned int got;
+	int ret;
+
+	clkrate = clk_get_rate(i2c->clk);
+
+	/* if we're post-change and the input clock has slowed down
+	 * or at pre-change and the clock is about to speed up, then
+	 * adjust our clock rate.
+	 */
+
+	if ((val == CPUFREQ_POSTCHANGE && clkrate < i2c->clkrate) ||
+	    (val == CPUFREQ_PRECHANGE && clkrate > i2c->clkrate)) {
+		spin_lock_irqsave(&i2c->lock, flags);
+		ret = s3c24xx_i2c_clockrate(i2c, &got);
+		spin_unlock_irqrestore(&i2c->lock, flags);
+
+		if (ret < 0)
+			dev_err(i2c->dev, "cannot find frequency\n");
+		else
+			dev_info(i2c->dev, "setting freq %d\n", got);
+	}
+
 	return 0;
 }
 
+static inline int s3c24xx_i2c_register_cpufreq(struct s3c24xx_i2c *i2c)
+{
+	i2c->freq_transition.notifier_call = s3c24xx_i2c_cpufreq_transition;
+
+	return cpufreq_register_notifier(&i2c->freq_transition,
+					 CPUFREQ_TRANSITION_NOTIFIER);
+}
+
+static inline void s3c24xx_i2c_deregister_cpufreq(struct s3c24xx_i2c *i2c)
+{
+	cpufreq_unregister_notifier(&i2c->freq_transition,
+				    CPUFREQ_TRANSITION_NOTIFIER);
+}
+
+#else
+static inline int s3c24xx_i2c_register_cpufreq(struct s3c24xx_i2c *i2c)
+{
+	return 0;
+}
+
+static inline void s3c24xx_i2c_deregister_cpufreq(struct s3c24xx_i2c *i2c)
+{
+}
+#endif
+
 /* s3c24xx_i2c_init
  *
  * initialise the controller, set the IO lines and frequency 
@@ -719,9 +797,12 @@ static int s3c24xx_i2c_init(struct s3c24
 
 	dev_info(i2c->dev, "slave address 0x%02x\n", pdata->slave_addr);
 
+	writel(iicon, i2c->regs + S3C2410_IICCON);
+
 	/* we need to work out the divisors for the clock... */
 
-	if (s3c24xx_i2c_getdivisor(i2c, pdata, &iicon, &freq) != 0) {
+	if (s3c24xx_i2c_clockrate(i2c, &freq) != 0) {
+		writel(0, i2c->regs + S3C2410_IICCON);
 		dev_err(i2c->dev, "cannot meet bus frequency required\n");
 		return -EINVAL;
 	}
@@ -730,8 +811,6 @@ static int s3c24xx_i2c_init(struct s3c24
 
 	dev_info(i2c->dev, "bus frequency set to %d KHz\n", freq);
 	dev_dbg(i2c->dev, "S3C2410_IICCON=0x%02lx\n", iicon);
-	
-	writel(iicon, i2c->regs + S3C2410_IICCON);
 
 	/* check for s3c2440 i2c controller  */
 
@@ -835,6 +914,12 @@ static int s3c24xx_i2c_probe(struct plat
 	dev_dbg(&pdev->dev, "irq resource %p (%lu)\n", res,
 		(unsigned long)res->start);
 
+	ret = s3c24xx_i2c_register_cpufreq(i2c);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register cpufreq notifier\n");
+		goto err_irq;
+	}
+
 	/* Note, previous versions of the driver used i2c_add_adapter()
 	 * to add an bus at any number. We now pass the bus number via
 	 * the platform data, so if unset it will now default to always
@@ -846,7 +931,7 @@ static int s3c24xx_i2c_probe(struct plat
 
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to add bus to i2c core\n");
-		goto err_irq;
+		goto err_cpufreq;
 	}
 
 	platform_set_drvdata(pdev, i2c);
@@ -854,6 +939,9 @@ static int s3c24xx_i2c_probe(struct plat
 	dev_info(&pdev->dev, "%s: S3C I2C adapter\n", i2c->adap.dev.bus_id);
 	return 0;
 
+ err_cpufreq:
+	s3c24xx_i2c_deregister_cpufreq(i2c);
+
  err_irq:
 	free_irq(i2c->irq->start, i2c);
 
@@ -881,6 +969,8 @@ static int s3c24xx_i2c_remove(struct pla
 {
 	struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
 
+	s3c24xx_i2c_deregister_cpufreq(i2c);
+
 	i2c_del_adapter(&i2c->adap);
 	free_irq(i2c->irq->start, i2c);
 

-- 

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