All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] ARM: omap2: GPMC cleanup
@ 2013-02-12 15:18 ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Jon Hunter, Felipe Balbi, Tony Lindgren, Afzal Mohammed, Ezequiel Garcia

This patchset is v2 of the small cleanup consisting in:
 * mark some functions as 'static' when appropriate
 * remove an unused function from gpmc.c
 * improve error messages when a CS request fails
 * migrate to dev_err and dev_warn

Changelog from v1:
 * fix gpmc_cs_reserved to return a boolean instead
   of an integer error code
 * add a new patch to the patchset cleaning redundant checks

It has been tested on a IGEP v2 board with OneNAND,
which means the gpmc-nand patch is tested by compilation only.

Altough these patchset is almost trivial,
any feedback or testing is more than welcome.

Ezequiel Garcia (8):
  ARM: omap2: gpmc: Mark local scoped functions static
  ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
  ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
  ARM: omap2: gpmc-nand: Print something useful on CS request failure
  ARM: omap2: gpmc-onenand: Print something useful on CS request failure
  ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
  ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
  ARM: omap2: gpmc: Remove redundant chip select out of range check

 arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
 arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
 arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
 arch/arm/mach-omap2/gpmc.h         |    7 -------
 4 files changed, 13 insertions(+), 32 deletions(-)

-- 
1.7.8.6


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 0/8] ARM: omap2: GPMC cleanup
@ 2013-02-12 15:18 ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset is v2 of the small cleanup consisting in:
 * mark some functions as 'static' when appropriate
 * remove an unused function from gpmc.c
 * improve error messages when a CS request fails
 * migrate to dev_err and dev_warn

Changelog from v1:
 * fix gpmc_cs_reserved to return a boolean instead
   of an integer error code
 * add a new patch to the patchset cleaning redundant checks

It has been tested on a IGEP v2 board with OneNAND,
which means the gpmc-nand patch is tested by compilation only.

Altough these patchset is almost trivial,
any feedback or testing is more than welcome.

Ezequiel Garcia (8):
  ARM: omap2: gpmc: Mark local scoped functions static
  ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
  ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
  ARM: omap2: gpmc-nand: Print something useful on CS request failure
  ARM: omap2: gpmc-onenand: Print something useful on CS request failure
  ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
  ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
  ARM: omap2: gpmc: Remove redundant chip select out of range check

 arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
 arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
 arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
 arch/arm/mach-omap2/gpmc.h         |    7 -------
 4 files changed, 13 insertions(+), 32 deletions(-)

-- 
1.7.8.6

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 1/8] ARM: omap2: gpmc: Mark local scoped functions static
  2013-02-12 15:18 ` Ezequiel Garcia
@ 2013-02-12 15:18   ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Jon Hunter, Felipe Balbi, Tony Lindgren, Afzal Mohammed, Ezequiel Garcia

This patch marks a bunch of functions that are local
to gpmc.c file only as static.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc.c |   14 +++++++-------
 arch/arm/mach-omap2/gpmc.h |    7 -------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 1e8bcb4..ffe3e1e 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -181,7 +181,7 @@ void gpmc_cs_write_reg(int cs, int idx, u32 val)
 	__raw_writel(val, reg_addr);
 }
 
-u32 gpmc_cs_read_reg(int cs, int idx)
+static u32 gpmc_cs_read_reg(int cs, int idx)
 {
 	void __iomem *reg_addr;
 
@@ -190,7 +190,7 @@ u32 gpmc_cs_read_reg(int cs, int idx)
 }
 
 /* TODO: Add support for gpmc_fck to clock framework and use it */
-unsigned long gpmc_get_fclk_period(void)
+static unsigned long gpmc_get_fclk_period(void)
 {
 	unsigned long rate = clk_get_rate(gpmc_l3_clk);
 
@@ -205,7 +205,7 @@ unsigned long gpmc_get_fclk_period(void)
 	return rate;
 }
 
-unsigned int gpmc_ns_to_ticks(unsigned int time_ns)
+static unsigned int gpmc_ns_to_ticks(unsigned int time_ns)
 {
 	unsigned long tick_ps;
 
@@ -215,7 +215,7 @@ unsigned int gpmc_ns_to_ticks(unsigned int time_ns)
 	return (time_ns * 1000 + tick_ps - 1) / tick_ps;
 }
 
-unsigned int gpmc_ps_to_ticks(unsigned int time_ps)
+static unsigned int gpmc_ps_to_ticks(unsigned int time_ps)
 {
 	unsigned long tick_ps;
 
@@ -230,7 +230,7 @@ unsigned int gpmc_ticks_to_ns(unsigned int ticks)
 	return ticks * gpmc_get_fclk_period() / 1000;
 }
 
-unsigned int gpmc_round_ns_to_ticks(unsigned int time_ns)
+static unsigned int gpmc_round_ns_to_ticks(unsigned int time_ns)
 {
 	unsigned long ticks = gpmc_ns_to_ticks(time_ns);
 
@@ -448,7 +448,7 @@ static int gpmc_cs_mem_enabled(int cs)
 	return l & GPMC_CONFIG7_CSVALID;
 }
 
-int gpmc_cs_set_reserved(int cs, int reserved)
+static int gpmc_cs_set_reserved(int cs, int reserved)
 {
 	if (cs > GPMC_CS_NUM)
 		return -ENODEV;
@@ -459,7 +459,7 @@ int gpmc_cs_set_reserved(int cs, int reserved)
 	return 0;
 }
 
-int gpmc_cs_reserved(int cs)
+static int gpmc_cs_reserved(int cs)
 {
 	if (cs > GPMC_CS_NUM)
 		return -ENODEV;
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index fe0a844..b79e35c 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -195,20 +195,13 @@ extern int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
 extern void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs);
 extern int gpmc_get_client_irq(unsigned irq_config);
 
-extern unsigned int gpmc_ns_to_ticks(unsigned int time_ns);
-extern unsigned int gpmc_ps_to_ticks(unsigned int time_ps);
 extern unsigned int gpmc_ticks_to_ns(unsigned int ticks);
-extern unsigned int gpmc_round_ns_to_ticks(unsigned int time_ns);
-extern unsigned long gpmc_get_fclk_period(void);
 
 extern void gpmc_cs_write_reg(int cs, int idx, u32 val);
-extern u32 gpmc_cs_read_reg(int cs, int idx);
 extern int gpmc_calc_divider(unsigned int sync_clk);
 extern int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t);
 extern int gpmc_cs_request(int cs, unsigned long size, unsigned long *base);
 extern void gpmc_cs_free(int cs);
-extern int gpmc_cs_set_reserved(int cs, int reserved);
-extern int gpmc_cs_reserved(int cs);
 extern void omap3_gpmc_save_context(void);
 extern void omap3_gpmc_restore_context(void);
 extern int gpmc_cs_configure(int cs, int cmd, int wval);
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 1/8] ARM: omap2: gpmc: Mark local scoped functions static
@ 2013-02-12 15:18   ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

This patch marks a bunch of functions that are local
to gpmc.c file only as static.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc.c |   14 +++++++-------
 arch/arm/mach-omap2/gpmc.h |    7 -------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 1e8bcb4..ffe3e1e 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -181,7 +181,7 @@ void gpmc_cs_write_reg(int cs, int idx, u32 val)
 	__raw_writel(val, reg_addr);
 }
 
-u32 gpmc_cs_read_reg(int cs, int idx)
+static u32 gpmc_cs_read_reg(int cs, int idx)
 {
 	void __iomem *reg_addr;
 
@@ -190,7 +190,7 @@ u32 gpmc_cs_read_reg(int cs, int idx)
 }
 
 /* TODO: Add support for gpmc_fck to clock framework and use it */
-unsigned long gpmc_get_fclk_period(void)
+static unsigned long gpmc_get_fclk_period(void)
 {
 	unsigned long rate = clk_get_rate(gpmc_l3_clk);
 
@@ -205,7 +205,7 @@ unsigned long gpmc_get_fclk_period(void)
 	return rate;
 }
 
-unsigned int gpmc_ns_to_ticks(unsigned int time_ns)
+static unsigned int gpmc_ns_to_ticks(unsigned int time_ns)
 {
 	unsigned long tick_ps;
 
@@ -215,7 +215,7 @@ unsigned int gpmc_ns_to_ticks(unsigned int time_ns)
 	return (time_ns * 1000 + tick_ps - 1) / tick_ps;
 }
 
-unsigned int gpmc_ps_to_ticks(unsigned int time_ps)
+static unsigned int gpmc_ps_to_ticks(unsigned int time_ps)
 {
 	unsigned long tick_ps;
 
@@ -230,7 +230,7 @@ unsigned int gpmc_ticks_to_ns(unsigned int ticks)
 	return ticks * gpmc_get_fclk_period() / 1000;
 }
 
-unsigned int gpmc_round_ns_to_ticks(unsigned int time_ns)
+static unsigned int gpmc_round_ns_to_ticks(unsigned int time_ns)
 {
 	unsigned long ticks = gpmc_ns_to_ticks(time_ns);
 
@@ -448,7 +448,7 @@ static int gpmc_cs_mem_enabled(int cs)
 	return l & GPMC_CONFIG7_CSVALID;
 }
 
-int gpmc_cs_set_reserved(int cs, int reserved)
+static int gpmc_cs_set_reserved(int cs, int reserved)
 {
 	if (cs > GPMC_CS_NUM)
 		return -ENODEV;
@@ -459,7 +459,7 @@ int gpmc_cs_set_reserved(int cs, int reserved)
 	return 0;
 }
 
-int gpmc_cs_reserved(int cs)
+static int gpmc_cs_reserved(int cs)
 {
 	if (cs > GPMC_CS_NUM)
 		return -ENODEV;
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index fe0a844..b79e35c 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -195,20 +195,13 @@ extern int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
 extern void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs);
 extern int gpmc_get_client_irq(unsigned irq_config);
 
-extern unsigned int gpmc_ns_to_ticks(unsigned int time_ns);
-extern unsigned int gpmc_ps_to_ticks(unsigned int time_ps);
 extern unsigned int gpmc_ticks_to_ns(unsigned int ticks);
-extern unsigned int gpmc_round_ns_to_ticks(unsigned int time_ns);
-extern unsigned long gpmc_get_fclk_period(void);
 
 extern void gpmc_cs_write_reg(int cs, int idx, u32 val);
-extern u32 gpmc_cs_read_reg(int cs, int idx);
 extern int gpmc_calc_divider(unsigned int sync_clk);
 extern int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t);
 extern int gpmc_cs_request(int cs, unsigned long size, unsigned long *base);
 extern void gpmc_cs_free(int cs);
-extern int gpmc_cs_set_reserved(int cs, int reserved);
-extern int gpmc_cs_reserved(int cs);
 extern void omap3_gpmc_save_context(void);
 extern void omap3_gpmc_restore_context(void);
 extern int gpmc_cs_configure(int cs, int cmd, int wval);
-- 
1.7.8.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 2/8] ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
  2013-02-12 15:18 ` Ezequiel Garcia
