TI DaVinci: Driver for the davinci SPI controller

PATCH New Sudhakar Rajashekhara 8 participants 22 messages Dec 23, 2009 – Mar 7, 2010 SPI

lore

Share & tools

lore

Sudhakar Rajashekhara

author

2009-12-23 07:44 UTC #0 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

From: Sekhar Nori <[email protected]>

This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.

Signed-off-by Sekhar Nori <[email protected]>

Signed-off-by Sudhakar Rajashekhara <[email protected]>

---
 drivers/spi/Makefile      |    1 +
 drivers/spi/davinci_spi.c |  205 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/spi/davinci_spi.h |   84 ++++++++++++++++++
 3 files changed, 290 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/davinci_spi.c
 create mode 100644 drivers/spi/davinci_spi.h

drivers/spi/Makefile +1 -0

index 824d8e7..07b9611 100644
@@ -34,6 +34,7 @@ COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
 COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
 COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
 COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)

drivers/spi/davinci_spi.c new file +205 -0

205 lines not on this page · raw message

drivers/spi/davinci_spi.h new file +84 -0

84 lines not on this page · raw message

Signature · 1 line

1.5.6

Mike Frysinger

2009-12-24 01:34 UTC #1 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

drivers/spi/Makefile COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o · line 37

Quoted 7 lines · On Wednesday 23 December 2009 02:44:36 Sudhakar Rajashekhara wrote:

drivers/spi/Makefile +1 -0

@@ -34,6 +34,7 @@ COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
 COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
 COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
 COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o

this is a sorted list

drivers/spi/davinci_spi.h line 3

Quoted 6 lines

drivers/spi/davinci_spi.h new file +3 -0

@@ -0,0 +1,84 @@
+/*
+ * Register definitions for the DaVinci SPI Controller
+ */

missing license/copyright

drivers/spi/davinci_spi.h line 83

Quoted 4 lines

drivers/spi/davinci_spi.h new file +4 -0

