diff mbox

[V2,RESEND] fsl-sata: I/O load balancing

Message ID 1329284944-17943-1-git-send-email-qiang.liu@freescale.com
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Qiang Liu Feb. 15, 2012, 5:49 a.m. UTC
Reduce interrupt signals through reset Interrupt Coalescing Control Reg.
Provide dynamic method to adjust interrupt signals and timer ticks by sysfs.
It is a tradeoff for different applications.

Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
---
change for V2
	support dynamic config interrupt coalescing register by /sysfs
	test random small file with iometer
	adjust kernel source baseline for apply upstream
Description:
  1. fsl-sata interrupt will be raised 130 thousand times when write 8G file
    (dd if=/dev/zero of=/dev/sda2 bs=128K count=65536);
  2. most of interrupts raised because of only 1-4 commands completed;
  3. only 30 thousand times will be raised after set max interrupt threshold,
    more interrupts are coalesced as the description of ICC;
Test methods and results:
  1. test sequential large file performance,
  [root@p2020ds root]# echo 31 524287 > \
	/sys/devices/soc.0/ffe18000.sata/intr_coalescing
  [root@p2020ds root]# dd if=/dev/zero of=/dev/sda2 bs=128K count=65536 &
  [root@p2020ds root]# top

   CPU %  |  dd   |  flush-8:0 | softirq
   ---------------------------------------
   before | 20-22 |    17-19   |    7
   ---------------------------------------
   after  | 18-21 |    15-16   |    5
   ---------------------------------------
   2. test random small file with iometer,
   iometer paramters:
   4 I/Os burst length, 1MB transfer request size, 100% write, 2MB file size
     as default configuration of interrupt coalescing register, 1 interrupts and
   no timeout config, total write performance is 119MB per second,
     after config with the maximum value, write performance is 110MB per second.

   After compare the test results, a configuable interrupt coalescing should be
   better when cope with flexible context.


 drivers/ata/sata_fsl.c |  111 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 107 insertions(+), 4 deletions(-)

--
1.6.4


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Yang Li Feb. 15, 2012, 7:22 a.m. UTC | #1
On Wed, Feb 15, 2012 at 1:49 PM, Qiang Liu <qiang.liu@freescale.com> wrote:

Hi Liu Qiang,

The patch is fine except for the title and comment.  It's too vague to
say I/O load balancing.  You need explicit description like "add
interrupt coalescing support" as title.

> Reduce interrupt signals through reset Interrupt Coalescing Control Reg.
> Provide dynamic method to adjust interrupt signals and timer ticks by sysfs.
> It is a tradeoff for different applications.

How about:

Adds support for interrupt coalescing feature to reduce interrupt
events.  Provides mechanism of adjusting coalescing count and timeout
tick by sysfs on runtime, so that tradeoff of latency and CPU load can
be made depending on applications.

- Leo
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Qiang-B32616 Feb. 15, 2012, 7:29 a.m. UTC | #2
> -----Original Message-----

> From: linux-ide-owner@vger.kernel.org [mailto:linux-ide-

> owner@vger.kernel.org] On Behalf Of Li Yang

> Sent: Wednesday, February 15, 2012 3:22 PM

> To: Liu Qiang-B32616

> Cc: jgarzik@pobox.com; linux-ide@vger.kernel.org; linuxppc-

> dev@lists.ozlabs.org; linux-kernel@vger.kernel.org

> Subject: Re: [PATCH V2 RESEND] fsl-sata: I/O load balancing

> 

> On Wed, Feb 15, 2012 at 1:49 PM, Qiang Liu <qiang.liu@freescale.com>

> wrote:

> 

> Hi Liu Qiang,

> 

> The patch is fine except for the title and comment.  It's too vague to

> say I/O load balancing.  You need explicit description like "add

> interrupt coalescing support" as title.

> 

> > Reduce interrupt signals through reset Interrupt Coalescing Control Reg.

> > Provide dynamic method to adjust interrupt signals and timer ticks by

> sysfs.

> > It is a tradeoff for different applications.

> 

> How about:

> 

> Adds support for interrupt coalescing feature to reduce interrupt events.

> Provides mechanism of adjusting coalescing count and timeout tick by

> sysfs on runtime, so that tradeoff of latency and CPU load can be made

> depending on applications.

> 

Ok, I will change the title and comments more specifically.

> - Leo

> --

> To unsubscribe from this list: send the line "unsubscribe linux-ide" in

> the body of a message to majordomo@vger.kernel.org More majordomo info at

> http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt Feb. 17, 2012, 12:40 a.m. UTC | #3
On Wed, 2012-02-15 at 13:49 +0800, Qiang Liu wrote:
]
> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
> index 0120b0d..d6577b9 100644
> --- a/drivers/ata/sata_fsl.c
> +++ b/drivers/ata/sata_fsl.c
> @@ -6,7 +6,7 @@
>   * Author: Ashish Kalra <ashish.kalra@freescale.com>
>   * Li Yang <leoli@freescale.com>
>   *
> - * Copyright (c) 2006-2007, 2011 Freescale Semiconductor, Inc.
> + * Copyright (c) 2006-2007, 2011-2012 Freescale Semiconductor, Inc.
>   *
>   * 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
> @@ -26,6 +26,15 @@
>  #include <asm/io.h>
>  #include <linux/of_platform.h>
> 
> +static unsigned int intr_coalescing_count;
> +module_param(intr_coalescing_count, int, S_IRUGO);
> +MODULE_PARM_DESC(intr_coalescing_count,
> +				 "INT coalescing count threshold (1..31)");
> +
> +static unsigned int intr_coalescing_ticks;
> +module_param(intr_coalescing_ticks, int, S_IRUGO);
> +MODULE_PARM_DESC(intr_coalescing_ticks,
> +				 "INT coalescing timer threshold in AHB ticks");

You don't seem to provide very useful defaults... for example,
intr_coalescing_count starts at 0 which isn't even in range
(the code fixes that up but still...).

I tried a debian install on the p5020ds here and I find SATA to be
extremely slow, generating millions of interrupts. I think the defaults
should be a lot more aggressive at coalescing.

Cheers,
Ben.