@ 2013-02-12 15:18   ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Jon Hunter, Felipe Balbi, Tony Lindgren, Afzal Mohammed, Ezequiel Garcia

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index ffe3e1e..bd3bc93 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -230,13 +230,6 @@ unsigned int gpmc_ticks_to_ns(unsigned int ticks)
 	return ticks * gpmc_get_fclk_period() / 1000;
 }
 
-static unsigned int gpmc_round_ns_to_ticks(unsigned int time_ns)
-{
-	unsigned long ticks = gpmc_ns_to_ticks(time_ns);
-
-	return ticks * gpmc_get_fclk_period() / 1000;
-}
-
 static unsigned int gpmc_ticks_to_ps(unsigned int ticks)
 {
 	return ticks * gpmc_get_fclk_period();
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 2/8] ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
@ 2013-02-12 15:18   ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index ffe3e1e..bd3bc93 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -230,13 +230,6 @@ unsigned int gpmc_ticks_to_ns(unsigned int ticks)
 	return ticks * gpmc_get_fclk_period() / 1000;
 }
 
-static unsigned int gpmc_round_ns_to_ticks(unsigned int time_ns)
-{
-	unsigned long ticks = gpmc_ns_to_ticks(time_ns);
-
-	return ticks * gpmc_get_fclk_period() / 1000;
-}
-
 static unsigned int gpmc_ticks_to_ps(unsigned int ticks)
 {
 	return ticks * gpmc_get_fclk_period();
-- 
1.7.8.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
  2013-02-12 15:18 ` Ezequiel Garcia
@ 2013-02-12 15:18   ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Jon Hunter, Felipe Balbi, Tony Lindgren, Afzal Mohammed, Ezequiel Garcia

Currently gpmc_cs_reserved() return value is somewhat inconsistent,
returning a negative value on an error condition, a positive value
if the chip select is reserved and zero if it's available.

Fix this by returning a boolean value as the function name suggests:
  * true if the chip select is reserved,
  * false if it's available

Suggested-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Changelog from v1:
 * As suggested by Felipe Balbi, fix return code to a boolean

 arch/arm/mach-omap2/gpmc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index bd3bc93..fa4764f 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -452,10 +452,10 @@ static int gpmc_cs_set_reserved(int cs, int reserved)
 	return 0;
 }
 
-static int gpmc_cs_reserved(int cs)
+static bool gpmc_cs_reserved(int cs)
 {
 	if (cs > GPMC_CS_NUM)
-		return -ENODEV;
+		return true;
 
 	return gpmc_cs_map & (1 << cs);
 }
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
@ 2013-02-12 15:18   ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

Currently gpmc_cs_reserved() return value is somewhat inconsistent,
returning a negative value on an error condition, a positive value
if the chip select is reserved and zero if it's available.

Fix this by returning a boolean value as the function name suggests:
  * true if the chip select is reserved,
  * false if it's available

Suggested-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Changelog from v1:
 * As suggested by Felipe Balbi, fix return code to a boolean

 arch/arm/mach-omap2/gpmc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index bd3bc93..fa4764f 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -452,10 +452,10 @@ static int gpmc_cs_set_reserved(int cs, int reserved)
 	return 0;
 }
 
-static int gpmc_cs_reserved(int cs)
+static bool gpmc_cs_reserved(int cs)
 {
 	if (cs > GPMC_CS_NUM)
-		return -ENODEV;
+		return true;
 
 	return gpmc_cs_map & (1 << cs);
 }
-- 
1.7.8.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 4/8] ARM: omap2: gpmc-nand: Print something useful on CS request failure
  2013-02-12 15:18 ` Ezequiel Garcia
@ 2013-02-12 15:18   ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Jon Hunter, Felipe Balbi, Tony Lindgren, Afzal Mohammed, Ezequiel Garcia

If CS request fails the current error message is rather unhelpful.
Fix it by printing the failing chip select and the error code.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-nand.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index afc1e8c..e50e438 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -122,7 +122,8 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
 	err = gpmc_cs_request(gpmc_nand_data->cs, NAND_IO_SIZE,
 				(unsigned long *)&gpmc_nand_resource[0].start);
 	if (err < 0) {
-		dev_err(dev, "Cannot request GPMC CS\n");
+		dev_err(dev, "Cannot request GPMC CS %d, error %d\n",
+			gpmc_nand_data->cs, err);
 		return err;
 	}
 
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 4/8] ARM: omap2: gpmc-nand: Print something useful on CS request failure
@ 2013-02-12 15:18   ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

If CS request fails the current error message is rather unhelpful.
Fix it by printing the failing chip select and the error code.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-nand.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index afc1e8c..e50e438 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -122,7 +122,8 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
 	err = gpmc_cs_request(gpmc_nand_data->cs, NAND_IO_SIZE,
 				(unsigned long *)&gpmc_nand_resource[0].start);
 	if (err < 0) {
-		dev_err(dev, "Cannot request GPMC CS\n");
+		dev_err(dev, "Cannot request GPMC CS %d, error %d\n",
+			gpmc_nand_data->cs, err);
 		return err;
 	}
 
