RE: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c

2012-04-13 Thread Thang Nguyen
Thanks Jeff and Sergei,

As your suggestion, I will separate the patch into smaller patches and
support more features on the SATA DWC driver. The patches I intend to do
on the SATA DWC are as below:
 - Support hardreset: currently the hardreset is not supported. This
causes sometime the SATA driver does cause kernel crash because of
not-determined state.
 - Let device tree specified DMA channel: currently only channel 0 is
supported (number of channel is set to 1). If device tree not specified
DMA channel, channel 0 will be used as default.
 - Support ATAPI.
 - Remove dma_interrupt_count. for each DMA transfer, we need 2 interrupts
for QC completion: transfer completion and DMA transfer completion
interrupt. The current code wait for both 2 interrupts occur before
calling qc_complete. This will make out-of-sync state when an interrupt
lost or when errors occur. The change will process DMA register when DMA
transfer complete interrupt occur and call qc_issue when command
completion interrupt occur.
 - Fix NCQ issue and set .can_queue back to ATA_MAX_QUEUE.
 - Support Port Multiplier.
 - Support 2 SATA ports on Maui.

Regards,
Thang Nguyen-

-Original Message-
From: Jeff Garzik [mailto:jgpo...@gmail.com] On Behalf Of Jeff Garzik
Sent: Friday, April 13, 2012 3:05 AM
To: Sergei Shtylyov
Cc: Thang Q. Nguyen; Benjamin Herrenschmidt; Paul Mackerras; Grant Likely;
Rob Herring; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org;
linux-...@vger.kernel.org; devicetree-disc...@lists.ozlabs.org
Subject: Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change
filename from sata_dwc_460ex.c to sata_dwc_4xx.c

On 04/03/2012 07:56 AM, Sergei Shtylyov wrote:
 Hello.

 On 03-04-2012 14:12, Thang Q. Nguyen wrote:

 Signed-off-by: Thang Q. Nguyentqngu...@apm.com
 ---
 Changes for v2:
 - Use git rename feature to change the driver to the newname and for
 easier review.

 arch/powerpc/boot/dts/bluestone.dts | 21 +
 drivers/ata/Makefile | 2 +-
 drivers/ata/{sata_dwc_460ex.c = sata_dwc_4xx.c} | 1371
 ++
 3 files changed, 904 insertions(+), 490 deletions(-)
 rename drivers/ata/{sata_dwc_460ex.c = sata_dwc_4xx.c} (56%)

 You submitted a magapatch doing several things at once (some even
 needlessly) and even in two areas of the kernel. This needs proper
 splitting/description.

Agreed...
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, 
is for the sole use of the intended recipient(s) and contains information 
that is confidential and proprietary to AppliedMicro Corporation or its 
subsidiaries. 
It is to be used solely for the purpose of furthering the parties' business 
relationship. 
All unauthorized review, use, disclosure or distribution is prohibited. 
If you are not the intended recipient, please contact the sender by reply 
e-mail 
and destroy all copies of the original message.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c

2012-04-13 Thread Jeff Garzik

On 04/13/2012 03:18 AM, Thang Nguyen wrote:

Thanks Jeff and Sergei,

As your suggestion, I will separate the patch into smaller patches and
support more features on the SATA DWC driver. The patches I intend to do
on the SATA DWC are as below:
  - Support hardreset: currently the hardreset is not supported. This
causes sometime the SATA driver does cause kernel crash because of
not-determined state.
  - Let device tree specified DMA channel: currently only channel 0 is
supported (number of channel is set to 1). If device tree not specified
DMA channel, channel 0 will be used as default.
  - Support ATAPI.
  - Remove dma_interrupt_count. for each DMA transfer, we need 2 interrupts
for QC completion: transfer completion and DMA transfer completion
interrupt. The current code wait for both 2 interrupts occur before
calling qc_complete. This will make out-of-sync state when an interrupt
lost or when errors occur. The change will process DMA register when DMA
transfer complete interrupt occur and call qc_issue when command
completion interrupt occur.
  - Fix NCQ issue and set .can_queue back to ATA_MAX_QUEUE.
  - Support Port Multiplier.
  - Support 2 SATA ports on Maui.