>  /* Controller information */
>  enum {
>  	SATA_FSL_QUEUE_DEPTH	= 16,
> @@ -83,6 +92,16 @@ enum {
>  };
> 
>  /*
> + * Interrupt Coalescing Control Register bitdefs  */
> +enum {
> +	ICC_MIN_INT_COUNT_THRESHOLD	= 1,
> +	ICC_MAX_INT_COUNT_THRESHOLD	= ((1 << 5) - 1),
> +	ICC_MIN_INT_TICKS_THRESHOLD	= 0,
> +	ICC_MAX_INT_TICKS_THRESHOLD	= ((1 << 19) - 1),
> +	ICC_SAFE_INT_TICKS		= 1,
> +};
> +
> +/*
>  * Host Controller command register set - per port
>  */
>  enum {
> @@ -263,8 +282,65 @@ struct sata_fsl_host_priv {
>  	void __iomem *csr_base;
>  	int irq;
>  	int data_snoop;
> +	struct device_attribute intr_coalescing;
>  };
> 
> +static void fsl_sata_set_irq_coalescing(struct ata_host *host,
> +		unsigned int count, unsigned int ticks)
> +{
> +	struct sata_fsl_host_priv *host_priv = host->private_data;
> +	void __iomem *hcr_base = host_priv->hcr_base;
> +
> +	if (count > ICC_MAX_INT_COUNT_THRESHOLD)
> +		count = ICC_MAX_INT_COUNT_THRESHOLD;
> +	else if (count < ICC_MIN_INT_COUNT_THRESHOLD)
> +		count = ICC_MIN_INT_COUNT_THRESHOLD;
> +
> +	if (ticks > ICC_MAX_INT_TICKS_THRESHOLD)
> +		ticks = ICC_MAX_INT_TICKS_THRESHOLD;
> +	else if ((ICC_MIN_INT_TICKS_THRESHOLD == ticks) &&
> +			(count > ICC_MIN_INT_COUNT_THRESHOLD))
> +		ticks = ICC_SAFE_INT_TICKS;
> +
> +	spin_lock(&host->lock);
> +	iowrite32((count << 24 | ticks), hcr_base + ICC);
> +
> +	intr_coalescing_count = count;
> +	intr_coalescing_ticks = ticks;
> +	spin_unlock(&host->lock);
> +
> +	DPRINTK("intrrupt coalescing, count = 0x%x, ticks = %x\n",
> +			intr_coalescing_count, intr_coalescing_ticks);
> +	DPRINTK("ICC register status: (hcr base: 0x%x) = 0x%x\n",
> +			hcr_base, ioread32(hcr_base + ICC));
> +}
> +
> +static ssize_t fsl_sata_intr_coalescing_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "%d	%d\n",
> +			intr_coalescing_count, intr_coalescing_ticks);
> +}
> +
> +static ssize_t fsl_sata_intr_coalescing_store(struct device *dev,
> +		struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	unsigned int coalescing_count,	coalescing_ticks;
> +
> +	if (sscanf(buf, "%d%d",
> +				&coalescing_count,
> +				&coalescing_ticks) != 2) {
> +		printk(KERN_ERR "fsl-sata: wrong parameter format.\n");
> +		return -EINVAL;
> +	}
> +
> +	fsl_sata_set_irq_coalescing(dev_get_drvdata(dev),
> +			coalescing_count, coalescing_ticks);
> +
> +	return strlen(buf);
> +}
> +
>  static inline unsigned int sata_fsl_tag(unsigned int tag,
>  					void __iomem *hcr_base)
>  {
> @@ -346,10 +422,10 @@ static unsigned int sata_fsl_fill_sg(struct ata_queued_cmd *qc, void *cmd_desc,
>  			(unsigned long long)sg_addr, sg_len);
> 
>  		/* warn if each s/g element is not dword aligned */
> -		if (sg_addr & 0x03)
> +		if (unlikely(sg_addr & 0x03))
>  			ata_port_err(qc->ap, "s/g addr unaligned : 0x%llx\n",
>  				     (unsigned long long)sg_addr);
> -		if (sg_len & 0x03)
> +		if (unlikely(sg_len & 0x03))
>  			ata_port_err(qc->ap, "s/g len unaligned : 0x%x\n",
>  				     sg_len);
> 
> @@ -1245,6 +1321,13 @@ static int sata_fsl_init_controller(struct ata_host *host)
>  	iowrite32(0x00000FFFF, hcr_base + CE);
>  	iowrite32(0x00000FFFF, hcr_base + DE);
> 
> + 	/*
> +	 * reset the number of command complete bits which will cause the
> +	 * interrupt to be signaled
> +	 */
> +	fsl_sata_set_irq_coalescing(host, intr_coalescing_count,
> +			intr_coalescing_ticks);
> +
>  	/*
>  	 * host controller will be brought on-line, during xx_port_start()
>  	 * callback, that should also initiate the OOB, COMINIT sequence
> @@ -1309,7 +1392,7 @@ static int sata_fsl_probe(struct platform_device *ofdev)
>  	void __iomem *csr_base = NULL;
>  	struct sata_fsl_host_priv *host_priv = NULL;
>  	int irq;
> -	struct ata_host *host;
> +	struct ata_host *host = NULL;
>  	u32 temp;
> 
>  	struct ata_port_info pi = sata_fsl_port_info[0];
> @@ -1356,6 +1439,10 @@ static int sata_fsl_probe(struct platform_device *ofdev)
> 
>  	/* allocate host structure */
>  	host = ata_host_alloc_pinfo(&ofdev->dev, ppi, SATA_FSL_MAX_PORTS);
> +	if (!host) {
> +		retval = -ENOMEM;
> +		goto error_exit_with_cleanup;
> +	}
> 
>  	/* host->iomap is not used currently */
>  	host->private_data = host_priv;
> @@ -1373,10 +1460,24 @@ static int sata_fsl_probe(struct platform_device *ofdev)
> 
>  	dev_set_drvdata(&ofdev->dev, host);
> 
> +	host_priv->intr_coalescing.show = fsl_sata_intr_coalescing_show;
> +	host_priv->intr_coalescing.store = fsl_sata_intr_coalescing_store;
> +	sysfs_attr_init(&host_priv->intr_coalescing.attr);
> +	host_priv->intr_coalescing.attr.name = "intr_coalescing";
> +	host_priv->intr_coalescing.attr.mode = S_IRUGO | S_IWUSR;
> +	retval = device_create_file(host->dev, &host_priv->intr_coalescing);
> +	if (retval)
> +		goto error_exit_with_cleanup;
> +
>  	return 0;
> 
>  error_exit_with_cleanup:
> 
> +	if (host) {
> +		dev_set_drvdata(&ofdev->dev, NULL);
> +		ata_host_detach(host);
> +	}
> +
>  	if (hcr_base)
>  		iounmap(hcr_base);
>  	if (host_priv)
> @@ -1390,6 +1491,8 @@ static int sata_fsl_remove(struct platform_device *ofdev)
>  	struct ata_host *host = dev_get_drvdata(&ofdev->dev);
>  	struct sata_fsl_host_priv *host_priv = host->private_data;
> 
> +	device_remove_file(&ofdev->dev, &host_priv->intr_coalescing);
> +
>  	ata_host_detach(host);
> 
>  	dev_set_drvdata(&ofdev->dev, NULL);
> --
> 1.6.4
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Qiang-B32616 Feb. 17, 2012, 1:54 a.m. UTC | #4
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBCZW5qYW1pbiBIZXJyZW5zY2ht
aWR0IFttYWlsdG86YmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnXQ0KPiBTZW50OiBGcmlkYXksIEZl
YnJ1YXJ5IDE3LCAyMDEyIDg6NDAgQU0NCj4gVG86IExpdSBRaWFuZy1CMzI2MTYNCj4gQ2M6IGpn
YXJ6aWtAcG9ib3guY29tOyBsaW51eC1pZGVAdmdlci5rZXJuZWwub3JnOyBsaW51eHBwYy0NCj4g
ZGV2QGxpc3RzLm96bGFicy5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmcNCj4gU3Vi
amVjdDogUmU6IFtQQVRDSCBWMiBSRVNFTkRdIGZzbC1zYXRhOiBJL08gbG9hZCBiYWxhbmNpbmcN
Cj4gDQo+IE9uIFdlZCwgMjAxMi0wMi0xNSBhdCAxMzo0OSArMDgwMCwgUWlhbmcgTGl1IHdyb3Rl
Og0KPiBdDQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvYXRhL3NhdGFfZnNsLmMgYi9kcml2ZXJz
L2F0YS9zYXRhX2ZzbC5jIGluZGV4DQo+ID4gMDEyMGIwZC4uZDY1NzdiOSAxMDA2NDQNCj4gPiAt
LS0gYS9kcml2ZXJzL2F0YS9zYXRhX2ZzbC5jDQo+ID4gKysrIGIvZHJpdmVycy9hdGEvc2F0YV9m
c2wuYw0KPiA+IEBAIC02LDcgKzYsNyBAQA0KPiA+ICAgKiBBdXRob3I6IEFzaGlzaCBLYWxyYSA8
YXNoaXNoLmthbHJhQGZyZWVzY2FsZS5jb20+DQo+ID4gICAqIExpIFlhbmcgPGxlb2xpQGZyZWVz
Y2FsZS5jb20+DQo+ID4gICAqDQo+ID4gLSAqIENvcHlyaWdodCAoYykgMjAwNi0yMDA3LCAyMDEx
IEZyZWVzY2FsZSBTZW1pY29uZHVjdG9yLCBJbmMuDQo+ID4gKyAqIENvcHlyaWdodCAoYykgMjAw
Ni0yMDA3LCAyMDExLTIwMTIgRnJlZXNjYWxlIFNlbWljb25kdWN0b3IsIEluYy4NCj4gPiAgICoN
Cj4gPiAgICogVGhpcyBwcm9ncmFtIGlzIGZyZWUgc29mdHdhcmU7IHlvdSBjYW4gcmVkaXN0cmli
dXRlICBpdCBhbmQvb3INCj4gbW9kaWZ5IGl0DQo+ID4gICAqIHVuZGVyICB0aGUgdGVybXMgb2Yg
IHRoZSBHTlUgR2VuZXJhbCAgUHVibGljIExpY2Vuc2UgYXMgcHVibGlzaGVkDQo+ID4gYnkgdGhl
IEBAIC0yNiw2ICsyNiwxNSBAQCAgI2luY2x1ZGUgPGFzbS9pby5oPiAgI2luY2x1ZGUNCj4gPiA8
bGludXgvb2ZfcGxhdGZvcm0uaD4NCj4gPg0KPiA+ICtzdGF0aWMgdW5zaWduZWQgaW50IGludHJf
Y29hbGVzY2luZ19jb3VudDsNCj4gPiArbW9kdWxlX3BhcmFtKGludHJfY29hbGVzY2luZ19jb3Vu
dCwgaW50LCBTX0lSVUdPKTsNCj4gPiArTU9EVUxFX1BBUk1fREVTQyhpbnRyX2NvYWxlc2Npbmdf
Y291bnQsDQo+ID4gKwkJCQkgIklOVCBjb2FsZXNjaW5nIGNvdW50IHRocmVzaG9sZCAoMS4uMzEp
Iik7DQo+ID4gKw0KPiA+ICtzdGF0aWMgdW5zaWduZWQgaW50IGludHJfY29hbGVzY2luZ190aWNr
czsNCj4gPiArbW9kdWxlX3BhcmFtKGludHJfY29hbGVzY2luZ190aWNrcywgaW50LCBTX0lSVUdP
KTsNCj4gPiArTU9EVUxFX1BBUk1fREVTQyhpbnRyX2NvYWxlc2NpbmdfdGlja3MsDQo+ID4gKwkJ
CQkgIklOVCBjb2FsZXNjaW5nIHRpbWVyIHRocmVzaG9sZCBpbiBBSEIgdGlja3MiKTsNCj4gDQo+
IFlvdSBkb24ndCBzZWVtIHRvIHByb3ZpZGUgdmVyeSB1c2VmdWwgZGVmYXVsdHMuLi4gZm9yIGV4
YW1wbGUsDQo+IGludHJfY29hbGVzY2luZ19jb3VudCBzdGFydHMgYXQgMCB3aGljaCBpc24ndCBl
dmVuIGluIHJhbmdlICh0aGUgY29kZQ0KPiBmaXhlcyB0aGF0IHVwIGJ1dCBzdGlsbC4uLikuDQo+
IA0KPiBJIHRyaWVkIGEgZGViaWFuIGluc3RhbGwgb24gdGhlIHA1MDIwZHMgaGVyZSBhbmQgSSBm
aW5kIFNBVEEgdG8gYmUNCj4gZXh0cmVtZWx5IHNsb3csIGdlbmVyYXRpbmcgbWlsbGlvbnMgb2Yg
aW50ZXJydXB0cy4gSSB0aGluayB0aGUgZGVmYXVsdHMNCj4gc2hvdWxkIGJlIGEgbG90IG1vcmUg
YWdncmVzc2l2ZSBhdCBjb2FsZXNjaW5nLg0KDQpIZWxsbyBCZW4sDQoNClRoZSBkZWZhdWx0IHdp
bGwgYmUgc2V0IGluIGEgY29tbW9uIGludGVyZmFjZSBmc2xfc2F0YV9zZXRfaXJxX2NvYWxlc2Np
bmcgd2hlbg0KaW5pdGlhbGl6ZSB0aGUgY29udHJvbGxlci4gVGhpcyBpbnRlcmZhY2Ugd2lsbCBj
aGVjayB0aGUgcmFuZ2Ugb2YgaW50ciBjb3VudA0KYW5kIHRpY2tzIGFuZCBtYWtlIHN1cmUgdGhl
IHZhbHVlcyBhcmUgcmVhc29uYWJseS4NCg0KSXQncyBoYXJkIHRvIGZpbmQgYSBhZ2dyZXNzaXZl
IHZhbHVlIHRvIGFkYXB0IGFsbCBzY2VuYXJpb3MsIHNvIEkgdXNlIGVjaG8gdG8gYWRqdXN0DQp0
aGUgdmFsdWUuIEkgcmVtZW1iZXIgUDUwMjAgaGF2ZSBzb21lIHBlcmZvcm1hbmNlIGlzc3VlLCBJ
IHdpbGwgY2hlY2sgaXQuDQpCVFcsIHdoaWNoIGZpbGVzeXN0ZW0gZG8geW91IHVzZT8gRXh0MiBp
cyBsb3dlciB0aGFuIGV4dDQgYmVjYXVzZSBtZXRhZGF0YSBpcyANCmNvbnRpbnVvdXNseSB3cm90
ZSB0byBkaXNrLiBZb3UgY2FuIHRyeSBleHQ0IG9yIHhmcy4NCg0KVGhhbmtzLg0KDQo+IA0KPiBD
aGVlcnMsDQo+IEJlbi4NCj4gDQo+ID4gIC8qIENvbnRyb2xsZXIgaW5mb3JtYXRpb24gKi8NCj4g
PiAgZW51bSB7DQo+ID4gIAlTQVRBX0ZTTF9RVUVVRV9ERVBUSAk9IDE2LA0KPiA+IEBAIC04Myw2
ICs5MiwxNiBAQCBlbnVtIHsNCj4gPiAgfTsNCj4gPg0KPiA+ICAvKg0KPiA+ICsgKiBJbnRlcnJ1
cHQgQ29hbGVzY2luZyBDb250cm9sIFJlZ2lzdGVyIGJpdGRlZnMgICovIGVudW0gew0KPiA+ICsJ
SUNDX01JTl9JTlRfQ09VTlRfVEhSRVNIT0xECT0gMSwNCj4gPiArCUlDQ19NQVhfSU5UX0NPVU5U
X1RIUkVTSE9MRAk9ICgoMSA8PCA1KSAtIDEpLA0KPiA+ICsJSUNDX01JTl9JTlRfVElDS1NfVEhS
RVNIT0xECT0gMCwNCj4gPiArCUlDQ19NQVhfSU5UX1RJQ0tTX1RIUkVTSE9MRAk9ICgoMSA8PCAx
OSkgLSAxKSwNCj4gPiArCUlDQ19TQUZFX0lOVF9USUNLUwkJPSAxLA0KPiA+ICt9Ow0KPiA+ICsN
Cj4gPiArLyoNCj4gPiAgKiBIb3N0IENvbnRyb2xsZXIgY29tbWFuZCByZWdpc3RlciBzZXQgLSBw
ZXIgcG9ydCAgKi8gIGVudW0geyBAQA0KPiA+IC0yNjMsOCArMjgyLDY1IEBAIHN0cnVjdCBzYXRh
X2ZzbF9ob3N0X3ByaXYgew0KPiA+ICAJdm9pZCBfX2lvbWVtICpjc3JfYmFzZTsNCj4gPiAgCWlu
dCBpcnE7DQo+ID4gIAlpbnQgZGF0YV9zbm9vcDsNCj4gPiArCXN0cnVjdCBkZXZpY2VfYXR0cmli
dXRlIGludHJfY29hbGVzY2luZzsNCj4gPiAgfTsNCj4gPg0KPiA+ICtzdGF0aWMgdm9pZCBmc2xf
c2F0YV9zZXRfaXJxX2NvYWxlc2Npbmcoc3RydWN0IGF0YV9ob3N0ICpob3N0LA0KPiA+ICsJCXVu
c2lnbmVkIGludCBjb3VudCwgdW5zaWduZWQgaW50IHRpY2tzKSB7DQo+ID4gKwlzdHJ1Y3Qgc2F0
YV9mc2xfaG9zdF9wcml2ICpob3N0X3ByaXYgPSBob3N0LT5wcml2YXRlX2RhdGE7DQo+ID4gKwl2
b2lkIF9faW9tZW0gKmhjcl9iYXNlID0gaG9zdF9wcml2LT5oY3JfYmFzZTsNCj4gPiArDQo+ID4g
KwlpZiAoY291bnQgPiBJQ0NfTUFYX0lOVF9DT1VOVF9USFJFU0hPTEQpDQo+ID4gKwkJY291bnQg
PSBJQ0NfTUFYX0lOVF9DT1VOVF9USFJFU0hPTEQ7DQo+ID4gKwllbHNlIGlmIChjb3VudCA8IElD
Q19NSU5fSU5UX0NPVU5UX1RIUkVTSE9MRCkNCj4gPiArCQljb3VudCA9IElDQ19NSU5fSU5UX0NP
VU5UX1RIUkVTSE9MRDsNCj4gPiArDQo+ID4gKwlpZiAodGlja3MgPiBJQ0NfTUFYX0lOVF9USUNL
U19USFJFU0hPTEQpDQo+ID4gKwkJdGlja3MgPSBJQ0NfTUFYX0lOVF9USUNLU19USFJFU0hPTEQ7
DQo+ID4gKwllbHNlIGlmICgoSUNDX01JTl9JTlRfVElDS1NfVEhSRVNIT0xEID09IHRpY2tzKSAm
Jg0KPiA+ICsJCQkoY291bnQgPiBJQ0NfTUlOX0lOVF9DT1VOVF9USFJFU0hPTEQpKQ0KPiA+ICsJ
CXRpY2tzID0gSUNDX1NBRkVfSU5UX1RJQ0tTOw0KPiA+ICsNCj4gPiArCXNwaW5fbG9jaygmaG9z
dC0+bG9jayk7DQo+ID4gKwlpb3dyaXRlMzIoKGNvdW50IDw8IDI0IHwgdGlja3MpLCBoY3JfYmFz
ZSArIElDQyk7DQo+ID4gKw0KPiA+ICsJaW50cl9jb2FsZXNjaW5nX2NvdW50ID0gY291bnQ7DQo+
ID4gKwlpbnRyX2NvYWxlc2NpbmdfdGlja3MgPSB0aWNrczsNCj4gPiArCXNwaW5fdW5sb2NrKCZo
b3N0LT5sb2NrKTsNCj4gPiArDQo+ID4gKwlEUFJJTlRLKCJpbnRycnVwdCBjb2FsZXNjaW5nLCBj
b3VudCA9IDB4JXgsIHRpY2tzID0gJXhcbiIsDQo+ID4gKwkJCWludHJfY29hbGVzY2luZ19jb3Vu
dCwgaW50cl9jb2FsZXNjaW5nX3RpY2tzKTsNCj4gPiArCURQUklOVEsoIklDQyByZWdpc3RlciBz
dGF0dXM6IChoY3IgYmFzZTogMHgleCkgPSAweCV4XG4iLA0KPiA+ICsJCQloY3JfYmFzZSwgaW9y
ZWFkMzIoaGNyX2Jhc2UgKyBJQ0MpKTsgfQ0KPiA+ICsNCj4gPiArc3RhdGljIHNzaXplX3QgZnNs
X3NhdGFfaW50cl9jb2FsZXNjaW5nX3Nob3coc3RydWN0IGRldmljZSAqZGV2LA0KPiA+ICsJCXN0
cnVjdCBkZXZpY2VfYXR0cmlidXRlICphdHRyLCBjaGFyICpidWYpIHsNCj4gPiArCXJldHVybiBz
cHJpbnRmKGJ1ZiwgIiVkCSVkXG4iLA0KPiA+ICsJCQlpbnRyX2NvYWxlc2NpbmdfY291bnQsIGlu
dHJfY29hbGVzY2luZ190aWNrcyk7IH0NCj4gPiArDQo+ID4gK3N0YXRpYyBzc2l6ZV90IGZzbF9z
YXRhX2ludHJfY29hbGVzY2luZ19zdG9yZShzdHJ1Y3QgZGV2aWNlICpkZXYsDQo+ID4gKwkJc3Ry
dWN0IGRldmljZV9hdHRyaWJ1dGUgKmF0dHIsDQo+ID4gKwkJY29uc3QgY2hhciAqYnVmLCBzaXpl
X3QgY291bnQpDQo+ID4gK3sNCj4gPiArCXVuc2lnbmVkIGludCBjb2FsZXNjaW5nX2NvdW50LAlj
b2FsZXNjaW5nX3RpY2tzOw0KPiA+ICsNCj4gPiArCWlmIChzc2NhbmYoYnVmLCAiJWQlZCIsDQo+
ID4gKwkJCQkmY29hbGVzY2luZ19jb3VudCwNCj4gPiArCQkJCSZjb2FsZXNjaW5nX3RpY2tzKSAh
PSAyKSB7DQo+ID4gKwkJcHJpbnRrKEtFUk5fRVJSICJmc2wtc2F0YTogd3JvbmcgcGFyYW1ldGVy
IGZvcm1hdC5cbiIpOw0KPiA+ICsJCXJldHVybiAtRUlOVkFMOw0KPiA+ICsJfQ0KPiA+ICsNCj4g
PiArCWZzbF9zYXRhX3NldF9pcnFfY29hbGVzY2luZyhkZXZfZ2V0X2RydmRhdGEoZGV2KSwNCj4g
PiArCQkJY29hbGVzY2luZ19jb3VudCwgY29hbGVzY2luZ190aWNrcyk7DQo+ID4gKw0KPiA+ICsJ
cmV0dXJuIHN0cmxlbihidWYpOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICBzdGF0aWMgaW5saW5lIHVu
c2lnbmVkIGludCBzYXRhX2ZzbF90YWcodW5zaWduZWQgaW50IHRhZywNCj4gPiAgCQkJCQl2b2lk
IF9faW9tZW0gKmhjcl9iYXNlKQ0KPiA+ICB7DQo+ID4gQEAgLTM0NiwxMCArNDIyLDEwIEBAIHN0
YXRpYyB1bnNpZ25lZCBpbnQgc2F0YV9mc2xfZmlsbF9zZyhzdHJ1Y3QNCj4gYXRhX3F1ZXVlZF9j
bWQgKnFjLCB2b2lkICpjbWRfZGVzYywNCj4gPiAgCQkJKHVuc2lnbmVkIGxvbmcgbG9uZylzZ19h
ZGRyLCBzZ19sZW4pOw0KPiA+DQo+ID4gIAkJLyogd2FybiBpZiBlYWNoIHMvZyBlbGVtZW50IGlz
IG5vdCBkd29yZCBhbGlnbmVkICovDQo+ID4gLQkJaWYgKHNnX2FkZHIgJiAweDAzKQ0KPiA+ICsJ
CWlmICh1bmxpa2VseShzZ19hZGRyICYgMHgwMykpDQo+ID4gIAkJCWF0YV9wb3J0X2VycihxYy0+
YXAsICJzL2cgYWRkciB1bmFsaWduZWQgOiAweCVsbHhcbiIsDQo+ID4gIAkJCQkgICAgICh1bnNp
Z25lZCBsb25nIGxvbmcpc2dfYWRkcik7DQo+ID4gLQkJaWYgKHNnX2xlbiAmIDB4MDMpDQo+ID4g
KwkJaWYgKHVubGlrZWx5KHNnX2xlbiAmIDB4MDMpKQ0KPiA+ICAJCQlhdGFfcG9ydF9lcnIocWMt
PmFwLCAicy9nIGxlbiB1bmFsaWduZWQgOiAweCV4XG4iLA0KPiA+ICAJCQkJICAgICBzZ19sZW4p
Ow0KPiA+DQo+ID4gQEAgLTEyNDUsNiArMTMyMSwxMyBAQCBzdGF0aWMgaW50IHNhdGFfZnNsX2lu
aXRfY29udHJvbGxlcihzdHJ1Y3QNCj4gYXRhX2hvc3QgKmhvc3QpDQo+ID4gIAlpb3dyaXRlMzIo
MHgwMDAwMEZGRkYsIGhjcl9iYXNlICsgQ0UpOw0KPiA+ICAJaW93cml0ZTMyKDB4MDAwMDBGRkZG
LCBoY3JfYmFzZSArIERFKTsNCj4gPg0KPiA+ICsgCS8qDQo+ID4gKwkgKiByZXNldCB0aGUgbnVt
YmVyIG9mIGNvbW1hbmQgY29tcGxldGUgYml0cyB3aGljaCB3aWxsIGNhdXNlIHRoZQ0KPiA+ICsJ
ICogaW50ZXJydXB0IHRvIGJlIHNpZ25hbGVkDQo+ID4gKwkgKi8NCj4gPiArCWZzbF9zYXRhX3Nl
dF9pcnFfY29hbGVzY2luZyhob3N0LCBpbnRyX2NvYWxlc2NpbmdfY291bnQsDQo+ID4gKwkJCWlu
dHJfY29hbGVzY2luZ190aWNrcyk7DQo+ID4gKw0KPiA+ICAJLyoNCj4gPiAgCSAqIGhvc3QgY29u
dHJvbGxlciB3aWxsIGJlIGJyb3VnaHQgb24tbGluZSwgZHVyaW5nIHh4X3BvcnRfc3RhcnQoKQ0K
PiA+ICAJICogY2FsbGJhY2ssIHRoYXQgc2hvdWxkIGFsc28gaW5pdGlhdGUgdGhlIE9PQiwgQ09N
SU5JVCBzZXF1ZW5jZSBAQA0KPiA+IC0xMzA5LDcgKzEzOTIsNyBAQCBzdGF0aWMgaW50IHNhdGFf
ZnNsX3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UNCj4gKm9mZGV2KQ0KPiA+ICAJdm9pZCBf
X2lvbWVtICpjc3JfYmFzZSA9IE5VTEw7DQo+ID4gIAlzdHJ1Y3Qgc2F0YV9mc2xfaG9zdF9wcml2
ICpob3N0X3ByaXYgPSBOVUxMOw0KPiA+ICAJaW50IGlycTsNCj4gPiAtCXN0cnVjdCBhdGFfaG9z
dCAqaG9zdDsNCj4gPiArCXN0cnVjdCBhdGFfaG9zdCAqaG9zdCA9IE5VTEw7DQo+ID4gIAl1MzIg
dGVtcDsNCj4gPg0KPiA+ICAJc3RydWN0IGF0YV9wb3J0X2luZm8gcGkgPSBzYXRhX2ZzbF9wb3J0
X2luZm9bMF07IEBAIC0xMzU2LDYNCj4gKzE0MzksMTANCj4gPiBAQCBzdGF0aWMgaW50IHNhdGFf
ZnNsX3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKm9mZGV2KQ0KPiA+DQo+ID4gIAkvKiBh
bGxvY2F0ZSBob3N0IHN0cnVjdHVyZSAqLw0KPiA+ICAJaG9zdCA9IGF0YV9ob3N0X2FsbG9jX3Bp
bmZvKCZvZmRldi0+ZGV2LCBwcGksIFNBVEFfRlNMX01BWF9QT1JUUyk7DQo+ID4gKwlpZiAoIWhv
c3QpIHsNCj4gPiArCQlyZXR2YWwgPSAtRU5PTUVNOw0KPiA+ICsJCWdvdG8gZXJyb3JfZXhpdF93
aXRoX2NsZWFudXA7DQo+ID4gKwl9DQo+ID4NCj4gPiAgCS8qIGhvc3QtPmlvbWFwIGlzIG5vdCB1
c2VkIGN1cnJlbnRseSAqLw0KPiA+ICAJaG9zdC0+cHJpdmF0ZV9kYXRhID0gaG9zdF9wcml2Ow0K
PiA+IEBAIC0xMzczLDEwICsxNDYwLDI0IEBAIHN0YXRpYyBpbnQgc2F0YV9mc2xfcHJvYmUoc3Ry
dWN0DQo+ID4gcGxhdGZvcm1fZGV2aWNlICpvZmRldikNCj4gPg0KPiA+ICAJZGV2X3NldF9kcnZk
YXRhKCZvZmRldi0+ZGV2LCBob3N0KTsNCj4gPg0KPiA+ICsJaG9zdF9wcml2LT5pbnRyX2NvYWxl
c2Npbmcuc2hvdyA9IGZzbF9zYXRhX2ludHJfY29hbGVzY2luZ19zaG93Ow0KPiA+ICsJaG9zdF9w
cml2LT5pbnRyX2NvYWxlc2Npbmcuc3RvcmUgPSBmc2xfc2F0YV9pbnRyX2NvYWxlc2Npbmdfc3Rv
cmU7DQo+ID4gKwlzeXNmc19hdHRyX2luaXQoJmhvc3RfcHJpdi0+aW50cl9jb2FsZXNjaW5nLmF0
dHIpOw0KPiA+ICsJaG9zdF9wcml2LT5pbnRyX2NvYWxlc2NpbmcuYXR0ci5uYW1lID0gImludHJf
Y29hbGVzY2luZyI7DQo+ID4gKwlob3N0X3ByaXYtPmludHJfY29hbGVzY2luZy5hdHRyLm1vZGUg
PSBTX0lSVUdPIHwgU19JV1VTUjsNCj4gPiArCXJldHZhbCA9IGRldmljZV9jcmVhdGVfZmlsZSho
b3N0LT5kZXYsICZob3N0X3ByaXYtPmludHJfY29hbGVzY2luZyk7DQo+ID4gKwlpZiAocmV0dmFs
KQ0KPiA+ICsJCWdvdG8gZXJyb3JfZXhpdF93aXRoX2NsZWFudXA7DQo+ID4gKw0KPiA+ICAJcmV0
dXJuIDA7DQo+ID4NCj4gPiAgZXJyb3JfZXhpdF93aXRoX2NsZWFudXA6DQo+ID4NCj4gPiArCWlm
IChob3N0KSB7DQo+ID4gKwkJZGV2X3NldF9kcnZkYXRhKCZvZmRldi0+ZGV2LCBOVUxMKTsNCj4g
PiArCQlhdGFfaG9zdF9kZXRhY2goaG9zdCk7DQo+ID4gKwl9DQo+ID4gKw0KPiA+ICAJaWYgKGhj
cl9iYXNlKQ0KPiA+ICAJCWlvdW5tYXAoaGNyX2Jhc2UpOw0KPiA+ICAJaWYgKGhvc3RfcHJpdikN
Cj4gPiBAQCAtMTM5MCw2ICsxNDkxLDggQEAgc3RhdGljIGludCBzYXRhX2ZzbF9yZW1vdmUoc3Ry
dWN0IHBsYXRmb3JtX2RldmljZQ0KPiAqb2ZkZXYpDQo+ID4gIAlzdHJ1Y3QgYXRhX2hvc3QgKmhv
c3QgPSBkZXZfZ2V0X2RydmRhdGEoJm9mZGV2LT5kZXYpOw0KPiA+ICAJc3RydWN0IHNhdGFfZnNs
X2hvc3RfcHJpdiAqaG9zdF9wcml2ID0gaG9zdC0+cHJpdmF0ZV9kYXRhOw0KPiA+DQo+ID4gKwlk
ZXZpY2VfcmVtb3ZlX2ZpbGUoJm9mZGV2LT5kZXYsICZob3N0X3ByaXYtPmludHJfY29hbGVzY2lu
Zyk7DQo+ID4gKw0KPiA+ICAJYXRhX2hvc3RfZGV0YWNoKGhvc3QpOw0KPiA+DQo+ID4gIAlkZXZf
c2V0X2RydmRhdGEoJm9mZGV2LT5kZXYsIE5VTEwpOw0KPiA+IC0tDQo+ID4gMS42LjQNCj4gPg0K
PiA+DQo+ID4gX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18N
Cj4gPiBMaW51eHBwYy1kZXYgbWFpbGluZyBsaXN0DQo+ID4gTGludXhwcGMtZGV2QGxpc3RzLm96
bGFicy5vcmcNCj4gPiBodHRwczovL2xpc3RzLm96bGFicy5vcmcvbGlzdGluZm8vbGludXhwcGMt
ZGV2DQo+IA0KPiANCg0K

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Li Yang-R58472 Feb. 17, 2012, 3:09 a.m. UTC | #5
>I tried a debian install on the p5020ds here and I find SATA to be
>extremely slow, generating millions of interrupts. I think the defaults
>should be a lot more aggressive at coalescing.

The P5020 has a hardware problem with SATA, making it generate more interrupts than it should.  A new revision of the silicon will fix it.

- Leo

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt Feb. 17, 2012, 3:17 a.m. UTC | #6
On Fri, 2012-02-17 at 01:54 +0000, Liu Qiang-B32616 wrote:
> The default will be set in a common interface fsl_sata_set_irq_coalescing when
> initialize the controller. This interface will check the range of intr count
> and ticks and make sure the values are reasonably.

Allright, but the current defaults are basically no coalescing right ?

> It's hard to find a aggressive value to adapt all scenarios, so I use echo to adjust
> the value. I remember P5020 have some performance issue, I will check it.
> BTW, which filesystem do you use? Ext2 is lower than ext4 because metadata is 
> continuously wrote to disk. You can try ext4 or xfs. 

ext3 at the moment, I plan to switch to ext4 when I finish that fsck
pass which is taking hours... I am not aware of the 5020 performance
issues, is this something documented and/or fixable ?

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt Feb. 17, 2012, 3:18 a.m. UTC | #7
On Fri, 2012-02-17 at 03:09 +0000, Li Yang-R58472 wrote:
> >I tried a debian install on the p5020ds here and I find SATA to be
> >extremely slow, generating millions of interrupts. I think the
> defaults
> >should be a lot more aggressive at coalescing.
> 
> The P5020 has a hardware problem with SATA, making it generate more
> interrupts than it should.  A new revision of the silicon will fix it.

Ok cool. With a bit of luck you guys can send me a new silicon when it's
available then :-) (The one in the board you got me seems to be in a
socket).

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Qiang-B32616 Feb. 17, 2012, 3:30 a.m. UTC | #8
> -----Original Message-----

> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]

> Sent: Friday, February 17, 2012 11:18 AM

> To: Liu Qiang-B32616

> Cc: jgarzik@pobox.com; linux-ide@vger.kernel.org; linuxppc-

> dev@lists.ozlabs.org; linux-kernel@vger.kernel.org

> Subject: RE: [PATCH V2 RESEND] fsl-sata: I/O load balancing

> 

> On Fri, 2012-02-17 at 01:54 +0000, Liu Qiang-B32616 wrote:

> > The default will be set in a common interface

> > fsl_sata_set_irq_coalescing when initialize the controller. This

> > interface will check the range of intr count and ticks and make sure

> the values are reasonably.

> 

> Allright, but the current defaults are basically no coalescing right ?

> 

> > It's hard to find a aggressive value to adapt all scenarios, so I use

> > echo to adjust the value. I remember P5020 have some performance issue,

> I will check it.

> > BTW, which filesystem do you use? Ext2 is lower than ext4 because

> > metadata is continuously wrote to disk. You can try ext4 or xfs.

> 

> ext3 at the moment, I plan to switch to ext4 when I finish that fsck pass

> which is taking hours... I am not aware of the 5020 performance issues,

> is this something documented and/or fixable ?

> 


My patch may not meet your requirement:(
For your performance requirement, I suggest use ext4 filesystem and SSD.

Thanks.

> Cheers,

> Ben.

> 

>
Zang Roy-R61911 Feb. 20, 2012, 4:40 a.m. UTC | #9
> -----Original Message-----
> From: linuxppc-dev-bounces+tie-fei.zang=freescale.com@lists.ozlabs.org
> [mailto:linuxppc-dev-bounces+tie-fei.zang=freescale.com@lists.ozlabs.org]
> On Behalf Of Benjamin Herrenschmidt
> Sent: Friday, February 17, 2012 11:19 AM
> To: Li Yang-R58472
> Cc: Liu Qiang-B32616; jgarzik@pobox.com; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; linux-ide@vger.kernel.org
> Subject: RE: [PATCH V2 RESEND] fsl-sata: I/O load balancing
> 
> On Fri, 2012-02-17 at 03:09 +0000, Li Yang-R58472 wrote:
> > >I tried a debian install on the p5020ds here and I find SATA to be
> > >extremely slow, generating millions of interrupts. I think the
> > defaults
> > >should be a lot more aggressive at coalescing.
> >
> > The P5020 has a hardware problem with SATA, making it generate more
> > interrupts than it should.  A new revision of the silicon will fix it.
> 
> Ok cool. With a bit of luck you guys can send me a new silicon when it's
> available then :-) (The one in the board you got me seems to be in a
> socket).
Ben,
We still do not have the a P5020 sample to fix the problem.
One workaround is to use the patch ( I think you have noticed the patch):
http://git.freescale.com/git/cgit.cgi/ppc/sdk/linux.git/commit/?id=92c0ce1e599e788ffc0908739db9bd5e0dbdad69