+#define spi_readl(ds, reg)					\
+	readl(ds->regs + DAVINCI_SPI_##reg)
+#define spi_writel(ds, reg, value)				\
+	writel(value, ds->regs + DAVINCI_SPI_##reg)

should be (ds)->regs
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091223/a699402e/attachment.pgp

Sudhakar Rajashekhara

author

2009-12-24 02:52 UTC #2 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

Hi,

Quoted 11 lines · Mike Frysinger

drivers/spi/Makefile COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o · line 37
Quoted 7 lines · On Wednesday 23 December 2009 02:44:36 Sudhakar Rajashekhara wrote:
drivers/spi/Makefile +1 -0
@@ -34,6 +34,7 @@ COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
 COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
 COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
 COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
this is a sorted list

Will fix it. Thanks for pointing it out.

Quoted 8 lines

drivers/spi/davinci_spi.h line 2
Quoted 5 lines
drivers/spi/davinci_spi.h new file +2 -0
@@ -0,0 +1,84 @@
+/*
+ * Register definitions for the DaVinci SPI Controller  */
missing license/copyright

Will add copyright/license.

Quoted 6 lines

drivers/spi/davinci_spi.h line 83
Quoted 4 lines
drivers/spi/davinci_spi.h new file +4 -0
+#define spi_readl(ds, reg)					\
+	readl(ds->regs + DAVINCI_SPI_##reg)
+#define spi_writel(ds, reg, value)				\
+	writel(value, ds->regs + DAVINCI_SPI_##reg)
should be (ds)->regs

I'll modify it and resubmit the patch.

Thanks,
Sudhakar

Dirk Behme

2009-12-25 17:11 UTC #3 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

In reply to Sudhakar Rajashekhara

Quoted 20 lines · Sudhakar Rajashekhara

From: Sekhar Nori<[email protected]>
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by Sekhar Nori<[email protected]>
Signed-off-by Sudhakar Rajashekhara<[email protected]>
---
  drivers/spi/Makefile      |    1 +
  drivers/spi/davinci_spi.c |  205 +++++++++++++++++++++++++++++++++++++++++++++
  drivers/spi/davinci_spi.h |   84 ++++++++++++++++++
  3 files changed, 290 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/davinci_spi.c
create mode 100644 drivers/spi/davinci_spi.h
drivers/spi/Makefile +0 -0
index 824d8e7..07b9611 100644

drivers/spi/Makefile COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o · line 37

Quoted 4 lines

drivers/spi/Makefile +1 -0

@@ -34,6 +34,7 @@ COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
  COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
  COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
  COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o

Should this list be kept sorted alphabetically? I.e.

...
COBJS-$(CONFIG_BFIN_SPI) += bfin_spi.o
COBJS-$(CONFIG_CF_SPI) += cf_spi.o
+COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
COBJS-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
...

Quoted 8 lines

drivers/spi/Makefile +0 -0
  COBJS	:= $(COBJS-y)
  SRCS	:= $(COBJS:.o=.c)
drivers/spi/davinci_spi.c new file +0 -0
new file mode 100644
index 0000000..1c988bb
@@ -0,0 +1,205 @@

...

Quoted 1 line

+void spi_init()

...

Quoted 2 lines

drivers/spi/davinci_spi.c new file +2 -0

+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+			unsigned int max_hz, unsigned int mode)

...

Quoted 1 line

+void spi_free_slave(struct spi_slave *slave)

....

Quoted 1 line

+int spi_claim_bus(struct spi_slave *slave)

....

Quoted 1 line

+void spi_release_bus(struct spi_slave *slave)

....

drivers/spi/davinci_spi.c line 131

Quoted 2 lines

drivers/spi/davinci_spi.c new file +2 -0

+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
+		const void *dout, void *din, unsigned long flags)

I wonder if all (SPI API) functions from include/spi.h should be implemented? I.e. do we need (dummy) functions for spi_cs_is_valid(), spi_cs_activate() and spi_cs_deactivate(), too? Just in case a driver does use them.

drivers/spi/davinci_spi.h line 3

Quoted 9 lines

drivers/spi/davinci_spi.h new file +3 -0

new file mode 100644
index 0000000..b3bf916
@@ -0,0 +1,84 @@
+/*
+ * Register definitions for the DaVinci SPI Controller
+ */

Do we want a complete GPL header, like in the .c file, here, too?

Do we want a protection against multiple inclusion

#ifndef _DAVINCI_SPI_H_
#define _DAVINCI_SPI_H_

in this header file?

Best regards

Dirk

Sudhakar Rajashekhara

author

2010-01-04 09:42 UTC #4 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

Hi,

Quoted 16 lines · Dirk Behme

Quoted 15 lines · Sudhakar Rajashekhara
From: Sekhar Nori<[email protected]>
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by Sekhar Nori<[email protected]>
Signed-off-by Sudhakar Rajashekhara<[email protected]>
---
  drivers/spi/Makefile      |    1 +
  drivers/spi/davinci_spi.c |  205 +++++++++++++++++++++++++++++++++++++++++++++
  drivers/spi/davinci_spi.h |   84 ++++++++++++++++++
  3 files changed, 290 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/davinci_spi.c
create mode 100644 drivers/spi/davinci_spi.h

Quoted 12 lines

drivers/spi/Makefile COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o · line 37
Quoted 9 lines
drivers/spi/Makefile +1 -0
index 824d8e7..07b9611 100644
@@ -34,6 +34,7 @@ COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
  COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
  COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
  COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
Should this list be kept sorted alphabetically? I.e.

I'll arrange them alphabetically.

Quoted 60 lines

...
COBJS-$(CONFIG_BFIN_SPI) += bfin_spi.o
COBJS-$(CONFIG_CF_SPI) += cf_spi.o
+COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
COBJS-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
...
?
Quoted 8 lines
drivers/spi/Makefile +0 -0
  COBJS	:= $(COBJS-y)
  SRCS	:= $(COBJS:.o=.c)
drivers/spi/davinci_spi.c new file +0 -0
new file mode 100644
index 0000000..1c988bb
@@ -0,0 +1,205 @@
...
Quoted 1 line
+void spi_init()
...
Quoted 2 lines
drivers/spi/davinci_spi.c new file +2 -0
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+			unsigned int max_hz, unsigned int mode)
...
Quoted 1 line
+void spi_free_slave(struct spi_slave *slave)
....
Quoted 1 line
+int spi_claim_bus(struct spi_slave *slave)
....
Quoted 1 line
+void spi_release_bus(struct spi_slave *slave)
....
drivers/spi/davinci_spi.c line 131
Quoted 2 lines
drivers/spi/davinci_spi.c new file +2 -0
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
+		const void *dout, void *din, unsigned long flags)
I wonder if all (SPI API) functions from include/spi.h should be implemented? I.e. do we need (dummy) functions for spi_cs_is_valid(), spi_cs_activate() and spi_cs_deactivate(), too? Just in case a driver does use them.
drivers/spi/davinci_spi.h line 3
Quoted 9 lines
drivers/spi/davinci_spi.h new file +3 -0
new file mode 100644
index 0000000..b3bf916
@@ -0,0 +1,84 @@
+/*
+ * Register definitions for the DaVinci SPI Controller
+ */
Do we want a complete GPL header, like in the .c file, here, too?
Do we want a protection against multiple inclusion
#ifndef _DAVINCI_SPI_H_
#define _DAVINCI_SPI_H_
in this header file?
Best regards
Dirk

Sudhakar Rajashekhara

author

2010-01-04 09:46 UTC #5 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