Sounds good, thanks for splitting up the patch into smaller pieces.  The 
main goal is that separate logical changes should go into separate patches.


Jeff




___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c

2012-04-12 Thread Jeff Garzik

On 04/03/2012 07:56 AM, Sergei Shtylyov wrote:

Hello.

On 03-04-2012 14:12, Thang Q. Nguyen wrote:


Signed-off-by: Thang Q. Nguyentqngu...@apm.com
---
Changes for v2:
- Use git rename feature to change the driver to the newname and for
easier review.



arch/powerpc/boot/dts/bluestone.dts | 21 +
drivers/ata/Makefile | 2 +-
drivers/ata/{sata_dwc_460ex.c = sata_dwc_4xx.c} | 1371
++
3 files changed, 904 insertions(+), 490 deletions(-)
rename drivers/ata/{sata_dwc_460ex.c = sata_dwc_4xx.c} (56%)


You submitted a magapatch doing several things at once (some even
needlessly) and even in two areas of the kernel. This needs proper
splitting/description.


Agreed...



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c

2012-04-12 Thread Thang Q. Nguyen
Hi,
Two SATA device nodes in bluestone.dts uses the same structure as SATA node
in the canyonlands.dts. All boards having SATA DWC currently use this
structure. So, changing it will need to change all corresponding device
tree. The current sata_dwc_460ex.c agrees with this structure.

I want to give more information for SATA controller on Maui:
 - There are 2 SATA controllers on Maui, each has its own register set. But
they use the same DMA registers for processing DMA transfer.
 - For DMA transfer, each SATA controller will use its own DMA channel in
the DMA registers.
 - DMA controller driver must be supported and enabled in order to issue
SATA DMA transfer.

I agree that declaring the same DMA register information in 2 device nodes
can cause confliction but I think the driver can handle it. In case this
approach is not accepted, I will add only 1 SATA port to the bluestone.dts
and another port will be considered later.

For the APM confidential information from my email, it is added
automatically by the APM system. I have submitted a ticket for asking IT
not adding it into my email and it is in-progress. I will continue to
submit patches after the ticket is processed.

Regards,
Thang Nguyen -

On Tue, Apr 10, 2012 at 6:22 PM, Sergei Shtylyov sshtyl...@mvista.comwrote:

 Hello.


 On 10-04-2012 7:46, Thang Nguyen wrote:

  Hi Sergei,
 Thanks for your review.


  On Maui, there are 2 separate SATA controllers but they share the same
 AHBDMA controller. Each SATA controller is assigned a fixed DMA channel on
 the AHBDMA (channel 0 is assigned to SATA controller 0 and channel 1 is
 assigned to SATA controller 1).
 For the 460EX, there is only 1 SATA controller and it uses channel 0 for
 transferring data.


  In my opinion, in the case of Maui, we can use the same DMA information in
 2 device nodes as they use the same DMA controller. And in another CPU, if
 they use different DMA controller, the corresponding information will also
 be different.


   No, either the DMA controller should be a separate device node, or both
 ports and DMA controller should be packed into the single device node. The
 way you're doing it is incorrect because it creates memory resource
 conflict between devices when they are instantiated as platfrom devices.


  Regards,
 Thang Nguyen -
 -Original Message-
 From: Sergei Shtylyov [mailto:sshtyl...@mvista.com]
 Sent: Monday, April 09, 2012 5:13 PM
 To: Thang Q. Nguyen
 Cc: Benjamin Herrenschmidt; Paul Mackerras; Jeff Garzik; Grant Likely; Rob
 Herring; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org;
 linux-...@vger.kernel.org; 
 devicetree-discuss@lists.**ozlabs.orgdevicetree-disc...@lists.ozlabs.org
 Subject: Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change
 filename from sata_dwc_460ex.c to sata_dwc_4xx.c


  Hello.


  On 03-04-2012 14:12, Thang Q. Nguyen wrote:


  Signed-off-by: Thang Q. Nguyentqngu...@apm.com

 [...]


  diff --git a/arch/powerpc/boot/dts/**bluestone.dts

 b/arch/powerpc/boot/dts/**bluestone.dts

 index cfa23bf..803fda6 100644
 --- a/arch/powerpc/boot/dts/**bluestone.dts
 +++ b/arch/powerpc/boot/dts/**bluestone.dts
 @@ -155,6 +155,27 @@
/*RXDE*/  0x5 0x4;
};

 +   /* SATA DWC devices */
 +   SATA0: sata@bffd1000 {
 +   compatible = amcc,sata-apm821xx;
 +   reg =4 0xbffd1000 0x800   /* SATA0 */
 +  4 0xbffd0800 0x400; /* AHBDMA */
 +   dma-channel=0;
 +   interrupt-parent =UIC0;
 +   interrupts =26 4/* SATA0 */
 + 25 4;  /* AHBDMA */
 +   };
 +
 +   SATA1: sata@bffd1800 {
 +   compatible = amcc,sata-apm821xx;
 +   reg =4 0xbffd1800 0x800   /* SATA1 */
 +  4 0xbffd0800 0x400; /* AHBDMA */
 +   dma-channel=1;
 +   interrupt-parent =UIC0;
 +   interrupts =27 4/* SATA1 */
 + 25 4;  /* AHBDMA */
 +   };
 +


  So, this is dual SATA controller, not dual port SATA controller?
 BTW, it's wrong to have the same AHBDMA resource in two device nodes I
 think.


  MBR, Sergei


   Can you get rid of the following? It looks stupid when you post to the
 maliing list.

  CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
 is for the sole use of the intended recipient(s) and contains information
 that is confidential and proprietary to AppliedMicro Corporation or its
 subsidiaries.
 It is to be used solely for the purpose of furthering the parties'
 business relationship.
 All unauthorized review, use, disclosure or distribution is prohibited.
 If you are not the intended recipient, please contact the sender by reply
 e-mail
 and destroy all copies of the original message.



Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c

