Skip to contentSkip to conversation

spi: add support SuperH SPI module

PATCH New Yoshihiro Shimoda 3 participants 5 messages Jan 17, 2011 – Jan 19, 2011 SPI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 spi: add support SuperH SPI module

SH7757 has SPI module. This patch supports it.

---
 drivers/spi/Makefile |    1 +
 drivers/spi/sh_spi.c |  295 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 296 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/sh_spi.c
drivers/spi/Makefile +1 -0
index e34a124..d582fbb 100644
@@ -37,6 +37,7 @@ COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
 COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
 COBJS-$(CONFIG_OMAP3_SPI) += omap3_spi.o
 COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_SH_SPI) += sh_spi.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/spi/sh_spi.c new file +295 -0

295 lines not on this page · raw message

Signature · 1 line
1.7.1
#1 lore raw

PATCH 1/1 spi: add support SuperH SPI module

Hi, Shioda-san.

2011/1/17 Yoshihiro Shimoda <[email protected]>:

Quoted 79 lines

SH7757 has SPI module. This patch supports it.

---
?drivers/spi/Makefile | ? ?1 +
?drivers/spi/sh_spi.c | ?295 ++++++++++++++++++++++++++++++++++++++++++++++++++ ?2 files changed, 296 insertions(+), 0 deletions(-)
?create mode 100644 drivers/spi/sh_spi.c

drivers/spi/Makefile +0 -0
index e34a124..d582fbb 100644
@@ -37,6 +37,7 @@ COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o

?COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
?COBJS-$(CONFIG_OMAP3_SPI) += omap3_spi.o
?COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_SH_SPI) += sh_spi.o

?COBJS ?:= $(COBJS-y)
?SRCS ? := $(COBJS:.o=.c)

drivers/spi/sh_spi.c new file +52 -0
new file mode 100644
index 0000000..89ea5e2
@@ -0,0 +1,295 @@
+/*
+ * SH SPI driver
+ *
+ * Copyright (C) 2011 Renesas Solutions Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA ?02110-1301 ?USA
+ *
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <spi.h>
+#include <asm/io.h>
+
+#define SPI_TBR ? ? ? ? ? ? ? ?0x00
+#define SPI_RBR ? ? ? ? ? ? ? ?0x00
+#define SPI_CR1 ? ? ? ? ? ? ? ?0x08
+#define SPI_CR2 ? ? ? ? ? ? ? ?0x10
+#define SPI_CR3 ? ? ? ? ? ? ? ?0x18
+#define SPI_CR4 ? ? ? ? ? ? ? ?0x20
+
+/* CR1 */
+#define SPI_TBE ? ? ? ? ? ? ? ?0x80
+#define SPI_TBF ? ? ? ? ? ? ? ?0x40
+#define SPI_RBE ? ? ? ? ? ? ? ?0x20
+#define SPI_RBF ? ? ? ? ? ? ? ?0x10
+#define SPI_PFONRD ? ? 0x08
+#define SPI_SSDB ? ? ? 0x04
+#define SPI_SSD ? ? ? ? ? ? ? ?0x02
+#define SPI_SSA ? ? ? ? ? ? ? ?0x01
+
+/* CR2 */
+#define SPI_RSTF ? ? ? 0x80
+#define SPI_LOOPBK ? ? 0x40
+#define SH_SPI_CPOL ? ?0x20
+#define SH_SPI_CPHA ? ?0x10
+#define SPI_L1M0 ? ? ? 0x08
+
+/* CR3 */
+#define SPI_MAX_BYTE ? 0xFF
+

drivers/spi/sh_spi.c line 62

Quoted 10 lines
drivers/spi/sh_spi.c new file +10 -0
+/* CR4 */
+#define SPI_TBEI ? ? ? 0x80
+#define SPI_TBFI ? ? ? 0x40
+#define SPI_RBEI ? ? ? 0x20
+#define SPI_RBFI ? ? ? 0x10
+#define SPI_WPABRT ? ? 0x04
+#define SPI_SSS ? ? ? ? ? ? ? ?0x01
+
+#define SPI_FIFO_SIZE ?32
+

Please add prefix of "SH_".

drivers/spi/sh_spi.c line 67

Quoted 5 lines
drivers/spi/sh_spi.c new file +5 -0
+static void sh_spi_write(unsigned long data, unsigned long offset)
+{
+ ? ? ? writel(data, CONFIG_SH_SPI_BASE + offset);
+}
+

<snip>

\> +

drivers/spi/sh_spi.c line 66

Quoted 4 lines
drivers/spi/sh_spi.c new file +4 -0
+int ?spi_cs_is_valid(unsigned int bus, unsigned int cs)
+{
+ ? ? ? return 1;
+}

Why does this function return "1" every time?

Quoted 12 lines
drivers/spi/sh_spi.c new file +11 -1
+
+void spi_cs_activate(struct spi_slave *slave)
+{
+
+}
+
+void spi_cs_deactivate(struct spi_slave *slave)
+{
+
+}
+
--
Quoted 1 line

1.7.1

Best regards,
Nobuhiro

Signature · 3 lines
Nobuhiro Iwamatsu
?? iwamatsu at {nigauri.org / debian.org}
?? GPG ID: 40AD1FA6
#2 lore raw

PATCH 1/1 spi: add support SuperH SPI module

Hi, Iwamatsu-san,

Thank you for your review.

Quoted 9 lines · Nobuhiro Iwamatsu

Hi, Shioda-san.

2011/1/17 Yoshihiro Shimoda <[email protected]>:

Quoted 6 lines
drivers/spi/sh_spi.c new file +6 -0
+#define SPI_TBR                0x00
+#define SPI_RBR                0x00
+#define SPI_CR1                0x08
+#define SPI_CR2                0x10
+#define SPI_CR3                0x18
+#define SPI_CR4                0x20

