[PATCH 2/6] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[PATCH 2/6] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Scott Ellis]

The McSPI_CHxCONF.CLKD register field has different limits for the
OMAP3 then the OMAP24xx. As per recommendation from the linux-omap
list, added a field for max_clk_div to the platform data for use in 
omap2_mcspi.c.

Used in a subsequent patch. 

Signed-off-by: Scott Ellis <scott@jumpnowtek.com>

 arch/arm/mach-omap2/devices.c           |   16 ++++++++++++++++
 arch/arm/plat-omap/include/plat/mcspi.h |    1 +
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 23e4d77..d4ad126 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -302,6 +302,11 @@ static inline void omap_init_sti(void) {}
 
 static struct omap2_mcspi_platform_config omap2_mcspi1_config = {
 	.num_cs		= 4,
+#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
+	.max_clk_div	= 0x0c,
+#else
+	.max_clk_div	= 0x0f,
+#endif
 };
 
 static struct resource omap2_mcspi1_resources[] = {
@@ -324,6 +329,11 @@ static struct platform_device omap2_mcspi1 = {
 
 static struct omap2_mcspi_platform_config omap2_mcspi2_config = {
 	.num_cs		= 2,
+#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
+	.max_clk_div	= 0x0c,
+#else
+	.max_clk_div	= 0x0f,
+#endif
 };
 
 static struct resource omap2_mcspi2_resources[] = {
@@ -348,6 +358,11 @@ static struct platform_device omap2_mcspi2 = {
 	defined(CONFIG_ARCH_OMAP4)
 static struct omap2_mcspi_platform_config omap2_mcspi3_config = {
 	.num_cs		= 2,
+#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
+	.max_clk_div	= 0x0c,
+#else
+	.max_clk_div	= 0x0f,
+#endif
 };
 
 static struct resource omap2_mcspi3_resources[] = {
@@ -372,6 +387,7 @@ static struct platform_device omap2_mcspi3 = {
 #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
 static struct omap2_mcspi_platform_config omap2_mcspi4_config = {
 	.num_cs		= 1,
+	.max_clk_div	= 0x0c,
 };
 
 static struct resource omap2_mcspi4_resources[] = {
diff --git a/arch/arm/plat-omap/include/plat/mcspi.h b/arch/arm/plat-omap/include/plat/mcspi.h
index 1254e49..9fadddd 100644
--- a/arch/arm/plat-omap/include/plat/mcspi.h
+++ b/arch/arm/plat-omap/include/plat/mcspi.h
@@ -3,6 +3,7 @@
 
 struct omap2_mcspi_platform_config {
 	unsigned short	num_cs;
+	unsigned short	max_clk_div;
 };
 
 struct omap2_mcspi_device_config {

Re: [PATCH 2/6] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Tony Lindgren]

* Scott Ellis <scott@jumpnowtek.com> [100312 07:20]:
> The McSPI_CHxCONF.CLKD register field has different limits for the
> OMAP3 then the OMAP24xx. As per recommendation from the linux-omap
> list, added a field for max_clk_div to the platform data for use in 
> omap2_mcspi.c.
> 
> Used in a subsequent patch. 
> 
> Signed-off-by: Scott Ellis <scott@jumpnowtek.com>
> 
>  arch/arm/mach-omap2/devices.c           |   16 ++++++++++++++++
>  arch/arm/plat-omap/include/plat/mcspi.h |    1 +
>  2 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 23e4d77..d4ad126 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -302,6 +302,11 @@ static inline void omap_init_sti(void) {}
>  
>  static struct omap2_mcspi_platform_config omap2_mcspi1_config = {
>  	.num_cs		= 4,
> +#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
> +	.max_clk_div	= 0x0c,
> +#else
> +	.max_clk_div	= 0x0f,
> +#endif
>  };
>  

Can't do ifdef else like this. It will break multi-omap booting on
omap2 + 3 + 4. In the platform init just set the values once with
cpu_is_omapXXXX() instead.

Regards,

Tony

[PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Scott Ellis]

The McSPI_CHxCONF.CLKD register field has different limits for
the OMAP3 then the OMAP24xx. New max_clk_div field added to
mcspi platform config structure to keep track of this.

Revised patch to not break multi-omap booting.

Signed-off-by: Scott Ellis <scott@jumpnowtek.com>

 arch/arm/mach-omap2/devices.c           |   14 ++++++++++++++
 arch/arm/plat-omap/include/plat/mcspi.h |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 23e4d77..200f382 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -415,6 +415,11 @@ static inline void omap4_mcspi_fixup(void)
 	defined(CONFIG_ARCH_OMAP4)
 static inline void omap2_mcspi3_init(void)
 {
+	if (cpu_is_omap343x() || cpu_is_omap44xx())
+		omap2_mcspi3_config.max_clk_div = 0x0c;
+	else
+		omap2_mcspi3_config.max_clk_div = 0x0f;
+
 	platform_device_register(&omap2_mcspi3);
 }
 #else
@@ -426,6 +431,7 @@ static inline void omap2_mcspi3_init(void)
 #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
 static inline void omap2_mcspi4_init(void)
 {
+	omap2_mcspi4_config.max_clk_div = 0x0c;
 	platform_device_register(&omap2_mcspi4);
 }
 #else
@@ -439,6 +445,14 @@ static void omap_init_mcspi(void)
 	if (cpu_is_omap44xx())
 		omap4_mcspi_fixup();
 
+	if (cpu_is_omap343x() || cpu_is_omap44xx()) {
+		omap2_mcspi1_config.max_clk_div = 0x0c;
+		omap2_mcspi2_config.max_clk_div = 0x0c;
+	} else {
+		omap2_mcspi1_config.max_clk_div = 0x0f;
+		omap2_mcspi2_config.max_clk_div = 0x0f;
+	}
+
 	platform_device_register(&omap2_mcspi1);
 	platform_device_register(&omap2_mcspi2);
 
diff --git a/arch/arm/plat-omap/include/plat/mcspi.h b/arch/arm/plat-omap/include/plat/mcspi.h
index 1254e49..9fadddd 100644
--- a/arch/arm/plat-omap/include/plat/mcspi.h
+++ b/arch/arm/plat-omap/include/plat/mcspi.h
@@ -3,6 +3,7 @@
 
 struct omap2_mcspi_platform_config {
 	unsigned short	num_cs;
+	unsigned short	max_clk_div;
 };
 
 struct omap2_mcspi_device_config {

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Tony Lindgren]

* Scott Ellis <scott-oZqG5KApg2M/jFmSBShxMA@public.gmane.org> [100314 10:22]:
> The McSPI_CHxCONF.CLKD register field has different limits for
> the OMAP3 then the OMAP24xx. New max_clk_div field added to
> mcspi platform config structure to keep track of this.
> 
> Revised patch to not break multi-omap booting.
> 
> Signed-off-by: Scott Ellis <scott-oZqG5KApg2M/jFmSBShxMA@public.gmane.org>
> 
>  arch/arm/mach-omap2/devices.c           |   14 ++++++++++++++
>  arch/arm/plat-omap/include/plat/mcspi.h |    1 +
>  2 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 23e4d77..200f382 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -415,6 +415,11 @@ static inline void omap4_mcspi_fixup(void)
>  	defined(CONFIG_ARCH_OMAP4)
>  static inline void omap2_mcspi3_init(void)
>  {
> +	if (cpu_is_omap343x() || cpu_is_omap44xx())
> +		omap2_mcspi3_config.max_clk_div = 0x0c;
> +	else
> +		omap2_mcspi3_config.max_clk_div = 0x0f;
> +
>  	platform_device_register(&omap2_mcspi3);
>  }
>  #else

Hmm now it looks like you're missing 3630 handling?

If the max_clk_div is 0x0f for 2420 and 2430, then you
can just check for cpu_is_omap24xx(). If it's only
different for 2420, then you can check for cpu_is_omap2420().

That way it should be more future proof, and you don't
need to change it for new processors.

Regards,

Tony

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Felipe Balbi]

Hi,
 
On Mon, Mar 15, 2010 at 09:32:46AM -0700, Tony Lindgren wrote:
> Hmm now it looks like you're missing 3630 handling?

a bit unrelated but Tony, would you mind looking at patches splitting
devices.c into something like dev34xx.c dev24xx.c and dev44xx.c ??

personally I think we should have mach-omap3 and mach-omap4 directories
but that would be too intrusive IMO.

-- 
balbi

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Tony Lindgren]

* Felipe Balbi <me@felipebalbi.com> [100315 10:59]:
> Hi,
>  
> On Mon, Mar 15, 2010 at 09:32:46AM -0700, Tony Lindgren wrote:
> > Hmm now it looks like you're missing 3630 handling?
> 
> a bit unrelated but Tony, would you mind looking at patches splitting
> devices.c into something like dev34xx.c dev24xx.c and dev44xx.c ??

Yeah I've been thinking about that too earlier. We could have common
devices.c with the init code, then dev24xx.c and dev34xx.c et al
would just call the common init functions with something like this:


static int __init dev34xx_init(void)
{
	if (!cpu_is_omap34xx())
		return -ENODEV;

	omap_init_mbox(omap34xx_mbox_resources, omap34xx_mbox_nr_resources);
	omap_init_sti(omap34xx_sti_resources, omap34xx_sti_nr_resources);
	...	

	return 0;
}

subsys_initcall(emu_init);

Of course that needs to be coordinated with the pending hwmod patches.

 
> personally I think we should have mach-omap3 and mach-omap4 directories
> but that would be too intrusive IMO.

I think we can achieve most of that by right naming of the files and
with hwmod. Having separate directories typically means multiple copies
of almost the same code.

Regards,

Tony

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Felipe Balbi]

Hi,

On Mon, Mar 15, 2010 at 11:52:13AM -0700, Tony Lindgren wrote:
> Yeah I've been thinking about that too earlier. We could have common
> devices.c with the init code, then dev24xx.c and dev34xx.c et al
> would just call the common init functions with something like this:
> 
> 
> static int __init dev34xx_init(void)
> {
> 	if (!cpu_is_omap34xx())
> 		return -ENODEV;
> 
> 	omap_init_mbox(omap34xx_mbox_resources, omap34xx_mbox_nr_resources);
> 	omap_init_sti(omap34xx_sti_resources, omap34xx_sti_nr_resources);
> 	...	
> 
> 	return 0;
> }
> 
> subsys_initcall(emu_init);
> 
> Of course that needs to be coordinated with the pending hwmod patches.

sure. Exactly what I was thinking :-p

> > personally I think we should have mach-omap3 and mach-omap4 directories
> > but that would be too intrusive IMO.
> 
> I think we can achieve most of that by right naming of the files and
> with hwmod. Having separate directories typically means multiple copies
> of almost the same code.

could be, but we already have separated clk, pm, cpuidle, mux and soon
to become devices. So pretty much the base support is already splitted,
then why not completely avoiding ifdefs also with dma (which today is
full of ifdefs and could be converted to a platform_device also).

-- 
balbi

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Tony Lindgren]

* Felipe Balbi <me@felipebalbi.com> [100315 12:12]:
> Hi,
> 
> On Mon, Mar 15, 2010 at 11:52:13AM -0700, Tony Lindgren wrote:
> > Yeah I've been thinking about that too earlier. We could have common
> > devices.c with the init code, then dev24xx.c and dev34xx.c et al
> > would just call the common init functions with something like this:
> > 
> > 
> > static int __init dev34xx_init(void)
> > {
> > 	if (!cpu_is_omap34xx())
> > 		return -ENODEV;
> > 
> > 	omap_init_mbox(omap34xx_mbox_resources, omap34xx_mbox_nr_resources);
> > 	omap_init_sti(omap34xx_sti_resources, omap34xx_sti_nr_resources);
> > 	...	
> > 
> > 	return 0;
> > }
> > 
> > subsys_initcall(emu_init);
> > 
> > Of course that needs to be coordinated with the pending hwmod patches.
> 
> sure. Exactly what I was thinking :-p
> 
> > > personally I think we should have mach-omap3 and mach-omap4 directories
> > > but that would be too intrusive IMO.
> > 
> > I think we can achieve most of that by right naming of the files and
> > with hwmod. Having separate directories typically means multiple copies
> > of almost the same code.
> 
> could be, but we already have separated clk, pm, cpuidle, mux and soon
> to become devices. So pretty much the base support is already splitted,
> then why not completely avoiding ifdefs also with dma (which today is
> full of ifdefs and could be converted to a platform_device also).

Yeah there are tons of things that should be fixed and split into
platform_data and generic code. At least gpio.c, dma.c and i2c-omap.c
need some serious work.

Regards,

Tony

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Felipe Balbi]

Hi,

On Mon, Mar 15, 2010 at 12:29:10PM -0700, Tony Lindgren wrote:
> > could be, but we already have separated clk, pm, cpuidle, mux and soon
> > to become devices. So pretty much the base support is already splitted,
> > then why not completely avoiding ifdefs also with dma (which today is
> > full of ifdefs and could be converted to a platform_device also).
> 
> Yeah there are tons of things that should be fixed and split into
> platform_data and generic code. At least gpio.c, dma.c and i2c-omap.c
> need some serious work.

problem with those guys is that they're used all over the place so any
changes there would always be extremelly intrusive and are prone to
cause regressions :-(

I could try to allocate some time to play with dma.c but I can only test
on omap3 since my n810 isn't mount root on mmc, maybe I screwed up
something and can't remember what :-s if you happen to have tips, I'm
all ears. The thing is rebooting with "mbus" bootreason. I'll have to
dig up some legacy docs to figure out what's that.

-- 
balbi

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Tony Lindgren]

* Felipe Balbi <me@felipebalbi.com> [100315 12:30]:
> Hi,
> 
> On Mon, Mar 15, 2010 at 12:29:10PM -0700, Tony Lindgren wrote:
> > > could be, but we already have separated clk, pm, cpuidle, mux and soon
> > > to become devices. So pretty much the base support is already splitted,
> > > then why not completely avoiding ifdefs also with dma (which today is
> > > full of ifdefs and could be converted to a platform_device also).
> > 
> > Yeah there are tons of things that should be fixed and split into
> > platform_data and generic code. At least gpio.c, dma.c and i2c-omap.c
> > need some serious work.
> 
> problem with those guys is that they're used all over the place so any
> changes there would always be extremelly intrusive and are prone to
> cause regressions :-(

Well it's not too hard, just need to be careful and do it in small
patches and test carefully between each patch :)

> I could try to allocate some time to play with dma.c but I can only test
> on omap3 since my n810 isn't mount root on mmc, maybe I screwed up
> something and can't remember what :-s if you happen to have tips, I'm
> all ears. The thing is rebooting with "mbus" bootreason. I'll have to
> dig up some legacy docs to figure out what's that.

For some reason only one of the MMC slots works on both n800 and n810,
maybe some menelaus slot switching issue. So you need to try mounting
root on the external card on n810 for now until somebody figures out
the problem.

Tony

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Scott Ellis]

> Hmm now it looks like you're missing 3630 handling?
> 
> If the max_clk_div is 0x0f for 2420 and 2430, then you
> can just check for cpu_is_omap24xx(). If it's only
> different for 2420, then you can check for cpu_is_omap2420().
> 
> That way it should be more future proof, and you don't
> need to change it for new processors.
> 
Anand Gadiyar <gadiyar <at> ti.com> verified 0x0f for the 2430.

I think SWPU177D is the correct TRM for the omap3630 and if so then 0x0c
is the correct value.

I did not verify the omap44xx value and just assumed similar to the
omap3's. My bad.

Can you or someone point me to links for the omap2420 and the omap44xx
TRMs? I'm not having any luck finding them.

Then it can be verified whether a cpu_is_omap24xx() check is sufficient.
It probably is.

Or if someone with access to those manuals could do a quick check...

It's the max value of the MCSPI_CHxCONF.CLKD register field.

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Tony Lindgren]

