* Re: Wiki account on ata.wiki.kernel.org?
From: Matthias Peter Walther @ 2017-12-04 20:36 UTC (permalink / raw)
To: Tejun Heo, m.wa; +Cc: linux-ide
In-Reply-To: <20171204202413.GG2421075@devbig577.frc2.facebook.com>
Hello,
okay thanks, I guess you've been the one who unlocked my account today.
So you wouldn't spend the time to update that page?
Where is the in-kernel documentation? I'm rather new to this. Is this
within the kernel's git or where do I find it?
Regards,
Matthias
Am 04.12.2017 um 21:24 schrieb Tejun Heo:
> On Sat, Dec 02, 2017 at 02:54:42PM +0100, Matthias Peter Walther wrote:
>> Hello,
>>
>> does someone know how to get an account on ata.wiki.kernel.org?
>>
>> I signed up for a new account. But it seems that an administrator needs
>> to unlock it, which has never happened in my case (signed up 22th of
>> November).
> So, I think I still have the admin but the wiki hasn't been updated
> for a long time and is really difficult to keep in sync. I'm not sure
> it's worthwhile to maintain it. The right thing to do probably is
> pulling in whatever relevant information to in-kernel documentation
> and killing the wiki.
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH 2/2] ahci: mtk: Change driver name to ahci-mtk
From: Tejun Heo @ 2017-12-04 20:35 UTC (permalink / raw)
To: Matthias Brugger
Cc: linux-ide, linux-arm-kernel, linux-mediatek, linux-kernel,
Matthias Brugger
In-Reply-To: <20171201104722.17353-1-matthias.bgg@gmail.com>
On Fri, Dec 01, 2017 at 11:47:22AM +0100, Matthias Brugger wrote:
> The driver name "ahci" is already used by the ahci platform driver.
> This leads to the following error:
> Error: Driver 'ahci' is already registered, aborting...
>
> Change the name to ahci-mtk to fix this.
>
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Applied to libata/for-4.15-fixes.
Thanks.
--
tejun
^ permalink raw reply
* Re: Wiki account on ata.wiki.kernel.org?
From: Tejun Heo @ 2017-12-04 20:24 UTC (permalink / raw)
To: m.wa; +Cc: linux-ide
In-Reply-To: <c5a9fde7-0950-08bb-bef4-eef563d0890a@uni-muenster.de>
On Sat, Dec 02, 2017 at 02:54:42PM +0100, Matthias Peter Walther wrote:
> Hello,
>
> does someone know how to get an account on ata.wiki.kernel.org?
>
> I signed up for a new account. But it seems that an administrator needs
> to unlock it, which has never happened in my case (signed up 22th of
> November).
So, I think I still have the admin but the wiki hasn't been updated
for a long time and is really difficult to keep in sync. I'm not sure
it's worthwhile to maintain it. The right thing to do probably is
pulling in whatever relevant information to in-kernel documentation
and killing the wiki.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] ahci: qoriq: refine port register configuration
From: Tejun Heo @ 2017-12-04 20:19 UTC (permalink / raw)
To: andy.tang; +Cc: linux-ide, linux-kernel
In-Reply-To: <20171204090120.2142-1-andy.tang@nxp.com>
On Mon, Dec 04, 2017 at 05:01:20PM +0800, andy.tang@nxp.com wrote:
> From: Yuantian Tang <andy.tang@nxp.com>
>
> These PP2C and PP3C registers control the configuration of the PHY
> control OOB timing for the COMINIT/COMWAKE parameters respectively
> for sata port. Overwrite default values with calculated ones to get
> better OOB timing.
>
> Signed-off-by: Tang Yuantian <andy.tang@nxp.com>
Applied to libata/for-4.15-fixes.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH] ahci: qoriq: refine port register configuration
From: andy.tang @ 2017-12-04 9:01 UTC (permalink / raw)
To: tj; +Cc: linux-ide, linux-kernel, Yuantian Tang
From: Yuantian Tang <andy.tang@nxp.com>
These PP2C and PP3C registers control the configuration of the PHY
control OOB timing for the COMINIT/COMWAKE parameters respectively
for sata port. Overwrite default values with calculated ones to get
better OOB timing.
Signed-off-by: Tang Yuantian <andy.tang@nxp.com>
---
drivers/ata/ahci_qoriq.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/ata/ahci_qoriq.c b/drivers/ata/ahci_qoriq.c
index b6b0bf76dfc7..2685f28160f7 100644
--- a/drivers/ata/ahci_qoriq.c
+++ b/drivers/ata/ahci_qoriq.c
@@ -35,6 +35,8 @@
/* port register default value */
#define AHCI_PORT_PHY_1_CFG 0xa003fffe
+#define AHCI_PORT_PHY2_CFG 0x28184d1f
+#define AHCI_PORT_PHY3_CFG 0x0e081509
#define AHCI_PORT_TRANS_CFG 0x08000029
#define AHCI_PORT_AXICC_CFG 0x3fffffff
@@ -183,6 +185,8 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
writel(readl(qpriv->ecc_addr) | ECC_DIS_ARMV8_CH2,
qpriv->ecc_addr);
writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
+ writel(AHCI_PORT_PHY2_CFG, reg_base + PORT_PHY2);
+ writel(AHCI_PORT_PHY3_CFG, reg_base + PORT_PHY3);
writel(AHCI_PORT_TRANS_CFG, reg_base + PORT_TRANS);
if (qpriv->is_dmacoherent)
writel(AHCI_PORT_AXICC_CFG, reg_base + PORT_AXICC);
@@ -190,6 +194,8 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
case AHCI_LS2080A:
writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
+ writel(AHCI_PORT_PHY2_CFG, reg_base + PORT_PHY2);
+ writel(AHCI_PORT_PHY3_CFG, reg_base + PORT_PHY3);
writel(AHCI_PORT_TRANS_CFG, reg_base + PORT_TRANS);
if (qpriv->is_dmacoherent)
writel(AHCI_PORT_AXICC_CFG, reg_base + PORT_AXICC);
@@ -201,6 +207,8 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
writel(readl(qpriv->ecc_addr) | ECC_DIS_ARMV8_CH2,
qpriv->ecc_addr);
writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
+ writel(AHCI_PORT_PHY2_CFG, reg_base + PORT_PHY2);
+ writel(AHCI_PORT_PHY3_CFG, reg_base + PORT_PHY3);
writel(AHCI_PORT_TRANS_CFG, reg_base + PORT_TRANS);
if (qpriv->is_dmacoherent)
writel(AHCI_PORT_AXICC_CFG, reg_base + PORT_AXICC);
@@ -212,6 +220,8 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
writel(readl(qpriv->ecc_addr) | ECC_DIS_LS1088A,
qpriv->ecc_addr);
writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
+ writel(AHCI_PORT_PHY2_CFG, reg_base + PORT_PHY2);
+ writel(AHCI_PORT_PHY3_CFG, reg_base + PORT_PHY3);
writel(AHCI_PORT_TRANS_CFG, reg_base + PORT_TRANS);
if (qpriv->is_dmacoherent)
writel(AHCI_PORT_AXICC_CFG, reg_base + PORT_AXICC);
@@ -219,6 +229,8 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
case AHCI_LS2088A:
writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
+ writel(AHCI_PORT_PHY2_CFG, reg_base + PORT_PHY2);
+ writel(AHCI_PORT_PHY3_CFG, reg_base + PORT_PHY3);
writel(AHCI_PORT_TRANS_CFG, reg_base + PORT_TRANS);
if (qpriv->is_dmacoherent)
writel(AHCI_PORT_AXICC_CFG, reg_base + PORT_AXICC);
--
2.14.1
^ permalink raw reply related
* Wiki account on ata.wiki.kernel.org?
From: Matthias Peter Walther @ 2017-12-02 13:54 UTC (permalink / raw)
To: linux-ide
Hello,
does someone know how to get an account on ata.wiki.kernel.org?
I signed up for a new account. But it seems that an administrator needs
to unlock it, which has never happened in my case (signed up 22th of
November).
I wanted to update this list
(https://ata.wiki.kernel.org/index.php/SATA_hardware_features) with new
cards from this pdf
(https://www.marvell.com/storage/system-solutions/assets/Marvell-88SE92xx-002-product-brief.pdf),
which I own and tested.
Regards,
Matthias
^ permalink raw reply
* [PATCH 2/2] ahci: mtk: Change driver name to ahci-mtk
From: Matthias Brugger @ 2017-12-01 10:47 UTC (permalink / raw)
To: tj
Cc: matthias.bgg, linux-ide, linux-arm-kernel, linux-mediatek,
linux-kernel, Matthias Brugger
The driver name "ahci" is already used by the ahci platform driver.
This leads to the following error:
Error: Driver 'ahci' is already registered, aborting...
Change the name to ahci-mtk to fix this.
Signed-off-by: Matthias Brugger <mbrugger@suse.com>
---
drivers/ata/ahci_mtk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/ahci_mtk.c b/drivers/ata/ahci_mtk.c
index 80854f71559a..22efb9370d1d 100644
--- a/drivers/ata/ahci_mtk.c
+++ b/drivers/ata/ahci_mtk.c
@@ -25,7 +25,7 @@
#include <linux/reset.h>
#include "ahci.h"
-#define DRV_NAME "ahci"
+#define DRV_NAME "ahci-mtk"
#define SYS_CFG 0x14
#define SYS_CFG_SATA_MSK GENMASK(31, 30)
--
2.12.3
^ permalink raw reply related
* Re: AM4 B350 chipset Sata/IDE problem
From: Mark Hounschell @ 2017-11-27 20:50 UTC (permalink / raw)
To: sonofagun, linux-ide
In-Reply-To: <20171123064050.37A674E0025@mta-1.openmailbox.og>
On 11/23/2017 01:40 AM, sonofagun@openmailbox.org wrote:
>
> Hello again, I wonder why my previous mail was wrapped like that. Leafpad and me are to blame!
>
>
>> libata.force=noncq option seems to prevent the messages from occurring. 5 kernel builds (j16) with none.
> I knew it, I was sure!
>
>> [ 2.314441] ata2.00: ATA-7: ST3160815AS, 4.AAB, max UDMA/133
> I suppose that your third ST3160815AS with 4.AAB firmware has no issue, correct?
The one with 4.AAB firmware has issues also:
Nov 27 08:54:32 harley kernel: ata2.00: exception Emask 0x11 SAct 0x7ffbffff SErr 0x400000 action 0x6 frozen
Nov 27 08:54:32 harley kernel: ata2.00: irq_stat 0x48000008, interface fatal error
Nov 27 08:54:32 harley kernel: ata2: SError: { Handshk }
Nov 27 08:54:32 harley kernel: ata2.00: failed command: WRITE FPDMA QUEUED
Nov 27 08:54:32 harley kernel: ata2.00: cmd 61/00:00:08:3b:a6/08:00:0c:00:00/40 tag 0 ncq dma 1048576 ou\x0a res 40/00:e0:98:30:90/00:00:0b:00:00/40 Emask 0x10 (ATA bus error)
Nov 27 08:54:32 harley kernel: ata2.00: status: { DRDY }
Nov 27 08:54:32 harley kernel: ata2.00: failed command: WRITE FPDMA QUEUED
.
.
.
Nov 27 08:54:32 harley kernel: ata2.00: status: { DRDY }
Nov 27 08:54:32 harley kernel: ata2: hard resetting link
Nov 27 08:54:32 harley kernel: ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
Nov 27 08:54:33 harley kernel: ata2.00: configured for UDMA/133
Nov 27 08:54:33 harley kernel: ata2: EH complete
>> [ 2.933706] ata5.00: ATA-7: ST3160811AS, 3.AAE, max UDMA/133
> I suppose that your second ST3160815AS with 3.AAE firmware has no issue, correct?
>
This one (3.AAE) has started acting up, in a totally different way though. So I can't at
this time say one way or the other.
> They were produced in different years but all of them have no firmware update!
> If you see the datecode on its sticker, the 3.AAD datecode must start from 07 or 08 while the 3.AAE datecode must start from 08 or 09 while the 4.AAB must start from 08, 09 or even 10.
> You must contact seagate and inform them that the older one is incompatible with B350 when NCQ enabled. I bet they already know it but you can try.
>
>> [ 1.721248] ata9.00: ATA-6: ST3500320NS, SN04, max UDMA/133
> I assume that your Barracuda ES.2 has caused no trouble, right?
>
That one is not connected to the B350 MB Sata ports. There are only 4 ports. The 3 160GB disks and one DVD are connected to the B350 MB. My other 4 disks are connected to a pci-e (x1) Sata card. I can connect
the 160GB disks to that Sata card and I have no problems. Only when connected to the B350
chipset do issues arise.
On B350:
[ 1.680027] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) ata1.00: ATA-7: ST3160815AS, 3.AAD, max UDMA/133
[ 2.336040] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300) ata2.00: ATA-7: ST3160815AS, 4.AAB, max UDMA/133
[ 2.976043] ata5: SATA link up 1.5 Gbps (SStatus 123 SControl 300) ata5.00: ATA-7: ST3160811AS, 3.AAE, max UDMA/133
[ 3.608030] ata6: SATA link up 1.5 Gbps (SStatus 113 SControl 300) ata6.00: ATAPI: SONY DVD RW AD-7260S, 1.61, max UDMA/100
On pci-e Sata card:
[ 1.731482] ata9: SATA link up 3.0 Gbps (SStatus 123 SControl 300) ata9.00: ATA-6: ST3500320NS, SN04, max UDMA/133
[ 1.707825] ata10: SATA link up 6.0 Gbps (SStatus 133 SControl 300) ata10.00: ATA-9: Samsung SSD 850 EVO 500GB, EMT02B6Q, max UDMA/133
[ 1.696032] ata11: SATA link up 3.0 Gbps (SStatus 123 SControl 300) ata11.00: ATA-8: ST3500320NS, SN06, max UDMA/133
[ 1.719646] ata12: SATA link up 3.0 Gbps (SStatus 123 SControl 300) ata12.00: ATA-8: ST3500418AS, CC38, max UDMA/133
At one point I had the SSD (ata10.00: ATA-9) connected to the B350 and did a Linux install to it. I had issues with it also. I still have the messages from that too. But these occurred only once.
Then never again until a reboot and another kernel build started. With the other drives, the messages come
as long as I'm building a kernel.
36.040607] ata2.00: exception Emask 0x11 SAct 0x1c00 SErr 0x680100 action 0x6 frozen
[ 36.047214] ata2.00: irq_stat 0x48000008, interface fatal error
[ 36.053772] ata2: SError: { UnrecovData 10B8B BadCRC Handshk }
[ 36.060263] ata2.00: failed command: READ FPDMA QUEUED
[ 36.060265] ata2.00: cmd 60/00:50:a7:07:a9/01:00:02:00:00/40 tag 10 ncq dma 131072 in
res 40/00:60:a7:08:a9/00:00:02:00:00/40 Emask 0x10 (ATA bus error)
[ 36.060266] ata2.00: status: { DRDY }
[ 36.060267] ata2.00: failed command: READ FPDMA QUEUED
[ 36.060269] ata2.00: cmd 60/00:58:4f:d8:a8/01:00:02:00:00/40 tag 11 ncq dma 131072 in
res 40/00:60:a7:08:a9/00:00:02:00:00/40 Emask 0x10 (ATA bus error)
[ 36.060269] ata2.00: status: { DRDY }
[ 36.060270] ata2.00: failed command: READ FPDMA QUEUED
[ 36.060272] ata2.00: cmd 60/00:60:a7:08:a9/01:00:02:00:00/40 tag 12 ncq dma 131072 in
res 40/00:60:a7:08:a9/00:00:02:00:00/40 Emask 0x10 (ATA bus error)
[ 36.060274] ata2.00: status: { DRDY }
[ 36.060277] ata2: hard resetting link
[ 36.529587] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[ 36.537694] ata2.00: supports DRM functions and may not be fully accessible
[ 36.544905] ata2.00: disabling queued TRIM support
[ 36.545526] ata2.00: supports DRM functions and may not be fully accessible
[ 36.552601] ata2.00: disabling queued TRIM support
[ 36.552973] ata2.00: configured for UDMA/133
[ 36.559496] ata2: EH complete[ 36.040607] ata2.00: exception Emask 0x11 SAct 0x1c00 SErr 0x680100 action 0x6 frozen
[ 36.047214] ata2.00: irq_stat 0x48000008, interface fatal error
[ 36.053772] ata2: SError: { UnrecovData 10B8B BadCRC Handshk }
[ 36.060263] ata2.00: failed command: READ FPDMA QUEUED
[ 36.060265] ata2.00: cmd 60/00:50:a7:07:a9/01:00:02:00:00/40 tag 10 ncq dma 131072 in
res 40/00:60:a7:08:a9/00:00:02:00:00/40 Emask 0x10 (ATA bus error)
[ 36.060266] ata2.00: status: { DRDY }
[ 36.060267] ata2.00: failed command: READ FPDMA QUEUED
[ 36.060269] ata2.00: cmd 60/00:58:4f:d8:a8/01:00:02:00:00/40 tag 11 ncq dma 131072 in
res 40/00:60:a7:08:a9/00:00:02:00:00/40 Emask 0x10 (ATA bus error)
[ 36.060269] ata2.00: status: { DRDY }
[ 36.060270] ata2.00: failed command: READ FPDMA QUEUED
[ 36.060272] ata2.00: cmd 60/00:60:a7:08:a9/01:00:02:00:00/40 tag 12 ncq dma 131072 in
res 40/00:60:a7:08:a9/00:00:02:00:00/40 Emask 0x10 (ATA bus error)
[ 36.060274] ata2.00: status: { DRDY }
[ 36.060277] ata2: hard resetting link
[ 36.529587] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[ 36.537694] ata2.00: supports DRM functions and may not be fully accessible
[ 36.544905] ata2.00: disabling queued TRIM support
[ 36.545526] ata2.00: supports DRM functions and may not be fully accessible
[ 36.552601] ata2.00: disabling queued TRIM support
[ 36.552973] ata2.00: configured for UDMA/133
[ 36.559496] ata2: EH complete
After the above, no more messages appear.
> Maybe all 7200.10 with 3.* must have NCQ disabled on AMD SATA controllers. It can be done but that would only do the trick for some affected disks not for all.
> OEMs like HP have different firmware with different versions. A blacklist for 3.* would not detect an HP ST3160815AS with HPF0 firmware.
> The same applies to DELL disks and so on...
>
>
>> [ 1.680035] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> [ 2.248065] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> [ 2.872065] ata5: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> Clearly you are stuck with SATA I speeds. This can be done using two or three ways. I will not say a thing on that.
> My brother read the information you uploaded and reminded me that those disks have a small jumper which can limit the maximum SATA interface speed.
> That was needed to make them work on VT8237 series chipsets.
> I suppose you have used that jumper configuration on all three disks but forgot it since you must have done it around 8 or more years ago...
>
Actually when these 3 disks were on my AM3 Sata-2 MB that jumper was removed. I recently installed
it on the 3 connected to the B350 to see if it would help. It didn't seem to make any difference. If I remember correctly the drives did come with the jumper installed and I removed them somewhere
along the line. The jumpers are in now though.
>
>> 5 Reallocated_Sector_Ct PO--CK 100 100 036 - 0
>> 10 Spin_Retry_Count PO--C- 100 100 097 - 0
>> 187 Reported_Uncorrect -O--CK 100 100 000 - 0
>> 189 High_Fly_Writes -O-RCK 100 100 000 - 0
>> 198 Offline_Uncorrectable ----C- 100 100 000 - 0
> Your disk is in great condition despite its 75675 hours.
> As I said before: "Those disks are very reliable".
>
> I would suggest running all self tests(with NCQ disabled) as the last one occured at 9725 hours. That was 65950 hours ago. It is a very long time.
> I have not seen any disk reaching 50000 operating hours!
>
>
> Can you please measure the speed of all three disks on the Ryzen board with AHCI with queue depth 31, 2 and 1?
>
This is done without NCQ disabled?
#echo 31 > /sys/block/sda/device/queue_depth
#hdparm -tT /dev/sda
/dev/sda:
Timing cached reads: 16758 MB in 2.00 seconds = 8386.77 MB/sec Timing buffered disk reads: 220 MB in 3.01 seconds = 73.05 MB/sec
#echo 2 > /sys/block/sda/device/queue_depth
# hdparm -tT /dev/sda
/dev/sda:
Timing cached reads: 16604 MB in 2.00 seconds = 8309.90 MB/sec Timing buffered disk reads: 220 MB in 3.01 seconds = 73.03 MB/sec
# echo 1 > /sys/block/sda/device/queue_depth
# hdparm -tT /dev/sda
/dev/sda:
Timing cached reads: 17000 MB in 2.00 seconds = 8508.61 MB/sec Timing buffered disk reads: 220 MB in 3.01 seconds = 73.12 MB/sec
# echo 31 > /sys/block/sdb/device/queue_depth # hdparm -tT /dev/sdb
/dev/sdb:
Timing cached reads: 17010 MB in 2.00 seconds = 8512.90 MB/sec Timing buffered disk reads: 224 MB in 3.00 seconds = 74.57 MB/sec
# echo 2 > /sys/block/sdb/device/queue_depth
# hdparm -tT /dev/sdb
/dev/sdb:
Timing cached reads: 16686 MB in 2.00 seconds = 8352.24 MB/sec Timing buffered disk reads: 224 MB in 3.01 seconds = 74.43 MB/sec
# echo 1 > /sys/block/sdb/device/queue_depth
# hdparm -tT /dev/sdb
/dev/sdb:
Timing cached reads: 16958 MB in 2.00 seconds = 8486.99 MB/sec Timing buffered disk reads: 224 MB in 3.01 seconds = 74.45 MB/sec
# echo 31 > /sys/block/sdc/device/queue_depth
# hdparm -tT /dev/sdc
/dev/sdc:
Timing cached reads: 16664 MB in 2.00 seconds = 8339.87 MB/sec
Timing buffered disk reads: 178 MB in 3.04 seconds = 58.55 MB/sec
# echo 2 > /sys/block/sdc/device/queue_depth
# hdparm -tT /dev/sdc
/dev/sdc:
Timing cached reads: 16944 MB in 2.00 seconds = 8480.25 MB/sec
Timing buffered disk reads: 180 MB in 3.02 seconds = 59.68 MB/sec
# echo 1 > /sys/block/sdc/device/queue_depth
# hdparm -tT /dev/sdc
/dev/sdc:
Timing cached reads: 17028 MB in 2.00 seconds = 8522.74 MB/sec
Timing buffered disk reads: 162 MB in 3.01 seconds = 53.85 MB/sec
Thanks
Mark
^ permalink raw reply
* Re: [PATCH] libata: sata_down_spd_limit should return if driver has not recorded sstatus speed
From: David Milburn @ 2017-11-27 20:54 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-ide
In-Reply-To: <20171127191948.GH983427@devbig577.frc2.facebook.com>
Hi Tejun,
On 11/27/2017 01:19 PM, Tejun Heo wrote:
> Hello, David.
>
> On Tue, Nov 14, 2017 at 04:17:25PM -0600, David Milburn wrote:
>> During hotplug, it is possible for 6Gbps link speed to
>> be limited all the way down to 1.5 Gbps which may lead
>> to a slower link speed when drive is re-connected.
>
> Can you please explain the scenario with more details?
Yes, in this particular case (I had a second reporter, but,
they didn't respond to my questions), when disconnecting
the drive, the error handler limits the speed to 1.5 Gpps
by setting bit 4 of the control register. And when the drive
is reconnected the status register show a link speed of 3Gbps,
even though before hot unplug it was running a 6Gbps.
>
>> /* Mask off all speeds higher than or equal to the current
>> - * one. Force 1.5Gbps if current SPD is not available.
>> + * one. At this point if current SPD is not available and we
>> + * previously recorded the link speed from the Status register,
>> + * the driver has already masked off the highest bit so mask
>> + * should already be set to 1 or 0. Otherwise, we should
>> + * not force 1.5Gbps on a link where we have not previously
>> + * recorded speed from Status register, just return in this case.
>> */
>> if (spd > 1)
>> mask &= (1 << (spd - 1)) - 1;
>> else
>> - mask &= 1;
>> + return -EINVAL;
>
> I get that the current behavior might not be ideal in certain
> scenarios but the above seems weird, especially given the we have
> link->sata_spd for cases where we can't access the spd value.
>
Yes, but link->sata_spd is initialized to 0, and the only time the
driver records "link->sata_spd" from the status register is in
ata_eh_reset(). And on hot unplug, the driver sets the mask to 1
dropping the link from 6 to 1.5Gbps. And, if the driver doesn't
downgrade the link speed by writing to the control register, the link
comes back at 6Gbps on hot plug. It maybe particular controller/configs
behave differently since it is not always reproducible.
Thanks,
David
^ permalink raw reply
* Re: [PATCH V3 04/29] ata: deprecate pci_get_bus_and_slot()
From: Tejun Heo @ 2017-11-27 19:50 UTC (permalink / raw)
To: Sinan Kaya
Cc: Bartlomiej Zolnierkiewicz, linux-pci, timur, open list,
open list:LIBATA PATA DRIVERS, linux-arm-msm, intel-gfx,
linux-arm-kernel
In-Reply-To: <1511801886-6753-5-git-send-email-okaya@codeaurora.org>
On Mon, Nov 27, 2017 at 11:57:41AM -0500, Sinan Kaya wrote:
> pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
> where a PCI device is present. This restricts the device drivers to be
> reused for other domain numbers.
>
> Getting ready to remove pci_get_bus_and_slot() function in favor of
> pci_get_domain_bus_and_slot().
>
> Use pci_get_domain_bus_and_slot() and extract the actual domain number
> from the pdev passed in.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
Acked-by: Tejun Heo <tj@kernel.org>
Please feel free to route with the rest of the series.
Thanks.
--
tejun
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH] pata_pdc2027x : make pdc2027x_*_timing structures const
From: Tejun Heo @ 2017-11-27 19:49 UTC (permalink / raw)
To: Arvind Yadav; +Cc: b.zolnierkie, sergei.shtylyov, linux-kernel, linux-ide
In-Reply-To: <568091f7b46ee8c9f15ae5cbfe0b76d3a8c47022.1511608541.git.arvind.yadav.cs@gmail.com>
On Sat, Nov 25, 2017 at 04:47:35PM +0530, Arvind Yadav wrote:
> Make these pdc2027x_*_timing structures const as it is never modified.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Applied to libata/for-4.15-fixes.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] ata: mediatek: Fix typo in module description
From: Tejun Heo @ 2017-11-27 19:27 UTC (permalink / raw)
To: Albert Pool; +Cc: linux-ide, ryder.lee
In-Reply-To: <979fd5c5-4375-9fd7-6f02-4613224a5783@solcon.nl>
On Mon, Nov 20, 2017 at 02:20:09PM +0100, Albert Pool wrote:
> Signed-off-by: Albert Pool <albertpool@solcon.nl>
Applied to libata/for-4.15-fixes.
Thanks.
--
tejun
^ permalink raw reply
* Re: [patch] lewisburg map/pcs register handling
From: Tejun Heo @ 2017-11-27 19:25 UTC (permalink / raw)
To: Peter Chang; +Cc: linux-ide
In-Reply-To: <CAF2xp_HuE+aV3ZhrC0E9CQopYgTQ4AHcK15v248AVgxXn8A0pA@mail.gmail.com>
Hello, Peter.
On Tue, Nov 14, 2017 at 03:55:12PM -0800, Peter Chang wrote:
> From 62ccc6118960204769809a1ea9d162cf2c1c95e1 Mon Sep 17 00:00:00 2001
> From: peter chang <dpf@google.com>
> Date: Tue, 14 Nov 2017 13:43:15 -0800
> Subject: [PATCH] ahci: lewisburg MAP / PCS register handling
>
> registers are now 32-bits and the existing offsets mean that the
> wrong registers are being updated.
>
> Change-Id: I992b31bc9e789f9dfbeb29afeb0b7777e325ea71
Can you please drop Change-Id and add Signed-off-by? Also, the
earlier part of the email where you explained what's going on was
actually easier to understand. Maybe put that in the description?
> - pci_read_config_word(pdev, 0x92, &tmp16);
> - if ((tmp16 & hpriv->port_map) != hpriv->port_map) {
> - tmp16 |= hpriv->port_map;
> - pci_write_config_word(pdev, 0x92, tmp16);
> + if (hpriv->flags & AHCI_HFLAG_32BIT_MAP_PCS)
> + pci_read_config_dword(pdev, 0x94, &tmp32);
> + else {
> + pci_read_config_word(pdev, 0x92, &tmp16);
> + tmp32 = tmp16;
> + }
The coding style says that we always add {} to both if and else bodies.
> + if ((tmp32 & hpriv->port_map) != hpriv->port_map) {
> + tmp32 |= hpriv->port_map;
> + if (hpriv->flags & AHCI_HFLAG_32BIT_MAP_PCS)
> + pci_write_config_dword(pdev, 0x94, tmp32);
> + else {
> + tmp16 = tmp32;
> + pci_write_config_word(pdev, 0x92, tmp16);
> + }
@@ -523,6 +524,24 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
> port_map &= hpriv->mask_port_map;
> }
>
> + if (hpriv->flags & AHCI_HFLAG_PCI_PORT_MAP) {
> + struct pci_dev *pdev = to_pci_dev(dev);
> + u16 disabled;
> +
> + if (hpriv->flags & AHCI_HFLAG_32BIT_MAP_PCS) {
> + u32 tmp;
> +
> + pci_read_config_dword(pdev, 0x90, &tmp);
> + disabled = (tmp >> 16) & 0xffff;
> + } else {
> + pci_read_config_word(pdev, 0x90, &disabled);
> + disabled = (disabled >> 8) & 0xff;
> + }
> +
> + dev_info(dev, "port_map:%x disabled:%x\n", port_map, disabled);
> + port_map &= ~disabled;
> + }
And the patch description doesn't explain the above chunk. Can you
please explain this part too?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] libata: sata_down_spd_limit should return if driver has not recorded sstatus speed
From: Tejun Heo @ 2017-11-27 19:19 UTC (permalink / raw)
To: David Milburn; +Cc: linux-ide
In-Reply-To: <1510697845-58071-1-git-send-email-dmilburn@redhat.com>
Hello, David.
On Tue, Nov 14, 2017 at 04:17:25PM -0600, David Milburn wrote:
> During hotplug, it is possible for 6Gbps link speed to
> be limited all the way down to 1.5 Gbps which may lead
> to a slower link speed when drive is re-connected.
Can you please explain the scenario with more details?
> /* Mask off all speeds higher than or equal to the current
> - * one. Force 1.5Gbps if current SPD is not available.
> + * one. At this point if current SPD is not available and we
> + * previously recorded the link speed from the Status register,
> + * the driver has already masked off the highest bit so mask
> + * should already be set to 1 or 0. Otherwise, we should
> + * not force 1.5Gbps on a link where we have not previously
> + * recorded speed from Status register, just return in this case.
> */
> if (spd > 1)
> mask &= (1 << (spd - 1)) - 1;
> else
> - mask &= 1;
> + return -EINVAL;
I get that the current behavior might not be ideal in certain
scenarios but the above seems weird, especially given the we have
link->sata_spd for cases where we can't access the spd value.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH V3 11/29] Drivers: ide: deprecate pci_get_bus_and_slot()
From: Sinan Kaya @ 2017-11-27 16:57 UTC (permalink / raw)
To: linux-pci, timur
Cc: linux-arm-msm, intel-gfx, open list, open list:IDE SUBSYSTEM,
David S. Miller, linux-arm-kernel
In-Reply-To: <1511801886-6753-1-git-send-email-okaya@codeaurora.org>
pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
where a PCI device is present. This restricts the device drivers to be
reused for other domain numbers.
Getting ready to remove pci_get_bus_and_slot() function in favor of
pci_get_domain_bus_and_slot().
Replace pci_get_bus_and_slot() with pci_get_domain_bus_and_slot()
and extract the domain number from struct pci_dev.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/ide/sl82c105.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/ide/sl82c105.c b/drivers/ide/sl82c105.c
index 8755df3..3300dac 100644
--- a/drivers/ide/sl82c105.c
+++ b/drivers/ide/sl82c105.c
@@ -239,8 +239,9 @@ static u8 sl82c105_bridge_revision(struct pci_dev *dev)
/*
* The bridge should be part of the same device, but function 0.
*/
- bridge = pci_get_bus_and_slot(dev->bus->number,
- PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
+ bridge = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
+ dev->bus->number,
+ PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
if (!bridge)
return -1;
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [PATCH V3 04/29] ata: deprecate pci_get_bus_and_slot()
From: Sinan Kaya @ 2017-11-27 16:57 UTC (permalink / raw)
To: linux-pci, timur
Cc: Bartlomiej Zolnierkiewicz, linux-arm-msm, intel-gfx, open list,
open list:LIBATA PATA DRIVERS, Tejun Heo, linux-arm-kernel
In-Reply-To: <1511801886-6753-1-git-send-email-okaya@codeaurora.org>
pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
where a PCI device is present. This restricts the device drivers to be
reused for other domain numbers.
Getting ready to remove pci_get_bus_and_slot() function in favor of
pci_get_domain_bus_and_slot().
Use pci_get_domain_bus_and_slot() and extract the actual domain number
from the pdev passed in.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/ata/pata_ali.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/pata_ali.c b/drivers/ata/pata_ali.c
index d19cd88..0b122f9 100644
--- a/drivers/ata/pata_ali.c
+++ b/drivers/ata/pata_ali.c
@@ -466,7 +466,8 @@ static void ali_init_chipset(struct pci_dev *pdev)
tmp |= 0x01; /* CD_ROM enable for DMA */
pci_write_config_byte(pdev, 0x53, tmp);
}
- north = pci_get_bus_and_slot(0, PCI_DEVFN(0,0));
+ north = pci_get_domain_bus_and_slot(pci_domain_nr(pdev->bus), 0,
+ PCI_DEVFN(0, 0));
if (north && north->vendor == PCI_VENDOR_ID_AL && ali_isa_bridge) {
/* Configure the ALi bridge logic. For non ALi rely on BIOS.
Set the south bridge enable bit */
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [PATCH] pata_pdc2027x : make pdc2027x_*_timing structures const
From: Arvind Yadav @ 2017-11-25 11:17 UTC (permalink / raw)
To: b.zolnierkie, tj, sergei.shtylyov; +Cc: linux-kernel, linux-ide
Make these pdc2027x_*_timing structures const as it is never modified.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/ata/pata_pdc2027x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index d1e8b63..141bf81 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -82,7 +82,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed
* is issued to the device. However, if the controller clock is 133MHz,
* the following tables must be used.
*/
-static struct pdc2027x_pio_timing {
+static const struct pdc2027x_pio_timing {
u8 value0, value1, value2;
} pdc2027x_pio_timing_tbl[] = {
{ 0xfb, 0x2b, 0xac }, /* PIO mode 0 */
@@ -92,7 +92,7 @@ static struct pdc2027x_pio_timing {
{ 0x23, 0x09, 0x25 }, /* PIO mode 4, IORDY on, Prefetch off */
};
-static struct pdc2027x_mdma_timing {
+static const struct pdc2027x_mdma_timing {
u8 value0, value1;
} pdc2027x_mdma_timing_tbl[] = {
{ 0xdf, 0x5f }, /* MDMA mode 0 */
@@ -100,7 +100,7 @@ static struct pdc2027x_mdma_timing {
{ 0x69, 0x25 }, /* MDMA mode 2 */
};
-static struct pdc2027x_udma_timing {
+static const struct pdc2027x_udma_timing {
u8 value0, value1, value2;
} pdc2027x_udma_timing_tbl[] = {
{ 0x4a, 0x0f, 0xd5 }, /* UDMA mode 0 */
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] pata_pdc2027x: Fix coding sytle error
From: arvindY @ 2017-11-25 11:10 UTC (permalink / raw)
To: Joe Perches, b.zolnierkie, tj, sergei.shtylyov; +Cc: linux-kernel, linux-ide
In-Reply-To: <1511607744.2503.1.camel@perches.com>
Hi Joe,
On Saturday 25 November 2017 04:32 PM, Joe Perches wrote:
> On Sat, 2017-11-25 at 16:04 +0530, Arvind Yadav wrote:
> []
>> diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
> []
>> @@ -84,7 +84,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed
>> */
>> static struct pdc2027x_pio_timing {
>> u8 value0, value1, value2;
>> -} pdc2027x_pio_timing_tbl [] = {
>> +} pdc2027x_pio_timing_tbl[] = {
>> { 0xfb, 0x2b, 0xac }, /* PIO mode 0 */
>> { 0x46, 0x29, 0xa4 }, /* PIO mode 1 */
>> { 0x23, 0x26, 0x64 }, /* PIO mode 2 */
> trivia:
>
> It seems all the <foo>_timing_tbl structs should be const
>
Yes, It should be const. I will push anther patch.
~arvind
^ permalink raw reply
* Re: [PATCH] pata_pdc2027x: Fix coding sytle error
From: Joe Perches @ 2017-11-25 11:02 UTC (permalink / raw)
To: Arvind Yadav, b.zolnierkie, tj, sergei.shtylyov; +Cc: linux-kernel, linux-ide
In-Reply-To: <68e25992911d7c4d6c17b9f31524a614466d9588.1511605703.git.arvind.yadav.cs@gmail.com>
On Sat, 2017-11-25 at 16:04 +0530, Arvind Yadav wrote:
[]
> diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
[]
> @@ -84,7 +84,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed
> */
> static struct pdc2027x_pio_timing {
> u8 value0, value1, value2;
> -} pdc2027x_pio_timing_tbl [] = {
> +} pdc2027x_pio_timing_tbl[] = {
> { 0xfb, 0x2b, 0xac }, /* PIO mode 0 */
> { 0x46, 0x29, 0xa4 }, /* PIO mode 1 */
> { 0x23, 0x26, 0x64 }, /* PIO mode 2 */
trivia:
It seems all the <foo>_timing_tbl structs should be const
^ permalink raw reply
* [PATCH] pata_pdc2027x: Fix coding sytle error
From: Arvind Yadav @ 2017-11-25 10:34 UTC (permalink / raw)
To: b.zolnierkie, tj, sergei.shtylyov; +Cc: linux-kernel, linux-ide
Fix these checkpatch.pl error:
ERROR: space prohibited before open square bracket '['.
ERROR: space prohibited after that '~' (ctx:WxW)
+ mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
ERROR: spaces required around that '?' (ctx:VxW)
+ long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
ERROR: that open brace { should be on the previous line
+ const struct ata_port_info *ppi[] =
+ { &pdc2027x_port_info[board_idx], NULL };
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
changes in v1:
These cheange was a part of '[PATCH] pata_pdc2027x:
Remove unnecessary error check and coding style error'
and got comment 'Please fix the checkpatch.pl errors
in a sperate patch'.
drivers/ata/pata_pdc2027x.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index 6348a83..d1e8b63 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -84,7 +84,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed
*/
static struct pdc2027x_pio_timing {
u8 value0, value1, value2;
-} pdc2027x_pio_timing_tbl [] = {
+} pdc2027x_pio_timing_tbl[] = {
{ 0xfb, 0x2b, 0xac }, /* PIO mode 0 */
{ 0x46, 0x29, 0xa4 }, /* PIO mode 1 */
{ 0x23, 0x26, 0x64 }, /* PIO mode 2 */
@@ -94,7 +94,7 @@ static struct pdc2027x_pio_timing {
static struct pdc2027x_mdma_timing {
u8 value0, value1;
-} pdc2027x_mdma_timing_tbl [] = {
+} pdc2027x_mdma_timing_tbl[] = {
{ 0xdf, 0x5f }, /* MDMA mode 0 */
{ 0x6b, 0x27 }, /* MDMA mode 1 */
{ 0x69, 0x25 }, /* MDMA mode 2 */
@@ -102,7 +102,7 @@ static struct pdc2027x_mdma_timing {
static struct pdc2027x_udma_timing {
u8 value0, value1, value2;
-} pdc2027x_udma_timing_tbl [] = {
+} pdc2027x_udma_timing_tbl[] = {
{ 0x4a, 0x0f, 0xd5 }, /* UDMA mode 0 */
{ 0x3a, 0x0a, 0xd0 }, /* UDMA mode 1 */
{ 0x2a, 0x07, 0xcd }, /* UDMA mode 2 */
@@ -277,7 +277,7 @@ static unsigned long pdc2027x_mode_filter(struct ata_device *adev, unsigned long
ATA_ID_PROD_LEN + 1);
/* If the master is a maxtor in UDMA6 then the slave should not use UDMA 6 */
if (strstr(model_num, "Maxtor") == NULL && pair->dma_mode == XFER_UDMA_6)
- mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
+ mask &= ~(1 << (6 + ATA_SHIFT_UDMA));
return mask;
}
@@ -520,7 +520,7 @@ static void pdc_adjust_pll(struct ata_host *host, long pll_clock, unsigned int b
void __iomem *mmio_base = host->iomap[PDC_MMIO_BAR];
u16 pll_ctl;
long pll_clock_khz = pll_clock / 1000;
- long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
+ long pout_required = board_idx ? PDC_133_MHZ : PDC_100_MHZ;
long ratio = pout_required / pll_clock_khz;
int F, R;
@@ -705,8 +705,8 @@ static int pdc2027x_init_one(struct pci_dev *pdev,
static const unsigned long cmd_offset[] = { 0x17c0, 0x15c0 };
static const unsigned long bmdma_offset[] = { 0x1000, 0x1008 };
unsigned int board_idx = (unsigned int) ent->driver_data;
- const struct ata_port_info *ppi[] =
- { &pdc2027x_port_info[board_idx], NULL };
+ const struct ata_port_info *ppi[] = {
+ &pdc2027x_port_info[board_idx], NULL };
struct ata_host *host;
void __iomem *mmio_base;
int i, rc;
--
2.7.4
^ permalink raw reply related
* [PATCH v3] pata_pdc2027x: Remove unnecessary error check
From: Arvind Yadav @ 2017-11-25 10:19 UTC (permalink / raw)
To: b.zolnierkie, tj, sergei.shtylyov; +Cc: linux-kernel, linux-ide
Here, The function pdc_hardware_init always return zero. So it is not
necessary to check its return value.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
changes in v2 :
Make function return type 'void' instead of 'int.
Add sapce between ':'.
changes in v3 :
Fix the checkpatch.pl errors in a sperate patch.
drivers/ata/pata_pdc2027x.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index 82bfd51..6348a83 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -649,7 +649,7 @@ static long pdc_detect_pll_input_clock(struct ata_host *host)
* @host: target ATA host
* @board_idx: board identifier
*/
-static int pdc_hardware_init(struct ata_host *host, unsigned int board_idx)
+static void pdc_hardware_init(struct ata_host *host, unsigned int board_idx)
{
long pll_clock;
@@ -665,8 +665,6 @@ static int pdc_hardware_init(struct ata_host *host, unsigned int board_idx)
/* Adjust PLL control register */
pdc_adjust_pll(host, pll_clock, board_idx);
-
- return 0;
}
/**
@@ -753,8 +751,7 @@ static int pdc2027x_init_one(struct pci_dev *pdev,
//pci_enable_intx(pdev);
/* initialize adapter */
- if (pdc_hardware_init(host, board_idx) != 0)
- return -EIO;
+ pdc_hardware_init(host, board_idx);
pci_set_master(pdev);
return ata_host_activate(host, pdev->irq, ata_bmdma_interrupt,
@@ -778,8 +775,7 @@ static int pdc2027x_reinit_one(struct pci_dev *pdev)
else
board_idx = PDC_UDMA_133;
- if (pdc_hardware_init(host, board_idx))
- return -EIO;
+ pdc_hardware_init(host, board_idx);
ata_host_resume(host);
return 0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] pata_pdc2027x: Remove unnecessary error check and coding style error.
From: arvindY @ 2017-11-25 10:19 UTC (permalink / raw)
To: Sergei Shtylyov, b.zolnierkie, tj; +Cc: linux-kernel, linux-ide
In-Reply-To: <4dcd6458-f289-57e0-a921-5e1b00d91800@cogentembedded.com>
Hi Sergei,
On Saturday 25 November 2017 03:30 PM, Sergei Shtylyov wrote:
> On 11/25/2017 7:15 AM, Arvind Yadav wrote:
>
>> Here, The function pdc_hardware_init always return zero. So it is not
>> necessary to check its return value.
>>
>> Fix these checkpatch.pl error:
>>
>> ERROR: space prohibited after that '~' (ctx:WxW)
>> + mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
>>
>> ERROR: spaces required around that '?' (ctx:VxW)
>> + long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
>>
>> ERROR: that open brace { should be on the previous line
>> + const struct ata_port_info *ppi[] =
>> + { &pdc2027x_port_info[board_idx], NULL };
>>
>
> Please fix the checkpatch.pl errors in a sperate patch.
Please find a patch v3, which does not include checkpatch.pl error fix.
>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> [...]
>
> MBR, Sergei
~arvind
^ permalink raw reply
* Re: [PATCH] pata_pdc2027x: Remove unnecessary error check and coding style error.
From: Sergei Shtylyov @ 2017-11-25 10:00 UTC (permalink / raw)
To: Arvind Yadav, b.zolnierkie, tj; +Cc: linux-kernel, linux-ide
In-Reply-To: <9c2e4c1081be44da4460072ae6129d29c7e6d6bc.1511583254.git.arvind.yadav.cs@gmail.com>
On 11/25/2017 7:15 AM, Arvind Yadav wrote:
> Here, The function pdc_hardware_init always return zero. So it is not
> necessary to check its return value.
>
> Fix these checkpatch.pl error:
>
> ERROR: space prohibited after that '~' (ctx:WxW)
> + mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
>
> ERROR: spaces required around that '?' (ctx:VxW)
> + long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
>
> ERROR: that open brace { should be on the previous line
> + const struct ata_port_info *ppi[] =
> + { &pdc2027x_port_info[board_idx], NULL };
>
Please fix the checkpatch.pl errors in a sperate patch.
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
[...]
MBR, Sergei
^ permalink raw reply
* [PATCH v2] pata_pdc2027x: Remove unnecessary error check and coding style error.
From: Arvind Yadav @ 2017-11-25 4:50 UTC (permalink / raw)
To: b.zolnierkie, tj, joe; +Cc: linux-kernel, linux-ide
Here, The function pdc_hardware_init always return zero. So it is not
necessary to check its return value. So make the function return void.
Fix these checkpatch.pl error:
ERROR: space prohibited after that '~' (ctx:WxW)
+ mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
ERROR: spaces required around that '?' (ctx:VxW)
+ long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
ERROR: that open brace { should be on the previous line
+ const struct ata_port_info *ppi[] =
+ { &pdc2027x_port_info[board_idx], NULL };
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
changes in v2 :
Make function return type 'void' instead of 'int.
Add sapce between ':'.
drivers/ata/pata_pdc2027x.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index ffd8d33..d1e8b63 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -277,7 +277,7 @@ static unsigned long pdc2027x_mode_filter(struct ata_device *adev, unsigned long
ATA_ID_PROD_LEN + 1);
/* If the master is a maxtor in UDMA6 then the slave should not use UDMA 6 */
if (strstr(model_num, "Maxtor") == NULL && pair->dma_mode == XFER_UDMA_6)
- mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
+ mask &= ~(1 << (6 + ATA_SHIFT_UDMA));
return mask;
}
@@ -520,7 +520,7 @@ static void pdc_adjust_pll(struct ata_host *host, long pll_clock, unsigned int b
void __iomem *mmio_base = host->iomap[PDC_MMIO_BAR];
u16 pll_ctl;
long pll_clock_khz = pll_clock / 1000;
- long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
+ long pout_required = board_idx ? PDC_133_MHZ : PDC_100_MHZ;
long ratio = pout_required / pll_clock_khz;
int F, R;
@@ -649,7 +649,7 @@ static long pdc_detect_pll_input_clock(struct ata_host *host)
* @host: target ATA host
* @board_idx: board identifier
*/
-static int pdc_hardware_init(struct ata_host *host, unsigned int board_idx)
+static void pdc_hardware_init(struct ata_host *host, unsigned int board_idx)
{
long pll_clock;
@@ -665,8 +665,6 @@ static int pdc_hardware_init(struct ata_host *host, unsigned int board_idx)
/* Adjust PLL control register */
pdc_adjust_pll(host, pll_clock, board_idx);
-
- return 0;
}
/**
@@ -707,8 +705,8 @@ static int pdc2027x_init_one(struct pci_dev *pdev,
static const unsigned long cmd_offset[] = { 0x17c0, 0x15c0 };
static const unsigned long bmdma_offset[] = { 0x1000, 0x1008 };
unsigned int board_idx = (unsigned int) ent->driver_data;
- const struct ata_port_info *ppi[] =
- { &pdc2027x_port_info[board_idx], NULL };
+ const struct ata_port_info *ppi[] = {
+ &pdc2027x_port_info[board_idx], NULL };
struct ata_host *host;
void __iomem *mmio_base;
int i, rc;
@@ -753,8 +751,7 @@ static int pdc2027x_init_one(struct pci_dev *pdev,
//pci_enable_intx(pdev);
/* initialize adapter */
- if (pdc_hardware_init(host, board_idx) != 0)
- return -EIO;
+ pdc_hardware_init(host, board_idx);
pci_set_master(pdev);
return ata_host_activate(host, pdev->irq, ata_bmdma_interrupt,
@@ -778,8 +775,7 @@ static int pdc2027x_reinit_one(struct pci_dev *pdev)
else
board_idx = PDC_UDMA_133;
- if (pdc_hardware_init(host, board_idx))
- return -EIO;
+ pdc_hardware_init(host, board_idx);
ata_host_resume(host);
return 0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] pata_pdc2027x: Remove unnecessary error check and coding style error.
From: Joe Perches @ 2017-11-25 4:37 UTC (permalink / raw)
To: Arvind Yadav, b.zolnierkie, tj; +Cc: linux-kernel, linux-ide
In-Reply-To: <9c2e4c1081be44da4460072ae6129d29c7e6d6bc.1511583254.git.arvind.yadav.cs@gmail.com>
On Sat, 2017-11-25 at 09:45 +0530, Arvind Yadav wrote:
> Here, The function pdc_hardware_init always return zero. So it is not
> necessary to check its return value.
>
> Fix these checkpatch.pl error:
>
> ERROR: space prohibited after that '~' (ctx:WxW)
> + mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
>
> ERROR: spaces required around that '?' (ctx:VxW)
> + long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
>
> ERROR: that open brace { should be on the previous line
> + const struct ata_port_info *ppi[] =
> + { &pdc2027x_port_info[board_idx], NULL };
[]
> diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
[]
> @@ -520,7 +520,7 @@ static void pdc_adjust_pll(struct ata_host *host, long pll_clock, unsigned int b
> void __iomem *mmio_base = host->iomap[PDC_MMIO_BAR];
> u16 pll_ctl;
> long pll_clock_khz = pll_clock / 1000;
> - long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
> + long pout_required = board_idx ? PDC_133_MHZ:PDC_100_MHZ;
If spaces are used around the ?, use spaces around the : too
> @@ -753,8 +753,7 @@ static int pdc2027x_init_one(struct pci_dev *pdev,
> //pci_enable_intx(pdev);
>
> /* initialize adapter */
> - if (pdc_hardware_init(host, board_idx) != 0)
> - return -EIO;
> + pdc_hardware_init(host, board_idx);
If this is so, then please make the function return void
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox