[PATCH 0/2] ARM: OMAP2+: GPMC Fixes

[PATCH 0/2] ARM: OMAP2+: GPMC Fixes

[Jon Hunter]

A couple GPMC fixes for OMAP2+ devices which were causing boot problems
on OMAP2420 H4.

Please note that long term we need to remove the cpu_is_omap24xx() from
the gpmc driver and pass the gpmc_mem_root information via platform data
and figure out a way to pass this information for device tree.

Boot tested on OMAP2420 H4, OMAP3430 SDP, OMAP4430 SDP and AM335x EVM.

Jon Hunter (2):
  ARM: OMAP2+: Prevent potential crash if GPMC probe fails
  ARM: OMAP2: Fix GPMC memory initialisation

 arch/arm/mach-omap2/gpmc.c |   31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

-- 
1.7.10.4

[PATCH 1/2] ARM: OMAP2+: Prevent potential crash if GPMC probe fails

[Jon Hunter]

If the GPMC probe fails, devices that use the GPMC (such as ethernet
chips, flash memories, etc) can still allocate a GPMC chip-select and
register the device. On the OMAP2420 H4 board, this was causing the
kernel to crash after the gpmc probe failed and the board attempted
to start networking. Prevent this by marking all the chip-selects as
reserved by default and only make them available for devices to request
if the GPMC probe succeeds.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 8033cb7..441cc63 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -145,7 +145,8 @@ static unsigned gpmc_irq_start;
 static struct resource	gpmc_mem_root;
 static struct resource	gpmc_cs_mem[GPMC_CS_NUM];
 static DEFINE_SPINLOCK(gpmc_mem_lock);
-static unsigned int gpmc_cs_map;	/* flag for cs which are initialized */
+/* Define chip-selects as reserved by default until probe completes */
+static unsigned int gpmc_cs_map = ((1 << GPMC_CS_NUM) - 1);
 static struct device *gpmc_dev;
 static int gpmc_irq;
 static resource_size_t phys_base, mem_size;
@@ -1174,6 +1175,9 @@ static int gpmc_probe(struct platform_device *pdev)
 	if (IS_ERR_VALUE(gpmc_setup_irq()))
 		dev_warn(gpmc_dev, "gpmc_setup_irq failed\n");
 
+	/* Now the GPMC is initialised, unreserve the chip-selects */
+	gpmc_cs_map = 0;
+
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Jon Hunter]

OMAP2+ devices have an internal ROM that by default is typically mapped
to the first 1MB of the GPMC address space (0x0).

For OMAP24xx devices, GPMC chip-select 0 (CS0) may be mapped to address
0x0 instead of the internal ROM if configured for an external boot mode.
If configured for an internal boot mode then the internal ROM is mapped
to 0x0.

Currently, the function gpmc_mem_init() function reserves the first 1MB
of GPMC address space for the internal OMAP ROM with the exception of
the OMAP2 APOLLON board. This prevents any device (ethernet chip, flash
memories, etc) from using this address range. This causes the GPMC probe
to fail on the OMAP2420 H4 when NOR flash is mapped to address 0x0. Fix
this by testing the boot mode for OMAP24xx devices to see if the
SYS_BOOT3 is low, indicating an external boot, and thus GPMC CS0 is
mapped to 0x0.

Please note that for OMAP3-5 devices, when booting from NOR or NAND
memories connected to CS0, these memories are always mapped to address
0x08000000 and so reserving this memory range does not present any
problems for these devices.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc.c |   25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 441cc63..9486b8e 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -32,6 +32,7 @@
 
 #include "soc.h"
 #include "common.h"
+#include "control.h"
 #include "omap_device.h"
 #include "gpmc.h"
 
@@ -778,18 +779,26 @@ static void gpmc_mem_exit(void)
 static int gpmc_mem_init(void)
 {
 	int cs, rc;
-	unsigned long boot_rom_space = 0;
 
-	/* never allocate the first page, to facilitate bug detection;
-	 * even if we didn't boot from ROM.
+	/*
+	 * The first 1MB of GPMC address space is mapped to the
+	 * internal ROM. OMAP2 devices are an exception to this
+	 * where the first 1MB may be mapped to the GPMC.
 	 */
-	boot_rom_space = BOOT_ROM_SPACE;
-	/* In apollon the CS0 is mapped as 0x0000 0000 */
-	if (machine_is_omap_apollon())
-		boot_rom_space = 0;
-	gpmc_mem_root.start = GPMC_MEM_START + boot_rom_space;
+	gpmc_mem_root.start = GPMC_MEM_START + BOOT_ROM_SPACE;
 	gpmc_mem_root.end = GPMC_MEM_END;
 
+	/*
+	 * OMAP2 devices that boot from external memory devices, will
+	 * map CS0 to the start of the GPMC address space (0x0). We can
+	 * test this by checking if SYS_BOOT3 pin is set. If not set
+	 * then CS0 is mapped to 0x0.
+	 */
+	if (cpu_is_omap24xx())
+		if (!(omap_ctrl_readl(OMAP24XX_CONTROL_STATUS) &
+		      OMAP2_SYSBOOT_3_MASK))
+			gpmc_mem_root.start = GPMC_MEM_START;
+
 	/* Reserve all regions that has been set up by bootloader */
 	for (cs = 0; cs < GPMC_CS_NUM; cs++) {
 		u32 base, size;
-- 
1.7.10.4

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Tony Lindgren]

Hi Jon,

* Jon Hunter <jon-hunter@ti.com> [130201 08:42]:
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -32,6 +32,7 @@
>  
>  #include "soc.h"
>  #include "common.h"
> +#include "control.h"
>  #include "omap_device.h"
>  #include "gpmc.h"
>  
> @@ -778,18 +779,26 @@ static void gpmc_mem_exit(void)
>  static int gpmc_mem_init(void)
>  {
>  	int cs, rc;
> -	unsigned long boot_rom_space = 0;
>  
> -	/* never allocate the first page, to facilitate bug detection;
> -	 * even if we didn't boot from ROM.
> +	/*
> +	 * The first 1MB of GPMC address space is mapped to the
> +	 * internal ROM. OMAP2 devices are an exception to this
> +	 * where the first 1MB may be mapped to the GPMC.
>  	 */
> -	boot_rom_space = BOOT_ROM_SPACE;
> -	/* In apollon the CS0 is mapped as 0x0000 0000 */
> -	if (machine_is_omap_apollon())
> -		boot_rom_space = 0;

This part is going away anyways with the patch dropping apollon
board support from Kyungin.

> -	gpmc_mem_root.start = GPMC_MEM_START + boot_rom_space;
> +	gpmc_mem_root.start = GPMC_MEM_START + BOOT_ROM_SPACE;
>  	gpmc_mem_root.end = GPMC_MEM_END;
>  
> +	/*
> +	 * OMAP2 devices that boot from external memory devices, will
> +	 * map CS0 to the start of the GPMC address space (0x0). We can
> +	 * test this by checking if SYS_BOOT3 pin is set. If not set
> +	 * then CS0 is mapped to 0x0.
> +	 */
> +	if (cpu_is_omap24xx())
> +		if (!(omap_ctrl_readl(OMAP24XX_CONTROL_STATUS) &
> +		      OMAP2_SYSBOOT_3_MASK))
> +			gpmc_mem_root.start = GPMC_MEM_START;
> +
>  	/* Reserve all regions that has been set up by bootloader */
>  	for (cs = 0; cs < GPMC_CS_NUM; cs++) {
>  		u32 base, size;

How about let's fix this properly to start with so we don't add
more blockers moving this code to drivers/bus?

Looks like gpmc_mem_init() gets called from gpmc_probe() so
we can pass that information in pdev.

Regards,

Tony

[PATCH 1/2] ARM: OMAP2+: Prevent potential crash if GPMC probe fails

[Tony Lindgren]

* Jon Hunter <jon-hunter@ti.com> [130201 08:42]:
> If the GPMC probe fails, devices that use the GPMC (such as ethernet
> chips, flash memories, etc) can still allocate a GPMC chip-select and
> register the device. On the OMAP2420 H4 board, this was causing the
> kernel to crash after the gpmc probe failed and the board attempted
> to start networking. Prevent this by marking all the chip-selects as
> reserved by default and only make them available for devices to request
> if the GPMC probe succeeds.

Thanks applying into omap-for-v3.9/gpmc.

Regards,

Tony

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Jon Hunter]

On 02/01/2013 03:51 PM, Tony Lindgren wrote:
> Hi Jon,
> 
> * Jon Hunter <jon-hunter@ti.com> [130201 08:42]:
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -32,6 +32,7 @@
>>  
>>  #include "soc.h"
>>  #include "common.h"
>> +#include "control.h"
>>  #include "omap_device.h"
>>  #include "gpmc.h"
>>  
>> @@ -778,18 +779,26 @@ static void gpmc_mem_exit(void)
>>  static int gpmc_mem_init(void)
>>  {
>>  	int cs, rc;
>> -	unsigned long boot_rom_space = 0;
>>  
>> -	/* never allocate the first page, to facilitate bug detection;
>> -	 * even if we didn't boot from ROM.
>> +	/*
>> +	 * The first 1MB of GPMC address space is mapped to the
>> +	 * internal ROM. OMAP2 devices are an exception to this
>> +	 * where the first 1MB may be mapped to the GPMC.
>>  	 */
>> -	boot_rom_space = BOOT_ROM_SPACE;
>> -	/* In apollon the CS0 is mapped as 0x0000 0000 */
>> -	if (machine_is_omap_apollon())
>> -		boot_rom_space = 0;
> 
> This part is going away anyways with the patch dropping apollon
> board support from Kyungin.

Yes just saw that today.

>> -	gpmc_mem_root.start = GPMC_MEM_START + boot_rom_space;
>> +	gpmc_mem_root.start = GPMC_MEM_START + BOOT_ROM_SPACE;
>>  	gpmc_mem_root.end = GPMC_MEM_END;
>>  
>> +	/*
>> +	 * OMAP2 devices that boot from external memory devices, will
>> +	 * map CS0 to the start of the GPMC address space (0x0). We can
>> +	 * test this by checking if SYS_BOOT3 pin is set. If not set
>> +	 * then CS0 is mapped to 0x0.
>> +	 */
>> +	if (cpu_is_omap24xx())
>> +		if (!(omap_ctrl_readl(OMAP24XX_CONTROL_STATUS) &
>> +		      OMAP2_SYSBOOT_3_MASK))
>> +			gpmc_mem_root.start = GPMC_MEM_START;
>> +
>>  	/* Reserve all regions that has been set up by bootloader */
>>  	for (cs = 0; cs < GPMC_CS_NUM; cs++) {
>>  		u32 base, size;
> 
> How about let's fix this properly to start with so we don't add
> more blockers moving this code to drivers/bus?
> 
> Looks like gpmc_mem_init() gets called from gpmc_probe() so
> we can pass that information in pdev.

I wondered if you would suggest that ;-)

I definitely can and it is probably best. Things like this become
painful when we move to device-tree. We really need a generic way for
passing stuff like this to drivers for omap. Our options are auxdata or
bus notifiers. I am wondering whether we can plug pdata in the
omap_device bus notifier for device-tree. Let me know if you have any
thoughts.

Cheers
Jon

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Tony Lindgren]

* Jon Hunter <jon-hunter@ti.com> [130201 17:25]:
> 
> On 02/01/2013 03:51 PM, Tony Lindgren wrote:
> > 
> > How about let's fix this properly to start with so we don't add
> > more blockers moving this code to drivers/bus?
> > 
> > Looks like gpmc_mem_init() gets called from gpmc_probe() so
> > we can pass that information in pdev.
> 
> I wondered if you would suggest that ;-)

:)
 
> I definitely can and it is probably best. Things like this become
> painful when we move to device-tree. We really need a generic way for
> passing stuff like this to drivers for omap. Our options are auxdata or
> bus notifiers. I am wondering whether we can plug pdata in the
> omap_device bus notifier for device-tree. Let me know if you have any
> thoughts.

Hmm but in this case can't you just do it based on the compatible
flag? For legacy systems we also need to pass it in pdata.

Regarding omap_device, we should find a way to keep the dependencies
between drivers and the bus code down to minimum. So ideally things
like this would be only done using just the compatible flag. But the
pdata we cannot remove quite yet.

Regards,

Tony

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Jon Hunter]

On 02/02/2013 12:11 PM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [130201 17:25]:
>>
>> On 02/01/2013 03:51 PM, Tony Lindgren wrote:
>>>
>>> How about let's fix this properly to start with so we don't add
>>> more blockers moving this code to drivers/bus?
>>>
>>> Looks like gpmc_mem_init() gets called from gpmc_probe() so
>>> we can pass that information in pdev.
>>
>> I wondered if you would suggest that ;-)
> 
> :)
>  
>> I definitely can and it is probably best. Things like this become
>> painful when we move to device-tree. We really need a generic way for
>> passing stuff like this to drivers for omap. Our options are auxdata or
>> bus notifiers. I am wondering whether we can plug pdata in the
>> omap_device bus notifier for device-tree. Let me know if you have any
>> thoughts.
> 
> Hmm but in this case can't you just do it based on the compatible
> flag? For legacy systems we also need to pass it in pdata.

This is a boot-time configuration. So really you need to read the
control status register at boot and then determine the mapping. We could
store the address of the control status read in the pdata, but I think
that is a bit of a hack.

> Regarding omap_device, we should find a way to keep the dependencies
> between drivers and the bus code down to minimum. So ideally things
> like this would be only done using just the compatible flag. But the
> pdata we cannot remove quite yet.

Agree. However, there are several drivers today (gpio, dmtimer, mmc,
serial, dss, etc), that make use of a function pointer to
omap_pm_get_dev_context_loss_count() to determine when the peripheral's
state has been lost. When booting with DT this function pointer is not
populated and so with DT we currently have no way to determine this. I
see this as a blocker to migrating completely to DT. Ideally we would
find a way for RPM to handle this and remove the function pointer.
However, right now we still need a generic way to pass this type of
platform data to drivers.

Cheers
Jon

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Tony Lindgren]