* Scott Ellis <scott-oZqG5KApg2M/jFmSBShxMA@public.gmane.org> [100315 13:27]:
> > Hmm now it looks like you're missing 3630 handling?
> > 
> > If the max_clk_div is 0x0f for 2420 and 2430, then you
> > can just check for cpu_is_omap24xx(). If it's only
> > different for 2420, then you can check for cpu_is_omap2420().
> > 
> > That way it should be more future proof, and you don't
> > need to change it for new processors.
> > 
> Anand Gadiyar <gadiyar <at> ti.com> verified 0x0f for the 2430.
> 
> I think SWPU177D is the correct TRM for the omap3630 and if so then 0x0c
> is the correct value.
> 
> I did not verify the omap44xx value and just assumed similar to the
> omap3's. My bad.
> 
> Can you or someone point me to links for the omap2420 and the omap44xx
> TRMs? I'm not having any luck finding them.

I don't think those are publicly available yet for 4430 and still for
2420..

But looks like 2420, 2430 and 4430 TRMs says that 0xf = 32768
max divider for CLKD.

Only 3430 and 3630 TRMs says 0xd, 0xe, 0xf = Division not supported.

But then again, the TRMs can have errors.

> Then it can be verified whether a cpu_is_omap24xx() check is sufficient.
> It probably is.