-- 
1.7.8.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 5/8] ARM: omap2: gpmc-onenand: Print something useful on CS request failure
  2013-02-12 15:18 ` Ezequiel Garcia
@ 2013-02-12 15:18   ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Jon Hunter, Felipe Balbi, Tony Lindgren, Afzal Mohammed, Ezequiel Garcia

If CS request fails the current error message is rather unhelpful.
Fix it by printing the failing chip select and the error code.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index fadd8743..0ee5317 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -379,7 +379,8 @@ void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 	err = gpmc_cs_request(gpmc_onenand_data->cs, ONENAND_IO_SIZE,
 				(unsigned long *)&gpmc_onenand_resource.start);
 	if (err < 0) {
-		pr_err("%s: Cannot request GPMC CS\n", __func__);
+		pr_err("%s: Cannot request GPMC CS %d, error %d\n",
+		       __func__, gpmc_onenand_data->cs, err);
 		return;
 	}
 
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 5/8] ARM: omap2: gpmc-onenand: Print something useful on CS request failure
@ 2013-02-12 15:18   ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

If CS request fails the current error message is rather unhelpful.
Fix it by printing the failing chip select and the error code.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index fadd8743..0ee5317 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -379,7 +379,8 @@ void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 	err = gpmc_cs_request(gpmc_onenand_data->cs, ONENAND_IO_SIZE,
 				(unsigned long *)&gpmc_onenand_resource.start);
 	if (err < 0) {
-		pr_err("%s: Cannot request GPMC CS\n", __func__);
+		pr_err("%s: Cannot request GPMC CS %d, error %d\n",
+		       __func__, gpmc_onenand_data->cs, err);
 		return;
 	}
 
-- 
1.7.8.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 6/8] ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
  2013-02-12 15:18 ` Ezequiel Garcia
@ 2013-02-12 15:18   ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Jon Hunter, Felipe Balbi, Tony Lindgren, Afzal Mohammed, Ezequiel Garcia

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 0ee5317..4771945 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -359,6 +359,7 @@ static int gpmc_onenand_setup(void __iomem *onenand_base, int *freq_ptr)
 void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 {
 	int err;
+	struct device *dev = &gpmc_onenand_device.dev;
 
 	gpmc_onenand_data = _onenand_data;
 	gpmc_onenand_data->onenand_setup = gpmc_onenand_setup;
@@ -379,8 +380,8 @@ void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 	err = gpmc_cs_request(gpmc_onenand_data->cs, ONENAND_IO_SIZE,
 				(unsigned long *)&gpmc_onenand_resource.start);
 	if (err < 0) {
-		pr_err("%s: Cannot request GPMC CS %d, error %d\n",
-		       __func__, gpmc_onenand_data->cs, err);
+		dev_err(dev, "Cannot request GPMC CS %d, error %d\n",
+			gpmc_onenand_data->cs, err);
 		return;
 	}
 
@@ -388,7 +389,7 @@ void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 							ONENAND_IO_SIZE - 1;
 
 	if (platform_device_register(&gpmc_onenand_device) < 0) {
-		pr_err("%s: Unable to register OneNAND device\n", __func__);
+		dev_err(dev, "Unable to register OneNAND device\n");
 		gpmc_cs_free(gpmc_onenand_data->cs);
 		return;
 	}
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 6/8] ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
@ 2013-02-12 15:18   ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 0ee5317..4771945 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -359,6 +359,7 @@ static int gpmc_onenand_setup(void __iomem *onenand_base, int *freq_ptr)
 void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 {
 	int err;
+	struct device *dev = &gpmc_onenand_device.dev;
 
 	gpmc_onenand_data = _onenand_data;
 	gpmc_onenand_data->onenand_setup = gpmc_onenand_setup;
@@ -379,8 +380,8 @@ void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 	err = gpmc_cs_request(gpmc_onenand_data->cs, ONENAND_IO_SIZE,
 				(unsigned long *)&gpmc_onenand_resource.start);
 	if (err < 0) {
-		pr_err("%s: Cannot request GPMC CS %d, error %d\n",
-		       __func__, gpmc_onenand_data->cs, err);
+		dev_err(dev, "Cannot request GPMC CS %d, error %d\n",
+			gpmc_onenand_data->cs, err);
 		return;
 	}
 
@@ -388,7 +389,7 @@ void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 							ONENAND_IO_SIZE - 1;
 
 	if (platform_device_register(&gpmc_onenand_device) < 0) {
-		pr_err("%s: Unable to register OneNAND device\n", __func__);
+		dev_err(dev, "Unable to register OneNAND device\n");
 		gpmc_cs_free(gpmc_onenand_data->cs);
 		return;
 	}
-- 
1.7.8.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 7/8] ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
  2013-02-12 15:18 ` Ezequiel Garcia
@ 2013-02-12 15:18   ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Jon Hunter, Felipe Balbi, Tony Lindgren, Afzal Mohammed, Ezequiel Garcia

Since the condition is not an error but a warning, replace
printk KERN_ERR with dev_warn.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 4771945..fd6e35b 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -367,7 +367,7 @@ void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 
 	if (cpu_is_omap24xx() &&
 			(gpmc_onenand_data->flags & ONENAND_SYNC_READWRITE)) {
-		printk(KERN_ERR "Onenand using only SYNC_READ on 24xx\n");
+		dev_warn(dev, "OneNAND using only SYNC_READ on 24xx\n");
 		gpmc_onenand_data->flags &= ~ONENAND_SYNC_READWRITE;
 		gpmc_onenand_data->flags |= ONENAND_SYNC_READ;
 	}
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 7/8] ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
@ 2013-02-12 15:18   ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

Since the condition is not an error but a warning, replace
printk KERN_ERR with dev_warn.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 4771945..fd6e35b 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -367,7 +367,7 @@ void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 
 	if (cpu_is_omap24xx() &&
 			(gpmc_onenand_data->flags & ONENAND_SYNC_READWRITE)) {
-		printk(KERN_ERR "Onenand using only SYNC_READ on 24xx\n");
+		dev_warn(dev, "OneNAND using only SYNC_READ on 24xx\n");
 		gpmc_onenand_data->flags &= ~ONENAND_SYNC_READWRITE;
 		gpmc_onenand_data->flags |= ONENAND_SYNC_READ;
 	}
-- 
1.7.8.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 8/8] ARM: omap2: gpmc: Remove redundant chip select out of range check
  2013-02-12 15:18 ` Ezequiel Garcia
@ 2013-02-12 15:18   ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Jon Hunter, Felipe Balbi, Tony Lindgren, Afzal Mohammed, Ezequiel Garcia

This check is done before the call to gpmc_cs_reserved() and
gpmc_cs_set_reserved() and it's redundant to do it again in each
function. This simplifies the code a bit.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index fa4764f..0201ea9 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -441,22 +441,14 @@ static int gpmc_cs_mem_enabled(int cs)
 	return l & GPMC_CONFIG7_CSVALID;
 }
 
-static int gpmc_cs_set_reserved(int cs, int reserved)
+static void gpmc_cs_set_reserved(int cs, int reserved)
 {
-	if (cs > GPMC_CS_NUM)
-		return -ENODEV;
-
 	gpmc_cs_map &= ~(1 << cs);
 	gpmc_cs_map |= (reserved ? 1 : 0) << cs;
-
-	return 0;
 }
 
 static bool gpmc_cs_reserved(int cs)
 {
-	if (cs > GPMC_CS_NUM)
-		return true;
-
 	return gpmc_cs_map & (1 << cs);
 }
 
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 8/8] ARM: omap2: gpmc: Remove redundant chip select out of range check
@ 2013-02-12 15:18   ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

This check is done before the call to gpmc_cs_reserved() and
gpmc_cs_set_reserved() and it's redundant to do it again in each
function. This simplifies the code a bit.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index fa4764f..0201ea9 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -441,22 +441,14 @@ static int gpmc_cs_mem_enabled(int cs)
 	return l & GPMC_CONFIG7_CSVALID;
 }
 
-static int gpmc_cs_set_reserved(int cs, int reserved)
+static void gpmc_cs_set_reserved(int cs, int reserved)
 {
-	if (cs > GPMC_CS_NUM)
-		return -ENODEV;
-
 	gpmc_cs_map &= ~(1 << cs);
 	gpmc_cs_map |= (reserved ? 1 : 0) << cs;
-
-	return 0;
 }
 
 static bool gpmc_cs_reserved(int cs)
 {
-	if (cs > GPMC_CS_NUM)
-		return true;
-
 	return gpmc_cs_map & (1 << cs);
 }
 
-- 
1.7.8.6

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 0/8] ARM: omap2: GPMC cleanup
  2013-02-12 15:18 ` Ezequiel Garcia
@ 2013-02-12 16:32   ` Jon Hunter
  -1 siblings, 0 replies; 34+ messages in thread
From: Jon Hunter @ 2013-02-12 16:32 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-omap, linux-arm-kernel, Felipe Balbi, Tony Lindgren,
	Afzal Mohammed