* Jon Hunter <jon-hunter@ti.com> [130204 07:09]:
> 
> On 02/02/2013 12:11 PM, Tony Lindgren wrote:
> > * Jon Hunter <jon-hunter@ti.com> [130201 17:25]:
> >>
> >> On 02/01/2013 03:51 PM, Tony Lindgren wrote:
> >>>
> >>> How about let's fix this properly to start with so we don't add
> >>> more blockers moving this code to drivers/bus?
> >>>
> >>> Looks like gpmc_mem_init() gets called from gpmc_probe() so
> >>> we can pass that information in pdev.
> >>
> >> I wondered if you would suggest that ;-)
> > 
> > :)
> >  
> >> I definitely can and it is probably best. Things like this become
> >> painful when we move to device-tree. We really need a generic way for
> >> passing stuff like this to drivers for omap. Our options are auxdata or
> >> bus notifiers. I am wondering whether we can plug pdata in the
> >> omap_device bus notifier for device-tree. Let me know if you have any
> >> thoughts.
> > 
> > Hmm but in this case can't you just do it based on the compatible
> > flag? For legacy systems we also need to pass it in pdata.
> 
> This is a boot-time configuration. So really you need to read the
> control status register at boot and then determine the mapping. We could
> store the address of the control status read in the pdata, but I think
> that is a bit of a hack.

Ah right. Well once we have Haojian's generic pinconf patches merged for
pinctrl-single, we can set up the CONTROL_STATUS register as a
pinctrl-single,bits register and query the SYSBOOT_3 pin value directly
from the driver.

AFAIK SYSBOOT_n values reflect the boot time values of the actual SYSBOOT
pins, so using generic pinconf there makes sense. But this of course should
be checked.
 
> > Regarding omap_device, we should find a way to keep the dependencies
> > between drivers and the bus code down to minimum. So ideally things
> > like this would be only done using just the compatible flag. But the
> > pdata we cannot remove quite yet.
> 
> Agree. However, there are several drivers today (gpio, dmtimer, mmc,
> serial, dss, etc), that make use of a function pointer to
> omap_pm_get_dev_context_loss_count() to determine when the peripheral's
> state has been lost. When booting with DT this function pointer is not
> populated and so with DT we currently have no way to determine this. I
> see this as a blocker to migrating completely to DT. Ideally we would
> find a way for RPM to handle this and remove the function pointer.
> However, right now we still need a generic way to pass this type of
> platform data to drivers.

Yeah pinconf generic won't help us with the legacy boot.

Regards,

Tony

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Jon Hunter]

On 02/04/2013 11:45 AM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [130204 07:09]:
>>
>> On 02/02/2013 12:11 PM, Tony Lindgren wrote:
>>> * Jon Hunter <jon-hunter@ti.com> [130201 17:25]:
>>>>
>>>> On 02/01/2013 03:51 PM, Tony Lindgren wrote:
>>>>>
>>>>> How about let's fix this properly to start with so we don't add
>>>>> more blockers moving this code to drivers/bus?
>>>>>
>>>>> Looks like gpmc_mem_init() gets called from gpmc_probe() so
>>>>> we can pass that information in pdev.
>>>>
>>>> I wondered if you would suggest that ;-)
>>>
>>> :)
>>>  
>>>> I definitely can and it is probably best. Things like this become
>>>> painful when we move to device-tree. We really need a generic way for
>>>> passing stuff like this to drivers for omap. Our options are auxdata or
>>>> bus notifiers. I am wondering whether we can plug pdata in the
>>>> omap_device bus notifier for device-tree. Let me know if you have any
>>>> thoughts.
>>>
>>> Hmm but in this case can't you just do it based on the compatible
>>> flag? For legacy systems we also need to pass it in pdata.
>>
>> This is a boot-time configuration. So really you need to read the
>> control status register at boot and then determine the mapping. We could
>> store the address of the control status read in the pdata, but I think
>> that is a bit of a hack.
> 
> Ah right. Well once we have Haojian's generic pinconf patches merged for
> pinctrl-single, we can set up the CONTROL_STATUS register as a
> pinctrl-single,bits register and query the SYSBOOT_3 pin value directly
> from the driver.
> 
> AFAIK SYSBOOT_n values reflect the boot time values of the actual SYSBOOT
> pins, so using generic pinconf there makes sense. But this of course should
> be checked.

Not sure I am a fan of that idea. It is possible the pins could be
re-used as GPIOs after reset. Given that the state at reset is latched
in a register, it is best just to read the register directly.

>>> Regarding omap_device, we should find a way to keep the dependencies
>>> between drivers and the bus code down to minimum. So ideally things
>>> like this would be only done using just the compatible flag. But the
>>> pdata we cannot remove quite yet.
>>
>> Agree. However, there are several drivers today (gpio, dmtimer, mmc,
>> serial, dss, etc), that make use of a function pointer to
>> omap_pm_get_dev_context_loss_count() to determine when the peripheral's
>> state has been lost. When booting with DT this function pointer is not
>> populated and so with DT we currently have no way to determine this. I
>> see this as a blocker to migrating completely to DT. Ideally we would
>> find a way for RPM to handle this and remove the function pointer.
>> However, right now we still need a generic way to pass this type of
>> platform data to drivers.
> 
> Yeah pinconf generic won't help us with the legacy boot.

Right. I view all this sort of thing as system-level device information
that some drivers may need. It does not seem that we have a good way to
handle that at the moment. Any ideas?

Jon

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Tony Lindgren]

* Jon Hunter <jon-hunter@ti.com> [130204 10:49]:
> On 02/04/2013 11:45 AM, Tony Lindgren wrote:
> > 
> > AFAIK SYSBOOT_n values reflect the boot time values of the actual SYSBOOT
> > pins, so using generic pinconf there makes sense. But this of course should
> > be checked.
> 
> Not sure I am a fan of that idea. It is possible the pins could be
> re-used as GPIOs after reset. Given that the state at reset is latched
> in a register, it is best just to read the register directly.

Yes the physical SYSBOOT pins can be reused as GPIO, but that's are already
handled by the padconf and GPIO registers. This is a different register
showing the boot time pin values for some pins. So it makes sense to use
generic pinconf to make the pin values available to the client drivers
as needed.

The advantage doing it this way is that we don't need to export any omap
custom functions to the drivers from the SCM driver. This way we need zero
platform glue code, and can deal with it directly in the drivers in a
generic way. And all we need to do is just need to map the SoC specific
SYSBOOT pin register in the .dts files.

It may also make sense to export DEVICETYPE this way. At least early omaps
had the GP vs HS mode configured by pulls on some pins during the boot time.
So those bits too may reflect actual physical pins during the boot time
configured by the efuse settings?

> >>> Regarding omap_device, we should find a way to keep the dependencies
> >>> between drivers and the bus code down to minimum. So ideally things
> >>> like this would be only done using just the compatible flag. But the
> >>> pdata we cannot remove quite yet.
> >>
> >> Agree. However, there are several drivers today (gpio, dmtimer, mmc,
> >> serial, dss, etc), that make use of a function pointer to
> >> omap_pm_get_dev_context_loss_count() to determine when the peripheral's
> >> state has been lost. When booting with DT this function pointer is not
> >> populated and so with DT we currently have no way to determine this. I
> >> see this as a blocker to migrating completely to DT. Ideally we would
> >> find a way for RPM to handle this and remove the function pointer.
> >> However, right now we still need a generic way to pass this type of
> >> platform data to drivers.
> > 
> > Yeah pinconf generic won't help us with the legacy boot.
> 
> Right. I view all this sort of thing as system-level device information
> that some drivers may need. It does not seem that we have a good way to
> handle that at the moment. Any ideas?

