Skip to contentSkip to conversation

input:Add clk api support for w90p910 touchscreen

PATCH In review Wan ZongShun 4 participants 11 messages Jul 9, 2009 – Jul 11, 2009

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 input:Add clk api support for w90p910 touchscreen

Dear sirs,

According to Trilok Soni's comments,I fixed up my ts patch again.

Thanks a lot for Trilok Soni's help.

Add clk api support for w90p910 touchscreen.

---
 drivers/input/touchscreen/w90p910_ts.c |   27 ++++++++++++++-------------
 1 files changed, 14 insertions(+), 13 deletions(-)
drivers/input/touchscreen/w90p910_ts.c +14 -13
index 6071f58..639117a 100644
@@ -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,7 @@ 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);
 }
 
 static int __devinit w90x900ts_probe(struct platform_device *pdev)
@@ -253,14 +252,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 +280,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);
@@ -311,6 +309,9 @@ static int __devexit w90x900ts_remove(struct platform_device *pdev)
 	del_timer_sync(&w90p910_ts->timer);
 	iounmap(w90p910_ts->ts_reg);
 
+	clk_disable(w90p910_ts->clk);
+	clk_put(w90p910_ts->clk);
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(res->start, res->end - res->start + 1);
 
Signature · 3 lines
1.5.6.3

Wan.
maintainer
#1 lore raw

Hi Wan,

drivers/input/touchscreen/w90p910_ts.c static int __devexit w90x900ts_remove(struct platform_device *pdev) · line 312

Quoted 5 lines · Wan ZongShun
drivers/input/touchscreen/w90p910_ts.c +1 -0
@@ -311,6 +309,9 @@ static int __devexit w90x900ts_remove(struct platform_device *pdev)
 	del_timer_sync(&w90p910_ts->timer);
 	iounmap(w90p910_ts->ts_reg);
 
+	clk_disable(w90p910_ts->clk);

This disable is unneeded since we already disable clock in close(). I will fix locally and apply, there is no need to resubmit. I will also fix pxa27 driver.

Thanks!

Signature · 1 line
Dmitry
#2 lore raw
Quoted 7 lines · Dmitry Torokhov

Hi Wan,

Quoted 4 lines · Wan ZongShun
drivers/input/touchscreen/w90p910_ts.c +0 -0
@@ -311,6 +309,9 @@ static int __devexit w90x900ts_remove(struct platform_device *pdev)
 	del_timer_sync(&w90p910_ts->timer);
 	iounmap(w90p910_ts->ts_reg);
 
Quoted 5 lines

Review static int __devexit w90x900ts_remove(struct platform_device *pdev) · line 312

Quoted 1 line
(quoted diff) +1 -0
+	clk_disable(w90p910_ts->clk);

This disable is unneeded since we already disable clock in close(). I will fix locally and apply, there is no need to resubmit. I will also fix pxa27 driver.

Okay, got it, thanks for your help!

Quoted 2 lines

Thanks!

--
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

maintainer
#3 lore raw
Quoted 8 lines · Wan ZongShun
Quoted 7 lines · Dmitry Torokhov

Hi Wan,

Quoted 4 lines · Wan ZongShun
drivers/input/touchscreen/w90p910_ts.c +0 -0
@@ -311,6 +309,9 @@ static int __devexit w90x900ts_remove(struct platform_device *pdev)
 	del_timer_sync(&w90p910_ts->timer);
 	iounmap(w90p910_ts->ts_reg);
 
Quoted 8 lines
Quoted 5 lines

Review static int __devexit w90x900ts_remove(struct platform_device *pdev) · line 312

Quoted 1 line
(quoted diff) +1 -0
+	clk_disable(w90p910_ts->clk);

This disable is unneeded since we already disable clock in close(). I will fix locally and apply, there is no need to resubmit. I will also fix pxa27 driver.

Okay, got it, thanks for your help!

Umm, one more thing - we need to add "depends on" to Kconfig since clk API is not implemented on every platform. Any ideas what is the most wide dependency is?

Signature · 5 lines
Dmitry
--
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
#4 lore raw

Dear Dmitry,

Please see below.

Dmitry Torokhov:

Quoted 13 lines · Wan ZongShun
Quoted 12 lines
Quoted 11 lines · Dmitry Torokhov

Hi Wan,

drivers/input/touchscreen/w90p910_ts.c static int __devexit w90x900ts_remove(struct platform_device *pdev) · line 312

Quoted 5 lines · Wan ZongShun
drivers/input/touchscreen/w90p910_ts.c +1 -0
@@ -311,6 +309,9 @@ static int __devexit w90x900ts_remove(struct platform_device *pdev)
 	del_timer_sync(&w90p910_ts->timer);
 	iounmap(w90p910_ts->ts_reg);
 
+	clk_disable(w90p910_ts->clk);

This disable is unneeded since we already disable clock in close(). I will fix locally and apply, there is no need to resubmit. I will also fix pxa27 driver.

Quoted 7 lines
Quoted 2 lines

Okay, got it, thanks for your help!

Umm, one more thing - we need to add "depends on" to Kconfig since clk API is not implemented on every platform. Any ideas what is the most wide dependency is?

Do you mean that w90p910 ts have missed this "depends on"? Adding this depends as following:

config TOUCHSCREEN_W90X900
tristate "W90P910 touchscreen driver" depends on ARCH_W90X900
help
Say Y here if you have a W90P910 based touchscreen.

To compile this driver as a module, choose M here: the module will be called w90p910_ts.

Is it okay, now?
--
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