On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> This patchset is v2 of the small cleanup consisting in:
>  * mark some functions as 'static' when appropriate
>  * remove an unused function from gpmc.c
>  * improve error messages when a CS request fails
>  * migrate to dev_err and dev_warn
> 
> Changelog from v1:
>  * fix gpmc_cs_reserved to return a boolean instead
>    of an integer error code
>  * add a new patch to the patchset cleaning redundant checks
> 
> It has been tested on a IGEP v2 board with OneNAND,
> which means the gpmc-nand patch is tested by compilation only.
> 
> Altough these patchset is almost trivial,
> any feedback or testing is more than welcome.
> 
> Ezequiel Garcia (8):
>   ARM: omap2: gpmc: Mark local scoped functions static
>   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
>   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
>   ARM: omap2: gpmc-nand: Print something useful on CS request failure
>   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
>   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
>   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
>   ARM: omap2: gpmc: Remove redundant chip select out of range check
> 
>  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
>  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
>  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
>  arch/arm/mach-omap2/gpmc.h         |    7 -------
>  4 files changed, 13 insertions(+), 32 deletions(-)

Looks good to me. I noticed that for some patches there is no changelog
and I understand that that is because they are some what trivial
clean-ups and the subject explains the patch. However, typically it is
good to have a changelog in the patch no matter how trivial it is. Tony
may ask you to add a changelog. Otherwise ...

Reviewed-by: Jon Hunter <jon-hunter@ti.com>

Cheers
Jon

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 0/8] ARM: omap2: GPMC cleanup
@ 2013-02-12 16:32   ` Jon Hunter
  0 siblings, 0 replies; 34+ messages in thread
From: Jon Hunter @ 2013-02-12 16:32 UTC (permalink / raw)
  To: linux-arm-kernel


On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> This patchset is v2 of the small cleanup consisting in:
>  * mark some functions as 'static' when appropriate
>  * remove an unused function from gpmc.c
>  * improve error messages when a CS request fails
>  * migrate to dev_err and dev_warn
> 
> Changelog from v1:
>  * fix gpmc_cs_reserved to return a boolean instead
>    of an integer error code
>  * add a new patch to the patchset cleaning redundant checks
> 
> It has been tested on a IGEP v2 board with OneNAND,
> which means the gpmc-nand patch is tested by compilation only.
> 
> Altough these patchset is almost trivial,
> any feedback or testing is more than welcome.
> 
> Ezequiel Garcia (8):
>   ARM: omap2: gpmc: Mark local scoped functions static
>   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
>   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
>   ARM: omap2: gpmc-nand: Print something useful on CS request failure
>   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
>   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
>   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
>   ARM: omap2: gpmc: Remove redundant chip select out of range check
> 
>  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
>  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
>  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
>  arch/arm/mach-omap2/gpmc.h         |    7 -------
>  4 files changed, 13 insertions(+), 32 deletions(-)

Looks good to me. I noticed that for some patches there is no changelog
and I understand that that is because they are some what trivial
clean-ups and the subject explains the patch. However, typically it is
good to have a changelog in the patch no matter how trivial it is. Tony
may ask you to add a changelog. Otherwise ...

Reviewed-by: Jon Hunter <jon-hunter@ti.com>

Cheers
Jon

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 0/8] ARM: omap2: GPMC cleanup
  2013-02-12 16:32   ` Jon Hunter
@ 2013-02-12 17:12     ` Tony Lindgren
  -1 siblings, 0 replies; 34+ messages in thread
From: Tony Lindgren @ 2013-02-12 17:12 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Ezequiel Garcia, linux-omap, linux-arm-kernel, Felipe Balbi,
	Afzal Mohammed

* Jon Hunter <jon-hunter@ti.com> [130212 08:36]:
> 
> On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> > This patchset is v2 of the small cleanup consisting in:
> >  * mark some functions as 'static' when appropriate
> >  * remove an unused function from gpmc.c
> >  * improve error messages when a CS request fails
> >  * migrate to dev_err and dev_warn
> > 
> > Changelog from v1:
> >  * fix gpmc_cs_reserved to return a boolean instead
> >    of an integer error code
> >  * add a new patch to the patchset cleaning redundant checks
> > 
> > It has been tested on a IGEP v2 board with OneNAND,
> > which means the gpmc-nand patch is tested by compilation only.
> > 
> > Altough these patchset is almost trivial,
> > any feedback or testing is more than welcome.
> > 
> > Ezequiel Garcia (8):
> >   ARM: omap2: gpmc: Mark local scoped functions static
> >   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
> >   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
> >   ARM: omap2: gpmc-nand: Print something useful on CS request failure
> >   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
> >   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
> >   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
> >   ARM: omap2: gpmc: Remove redundant chip select out of range check
> > 
> >  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
> >  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
> >  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
> >  arch/arm/mach-omap2/gpmc.h         |    7 -------
> >  4 files changed, 13 insertions(+), 32 deletions(-)
> 
> Looks good to me. I noticed that for some patches there is no changelog
> and I understand that that is because they are some what trivial
> clean-ups and the subject explains the patch. However, typically it is
> good to have a changelog in the patch no matter how trivial it is. Tony
> may ask you to add a changelog. Otherwise ...
> 
> Reviewed-by: Jon Hunter <jon-hunter@ti.com>

Yes please add a changelog.

Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 0/8] ARM: omap2: GPMC cleanup
@ 2013-02-12 17:12     ` Tony Lindgren
  0 siblings, 0 replies; 34+ messages in thread
From: Tony Lindgren @ 2013-02-12 17:12 UTC (permalink / raw)
  To: linux-arm-kernel

* Jon Hunter <jon-hunter@ti.com> [130212 08:36]:
> 
> On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> > This patchset is v2 of the small cleanup consisting in:
> >  * mark some functions as 'static' when appropriate
> >  * remove an unused function from gpmc.c
> >  * improve error messages when a CS request fails
> >  * migrate to dev_err and dev_warn
> > 
> > Changelog from v1:
> >  * fix gpmc_cs_reserved to return a boolean instead
> >    of an integer error code
> >  * add a new patch to the patchset cleaning redundant checks
> > 
> > It has been tested on a IGEP v2 board with OneNAND,
> > which means the gpmc-nand patch is tested by compilation only.
> > 
> > Altough these patchset is almost trivial,
> > any feedback or testing is more than welcome.
> > 
> > Ezequiel Garcia (8):
> >   ARM: omap2: gpmc: Mark local scoped functions static
> >   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
> >   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
> >   ARM: omap2: gpmc-nand: Print something useful on CS request failure
> >   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
> >   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
> >   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
> >   ARM: omap2: gpmc: Remove redundant chip select out of range check
> > 
> >  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
> >  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
> >  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
> >  arch/arm/mach-omap2/gpmc.h         |    7 -------
> >  4 files changed, 13 insertions(+), 32 deletions(-)
> 
> Looks good to me. I noticed that for some patches there is no changelog
> and I understand that that is because they are some what trivial
> clean-ups and the subject explains the patch. However, typically it is
> good to have a changelog in the patch no matter how trivial it is. Tony
> may ask you to add a changelog. Otherwise ...
> 
> Reviewed-by: Jon Hunter <jon-hunter@ti.com>