I suggest just passing it in in pdata for now for the legacy boot. Then
I suggest we make what we can generic with pinconf in the long run.

Regards,

Tony

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Jon Hunter]

On 02/04/2013 01:15 PM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [130204 10:49]:
>> On 02/04/2013 11:45 AM, Tony Lindgren wrote:
>>>
>>> AFAIK SYSBOOT_n values reflect the boot time values of the actual SYSBOOT
>>> pins, so using generic pinconf there makes sense. But this of course should
>>> be checked.
>>
>> Not sure I am a fan of that idea. It is possible the pins could be
>> re-used as GPIOs after reset. Given that the state at reset is latched
>> in a register, it is best just to read the register directly.
> 
> Yes the physical SYSBOOT pins can be reused as GPIO, but that's are already
> handled by the padconf and GPIO registers. This is a different register
> showing the boot time pin values for some pins. So it makes sense to use
> generic pinconf to make the pin values available to the client drivers
> as needed.
> 
> The advantage doing it this way is that we don't need to export any omap
> custom functions to the drivers from the SCM driver. This way we need zero
> platform glue code, and can deal with it directly in the drivers in a
> generic way. And all we need to do is just need to map the SoC specific
> SYSBOOT pin register in the .dts files.

I see what you are saying exporting the state in control_status register
via the pinconf. That could work.

> It may also make sense to export DEVICETYPE this way. At least early omaps
> had the GP vs HS mode configured by pulls on some pins during the boot time.
> So those bits too may reflect actual physical pins during the boot time
> configured by the efuse settings?

I *believe* that was only omap1.

>>>>> Regarding omap_device, we should find a way to keep the dependencies
>>>>> between drivers and the bus code down to minimum. So ideally things
>>>>> like this would be only done using just the compatible flag. But the
>>>>> pdata we cannot remove quite yet.
>>>>
>>>> Agree. However, there are several drivers today (gpio, dmtimer, mmc,
>>>> serial, dss, etc), that make use of a function pointer to
>>>> omap_pm_get_dev_context_loss_count() to determine when the peripheral's
>>>> state has been lost. When booting with DT this function pointer is not
>>>> populated and so with DT we currently have no way to determine this. I
>>>> see this as a blocker to migrating completely to DT. Ideally we would
>>>> find a way for RPM to handle this and remove the function pointer.
>>>> However, right now we still need a generic way to pass this type of
>>>> platform data to drivers.
>>>
>>> Yeah pinconf generic won't help us with the legacy boot.
>>
>> Right. I view all this sort of thing as system-level device information
>> that some drivers may need. It does not seem that we have a good way to
>> handle that at the moment. Any ideas?
> 
> I suggest just passing it in in pdata for now for the legacy boot. Then
> I suggest we make what we can generic with pinconf in the long run.

I don't see why we would want to export a function pointer to
omap_pm_get_dev_context_loss_count() with pinconf. Have we got our wires
crossed here?

Jon

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Tony Lindgren]

* Jon Hunter <jon-hunter@ti.com> [130204 11:37]:
> 
> On 02/04/2013 01:15 PM, Tony Lindgren wrote:
> > * Jon Hunter <jon-hunter@ti.com> [130204 10:49]:
> >> On 02/04/2013 11:45 AM, Tony Lindgren wrote:
> >>>
> >>> AFAIK SYSBOOT_n values reflect the boot time values of the actual SYSBOOT
> >>> pins, so using generic pinconf there makes sense. But this of course should
> >>> be checked.
> >>
> >> Not sure I am a fan of that idea. It is possible the pins could be
> >> re-used as GPIOs after reset. Given that the state at reset is latched
> >> in a register, it is best just to read the register directly.
> > 
> > Yes the physical SYSBOOT pins can be reused as GPIO, but that's are already
> > handled by the padconf and GPIO registers. This is a different register
> > showing the boot time pin values for some pins. So it makes sense to use
> > generic pinconf to make the pin values available to the client drivers
> > as needed.
> > 
> > The advantage doing it this way is that we don't need to export any omap
> > custom functions to the drivers from the SCM driver. This way we need zero
> > platform glue code, and can deal with it directly in the drivers in a
> > generic way. And all we need to do is just need to map the SoC specific
> > SYSBOOT pin register in the .dts files.
> 
> I see what you are saying exporting the state in control_status register
> via the pinconf. That could work.
> 
> > It may also make sense to export DEVICETYPE this way. At least early omaps
> > had the GP vs HS mode configured by pulls on some pins during the boot time.
> > So those bits too may reflect actual physical pins during the boot time
> > configured by the efuse settings?
> 
> I *believe* that was only omap1.

Yeah but maybe the efuses just configure some pulls for selected pins for
later omaps?
 
> >>>>> Regarding omap_device, we should find a way to keep the dependencies
> >>>>> between drivers and the bus code down to minimum. So ideally things
> >>>>> like this would be only done using just the compatible flag. But the
> >>>>> pdata we cannot remove quite yet.
> >>>>
> >>>> Agree. However, there are several drivers today (gpio, dmtimer, mmc,
> >>>> serial, dss, etc), that make use of a function pointer to
> >>>> omap_pm_get_dev_context_loss_count() to determine when the peripheral's
> >>>> state has been lost. When booting with DT this function pointer is not
> >>>> populated and so with DT we currently have no way to determine this. I
> >>>> see this as a blocker to migrating completely to DT. Ideally we would
> >>>> find a way for RPM to handle this and remove the function pointer.
> >>>> However, right now we still need a generic way to pass this type of
> >>>> platform data to drivers.
> >>>
> >>> Yeah pinconf generic won't help us with the legacy boot.
> >>
> >> Right. I view all this sort of thing as system-level device information
> >> that some drivers may need. It does not seem that we have a good way to
> >> handle that at the moment. Any ideas?
> > 
> > I suggest just passing it in in pdata for now for the legacy boot. Then
> > I suggest we make what we can generic with pinconf in the long run.
> 
> I don't see why we would want to export a function pointer to
> omap_pm_get_dev_context_loss_count() with pinconf. Have we got our wires
> crossed here?

Yes sorry, too many muxes here. I got this topic mixed up with the sysboot
pin topic :)

We really need to find a Linux generic API to query this. Sounds like it
should be part of runtime PM?

Regards,

Tony

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Jon Hunter]