< snip >

Quoted 1 line

Please add prefix of "SH_".

Yes, I will add it.

Quoted 6 lines

drivers/spi/sh_spi.c line 284

Quoted 4 lines
drivers/spi/sh_spi.c new file +4 -0
+int  spi_cs_is_valid(unsigned int bus, unsigned int cs)
+{
+       return 1;
+}

Why does this function return "1" every time?

Oh, it's temporary code. I will fix this function.

Best regards,
Yoshihiro Shimoda

#3 lore raw

PATCH 1/1 spi: add support SuperH SPI module

In reply to Yoshihiro Shimoda

Dear Yoshihiro Shimoda,

Quoted 14 lines · In message <[email protected]> you wrote:

SH7757 has SPI module. This patch supports it.

---
 drivers/spi/Makefile |    1 +
 drivers/spi/sh_spi.c |  295 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 296 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/sh_spi.c
drivers/spi/Makefile +0 -0
index e34a124..d582fbb 100644

drivers/spi/Makefile COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o · line 40

Quoted 4 lines
drivers/spi/Makefile +1 -0
@@ -37,6 +37,7 @@ COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
 COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
 COBJS-$(CONFIG_OMAP3_SPI) += omap3_spi.o
 COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_SH_SPI) += sh_spi.o

Please keep list sorted.

Quoted 5 lines
drivers/spi/sh_spi.c new file +0 -0
new file mode 100644
index 0000000..89ea5e2

...

drivers/spi/sh_spi.c line 31

Quoted 6 lines
drivers/spi/sh_spi.c new file +6 -0
+#define SPI_TBR		0x00
+#define SPI_RBR		0x00
+#define SPI_CR1		0x08
+#define SPI_CR2		0x10
+#define SPI_CR3		0x18
+#define SPI_CR4		0x20

Please declare a poper C struct instead.

Quoted 19 lines
drivers/spi/sh_spi.c new file +19 -0
+/* CR1 */
+#define SPI_TBE		0x80
+#define SPI_TBF		0x40
+#define SPI_RBE		0x20
+#define SPI_RBF		0x10
+#define SPI_PFONRD	0x08
+#define SPI_SSDB	0x04
+#define SPI_SSD		0x02
+#define SPI_SSA		0x01
+
+/* CR2 */
+#define SPI_RSTF	0x80
+#define SPI_LOOPBK	0x40
+#define SH_SPI_CPOL	0x20
+#define SH_SPI_CPHA	0x10
+#define SPI_L1M0	0x08
+
+/* CR3 */
+#define SPI_MAX_BYTE	0xFF

drivers/spi/sh_spi.c line 61

Quoted 10 lines
drivers/spi/sh_spi.c new file +10 -0
+
+/* CR4 */
+#define SPI_TBEI	0x80
+#define SPI_TBFI	0x40
+#define SPI_RBEI	0x20
+#define SPI_RBFI	0x10
+#define SPI_WPABRT	0x04
+#define SPI_SSS		0x01
+
+#define SPI_FIFO_SIZE	32

All this stuff should better go into a separate header file.

drivers/spi/sh_spi.c line 65

Quoted 3 lines
drivers/spi/sh_spi.c new file +3 -0
+static void sh_spi_write(unsigned long data, unsigned long offset)
+{
+	writel(data, CONFIG_SH_SPI_BASE + offset);

NAK. We do not allow the base + offset notation. Pleaseuse a proper C struct instead.

Please fix globally.

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
"When the only  tool  you  have  is  a  hammer,  you  tend  to  treat
everything as if it were a nail."                    - Abraham Maslow
#4 lore raw

PATCH 1/1 spi: add support SuperH SPI module

Dear Wolfgang Denk,

Thank you very much for your review.

Quoted 7 lines · Wolfgang Denk

drivers/spi/Makefile COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o · line 40

Quoted 5 lines
drivers/spi/Makefile +1 -0
@@ -37,6 +37,7 @@ COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
 COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
 COBJS-$(CONFIG_OMAP3_SPI) += omap3_spi.o
 COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
+COBJS-$(CONFIG_SH_SPI) += sh_spi.o

Please keep list sorted.

I will fix it.

Quoted 8 lines

drivers/spi/sh_spi.c line 31

Quoted 6 lines
drivers/spi/sh_spi.c new file +6 -0
+#define SPI_TBR		0x00
+#define SPI_RBR		0x00
+#define SPI_CR1		0x08
+#define SPI_CR2		0x10
+#define SPI_CR3		0x18
+#define SPI_CR4		0x20

Please declare a poper C struct instead.

I will modify it.

Quoted 11 lines

drivers/spi/sh_spi.c line 61

Quoted 9 lines
drivers/spi/sh_spi.c new file +9 -0
+/* CR4 */
+#define SPI_TBEI	0x80
+#define SPI_TBFI	0x40
+#define SPI_RBEI	0x20
+#define SPI_RBFI	0x10
+#define SPI_WPABRT	0x04
+#define SPI_SSS		0x01
+
+#define SPI_FIFO_SIZE	32

All this stuff should better go into a separate header file.

OK, I will create the sh_spi.h file.

Quoted 8 lines

drivers/spi/sh_spi.c line 65

Quoted 3 lines
drivers/spi/sh_spi.c new file +3 -0
+static void sh_spi_write(unsigned long data, unsigned long offset)
+{
+	writel(data, CONFIG_SH_SPI_BASE + offset);

NAK. We do not allow the base + offset notation. Pleaseuse a proper C struct instead.

Please fix globally.

Yes, I will fix the driver.

Best regards,
Yoshihiro Shimoda