Skip to contentSkip to conversation

460EX on-chip SATA driver<kernel 2.6.33><resubmission>

PATCH In review Rupjyoti Sarmah 7 participants 7 messages Jun 4, 2010 – Jun 29, 2010 libata

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 460EX on-chip SATA driver<kernel 2.6.33><resubmission>

This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.

---
 drivers/ata/Kconfig          |    9 +
 drivers/ata/Makefile         |    1 +
 drivers/ata/sata_dwc_460ex.c | 1808 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1818 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ata/sata_dwc_460ex.c
drivers/ata/Kconfig +9 -0
index 56c6374..bba7b8a 100644
@@ -82,6 +82,15 @@ config SATA_FSL
 
 	  If unsure, say N.
 
+config SATA_DWC
+	tristate "DesignWare Cores SATA support"
+	depends on 460EX
+	help
+	  This option enables support for the on-chip SATA controller of the
+	  AppliedMicro processor 460EX.
+
+	  If unsure, say N.
+
 config ATA_SFF
 	bool "ATA SFF support"
 	default y
drivers/ata/Makefile +1 -0
index fc936d4..96ff315 100644
@@ -19,6 +19,7 @@ obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o
 obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
 obj-$(CONFIG_SATA_FSL)		+= sata_fsl.o
 obj-$(CONFIG_PATA_MACIO)	+= pata_macio.o
+obj-$(CONFIG_SATA_DWC)		+= sata_dwc_460ex.o
 
 obj-$(CONFIG_PATA_ALI)		+= pata_ali.o
 obj-$(CONFIG_PATA_AMD)		+= pata_amd.o
drivers/ata/sata_dwc_460ex.c new file +1808 -0

1,808 lines not on this page · raw message

Signature · 1 line
1.5.6.3
#1 lore raw
Quoted 5 lines · Rupjyoti Sarmah

This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.

What does the <kernel 2.6.33> mean in the Subject?

Quoted 46 lines

---

drivers/ata/Kconfig          |    9 +
drivers/ata/Makefile         |    1 +
drivers/ata/sata_dwc_460ex.c | 1808 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 1818 insertions(+), 0 deletions(-)

create mode 100644 drivers/ata/sata_dwc_460ex.c

drivers/ata/Kconfig +9 -0
index 56c6374..bba7b8a 100644
@@ -82,6 +82,15 @@ config SATA_FSL
 
 	  If unsure, say N.
 
+config SATA_DWC
+	tristate "DesignWare Cores SATA support"
+	depends on 460EX
+	help
+	  This option enables support for the on-chip SATA controller of the
+	  AppliedMicro processor 460EX.
+
+	  If unsure, say N.
+

config ATA_SFF
bool "ATA SFF support"
default y

drivers/ata/Makefile +0 -0
index fc936d4..96ff315 100644
@@ -19,6 +19,7 @@ obj-$(CONFIG_SATA_INIC162X)	+= sata_inic162x.o

obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
obj-$(CONFIG_SATA_FSL) += sata_fsl.o
obj-$(CONFIG_PATA_MACIO) += pata_macio.o
+obj-$(CONFIG_SATA_DWC) += sata_dwc_460ex.o

obj-$(CONFIG_PATA_ALI) += pata_ali.o
obj-$(CONFIG_PATA_AMD) += pata_amd.o

drivers/ata/sata_dwc_460ex.c new file +0 -0
new file mode 100644
index 0000000..e6e2896

drivers/ata/sata_dwc_460ex.c line 9

