input:Add clk api support for w90p910 touchscreen
Subsystems (2)
PATCH 1/1 input:Add clk api support for w90p910 touchscreen
Dear Dmitry,
Now,the w90p910 clock API has been submitted to "linux-arm-kernel",so using it for w90p910 touchscreen driver.Thanks!
Add clk api support for w90p910 touchscreen.
--- drivers/input/touchscreen/w90p910_ts.c | 22 ++++++++++++---------- 1 files changed, 12 insertions(+), 10 deletions(-)
drivers/input/touchscreen/w90p910_ts.c +12 -10
@@ -13,6 +13,7 @@#include <linux/module.h> #include <linux/platform_device.h> #include <linux/io.h> +#include <linux/clk.h> #include <linux/input.h> #include <linux/interrupt.h>
@@ -47,6 +48,7 @@ enum ts_state {struct w90p910_ts {struct input_dev *input;struct timer_list timer; +struct clk *clk;int irq_num;void __iomem *clocken;void __iomem *ts_reg;
@@ -166,8 +168,7 @@ static int w90p910_open(struct input_dev *dev)unsigned long val;/* enable the ADC clock */ - val =__raw_readl (w90p910_ts->clocken); -__raw_writel (val |ADC_CLK_EN , w90p910_ts->clocken); +clk_enable (w90p910_ts->clk);__raw_writel (ADC_RST1 , w90p910_ts->ts_reg);msleep (1 );
@@ -211,8 +212,8 @@ static void w90p910_close(struct input_dev *dev)del_timer_sync (&w90p910_ts->timer);/* stop the ADC clock */ - val =__raw_readl (w90p910_ts->clocken); -__raw_writel (val & ~ADC_CLK_EN , w90p910_ts->clocken); +clk_disable (w90p910_ts->clk); +clk_put (w90p910_ts->clk); }static int __devinit w90x900ts_probe (struct platform_device *pdev)
@@ -253,9 +254,9 @@ static int __devinit w90x900ts_probe(struct platform_device *pdev) goto fail2; } - res =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); -if (!res) { - err = -ENXIO ; + w90p910_ts->clk =clk_get (&pdev->dev, pdev->name); +if (IS_ERR (w90p910_ts->clk)) { + err =PTR_ERR (w90p910_ts->clk); goto fail3; }
@@ -283,18 +284,19 @@ static int __devinit w90x900ts_probe(struct platform_device *pdev)if (request_irq (w90p910_ts->irq_num, w90p910_ts_interrupt,IRQF_DISABLED ,"w90p910ts" , w90p910_ts)) { err = -EBUSY ; - goto fail3; + goto fail4; } err =input_register_device (w90p910_ts->input);if (err) - goto fail4; + goto fail5;platform_set_drvdata (pdev, w90p910_ts);return 0 ; -fail4:free_irq (w90p910_ts->irq_num, w90p910_ts); +fail5:free_irq (w90p910_ts->irq_num, w90p910_ts); +fail4:clk_put (w90p910_ts->clk); fail3:iounmap (w90p910_ts->ts_reg); fail2:release_mem_region (res->start, res->end - res->start +1 ); fail1:input_free_device (input_dev);
Signature · 1 line
1.5.6.3
Hi,
Quoted 1 line
+
w90p910_ts->clk=clk_get(&pdev->dev, pdev->name);
Having clk_name coming from pdev->name looks strange to me. I feel it should either "NULL" or const string.
Quoted 5 lines
+ if (
IS_ERR(w90p910_ts->clk)) {
+ err =PTR_ERR(w90p910_ts->clk);
goto fail3;
}
Signature · 7 lines
---Trilok Soni http://triloksoni.wordpress.com http://www.linkedin.com/in/triloksoni -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Trilok,
You are right, here should be put "NULL",thanks for your help. Now, I fixed up my patch.
Add clk api support for w90p910 touchscreen.
--- drivers/input/touchscreen/w90p910_ts.c | 22 ++++++++++++---------- 1 files changed, 12 insertions(+), 10 deletions(-)
drivers/input/touchscreen/w90p910_ts.c +12 -10
@@ -13,6 +13,7 @@#include <linux/module.h> #include <linux/platform_device.h> #include <linux/io.h> +#include <linux/clk.h> #include <linux/input.h> #include <linux/interrupt.h>
@@ -47,6 +48,7 @@ enum ts_state {struct w90p910_ts {struct input_dev *input;struct timer_list timer; +struct clk *clk;int irq_num;void __iomem *clocken;void __iomem *ts_reg;
@@ -166,8 +168,7 @@ static int w90p910_open(struct input_dev *dev)unsigned long val;/* enable the ADC clock */ - val =__raw_readl (w90p910_ts->clocken); -__raw_writel (val |ADC_CLK_EN , w90p910_ts->clocken); +clk_enable (w90p910_ts->clk);__raw_writel (ADC_RST1 , w90p910_ts->ts_reg);msleep (1 );
@@ -211,8 +212,8 @@ static void w90p910_close(struct input_dev *dev)del_timer_sync (&w90p910_ts->timer);/* stop the ADC clock */ - val =__raw_readl (w90p910_ts->clocken); -__raw_writel (val & ~ADC_CLK_EN , w90p910_ts->clocken); +clk_disable (w90p910_ts->clk); +clk_put (w90p910_ts->clk); }static int __devinit w90x900ts_probe (struct platform_device *pdev)
@@ -253,9 +254,9 @@ static int __devinit w90x900ts_probe(struct platform_device *pdev) goto fail2; } - res =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); -if (!res) { - err = -ENXIO ; + w90p910_ts->clk =clk_get (&pdev->dev,NULL ); +if (IS_ERR (w90p910_ts->clk)) { + err =PTR_ERR (w90p910_ts->clk); goto fail3; }
@@ -283,18 +284,19 @@ static int __devinit w90x900ts_probe(struct platform_device *pdev)if (request_irq (w90p910_ts->irq_num, w90p910_ts_interrupt,IRQF_DISABLED ,"w90p910ts" , w90p910_ts)) { err = -EBUSY ; - goto fail3; + goto fail4; } err =input_register_device (w90p910_ts->input);if (err) - goto fail4; + goto fail5;platform_set_drvdata (pdev, w90p910_ts);return 0 ; -fail4:free_irq (w90p910_ts->irq_num, w90p910_ts); +fail5:free_irq (w90p910_ts->irq_num, w90p910_ts); +fail4:clk_put (w90p910_ts->clk); fail3:iounmap (w90p910_ts->ts_reg); fail2:release_mem_region (res->start, res->end - res->start +1 ); fail1:input_free_device (input_dev);
Signature · 4 lines
1.5.6.3 B&R Wan.
Dmitry Torokhov
maintainerQuoted 5 lines · Wan ZongShun
Dear Trilok,
You are right, here should be put "NULL",thanks for your help. Now, I fixed up my patch.
It looks like clocken can be removed as well?
Signature · 1 line
Dmitry
Dear Dmitry,
Okay, this clocken should be removed here.
I fixed up my patch and re-submitted it.
thanks a lot for your help.
Add clk api support for w90p910 touchscreen.
--- drivers/input/touchscreen/w90p910_ts.c | 25 ++++++++++++------------- 1 files changed, 12 insertions(+), 13 deletions(-)
drivers/input/touchscreen/w90p910_ts.c +12 -13
@@ -13,6 +13,7 @@#include <linux/module.h> #include <linux/platform_device.h> #include <linux/io.h> +#include <linux/clk.h> #include <linux/input.h> #include <linux/interrupt.h>
@@ -47,8 +48,8 @@ enum ts_state {struct w90p910_ts {struct input_dev *input;struct timer_list timer; +struct clk *clk;int irq_num; -void __iomem *clocken;void __iomem *ts_reg;spinlock_t lock;enum ts_state state;
@@ -166,8 +167,7 @@ static int w90p910_open(struct input_dev *dev)unsigned long val;/* enable the ADC clock */ - val =__raw_readl (w90p910_ts->clocken); -__raw_writel (val |ADC_CLK_EN , w90p910_ts->clocken); +clk_enable (w90p910_ts->clk);__raw_writel (ADC_RST1 , w90p910_ts->ts_reg);msleep (1 );
@@ -211,8 +211,8 @@ static void w90p910_close(struct input_dev *dev)del_timer_sync (&w90p910_ts->timer);/* stop the ADC clock */ - val =__raw_readl (w90p910_ts->clocken); -__raw_writel (val & ~ADC_CLK_EN , w90p910_ts->clocken); +clk_disable (w90p910_ts->clk); +clk_put (w90p910_ts->clk); }static int __devinit w90x900ts_probe (struct platform_device *pdev)
@@ -253,14 +253,12 @@ static int __devinit w90x900ts_probe(struct platform_device *pdev) goto fail2; } - res =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); -if (!res) { - err = -ENXIO ; + w90p910_ts->clk =clk_get (&pdev->dev,NULL ); +if (IS_ERR (w90p910_ts->clk)) { + err =PTR_ERR (w90p910_ts->clk); goto fail3; } - w90p910_ts->clocken = (void __iomem *)res->start; - input_dev->name ="W90P910 TouchScreen" ; input_dev->phys ="w90p910ts/event0" ; input_dev->id.bustype =BUS_HOST ;
@@ -283,18 +281,19 @@ static int __devinit w90x900ts_probe(struct platform_device *pdev)if (request_irq (w90p910_ts->irq_num, w90p910_ts_interrupt,IRQF_DISABLED ,"w90p910ts" , w90p910_ts)) { err = -EBUSY ; - goto fail3; + goto fail4; } err =input_register_device (w90p910_ts->input);if (err) - goto fail4; + goto fail5;platform_set_drvdata (pdev, w90p910_ts);return 0 ; -fail4:free_irq (w90p910_ts->irq_num, w90p910_ts); +fail5:free_irq (w90p910_ts->irq_num, w90p910_ts); +fail4:clk_put (w90p910_ts->clk); fail3:iounmap (w90p910_ts->ts_reg); fail2:release_mem_region (res->start, res->end - res->start +1 ); fail1:input_free_device (input_dev);
Signature · 1 line
1.5.6.3
Hi Wan ZongShun,
Quoted 38 lines
---
drivers/input/touchscreen/w90p910_ts.c | 25 ++++++++++++-------------1 files changed, 12 insertions(+), 13 deletions(-)drivers/input/touchscreen/w90p910_ts.c +3
-3@@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/io.h> +#include <linux/clk.h> #include <linux/input.h> #include <linux/interrupt.h>@@ -47,8 +48,8 @@ enum ts_state { struct w90p910_ts { struct input_dev *input; struct timer_list timer; + struct clk *clk; int irq_num; - void __iomem *clocken; void __iomem *ts_reg; spinlock_t lock; enum ts_state state;@@ -166,8 +167,7 @@ static int w90p910_open(struct input_dev *dev) unsigned long val; /* enable the ADC clock */ - val = __raw_readl(w90p910_ts->clocken); - __raw_writel(val | ADC_CLK_EN, w90p910_ts->clocken); + clk_enable(w90p910_ts->clk); __raw_writel(ADC_RST1, w90p910_ts->ts_reg); msleep(1);
drivers/input/touchscreen/w90p910_ts.c static void w90p910_close(struct input_dev *dev) · line 215
Quoted 7 lines
drivers/input/touchscreen/w90p910_ts.c +2
-2@@ -211,8 +211,8 @@ static void w90p910_close(struct input_dev *dev) del_timer_sync(&w90p910_ts->timer); /* stop the ADC clock */ - val = __raw_readl(w90p910_ts->clocken); - __raw_writel(val & ~ADC_CLK_EN, w90p910_ts->clocken); + clk_disable(w90p910_ts->clk); + clk_put(w90p910_ts->clk);
why you are doing clk_put on _close? I think you are calling clk_get only on _probe, so there is a mismatch here. Also I don't see clk_disable/clk_put in driver remove function.
Signature · 7 lines
---Trilok Soni http://triloksoni.wordpress.com http://www.linkedin.com/in/triloksoni -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Trilok ，
2009/7/8 Trilok Soni <[email protected]>:
Quoted 42 lines
Hi Wan ZongShun,
Quoted 40 lines
---
drivers/input/touchscreen/w90p910_ts.c | 25 ++++++++++++-------------1 files changed, 12 insertions(+), 13 deletions(-)drivers/input/touchscreen/w90p910_ts.c +3
-3@@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/io.h> +#include <linux/clk.h> #include <linux/input.h> #include <linux/interrupt.h>@@ -47,8 +48,8 @@ enum ts_state { struct w90p910_ts { struct input_dev *input; struct timer_list timer; + struct clk *clk; int irq_num; - void __iomem *clocken; void __iomem *ts_reg; spinlock_t lock; enum ts_state state;@@ -166,8 +167,7 @@ static int w90p910_open(struct input_dev *dev) unsigned long val; /* enable the ADC clock */ - val = __raw_readl(w90p910_ts->clocken); - __raw_writel(val | ADC_CLK_EN, w90p910_ts->clocken); + clk_enable(w90p910_ts->clk); __raw_writel(ADC_RST1, w90p910_ts->ts_reg); msleep(1);@@ -211,8 +211,8 @@ static void w90p910_close(struct input_dev *dev) del_timer_sync(&w90p910_ts->timer);
Quoted 10 lines
drivers/input/touchscreen/w90p910_ts.c static void w90p910_close(struct input_dev *dev) · line 215
Quoted 5 lines
drivers/input/touchscreen/w90p910_ts.c +2
-2/* stop the ADC clock */ - val = __raw_readl(w90p910_ts->clocken); - __raw_writel(val & ~ADC_CLK_EN, w90p910_ts->clocken); + clk_disable(w90p910_ts->clk); + clk_put(w90p910_ts->clk);why you are doing
clk_puton_close? I think you are callingclk_getonly on_probe, so there is a mismatch here. Also I don't see clk_disable/clk_put in driver remove function.
Just imitate the method from pxa27x_keypad.c.
Do you mean there is no need to free the clock source here?
Quoted 5 lines
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
Signature · 5 lines
Wan z.s -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Wan ZongShun,
Quoted 57 lines · Wan ZongShun
Dear Trilok ，
2009/7/8 Trilok Soni <[email protected]>:
Quoted 42 lines
Hi Wan ZongShun,
Quoted 40 lines
---
drivers/input/touchscreen/w90p910_ts.c | 25 ++++++++++++-------------1 files changed, 12 insertions(+), 13 deletions(-)drivers/input/touchscreen/w90p910_ts.c +3
-3@@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/io.h> +#include <linux/clk.h> #include <linux/input.h> #include <linux/interrupt.h>@@ -47,8 +48,8 @@ enum ts_state { struct w90p910_ts { struct input_dev *input; struct timer_list timer; + struct clk *clk; int irq_num; - void __iomem *clocken; void __iomem *ts_reg; spinlock_t lock; enum ts_state state;@@ -166,8 +167,7 @@ static int w90p910_open(struct input_dev *dev) unsigned long val; /* enable the ADC clock */ - val = __raw_readl(w90p910_ts->clocken); - __raw_writel(val | ADC_CLK_EN, w90p910_ts->clocken); + clk_enable(w90p910_ts->clk); __raw_writel(ADC_RST1, w90p910_ts->ts_reg); msleep(1);@@ -211,8 +211,8 @@ static void w90p910_close(struct input_dev *dev) del_timer_sync(&w90p910_ts->timer);Quoted 10 lines
drivers/input/touchscreen/w90p910_ts.c static void w90p910_close(struct input_dev *dev) · line 215
Quoted 5 lines
drivers/input/touchscreen/w90p910_ts.c +2
-2/* stop the ADC clock */ - val = __raw_readl(w90p910_ts->clocken); - __raw_writel(val & ~ADC_CLK_EN, w90p910_ts->clocken); + clk_disable(w90p910_ts->clk); + clk_put(w90p910_ts->clk);why you are doing
clk_puton_close? I think you are callingclk_getonly on_probe, so there is a mismatch here. Also I don't see clk_disable/clk_put in driver remove function.
Quoted 3 lines
Just imitate the method from
pxa27x_keypad.c.
Do you mean there is no need to free the clock source here?
I don't see clk_put in pxa27x_keypad.c _close function, which driver you are referring too?
clk_put is not needed in _close function. You should instead update _remove method of your driver with clk_disable + clk_put.
Signature · 7 lines
---Trilok Soni http://triloksoni.wordpress.com http://www.linkedin.com/in/triloksoni -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Trilok,
2009/7/8 Trilok Soni <[email protected]>:
Quoted 60 lines
Hi Wan ZongShun,
Quoted 57 lines · Wan ZongShun
Dear Trilok ，
2009/7/8 Trilok Soni <[email protected]>:
Quoted 52 lines
Hi Wan ZongShun,
Quoted 38 lines
---
drivers/input/touchscreen/w90p910_ts.c | 25 ++++++++++++-------------1 files changed, 12 insertions(+), 13 deletions(-)drivers/input/touchscreen/w90p910_ts.c +3
-3@@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/io.h> +#include <linux/clk.h> #include <linux/input.h> #include <linux/interrupt.h>@@ -47,8 +48,8 @@ enum ts_state { struct w90p910_ts { struct input_dev *input; struct timer_list timer; + struct clk *clk; int irq_num; - void __iomem *clocken; void __iomem *ts_reg; spinlock_t lock; enum ts_state state;@@ -166,8 +167,7 @@ static int w90p910_open(struct input_dev *dev) unsigned long val; /* enable the ADC clock */ - val = __raw_readl(w90p910_ts->clocken); - __raw_writel(val | ADC_CLK_EN, w90p910_ts->clocken); + clk_enable(w90p910_ts->clk); __raw_writel(ADC_RST1, w90p910_ts->ts_reg); msleep(1);drivers/input/touchscreen/w90p910_ts.c static void w90p910_close(struct input_dev *dev) · line 215
Quoted 7 lines
drivers/input/touchscreen/w90p910_ts.c +2
-2@@ -211,8 +211,8 @@ static void w90p910_close(struct input_dev *dev) del_timer_sync(&w90p910_ts->timer); /* stop the ADC clock */ - val = __raw_readl(w90p910_ts->clocken); - __raw_writel(val & ~ADC_CLK_EN, w90p910_ts->clocken); + clk_disable(w90p910_ts->clk); + clk_put(w90p910_ts->clk);why you are doing
clk_puton_close? I think you are callingclk_getonly on_probe, so there is a mismatch here. Also I don't see clk_disable/clk_put in driver remove function.
Quoted 6 lines
Quoted 3 lines
Just imitate the method from
pxa27x_keypad.c.
Do you mean there is no need to free the clock source here?I don't see
clk_putinpxa27x_keypad.c_closefunction, which driver you are referring too?
Okay, got it ,I should put clk_disable + clk_put in _remove rather than in _close.
thanks for your help.
Quoted 10 lines
clk_putis not needed in_closefunction. You should instead update_removemethod of your driver withclk_disable+clk_put.--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
Signature · 5 lines
Wan z.s -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html