h1940 series

PATCH New Vasily Khoruzhick 3 participants 22 messages Aug 19, 2010 – Sep 2, 2010

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm Samsung Exynos

lore

Share & tools

lore

Vasily Khoruzhick

author

2010-08-19 15:00 UTC #0 lore raw

cover h1940 series

This patch series replaces h1940-specific latch-related functions with gpiolib, fixes compilation error for h1940-bluetooth and adds PM support for mmc driver.

Vasily Khoruzhick

author

2010-08-19 15:00 UTC #1 lore raw

PATCH 1/3 h1940: use gpiolib for latch access

This patch adds gpiolib support for h1940 latch. With this patch it's possible to use leds-gpio and uda1380 drivers (they require gpiolib support for appropriate pins). And now it's possible to drop leds-h1940 driver.

Signed-off-by Vasily Khoruzhick <[email protected]>

---
 arch/arm/mach-s3c2410/h1940-bluetooth.c          |   13 +++-
 arch/arm/mach-s3c2410/include/mach/h1940-latch.h |   57 +++++-----------
 arch/arm/mach-s3c2410/mach-h1940.c               |   77 ++++++++++++++++++++--
 arch/arm/plat-s3c24xx/Kconfig                    |    1 +
 4 files changed, 100 insertions(+), 48 deletions(-)

arch/arm/mach-s3c2410/h1940-bluetooth.c +10 -3

index 8cdeb14..8aa2f19 100644
@@ -30,7 +30,7 @@ static void h1940bt_enable(int on)
 {
 	if (on) {
 		/* Power on the chip */
-		h1940_latch_control(0, H1940_LATCH_BLUETOOTH_POWER);
+		gpio_set_value(H1940_LATCH_BLUETOOTH_POWER, 1);
 		/* Reset the chip */
 		mdelay(10);

@@ -43,7 +43,7 @@ static void h1940bt_enable(int on)
 		mdelay(10);
 		gpio_set_value(S3C2410_GPH(1), 0);
 		mdelay(10);
-		h1940_latch_control(H1940_LATCH_BLUETOOTH_POWER, 0);
+		gpio_set_value(H1940_LATCH_BLUETOOTH_POWER, 0);
 	}
 }

@@ -64,7 +64,14 @@ static int __devinit h1940bt_probe(struct platform_device *pdev)
 
 	ret = gpio_request(S3C2410_GPH(1), dev_name(&pdev->dev));
 	if (ret) {
-		dev_err(&pdev->dev, "could not get GPH1\n");\
+		dev_err(&pdev->dev, "could not get GPH1\n");
+		return ret;
+	}
+
+	ret = gpio_request(H1940_LATCH_BLUETOOTH_POWER, dev_name(&pdev->dev));
+	if (ret) {
+		gpio_free(S3C2410_GPH(1));
+		dev_err(&pdev->dev, "could not get BT_POWER\n");
 		return ret;
 	}

arch/arm/mach-s3c2410/include/mach/h1940-latch.h +18 ~~-39~~

69 lines not on this page · raw message

arch/arm/mach-s3c2410/mach-h1940.c +71 -6

124 lines not on this page · raw message

arch/arm/plat-s3c24xx/Kconfig +1 -0

index 984bf66..5a27b1b 100644
@@ -69,6 +69,7 @@ config S3C24XX_GPIO_EXTRA
 	int
 	default 128 if S3C24XX_GPIO_EXTRA128
 	default 64 if S3C24XX_GPIO_EXTRA64
+	default 16 if ARCH_H1940
 	default 0
 
 config S3C24XX_GPIO_EXTRA64

Signature · 1 line

1.7.2

Marek Vasut

2010-08-24 13:49 UTC #2 lore raw

PATCH 1/3 h1940: use gpiolib for latch access

Dne ?t 19. srpna 2010 17:00:01 Vasily Khoruzhick napsal(a):

Quoted 37 lines

Signed-off-by Vasily Khoruzhick <[email protected]>

---
 arch/arm/mach-s3c2410/h1940-bluetooth.c          |   13 +++-
 arch/arm/mach-s3c2410/include/mach/h1940-latch.h |   57 +++++-----------
 arch/arm/mach-s3c2410/mach-h1940.c               |   77
++++++++++++++++++++-- arch/arm/plat-s3c24xx/Kconfig                    | 
  1 +
 4 files changed, 100 insertions(+), 48 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-s3c2410/h1940-bluetooth.c index 8cdeb14..8aa2f19 100644

arch/arm/mach-s3c2410/h1940-bluetooth.c +2 -2

@@ -30,7 +30,7 @@ static void h1940bt_enable(int on)
 {
 	if (on) {
 		/* Power on the chip */
-		h1940_latch_control(0, H1940_LATCH_BLUETOOTH_POWER);
+		gpio_set_value(H1940_LATCH_BLUETOOTH_POWER, 1);
 		/* Reset the chip */
 		mdelay(10);

@@ -43,7 +43,7 @@ static void h1940bt_enable(int on)
 		mdelay(10);
 		gpio_set_value(S3C2410_GPH(1), 0);
 		mdelay(10);
-		h1940_latch_control(H1940_LATCH_BLUETOOTH_POWER, 0);
+		gpio_set_value(H1940_LATCH_BLUETOOTH_POWER, 0);
 	}
 }