#5 lore raw
In reply to Dmitry Torokhov

Hi Dmitry,

Quoted 15 lines · Dmitry Torokhov
Quoted 14 lines · Wan ZongShun
Quoted 12 lines · Dmitry Torokhov

Hi Wan,

drivers/input/touchscreen/w90p910_ts.c static int __devexit w90x900ts_remove(struct platform_device *pdev) · line 312

Quoted 5 lines · Wan ZongShun
drivers/input/touchscreen/w90p910_ts.c +1 -0
@@ -311,6 +309,9 @@ static int __devexit w90x900ts_remove(struct platform_device *pdev)
 	del_timer_sync(&w90p910_ts->timer);
 	iounmap(w90p910_ts->ts_reg);
 
+	clk_disable(w90p910_ts->clk);

This disable is unneeded since we already disable clock in close(). I will fix locally and apply, there is no need to resubmit. I will also fix pxa27 driver.

Quoted 6 lines
Quoted 2 lines

Okay, got it, thanks for your help!

Umm, one more thing - we need to add "depends on" to Kconfig since clk API is not implemented on every platform. Any ideas what is the most wide dependency is?

The most wide dependency for the clk API is HAVE_CLK.

baruch

Signature · 8 lines
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php
maintainer
#6 lore raw
Quoted 18 lines · Baruch Siach

Hi Dmitry,

Quoted 15 lines · Dmitry Torokhov
Quoted 14 lines · Wan ZongShun
Quoted 12 lines · Dmitry Torokhov

Hi Wan,

drivers/input/touchscreen/w90p910_ts.c static int __devexit w90x900ts_remove(struct platform_device *pdev) · line 312

Quoted 5 lines · Wan ZongShun
drivers/input/touchscreen/w90p910_ts.c +1 -0
@@ -311,6 +309,9 @@ static int __devexit w90x900ts_remove(struct platform_device *pdev)
 	del_timer_sync(&w90p910_ts->timer);
 	iounmap(w90p910_ts->ts_reg);
 
+	clk_disable(w90p910_ts->clk);

This disable is unneeded since we already disable clock in close(). I will fix locally and apply, there is no need to resubmit. I will also fix pxa27 driver.

Quoted 9 lines
Quoted 6 lines
Quoted 2 lines

Okay, got it, thanks for your help!

Umm, one more thing - we need to add "depends on" to Kconfig since clk API is not implemented on every platform. Any ideas what is the most wide dependency is?

The most wide dependency for the clk API is HAVE_CLK.

Cool, thanks.

Signature · 5 lines
Dmitry
--
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
#7 lore raw

Dear Dmitry,

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Umm, one more thing - we need to add "depends on" to Kconfig since clk API is not implemented on every platform. Any ideas what is the most wide dependency is?

The most wide dependency for the clk API is HAVE_CLK.

Cool, thanks.

Sure, but why to add it to touchscreen's kconfig?
it seems it should be put arch/arm/kconfig.

In addition,I never use this HAVE_CLK in ARCH_W90X900, only using COMMON_CLKDEV to support clk API.

#8 lore raw
Quoted 8 lines · Wan ZongShun

Dear Dmitry,

Quoted 6 lines
Quoted 5 lines
Quoted 3 lines

Umm, one more thing - we need to add "depends on" to Kconfig since clk API is not implemented on every platform. Any ideas what is the most wide dependency is?

The most wide dependency for the clk API is HAVE_CLK.

Quoted 7 lines
Quoted 2 lines

Cool, thanks.

Sure, but why to add it to touchscreen's kconfig?
it seems it should be put arch/arm/kconfig.

In addition,I never use this HAVE_CLK in ARCH_W90X900, only using COMMON_CLKDEV to support clk API.

HAVE_CLK is the configuration symbol which tells the rest of the kernel configuration system that the clk API is available. Drivers using the clk API should depend on HAVE_CLK.

COMMON_CLKDEV is just one implementation of a subset of the clk API, and nothing should ever depend on this symbol.

#9 lore raw

Dear Russell,

Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

Umm, one more thing - we need to add "depends on" to Kconfig since clk API is not implemented on every platform. Any ideas what is the most wide dependency is?

The most wide dependency for the clk API is HAVE_CLK.

Quoted 7 lines
Quoted 2 lines

Cool, thanks.

Sure, but why to add it to touchscreen's kconfig?
it seems it should be put arch/arm/kconfig.

In addition,I never use this HAVE_CLK in ARCH_W90X900, only using COMMON_CLKDEV to support clk API.

Quoted 7 lines

HAVE_CLK is the configuration symbol which tells the rest of the kernel configuration system that the clk API is available. Drivers using the clk API should depend on HAVE_CLK.

COMMON_CLKDEV is just one implementation of a subset of the clk API, and nothing should ever depend on this symbol.

Maybe I should add this "select HAVE_CLK" to arch/arm/Kconfig too, as following:

config ARCH_W90X900

	bool "Nuvoton W90X900 CPU"
	select CPU_ARM926T
	select ARCH_REQUIRE_GPIOLIB
	select GENERIC_GPIO
	select COMMON_CLKDEV
	select HAVE_CLK

Is it right?

-------------------------------------------------------------------

List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php
#10 lore raw
Quoted 9 lines · Wan ZongShun

config ARCH_W90X900

	bool "Nuvoton W90X900 CPU"
	select CPU_ARM926T
	select ARCH_REQUIRE_GPIOLIB
	select GENERIC_GPIO
	select COMMON_CLKDEV
	select HAVE_CLK

Is it right?

Yes.