headers
Mark Lord | 1 Apr 2008 01:27
Picon
Favicon

[PATCH 0/5] sata_mv cleanups

Here are five new patches for sata_mv.
These serve to clean things up in preparation
for the PMP patches which will follow.

Hopefully nothing controversial here.

01_sata_mv_cosmetics.patch
02_sata_mv_clean_up_stop_edma.patch
03_sata_mv_fix_ifcfg_handling.patch
04_sata_mv_new_hardreset_handler.patch
05_sata_mv_remove_phy_reset_and_postreset.patch

I generated these against Jeff's upstream branch as of today.

Please let me know ASAP of any issues, as there's a bunch of
PMP patches for sata_mv hot on the heels of these ones.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 29 comments |
headers
Mark Lord | 1 Apr 2008 01:33
Picon
Favicon

[PATCH 1/5] sata_mv cosmetic fixes

Various cosmetic fixes in preparation for real code changes later on.

Signed-off-by: Mark Lord <mlord <at> pobox.com>
---

--- old/drivers/ata/sata_mv.c	2008-03-31 16:09:08.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-03-31 17:18:15.000000000 -0400
 <at>  <at>  -1,6 +1,7  <at>  <at> 
 /*
  * sata_mv.c - Marvell SATA support
  *
+ * Copyright 2008: Marvell Corporation, all rights reserved.
  * Copyright 2005: EMC Corporation, all rights reserved.
  * Copyright 2005 Red Hat, Inc.  All rights reserved.
  *
 <at>  <at>  -61,7 +62,6  <at>  <at> 

 */

-
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 <at>  <at>  -131,7 +131,7  <at>  <at> 
 	MV_FLAG_DUAL_HC		= (1 << 30),  /* two SATA Host Controllers */
 	MV_FLAG_IRQ_COALESCE	= (1 << 29),  /* IRQ coalescing capability */
 	/* SoC integrated controllers, no PCI interface */
-	MV_FLAG_SOC = (1 << 28),
+	MV_FLAG_SOC		= (1 << 28),
(Continue reading)

Permalink | Reply | 1 comment |
headers
Mark Lord | 1 Apr 2008 01:34
Picon
Favicon

[PATCH 2/5] sata_mv clean up mv_stop_edma usage

Clean up uses of mv_stop_edma{_engine}() to match datasheet requirements.

Signed-off-by: Mark Lord <mlord <at> pobox.com>
---

--- old/drivers/ata/sata_mv.c	2008-03-31 17:18:15.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-03-31 18:12:56.000000000 -0400
 <at>  <at>  -517,7 +517,7  <at>  <at> 
 static void mv_reset_channel(struct mv_host_priv *hpriv, void __iomem *mmio,
 			     unsigned int port_no);
 static int mv_stop_edma(struct ata_port *ap);