Then the 4430 TRM must have an error.. Can somebody from TI
confirm the CLKD max value for 4430 please?
 
> Or if someone with access to those manuals could do a quick check...
> 
> It's the max value of the MCSPI_CHxCONF.CLKD register field.

Regards,

Tony

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Scott Ellis]

> Only 3430 and 3630 TRMs says 0xd, 0xe, 0xf = Division not supported.
> 
I tested a 3503 with clock divider values of 0x0d, 0x0e and 0x0f.
It worked fine.
I collected data off the SPI bus successfully at the expected
frequencies of 5859 Hz, 2929 Hz and 1464 Hz.

> But then again, the TRMs can have errors.
> 
Looks like this is a case of that. 

My patches #2 and #3 are unnecessary then and #4 makes use of a new 
field added in #3.

I can resubmit #4, "Use transfer speed_hz if provided". That was the 
original problem I was working on. 

Should I leave the hard-coded 0x0f in the code or would you prefer a
named constant?

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Tony Lindgren]

* Scott Ellis <scott@jumpnowtek.com> [100319 12:43]:
> > Only 3430 and 3630 TRMs says 0xd, 0xe, 0xf = Division not supported.
> > 
> I tested a 3503 with clock divider values of 0x0d, 0x0e and 0x0f.
> It worked fine.
> I collected data off the SPI bus successfully at the expected
> frequencies of 5859 Hz, 2929 Hz and 1464 Hz.
> 
> > But then again, the TRMs can have errors.
> > 
> Looks like this is a case of that. 

OK, good thing you checked it :)
 
> My patches #2 and #3 are unnecessary then and #4 makes use of a new 
> field added in #3.
> 
> I can resubmit #4, "Use transfer speed_hz if provided". That was the 
> original problem I was working on. 
> 
> Should I leave the hard-coded 0x0f in the code or would you prefer a
> named constant?

Named constant in the long run, but a minimal fix is best for the
-rc cycle.

Regards,

Tony

Re: [PATCH 2/6 Revised] SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

[Scott Ellis]

Please disregard this patch.

PaulM from TI responded,

"It appears that this is a documentation error. The module does support
all 16 divide values for the CLKD field"

http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/38754/142276.aspx#142276