2012-04-10 Thread Sergei Shtylyov

Hello.

On 10-04-2012 7:46, Thang Nguyen wrote:


Hi Sergei,
Thanks for your review.



On Maui, there are 2 separate SATA controllers but they share the same
AHBDMA controller. Each SATA controller is assigned a fixed DMA channel on
the AHBDMA (channel 0 is assigned to SATA controller 0 and channel 1 is
assigned to SATA controller 1).
For the 460EX, there is only 1 SATA controller and it uses channel 0 for
transferring data.



In my opinion, in the case of Maui, we can use the same DMA information in
2 device nodes as they use the same DMA controller. And in another CPU, if
they use different DMA controller, the corresponding information will also
be different.


   No, either the DMA controller should be a separate device node, or both 
ports and DMA controller should be packed into the single device node. The way 
you're doing it is incorrect because it creates memory resource conflict 
between devices when they are instantiated as platfrom devices.



Regards,
Thang Nguyen -
-Original Message-
From: Sergei Shtylyov [mailto:sshtyl...@mvista.com]
Sent: Monday, April 09, 2012 5:13 PM
To: Thang Q. Nguyen
Cc: Benjamin Herrenschmidt; Paul Mackerras; Jeff Garzik; Grant Likely; Rob
Herring; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org;
linux-...@vger.kernel.org; devicetree-disc...@lists.ozlabs.org
Subject: Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change
filename from sata_dwc_460ex.c to sata_dwc_4xx.c



Hello.



On 03-04-2012 14:12, Thang Q. Nguyen wrote:



Signed-off-by: Thang Q. Nguyentqngu...@apm.com

[...]



diff --git a/arch/powerpc/boot/dts/bluestone.dts

b/arch/powerpc/boot/dts/bluestone.dts

index cfa23bf..803fda6 100644
--- a/arch/powerpc/boot/dts/bluestone.dts
+++ b/arch/powerpc/boot/dts/bluestone.dts
@@ -155,6 +155,27 @@
/*RXDE*/  0x5 0x4;
};