In reply to Dirk Behme

Hi,

Quoted 16 lines · Dirk Behme

Quoted 15 lines · Sudhakar Rajashekhara
From: Sekhar Nori<[email protected]>
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by Sekhar Nori<[email protected]>
Signed-off-by Sudhakar Rajashekhara<[email protected]>
---
  drivers/spi/Makefile      |    1 +
  drivers/spi/davinci_spi.c |  205 +++++++++++++++++++++++++++++++++++++++++++++
  drivers/spi/davinci_spi.h |   84 ++++++++++++++++++
  3 files changed, 290 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/davinci_spi.c
create mode 100644 drivers/spi/davinci_spi.h

Quoted 12 lines

drivers/spi/Makefile COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o · line 37
Quoted 9 lines
drivers/spi/Makefile +1 -0
index 824d8e7..07b9611 100644
@@ -34,6 +34,7 @@ COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
  COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
  COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
  COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
Should this list be kept sorted alphabetically? I.e.

I'll arrange them alphabetically.

Quoted 28 lines

...
COBJS-$(CONFIG_BFIN_SPI) += bfin_spi.o
COBJS-$(CONFIG_CF_SPI) += cf_spi.o
+COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
COBJS-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
...
?
Quoted 8 lines
drivers/spi/Makefile +0 -0
  COBJS	:= $(COBJS-y)
  SRCS	:= $(COBJS:.o=.c)
drivers/spi/davinci_spi.c new file +0 -0
new file mode 100644
index 0000000..1c988bb
@@ -0,0 +1,205 @@
...
Quoted 1 line
+void spi_init()
...
Quoted 2 lines
drivers/spi/davinci_spi.c new file +2 -0
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+			unsigned int max_hz, unsigned int mode)
...
Quoted 1 line
+void spi_free_slave(struct spi_slave *slave)
....
Quoted 1 line
+int spi_claim_bus(struct spi_slave *slave)
....
Quoted 1 line
+void spi_release_bus(struct spi_slave *slave)

Quoted 9 lines

....
drivers/spi/davinci_spi.c line 131
Quoted 2 lines
drivers/spi/davinci_spi.c new file +2 -0
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
+		const void *dout, void *din, unsigned long flags)
I wonder if all (SPI API) functions from include/spi.h should be implemented? I.e. do we need (dummy) functions for spi_cs_is_valid(), spi_cs_activate() and spi_cs_deactivate(), too? Just in case a driver does use them.

Currently none of the SPI Flash drivers are using the above functions. I'll implement the above functions as dummy and later as and when the need arises, these functions will be implemented.

Quoted 12 lines

drivers/spi/davinci_spi.h line 3
Quoted 9 lines
drivers/spi/davinci_spi.h new file +3 -0
new file mode 100644
index 0000000..b3bf916
@@ -0,0 +1,84 @@
+/*
+ * Register definitions for the DaVinci SPI Controller
+ */
Do we want a complete GPL header, like in the .c file, here, too?

Quoted 6 lines

Do we want a protection against multiple inclusion
#ifndef _DAVINCI_SPI_H_
#define _DAVINCI_SPI_H_
in this header file?

I'll add the GPL header and guard against multiple inclusion.

Thanks for the comments. I'll submit an updated version of the patch soon.

Regards,
Sudhakar

Mike Frysinger

2010-01-05 06:01 UTC #6 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

Quoted 9 lines · On Monday 04 January 2010 04:46:19 Sudhakar Rajashekhara wrote:

Quoted 4 lines · Dirk Behme
I wonder if all (SPI API) functions from include/spi.h should be implemented? I.e. do we need (dummy) functions for spi_cs_is_valid(), spi_cs_activate() and spi_cs_deactivate(), too? Just in case a driver does use them.
Currently none of the SPI Flash drivers are using the above functions. I'll implement the above functions as dummy and later as and when the need
arises, these functions will be implemented.

considering the SPI protocol, the implementation for the spi_cs_* functions should fall naturally into place if the driver is architected properly. in other words, your bus driver should already be relying on these functions -- you call them when you begin/end a transfer.

same goes for spi_cs_is_valid ... your spi_setup_slave function shouldnt be accepting any random cs value given to it.

the Blackfin driver shows that these are all pretty trivial to get right. -mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100105/be908eaf/attachment.pgp

Pedanekar

2010-01-05 07:14 UTC #7 lore raw

[U-Boot] NS16550 Register structure

Hello,

I have observed that in the recent commit (file include/ns16550.h), the ns16550 UART register structure declaration is changed so that irrespective of the CONFIG_SYS_NS16550_REG_SIZE setting the register access will be done at byte level.

This crates problem when UART space access is required to be word-aligned. I am particularly referring to DaVinci DM6467, where STRB over UART register space is not supported and requires 32-bit access (L/STR).