On 02/04/2013 01:47 PM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [130204 11:37]:
>>
>> On 02/04/2013 01:15 PM, Tony Lindgren wrote:
>>> * Jon Hunter <jon-hunter@ti.com> [130204 10:49]:
>>>> On 02/04/2013 11:45 AM, Tony Lindgren wrote:
>>>>>
>>>>> AFAIK SYSBOOT_n values reflect the boot time values of the actual SYSBOOT
>>>>> pins, so using generic pinconf there makes sense. But this of course should
>>>>> be checked.
>>>>
>>>> Not sure I am a fan of that idea. It is possible the pins could be
>>>> re-used as GPIOs after reset. Given that the state at reset is latched
>>>> in a register, it is best just to read the register directly.
>>>
>>> Yes the physical SYSBOOT pins can be reused as GPIO, but that's are already
>>> handled by the padconf and GPIO registers. This is a different register
>>> showing the boot time pin values for some pins. So it makes sense to use
>>> generic pinconf to make the pin values available to the client drivers
>>> as needed.
>>>
>>> The advantage doing it this way is that we don't need to export any omap
>>> custom functions to the drivers from the SCM driver. This way we need zero
>>> platform glue code, and can deal with it directly in the drivers in a
>>> generic way. And all we need to do is just need to map the SoC specific
>>> SYSBOOT pin register in the .dts files.
>>
>> I see what you are saying exporting the state in control_status register
>> via the pinconf. That could work.
>>
>>> It may also make sense to export DEVICETYPE this way. At least early omaps
>>> had the GP vs HS mode configured by pulls on some pins during the boot time.
>>> So those bits too may reflect actual physical pins during the boot time
>>> configured by the efuse settings?
>>
>> I *believe* that was only omap1.
> 
> Yeah but maybe the efuses just configure some pulls for selected pins for
> later omaps?
>  
>>>>>>> Regarding omap_device, we should find a way to keep the dependencies
>>>>>>> between drivers and the bus code down to minimum. So ideally things
>>>>>>> like this would be only done using just the compatible flag. But the
>>>>>>> pdata we cannot remove quite yet.
>>>>>>
>>>>>> Agree. However, there are several drivers today (gpio, dmtimer, mmc,
>>>>>> serial, dss, etc), that make use of a function pointer to
>>>>>> omap_pm_get_dev_context_loss_count() to determine when the peripheral's
>>>>>> state has been lost. When booting with DT this function pointer is not
>>>>>> populated and so with DT we currently have no way to determine this. I
>>>>>> see this as a blocker to migrating completely to DT. Ideally we would
>>>>>> find a way for RPM to handle this and remove the function pointer.
>>>>>> However, right now we still need a generic way to pass this type of
>>>>>> platform data to drivers.
>>>>>
>>>>> Yeah pinconf generic won't help us with the legacy boot.
>>>>
>>>> Right. I view all this sort of thing as system-level device information
>>>> that some drivers may need. It does not seem that we have a good way to
>>>> handle that at the moment. Any ideas?
>>>
>>> I suggest just passing it in in pdata for now for the legacy boot. Then
>>> I suggest we make what we can generic with pinconf in the long run.
>>
>> I don't see why we would want to export a function pointer to
>> omap_pm_get_dev_context_loss_count() with pinconf. Have we got our wires
>> crossed here?
> 
> Yes sorry, too many muxes here. I got this topic mixed up with the sysboot
> pin topic :)
> 
> We really need to find a Linux generic API to query this. Sounds like it
> should be part of runtime PM?

Ideally, yes. I had discussed with Kevin about adding a new state for
"logic state lost" and then if this could be passed to the RPM resume
callback then we could use this. May be I need to bring this up with Tero.

Cheers
Jon

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Jon Hunter]

On 02/01/2013 03:51 PM, Tony Lindgren wrote:

> How about let's fix this properly to start with so we don't add
> more blockers moving this code to drivers/bus?
> 
> Looks like gpmc_mem_init() gets called from gpmc_probe() so
> we can pass that information in pdev.

I have re-worked this a bit to use platform data. I have also updated
the logic for testing internal/external boot on omap2430 which is
different from omap2420 (according to the TRM). However, I have only
boot tested on omap2420. 

Do you know why this was changed in the first place? See below ...

http://permalink.gmane.org/gmane.linux.kernel.commits.head/95931

Heres what I have now ...


>From df2eebd3580f478f569241d294c2f810359fab6a Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Thu, 31 Jan 2013 15:56:08 -0600
Subject: [PATCH] ARM: OMAP2: Fix GPMC memory initialisation

OMAP2+ devices have an internal ROM that by default is typically mapped
to the first 1MB of the GPMC address space (0x0).

For OMAP24xx devices, GPMC chip-select 0 (CS0) may be mapped to address
0x0 instead of the internal ROM if configured for an external boot mode.
If configured for an internal boot mode then the internal ROM is mapped
to 0x0.

Currently, the function gpmc_mem_init() function reserves the first 1MB
of GPMC address space for the internal OMAP ROM. This prevents any
device (ethernet chip, flash memories, etc) from using this address
range. This causes the GPMC probe to fail on the OMAP2420 H4 when NOR
flash is mapped to address 0x0. Fix this by checking the SYS_BOOT pin
setting for OMAP24xx devices, to see the device is configured for
an internal or external boot and pass the amount of ROM mapped to
address 0x0 to the GPMC driver via platform data.

Please note that for OMAP3-5 devices, when booting from NOR or NAND
memories connected to CS0, these memories are always mapped to address
0x08000000 and so reserving this memory range does not present any
problems for these devices.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc.c |   53 +++++++++++++++++++++++++++++++++++---------
 arch/arm/mach-omap2/gpmc.h |    5 +++++
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index a124849..71c0134 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -20,6 +20,7 @@
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/ioport.h>
+#include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -32,6 +33,7 @@
 
 #include "soc.h"
 #include "common.h"
+#include "control.h"
 #include "omap_device.h"
 #include "gpmc.h"
 
@@ -87,7 +89,6 @@
 
 #define GPMC_MEM_START		0x00000000
 #define GPMC_MEM_END		0x3FFFFFFF
-#define BOOT_ROM_SPACE		0x100000	/* 1MB */
 
 #define GPMC_CHUNK_SHIFT	24		/* 16 MB */
 #define GPMC_SECTION_SHIFT	28		/* 128 MB */
@@ -775,16 +776,11 @@ static void gpmc_mem_exit(void)
 
 }
 
-static int gpmc_mem_init(void)
+static int gpmc_mem_init(u32 offset)
 {
 	int cs, rc;
-	unsigned long boot_rom_space = 0;
 
-	/* never allocate the first page, to facilitate bug detection;
-	 * even if we didn't boot from ROM.
-	 */
-	boot_rom_space = BOOT_ROM_SPACE;
-	gpmc_mem_root.start = GPMC_MEM_START + boot_rom_space;
+	gpmc_mem_root.start = GPMC_MEM_START + offset;
 	gpmc_mem_root.end = GPMC_MEM_END;
 
 	/* Reserve all regions that has been set up by bootloader */
@@ -1124,6 +1120,12 @@ static int gpmc_probe(struct platform_device *pdev)
 	int rc;
 	u32 l;
 	struct resource *res;
+	struct gpmc_platform_data *pdata = pdev->dev.platform_data;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "%s: no platform data.\n", __func__);
+		return -ENODEV;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL)
@@ -1161,7 +1163,7 @@ static int gpmc_probe(struct platform_device *pdev)
 	dev_info(gpmc_dev, "GPMC revision %d.%d\n", GPMC_REVISION_MAJOR(l),
 		 GPMC_REVISION_MINOR(l));
 
