From: Josh Boyer on
On Fri, Jun 04, 2010 at 05:56:17PM +0530, Rupjyoti Sarmah wrote:
>This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.
>
>Signed-off-by: Rupjyoti Sarmah <rsarmah(a)appliedmicro.com>
>Signed-off-by: Mark Miesfeld <mmiesfeld(a)appliedmicro.com>
>Signed-off-by: Prodyut Hazarika <phazarika(a)appliedmicro.com>

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

>
>---
> 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
>
>diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
>index 56c6374..bba7b8a 100644
>--- a/drivers/ata/Kconfig
>+++ b/drivers/ata/Kconfig
>@@ -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
>diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
>index fc936d4..96ff315 100644
>--- a/drivers/ata/Makefile
>+++ b/drivers/ata/Makefile
>@@ -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
>diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
>new file mode 100644
>index 0000000..e6e2896
>--- /dev/null
>+++ b/drivers/ata/sata_dwc_460ex.c
>@@ -0,0 +1,1808 @@
>+/*
>+ * drivers/ata/sata_dwc_460ex.c
>+ *
>+ * Synopsys DesignWare Cores (DWC) SATA host driver
>+ *
>+ * Author: Mark Miesfeld <mmiesfeld(a)amcc.com>
>+ *
>+ * Ported from 2.6.19.2 to 2.6.25/26 by Stefan Roese <sr(a)denx.de>
>+ * 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.

>+/******************************************************************************
>+ * 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Wolfgang Denk on
Dear Rupjyoti Sarmah,

In message <201006041226.o54CQH2V017366(a)amcc.com> you wrote:
> This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.
>
> Signed-off-by: Rupjyoti Sarmah <rsarmah(a)appliedmicro.com>
> Signed-off-by: Mark Miesfeld <mmiesfeld(a)appliedmicro.com>
> Signed-off-by: Prodyut Hazarika <phazarika(a)appliedmicro.com>
>
> ---
> 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.

....
> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> new file mode 100644
> index 0000000..e6e2896
> --- /dev/null
> +++ b/drivers/ata/sata_dwc_460ex.c
> @@ -0,0 +1,1808 @@
....
> +#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.

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

--
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(a)denx.de
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_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jassi Brar on
On Fri, Jun 4, 2010 at 9:26 PM, Rupjyoti Sarmah <rsarmah(a)amcc.com> wrote:
> This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.

Doesn't this DWC sata IP provide AHCI ? Just curious.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Marek Vasut on
Dne Pá 4. června 2010 14:26:17 Rupjyoti Sarmah napsal(a):
> This patch enables the on-chip DWC SATA controller of the AppliedMicro
> processor 460EX.
>
> Signed-off-by: Rupjyoti Sarmah <rsarmah(a)appliedmicro.com>
> Signed-off-by: Mark Miesfeld <mmiesfeld(a)appliedmicro.com>
> Signed-off-by: Prodyut Hazarika <phazarika(a)appliedmicro.com>
>

--SNIP--

> + 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Rupjyoti Sarmah on
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:marek.vasut(a)gmail.com]
Sent: Tuesday, June 29, 2010 7:04 AM
To: Rupjyoti Sarmah
Cc: linux-ide(a)vger.kernel.org; linux-kernel(a)vger.kernel.org;
jgarzik(a)pobox.com; sr(a)denx.de; linuxppc-dev(a)ozlabs.org
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):
> This patch enables the on-chip DWC SATA controller of the AppliedMicro
> processor 460EX.
>
> Signed-off-by: Rupjyoti Sarmah <rsarmah(a)appliedmicro.com>
> Signed-off-by: Mark Miesfeld <mmiesfeld(a)appliedmicro.com>
> Signed-off-by: Prodyut Hazarika <phazarika(a)appliedmicro.com>
>

--SNIP--

> + 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/