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

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

[Ezequiel Garcia]

This patchset is v3 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

Changes from v2:
 * add a commit message to some trivial patches,
   that omitted it due to author's laziness.

Changes 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 this patchset is almost trivial,
any feedback or testing is more than welcome.

Thanks to Jon Hunter for his kind review!

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 v3 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>
Reviewed-by: Jon Hunter <jon-hunter@ti.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 v3 2/8] ARM: omap2: gpmc: Remove unused gpmc_round_ns_to_ticks() function

[Ezequiel Garcia]

This function is not used anywhere, so it's safe to remove it.
This means less code to maintain.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Reviewed-by: Jon Hunter <jon-hunter@ti.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 v3 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>
Reviewed-by: Jon Hunter <jon-hunter@ti.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 v3 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>
Reviewed-by: Jon Hunter <jon-hunter@ti.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 v3 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>
Reviewed-by: Jon Hunter <jon-hunter@ti.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 v3 6/8] ARM: omap2: gpmc-onenand: Replace pr_err() with dev_err()

[Ezequiel Garcia]

Do this becasue dev_err() is preferred over pr_err() and because
it will match gpmc-nand, thus the code shows looks more consistent.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Reviewed-by: Jon Hunter <jon-hunter@ti.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 v3 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>
Reviewed-by: Jon Hunter <jon-hunter@ti.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 v3 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>
Reviewed-by: Jon Hunter <jon-hunter@ti.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

Re: [PATCH v3 0/8] ARM: omap2: GPMC cleanup

[Tony Lindgren]

* Ezequiel Garcia <ezequiel.garcia@free-electrons.com> [130212 11:26]:
> This patchset is v3 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
> 
> Changes from v2:
>  * add a commit message to some trivial patches,
>    that omitted it due to author's laziness.
> 
> Changes 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 this patchset is almost trivial,
> any feedback or testing is more than welcome.
> 
> Thanks to Jon Hunter for his kind review!

Looks good to me. Jon, can you please queue all the GPMC related
patches into one pull request around -rc2 time assuming no more
comments? 

Regards,

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

Re: [PATCH v3 0/8] ARM: omap2: GPMC cleanup

[Jon Hunter]

On 03/04/2013 12:53 PM, Tony Lindgren wrote:
> * Ezequiel Garcia <ezequiel.garcia@free-electrons.com> [130212 11:26]:
>> This patchset is v3 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
>>
>> Changes from v2:
>>  * add a commit message to some trivial patches,
>>    that omitted it due to author's laziness.
>>
>> Changes 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 this patchset is almost trivial,
>> any feedback or testing is more than welcome.
>>
>> Thanks to Jon Hunter for his kind review!
> 
> Looks good to me. Jon, can you please queue all the GPMC related
> patches into one pull request around -rc2 time assuming no more
> comments? 

Yes will do.

Jon