If I go ahead and add a specific handling in ns16550.h file (structure register members as "long"), I would end up making code similar to the one earlier to last commit. Is there a better way to handle such case?

Thanks.
-
Hemant

Wolfgang Denk

2010-01-05 07:35 UTC #8 lore raw

[U-Boot] NS16550 Register structure

Dear "Pedanekar, Hemant",

Quoted 2 lines · In message <[email protected]> you…

I have observed that in the recent commit (file include/ns16550.h), the ns16550 UART register structure declaration is changed so that irrespective of the CONFIG_SYS_NS16550_REG_SIZE setting the register access will be done at byte level.

This seems to be working on all systems so far, which is not so much of a surprise as the registers are 8 bit wide only.

Quoted 1 line

This crates problem when UART space access is required to be word-aligned. I am particularly referring to DaVinci DM6467, where STRB over UART register space is not supported and requires 32-bit access (L/STR).

Please make sure to use exact terms. Alignment is one thing, and bus width is another one. Alignment can be easily adjusted using the prepad_* or postpad_* settings.

Quoted 1 line

If I go ahead and add a specific handling in ns16550.h file (structure register members as "long"), I would end up making code similar to the one earlier to last commit. Is there a better way to handle such case?

No such changes should be needed, as far as I understand the code.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In theory, there is no difference between  theory  and  practice.  In
practice, however, there is.

Pedanekar

2010-01-05 08:55 UTC #9 lore raw

[U-Boot] NS16550 Register structure

Wolfgang,

Yes, that is not just word-aligned, instead, the Dm6467 UART access need to be "Word LOAD/STORE". Thus, if I use STRB <THR> instead of STR <THR> to transmit a character, it doesn't work.

As you said, using prepad/postpad, we can define the alignment but my understanding is that the final register access will be byte access only as the register will be declared as "unsigned char".

For generating 32-bit load/store, probably we need to declare register members as "unsigned long". Or can we achieve the same without any code changes?

Thanks
-
Hemant

Quoted 44 lines

-----Original Message-----
From: Wolfgang Denk [mailto:wd at denx.de]
Sent: Tuesday, January 05, 2010 1:05 PM
To: Pedanekar, Hemant
Cc: u-boot at lists.denx.de; dzu at denx.de
Subject: Re: [U-Boot] NS16550 Register structure
Dear "Pedanekar, Hemant",
In message <[email protected]>
Quoted 2 lines · you
I have observed that in the recent commit (file include/ns16550.h), the
ns16550 UART register structure declaration is changed so that irrespective of the CONFIG_SYS_NS16550_REG_SIZE setting the register access will be done at byte level.
This seems to be working on all systems so far, which is not so much of a surprise as the registers are 8 bit wide only.
Quoted 1 line
This crates problem when UART space access is required to be word-
aligned. I am particularly referring to DaVinci DM6467, where STRB over UART register space is not supported and requires 32-bit access (L/STR).
Please make sure to use exact terms. Alignment is one thing, and bus width is another one. Alignment can be easily adjusted using the prepad_* or postpad_* settings.
Quoted 1 line
If I go ahead and add a specific handling in ns16550.h file (structure
register members as "long"), I would end up making code similar to the one earlier to last commit. Is there a better way to handle such case?
No such changes should be needed, as far as I understand the code.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de In theory, there is no difference between theory and practice. In practice, however, there is.

Wolfgang Denk

2010-01-05 09:58 UTC #10 lore raw

[U-Boot] NS16550 Register structure

Dear "Pedanekar, Hemant",

Quoted 4 lines · In message <[email protected]> you…

Yes, that is not just word-aligned, instead, the Dm6467 UART access need to=
be "Word LOAD/STORE". Thus, if I use STRB <THR> instead of STR <THR> to tr=
ansmit a character, it doesn't work.

Please use shorter lines (something like < 70 characters).

Can you please explain why byte I/O does not work for you? Is this a hardware problem on your specific board?

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected]
It is necessary to have purpose.
	-- Alice #1, "I, Mudd", stardate 4513.3

Pedanekar

2010-01-18 14:51 UTC #11 lore raw

[U-Boot] NS16550 Register structure

Hi Wolfgang,

Only 32-bit access is supported for UART MMRs on DM6467 and hence need to use STR/LDR instead of STRB/LDRB.

Thanks
-
Hemant

Quoted 33 lines

-----Original Message-----
From: Wolfgang Denk [mailto:wd at denx.de]
Sent: Tuesday, January 05, 2010 3:29 PM
To: Pedanekar, Hemant
Cc: u-boot at lists.denx.de; dzu at denx.de
Subject: Re: [U-Boot] NS16550 Register structure
Dear "Pedanekar, Hemant",
In message <[email protected]>
Quoted 2 lines · you
Yes, that is not just word-aligned, instead, the Dm6467 UART access need
to=
Quoted 1 line
be "Word LOAD/STORE". Thus, if I use STRB <THR> instead of STR <THR> to
tr=
Quoted 1 line
ansmit a character, it doesn't work.
Please use shorter lines (something like < 70 characters).
Can you please explain why byte I/O does not work for you? Is this a hardware problem on your specific board?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de It is necessary to have purpose.
	-- Alice #1, "I, Mudd", stardate 4513.3