+   /* SATA DWC devices */
+   SATA0: sata@bffd1000 {
+   compatible = amcc,sata-apm821xx;
+   reg =4 0xbffd1000 0x800   /* SATA0 */
+  4 0xbffd0800 0x400; /* AHBDMA */
+   dma-channel=0;
+   interrupt-parent =UIC0;
+   interrupts =26 4/* SATA0 */
+ 25 4;  /* AHBDMA */
+   };
+
+   SATA1: sata@bffd1800 {
+   compatible = amcc,sata-apm821xx;
+   reg =4 0xbffd1800 0x800   /* SATA1 */
+  4 0xbffd0800 0x400; /* AHBDMA */
+   dma-channel=1;
+   interrupt-parent =UIC0;
+   interrupts =27 4/* SATA1 */
+ 25 4;  /* AHBDMA */
+   };
+



 So, this is dual SATA controller, not dual port SATA controller?
BTW, it's wrong to have the same AHBDMA resource in two device nodes I
think.



MBR, Sergei


   Can you get rid of the following? It looks stupid when you post to the 
maliing list.



CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
is for the sole use of the intended recipient(s) and contains information
that is confidential and proprietary to AppliedMicro Corporation or its 
subsidiaries.
It is to be used solely for the purpose of furthering the parties' business 
relationship.
All unauthorized review, use, disclosure or distribution is prohibited.
If you are not the intended recipient, please contact the sender by reply e-mail
and destroy all copies of the original message.


WBR, Sergei
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c

2012-04-09 Thread Sergei Shtylyov

Hello.

On 03-04-2012 14:12, Thang Q. Nguyen wrote:


Signed-off-by: Thang Q. Nguyentqngu...@apm.com

[...]


diff --git a/arch/powerpc/boot/dts/bluestone.dts 
b/arch/powerpc/boot/dts/bluestone.dts
index cfa23bf..803fda6 100644
--- a/arch/powerpc/boot/dts/bluestone.dts
+++ b/arch/powerpc/boot/dts/bluestone.dts
@@ -155,6 +155,27 @@
/*RXDE*/  0x5 0x4;
};

+   /* SATA DWC devices */
+   SATA0: sata@bffd1000 {
+   compatible = amcc,sata-apm821xx;
+   reg =4 0xbffd1000 0x800   /* SATA0 */
+  4 0xbffd0800 0x400; /* AHBDMA */
+   dma-channel=0;
+   interrupt-parent =UIC0;
+   interrupts =26 4/* SATA0 */
+ 25 4;  /* AHBDMA */
+   };
+
+   SATA1: sata@bffd1800 {
+   compatible = amcc,sata-apm821xx;
+   reg =4 0xbffd1800 0x800   /* SATA1 */
+  4 0xbffd0800 0x400; /* AHBDMA */
+   dma-channel=1;
+   interrupt-parent =UIC0;
+   interrupts =27 4/* SATA1 */
+ 25 4;  /* AHBDMA */
+   };
+


   So, this is dual SATA controller, not dual port SATA controller?
BTW, it's wrong to have the same AHBDMA resource in two device nodes I think.

MBR, Sergei
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


RE: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c

2012-04-09 Thread Thang Nguyen
Hi Sergei,
Thanks for your review.
On Maui, there are 2 separate SATA controllers but they share the same
AHBDMA controller. Each SATA controller is assigned a fixed DMA channel on
the AHBDMA (channel 0 is assigned to SATA controller 0 and channel 1 is
assigned to SATA controller 1).
For the 460EX, there is only 1 SATA controller and it uses channel 0 for
transferring data.

In my opinion, in the case of Maui, we can use the same DMA information in
2 device nodes as they use the same DMA controller. And in another CPU, if
they use different DMA controller, the corresponding information will also
be different.

Regards,
Thang Nguyen -
-Original Message-
From: Sergei Shtylyov [mailto:sshtyl...@mvista.com]
Sent: Monday, April 09, 2012 5:13 PM
To: Thang Q. Nguyen
Cc: Benjamin Herrenschmidt; Paul Mackerras; Jeff Garzik; Grant Likely; Rob
Herring; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org;
linux-...@vger.kernel.org; devicetree-disc...@lists.ozlabs.org
Subject: Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change
filename from sata_dwc_460ex.c to sata_dwc_4xx.c

Hello.

On 03-04-2012 14:12, Thang Q. Nguyen wrote:

 Signed-off-by: Thang Q. Nguyentqngu...@apm.com
[...]

 diff --git a/arch/powerpc/boot/dts/bluestone.dts