Yes please add a changelog.

Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 0/8] ARM: omap2: GPMC cleanup
  2013-02-12 17:12     ` Tony Lindgren
@ 2013-02-12 18:26       ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 18:26 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Jon Hunter, linux-omap, linux-arm-kernel, Felipe Balbi, Afzal Mohammed

On Tue, Feb 12, 2013 at 09:12:53AM -0800, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [130212 08:36]:
> > 
> > On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> > > This patchset is v2 of the small cleanup consisting in:
> > >  * mark some functions as 'static' when appropriate
> > >  * remove an unused function from gpmc.c
> > >  * improve error messages when a CS request fails
> > >  * migrate to dev_err and dev_warn
> > > 
> > > Changelog from v1:
> > >  * fix gpmc_cs_reserved to return a boolean instead
> > >    of an integer error code
> > >  * add a new patch to the patchset cleaning redundant checks
> > > 
> > > It has been tested on a IGEP v2 board with OneNAND,
> > > which means the gpmc-nand patch is tested by compilation only.
> > > 
> > > Altough these patchset is almost trivial,
> > > any feedback or testing is more than welcome.
> > > 
> > > Ezequiel Garcia (8):
> > >   ARM: omap2: gpmc: Mark local scoped functions static
> > >   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
> > >   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
> > >   ARM: omap2: gpmc-nand: Print something useful on CS request failure
> > >   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
> > >   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
> > >   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
> > >   ARM: omap2: gpmc: Remove redundant chip select out of range check
> > > 
> > >  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
> > >  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
> > >  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
> > >  arch/arm/mach-omap2/gpmc.h         |    7 -------
> > >  4 files changed, 13 insertions(+), 32 deletions(-)
> > 
> > Looks good to me. I noticed that for some patches there is no changelog
> > and I understand that that is because they are some what trivial
> > clean-ups and the subject explains the patch. However, typically it is
> > good to have a changelog in the patch no matter how trivial it is. Tony
> > may ask you to add a changelog. Otherwise ...
> > 
> > Reviewed-by: Jon Hunter <jon-hunter@ti.com>
> 
> Yes please add a changelog.
> 

Patches with no changelog have no changelog ;-)

My usual workflow is to re-send a whole series, and only
add a changelog to the ones that actually change.
For instance, for this patchset, the only one that actually changed
is "ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value".

The rest is just the same it was in v1.

I like to do it this way, because I think it keeps the patches
together, and I hope I make maintainers life easier; of course,
I might be wrong.

So, should I use a different workflow and send only the patches
that change with new versions?

Thanks,

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 0/8] ARM: omap2: GPMC cleanup
@ 2013-02-12 18:26       ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 18:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 12, 2013 at 09:12:53AM -0800, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [130212 08:36]:
> > 
> > On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> > > This patchset is v2 of the small cleanup consisting in:
> > >  * mark some functions as 'static' when appropriate
> > >  * remove an unused function from gpmc.c
> > >  * improve error messages when a CS request fails
> > >  * migrate to dev_err and dev_warn
> > > 
> > > Changelog from v1:
> > >  * fix gpmc_cs_reserved to return a boolean instead
> > >    of an integer error code
> > >  * add a new patch to the patchset cleaning redundant checks
> > > 
> > > It has been tested on a IGEP v2 board with OneNAND,
> > > which means the gpmc-nand patch is tested by compilation only.
> > > 
> > > Altough these patchset is almost trivial,
> > > any feedback or testing is more than welcome.
> > > 
> > > Ezequiel Garcia (8):
> > >   ARM: omap2: gpmc: Mark local scoped functions static
> > >   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
> > >   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
> > >   ARM: omap2: gpmc-nand: Print something useful on CS request failure
> > >   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
> > >   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
> > >   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
> > >   ARM: omap2: gpmc: Remove redundant chip select out of range check
> > > 
> > >  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
> > >  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
> > >  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
> > >  arch/arm/mach-omap2/gpmc.h         |    7 -------
> > >  4 files changed, 13 insertions(+), 32 deletions(-)
> > 
> > Looks good to me. I noticed that for some patches there is no changelog
> > and I understand that that is because they are some what trivial
> > clean-ups and the subject explains the patch. However, typically it is
> > good to have a changelog in the patch no matter how trivial it is. Tony
> > may ask you to add a changelog. Otherwise ...
> > 
> > Reviewed-by: Jon Hunter <jon-hunter@ti.com>
> 
> Yes please add a changelog.
> 

Patches with no changelog have no changelog ;-)

My usual workflow is to re-send a whole series, and only
add a changelog to the ones that actually change.
For instance, for this patchset, the only one that actually changed
is "ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value".

The rest is just the same it was in v1.

I like to do it this way, because I think it keeps the patches
together, and I hope I make maintainers life easier; of course,
I might be wrong.

So, should I use a different workflow and send only the patches
that change with new versions?

Thanks,

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 0/8] ARM: omap2: GPMC cleanup
  2013-02-12 18:26       ` Ezequiel Garcia
@ 2013-02-12 18:43         ` Tony Lindgren
  -1 siblings, 0 replies; 34+ messages in thread
From: Tony Lindgren @ 2013-02-12 18:43 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Jon Hunter, linux-omap, linux-arm-kernel, Felipe Balbi, Afzal Mohammed

* Ezequiel Garcia <ezequiel.garcia@free-electrons.com> [130212 10:29]:
> On Tue, Feb 12, 2013 at 09:12:53AM -0800, Tony Lindgren wrote:
> > * Jon Hunter <jon-hunter@ti.com> [130212 08:36]:
> > > 
> > > On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> > > > This patchset is v2 of the small cleanup consisting in:
> > > >  * mark some functions as 'static' when appropriate
> > > >  * remove an unused function from gpmc.c
> > > >  * improve error messages when a CS request fails
> > > >  * migrate to dev_err and dev_warn
> > > > 
> > > > Changelog from v1:
> > > >  * fix gpmc_cs_reserved to return a boolean instead
> > > >    of an integer error code
> > > >  * add a new patch to the patchset cleaning redundant checks
> > > > 
> > > > It has been tested on a IGEP v2 board with OneNAND,
> > > > which means the gpmc-nand patch is tested by compilation only.
> > > > 
> > > > Altough these patchset is almost trivial,
> > > > any feedback or testing is more than welcome.
> > > > 
> > > > Ezequiel Garcia (8):
> > > >   ARM: omap2: gpmc: Mark local scoped functions static
> > > >   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
> > > >   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
> > > >   ARM: omap2: gpmc-nand: Print something useful on CS request failure
> > > >   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
> > > >   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
> > > >   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
> > > >   ARM: omap2: gpmc: Remove redundant chip select out of range check
> > > > 
> > > >  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
> > > >  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
> > > >  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
> > > >  arch/arm/mach-omap2/gpmc.h         |    7 -------
> > > >  4 files changed, 13 insertions(+), 32 deletions(-)
> > > 
> > > Looks good to me. I noticed that for some patches there is no changelog
> > > and I understand that that is because they are some what trivial
> > > clean-ups and the subject explains the patch. However, typically it is
> > > good to have a changelog in the patch no matter how trivial it is. Tony
> > > may ask you to add a changelog. Otherwise ...
> > > 
> > > Reviewed-by: Jon Hunter <jon-hunter@ti.com>
> > 
> > Yes please add a changelog.
> > 
> 
> Patches with no changelog have no changelog ;-)
> 
> My usual workflow is to re-send a whole series, and only
> add a changelog to the ones that actually change.
> For instance, for this patchset, the only one that actually changed
> is "ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value".
> 
> The rest is just the same it was in v1.
> 
> I like to do it this way, because I think it keeps the patches
> together, and I hope I make maintainers life easier; of course,
> I might be wrong.
> 
> So, should I use a different workflow and send only the patches
> that change with new versions?

Sorry I think there's a misunderstanding here.. Jon and I mean
that each patch should have a description in addition to the 
Subject line even if a trival patch :)

Regards,

Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 0/8] ARM: omap2: GPMC cleanup
@ 2013-02-12 18:43         ` Tony Lindgren
  0 siblings, 0 replies; 34+ messages in thread
From: Tony Lindgren @ 2013-02-12 18:43 UTC (permalink / raw)
  To: linux-arm-kernel

* Ezequiel Garcia <ezequiel.garcia@free-electrons.com> [130212 10:29]:
> On Tue, Feb 12, 2013 at 09:12:53AM -0800, Tony Lindgren wrote:
> > * Jon Hunter <jon-hunter@ti.com> [130212 08:36]:
> > > 
> > > On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> > > > This patchset is v2 of the small cleanup consisting in:
> > > >  * mark some functions as 'static' when appropriate
> > > >  * remove an unused function from gpmc.c
> > > >  * improve error messages when a CS request fails
> > > >  * migrate to dev_err and dev_warn
> > > > 
> > > > Changelog from v1:
> > > >  * fix gpmc_cs_reserved to return a boolean instead
> > > >    of an integer error code
> > > >  * add a new patch to the patchset cleaning redundant checks
> > > > 
> > > > It has been tested on a IGEP v2 board with OneNAND,
> > > > which means the gpmc-nand patch is tested by compilation only.
> > > > 
> > > > Altough these patchset is almost trivial,
> > > > any feedback or testing is more than welcome.
> > > > 
> > > > Ezequiel Garcia (8):
> > > >   ARM: omap2: gpmc: Mark local scoped functions static
> > > >   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
> > > >   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
> > > >   ARM: omap2: gpmc-nand: Print something useful on CS request failure
> > > >   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
> > > >   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
> > > >   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
> > > >   ARM: omap2: gpmc: Remove redundant chip select out of range check
> > > > 
> > > >  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
> > > >  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
> > > >  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
> > > >  arch/arm/mach-omap2/gpmc.h         |    7 -------
> > > >  4 files changed, 13 insertions(+), 32 deletions(-)
> > > 
> > > Looks good to me. I noticed that for some patches there is no changelog
> > > and I understand that that is because they are some what trivial
> > > clean-ups and the subject explains the patch. However, typically it is
> > > good to have a changelog in the patch no matter how trivial it is. Tony
> > > may ask you to add a changelog. Otherwise ...
> > > 
> > > Reviewed-by: Jon Hunter <jon-hunter@ti.com>
> > 
> > Yes please add a changelog.
> > 
> 
> Patches with no changelog have no changelog ;-)
> 
> My usual workflow is to re-send a whole series, and only
> add a changelog to the ones that actually change.
> For instance, for this patchset, the only one that actually changed
> is "ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value".
> 
> The rest is just the same it was in v1.
> 
> I like to do it this way, because I think it keeps the patches
> together, and I hope I make maintainers life easier; of course,
> I might be wrong.
> 
> So, should I use a different workflow and send only the patches
> that change with new versions?

