[U-Boot] omap3 spi registers

[U-Boot] omap3 spi registers

[Martin Hejnfelt]

Hi,

I am working with a board based off the am335x CPU and I am going to
use SPI through u-boot. I've therefore been fiddling with u-boot and
enabling spi0 through devicetree and all, which works fine (at least
probing and so on). I am using CONFIG_DM_SPI for this, and
u-boot-2016.05-rc3.
However when I try to access the SPI peripheral, u-boot hanged.

Debugging through this showed that the culprit lies within the
spi_reset call inside omap3_spi.c when the SPI bus is being claimed.
The function tries to write to the sysconfig register, based 0x110 off
the BASE. Problem is that the base defined in device tree include I am
basing myself off (am335x.dtsi) is 0x48030000, and the offset is
defined to 0x10, which means the register being written to is
0x48030010 and not 0x48030110 as it is supposed to, again resulting in
the hang due to the endless do-while loop in spi_reset. This I guess
is due to this part priv->regs = (struct mcspi *)dev_get_addr(dev); of
omap3_spi_probe as it reads the reg property off the device tree.

OK, so for my question, what is really the correct way to fix this?
0x48030000 is the "real" base of McSPI0, although the first 0x100 part
is revision stuff according to the technical datasheet of the am335x.
If I change the reg value of the device tree to 0x48030100 then it
does not hang, reset seems to work fine, however I have no idea of the
influences this will have (the device tree is also used in the kernel
and so on), I would guess its not the "right" way.

Any input/output is appreciated.

Best regards,
Martin Hejnfelt

[U-Boot] omap3 spi registers

[Vignesh R]

Hi,

On 05/12/2016 06:14 PM, Martin Hejnfelt wrote:
> Hi,
> 
> I am working with a board based off the am335x CPU and I am going to
> use SPI through u-boot. I've therefore been fiddling with u-boot and
> enabling spi0 through devicetree and all, which works fine (at least
> probing and so on). I am using CONFIG_DM_SPI for this, and
> u-boot-2016.05-rc3.
> However when I try to access the SPI peripheral, u-boot hanged.
> 
> Debugging through this showed that the culprit lies within the
> spi_reset call inside omap3_spi.c when the SPI bus is being claimed.
> The function tries to write to the sysconfig register, based 0x110 off
> the BASE. Problem is that the base defined in device tree include I am
> basing myself off (am335x.dtsi) is 0x48030000, and the offset is
> defined to 0x10, which means the register being written to is
> 0x48030010 and not 0x48030110 as it is supposed to, again resulting in
> the hang due to the endless do-while loop in spi_reset. This I guess
> is due to this part priv->regs = (struct mcspi *)dev_get_addr(dev); of
> omap3_spi_probe as it reads the reg property off the device tree.
> 
> OK, so for my question, what is really the correct way to fix this?
> 0x48030000 is the "real" base of McSPI0, although the first 0x100 part
> is revision stuff according to the technical datasheet of the am335x.
> If I change the reg value of the device tree to 0x48030100 then it
> does not hang, reset seems to work fine, however I have no idea of the
> influences this will have (the device tree is also used in the kernel
> and so on), I would guess its not the "right" way.
> 
> Any input/output is appreciated.
> 

I think, the right to handle this is based on compatible as done in
kernel driver (drivers/spi/spi-omap2-mcspi.c)
Something like, if compatible is "ti,omap4-mcspi" then add offset 0x100
to regs property


-- 
Regards
Vignesh

[U-Boot] omap3 spi registers

[Martin Hejnfelt]

Hi Vignesh,

2016-05-18 10:44 GMT+02:00 Vignesh R <vigneshr@ti.com>:
>
> I think, the right to handle this is based on compatible as done in
> kernel driver (drivers/spi/spi-omap2-mcspi.c)
> Something like, if compatible is "ti,omap4-mcspi" then add offset 0x100
> to regs property
>

It didn't even cross my mind to check how it was handled in linux, but
I think you are right, this sounds like a good idea. I'll try to give
it a spin and see if I can cook something up.

Thank you.

Best regards
Martin Hejnfelt

[U-Boot] omap3 spi registers

[Martin Hejnfelt]

Hi Vignesh,

How about something like this, naming and so on shamelessly stolen
from the linux kernel:

Index: u-boot-2016.05/drivers/spi/omap3_spi.c
===================================================================
--- u-boot-2016.05.orig/drivers/spi/omap3_spi.c 2016-05-16
16:40:32.000000000 +0200
+++ u-boot-2016.05/drivers/spi/omap3_spi.c 2016-05-18 13:46:18.810353297 +0200
@@ -35,6 +35,12 @@
 #define OMAP3_MCSPI4_BASE 0x480BA000
 #endif

+#define OMAP4_MCSPI_REG_OFFSET 0x100
+
+struct omap2_mcspi_platform_config {
+        unsigned int regs_offset;
+};
+
 /* per-register bitmasks */
 #define OMAP3_MCSPI_SYSCONFIG_SMARTIDLE (2 << 3)
 #define OMAP3_MCSPI_SYSCONFIG_ENAWAKEUP BIT(2)
@@ -623,7 +629,10 @@
  const void *blob = gd->fdt_blob;
  int node = dev->of_offset;

- priv->regs = (struct mcspi *)dev_get_addr(dev);
+ struct omap2_mcspi_platform_config* data =
+ (struct omap2_mcspi_platform_config*)dev_get_driver_data(dev);
+
+ priv->regs = (struct mcspi *)(dev_get_addr(dev) + data->regs_offset);
  priv->pin_dir = fdtdec_get_uint(blob, node, "ti,pindir-d0-out-d1-in",
     MCSPI_PINDIR_D0_IN_D1_OUT);
  priv->wordlen = SPI_DEFAULT_WORDLEN;
@@ -662,9 +671,17 @@
  */
 };

+static struct omap2_mcspi_platform_config omap2_pdata = {
+        .regs_offset = 0,
+};
+
+static struct omap2_mcspi_platform_config omap4_pdata = {
+        .regs_offset = OMAP4_MCSPI_REG_OFFSET,
+};
+
 static const struct udevice_id omap3_spi_ids[] = {
- { .compatible = "ti,omap2-mcspi" },
- { .compatible = "ti,omap4-mcspi" },
+ { .compatible = "ti,omap2-mcspi", .data = (ulong)&omap2_pdata },
+ { .compatible = "ti,omap4-mcspi", .data = (ulong)&omap4_pdata },
  { }
 };

Best regards,
Martin Hejnfelt