b/arch/powerpc/boot/dts/bluestone.dts
 index cfa23bf..803fda6 100644
 --- a/arch/powerpc/boot/dts/bluestone.dts
 +++ b/arch/powerpc/boot/dts/bluestone.dts
 @@ -155,6 +155,27 @@
   /*RXDE*/  0x5 0x4;
   };

 + /* SATA DWC devices */
 + SATA0: sata@bffd1000 {
 + compatible = amcc,sata-apm821xx;
 + reg =4 0xbffd1000 0x800   /* SATA0 */
 +4 0xbffd0800 0x400; /* AHBDMA */
 + dma-channel=0;
 + interrupt-parent =UIC0;
 + interrupts =26 4/* SATA0 */
 +   25 4;  /* AHBDMA */
 + };
 +
 + SATA1: sata@bffd1800 {
 + compatible = amcc,sata-apm821xx;
 + reg =4 0xbffd1800 0x800   /* SATA1 */
 +4 0xbffd0800 0x400; /* AHBDMA */
 + dma-channel=1;
 + interrupt-parent =UIC0;
 + interrupts =27 4/* SATA1 */
 +   25 4;  /* AHBDMA */
 + };
 +

So, this is dual SATA controller, not dual port SATA controller?
BTW, it's wrong to have the same AHBDMA resource in two device nodes I
think.

MBR, Sergei
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, 
is for the sole use of the intended recipient(s) and contains information 
that is confidential and proprietary to AppliedMicro Corporation or its 
subsidiaries. 
It is to be used solely for the purpose of furthering the parties' business 
relationship. 
All unauthorized review, use, disclosure or distribution is prohibited. 
If you are not the intended recipient, please contact the sender by reply 
e-mail 
and destroy all copies of the original message.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


RE: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c

2012-04-04 Thread Thang Nguyen
Thanks Sergei for quick response.
In the original driver (sata_dwc_460ex.c), an instance of the
sata_dwc_host_priv structure is declared and used globally. This will not
require functions to have ata_port structure or alternative structures
(ata_link, hsdev,...) to be declared in the function calls.

However, for supporting 2 ports, we can't use the sata_dwc_host_priv
structure as functions need to know what SATA port they are working on.
This is the reason why you see the ata_port, ata_link, sata_dwc_device,
sata_dwc_device_port,... structures declared in functions.

I will update the patch as your feedback and separate also the
bluestone.dts device tree to the another patch


Thanks and best regards,
Thang Nguyen -
Applied Micro

-Original Message-
From: Sergei Shtylyov [mailto:sshtyl...@mvista.com]
Sent: Tuesday, April 03, 2012 6:57 PM
To: Thang Q. Nguyen
Cc: Benjamin Herrenschmidt; Paul Mackerras; Jeff Garzik; Grant Likely; Rob
Herring; linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org;
linux-...@vger.kernel.org; devicetree-disc...@lists.ozlabs.org
Subject: Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change
filename from sata_dwc_460ex.c to sata_dwc_4xx.c

Hello.

On 03-04-2012 14:12, Thang Q. Nguyen wrote:

 Signed-off-by: Thang Q. Nguyentqngu...@apm.com
 ---
 Changes for v2:
   - Use git rename feature to change the driver to the newname and
for
 easier review.

   arch/powerpc/boot/dts/bluestone.dts  |   21 +
   drivers/ata/Makefile |2 +-
   drivers/ata/{sata_dwc_460ex.c =  sata_dwc_4xx.c} | 1371
++
   3 files changed, 904 insertions(+), 490 deletions(-)
   rename drivers/ata/{sata_dwc_460ex.c =  sata_dwc_4xx.c} (56%)

You submitted a magapatch doing several things at once (some even
needlessly) and even in two areas of the kernel. This needs proper
splitting/description.

 diff --git a/arch/powerpc/boot/dts/bluestone.dts