Sorry I think there's a misunderstanding here.. Jon and I mean
that each patch should have a description in addition to the 
Subject line even if a trival patch :)

Regards,

Tony

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 0/8] ARM: omap2: GPMC cleanup
  2013-02-12 18:43         ` Tony Lindgren
@ 2013-02-12 18:46           ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 18:46 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Jon Hunter, linux-omap, linux-arm-kernel, Felipe Balbi, Afzal Mohammed

On Tue, Feb 12, 2013 at 10:43:25AM -0800, Tony Lindgren wrote:
> * Ezequiel Garcia <ezequiel.garcia@free-electrons.com> [130212 10:29]:
> > On Tue, Feb 12, 2013 at 09:12:53AM -0800, Tony Lindgren wrote:
> > > * Jon Hunter <jon-hunter@ti.com> [130212 08:36]:
> > > > 
> > > > On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> > > > > This patchset is v2 of the small cleanup consisting in:
> > > > >  * mark some functions as 'static' when appropriate
> > > > >  * remove an unused function from gpmc.c
> > > > >  * improve error messages when a CS request fails
> > > > >  * migrate to dev_err and dev_warn
> > > > > 
> > > > > Changelog from v1:
> > > > >  * fix gpmc_cs_reserved to return a boolean instead
> > > > >    of an integer error code
> > > > >  * add a new patch to the patchset cleaning redundant checks
> > > > > 
> > > > > It has been tested on a IGEP v2 board with OneNAND,
> > > > > which means the gpmc-nand patch is tested by compilation only.
> > > > > 
> > > > > Altough these patchset is almost trivial,
> > > > > any feedback or testing is more than welcome.
> > > > > 
> > > > > Ezequiel Garcia (8):
> > > > >   ARM: omap2: gpmc: Mark local scoped functions static
> > > > >   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
> > > > >   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
> > > > >   ARM: omap2: gpmc-nand: Print something useful on CS request failure
> > > > >   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
> > > > >   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
> > > > >   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
> > > > >   ARM: omap2: gpmc: Remove redundant chip select out of range check
> > > > > 
> > > > >  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
> > > > >  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
> > > > >  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
> > > > >  arch/arm/mach-omap2/gpmc.h         |    7 -------
> > > > >  4 files changed, 13 insertions(+), 32 deletions(-)
> > > > 
> > > > Looks good to me. I noticed that for some patches there is no changelog
> > > > and I understand that that is because they are some what trivial
> > > > clean-ups and the subject explains the patch. However, typically it is
> > > > good to have a changelog in the patch no matter how trivial it is. Tony
> > > > may ask you to add a changelog. Otherwise ...
> > > > 
> > > > Reviewed-by: Jon Hunter <jon-hunter@ti.com>
> > > 
> > > Yes please add a changelog.
> > > 
> > 
> > Patches with no changelog have no changelog ;-)
> > 
> > My usual workflow is to re-send a whole series, and only
> > add a changelog to the ones that actually change.
> > For instance, for this patchset, the only one that actually changed
> > is "ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value".
> > 
> > The rest is just the same it was in v1.
> > 
> > I like to do it this way, because I think it keeps the patches
> > together, and I hope I make maintainers life easier; of course,
> > I might be wrong.
> > 
> > So, should I use a different workflow and send only the patches
> > that change with new versions?
> 
> Sorry I think there's a misunderstanding here.. Jon and I mean
> that each patch should have a description in addition to the 
> Subject line even if a trival patch :)
> 

Oops, my bad! I'll re-send adding a description to each patch.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 0/8] ARM: omap2: GPMC cleanup
@ 2013-02-12 18:46           ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-12 18:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 12, 2013 at 10:43:25AM -0800, Tony Lindgren wrote:
> * Ezequiel Garcia <ezequiel.garcia@free-electrons.com> [130212 10:29]:
> > On Tue, Feb 12, 2013 at 09:12:53AM -0800, Tony Lindgren wrote:
> > > * Jon Hunter <jon-hunter@ti.com> [130212 08:36]:
> > > > 
> > > > On 02/12/2013 09:18 AM, Ezequiel Garcia wrote:
> > > > > This patchset is v2 of the small cleanup consisting in:
> > > > >  * mark some functions as 'static' when appropriate
> > > > >  * remove an unused function from gpmc.c
> > > > >  * improve error messages when a CS request fails
> > > > >  * migrate to dev_err and dev_warn
> > > > > 
> > > > > Changelog from v1:
> > > > >  * fix gpmc_cs_reserved to return a boolean instead
> > > > >    of an integer error code
> > > > >  * add a new patch to the patchset cleaning redundant checks
> > > > > 
> > > > > It has been tested on a IGEP v2 board with OneNAND,
> > > > > which means the gpmc-nand patch is tested by compilation only.
> > > > > 
> > > > > Altough these patchset is almost trivial,
> > > > > any feedback or testing is more than welcome.
> > > > > 
> > > > > Ezequiel Garcia (8):
> > > > >   ARM: omap2: gpmc: Mark local scoped functions static
> > > > >   ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function
> > > > >   ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
> > > > >   ARM: omap2: gpmc-nand: Print something useful on CS request failure
> > > > >   ARM: omap2: gpmc-onenand: Print something useful on CS request failure
> > > > >   ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()
> > > > >   ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()
> > > > >   ARM: omap2: gpmc: Remove redundant chip select out of range check
> > > > > 
> > > > >  arch/arm/mach-omap2/gpmc-nand.c    |    3 ++-
> > > > >  arch/arm/mach-omap2/gpmc-onenand.c |    8 +++++---
> > > > >  arch/arm/mach-omap2/gpmc.c         |   27 ++++++---------------------
> > > > >  arch/arm/mach-omap2/gpmc.h         |    7 -------
> > > > >  4 files changed, 13 insertions(+), 32 deletions(-)
> > > > 
> > > > Looks good to me. I noticed that for some patches there is no changelog
> > > > and I understand that that is because they are some what trivial
> > > > clean-ups and the subject explains the patch. However, typically it is
> > > > good to have a changelog in the patch no matter how trivial it is. Tony
> > > > may ask you to add a changelog. Otherwise ...
> > > > 
> > > > Reviewed-by: Jon Hunter <jon-hunter@ti.com>
> > > 
> > > Yes please add a changelog.
> > > 
> > 
> > Patches with no changelog have no changelog ;-)
> > 
> > My usual workflow is to re-send a whole series, and only
> > add a changelog to the ones that actually change.
> > For instance, for this patchset, the only one that actually changed
> > is "ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value".
> > 
> > The rest is just the same it was in v1.
> > 
> > I like to do it this way, because I think it keeps the patches
> > together, and I hope I make maintainers life easier; of course,
> > I might be wrong.
> > 
> > So, should I use a different workflow and send only the patches
> > that change with new versions?
> 
> Sorry I think there's a misunderstanding here.. Jon and I mean
> that each patch should have a description in addition to the 
> Subject line even if a trival patch :)
> 

Oops, my bad! I'll re-send adding a description to each patch.

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
  2013-02-12 15:18   ` Ezequiel Garcia