-static int mv_stop_edma_engine(struct ata_port *ap);
+static int mv_stop_edma_engine(void __iomem *port_mmio);
 static void mv_edma_cfg(struct ata_port *ap, int want_ncq);

 /* .sg_tablesize is (MV_MAX_SG_CT / 2) in the structures below
 <at>  <at>  -805,7 +805,7  <at>  <at> 
 	if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
 		int using_ncq = ((pp->pp_flags & MV_PP_FLAG_NCQ_EN) != 0);
 		if (want_ncq != using_ncq)
-			mv_stop_edma_engine(ap);
+			mv_stop_edma(ap);
 	}
 	if (!(pp->pp_flags & MV_PP_FLAG_EDMA_EN)) {
 		struct mv_host_priv *hpriv = ap->host->private_data;
 <at>  <at>  -841,57 +841,41  <at>  <at> 

 /**
  *      mv_stop_edma_engine - Disable eDMA engine
- *       <at> ap: ATA channel to manipulate
(Continue reading)

Permalink | Reply | 6 comments |
headers
Mark Lord | 1 Apr 2008 01:35
Picon
Favicon

[PATCH 3/5] sata_mv fix ifctl handling

Fix handling of the SATA_INTERFACE_CFG register to match datasheet requirements.

Signed-off-by: Mark Lord <mlord <at> pobox.com>
---

--- old/drivers/ata/sata_mv.c	2008-03-31 18:12:56.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-03-31 18:14:40.000000000 -0400
 <at>  <at>  -2242,6 +2242,16  <at>  <at> 
 	return;
 }

+static void mv_setup_ifctl(void __iomem *port_mmio, int want_gen2i)
+{
+	u32 ifctl = readl(port_mmio + SATA_INTERFACE_CFG);
+
+	ifctl = (ifctl & 0xf7f) | 0x9b1000;	/* from chip spec */
+	if (want_gen2i)
+		ifctl |= (1 << 7);		/* enable gen2i speed */
+	writelfl(ifctl, port_mmio + SATA_INTERFACE_CFG);
+}
+
 /*
  * Caller must ensure that EDMA is not active,
  * by first doing mv_stop_edma() where needed.
 <at>  <at>  -2253,18 +2263,17  <at>  <at> 

 	writelfl(ATA_RST, port_mmio + EDMA_CMD_OFS);

-	if (IS_GEN_II(hpriv)) {
-		u32 ifctl = readl(port_mmio + SATA_INTERFACE_CFG);
(Continue reading)

Permalink | Reply | 0 comments |
headers
Mark Lord | 1 Apr 2008 01:35
Picon
Favicon

[PATCH 4/5] sata_mv new mv_sata_hardreset handler

Introduce mv_sata_hardreset() to perform a link hard reset
while dealing with certain chipset errata during the reset.

Also beef up error-handling in mv_prereset() to trigger a hard reset
whenever mv_stop_edma() fails.

Signed-off-by: Mark Lord <mlord <at> pobox.com>
---

--- old/drivers/ata/sata_mv.c	2008-03-31 18:14:40.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-03-31 18:24:50.000000000 -0400
 <at>  <at>  -2388,7 +2388,97  <at>  <at> 

 static int mv_prereset(struct ata_link *link, unsigned long deadline)
 {
-	mv_stop_edma(link->ap);
+	if (mv_stop_edma(link->ap))
+		link->eh_context.i.action |= ATA_EH_HARDRESET;
+	return ata_std_prereset(link, deadline);
+}
+
+/**
+ *	mv_sata_hardreset - reset host port via SATA phy reset
+ *	 <at> link: link to reset
+ *	 <at> class: resulting class of attached device
+ *	 <at> deadline: deadline jiffies for the operation
+ *
+ *	SATA phy-reset host port using DET bits of SControl register,
+ *	wait for !BSY and classify the attached device.
+ *
(Continue reading)

Permalink | Reply | 13 comments |
headers
Mark Lord | 1 Apr 2008 01:36
Picon
Favicon

[PATCH 5/5] sata_mv remove mv_phy_reset and mv_postreset

Remove the now unused mv_phy_reset() code,
as well as the unnecessary mv_postreset() function.

Signed-off-by: Mark Lord <mlord <at> pobox.com>
---

--- old/drivers/ata/sata_mv.c	2008-03-31 18:46:44.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-03-31 18:46:49.000000000 -0400
 <at>  <at>  -481,7 +481,6  <at>  <at> 
 static int mv_prereset(struct ata_link *link, unsigned long deadline);
 static int mv_hardreset(struct ata_link *link, unsigned int *class,
 			unsigned long deadline);
-static void mv_postreset(struct ata_link *link, unsigned int *classes);
 static void mv_eh_freeze(struct ata_port *ap);
 static void mv_eh_thaw(struct ata_port *ap);
 static void mv6_dev_config(struct ata_device *dev);
 <at>  <at>  -547,7 +546,6  <at>  <at> 
 	.thaw			= mv_eh_thaw,
 	.prereset		= mv_prereset,
 	.hardreset		= mv_hardreset,
-	.postreset		= mv_postreset,
 	.error_handler		= ata_std_error_handler, /* avoid SFF EH */
 	.post_internal_cmd	= ATA_OP_NULL,

 <at>  <at>  -2282,110 +2280,6  <at>  <at> 
 		mdelay(1);
 }