and  use a SSD hard drive.
It will have a usable performance.
Thanks.
Roy

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 0120b0d..d6577b9 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -6,7 +6,7 @@ 
  * Author: Ashish Kalra <ashish.kalra@freescale.com>
  * Li Yang <leoli@freescale.com>
  *
- * Copyright (c) 2006-2007, 2011 Freescale Semiconductor, Inc.
+ * Copyright (c) 2006-2007, 2011-2012 Freescale Semiconductor, Inc.
  *
  * 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
@@ -26,6 +26,15 @@ 
 #include <asm/io.h>
 #include <linux/of_platform.h>

+static unsigned int intr_coalescing_count;
+module_param(intr_coalescing_count, int, S_IRUGO);
+MODULE_PARM_DESC(intr_coalescing_count,
+				 "INT coalescing count threshold (1..31)");
+
+static unsigned int intr_coalescing_ticks;
+module_param(intr_coalescing_ticks, int, S_IRUGO);
+MODULE_PARM_DESC(intr_coalescing_ticks,
+				 "INT coalescing timer threshold in AHB ticks");
 /* Controller information */
 enum {
 	SATA_FSL_QUEUE_DEPTH	= 16,
@@ -83,6 +92,16 @@  enum {
 };

 /*
+ * Interrupt Coalescing Control Register bitdefs  */
+enum {
+	ICC_MIN_INT_COUNT_THRESHOLD	= 1,
+	ICC_MAX_INT_COUNT_THRESHOLD	= ((1 << 5) - 1),
+	ICC_MIN_INT_TICKS_THRESHOLD	= 0,
+	ICC_MAX_INT_TICKS_THRESHOLD	= ((1 << 19) - 1),
+	ICC_SAFE_INT_TICKS		= 1,
+};
+
+/*
 * Host Controller command register set - per port
 */
 enum {
@@ -263,8 +282,65 @@  struct sata_fsl_host_priv {
 	void __iomem *csr_base;
 	int irq;
 	int data_snoop;
+	struct device_attribute intr_coalescing;
 };

+static void fsl_sata_set_irq_coalescing(struct ata_host *host,
+		unsigned int count, unsigned int ticks)
+{
+	struct sata_fsl_host_priv *host_priv = host->private_data;
+	void __iomem *hcr_base = host_priv->hcr_base;
+
+	if (count > ICC_MAX_INT_COUNT_THRESHOLD)
+		count = ICC_MAX_INT_COUNT_THRESHOLD;
+	else if (count < ICC_MIN_INT_COUNT_THRESHOLD)
+		count = ICC_MIN_INT_COUNT_THRESHOLD;
+
+	if (ticks > ICC_MAX_INT_TICKS_THRESHOLD)
+		ticks = ICC_MAX_INT_TICKS_THRESHOLD;
+	else if ((ICC_MIN_INT_TICKS_THRESHOLD == ticks) &&
+			(count > ICC_MIN_INT_COUNT_THRESHOLD))
+		ticks = ICC_SAFE_INT_TICKS;
+
+	spin_lock(&host->lock);
+	iowrite32((count << 24 | ticks), hcr_base + ICC);
+
+	intr_coalescing_count = count;
+	intr_coalescing_ticks = ticks;
+	spin_unlock(&host->lock);
+
+	DPRINTK("intrrupt coalescing, count = 0x%x, ticks = %x\n",
+			intr_coalescing_count, intr_coalescing_ticks);
+	DPRINTK("ICC register status: (hcr base: 0x%x) = 0x%x\n",
+			hcr_base, ioread32(hcr_base + ICC));
+}
+
+static ssize_t fsl_sata_intr_coalescing_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d	%d\n",
+			intr_coalescing_count, intr_coalescing_ticks);
+}
+
+static ssize_t fsl_sata_intr_coalescing_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	unsigned int coalescing_count,	coalescing_ticks;
+
+	if (sscanf(buf, "%d%d",
+				&coalescing_count,
+				&coalescing_ticks) != 2) {
+		printk(KERN_ERR "fsl-sata: wrong parameter format.\n");
+		return -EINVAL;
+	}
+
+	fsl_sata_set_irq_coalescing(dev_get_drvdata(dev),
+			coalescing_count, coalescing_ticks);
+
+	return strlen(buf);
+}
+
 static inline unsigned int sata_fsl_tag(unsigned int tag,
 					void __iomem *hcr_base)
 {
@@ -346,10 +422,10 @@  static unsigned int sata_fsl_fill_sg(struct ata_queued_cmd *qc, void *cmd_desc,
 			(unsigned long long)sg_addr, sg_len);

 		/* warn if each s/g element is not dword aligned */
-		if (sg_addr & 0x03)
+		if (unlikely(sg_addr & 0x03))
 			ata_port_err(qc->ap, "s/g addr unaligned : 0x%llx\n",
 				     (unsigned long long)sg_addr);
-		if (sg_len & 0x03)
+		if (unlikely(sg_len & 0x03))
 			ata_port_err(qc->ap, "s/g len unaligned : 0x%x\n",
 				     sg_len);

@@ -1245,6 +1321,13 @@  static int sata_fsl_init_controller(struct ata_host *host)
 	iowrite32(0x00000FFFF, hcr_base + CE);
 	iowrite32(0x00000FFFF, hcr_base + DE);

+ 	/*
+	 * reset the number of command complete bits which will cause the
+	 * interrupt to be signaled
+	 */
+	fsl_sata_set_irq_coalescing(host, intr_coalescing_count,
+			intr_coalescing_ticks);
+
 	/*
 	 * host controller will be brought on-line, during xx_port_start()
 	 * callback, that should also initiate the OOB, COMINIT sequence
@@ -1309,7 +1392,7 @@  static int sata_fsl_probe(struct platform_device *ofdev)
 	void __iomem *csr_base = NULL;
 	struct sata_fsl_host_priv *host_priv = NULL;
 	int irq;
-	struct ata_host *host;
+	struct ata_host *host = NULL;
 	u32 temp;

 	struct ata_port_info pi = sata_fsl_port_info[0];
@@ -1356,6 +1439,10 @@  static int sata_fsl_probe(struct platform_device *ofdev)

 	/* allocate host structure */
 	host = ata_host_alloc_pinfo(&ofdev->dev, ppi, SATA_FSL_MAX_PORTS);
+	if (!host) {
+		retval = -ENOMEM;
+		goto error_exit_with_cleanup;
+	}

 	/* host->iomap is not used currently */
 	host->private_data = host_priv;
@@ -1373,10 +1460,24 @@  static int sata_fsl_probe(struct platform_device *ofdev)

 	dev_set_drvdata(&ofdev->dev, host);

+	host_priv->intr_coalescing.show = fsl_sata_intr_coalescing_show;
+	host_priv->intr_coalescing.store = fsl_sata_intr_coalescing_store;
+	sysfs_attr_init(&host_priv->intr_coalescing.attr);
+	host_priv->intr_coalescing.attr.name = "intr_coalescing";
+	host_priv->intr_coalescing.attr.mode = S_IRUGO | S_IWUSR;
+	retval = device_create_file(host->dev, &host_priv->intr_coalescing);
+	if (retval)
+		goto error_exit_with_cleanup;
+
 	return 0;

 error_exit_with_cleanup:

+	if (host) {
+		dev_set_drvdata(&ofdev->dev, NULL);
+		ata_host_detach(host);
+	}
+
 	if (hcr_base)
 		iounmap(hcr_base);
 	if (host_priv)
@@ -1390,6 +1491,8 @@  static int sata_fsl_remove(struct platform_device *ofdev)
 	struct ata_host *host = dev_get_drvdata(&ofdev->dev);
 	struct sata_fsl_host_priv *host_priv = host->private_data;

+	device_remove_file(&ofdev->dev, &host_priv->intr_coalescing);
+
 	ata_host_detach(host);

 	dev_set_drvdata(&ofdev->dev, NULL);