@ 2013-02-15 16:19     ` Anil Kumar
  -1 siblings, 0 replies; 34+ messages in thread
From: Anil Kumar @ 2013-02-15 16:19 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-omap, linux-arm-kernel, Tony Lindgren, Afzal Mohammed,
	Jon Hunter, Felipe Balbi

Hi,

On Tue, Feb 12, 2013 at 8:48 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Currently gpmc_cs_reserved() return value is somewhat inconsistent,
> returning a negative value on an error condition, a positive value
> if the chip select is reserved and zero if it's available.
>
> Fix this by returning a boolean value as the function name suggests:
>   * true if the chip select is reserved,
>   * false if it's available
>
> Suggested-by: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> Changelog from v1:
>  * As suggested by Felipe Balbi, fix return code to a boolean
>
>  arch/arm/mach-omap2/gpmc.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index bd3bc93..fa4764f 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -452,10 +452,10 @@ static int gpmc_cs_set_reserved(int cs, int reserved)
>         return 0;
>  }
>
> -static int gpmc_cs_reserved(int cs)
> +static bool gpmc_cs_reserved(int cs)
>  {
>         if (cs > GPMC_CS_NUM)
> -               return -ENODEV;
> +               return true;
>
>         return gpmc_cs_map & (1 << cs);
>  }

commit "6797b4fe0e554ce71f47038fd929c9ca929a9f3c"
Marking all the chip-selects as reserved by default.

In this case gpmc_cs_map is 0xff. So it will return 0x1 if cs is 0.
So gpmc_cs_request() function will fail in nand (gpmc-nand.c) case.

I am taking reference, branch "omap-for-v3.9/gpmc" of git tree
http://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git

I am sorry if this question is very vague.

Thanks,
Anil

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
@ 2013-02-15 16:19     ` Anil Kumar
  0 siblings, 0 replies; 34+ messages in thread
From: Anil Kumar @ 2013-02-15 16:19 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Tue, Feb 12, 2013 at 8:48 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Currently gpmc_cs_reserved() return value is somewhat inconsistent,
> returning a negative value on an error condition, a positive value
> if the chip select is reserved and zero if it's available.
>
> Fix this by returning a boolean value as the function name suggests:
>   * true if the chip select is reserved,
>   * false if it's available
>
> Suggested-by: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> Changelog from v1:
>  * As suggested by Felipe Balbi, fix return code to a boolean
>
>  arch/arm/mach-omap2/gpmc.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index bd3bc93..fa4764f 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -452,10 +452,10 @@ static int gpmc_cs_set_reserved(int cs, int reserved)
>         return 0;
>  }
>
> -static int gpmc_cs_reserved(int cs)
> +static bool gpmc_cs_reserved(int cs)
>  {
>         if (cs > GPMC_CS_NUM)
> -               return -ENODEV;
> +               return true;
>
>         return gpmc_cs_map & (1 << cs);
>  }

commit "6797b4fe0e554ce71f47038fd929c9ca929a9f3c"
Marking all the chip-selects as reserved by default.

In this case gpmc_cs_map is 0xff. So it will return 0x1 if cs is 0.
So gpmc_cs_request() function will fail in nand (gpmc-nand.c) case.

I am taking reference, branch "omap-for-v3.9/gpmc" of git tree
http://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git

I am sorry if this question is very vague.

Thanks,
Anil

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
  2013-02-15 16:19     ` Anil Kumar
@ 2013-02-15 17:01       ` Ezequiel Garcia
  -1 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-15 17:01 UTC (permalink / raw)
  To: Anil Kumar
  Cc: linux-omap, linux-arm-kernel, Tony Lindgren, Afzal Mohammed,
	Jon Hunter, Felipe Balbi

Hi Anil,

On Fri, Feb 15, 2013 at 09:49:21PM +0530, Anil Kumar wrote:
> Hi,
> 
> On Tue, Feb 12, 2013 at 8:48 PM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > Currently gpmc_cs_reserved() return value is somewhat inconsistent,
> > returning a negative value on an error condition, a positive value
> > if the chip select is reserved and zero if it's available.
> >
> > Fix this by returning a boolean value as the function name suggests:
> >   * true if the chip select is reserved,
> >   * false if it's available
> >
> > Suggested-by: Felipe Balbi <balbi@ti.com>
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> > Changelog from v1:
> >  * As suggested by Felipe Balbi, fix return code to a boolean
> >
> >  arch/arm/mach-omap2/gpmc.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> > index bd3bc93..fa4764f 100644
> > --- a/arch/arm/mach-omap2/gpmc.c
> > +++ b/arch/arm/mach-omap2/gpmc.c
> > @@ -452,10 +452,10 @@ static int gpmc_cs_set_reserved(int cs, int reserved)
> >         return 0;
> >  }
> >
> > -static int gpmc_cs_reserved(int cs)
> > +static bool gpmc_cs_reserved(int cs)
> >  {
> >         if (cs > GPMC_CS_NUM)
> > -               return -ENODEV;
> > +               return true;
> >
> >         return gpmc_cs_map & (1 << cs);
> >  }
> 
> commit "6797b4fe0e554ce71f47038fd929c9ca929a9f3c"
> Marking all the chip-selects as reserved by default.
> 
> In this case gpmc_cs_map is 0xff. So it will return 0x1 if cs is 0.
> So gpmc_cs_request() function will fail in nand (gpmc-nand.c) case.
> 
> I am taking reference, branch "omap-for-v3.9/gpmc" of git tree
> http://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git
> 
> I am sorry if this question is very vague.
> 

If I understand correctly your concern, I believe you've missed this
patch submitted by Jon Hunter and not yet merged:

  ARM: OMAP2+: Fix-up gpmc merge error

With this patch every chip-select is cleared on gpmc_probe()
as soon as GPMC driver initializes, and before we can request any
NAND/NOR child.

I hope this answers your question.

Feel free to test the series and send your Tested-by!

Thanks,
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
@ 2013-02-15 17:01       ` Ezequiel Garcia
  0 siblings, 0 replies; 34+ messages in thread
From: Ezequiel Garcia @ 2013-02-15 17:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Anil,

On Fri, Feb 15, 2013 at 09:49:21PM +0530, Anil Kumar wrote:
> Hi,
> 
> On Tue, Feb 12, 2013 at 8:48 PM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > Currently gpmc_cs_reserved() return value is somewhat inconsistent,
> > returning a negative value on an error condition, a positive value
> > if the chip select is reserved and zero if it's available.
> >
> > Fix this by returning a boolean value as the function name suggests:
> >   * true if the chip select is reserved,
> >   * false if it's available
> >
> > Suggested-by: Felipe Balbi <balbi@ti.com>
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> > Changelog from v1:
> >  * As suggested by Felipe Balbi, fix return code to a boolean
> >
> >  arch/arm/mach-omap2/gpmc.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> > index bd3bc93..fa4764f 100644
> > --- a/arch/arm/mach-omap2/gpmc.c
> > +++ b/arch/arm/mach-omap2/gpmc.c
> > @@ -452,10 +452,10 @@ static int gpmc_cs_set_reserved(int cs, int reserved)
> >         return 0;
> >  }
> >
> > -static int gpmc_cs_reserved(int cs)
> > +static bool gpmc_cs_reserved(int cs)
> >  {
> >         if (cs > GPMC_CS_NUM)
> > -               return -ENODEV;
> > +               return true;
> >
> >         return gpmc_cs_map & (1 << cs);
> >  }
> 
> commit "6797b4fe0e554ce71f47038fd929c9ca929a9f3c"
> Marking all the chip-selects as reserved by default.
> 
> In this case gpmc_cs_map is 0xff. So it will return 0x1 if cs is 0.
> So gpmc_cs_request() function will fail in nand (gpmc-nand.c) case.
> 
> I am taking reference, branch "omap-for-v3.9/gpmc" of git tree
> http://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git
> 
> I am sorry if this question is very vague.
> 

If I understand correctly your concern, I believe you've missed this
patch submitted by Jon Hunter and not yet merged:

  ARM: OMAP2+: Fix-up gpmc merge error

With this patch every chip-select is cleared on gpmc_probe()
as soon as GPMC driver initializes, and before we can request any
NAND/NOR child.

I hope this answers your question.

Feel free to test the series and send your Tested-by!

Thanks,
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
  2013-02-15 17:01       ` Ezequiel Garcia