-	rc = gpmc_mem_init();
+	rc = gpmc_mem_init(pdata->gpmc_rom_space);
 	if (IS_ERR_VALUE(rc)) {
 		clk_disable_unprepare(gpmc_l3_clk);
 		clk_put(gpmc_l3_clk);
@@ -1213,15 +1215,46 @@ static int __init omap_gpmc_init(void)
 {
 	struct omap_hwmod *oh;
 	struct platform_device *pdev;
+	struct gpmc_platform_data *pdata;
 	char *oh_name = "gpmc";
 
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		pr_err("%s: No memory for gpmc\n", __func__);
+		return -ENOMEM;
+	}
+
+	/*
+	 * The first 1MB of GPMC address space is mapped to the internal
+	 * ROM. OMAP2 devices are an exception to this where the first
+	 * 1MB may be mapped to the GPMC. OMAP2 devices that boot from
+	 * external memory devices, will map CS0 to the start of the
+	 * GPMC address space (0x0). We can test this by checking the
+	 * SYS_BOOT pins.
+	 *
+	 * OMAP2420 SYS_BOOT[3] = 1b, then GPMC CS0 is mapped to 0x0.
+	 * OMAP2430 SYS_BOOT[2:1] = 00b, then GPMC CS0 is mapped to 0x0.
+	 */
+	pdata->gpmc_rom_space = SZ_1M;
+
+	if (cpu_is_omap242x()) {
+		if (!(omap_ctrl_readl(OMAP24XX_CONTROL_STATUS) &
+		      OMAP2_SYSBOOT_3_MASK))
+			pdata->gpmc_rom_space = 0;
+	} else if (cpu_is_omap243x()) {
+		if (!(omap_ctrl_readl(OMAP24XX_CONTROL_STATUS) &
+		      (OMAP2_SYSBOOT_2_MASK | OMAP2_SYSBOOT_1_MASK)))
+			pdata->gpmc_rom_space = 0;
+	}
+
 	oh = omap_hwmod_lookup(oh_name);
 	if (!oh) {
 		pr_err("Could not look up %s\n", oh_name);
 		return -ENODEV;
 	}
 
-	pdev = omap_device_build(DEVICE_NAME, -1, oh, NULL, 0, NULL, 0, 0);
+	pdev = omap_device_build(DEVICE_NAME, -1, oh, pdata, sizeof(*pdata),
+				 NULL, 0, 0);
 	WARN(IS_ERR(pdev), "could not build omap_device for %s\n", oh_name);
 
 	return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index fe0a844..b33ef26 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -189,6 +189,11 @@ struct gpmc_device_timings {
 	bool we_xdelay;
 };
 
+/* GPMC platform data */
+struct gpmc_platform_data {
+	u32 gpmc_rom_space;
+};
+
 extern int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
 				struct gpmc_device_timings *dev_t);
 
-- 
1.7.10.4

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Jon Hunter]

On 02/04/2013 04:12 PM, Jon Hunter wrote:
> 
> On 02/01/2013 03:51 PM, Tony Lindgren wrote:
> 
>> How about let's fix this properly to start with so we don't add
>> more blockers moving this code to drivers/bus?
>>
>> Looks like gpmc_mem_init() gets called from gpmc_probe() so
>> we can pass that information in pdev.
> 
> I have re-worked this a bit to use platform data. I have also update
> the logic for testing internal/external boot on omap2430 which is
> different from omap2420 (according to the TRM). However, I have only
> boot tested on omap2420. 
> 
> Do you know why this was changed in the first place? See below ...
> 
> http://permalink.gmane.org/gmane.linux.kernel.commits.head/95931
> 
> Heres what I have now ...

Updated patch (I was missing a kfree()). By the way, this is based
upon the apollon removal patch. I understand probably needs to be
rebased on your latest gpmc series too.

Jon

>From 643656e3754ed949d9a8af31b22ed3727e0962f6 Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Thu, 31 Jan 2013 15:56:08 -0600
Subject: [PATCH] ARM: OMAP2: Fix GPMC memory initialisation

OMAP2+ devices have an internal ROM that by default is typically mapped
to the first 1MB of the GPMC address space (0x0).

For OMAP24xx devices, GPMC chip-select 0 (CS0) may be mapped to address
0x0 instead of the internal ROM if configured for an external boot mode.
If configured for an internal boot mode then the internal ROM is mapped
to 0x0.

Currently, the function gpmc_mem_init() function reserves the first 1MB
of GPMC address space for the internal OMAP ROM. This prevents any
device (ethernet chip, flash memories, etc) from using this address
range. This causes the GPMC probe to fail on the OMAP2420 H4 when NOR
flash is mapped to address 0x0. Fix this by checking the SYS_BOOT pin
setting for OMAP24xx devices, to see the device is configured for
an internal or external boot and pass the amount of ROM mapped to
address 0x0 to the GPMC driver via platform data.

Please note that for OMAP3-5 devices, when booting from NOR or NAND
memories connected to CS0, these memories are always mapped to address
0x08000000 and so reserving this memory range does not present any
problems for these devices.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc.c |   60 ++++++++++++++++++++++++++++++++++++--------
 arch/arm/mach-omap2/gpmc.h |    5 ++++
 2 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index a124849..aa58258 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -20,6 +20,7 @@
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/ioport.h>
+#include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -32,6 +33,7 @@
 
 #include "soc.h"
 #include "common.h"
+#include "control.h"
 #include "omap_device.h"
 #include "gpmc.h"
 
@@ -87,7 +89,6 @@
 
 #define GPMC_MEM_START		0x00000000
 #define GPMC_MEM_END		0x3FFFFFFF
-#define BOOT_ROM_SPACE		0x100000	/* 1MB */
 
 #define GPMC_CHUNK_SHIFT	24		/* 16 MB */
 #define GPMC_SECTION_SHIFT	28		/* 128 MB */
@@ -775,16 +776,11 @@ static void gpmc_mem_exit(void)
 
 }
 
-static int gpmc_mem_init(void)
+static int gpmc_mem_init(u32 offset)
 {
 	int cs, rc;
-	unsigned long boot_rom_space = 0;
 
-	/* never allocate the first page, to facilitate bug detection;
-	 * even if we didn't boot from ROM.
-	 */
-	boot_rom_space = BOOT_ROM_SPACE;
-	gpmc_mem_root.start = GPMC_MEM_START + boot_rom_space;
+	gpmc_mem_root.start = GPMC_MEM_START + offset;
 	gpmc_mem_root.end = GPMC_MEM_END;
 
 	/* Reserve all regions that has been set up by bootloader */
@@ -1124,6 +1120,12 @@ static int gpmc_probe(struct platform_device *pdev)
 	int rc;
 	u32 l;
 	struct resource *res;
+	struct gpmc_platform_data *pdata = pdev->dev.platform_data;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "%s: no platform data.\n", __func__);
+		return -ENODEV;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL)
@@ -1161,7 +1163,7 @@ static int gpmc_probe(struct platform_device *pdev)
 	dev_info(gpmc_dev, "GPMC revision %d.%d\n", GPMC_REVISION_MAJOR(l),
 		 GPMC_REVISION_MINOR(l));
 
