SPI omap2_mcspi: Add max_clk_div field to mcspi platform config

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 <[email protected]>

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

arch/arm/mach-omap2/devices.c +0 -0

index 23e4d77..d4ad126 100644

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/[email protected]>

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

arch/arm/mach-omap2/devices.c +0 -0

index 23e4d77..200f382 100644
@@ -415,6 +415,11 @@ static inline void omap4_mcspi_fixup(void)
 	defined(CONFIG_ARCH_OMAP4)

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

Hi,

Quoted 1 line · Tony Lindgren

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.

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.

Quoted 2 lines

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.

Hi,

Quoted 17 lines · Tony Lindgren

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;
}

Quoted 3 lines

subsys_initcall(emu_init);

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

sure. Exactly what I was thinking :-p

Quoted 3 lines

Quoted 2 lines

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

Quoted 3 lines

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).

Quoted 4 lines

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.

Hi,

Quoted 5 lines · Tony Lindgren

Quoted 4 lines

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).

Quoted 3 lines

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.

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.

Quoted 9 lines

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.

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

But then again, the TRMs can have errors.

Quoted 2 lines

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.

Quoted 2 lines

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?