[PATCH v2 0/8] ARM: omap2: GPMC cleanup

[PATCH v2 0/8] ARM: omap2: GPMC cleanup

[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

[PATCH v2 1/8] ARM: omap2: gpmc: Mark local scoped functions static

[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

[PATCH v2 2/8] ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function

[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

[PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value

[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

[PATCH v2 4/8] ARM: omap2: gpmc-nand: Print something useful on CS request failure

[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

[PATCH v2 5/8] ARM: omap2: gpmc-onenand: Print something useful on CS request failure

[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

[PATCH v2 6/8] ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()

[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

[PATCH v2 7/8] ARM: omap2: gpmc-onenand: Replace printk KERN_ERR with dev_warn()

[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

[PATCH v2 8/8] ARM: omap2: gpmc: Remove redundant chip select out of range check

[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

[PATCH v2 0/8] ARM: omap2: GPMC cleanup

[Jon Hunter]

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

[PATCH v2 0/8] ARM: omap2: GPMC cleanup

[Tony Lindgren]

* 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

[PATCH v2 0/8] ARM: omap2: GPMC cleanup

[Ezequiel Garcia]

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

[PATCH v2 0/8] ARM: omap2: GPMC cleanup

[Tony Lindgren]

* 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

[PATCH v2 0/8] ARM: omap2: GPMC cleanup

[Ezequiel Garcia]

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

[PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value

[Anil Kumar]

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

[PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value

[Ezequiel Garcia]

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

[PATCH v2 3/8] ARM: omap2: gpmc: Fix gpmc_cs_reserved() return value

[Anil Kumar]

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