diff mbox

ata: hpt366: fix incorrect mask when checking at cmd_high_time

Message ID 1468322179-9398-1-git-send-email-colin.king@canonical.com
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Colin Ian King July 12, 2016, 11:16 a.m. UTC
From: Colin Ian King <colin.king@canonical.com>

According to the HPT366 data sheet, PCI config space dword 0x40-0x43
bits 11:8 specify the primary drive cmd_high_time, however,
currently just 3 bits of the 4 are being used because the mask
is 0x700 and not 0x0f00.  Fix the mask, allowing for the 40MHz clock
to be detected.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/ata/pata_hpt366.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bartlomiej Zolnierkiewicz July 12, 2016, 11:27 a.m. UTC | #1
Hi,

On Tuesday, July 12, 2016 12:16:19 PM Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> According to the HPT366 data sheet, PCI config space dword 0x40-0x43
> bits 11:8 specify the primary drive cmd_high_time, however,
> currently just 3 bits of the 4 are being used because the mask
> is 0x700 and not 0x0f00.  Fix the mask, allowing for the 40MHz clock
> to be detected.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Thanks for the patch, could you also fix also the old driver
(drivers/ide/hpt366.c)?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> ---
>  drivers/ata/pata_hpt366.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/pata_hpt366.c b/drivers/ata/pata_hpt366.c
> index e5fb752..a219a50 100644
> --- a/drivers/ata/pata_hpt366.c
> +++ b/drivers/ata/pata_hpt366.c
> @@ -368,7 +368,7 @@ static int hpt36x_init_one(struct pci_dev *dev, const struct pci_device_id *id)
>  
>  	/* PCI clocking determines the ATA timing values to use */
>  	/* info_hpt366 is safe against re-entry so we can scribble on it */
> -	switch ((reg1 & 0x700) >> 8) {
> +	switch ((reg1 & 0xf00) >> 8) {
>  	case 9:
>  		hpriv = &hpt366_40;
>  		break;

--
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
Colin Ian King July 12, 2016, 11:33 a.m. UTC | #2
On 12/07/16 12:27, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Tuesday, July 12, 2016 12:16:19 PM Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> According to the HPT366 data sheet, PCI config space dword 0x40-0x43
>> bits 11:8 specify the primary drive cmd_high_time, however,
>> currently just 3 bits of the 4 are being used because the mask
>> is 0x700 and not 0x0f00.  Fix the mask, allowing for the 40MHz clock
>> to be detected.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> 
> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> 
> Thanks for the patch, could you also fix also the old driver
> (drivers/ide/hpt366.c)?

Yep, patch already sent.

https://lkml.org/lkml/2016/7/12/199

> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
>> ---
>>  drivers/ata/pata_hpt366.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ata/pata_hpt366.c b/drivers/ata/pata_hpt366.c
>> index e5fb752..a219a50 100644
>> --- a/drivers/ata/pata_hpt366.c
>> +++ b/drivers/ata/pata_hpt366.c
>> @@ -368,7 +368,7 @@ static int hpt36x_init_one(struct pci_dev *dev, const struct pci_device_id *id)
>>  
>>  	/* PCI clocking determines the ATA timing values to use */
>>  	/* info_hpt366 is safe against re-entry so we can scribble on it */
>> -	switch ((reg1 & 0x700) >> 8) {
>> +	switch ((reg1 & 0xf00) >> 8) {
>>  	case 9:
>>  		hpriv = &hpt366_40;
>>  		break;
> 

--
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
Tejun Heo July 12, 2016, 3:03 p.m. UTC | #3
On Tue, Jul 12, 2016 at 12:16:19PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> According to the HPT366 data sheet, PCI config space dword 0x40-0x43
> bits 11:8 specify the primary drive cmd_high_time, however,
> currently just 3 bits of the 4 are being used because the mask
> is 0x700 and not 0x0f00.  Fix the mask, allowing for the 40MHz clock
> to be detected.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied to libata/for-4.8.

Thanks.
Alan Cox July 18, 2016, 8:14 p.m. UTC | #4
On Tue, 12 Jul 2016 12:16:19 +0100
Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> According to the HPT366 data sheet, PCI config space dword 0x40-0x43
> bits 11:8 specify the primary drive cmd_high_time, however,
> currently just 3 bits of the 4 are being used because the mask
> is 0x700 and not 0x0f00.  Fix the mask, allowing for the 40MHz clock
> to be detected.

Is this tested on real hardware ? I learned long ago never to blindly
trust IDE data sheets. It looks right but if anyone actually has a 40MHz
PCI bus box with an HPT366 it would be good to test.

Alan
--
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/pata_hpt366.c b/drivers/ata/pata_hpt366.c
index e5fb752..a219a50 100644
--- a/drivers/ata/pata_hpt366.c
+++ b/drivers/ata/pata_hpt366.c
@@ -368,7 +368,7 @@  static int hpt36x_init_one(struct pci_dev *dev, const struct pci_device_id *id)
 
 	/* PCI clocking determines the ATA timing values to use */
 	/* info_hpt366 is safe against re-entry so we can scribble on it */
-	switch ((reg1 & 0x700) >> 8) {
+	switch ((reg1 & 0xf00) >> 8) {
 	case 9:
 		hpriv = &hpt366_40;
 		break;