-/**
- *      mv_phy_reset - Perform eDMA reset followed by COMRESET
(Continue reading)

Permalink | Reply | 4 comments |
headers
Tejun Heo | 1 Apr 2008 02:26
Picon

Re: Need help understanding SATA error message.

Mark Lord wrote:
>> The response to an unrecoverable sector shouldn't be 51/04 if the
>> flush fails, it should be 51/10 or 51/40.
>>
>> 51/04 would be the response if the FLUSH CACHE command was issued when
>> there were still outstanding NCQ commands active.
> ..
> 
> Tejun:  I see we have another thread as well with FLUSH errors.
> I really doubt that these are bad drives.
> There's very likely a bug in libata / LLD there someplace.

Possibly.  The only thing I can think of which can screw FLUSH is 
issuing it when NCQ phase is still in progress as was in the case for 
ADMA.  FLUSH being a non-data command, it's pretty difficult to get it 
wrong otherwise.  The thing is that sata_sil24 does its own command 
sequencing and even if libata slips there a bit, the silicon won't issue 
FLUSH if NCQ is in progress, so I'm a bit skeptical.  Any other ideas?

--

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 7 comments |
headers
Mark Lord | 1 Apr 2008 04:01
Picon
Favicon

Re: Need help understanding SATA error message.

Tejun Heo wrote:
> Mark Lord wrote:
>>> The response to an unrecoverable sector shouldn't be 51/04 if the
>>> flush fails, it should be 51/10 or 51/40.
>>>
>>> 51/04 would be the response if the FLUSH CACHE command was issued when
>>> there were still outstanding NCQ commands active.
>> ..
>>
>> Tejun:  I see we have another thread as well with FLUSH errors.
>> I really doubt that these are bad drives.
>> There's very likely a bug in libata / LLD there someplace.
> 
> Possibly.  The only thing I can think of which can screw FLUSH is 
> issuing it when NCQ phase is still in progress as was in the case for 
> ADMA.  FLUSH being a non-data command, it's pretty difficult to get it 
> wrong otherwise.  The thing is that sata_sil24 does its own command 
> sequencing and even if libata slips there a bit, the silicon won't issue 
> FLUSH if NCQ is in progress, so I'm a bit skeptical.  Any other ideas?
..

Mmm.. the one Tomas Lund has is on what appears to be AHCI (ICH9R).

Tomas, if you move the "problem drive" to another port, does the error
follow the drive, or stay with the same port?

(Hopefully you can try that)

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
(Continue reading)

Permalink | Reply | 6 comments |
headers
Kumar Gala | 1 Apr 2008 04:15

Re: [PATCH 3/3] sata_fsl: reduce compatibility to fsl,pq-sata


On Mar 28, 2008, at 10:51 AM, Kim Phillips wrote:
> as prescribed in Documentation/powerpc/booting-without-of.txt.
>
> Signed-off-by: Kim Phillips <kim.phillips <at> freescale.com>
> Cc: Jeff Garzik <jgarzik <at> pobox.com>
> ---
> arch/powerpc/boot/dts/mpc8377_mds.dts |    4 ++--
> arch/powerpc/boot/dts/mpc8379_mds.dts |    8 ++++----
> drivers/ata/sata_fsl.c                |    5 +----
> 3 files changed, 7 insertions(+), 10 deletions(-)

applied.

- k
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |
headers
Robert Hancock | 1 Apr 2008 05:14
Picon
Favicon

Re: [patch 3/3] ata: SWNCQ should be enabled by default

Tejun Heo wrote:
> Kuan Luo wrote:
>> For ADMA, I lean to disable it for default because stability is more
>> important to our customers. At least, bug in this link:
>> http://www.gossamer-threads.com/lists/linux/kernel/885929  is not
>> resolved.
> 
> Robert, what do you think?  Should we disable ADMA by default from 
> 2.6.26-rcX?
> 

I can't say I'm inherently in favor of it, but if people feel strongly 
enough about it I won't yell too loudly. If we do disable it, I would 
suggest printing out a message saying it can be enabled when the driver 
loads if they didn't switch it on, in case people complain about it as a 
regression otherwise.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 7 comments |

Gmane