Detlev Zundel

2010-01-19 13:04 UTC #12 lore raw

[U-Boot] NS16550 Register structure

Hi Hemant,

Quoted 2 lines

Only 32-bit access is supported for UART MMRs on DM6467 and hence need to use STR/LDR instead of STRB/LDRB.

Argh. Why do hw people always think software can fix anything? So out of interest - do you get a bus fault or what? Maybe we can fixup such non-aligned accesses outside of the serial driver?

I'm really reluctant to topple over an infratstructure working on _many_ differenty hardware architectures and platforms only for one special case which may be considered to be buggy hardware...

Cheers
Detlev

Signature · 8 lines

While  the list of  different methods is not  endless, it is certainly
pretty long, with new ones being dreamed up all the time. Fortunately,
space limitations prevent us from looking at all of them.
                                            -- Andrew S. Tanenbaum
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Pedanekar

2010-01-21 11:58 UTC #13 lore raw

[U-Boot] NS16550 Register structure

Hi Detlev,

There is no abort on ARM and also nothing gets to the console when UART register is accessed.

Do you think fixing this outside uart driver is possible? Maybe overriding UART register structure declaration or using some compiler magic? Please suggest.

Thanks
-
Hemant

Quoted 32 lines

-----Original Message-----
From: Detlev Zundel [mailto:dzu at denx.de]
Sent: Tuesday, January 19, 2010 6:34 PM
To: Pedanekar, Hemant
Cc: Wolfgang Denk; u-boot at lists.denx.de
Subject: Re: [U-Boot] NS16550 Register structure
Hi Hemant,
Quoted 2 lines
Only 32-bit access is supported for UART MMRs on DM6467 and hence need to use STR/LDR instead of STRB/LDRB.
Argh. Why do hw people always think software can fix anything? So out of interest - do you get a bus fault or what? Maybe we can fixup such non-aligned accesses outside of the serial driver?
I'm really reluctant to topple over an infratstructure working on _many_ differenty hardware architectures and platforms only for one special case which may be considered to be buggy hardware...
Cheers
Detlev
--
While the list of different methods is not endless, it is certainly pretty long, with new ones being dreamed up all the time. Fortunately, space limitations prevent us from looking at all of them.
-- Andrew S. Tanenbaum
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Pedanekar

2010-01-28 09:54 UTC #14 lore raw

[U-Boot] NS16550 Register structure

In reply to Detlev Zundel

Hi Detlev, Wolfgang,

Any suggestions on this?

Thanks
-
Hemant

Quoted 54 lines

-----Original Message-----
From: Pedanekar, Hemant
Sent: Thursday, January 21, 2010 4:34 PM
To: 'Detlev Zundel'
Cc: Wolfgang Denk; u-boot at lists.denx.de
Subject: RE: [U-Boot] NS16550 Register structure
Hi Detlev,
There is no abort on ARM and also nothing gets to the console when UART register is accessed.
Do you think fixing this outside uart driver is possible? Maybe overriding UART register structure declaration or using some compiler magic? Please suggest.
Thanks
-
Hemant
Quoted 32 lines
-----Original Message-----
From: Detlev Zundel [mailto:dzu at denx.de]
Sent: Tuesday, January 19, 2010 6:34 PM
To: Pedanekar, Hemant
Cc: Wolfgang Denk; u-boot at lists.denx.de
Subject: Re: [U-Boot] NS16550 Register structure
Hi Hemant,
Quoted 2 lines
Only 32-bit access is supported for UART MMRs on DM6467 and hence need to use STR/LDR instead of STRB/LDRB.
Argh. Why do hw people always think software can fix anything? So out of interest - do you get a bus fault or what? Maybe we can fixup such non-aligned accesses outside of the serial driver?
I'm really reluctant to topple over an infratstructure working on _many_ differenty hardware architectures and platforms only for one special case which may be considered to be buggy hardware...
Cheers
Detlev
--
While the list of different methods is not endless, it is certainly pretty long, with new ones being dreamed up all the time. Fortunately, space limitations prevent us from looking at all of them.
-- Andrew S. Tanenbaum
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Wolfgang Denk

2010-01-28 11:00 UTC #15 lore raw

[U-Boot] NS16550 Register structure

Dear Hemant,

Quoted 2 lines · In message <[email protected]> you…

Any suggestions on this?

Well, the best solution would obviously be to fix the hardware...