b/arch/powerpc/boot/dts/bluestone.dts
 index cfa23bf..803fda6 100644
 --- a/arch/powerpc/boot/dts/bluestone.dts
 +++ b/arch/powerpc/boot/dts/bluestone.dts
 @@ -155,6 +155,27 @@
   /*RXDE*/  0x5 0x4;
   };

 + /* SATA DWC devices */
 + SATA0: sata@bffd1000 {
 + compatible = amcc,sata-apm821xx;
 + reg =4 0xbffd1000 0x800   /* SATA0 */
 +4 0xbffd0800 0x400; /* AHBDMA */
 + dma-channel=0;
 + interrupt-parent =UIC0;
 + interrupts =26 4/* SATA0 */
 +   25 4;  /* AHBDMA */
 + };
 +
 + SATA1: sata@bffd1800 {
 + compatible = amcc,sata-apm821xx;
 + reg =4 0xbffd1800 0x800   /* SATA1 */
 +4 0xbffd0800 0x400; /* AHBDMA */
 + dma-channel=1;
 + interrupt-parent =UIC0;
 + interrupts =27 4/* SATA1 */
 +   25 4;  /* AHBDMA */
 + };
 +
   POB0: opb {
   compatible = ibm,opb;
   #address-cells =1;

The above should be in a separate patch for PPC people, of course.

 diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_4xx.c
 similarity index 56%
 rename from drivers/ata/sata_dwc_460ex.c
 rename to drivers/ata/sata_dwc_4xx.c
 index 69f7cde..bdbb35a 100644
 --- a/drivers/ata/sata_dwc_460ex.c
 +++ b/drivers/ata/sata_dwc_4xx.c
 @@ -1,5 +1,5 @@
   /*
 - * drivers/ata/sata_dwc_460ex.c
 + * drivers/ata/sata_dwc_4xx.c

This line should be removed altogether.

*
* Synopsys DesignWare Cores (DWC) SATA host driver
*
[...]
 @@ -135,13 +146,12 @@ enum {
   DMA_CTL_LLP_DSTEN = 0x0800, /* Blk chain enable Dst */
   };

 -#define  DMA_CTL_BLK_TS(size)((size)  0x00FFF)  /* Blk
Transfer size */
 +#define DMA_CTL_BLK_TS(size) ((size)  0x00FFF)  /* Blk Transfer
size */

Avoid random whitespoace changes.

   #define DMA_CHANNEL(ch) (0x0001  (ch))/* Select
channel */
   /* Enable channel */
 -#define  DMA_ENABLE_CHAN(ch) ((0x0001  (ch)) |
\
 -  ((0x1  (ch))  8))
 +#define  DMA_ENABLE_CHAN(ch) (0x0101  (ch))
   /* Disable channel */
 -#define  DMA_DISABLE_CHAN(ch)(0x | ((0x1
(ch))  8))
 +#define  DMA_DISABLE_CHAN(ch)(0x00100  (ch))
   /* Transfer Type  Flow Controller */

   These cleanups are not related to adding support for 2 channels

 @@ -298,43 +313,32 @@ struct sata_dwc_device_port {
   #define HSDEV_FROM_QC(qc)   ((struct sata_dwc_device *)\
   (qc)-ap-host-private_data)
   #define HSDEV_FROM_HSDEVP(p)((struct sata_dwc_device *)\
 - (hsdevp)-hsdev)
 + 

Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c

2012-04-03 Thread Sergei Shtylyov

Hello.

On 03-04-2012 14:12, Thang Q. Nguyen wrote:


Signed-off-by: Thang Q. Nguyentqngu...@apm.com
---
Changes for v2:
- Use git rename feature to change the driver to the newname and for
  easier review.



  arch/powerpc/boot/dts/bluestone.dts  |   21 +
  drivers/ata/Makefile |2 +-
  drivers/ata/{sata_dwc_460ex.c =  sata_dwc_4xx.c} | 1371 
++
  3 files changed, 904 insertions(+), 490 deletions(-)
  rename drivers/ata/{sata_dwc_460ex.c =  sata_dwc_4xx.c} (56%)


   You submitted a magapatch doing several things at once (some even 
needlessly) and even in two areas of the kernel. This needs proper 
splitting/description.



diff --git a/arch/powerpc/boot/dts/bluestone.dts 
b/arch/powerpc/boot/dts/bluestone.dts
index cfa23bf..803fda6 100644
--- a/arch/powerpc/boot/dts/bluestone.dts
+++ b/arch/powerpc/boot/dts/bluestone.dts
@@ -155,6 +155,27 @@
/*RXDE*/  0x5 0x4;
};