arch/arm/mach-s3c2410/h1940-bluetooth.c static int __devinit h1940bt_probe(struct · line 72

Quoted 10 lines

arch/arm/mach-s3c2410/h1940-bluetooth.c +5 -1

@@ -64,7 +64,14 @@ static int __devinit h1940bt_probe(struct
 platform_device *pdev)
 
 	ret = gpio_request(S3C2410_GPH(1), dev_name(&pdev->dev));
 	if (ret) {
-		dev_err(&pdev->dev, "could not get GPH1\n");\
+		dev_err(&pdev->dev, "could not get GPH1\n");
+		return ret;
+	}
+
+	ret = gpio_request(H1940_LATCH_BLUETOOTH_POWER, dev_name(&pdev->dev));

This should contain the name of the GPIO, not dev_name I assume.

arch/arm/mach-s3c2410/h1940-bluetooth.c static int __devinit h1940bt_probe(struct platform_device *pdev) · line 73

Quoted 2 lines

arch/arm/mach-s3c2410/h1940-bluetooth.c +2 -0
+	if (ret) {
+		gpio_free(S3C2410_GPH(1));

What's this constant (the 1) here ? Maybe some sane #define wont hurt or comment around it.

Quoted 21 lines

arch/arm/mach-s3c2410/h1940-bluetooth.c +1 -0
+		dev_err(&pdev->dev, "could not get BT_POWER\n");
 		return ret;
 	}
(quoted diff) +0 -0
b/arch/arm/mach-s3c2410/include/mach/h1940-latch.h index d8a8327..73586f2 100644
arch/arm/mach-s3c2410/include/mach/h1940-latch.h +1 -6
@@ -14,51 +14,30 @@
 #ifndef __ASM_ARCH_H1940_LATCH_H
 #define __ASM_ARCH_H1940_LATCH_H
 
+#include <mach/gpio.h>
 
-#ifndef __ASSEMBLY__
-#define H1940_LATCH		((void __force __iomem *)0xF8000000)
-#else
-#define H1940_LATCH		0xF8000000
-#endif
-

arch/arm/mach-s3c2410/include/mach/h1940-latch.h line 32

Quoted 10 lines

arch/arm/mach-s3c2410/include/mach/h1940-latch.h +1 -6

-#define H1940_PA_LATCH		(S3C2410_CS2)
+#define H1940_LATCH_GPIO(x)		(S3C_GPIO_END + (x))
 
 /* SD layer latch */
 
-#define H1940_LATCH_SDQ1		(1<<16)
-#define H1940_LATCH_LCD_P1		(1<<17)
-#define H1940_LATCH_LCD_P2		(1<<18)
-#define H1940_LATCH_LCD_P3		(1<<19)
-#define H1940_LATCH_MAX1698_nSHUTDOWN	(1<<20)		/* LCD backlight

Quoted 61 lines

arch/arm/mach-s3c2410/include/mach/h1940-latch.h +16 ~~-27~~

-#define H1940_LATCH_LED_RED		(1<<21)
-#define H1940_LATCH_SDQ7		(1<<22)
-#define H1940_LATCH_USB_DP		(1<<23)
+#define H1940_LATCH_SDQ1		H1940_LATCH_GPIO(0)
+#define H1940_LATCH_LCD_P1		H1940_LATCH_GPIO(1)
+#define H1940_LATCH_LCD_P2		H1940_LATCH_GPIO(2)
+#define H1940_LATCH_LCD_P3		H1940_LATCH_GPIO(3)
+#define H1940_LATCH_MAX1698_nSHUTDOWN	H1940_LATCH_GPIO(4)
+#define H1940_LATCH_LED_RED		H1940_LATCH_GPIO(5)
+#define H1940_LATCH_SDQ7		H1940_LATCH_GPIO(6)
+#define H1940_LATCH_USB_DP		H1940_LATCH_GPIO(7)
 
 /* CPU layer latch */
 
-#define H1940_LATCH_UDA_POWER		(1<<24)
-#define H1940_LATCH_AUDIO_POWER		(1<<25)
-#define H1940_LATCH_SM803_ENABLE	(1<<26)
-#define H1940_LATCH_LCD_P4		(1<<27)
-#define H1940_LATCH_CPUQ5		(1<<28)		/* untraced */
-#define H1940_LATCH_BLUETOOTH_POWER	(1<<29)		/* active high */
-#define H1940_LATCH_LED_GREEN		(1<<30)
-#define H1940_LATCH_LED_FLASH		(1<<31)
-
-/* default settings */
-
-#define H1940_LATCH_DEFAULT		\
-	H1940_LATCH_LCD_P4		| \
-	H1940_LATCH_SM803_ENABLE	| \
-	H1940_LATCH_SDQ1		| \
-	H1940_LATCH_LCD_P1		| \
-	H1940_LATCH_LCD_P2		| \
-	H1940_LATCH_LCD_P3		| \
-	H1940_LATCH_MAX1698_nSHUTDOWN   | \
-	H1940_LATCH_CPUQ5
-
-/* control functions */
-
-extern void h1940_latch_control(unsigned int clear, unsigned int set);
+#define H1940_LATCH_UDA_POWER		H1940_LATCH_GPIO(8)
+#define H1940_LATCH_AUDIO_POWER		H1940_LATCH_GPIO(9)
+#define H1940_LATCH_SM803_ENABLE	H1940_LATCH_GPIO(10)
+#define H1940_LATCH_LCD_P4		H1940_LATCH_GPIO(11)
+#define H1940_LATCH_CPUQ5		H1940_LATCH_GPIO(12)
+#define H1940_LATCH_BLUETOOTH_POWER	H1940_LATCH_GPIO(13)
+#define H1940_LATCH_LED_GREEN		H1940_LATCH_GPIO(14)
+#define H1940_LATCH_LED_FLASH		H1940_LATCH_GPIO(15)
 
 #endif /* __ASM_ARCH_H1940_LATCH_H */

(quoted diff) +0 -0

b/arch/arm/mach-s3c2410/mach-h1940.c index 779b45b..9717790 100644

arch/arm/mach-s3c2410/mach-h1940.c +1 -0

@@ -42,6 +42,7 @@
 #include <mach/regs-gpio.h>
 #include <mach/gpio-fns.h>
 #include <mach/gpio-nrs.h>
+#include <mach/gpio.h>
 
 #include <mach/h1940.h>
 #include <mach/h1940-latch.h>

arch/arm/mach-s3c2410/mach-h1940.c line 66

Quoted 8 lines

arch/arm/mach-s3c2410/mach-h1940.c +5 -0

@@ -58,6 +59,16 @@
 #include <plat/mci.h>
 #include <plat/ts.h>
 
+#ifndef __ASSEMBLY__
+#define H1940_LATCH		((void __force __iomem *)0xF8000000)
+#else
+#define H1940_LATCH		0xF8000000
+#endif

Is the __ASSEMBLY__ really needed ? You can establish mapping when the kernel boots (looks like you're doing that already) but then use __raw_readX __raw_writeX to access that space instead of this stuff above.

Possibly like this:

#define H1940_VA_LATCH 0xf8000000

some_fn()
{
...

	val = __raw_readl(H1940_VA_LATCH + offset);

...
}

Cheers

Quoted 127 lines

arch/arm/mach-s3c2410/mach-h1940.c +64 -6

+
+#define H1940_PA_LATCH		(S3C2410_CS2)
+
+#define H1940_LATCH_BIT(x)		(1 << ((x) + 16 - S3C_GPIO_END))
+
 static struct map_desc h1940_iodesc[] __initdata = {
 	[0] = {
 		.virtual	= (unsigned long)H1940_LATCH,

@@ -99,9 +110,16 @@ static struct s3c2410_uartcfg h1940_uartcfgs[]
 __initdata = {
 
 /* Board control latch control */
 
-static unsigned int latch_state = H1940_LATCH_DEFAULT;
+static unsigned int latch_state = H1940_LATCH_BIT(H1940_LATCH_LCD_P4) |
+	H1940_LATCH_BIT(H1940_LATCH_SM803_ENABLE)	|
+	H1940_LATCH_BIT(H1940_LATCH_SDQ1)			|
+	H1940_LATCH_BIT(H1940_LATCH_LCD_P1)			|
+	H1940_LATCH_BIT(H1940_LATCH_LCD_P2)			|
+	H1940_LATCH_BIT(H1940_LATCH_LCD_P3)			|
+	H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN)   |
+	H1940_LATCH_BIT(H1940_LATCH_CPUQ5);
 
-void h1940_latch_control(unsigned int clear, unsigned int set)
+static void h1940_latch_control(unsigned int clear, unsigned int set)
 {
 	unsigned long flags;

@@ -115,7 +133,50 @@ void h1940_latch_control(unsigned int clear, unsigned
 int set) local_irq_restore(flags);
 }
 
-EXPORT_SYMBOL_GPL(h1940_latch_control);
+static int h1940_gpiolib_latch_input(struct gpio_chip *chip, unsigned
 offset) +{
+	return -EINVAL;
+}
+
+
+static inline int h1940_gpiolib_to_latch(int offset)
+{
+	return 1 << (offset + 16);
+}
+
+static void h1940_gpiolib_latch_set(struct gpio_chip *chip,
+					unsigned offset, int value)
+{
+	int latch_bit = h1940_gpiolib_to_latch(offset);
+
+	h1940_latch_control(value ? 0 : latch_bit,
+		value ? latch_bit : 0);
+}
+
+static int h1940_gpiolib_latch_output(struct gpio_chip *chip,
+					unsigned offset, int value)
+{
+	h1940_gpiolib_latch_set(chip, offset, value);
+
+	return 0;
+}
+
+static int h1940_gpiolib_latch_get(struct gpio_chip *chip,
+					unsigned offset)
+{
+	return (latch_state >> (offset + 16)) & 1;
+}
+
+struct gpio_chip h1940_latch_gpiochip = {
+	.base				= H1940_LATCH_GPIO(0),
+	.owner				= THIS_MODULE,
+	.label				= "H1940_LATCH",
+	.ngpio				= 16,
+	.direction_input	= h1940_gpiolib_latch_input,
+	.direction_output	= h1940_gpiolib_latch_output,
+	.set				= h1940_gpiolib_latch_set,
+	.get				= h1940_gpiolib_latch_get,
+};
 
 static void h1940_udc_pullup(enum s3c2410_udc_cmd_e cmd)
 {

@@ -124,10 +185,10 @@ static void h1940_udc_pullup(enum s3c2410_udc_cmd_e
 cmd) switch (cmd)
 	{
 		case S3C2410_UDC_P_ENABLE :
-			h1940_latch_control(0, H1940_LATCH_USB_DP);
+			gpio_set_value(H1940_LATCH_USB_DP, 1);
 			break;
 		case S3C2410_UDC_P_DISABLE :
-			h1940_latch_control(H1940_LATCH_USB_DP, 0);
+			gpio_set_value(H1940_LATCH_USB_DP, 0);
 			break;
 		case S3C2410_UDC_P_RESET :
 			break;

@@ -302,6 +363,8 @@ static void __init h1940_map_io(void)
 	memcpy(phys_to_virt(H1940_SUSPEND_RESUMEAT), h1940_pm_return, 1024);
 #endif
 	s3c_pm_init();
+
+	WARN_ON(gpiochip_add(&h1940_latch_gpiochip));
 }
 
 static void __init h1940_init_irq(void)

@@ -334,9 +397,11 @@ static void __init h1940_init(void)
 	gpio_request(S3C2410_GPC(0), "LCD power");
 	gpio_request(S3C2410_GPC(5), "LCD power");
 	gpio_request(S3C2410_GPC(6), "LCD power");
-
 	gpio_direction_input(S3C2410_GPC(6));
 
+	gpio_request(H1940_LATCH_USB_DP, "USB pullup");
+	gpio_direction_output(H1940_LATCH_USB_DP, 0);
+
 	platform_add_devices(h1940_devices, ARRAY_SIZE(h1940_devices));
 }

arch/arm/plat-s3c24xx/Kconfig +1 -0

index 984bf66..5a27b1b 100644
@@ -69,6 +69,7 @@ config S3C24XX_GPIO_EXTRA
 	int
 	default 128 if S3C24XX_GPIO_EXTRA128
 	default 64 if S3C24XX_GPIO_EXTRA64
+	default 16 if ARCH_H1940
 	default 0
 
 config S3C24XX_GPIO_EXTRA64

Vasily Khoruzhick

author

2010-08-24 14:04 UTC #3 lore raw

PATCH 1/3 h1940: use gpiolib for latch access

? ????????? ?? 24 ??????? 2010 16:49:51 ????? Marek Vasut ???????:

Quoted 4 lines

Review
Quoted 1 line
(quoted diff) +1 -0
+	ret = gpio_request(H1940_LATCH_BLUETOOTH_POWER, dev_name(&pdev-
dev));
This should contain the name of the GPIO, not dev_name I assume.

Quoted 5 lines

arch/arm/mach-s3c2410/h1940-bluetooth.c static int __devinit h1940bt_probe(struct platform_device *pdev) · line 73
Quoted 2 lines
arch/arm/mach-s3c2410/h1940-bluetooth.c +2 -0
+	if (ret) {
+		gpio_free(S3C2410_GPH(1));
What's this constant (the 1) here ? Maybe some sane #define wont hurt or comment around it.

GPH(1) specifies gpio bit, that's not magic constant.

Quoted 9 lines

arch/arm/mach-s3c2410/mach-h1940.c line 66
Quoted 5 lines
arch/arm/mach-s3c2410/mach-h1940.c +5 -0
+#ifndef __ASSEMBLY__
+#define H1940_LATCH		((void __force __iomem *)0xF8000000)
+#else
+#define H1940_LATCH		0xF8000000
+#endif
Is the __ASSEMBLY__ really needed ? You can establish mapping when the kernel boots (looks like you're doing that already) but then use __raw_readX __raw_writeX to access that space instead of this stuff above.

Uh, I just moved this code from another file, not sure if __ASSEMBLY__ is necessary, I'll remove it if it is not.

Thanks for review :)

Regards
Vasily
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100824/f6e24406/attachment-0001.sig>

Marek Vasut

2010-08-24 14:40 UTC #4 lore raw

PATCH 1/3 h1940: use gpiolib for latch access

Dne ?t 24. srpna 2010 16:04:00 Vasily Khoruzhick napsal(a):

Quoted 9 lines

? ????????? ?? 24 ??????? 2010 16:49:51 ????? Marek Vasut ???????:
Quoted 5 lines
Review
Quoted 1 line
(quoted diff) +1 -0
+	ret = gpio_request(H1940_LATCH_BLUETOOTH_POWER, dev_name(&pdev-
dev));
This should contain the name of the GPIO, not dev_name I assume.
Ok

Quoted 7 lines

Quoted 5 lines
arch/arm/mach-s3c2410/h1940-bluetooth.c static int __devinit h1940bt_probe(struct platform_device *pdev) · line 73
Quoted 2 lines
arch/arm/mach-s3c2410/h1940-bluetooth.c +2 -0
+	if (ret) {
+		gpio_free(S3C2410_GPH(1));
What's this constant (the 1) here ? Maybe some sane #define wont hurt or comment around it.
GPH(1) specifies gpio bit, that's not magic constant.

Defining S3C2410_GPH_GPIONAME for each bit might be better then.

Quoted 12 lines

Quoted 11 lines
arch/arm/mach-s3c2410/mach-h1940.c line 66
Quoted 6 lines
arch/arm/mach-s3c2410/mach-h1940.c +5 -0
+#ifndef __ASSEMBLY__
+#define H1940_LATCH		((void __force __iomem *)0xF8000000)
+#else
+#define H1940_LATCH		0xF8000000
+#endif
Is the __ASSEMBLY__ really needed ? You can establish mapping when the kernel boots (looks like you're doing that already) but then use __raw_readX __raw_writeX to access that space instead of this stuff above.

Quoted 2 lines

Uh, I just moved this code from another file, not sure if __ASSEMBLY__ is necessary, I'll remove it if it is not.

Well this isn't included in any assembly file. So it's not. Also, I'd be happier if you could fix it the way I outlined.

Check for example mach-pxa/balloon3.c and how NAND access is done there. Cheers

Quoted 2 lines

Thanks for review :)

You're welcome.

Quoted 3 lines

Regards
Vasily

Vasily Khoruzhick

author

2010-08-24 14:50 UTC #5 lore raw

PATCH 1/3 h1940: use gpiolib for latch access

? ????????? ?? 24 ??????? 2010 17:40:21 ????? Marek Vasut ???????:

Quoted 1 line

Defining S3C2410_GPH_GPIONAME for each bit might be better then.

Well, that requires massive refactoring of h1940 (and rx1950) support. And btw, that's common for s3c24xx-based machines to use generic s3c24xx gpio names, not machine specific.

Marek Vasut

2010-08-24 15:04 UTC #6 lore raw

PATCH 1/3 h1940: use gpiolib for latch access

Dne ?t 24. srpna 2010 16:50:36 Vasily Khoruzhick napsal(a):

Quoted 6 lines

? ????????? ?? 24 ??????? 2010 17:40:21 ????? Marek Vasut ???????:
Quoted 1 line
Defining S3C2410_GPH_GPIONAME for each bit might be better then.
Well, that requires massive refactoring of h1940 (and rx1950) support. And btw, that's common for s3c24xx-based machines to use generic s3c24xx gpio names, not machine specific.

I assume I see how it works now. Ok, please put a comment around it then so others will understand what it does.

Using machine-specific GPIO names might help readability btw. -- the approach "everyone does it this way so I'll do it this way without thinking" is bad :)

Well, it's just my suggestion for improvement of the overall code quality. Ben, what's your opinion?

Cheers

Quoted 3 lines

Regards
Vasily

Russell King - ARM Linux

2010-09-02 10:53 UTC #7 lore raw

PATCH 1/3 h1940: use gpiolib for latch access

In reply to Vasily Khoruzhick

arch/arm/mach-s3c2410/mach-h1940.c line 45

Quoted 9 lines · Vasily Khoruzhick

arch/arm/mach-s3c2410/mach-h1940.c +1 -0

index 779b45b..9717790 100644
@@ -42,6 +42,7 @@
 #include <mach/regs-gpio.h>
 #include <mach/gpio-fns.h>
 #include <mach/gpio-nrs.h>
+#include <mach/gpio.h>

linux/gpio.h ?

arch/arm/mach-s3c2410/mach-h1940.c line 66

Quoted 12 lines

arch/arm/mach-s3c2410/mach-h1940.c +5 -0

 #include <mach/h1940.h>
 #include <mach/h1940-latch.h>

@@ -58,6 +59,16 @@
 #include <plat/mci.h>
 #include <plat/ts.h>
 
+#ifndef __ASSEMBLY__
+#define H1940_LATCH		((void __force __iomem *)0xF8000000)
+#else
+#define H1940_LATCH		0xF8000000
+#endif

This file can never be built by the assembler, so the above is pointless.

arch/arm/mach-s3c2410/mach-h1940.c line 68

Quoted 2 lines

arch/arm/mach-s3c2410/mach-h1940.c +2 -0
+
+#define H1940_PA_LATCH		(S3C2410_CS2)

Parents not required.

Rest looks fine to me.

Vasily Khoruzhick

author

2010-08-19 15:00 UTC #8 lore raw

PATCH 2/3 h1940: fix h1940-bluetooth compilation

In reply to Vasily Khoruzhick

Signed-off-by Vasily Khoruzhick <[email protected]>

---
 arch/arm/mach-s3c2410/h1940-bluetooth.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

arch/arm/mach-s3c2410/h1940-bluetooth.c +4 -4

index 8aa2f19..6b86a72 100644
@@ -77,13 +77,13 @@ static int __devinit h1940bt_probe(struct platform_device *pdev)
 
 	/* Configures BT serial port GPIOs */
 	s3c_gpio_cfgpin(S3C2410_GPH(0), S3C2410_GPH0_nCTS0);
-	s3c_gpio_cfgpull(S3C2410_GPH(0), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPH(0), S3C_GPIO_PULL_NONE);
 	s3c_gpio_cfgpin(S3C2410_GPH(1), S3C2410_GPIO_OUTPUT);
-	s3c_gpio_cfgpull(S3C2410_GPH(1), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPH(1), S3C_GPIO_PULL_NONE);
 	s3c_gpio_cfgpin(S3C2410_GPH(2), S3C2410_GPH2_TXD0);
-	s3c_gpio_cfgpull(S3C2410_GPH(2), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPH(2), S3C_GPIO_PULL_NONE);
 	s3c_gpio_cfgpin(S3C2410_GPH(3), S3C2410_GPH3_RXD0);
-	s3c_gpio_cfgpull(S3C2410_GPH(3), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPH(3), S3C_GPIO_PULL_NONE);
 
 
 	rfk = rfkill_alloc(DRV_NAME, &pdev->dev, RFKILL_TYPE_BLUETOOTH,

Signature · 1 line

1.7.2

Marek Vasut

2010-08-24 13:50 UTC #9 lore raw

PATCH 2/3 h1940: fix h1940-bluetooth compilation

Dne ?t 19. srpna 2010 17:00:02 Vasily Khoruzhick napsal(a):

Quoted 1 line

Signed-off-by Vasily Khoruzhick <[email protected]>

Errr ... fix ... in what way? What was broken? I don't understand, please add more explanatory text. Thanks
Cheers

Quoted 27 lines

---
 arch/arm/mach-s3c2410/h1940-bluetooth.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-s3c2410/h1940-bluetooth.c index 8aa2f19..6b86a72 100644

arch/arm/mach-s3c2410/h1940-bluetooth.c +4 -4

@@ -77,13 +77,13 @@ static int __devinit h1940bt_probe(struct
 platform_device *pdev)
 
 	/* Configures BT serial port GPIOs */
 	s3c_gpio_cfgpin(S3C2410_GPH(0), S3C2410_GPH0_nCTS0);
-	s3c_gpio_cfgpull(S3C2410_GPH(0), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPH(0), S3C_GPIO_PULL_NONE);
 	s3c_gpio_cfgpin(S3C2410_GPH(1), S3C2410_GPIO_OUTPUT);
-	s3c_gpio_cfgpull(S3C2410_GPH(1), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPH(1), S3C_GPIO_PULL_NONE);
 	s3c_gpio_cfgpin(S3C2410_GPH(2), S3C2410_GPH2_TXD0);
-	s3c_gpio_cfgpull(S3C2410_GPH(2), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPH(2), S3C_GPIO_PULL_NONE);
 	s3c_gpio_cfgpin(S3C2410_GPH(3), S3C2410_GPH3_RXD0);
-	s3c_gpio_cfgpull(S3C2410_GPH(3), S3C_GPIO_PULL_NONE);
+	s3c_gpio_setpull(S3C2410_GPH(3), S3C_GPIO_PULL_NONE);

rfk = rfkill_alloc(DRV_NAME, &pdev->dev, RFKILL_TYPE_BLUETOOTH,

Vasily Khoruzhick

author

2010-08-24 13:57 UTC #10 lore raw

PATCH 2/3 h1940: fix h1940-bluetooth compilation

? ????????? ?? 24 ??????? 2010 16:50:46 ????? Marek Vasut ???????:

Quoted 6 lines

Dne ?t 19. srpna 2010 17:00:02 Vasily Khoruzhick napsal(a):
Quoted 1 line
Signed-off-by Vasily Khoruzhick <[email protected]>
Errr ... fix ... in what way? What was broken? I don't understand, please add more explanatory text. Thanks
Cheers

It's compile fix, without this patch it just doesn't compile.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100824/40d09b73/attachment.sig>

Marek Vasut

2010-08-24 14:00 UTC #11 lore raw

PATCH 2/3 h1940: fix h1940-bluetooth compilation

Dne ?t 24. srpna 2010 15:57:07 Vasily Khoruzhick napsal(a):

Quoted 9 lines

? ????????? ?? 24 ??????? 2010 16:50:46 ????? Marek Vasut ???????:
Quoted 6 lines
Dne ?t 19. srpna 2010 17:00:02 Vasily Khoruzhick napsal(a):
Quoted 1 line
Signed-off-by Vasily Khoruzhick <[email protected]>
Errr ... fix ... in what way? What was broken? I don't understand, please add more explanatory text. Thanks
Cheers
It's compile fix, without this patch it just doesn't compile.

Because the GPIO api changed or what ? Please stick this text into the next version of the patch.

Thanks!

Vasily Khoruzhick

author

2010-08-19 15:00 UTC #12 lore raw

PATCH 3/3 h1940: implement mmc_power function

In reply to Vasily Khoruzhick

Signed-off-by Vasily Khoruzhick <[email protected]>

---
 arch/arm/mach-s3c2410/include/mach/h1940-latch.h |    2 +-
 arch/arm/mach-s3c2410/mach-h1940.c               |   23 +++++++++++++++++++--
 2 files changed, 21 insertions(+), 4 deletions(-)

arch/arm/mach-s3c2410/include/mach/h1940-latch.h +1 -1

index 73586f2..ef7d8cf 100644
@@ -35,7 +35,7 @@
 #define H1940_LATCH_AUDIO_POWER		H1940_LATCH_GPIO(9)
 #define H1940_LATCH_SM803_ENABLE	H1940_LATCH_GPIO(10)
 #define H1940_LATCH_LCD_P4		H1940_LATCH_GPIO(11)
-#define H1940_LATCH_CPUQ5		H1940_LATCH_GPIO(12)
+#define H1940_LATCH_SD_POWER		H1940_LATCH_GPIO(12)
 #define H1940_LATCH_BLUETOOTH_POWER	H1940_LATCH_GPIO(13)
 #define H1940_LATCH_LED_GREEN		H1940_LATCH_GPIO(14)
 #define H1940_LATCH_LED_FLASH		H1940_LATCH_GPIO(15)

arch/arm/mach-s3c2410/mach-h1940.c +20 -3

index 9717790..c1ccc8e 100644
@@ -116,8 +116,7 @@ static unsigned int latch_state = H1940_LATCH_BIT(H1940_LATCH_LCD_P4) |
 	H1940_LATCH_BIT(H1940_LATCH_LCD_P1)			|
 	H1940_LATCH_BIT(H1940_LATCH_LCD_P2)			|
 	H1940_LATCH_BIT(H1940_LATCH_LCD_P3)			|
-	H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN)   |
-	H1940_LATCH_BIT(H1940_LATCH_CPUQ5);
+	H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN);
 
 static void h1940_latch_control(unsigned int clear, unsigned int set)
 {

@@ -259,10 +258,25 @@ static struct platform_device h1940_device_bluetooth = {
 	.id               = -1,
 };
 
+static void h1940_set_mmc_power(unsigned char power_mode, unsigned short vdd)
+{
+	switch (power_mode) {
+	case MMC_POWER_OFF:
+		gpio_set_value(H1940_LATCH_SD_POWER, 0);
+		break;
+	case MMC_POWER_UP:
+	case MMC_POWER_ON:
+		gpio_set_value(H1940_LATCH_SD_POWER, 1);
+		break;
+	default:
+		break;
+	};
+}
+
 static struct s3c24xx_mci_pdata h1940_mmc_cfg __initdata = {
 	.gpio_detect   = S3C2410_GPF(5),
 	.gpio_wprotect = S3C2410_GPH(8),
-	.set_power     = NULL,
+	.set_power     = h1940_set_mmc_power,
 	.ocr_avail     = MMC_VDD_32_33,
 };

@@ -402,6 +416,9 @@ static void __init h1940_init(void)
 	gpio_request(H1940_LATCH_USB_DP, "USB pullup");
 	gpio_direction_output(H1940_LATCH_USB_DP, 0);
 
+	gpio_request(H1940_LATCH_SD_POWER, "SD power");
+	gpio_direction_output(H1940_LATCH_SD_POWER, 0);
+
 	platform_add_devices(h1940_devices, ARRAY_SIZE(h1940_devices));
 }

Signature · 1 line

1.7.2

Marek Vasut

2010-08-24 13:52 UTC #13 lore raw

PATCH 3/3 h1940: implement mmc_power function

Dne ?t 19. srpna 2010 17:00:03 Vasily Khoruzhick napsal(a):

Quoted 52 lines

Signed-off-by Vasily Khoruzhick <[email protected]>

---
 arch/arm/mach-s3c2410/include/mach/h1940-latch.h |    2 +-
 arch/arm/mach-s3c2410/mach-h1940.c               |   23

+++++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-s3c2410/include/mach/h1940-latch.h index 73586f2..ef7d8cf 100644

arch/arm/mach-s3c2410/include/mach/h1940-latch.h +1 -1

@@ -35,7 +35,7 @@
 #define H1940_LATCH_AUDIO_POWER		H1940_LATCH_GPIO(9)
 #define H1940_LATCH_SM803_ENABLE	H1940_LATCH_GPIO(10)
 #define H1940_LATCH_LCD_P4		H1940_LATCH_GPIO(11)
-#define H1940_LATCH_CPUQ5		H1940_LATCH_GPIO(12)
+#define H1940_LATCH_SD_POWER		H1940_LATCH_GPIO(12)
 #define H1940_LATCH_BLUETOOTH_POWER	H1940_LATCH_GPIO(13)
 #define H1940_LATCH_LED_GREEN		H1940_LATCH_GPIO(14)
 #define H1940_LATCH_LED_FLASH		H1940_LATCH_GPIO(15)

(quoted diff) +0 -0

b/arch/arm/mach-s3c2410/mach-h1940.c index 9717790..c1ccc8e 100644

arch/arm/mach-s3c2410/mach-h1940.c +0 -0

@@ -116,8 +116,7 @@ static unsigned int latch_state =

H1940_LATCH_BIT(H1940_LATCH_LCD_P4) |
H1940_LATCH_BIT(H1940_LATCH_LCD_P1) |

arch/arm/mach-s3c2410/mach-h1940.c +12 -2

 	H1940_LATCH_BIT(H1940_LATCH_LCD_P2)			|
 	H1940_LATCH_BIT(H1940_LATCH_LCD_P3)			|
-	H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN)   |
-	H1940_LATCH_BIT(H1940_LATCH_CPUQ5);
+	H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN);
 
 static void h1940_latch_control(unsigned int clear, unsigned int set)
 {

@@ -259,10 +258,25 @@ static struct platform_device h1940_device_bluetooth
 = { .id               = -1,
 };
 
+static void h1940_set_mmc_power(unsigned char power_mode, unsigned short
 vdd) +{
+	switch (power_mode) {
+	case MMC_POWER_OFF:
+		gpio_set_value(H1940_LATCH_SD_POWER, 0);
+		break;
+	case MMC_POWER_UP:
+	case MMC_POWER_ON:
+		gpio_set_value(H1940_LATCH_SD_POWER, 1);
+		break;
+	default:
+		break;

arch/arm/mach-s3c2410/mach-h1940.c static struct platform_device h1940_device_bluetooth · line 281

Quoted 10 lines

arch/arm/mach-s3c2410/mach-h1940.c +4 -1

+	};
+}
+
 static struct s3c24xx_mci_pdata h1940_mmc_cfg __initdata = {
 	.gpio_detect   = S3C2410_GPF(5),
 	.gpio_wprotect = S3C2410_GPH(8),
-	.set_power     = NULL,
+	.set_power     = h1940_set_mmc_power,
 	.ocr_avail     = MMC_VDD_32_33,
 };

Can't you implement gpio_power into the s3c24xx mmc driver ? Then you can fix mach-n30 too.

arch/arm/mach-s3c2410/mach-h1940.c static void __init h1940_init(void) · line 420

Quoted 6 lines

arch/arm/mach-s3c2410/mach-h1940.c +2 -0

@@ -402,6 +416,9 @@ static void __init h1940_init(void)
 	gpio_request(H1940_LATCH_USB_DP, "USB pullup");
 	gpio_direction_output(H1940_LATCH_USB_DP, 0);
 
+	gpio_request(H1940_LATCH_SD_POWER, "SD power");
+	gpio_direction_output(H1940_LATCH_SD_POWER, 0);

Please handle possible return values here !

arch/arm/mach-s3c2410/mach-h1940.c static void __init h1940_init(void) · line 423

Quoted 3 lines

arch/arm/mach-s3c2410/mach-h1940.c +1 -0
+
 	platform_add_devices(h1940_devices, ARRAY_SIZE(h1940_devices));
 }

Cheers

Vasily Khoruzhick

author

2010-08-24 14:00 UTC #14 lore raw

PATCH 3/3 h1940: implement mmc_power function

? ????????? ?? 24 ??????? 2010 16:52:42 ????? Marek Vasut ???????:

Quoted 74 lines

Dne ?t 19. srpna 2010 17:00:03 Vasily Khoruzhick napsal(a):
Quoted 73 lines
Signed-off-by Vasily Khoruzhick <[email protected]>
---
 arch/arm/mach-s3c2410/include/mach/h1940-latch.h |    2 +-
 arch/arm/mach-s3c2410/mach-h1940.c               |   23
+++++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-)
(quoted diff) +0 -0
b/arch/arm/mach-s3c2410/include/mach/h1940-latch.h index 73586f2..ef7d8cf 100644
arch/arm/mach-s3c2410/include/mach/h1940-latch.h +1 -1
@@ -35,7 +35,7 @@
 
 #define H1940_LATCH_AUDIO_POWER		H1940_LATCH_GPIO(9)
 #define H1940_LATCH_SM803_ENABLE	H1940_LATCH_GPIO(10)
 #define H1940_LATCH_LCD_P4		H1940_LATCH_GPIO(11)
 
-#define H1940_LATCH_CPUQ5		H1940_LATCH_GPIO(12)
+#define H1940_LATCH_SD_POWER		H1940_LATCH_GPIO(12)
 
#define H1940_LATCH_BLUETOOTH_POWER H1940_LATCH_GPIO(13)
#define H1940_LATCH_LED_GREEN H1940_LATCH_GPIO(14)
#define H1940_LATCH_LED_FLASH H1940_LATCH_GPIO(15)
(quoted diff) +0 -0
b/arch/arm/mach-s3c2410/mach-h1940.c index 9717790..c1ccc8e 100644
arch/arm/mach-s3c2410/mach-h1940.c +0 -0
@@ -116,8 +116,7 @@ static unsigned int latch_state =
H1940_LATCH_BIT(H1940_LATCH_LCD_P4) |
H1940_LATCH_BIT(H1940_LATCH_LCD_P1) |
arch/arm/mach-s3c2410/mach-h1940.c +16 -3
 	H1940_LATCH_BIT(H1940_LATCH_LCD_P2)			|
 	H1940_LATCH_BIT(H1940_LATCH_LCD_P3)			|
 
-	H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN)   |
-	H1940_LATCH_BIT(H1940_LATCH_CPUQ5);
+	H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN);
 
 static void h1940_latch_control(unsigned int clear, unsigned int set)
 {
@@ -259,10 +258,25 @@ static struct platform_device
 h1940_device_bluetooth = { .id               = -1,
 
 };
 
+static void h1940_set_mmc_power(unsigned char power_mode, unsigned short
 vdd) +{
+	switch (power_mode) {
+	case MMC_POWER_OFF:
+		gpio_set_value(H1940_LATCH_SD_POWER, 0);
+		break;
+	case MMC_POWER_UP:
+	case MMC_POWER_ON:
+		gpio_set_value(H1940_LATCH_SD_POWER, 1);
+		break;
+	default:
+		break;
+	};
+}
+
 
 static struct s3c24xx_mci_pdata h1940_mmc_cfg __initdata = {
 
 	.gpio_detect   = S3C2410_GPF(5),
 	.gpio_wprotect = S3C2410_GPH(8),
 
-	.set_power     = NULL,
+	.set_power     = h1940_set_mmc_power,

Quoted 6 lines

Quoted 3 lines
.ocr_avail = MMC_VDD_32_33,
};
Can't you implement gpio_power into the s3c24xx mmc driver ? Then you can fix mach-n30 too.

Well, I'm happy with callback. However, maybe it worth to replace set_power callback with regulator.

Quoted 9 lines

arch/arm/mach-s3c2410/mach-h1940.c static void __init h1940_init(void) · line 421
Quoted 7 lines
arch/arm/mach-s3c2410/mach-h1940.c +2 -0
@@ -402,6 +416,9 @@ static void __init h1940_init(void)
 
 	gpio_request(H1940_LATCH_USB_DP, "USB pullup");
 	gpio_direction_output(H1940_LATCH_USB_DP, 0);
 
+	gpio_request(H1940_LATCH_SD_POWER, "SD power");
+	gpio_direction_output(H1940_LATCH_SD_POWER, 0);
Please handle possible return values here !

Ok, I suppose WARN_ON will be enought?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100824/80915fef/attachment.sig>

Marek Vasut

2010-08-24 14:01 UTC #15 lore raw

PATCH 3/3 h1940: implement mmc_power function

Dne ?t 24. srpna 2010 16:00:26 Vasily Khoruzhick napsal(a):

Quoted 90 lines

? ????????? ?? 24 ??????? 2010 16:52:42 ????? Marek Vasut ???????:
Quoted 74 lines
Dne ?t 19. srpna 2010 17:00:03 Vasily Khoruzhick napsal(a):
Quoted 73 lines
Signed-off-by Vasily Khoruzhick <[email protected]>
---
 arch/arm/mach-s3c2410/include/mach/h1940-latch.h |    2 +-
 arch/arm/mach-s3c2410/mach-h1940.c               |   23
+++++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-)
(quoted diff) +0 -0
b/arch/arm/mach-s3c2410/include/mach/h1940-latch.h index 73586f2..ef7d8cf 100644
arch/arm/mach-s3c2410/include/mach/h1940-latch.h +1 -1
@@ -35,7 +35,7 @@
 
 #define H1940_LATCH_AUDIO_POWER		H1940_LATCH_GPIO(9)
 #define H1940_LATCH_SM803_ENABLE	H1940_LATCH_GPIO(10)
 #define H1940_LATCH_LCD_P4		H1940_LATCH_GPIO(11)
 