Quoted 2 lines

Quoted 2 lines
There is no abort on ARM and also nothing gets to the console when UART register is accessed.

Then what exactly is happening on your hardware when you read the register? Did you really try out all options, i. e. reading as a 8 bit access with 0, 1, 2 and 3 bytes offset, etc.?

If you don't see any ecxceptions, then these accesses must be somehow visible on the bus. Find out (using a scope / logic analyzer) which byte lanes get accessed when, and show us a table here what happens when.

Only after we understand what is happening on your system we can come up with any recommendations.

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Alliance: In international politics, the union  of  two  thieves  who
have  their hands so deeply inserted in each other's pocket that they
cannot separately plunder a third.                   - Ambrose Bierce

Nick Thompson

2010-01-04 09:47 UTC #16 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

In reply to Sudhakar Rajashekhara

Quoted 14 lines · Sudhakar Rajashekhara

From: Sekhar Nori <[email protected]>
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by Sekhar Nori <[email protected]>
Signed-off-by Sudhakar Rajashekhara <[email protected]>
---
 drivers/spi/Makefile      |    1 +
 drivers/spi/davinci_spi.c |  205 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/spi/davinci_spi.h |   84 ++++++++++++++++++
 3 files changed, 290 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/davinci_spi.c
 create mode 100644 drivers/spi/davinci_spi.h

...

Quoted 24 lines

drivers/spi/davinci_spi.h new file +18 -0

new file mode 100644
index 0000000..b3bf916
@@ -0,0 +1,84 @@
+/*
+ * Register definitions for the DaVinci SPI Controller
+ */
+
+/* Register offsets */
+#define DAVINCI_SPI_GCR0	0x0000
+#define DAVINCI_SPI_GCR1	0x0004
+#define DAVINCI_SPI_INT0	0x0008
+#define DAVINCI_SPI_LVL		0x000c
+#define DAVINCI_SPI_FLG		0x0010
+#define DAVINCI_SPI_PC0		0x0014
+#define DAVINCI_SPI_PC1		0x0018
+#define DAVINCI_SPI_PC2		0x001c
+#define DAVINCI_SPI_PC3		0x0020
+#define DAVINCI_SPI_PC4		0x0024
+#define DAVINCI_SPI_PC5		0x0028
+#define DAVINCI_SPI_DAT0	0x0038
+#define DAVINCI_SPI_DAT1	0x003c

drivers/spi/davinci_spi.h line 28

Quoted 10 lines

drivers/spi/davinci_spi.h new file +10 -0

+#define DAVINCI_SPI_BUF		0x0040
+#define DAVINCI_SPI_EMU		0x0044
+#define DAVINCI_SPI_DELAY	0x0048
+#define DAVINCI_SPI_DEF		0x004c
+#define DAVINCI_SPI_FMT0	0x0050
+#define DAVINCI_SPI_FMT1	0x0054
+#define DAVINCI_SPI_FMT2	0x0058
+#define DAVINCI_SPI_FMT3	0x005c
+#define DAVINCI_SPI_INTVEC0	0x0060
+#define DAVINCI_SPI_INTVEC1	0x0064

I think this ought to be a C structure, rather than register offsets?

...

drivers/spi/davinci_spi.h line 70

Quoted 4 lines

drivers/spi/davinci_spi.h new file +4 -0

+
+struct davinci_spi_slave {
+	struct spi_slave slave;
+	void		*regs;

This should have the type of the C structure to be defined above.

Quoted 4 lines

drivers/spi/davinci_spi.h new file +4 -0
+	u32		mr;
+	unsigned int freq;
+};
+

...

drivers/spi/davinci_spi.h line 83

Quoted 5 lines

drivers/spi/davinci_spi.h new file +5 -0