-	rc = gpmc_mem_init();
+	rc = gpmc_mem_init(pdata->gpmc_rom_space);
 	if (IS_ERR_VALUE(rc)) {
 		clk_disable_unprepare(gpmc_l3_clk);
 		clk_put(gpmc_l3_clk);
@@ -1213,18 +1215,54 @@ static int __init omap_gpmc_init(void)
 {
 	struct omap_hwmod *oh;
 	struct platform_device *pdev;
+	struct gpmc_platform_data *pdata;
 	char *oh_name = "gpmc";
 
+	pdata = kzalloc(sizeof(struct gpmc_platform_data), GFP_KERNEL);
+	if (!pdata) {
+		pr_err("%s: No memory for gpmc\n", __func__);
+		return -ENOMEM;
+	}
+
+	/*
+	 * The first 1MB of GPMC address space is mapped to the internal
+	 * ROM. OMAP2 devices are an exception to this where the first
+	 * 1MB may be mapped to the GPMC. OMAP2 devices that boot from
+	 * external memory devices, will map CS0 to the start of the
+	 * GPMC address space (0x0). We can test this by checking the
+	 * SYS_BOOT pins.
+	 *
+	 * OMAP2420 SYS_BOOT[3] = 1b, then GPMC CS0 is mapped to 0x0.
+	 * OMAP2430 SYS_BOOT[2:1] = 00b, then GPMC CS0 is mapped to 0x0.
+	 */
+	pdata->gpmc_rom_space = SZ_1M;
+
+	if (cpu_is_omap242x()) {
+		if (!(omap_ctrl_readl(OMAP24XX_CONTROL_STATUS) &
+		      OMAP2_SYSBOOT_3_MASK))
+			pdata->gpmc_rom_space = 0;
+	} else if (cpu_is_omap243x()) {
+		if (!(omap_ctrl_readl(OMAP24XX_CONTROL_STATUS) &
+		      (OMAP2_SYSBOOT_2_MASK | OMAP2_SYSBOOT_1_MASK)))
+			pdata->gpmc_rom_space = 0;
+	}
+
 	oh = omap_hwmod_lookup(oh_name);
 	if (!oh) {
 		pr_err("Could not look up %s\n", oh_name);
 		return -ENODEV;
 	}
 
-	pdev = omap_device_build(DEVICE_NAME, -1, oh, NULL, 0, NULL, 0, 0);
+	pdev = omap_device_build(DEVICE_NAME, -1, oh, pdata, sizeof(*pdata),
+				 NULL, 0, 0);
 	WARN(IS_ERR(pdev), "could not build omap_device for %s\n", oh_name);
 
-	return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
+	if (IS_ERR(pdev)) {
+		kfree(pdata);
+		return PTR_ERR(pdev);
+	}
+
+	return 0;
 }
 postcore_initcall(omap_gpmc_init);
 
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index fe0a844..b33ef26 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -189,6 +189,11 @@ struct gpmc_device_timings {
 	bool we_xdelay;
 };
 
+/* GPMC platform data */
+struct gpmc_platform_data {
+	u32 gpmc_rom_space;
+};
+
 extern int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
 				struct gpmc_device_timings *dev_t);
 
-- 
1.7.10.4

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Tony Lindgren]

* Jon Hunter <jon-hunter@ti.com> [130204 14:49]:
> 
> On 02/04/2013 04:12 PM, Jon Hunter wrote:
> > 
> > On 02/01/2013 03:51 PM, Tony Lindgren wrote:
> > 
> >> How about let's fix this properly to start with so we don't add
> >> more blockers moving this code to drivers/bus?
> >>
> >> Looks like gpmc_mem_init() gets called from gpmc_probe() so
> >> we can pass that information in pdev.
> > 
> > I have re-worked this a bit to use platform data. I have also update
> > the logic for testing internal/external boot on omap2430 which is
> > different from omap2420 (according to the TRM). However, I have only
> > boot tested on omap2420. 
> > 
> > Do you know why this was changed in the first place? See below ...
> > 
> > http://permalink.gmane.org/gmane.linux.kernel.commits.head/95931
> > 
> > Heres what I have now ...
> 
> Updated patch (I was missing a kfree()). By the way, this is based
> upon the apollon removal patch. I understand probably needs to be
> rebased on your latest gpmc series too.

Yeah makes sense to me thanks. I'll figure out where to apply this
so we don't get too many dependencies.

Regards,

Tony

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Jon Hunter]

On 02/05/2013 05:34 PM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [130204 14:49]:
>>
>> On 02/04/2013 04:12 PM, Jon Hunter wrote:
>>>
>>> On 02/01/2013 03:51 PM, Tony Lindgren wrote:
>>>
>>>> How about let's fix this properly to start with so we don't add
>>>> more blockers moving this code to drivers/bus?
>>>>
>>>> Looks like gpmc_mem_init() gets called from gpmc_probe() so
>>>> we can pass that information in pdev.
>>>
>>> I have re-worked this a bit to use platform data. I have also update
>>> the logic for testing internal/external boot on omap2430 which is
>>> different from omap2420 (according to the TRM). However, I have only
>>> boot tested on omap2420. 
>>>
>>> Do you know why this was changed in the first place? See below ...
>>>
>>> http://permalink.gmane.org/gmane.linux.kernel.commits.head/95931
>>>
>>> Heres what I have now ...
>>
>> Updated patch (I was missing a kfree()). By the way, this is based
>> upon the apollon removal patch. I understand probably needs to be
>> rebased on your latest gpmc series too.
> 
> Yeah makes sense to me thanks. I'll figure out where to apply this
> so we don't get too many dependencies.

Actually, let me look into this a bit more. It appears that for all
omap2+ devices NOR should be mapped to CS0 at 0x08000000. So I am
wondering if the boot-loader is re-mapping the CS0 space. If it is then
may be we can avoid having such hacks in the kernel by fixing the
bootloader. To date only the apollon board has really had this problem
and I need to check what I have on my H4 (which has been hacked by me ;-)

Cheers
Jon

[PATCH 2/2] ARM: OMAP2: Fix GPMC memory initialisation

[Tony Lindgren]

* Jon Hunter <jon-hunter@ti.com> [130205 16:26]:
> 
> Actually, let me look into this a bit more. It appears that for all
> omap2+ devices NOR should be mapped to CS0 at 0x08000000. So I am
> wondering if the boot-loader is re-mapping the CS0 space. If it is then
> may be we can avoid having such hacks in the kernel by fixing the
> bootloader. To date only the apollon board has really had this problem
> and I need to check what I have on my H4 (which has been hacked by me ;-)

OK. Yeah sorry I don't remember the history of why the different
mappings were needed. It could be it was just needed to keep some
devices working that were only initialized in the bootloader like
you're suspecting.

Tony

[PATCH 1/2] ARM: OMAP2+: Prevent potential crash if GPMC probe fails

[Jon Hunter]

On 02/01/2013 04:08 PM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [130201 08:42]:
>> If the GPMC probe fails, devices that use the GPMC (such as ethernet
>> chips, flash memories, etc) can still allocate a GPMC chip-select and
>> register the device. On the OMAP2420 H4 board, this was causing the
>> kernel to crash after the gpmc probe failed and the board attempted
>> to start networking. Prevent this by marking all the chip-selects as
>> reserved by default and only make them available for devices to request
>> if the GPMC probe succeeds.
> 
> Thanks applying into omap-for-v3.9/gpmc.

Hi Tony, this one appears to be merged incorrectly. The unreserve ended 
up in the gpmc_calc_timings() function. Here is a patch to fix.

Cheers
Jon

>From ebc0613fb5a70f36fcb119cbe58724f9b442903a Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Fri, 8 Feb 2013 16:48:25 -0600
Subject: [PATCH] ARM: OMAP2+: Fix-up gpmc merge error

