Re: [PATCH v2] ARM: OMAP2+: fix gpmc_cs_remap: re-allocating chip-select address space based on DT

2014-08-26 Thread Roger Quadros
On 08/25/2014 09:50 PM, Tony Lindgren wrote:
 * Roger Quadros rog...@ti.com [140825 04:27]:
 From: Pekon Gupta pe...@ti.com

 Each GPMC chip-select needs to be configured for (base-address,CS-size) so 
 that
 GPMC understands the address-space allocated to device connected externally.
 These chip-select configurations (base-address, CS-size) follow some basic
 mapping rules like:
 - The CS size is programmable from 256 MBytes to 16 MBytes (must be a power 
 of 2)
   and is defined by the mask field. Attached memory smaller than the 
 programmed
   CS region size is accessed through the entire CS region (aliasing).
 - The programmed 'base-address' must be aligned to the 'CS-size' boundary and
   be a power of 2.
 - Valid CS-size values are {256MB(max), 128MB, 64MB, 32MB and 16MB (min)}
   Any intermediate values creates holes in the chip-select memory-map.

 This patch adds above checks in gpmc_cs_remap() so that any invalid value
 passed by DT reg property can be filtered before actually allocating the
 address space.
 
 There's now an issue here where it mixes up the configured CS
 size and the configured device IO size. With this patch GPMC based
 Ethernet devices trigger warning at arch/arm/mach-omap2/gpmc.c:556
 as we have the minimal GPMC CS range of 16MB with the smsc IO size
 being either 0xf or 0xff depending on the model.
 
 So that check should be done on the CS size, not the device IO size.
 

Good catch. 

This patch does not address the following issues from long term point of view
- The remap should be done not only for NOR/Ethernet devices but for all the
GPMC child nodes.
- The base address/size should be taken from the reg property
of the GPMC node and not of the child node as done in this patch.

Please drop this patch. I will include this in the clean up series which
I will post once ready. Currently I'm caught up with bug fixing and settling on
a stable version of GPMC which works on all boards that I have.