+   /* SATA DWC devices */
+   SATA0: sata@bffd1000 {
+   compatible = amcc,sata-apm821xx;
+   reg =4 0xbffd1000 0x800   /* SATA0 */
+  4 0xbffd0800 0x400; /* AHBDMA */
+   dma-channel=0;
+   interrupt-parent =UIC0;
+   interrupts =26 4/* SATA0 */
+ 25 4;  /* AHBDMA */
+   };
+
+   SATA1: sata@bffd1800 {
+   compatible = amcc,sata-apm821xx;
+   reg =4 0xbffd1800 0x800   /* SATA1 */
+  4 0xbffd0800 0x400; /* AHBDMA */
+   dma-channel=1;
+   interrupt-parent =UIC0;
+   interrupts =27 4/* SATA1 */
+ 25 4;  /* AHBDMA */
+   };
+
POB0: opb {
compatible = ibm,opb;
#address-cells =1;


   The above should be in a separate patch for PPC people, of course.


diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_4xx.c
similarity index 56%
rename from drivers/ata/sata_dwc_460ex.c
rename to drivers/ata/sata_dwc_4xx.c
index 69f7cde..bdbb35a 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_4xx.c
@@ -1,5 +1,5 @@
  /*
- * drivers/ata/sata_dwc_460ex.c
+ * drivers/ata/sata_dwc_4xx.c


   This line should be removed altogether.


   *
   * Synopsys DesignWare Cores (DWC) SATA host driver
   *

[...]

@@ -135,13 +146,12 @@ enum {
DMA_CTL_LLP_DSTEN = 0x0800, /* Blk chain enable Dst */
  };

-#defineDMA_CTL_BLK_TS(size)((size)  0x00FFF)  /* Blk 
Transfer size */
+#define DMA_CTL_BLK_TS(size)   ((size)  0x00FFF)  /* Blk Transfer 
size */


   Avoid random whitespoace changes.


  #define DMA_CHANNEL(ch)   (0x0001  (ch))  /* Select 
channel */
/* Enable channel */
-#defineDMA_ENABLE_CHAN(ch) ((0x0001  (ch)) | 
  \
-((0x1  (ch))  8))
+#defineDMA_ENABLE_CHAN(ch) (0x0101  (ch))
/* Disable channel */
-#defineDMA_DISABLE_CHAN(ch)(0x | ((0x1  (ch))  
8))
+#defineDMA_DISABLE_CHAN(ch)(0x00100  (ch))
/* Transfer Type  Flow Controller */


  These cleanups are not related to adding support for 2 channels


@@ -298,43 +313,32 @@ struct sata_dwc_device_port {
  #define HSDEV_FROM_QC(qc) ((struct sata_dwc_device *)\
(qc)-ap-host-private_data)
  #define HSDEV_FROM_HSDEVP(p)  ((struct sata_dwc_device *)\
-   (hsdevp)-hsdev)
+   (hsdevp)-hsdev)


   Avoid random whitespoace changes.


+/*
+ * Globals
+ */
+static struct sata_dwc_device *dwc_dev_list[2];
+static struct ahb_dma_regs *sata_dma_regs;


   This assumes that the system only has single controller, doesn't it?


 /*
- * Function: get_burst_length_encode
- * arguments: datalength: length in bytes of data
- * returns value to be programmed in register corresponding to data length
+ * Calculate value to be programmed in register corresponding to data length
  * This value is effectively the log(base 2) of the length
  */
-static  int get_burst_length_encode(int datalength)
+static int get_burst_length_encode(int datalength)


   Is it releated to adding support to 2 ports?


  {
int items = datalength  2;  /* div by 4 to get lword count */

@@ -414,152 +416,205 @@ static  int get_burst_length_encode(int datalength)
return 0;
  }

-static  void clear_chan_interrupts(int c)
+/*
+ * Clear channel interrupt. No interrupt for the specified channel
+ * generated until it is enabled again.
+ */
+static void clear_chan_interrupts(int c)
  {
-