Quoted 9 lines
drivers/ata/sata_dwc_460ex.c new file +9 -0
@@ -0,0 +1,1808 @@
+/*
+ * drivers/ata/sata_dwc_460ex.c
+ *
+ * Synopsys DesignWare Cores (DWC) SATA host driver
+ *
+ * Author: Mark Miesfeld <[email protected]>
+ *
+ * Ported from 2.6.19.2 to 2.6.25/26 by Stefan Roese <[email protected]>
+ * Copyright 2008 DENX Software Engineering

I'm pretty sure Denx uses Signed-off-by lines in their trees. If you've ported it from their tree, perhaps you should keep Stefan's S-o-b line intact.

drivers/ata/sata_dwc_460ex.c line 372

Quoted 9 lines
drivers/ata/sata_dwc_460ex.c new file +9 -0
+/******************************************************************************
+ * Function: get_burst_length_encode
+ * arguments: datalength: length in bytes of data
+ * returns value to be programmed in register corrresponding to data length
+ * This value is effectively the log(base 2) of the length
+ *****************************************************************************/
+static  int get_burst_length_encode(int datalength)
+{
+	int items = datalength >> 2;	/* div by 4 to get lword count */

A minor suggestion, but if you're going to document the interfaces this way you might want to go ahead and use KernelDoc. It's pretty close to what you have already.

josh

#2 lore raw
In reply to Rupjyoti Sarmah

Dear Rupjyoti Sarmah,

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

This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.

Quoted 6 lines
---
 drivers/ata/Kconfig          |    9 +
 drivers/ata/Makefile         |    1 +
 drivers/ata/sata_dwc_460ex.c | 1808 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1818 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ata/sata_dwc_460ex.c

This driver needs fixing. It will not compile against recent kernel versions.

...

Quoted 6 lines
drivers/ata/sata_dwc_460ex.c new file +0 -0
new file mode 100644
index 0000000..e6e2896
@@ -0,0 +1,1808 @@

...

drivers/ata/sata_dwc_460ex.c line 40

Quoted 10 lines
drivers/ata/sata_dwc_460ex.c new file +10 -0
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/of_platform.h>
+#include <linux/libata.h>
+#include "libata.h"
+
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_cmnd.h>

You miss a "#include <linux/slab.h>" here.

...

drivers/ata/sata_dwc_460ex.c line 1311

Quoted 5 lines
drivers/ata/sata_dwc_460ex.c new file +5 -0
+	for (i = 0; i < SATA_DWC_QCMD_MAX; i++)
+		hsdevp->cmd_issued[i] = SATA_DWC_CMD_ISSUED_NOT;
+
+	ap->prd = 0;	/* set these so libata doesn't use them */
+	ap->prd_dma = 0;

s/prd/bmdma_prd/ in these two lines (cf. commit f60d7011).

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: [email protected]
He had quite a powerful intellect, but it  was  as  powerful  like  a
locomotive,  and  ran on rails and was therefore almost impossible to
steer.                          - Terry Pratchett, _Lords and Ladies_
#3 lore raw
In reply to Rupjyoti Sarmah
Quoted 1 line · Rupjyoti Sarmah

This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.

Doesn't this DWC sata IP provide AHCI ? Just curious.

#4 lore raw
In reply to Rupjyoti Sarmah

Dne Pá 4. června 2010 14:26:17 Rupjyoti Sarmah napsal(a):

Quoted 7 lines

This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.

--SNIP--

drivers/ata/sata_dwc_460ex.c line 1455

Quoted 3 lines
drivers/ata/sata_dwc_460ex.c new file +3 -0
+		dev_err(ap->dev, "%s: Command not pending cmd_issued=%d "
+			"(tag=%d) DMA NOT started\n", __func__,
+			hsdevp->cmd_issued[tag], tag);

Just a nitpick, but don't break strings if possible.

#5 lore raw

Hi,

Thanks for your feedback. I submitted an updated version of this patch later.
I tried to limit the lines within 80 characters.

Regards,
Rup

-----Original Message-----
From: Marek Vasut [mailto:[email protected]]
Sent: Tuesday, June 29, 2010 7:04 AM
To: Rupjyoti Sarmah
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH]460EX on-chip SATA driver<kernel 2.6.33><resubmission>

Dne Pá 4. června 2010 14:26:17 Rupjyoti Sarmah napsal(a):

Quoted 7 lines

This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.

--SNIP--

drivers/ata/sata_dwc_460ex.c line 1455

Quoted 3 lines
drivers/ata/sata_dwc_460ex.c new file +3 -0
+		dev_err(ap->dev, "%s: Command not pending cmd_issued=%d "
+			"(tag=%d) DMA NOT started\n", __func__,
+			hsdevp->cmd_issued[tag], tag);

Just a nitpick, but don't break strings if possible.
_______________________________________________
Linuxppc-dev mailing list [email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

#6 lore raw
In reply to Marek Vasut
Quoted 9 lines · Marek Vasut

Dne Pá 4. června 2010 14:26:17 Rupjyoti Sarmah napsal(a):

Quoted 7 lines

This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.

Quoted 7 lines

--SNIP--

drivers/ata/sata_dwc_460ex.c line 1455

Quoted 3 lines
drivers/ata/sata_dwc_460ex.c new file +3 -0
+		dev_err(ap->dev, "%s: Command not pending cmd_issued=%d "
+			"(tag=%d) DMA NOT started\n", __func__,
+			hsdevp->cmd_issued[tag], tag);

Just a nitpick, but don't break strings if possible.

Note that in this particular case breaking strings was OK, and was actually done in a clever manner.

That is, I doubt that anyone grep for error messages with regexps, so grep "Command not pending" or grep "DMA NOT started" would work.

This makes the code more readable (literally, because you don't have to scroll to see dev_err's arguments) without sacrificing grep'ability.

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2