@ 2013-02-15 17:24         ` Anil Kumar
  -1 siblings, 0 replies; 34+ messages in thread
From: Anil Kumar @ 2013-02-15 17:24 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-omap, linux-arm-kernel, Tony Lindgren, Afzal Mohammed,
	Jon Hunter, Felipe Balbi

Hi,

On Fri, Feb 15, 2013 at 10:31 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Hi Anil,
>
> On Fri, Feb 15, 2013 at 09:49:21PM +0530, Anil Kumar wrote:
>> Hi,
>>
>> On Tue, Feb 12, 2013 at 8:48 PM, Ezequiel Garcia
>> <ezequiel.garcia@free-electrons.com> wrote:
>> > Currently gpmc_cs_reserved() return value is somewhat inconsistent,
>> > returning a negative value on an error condition, a positive value
>> > if the chip select is reserved and zero if it's available.
>> >
>> > Fix this by returning a boolean value as the function name suggests:
>> >   * true if the chip select is reserved,
>> >   * false if it's available
>> >
>> > Suggested-by: Felipe Balbi <balbi@ti.com>
>> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> > ---
>> > Changelog from v1:
>> >  * As suggested by Felipe Balbi, fix return code to a boolean
>> >
>> >  arch/arm/mach-omap2/gpmc.c |    4 ++--
>> >  1 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> > index bd3bc93..fa4764f 100644
>> > --- a/arch/arm/mach-omap2/gpmc.c
>> > +++ b/arch/arm/mach-omap2/gpmc.c
>> > @@ -452,10 +452,10 @@ static int gpmc_cs_set_reserved(int cs, int reserved)
>> >         return 0;
>> >  }
>> >
>> > -static int gpmc_cs_reserved(int cs)
>> > +static bool gpmc_cs_reserved(int cs)
>> >  {
>> >         if (cs > GPMC_CS_NUM)
>> > -               return -ENODEV;
>> > +               return true;
>> >
>> >         return gpmc_cs_map & (1 << cs);
>> >  }
>>
>> commit "6797b4fe0e554ce71f47038fd929c9ca929a9f3c"
>> Marking all the chip-selects as reserved by default.
>>
>> In this case gpmc_cs_map is 0xff. So it will return 0x1 if cs is 0.
>> So gpmc_cs_request() function will fail in nand (gpmc-nand.c) case.
>>
>> I am taking reference, branch "omap-for-v3.9/gpmc" of git tree
>> http://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git
>>
>> I am sorry if this question is very vague.
>>
>
> If I understand correctly your concern, I believe you've missed this
> patch submitted by Jon Hunter and not yet merged:
>
>   ARM: OMAP2+: Fix-up gpmc merge error
>
> With this patch every chip-select is cleared on gpmc_probe()
> as soon as GPMC driver initializes, and before we can request any
> NAND/NOR child.
>
> I hope this answers your question.
>

Yes, with patch  "ARM: OMAP2+: Fix-up gpmc merge error" now nand is
detected on my
devkit8000(omap3 based board) with DT boot.

Thank you very much.
Anil

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value
@ 2013-02-15 17:24         ` Anil Kumar
  0 siblings, 0 replies; 34+ messages in thread
From: Anil Kumar @ 2013-02-15 17:24 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Fri, Feb 15, 2013 at 10:31 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Hi Anil,
>
> On Fri, Feb 15, 2013 at 09:49:21PM +0530, Anil Kumar wrote:
>> Hi,
>>
>> On Tue, Feb 12, 2013 at 8:48 PM, Ezequiel Garcia
>> <ezequiel.garcia@free-electrons.com> wrote:
>> > Currently gpmc_cs_reserved() return value is somewhat inconsistent,
>> > returning a negative value on an error condition, a positive value
>> > if the chip select is reserved and zero if it's available.
>> >
>> > Fix this by returning a boolean value as the function name suggests:
>> >   * true if the chip select is reserved,
>> >   * false if it's available
>> >
>> > Suggested-by: Felipe Balbi <balbi@ti.com>
>> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> > ---
>> > Changelog from v1:
>> >  * As suggested by Felipe Balbi, fix return code to a boolean
>> >
>> >  arch/arm/mach-omap2/gpmc.c |    4 ++--
>> >  1 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> > index bd3bc93..fa4764f 100644
>> > --- a/arch/arm/mach-omap2/gpmc.c
>> > +++ b/arch/arm/mach-omap2/gpmc.c
>> > @@ -452,10 +452,10 @@ static int gpmc_cs_set_reserved(int cs, int reserved)
>> >         return 0;
>> >  }
>> >
>> > -static int gpmc_cs_reserved(int cs)
>> > +static bool gpmc_cs_reserved(int cs)
>> >  {
>> >         if (cs > GPMC_CS_NUM)
>> > -               return -ENODEV;
>> > +               return true;
>> >
>> >         return gpmc_cs_map & (1 << cs);
>> >  }
>>
>> commit "6797b4fe0e554ce71f47038fd929c9ca929a9f3c"
>> Marking all the chip-selects as reserved by default.
>>
>> In this case gpmc_cs_map is 0xff. So it will return 0x1 if cs is 0.
>> So gpmc_cs_request() function will fail in nand (gpmc-nand.c) case.
>>
>> I am taking reference, branch "omap-for-v3.9/gpmc" of git tree
>> http://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git
>>
>> I am sorry if this question is very vague.
>>
>
> If I understand correctly your concern, I believe you've missed this
> patch submitted by Jon Hunter and not yet merged:
>
>   ARM: OMAP2+: Fix-up gpmc merge error
>
> With this patch every chip-select is cleared on gpmc_probe()
> as soon as GPMC driver initializes, and before we can request any
> NAND/NOR child.
>
> I hope this answers your question.
>

Yes, with patch  "ARM: OMAP2+: Fix-up gpmc merge error" now nand is
detected on my
devkit8000(omap3 based board) with DT boot.

Thank you very much.
Anil

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2013-02-15 17:24 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-12 15:18 [PATCH v2 0/8] ARM: omap2: GPMC cleanup Ezequiel Garcia
2013-02-12 15:18 ` Ezequiel Garcia
2013-02-12 15:18 ` [PATCH v2 1/8] ARM: omap2: gpmc: Mark local scoped functions static Ezequiel Garcia
2013-02-12 15:18   ` Ezequiel Garcia
2013-02-12 15:18 ` [PATCH v2 2/8] ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function Ezequiel Garcia
2013-02-12 15:18   ` Ezequiel Garcia
2013-02-12 15:18 ` [PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value Ezequiel Garcia
2013-02-12 15:18   ` Ezequiel Garcia
2013-02-15 16:19   ` Anil Kumar
2013-02-15 16:19     ` Anil Kumar
2013-02-15 17:01     ` Ezequiel Garcia
2013-02-15 17:01       ` Ezequiel Garcia
2013-02-15 17:24       ` Anil Kumar
2013-02-15 17:24         ` Anil Kumar
2013-02-12 15:18 ` [PATCH v2 4/8] ARM: omap2: gpmc-nand: Print something useful on CS request failure Ezequiel Garcia
2013-02-12 15:18   ` Ezequiel Garcia
2013-02-12 15:18 ` [PATCH v2 5/8] ARM: omap2: gpmc-onenand: " Ezequiel Garcia
2013-02-12 15:18   ` Ezequiel Garcia
2013-02-12 15:18 ` [PATCH v2 6/8] ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err() Ezequiel Garcia
2013-02-12 15:18   ` Ezequiel Garcia
2013-02-12 15:18 ` [PATCH v2 7/8] ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn() Ezequiel Garcia
2013-02-12 15:18   ` Ezequiel Garcia
2013-02-12 15:18 ` [PATCH v2 8/8] ARM: omap2: gpmc: Remove redundant chip select out of range check Ezequiel Garcia
2013-02-12 15:18   ` Ezequiel Garcia
2013-02-12 16:32 ` [PATCH v2 0/8] ARM: omap2: GPMC cleanup Jon Hunter
2013-02-12 16:32   ` Jon Hunter
2013-02-12 17:12   ` Tony Lindgren
2013-02-12 17:12     ` Tony Lindgren
2013-02-12 18:26     ` Ezequiel Garcia
2013-02-12 18:26       ` Ezequiel Garcia
2013-02-12 18:43       ` Tony Lindgren
2013-02-12 18:43         ` Tony Lindgren
2013-02-12 18:46         ` Ezequiel Garcia
2013-02-12 18:46           ` Ezequiel Garcia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.