-#define H1940_LATCH_CPUQ5		H1940_LATCH_GPIO(12)
+#define H1940_LATCH_SD_POWER		H1940_LATCH_GPIO(12)
 
#define H1940_LATCH_BLUETOOTH_POWER H1940_LATCH_GPIO(13)
#define H1940_LATCH_LED_GREEN H1940_LATCH_GPIO(14)
#define H1940_LATCH_LED_FLASH H1940_LATCH_GPIO(15)
(quoted diff) +0 -0
b/arch/arm/mach-s3c2410/mach-h1940.c index 9717790..c1ccc8e 100644
arch/arm/mach-s3c2410/mach-h1940.c +0 -0
@@ -116,8 +116,7 @@ static unsigned int latch_state =
H1940_LATCH_BIT(H1940_LATCH_LCD_P4) |
H1940_LATCH_BIT(H1940_LATCH_LCD_P1) |
arch/arm/mach-s3c2410/mach-h1940.c +16 -3
 	H1940_LATCH_BIT(H1940_LATCH_LCD_P2)			|
 	H1940_LATCH_BIT(H1940_LATCH_LCD_P3)			|
 
-	H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN)   |
-	H1940_LATCH_BIT(H1940_LATCH_CPUQ5);
+	H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN);
 
 static void h1940_latch_control(unsigned int clear, unsigned int set)
 {
@@ -259,10 +258,25 @@ static struct platform_device
 h1940_device_bluetooth = { .id               = -1,
 
 };
 
+static void h1940_set_mmc_power(unsigned char power_mode, unsigned
 short vdd) +{
+	switch (power_mode) {
+	case MMC_POWER_OFF:
+		gpio_set_value(H1940_LATCH_SD_POWER, 0);
+		break;
+	case MMC_POWER_UP:
+	case MMC_POWER_ON:
+		gpio_set_value(H1940_LATCH_SD_POWER, 1);
+		break;
+	default:
+		break;
+	};
+}
+
 
 static struct s3c24xx_mci_pdata h1940_mmc_cfg __initdata = {
 
 	.gpio_detect   = S3C2410_GPF(5),
 	.gpio_wprotect = S3C2410_GPH(8),
 
-	.set_power     = NULL,
+	.set_power     = h1940_set_mmc_power,
Quoted 6 lines
Quoted 3 lines
.ocr_avail = MMC_VDD_32_33,
};
Can't you implement gpio_power into the s3c24xx mmc driver ? Then you can fix mach-n30 too.
Well, I'm happy with callback. However, maybe it worth to replace set_power callback with regulator.
Quoted 5 lines
Quoted 5 lines
arch/arm/mach-s3c2410/mach-h1940.c +0 -0
@@ -402,6 +416,9 @@ static void __init h1940_init(void)
 
 	gpio_request(H1940_LATCH_USB_DP, "USB pullup");
 	gpio_direction_output(H1940_LATCH_USB_DP, 0);
 

Quoted 6 lines

Quoted 4 lines
arch/arm/mach-s3c2410/mach-h1940.c static void __init h1940_init(void) · line 420
Quoted 2 lines
arch/arm/mach-s3c2410/mach-h1940.c +2 -0
+	gpio_request(H1940_LATCH_SD_POWER, "SD power");
+	gpio_direction_output(H1940_LATCH_SD_POWER, 0);
Please handle possible return values here !
Ok, I suppose WARN_ON will be enought?

pr_err() and don't probe the device for which you couldn't request GPIO.

Cheers

Vasily Khoruzhick

author

2010-08-24 14:21 UTC #16 lore raw

PATCH 3/3 h1940: implement mmc_power function

? ????????? ?? 24 ??????? 2010 17:01:12 ????? Marek Vasut ???????:

Quoted 1 line

pr_err() and don't probe the device for which you couldn't request GPIO.

Well, handling those errors in machine init functions doesn't make sense. PDA is not usable without basic hardware.

Marek Vasut

2010-08-24 14:41 UTC #17 lore raw

PATCH 3/3 h1940: implement mmc_power function

Dne ?t 24. srpna 2010 16:21:01 Vasily Khoruzhick napsal(a):

Quoted 6 lines

? ????????? ?? 24 ??????? 2010 17:01:12 ????? Marek Vasut ???????:
Quoted 1 line
pr_err() and don't probe the device for which you couldn't request GPIO.
Well, handling those errors in machine init functions doesn't make sense. PDA is not usable without basic hardware.

You can use PDA without LCD for example (there are such people, trust me). So, this comment really isn't too valid. You can also use PDA without MMC support, you still have onboard flash, etc. etc.

Cheers

Quoted 2 lines

Regards
Vasily

Vasily Khoruzhick

author

2010-08-24 14:56 UTC #18 lore raw

PATCH 3/3 h1940: implement mmc_power function

? ????????? ?? 24 ??????? 2010 17:41:57 ????? Marek Vasut ???????:

Quoted 11 lines

Dne ?t 24. srpna 2010 16:21:01 Vasily Khoruzhick napsal(a):
Quoted 6 lines
? ????????? ?? 24 ??????? 2010 17:01:12 ????? Marek Vasut ???????:
Quoted 2 lines
pr_err() and don't probe the device for which you couldn't request GPIO.
Well, handling those errors in machine init functions doesn't make sense. PDA is not usable without basic hardware.
You can use PDA without LCD for example (there are such people, trust me). So, this comment really isn't too valid. You can also use PDA without MMC support, you still have onboard flash, etc. etc.

But error handling here just increases code size, and gpio_request in init function should always succeed. It means something's going really wrong (kernel bug or misconfiguration) if gpio_request fails. Btw, Ben suggested to use WARN_ON in machine init, look through init functions of qt2410, n30, rx1950, vr1000, etc.

Marek Vasut

2010-08-24 15:07 UTC #19 lore raw

PATCH 3/3 h1940: implement mmc_power function

Dne ?t 24. srpna 2010 16:56:06 Vasily Khoruzhick napsal(a):

Quoted 13 lines

? ????????? ?? 24 ??????? 2010 17:41:57 ????? Marek Vasut ???????:
Quoted 11 lines
Dne ?t 24. srpna 2010 16:21:01 Vasily Khoruzhick napsal(a):
Quoted 6 lines
? ????????? ?? 24 ??????? 2010 17:01:12 ????? Marek Vasut ???????:
Quoted 2 lines
pr_err() and don't probe the device for which you couldn't request GPIO.
Well, handling those errors in machine init functions doesn't make sense. PDA is not usable without basic hardware.
You can use PDA without LCD for example (there are such people, trust me). So, this comment really isn't too valid. You can also use PDA without MMC support, you still have onboard flash, etc. etc.

Quoted 5 lines

But error handling here just increases code size, and gpio_request in init function should always succeed. It means something's going really wrong (kernel bug or misconfiguration) if gpio_request fails. Btw, Ben suggested to use WARN_ON in machine init, look through init functions of qt2410, n30, rx1950, vr1000, etc.

See my previous response. It'd increase code size, but if you mark that code __init, it'll be freed after that function finishes so it doesn't matter.

If something wrong goes on, you can still debug it without for example MMC card (though if you continued probing the MMC driver, it might cause additional unwanted noise and make the debugging harder). That's the whole point.

Cheers

Quoted 3 lines

Regards
Vasily

Vasily Khoruzhick

author

2010-08-24 15:19 UTC #20 lore raw

PATCH 3/3 h1940: implement mmc_power function

? ????????? ?? 24 ??????? 2010 18:07:31 ????? Marek Vasut ???????:

Quoted 8 lines

See my previous response. It'd increase code size, but if you mark that code __init, it'll be freed after that function finishes so it doesn't matter.
If something wrong goes on, you can still debug it without for example MMC card (though if you continued probing the MMC driver, it might cause additional unwanted noise and make the debugging harder). That's the whole point.

Ok, but I also want to hear Ben's opinion on this point.

Vasily Khoruzhick

author

2010-08-24 13:24 UTC #21 lore raw

cover h1940 series

In reply to Vasily Khoruzhick

? ????????? ?? 19 ??????? 2010 18:00:00 ????? Vasily Khoruzhick ???????:

Quoted 3 lines

This patch series replaces h1940-specific latch-related functions with gpiolib, fixes compilation error for h1940-bluetooth and adds PM support for mmc driver.

Ping
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100824/51866d68/attachment.sig>