Commit "ARM: OMAP2+: Prevent potential crash if GPMC probe fails" added
code to ensure that GPMC chip-selects could not be requested until the
device probe was successful. The chip-selects should have been
unreserved at the end of the probe function, but the code to unreserve
them appears to have ended up in the gpmc_calc_timings() function and
hence, this is causing problems requesting chip-selects. Fix this merge
error by unreserving the chip-selects at the end of the probe, but
before we call the gpmc child probe functions (for device-tree) which
request a chip-select.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 1adb2d4..1e8bcb4 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1125,9 +1125,6 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
 	/* TODO: remove, see function definition */
 	gpmc_convert_ps_to_ns(gpmc_t);
 
-	/* Now the GPMC is initialised, unreserve the chip-selects */
-	gpmc_cs_map = 0;
-
 	return 0;
 }
 
@@ -1388,6 +1385,9 @@ static int gpmc_probe(struct platform_device *pdev)
 	if (IS_ERR_VALUE(gpmc_setup_irq()))
 		dev_warn(gpmc_dev, "gpmc_setup_irq failed\n");
 
+	/* Now the GPMC is initialised, unreserve the chip-selects */
+	gpmc_cs_map = 0;
+
 	rc = gpmc_probe_dt(pdev);
 	if (rc < 0) {
 		clk_disable_unprepare(gpmc_l3_clk);
-- 
1.7.10.4

[PATCH 1/2] ARM: OMAP2+: Prevent potential crash if GPMC probe fails

[Ezequiel Garcia]

On Fri, Feb 08, 2013 at 04:56:19PM -0600, Jon Hunter wrote:
> 
> On 02/01/2013 04:08 PM, Tony Lindgren wrote:
> > * Jon Hunter <jon-hunter@ti.com> [130201 08:42]:
> >> If the GPMC probe fails, devices that use the GPMC (such as ethernet
> >> chips, flash memories, etc) can still allocate a GPMC chip-select and
> >> register the device. On the OMAP2420 H4 board, this was causing the
> >> kernel to crash after the gpmc probe failed and the board attempted
> >> to start networking. Prevent this by marking all the chip-selects as
> >> reserved by default and only make them available for devices to request
> >> if the GPMC probe succeeds.
> > 
> > Thanks applying into omap-for-v3.9/gpmc.
> 
> Hi Tony, this one appears to be merged incorrectly. The unreserve ended 
> up in the gpmc_calc_timings() function. Here is a patch to fix.
> 
> Cheers
> Jon
> 
> From ebc0613fb5a70f36fcb119cbe58724f9b442903a Mon Sep 17 00:00:00 2001
> From: Jon Hunter <jon-hunter@ti.com>
> Date: Fri, 8 Feb 2013 16:48:25 -0600
> Subject: [PATCH] ARM: OMAP2+: Fix-up gpmc merge error
> 
> Commit "ARM: OMAP2+: Prevent potential crash if GPMC probe fails" added
> code to ensure that GPMC chip-selects could not be requested until the
> device probe was successful. The chip-selects should have been
> unreserved at the end of the probe function, but the code to unreserve
> them appears to have ended up in the gpmc_calc_timings() function and
> hence, this is causing problems requesting chip-selects. Fix this merge
> error by unreserving the chip-selects at the end of the probe, but
> before we call the gpmc child probe functions (for device-tree) which
> request a chip-select.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>

Without this patch, GPMC is currently broken on my igep board setup,
if initialized through a device tree.

Tested-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Thanks a lot for the fix,

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

[PATCH 1/2] ARM: OMAP2+: Prevent potential crash if GPMC probe fails

[Philip, Avinash]

Hi Tony,

On Sat, Feb 09, 2013 at 21:25:49, Ezequiel Garcia wrote:
> On Fri, Feb 08, 2013 at 04:56:19PM -0600, Jon Hunter wrote:
> > 
> > On 02/01/2013 04:08 PM, Tony Lindgren wrote:
> > > * Jon Hunter <jon-hunter@ti.com> [130201 08:42]:
> > >> If the GPMC probe fails, devices that use the GPMC (such as ethernet
> > >> chips, flash memories, etc) can still allocate a GPMC chip-select and
> > >> register the device. On the OMAP2420 H4 board, this was causing the
> > >> kernel to crash after the gpmc probe failed and the board attempted
> > >> to start networking. Prevent this by marking all the chip-selects as
> > >> reserved by default and only make them available for devices to request
> > >> if the GPMC probe succeeds.
> > > 
> > > Thanks applying into omap-for-v3.9/gpmc.
> > 
> > Hi Tony, this one appears to be merged incorrectly. The unreserve ended 
> > up in the gpmc_calc_timings() function. Here is a patch to fix.
> > 
> > Cheers
> > Jon
> > 
> > From ebc0613fb5a70f36fcb119cbe58724f9b442903a Mon Sep 17 00:00:00 2001
> > From: Jon Hunter <jon-hunter@ti.com>
> > Date: Fri, 8 Feb 2013 16:48:25 -0600
> > Subject: [PATCH] ARM: OMAP2+: Fix-up gpmc merge error
> > 
> > Commit "ARM: OMAP2+: Prevent potential crash if GPMC probe fails" added
> > code to ensure that GPMC chip-selects could not be requested until the
> > device probe was successful. The chip-selects should have been
> > unreserved at the end of the probe function, but the code to unreserve
> > them appears to have ended up in the gpmc_calc_timings() function and
> > hence, this is causing problems requesting chip-selects. Fix this merge
> > error by unreserving the chip-selects at the end of the probe, but
> > before we call the gpmc child probe functions (for device-tree) which
> > request a chip-select.
> > 
> > Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> 
> Without this patch, GPMC is currently broken on my igep board setup,
> if initialized through a device tree.
> 
> Tested-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Without this patch GPMC is not working in am335x-evm.

Tested-by: Philip Avinash <avinashphilip@ti.com>

See Jon's comments.

JON >> Hi Tony, this one appears to be merged incorrectly. The unreserve ended 
JON >> up in the gpmc_calc_timings() function. Here is a patch to fix.

I have tested based on linux-omap/master.

http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap.git;a=shortlog;h=refs/heads/master

Thanks
Avinash

> Without this patch, GPMC is currently broken on my igep board setup,
> if initialized through a device tree.
> 
> Thanks a lot for the fix,
> 
> -- 
> 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 at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 1/2] ARM: OMAP2+: Prevent potential crash if GPMC probe fails

[Grazvydas Ignotas]

On Thu, Feb 14, 2013 at 2:04 PM, Philip, Avinash <avinashphilip@ti.com> wrote:
> Hi Tony,
>
> On Sat, Feb 09, 2013 at 21:25:49, Ezequiel Garcia wrote:
>> On Fri, Feb 08, 2013 at 04:56:19PM -0600, Jon Hunter wrote:
>> Without this patch, GPMC is currently broken on my igep board setup,
>> if initialized through a device tree.
>>
>> Tested-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>
> Without this patch GPMC is not working in am335x-evm.
>
> Tested-by: Philip Avinash <avinashphilip@ti.com>
>
> See Jon's comments.
>
> JON >> Hi Tony, this one appears to be merged incorrectly. The unreserve ended
> JON >> up in the gpmc_calc_timings() function. Here is a patch to fix.

Just wasted a hour or a few trying to figure out this problem, can we
get this merged now? Maybe Jon can resend this with all the tested-bys
to catch Tony's attention?

Tested-by: Grazvydas Ignotas <notasas@gmail.com>


-- 
Gra?vydas