cheers,
-roger

 Regards,
 
 Tony
  
 [rog...@ti.com] typo and print message fixes.

 Signed-off-by: Pekon Gupta pe...@ti.com
 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  arch/arm/mach-omap2/gpmc.c | 51 
 +-
  1 file changed, 37 insertions(+), 14 deletions(-)

 diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
 index 8bc1338..793f3a9 100644
 --- a/arch/arm/mach-omap2/gpmc.c
 +++ b/arch/arm/mach-omap2/gpmc.c
 @@ -516,31 +516,54 @@ static int gpmc_cs_delete_mem(int cs)
   * gpmc_cs_remap - remaps a chip-select physical base address
   * @cs: chip-select to remap
   * @base:   physical base address to re-map chip-select to
 + * @size:   size of the chip select map
   *
   * Re-maps a chip-select to a new physical base address specified by
   * base. Returns 0 on success and appropriate negative error code
 - * on failure.
 + * on failure. size of the map must be either 16M, 32M, 64M or 128M.
   */
 -static int gpmc_cs_remap(int cs, u32 base)
 +static int gpmc_cs_remap(int cs, u32 base, u32 size)
  {
  int ret;
 -u32 old_base, size;
  
  if (cs  gpmc_cs_num) {
  pr_err(%s: requested chip-select is disabled\n, __func__);
  return -ENODEV;
  }
  
 -/*
 - * Make sure we ignore any device offsets from the GPMC partition
 - * allocated for the chip select and that the new base confirms
 - * to the GPMC 16MB minimum granularity.
 - */ 
 -base = ~(SZ_16M - 1);
 -
 -gpmc_cs_get_memconf(cs, old_base, size);
 -if (base == old_base)
 -return 0;
 +/* Align size to meet SoC limitations */
 +if (size  SZ_256M) {
 +pr_err(%s: CS memory map  256MB not supported\n, __func__);
 +return -ENODEV;
 +} else if (size  SZ_128M) {
 +WARN((size != SZ_256M), %s: cs=%d: allocating 256MB\n,
 + __func__, cs);
 +size = SZ_256M;
 +} else if (size  SZ_64M) {
 +WARN((size != SZ_128M), %s: cs=%d: allocating 128MB\n,
 + __func__, cs);
 +size = SZ_128M;
 +} else if (size  SZ_32M) {
 +WARN((size != SZ_64M), %s: cs=%d: allocating 64MB\n,
 + __func__, cs);
 +size = SZ_64M;
 +} else if (size  SZ_16M) {
 +WARN((size != SZ_32M), %s: cs=%d: allocating 32MB\n,
 + __func__, cs);
 +size = SZ_32M;
 +} else {
 +WARN((size != SZ_16M), %s: cs=%d: allocating 16MB\n,
 + __func__, cs);
 +size = SZ_16M;
 +}
 +
 +/* base address should be aligned with address-space size */
 +if (base  (size - 1)) {
 +pr_err(%s: cs base-addr: %x should be aligned to cs size: %x,
 +   __func__, base, size);
 +return -EINVAL;
 +}
 +
  gpmc_cs_disable_mem(cs);
  ret = gpmc_cs_delete_mem(cs);
  if (ret  0)
 @@ -1551,7 +1574,7 @@ static int gpmc_probe_generic_child(struct 
 

[PATCH v2] ARM: OMAP2+: fix gpmc_cs_remap: re-allocating chip-select address space based on DT

2014-08-25 Thread Roger Quadros
From: Pekon Gupta pe...@ti.com

Each GPMC chip-select needs to be configured for (base-address,CS-size) so that
GPMC understands the address-space allocated to device connected externally.
These chip-select configurations (base-address, CS-size) follow some basic
mapping rules like:
- The CS size is programmable from 256 MBytes to 16 MBytes (must be a power of 
2)
  and is defined by the mask field. Attached memory smaller than the programmed
  CS region size is accessed through the entire CS region (aliasing).
- The programmed 'base-address' must be aligned to the 'CS-size' boundary and
  be a power of 2.
- Valid CS-size values are {256MB(max), 128MB, 64MB, 32MB and 16MB (min)}
  Any intermediate values creates holes in the chip-select memory-map.

This patch adds above checks in gpmc_cs_remap() so that any invalid value
passed by DT reg property can be filtered before actually allocating the
address space.

[rog...@ti.com] typo and print message fixes.

Signed-off-by: Pekon Gupta pe...@ti.com
Signed-off-by: Roger Quadros rog...@ti.com
---
 arch/arm/mach-omap2/gpmc.c | 51 +-
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 8bc1338..793f3a9 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -516,31 +516,54 @@ static int gpmc_cs_delete_mem(int cs)
  * gpmc_cs_remap - remaps a chip-select physical base address
  * @cs:chip-select to remap
  * @base:  physical base address to re-map chip-select to
+ * @size:  size of the chip select map
  *
  * Re-maps a chip-select to a new physical base address specified by
  * base. Returns 0 on success and appropriate negative error code
- * on failure.
+ * on failure. size of the map must be either 16M, 32M, 64M or 128M.
  */
-static int gpmc_cs_remap(int cs, u32 base)
+static int gpmc_cs_remap(int cs, u32 base, u32 size)
 {
int ret;
-   u32 old_base, size;
 
if (cs  gpmc_cs_num) {
pr_err(%s: requested chip-select is disabled\n, __func__);
return -ENODEV;
}
 
-   /*
-* Make sure we ignore any device offsets from the GPMC partition
-* allocated for the chip select and that the new base confirms
-* to the GPMC 16MB minimum granularity.
-*/ 
-   base = ~(SZ_16M - 1);
-
-   gpmc_cs_get_memconf(cs, old_base, size);
-   if (base == old_base)
-   return 0;
+   /* Align size to meet SoC limitations */
+   if (size  SZ_256M) {
+   pr_err(%s: CS memory map  256MB not supported\n, __func__);
+   return -ENODEV;
+   } else if (size  SZ_128M) {
+   WARN((size != SZ_256M), %s: cs=%d: allocating 256MB\n,
+__func__, cs);
+   size = SZ_256M;
+   } else if (size  SZ_64M) {
+   WARN((size != SZ_128M), %s: cs=%d: allocating 128MB\n,
+__func__, cs);
+   size = SZ_128M;
+   } else if (size  SZ_32M) {
+   WARN((size != SZ_64M), %s: cs=%d: allocating 64MB\n,
+__func__, cs);
+   size = SZ_64M;
+   } else if (size  SZ_16M) {
+   WARN((size != SZ_32M), %s: cs=%d: allocating 32MB\n,
+__func__, cs);
+   size = SZ_32M;
+   } else {
+   WARN((size != SZ_16M), %s: cs=%d: allocating 16MB\n,
+__func__, cs);
+   size = SZ_16M;
+   }
+
+   /* base address should be aligned with address-space size */
+   if (base  (size - 1)) {
+   pr_err(%s: cs base-addr: %x should be aligned to cs size: %x,
+  __func__, base, size);
+   return -EINVAL;
+   }
+
gpmc_cs_disable_mem(cs);
ret = gpmc_cs_delete_mem(cs);
if (ret  0)
@@ -1551,7 +1574,7 @@ static int gpmc_probe_generic_child(struct 
platform_device *pdev,
 * CS to this location. Once DT migration is complete should
 * just make gpmc_cs_request() map a specific address.
 */
-   ret = gpmc_cs_remap(cs, res.start);
+   ret = gpmc_cs_remap(cs, res.start, resource_size(res));
if (ret  0) {
dev_err(pdev-dev, cannot remap GPMC CS %d to %pa\n,
cs, res.start);
-- 
1.8.3.2

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] ARM: OMAP2+: fix gpmc_cs_remap: re-allocating chip-select address space based on DT

2014-08-25 Thread Tony Lindgren
* Roger Quadros rog...@ti.com [140825 04:27]:
 From: Pekon Gupta pe...@ti.com
 
 Each GPMC chip-select needs to be configured for (base-address,CS-size) so 
 that
 GPMC understands the address-space allocated to device connected externally.
 These chip-select configurations (base-address, CS-size) follow some basic
 mapping rules like:
 - The CS size is programmable from 256 MBytes to 16 MBytes (must be a power 
 of 2)
   and is defined by the mask field. Attached memory smaller than the 
 programmed
   CS region size is accessed through the entire CS region (aliasing).
 - The programmed 'base-address' must be aligned to the 'CS-size' boundary and
   be a power of 2.
 - Valid CS-size values are {256MB(max), 128MB, 64MB, 32MB and 16MB (min)}
   Any intermediate values creates holes in the chip-select memory-map.
 
 This patch adds above checks in gpmc_cs_remap() so that any invalid value
 passed by DT reg property can be filtered before actually allocating the
 address space.

There's now an issue here where it mixes up the configured CS
size and the configured device IO size. With this patch GPMC based
Ethernet devices trigger warning at arch/arm/mach-omap2/gpmc.c:556
as we have the minimal GPMC CS range of 16MB with the smsc IO size
being either 0xf or 0xff depending on the model.

So that check should be done on the CS size, not the device IO size.

Regards,

Tony
 
 [rog...@ti.com] typo and print message fixes.
 
 Signed-off-by: Pekon Gupta pe...@ti.com
 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  arch/arm/mach-omap2/gpmc.c | 51 
 +-
  1 file changed, 37 insertions(+), 14 deletions(-)
 
 diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
 index 8bc1338..793f3a9 100644
 --- a/arch/arm/mach-omap2/gpmc.c
 +++ b/arch/arm/mach-omap2/gpmc.c
 @@ -516,31 +516,54 @@ static int gpmc_cs_delete_mem(int cs)
   * gpmc_cs_remap - remaps a chip-select physical base address
   * @cs:  chip-select to remap
   * @base:physical base address to re-map chip-select to
 + * @size:size of the chip select map
   *
   * Re-maps a chip-select to a new physical base address specified by
   * base. Returns 0 on success and appropriate negative error code
 - * on failure.
 + * on failure. size of the map must be either 16M, 32M, 64M or 128M.
   */
 -static int gpmc_cs_remap(int cs, u32 base)
 +static int gpmc_cs_remap(int cs, u32 base, u32 size)
  {
   int ret;
 - u32 old_base, size;
  
   if (cs  gpmc_cs_num) {
   pr_err(%s: requested chip-select is disabled\n, __func__);
   return -ENODEV;
   }
  
 - /*
 -  * Make sure we ignore any device offsets from the GPMC partition
 -  * allocated for the chip select and that the new base confirms
 -  * to the GPMC 16MB minimum granularity.
 -  */ 
 - base = ~(SZ_16M - 1);
 -
 - gpmc_cs_get_memconf(cs, old_base, size);
 - if (base == old_base)
 - return 0;
 + /* Align size to meet SoC limitations */
 + if (size  SZ_256M) {
 + pr_err(%s: CS memory map  256MB not supported\n, __func__);
 + return -ENODEV;
 + } else if (size  SZ_128M) {
 + WARN((size != SZ_256M), %s: cs=%d: allocating 256MB\n,
 +  __func__, cs);
 + size = SZ_256M;
 + } else if (size  SZ_64M) {
 + WARN((size != SZ_128M), %s: cs=%d: allocating 128MB\n,
 +  __func__, cs);
 + size = SZ_128M;
 + } else if (size  SZ_32M) {
 + WARN((size != SZ_64M), %s: cs=%d: allocating 64MB\n,
 +  __func__, cs);
 + size = SZ_64M;
 + } else if (size  SZ_16M) {
 + WARN((size != SZ_32M), %s: cs=%d: allocating 32MB\n,
 +  __func__, cs);
 + size = SZ_32M;
 + } else {
 + WARN((size != SZ_16M), %s: cs=%d: allocating 16MB\n,
 +  __func__, cs);
 + size = SZ_16M;
 + }
 +
 + /* base address should be aligned with address-space size */
 + if (base  (size - 1)) {
 + pr_err(%s: cs base-addr: %x should be aligned to cs size: %x,
 +__func__, base, size);
 + return -EINVAL;
 + }
 +
   gpmc_cs_disable_mem(cs);
   ret = gpmc_cs_delete_mem(cs);
   if (ret  0)
 @@ -1551,7 +1574,7 @@ static int gpmc_probe_generic_child(struct 
 platform_device *pdev,
* CS to this location. Once DT migration is complete should
* just make gpmc_cs_request() map a specific address.
*/
 - ret = gpmc_cs_remap(cs, res.start);
 + ret = gpmc_cs_remap(cs, res.start, resource_size(res));
   if (ret  0) {
   dev_err(pdev-dev, cannot remap GPMC CS %d to %pa\n,
   cs, res.start);
 -- 
 1.8.3.2
 
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at