+
+#define spi_readl(ds, reg)					\
+	readl(ds->regs + DAVINCI_SPI_##reg)
+#define spi_writel(ds, reg, value)				\
+	writel(value, ds->regs + DAVINCI_SPI_##reg)

These can be rewritten (and the usages changed slightly) to access via the structure. You will then not be circumventing any type checking.

Maybe these defs then become too trivial and can be dropped altogether?

Regards,
Nick.

Sudhakar Rajashekhara

author

2010-01-04 10:42 UTC #17 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

Hi Nick,

Quoted 46 lines · Nick Thompson

Quoted 14 lines · Sudhakar Rajashekhara

From: Sekhar Nori <[email protected]>
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by Sekhar Nori <[email protected]>
Signed-off-by Sudhakar Rajashekhara <[email protected]>
---
 drivers/spi/Makefile      |    1 +
 drivers/spi/davinci_spi.c |  205 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/spi/davinci_spi.h |   84 ++++++++++++++++++
 3 files changed, 290 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/davinci_spi.c
 create mode 100644 drivers/spi/davinci_spi.h

...

Quoted 28 lines

drivers/spi/davinci_spi.h new file +22 -0

new file mode 100644
index 0000000..b3bf916
@@ -0,0 +1,84 @@
+/*
+ * Register definitions for the DaVinci SPI Controller
+ */
+
+/* Register offsets */
+#define DAVINCI_SPI_GCR0	0x0000
+#define DAVINCI_SPI_GCR1	0x0004
+#define DAVINCI_SPI_INT0	0x0008
+#define DAVINCI_SPI_LVL		0x000c
+#define DAVINCI_SPI_FLG		0x0010
+#define DAVINCI_SPI_PC0		0x0014
+#define DAVINCI_SPI_PC1		0x0018
+#define DAVINCI_SPI_PC2		0x001c
+#define DAVINCI_SPI_PC3		0x0020
+#define DAVINCI_SPI_PC4		0x0024
+#define DAVINCI_SPI_PC5		0x0028
+#define DAVINCI_SPI_DAT0	0x0038
+#define DAVINCI_SPI_DAT1	0x003c
+#define DAVINCI_SPI_BUF		0x0040
+#define DAVINCI_SPI_EMU		0x0044
+#define DAVINCI_SPI_DELAY	0x0048
+#define DAVINCI_SPI_DEF		0x004c

Quoted 9 lines

drivers/spi/davinci_spi.h line 28
Quoted 6 lines
drivers/spi/davinci_spi.h new file +6 -0
+#define DAVINCI_SPI_FMT0	0x0050
+#define DAVINCI_SPI_FMT1	0x0054
+#define DAVINCI_SPI_FMT2	0x0058
+#define DAVINCI_SPI_FMT3	0x005c
+#define DAVINCI_SPI_INTVEC0	0x0060
+#define DAVINCI_SPI_INTVEC1	0x0064
I think this ought to be a C structure, rather than register offsets?

I see that existing SPI drivers are not using structures for register defines. Off-late is there a shift in u-boot in favor of structures than macros for register definitions?

Regards,
Sudhakar

Dirk Behme

2010-01-04 19:50 UTC #18 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

Quoted 59 lines · Sudhakar Rajashekhara

Hi Nick,
Quoted 46 lines · Nick Thompson
Quoted 14 lines · Sudhakar Rajashekhara
From: Sekhar Nori<[email protected]>
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by Sekhar Nori<[email protected]>
Signed-off-by Sudhakar Rajashekhara<[email protected]>
---
  drivers/spi/Makefile      |    1 +
  drivers/spi/davinci_spi.c |  205 +++++++++++++++++++++++++++++++++++++++++++++
  drivers/spi/davinci_spi.h |   84 ++++++++++++++++++
  3 files changed, 290 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/davinci_spi.c
create mode 100644 drivers/spi/davinci_spi.h
...
Quoted 28 lines
drivers/spi/davinci_spi.h new file +22 -0
new file mode 100644
index 0000000..b3bf916
@@ -0,0 +1,84 @@
+/*
+ * Register definitions for the DaVinci SPI Controller
+ */
+
+/* Register offsets */
+#define DAVINCI_SPI_GCR0	0x0000
+#define DAVINCI_SPI_GCR1	0x0004
+#define DAVINCI_SPI_INT0	0x0008
+#define DAVINCI_SPI_LVL		0x000c
+#define DAVINCI_SPI_FLG		0x0010
+#define DAVINCI_SPI_PC0		0x0014
+#define DAVINCI_SPI_PC1		0x0018
+#define DAVINCI_SPI_PC2		0x001c
+#define DAVINCI_SPI_PC3		0x0020
+#define DAVINCI_SPI_PC4		0x0024
+#define DAVINCI_SPI_PC5		0x0028
+#define DAVINCI_SPI_DAT0	0x0038
+#define DAVINCI_SPI_DAT1	0x003c
+#define DAVINCI_SPI_BUF		0x0040
+#define DAVINCI_SPI_EMU		0x0044
+#define DAVINCI_SPI_DELAY	0x0048
+#define DAVINCI_SPI_DEF		0x004c
Quoted 9 lines
drivers/spi/davinci_spi.h line 28
Quoted 6 lines
drivers/spi/davinci_spi.h new file +6 -0
+#define DAVINCI_SPI_FMT0	0x0050
+#define DAVINCI_SPI_FMT1	0x0054
+#define DAVINCI_SPI_FMT2	0x0058
+#define DAVINCI_SPI_FMT3	0x005c
+#define DAVINCI_SPI_INTVEC0	0x0060
+#define DAVINCI_SPI_INTVEC1	0x0064
I think this ought to be a C structure, rather than register offsets?

Quoted 3 lines

I see that existing SPI drivers are not using structures for register defines. Off-late is there a shift in u-boot in favor of structures than macros for register definitions?

Yes.

E.g. what I did for OMAP3 SPI driver:

/* OMAP3 McSPI registers */

struct mcspi_channel {
	unsigned int chconf;		/* 0x2C, 0x40, 0x54, 0x68 */
	unsigned int chstat;		/* 0x30, 0x44, 0x58, 0x6C */
	unsigned int chctrl;		/* 0x34, 0x48, 0x5C, 0x70 */
	unsigned int tx;		/* 0x38, 0x4C, 0x60, 0x74 */
	unsigned int rx;		/* 0x3C, 0x50, 0x64, 0x78 */
};

struct mcspi {
	unsigned char res1[0x10];
	unsigned int sysconfig;		/* 0x10 */
	unsigned int sysstatus;		/* 0x14 */
	unsigned int irqstatus;		/* 0x18 */
	unsigned int irqenable;		/* 0x1C */
	unsigned int wakeupenable;	/* 0x20 */
	unsigned int syst;		/* 0x24 */
	unsigned int modulctrl;		/* 0x28 */
	struct mcspi_channel channel[4];
}

regs = (struct mcspi *)OMAP3_MCSPI1_BASE;

writel(value, regs->modulctrl);

Best regards

Dirk

Mike Frysinger

2010-01-05 00:20 UTC #19 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

Quoted 12 lines · On Monday 04 January 2010 14:50:54 Dirk Behme wrote:

struct mcspi {
	unsigned char res1[0x10];
	unsigned int sysconfig;		/* 0x10 */
	unsigned int sysstatus;		/* 0x14 */
	unsigned int irqstatus;		/* 0x18 */
	unsigned int irqenable;		/* 0x1C */
	unsigned int wakeupenable;	/* 0x20 */
	unsigned int syst;		/* 0x24 */
	unsigned int modulctrl;		/* 0x28 */
	struct mcspi_channel channel[4];
}

Quoted 3 lines

regs = (struct mcspi *)OMAP3_MCSPI1_BASE;
writel(value, regs->modulctrl);

i think you mean &regs->modulctrl
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100104/1e061348/attachment.pgp

Mansoor

2010-03-05 06:47 UTC #20 lore raw

PATCH 1/1 TI DaVinci: Driver for the davinci SPI controller

In reply to Dirk Behme

Quoted 12 lines · Dirk Behme <dirk.behme <at> googlemail.com> writes:

E.g. what I did for OMAP3 SPI driver:

/* OMAP3 McSPI registers */

struct mcspi_channel {
	unsigned int chconf;		/* 0x2C, 0x40, 0x54, 0x68 */
	unsigned int chstat;		/* 0x30, 0x44, 0x58, 0x6C */
	unsigned int chctrl;		/* 0x34, 0x48, 0x5C, 0x70 */
	unsigned int tx;		/* 0x38, 0x4C, 0x60, 0x74 */
	unsigned int rx;		/* 0x3C, 0x50, 0x64, 0x78 */
};

[...]

Quoted 2 lines

writel(value, regs->modulctrl);

Hi Dirk,

I could not find this code in any of the repositories. Could you share the u-boot omap3 spi driver?

Thanks
Mansoor

Dirk Behme

2010-03-07 12:38 UTC #21 lore raw

[U-Boot] OMAP3 SPI driver ( was Re: [PATCH] TI DaVinci: Driver for the davinci SPI controller)

Hi Mansoor,

Quoted 19 lines · Mansoor

Quoted 12 lines · Dirk Behme<dirk.behme<at> googlemail.com> writes:
E.g. what I did for OMAP3 SPI driver:
/* OMAP3 McSPI registers */
struct mcspi_channel {
	unsigned int chconf;		/* 0x2C, 0x40, 0x54, 0x68 */
	unsigned int chstat;		/* 0x30, 0x44, 0x58, 0x6C */
	unsigned int chctrl;		/* 0x34, 0x48, 0x5C, 0x70 */
	unsigned int tx;		/* 0x38, 0x4C, 0x60, 0x74 */
	unsigned int rx;		/* 0x3C, 0x50, 0x64, 0x78 */
};
[...]

Quoted 7 lines

Quoted 2 lines
writel(value, regs->modulctrl);
Hi Dirk,
I could not find this code in any of the repositories. Could you share the u-boot omap3 spi driver?

I used the SPI driver for DaVinci (which this thread is about) as starting point for an U-Boot OMAP3 driver. The goal was to be able to access SPI ethernet on Zippy expansion board for Beagle. Unfortunately, I never found the time to finalize it.

Please find my latest patch from beginning of January 2010 in attachment.

I really hope it might help to finalize it!

Many thanks for asking and best regards

Dirk

P.S.: You might notice that the patch contains changes for the SPI ethernet, too. This should be moved to a separate patch.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: omap3_spi_patch.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20100307/3ea76be9/attachment.txt