pata_atiixp regression in 4.11-rc1

pata_atiixp regression in 4.11-rc1

[Markku Pesonen]

Hello,

While testing Linux 4.11-rc1, I noticed that the IDE port on my
Asrock 980DE3/U3S3 motherboard stopped working.

Reverting commit 5946fdaee4ba449e8fbb5d403e1ed69437f916e8
(pata_atiixp: Don't use unconnected secondary port on SB600/SB700)
made things work again.

Apparently the secondary port is used on this motherboard.

Re: pata_atiixp regression in 4.11-rc1

[Bartlomiej Zolnierkiewicz]

added patch author to Cc:

On Sunday, March 26, 2017 10:31:17 AM Markku Pesonen wrote:
> Hello,
> 
> While testing Linux 4.11-rc1, I noticed that the IDE port on my
> Asrock 980DE3/U3S3 motherboard stopped working.
> 
> Reverting commit 5946fdaee4ba449e8fbb5d403e1ed69437f916e8
> (pata_atiixp: Don't use unconnected secondary port on SB600/SB700)
> made things work again.
> 
> Apparently the secondary port is used on this motherboard.

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

Re: pata_atiixp regression in 4.11-rc1

[Tejun Heo]

Hello,

On Mon, Mar 27, 2017 at 11:22:41AM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> added patch author to Cc:
> 
> On Sunday, March 26, 2017 10:31:17 AM Markku Pesonen wrote:
> > Hello,
> > 
> > While testing Linux 4.11-rc1, I noticed that the IDE port on my
> > Asrock 980DE3/U3S3 motherboard stopped working.
> > 
> > Reverting commit 5946fdaee4ba449e8fbb5d403e1ed69437f916e8
> > (pata_atiixp: Don't use unconnected secondary port on SB600/SB700)
> > made things work again.
> > 
> > Apparently the secondary port is used on this motherboard.

I'm gonna revert the commit for now.

Thanks.

-- 
tejun

Re: pata_atiixp regression in 4.11-rc1

[Tejun Heo]

>From a431ecd2d459da3c91a612061f09eb422ffe78e2 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Mon, 27 Mar 2017 13:52:00 -0400
Subject: [PATCH] Revert "pata_atiixp: Don't use unconnected secondary port on
 SB600/SB700"

This reverts commit 5946fdaee4ba449e8fbb5d403e1ed69437f916e8.

The original commit's assumption that the secondary port is
unconnected turns out to be false.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Markku Pesonen <tourula@gmail.com>
Fixes: 5946fdaee4ba ("pata_atiixp: Don't use unconnected secondary port on SB600/SB700")
Cc: Darren Stevens <darren@stevens-zone.net>
---
 drivers/ata/pata_atiixp.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/ata/pata_atiixp.c b/drivers/ata/pata_atiixp.c
index 6c9aa95..49d705c 100644
--- a/drivers/ata/pata_atiixp.c
+++ b/drivers/ata/pata_atiixp.c
@@ -278,11 +278,6 @@ static int atiixp_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	};
 	const struct ata_port_info *ppi[] = { &info, &info };
 
-	/* SB600/700 don't have secondary port wired */
-	if ((pdev->device == PCI_DEVICE_ID_ATI_IXP600_IDE) ||
-		(pdev->device == PCI_DEVICE_ID_ATI_IXP700_IDE))
-		ppi[1] = &ata_dummy_port_info;
-
 	return ata_pci_bmdma_init_one(pdev, ppi, &atiixp_sht, NULL,
 				      ATA_HOST_PARALLEL_SCAN);
 }
-- 
2.9.3

Re: pata_atiixp regression in 4.11-rc1

[Darren Stevens]

Hello Tejun

On 27/03/2017, Tejun Heo wrote:
> From a431ecd2d459da3c91a612061f09eb422ffe78e2 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> Date: Mon, 27 Mar 2017 13:52:00 -0400
> Subject: [PATCH] Revert "pata_atiixp: Don't use unconnected secondary
>  port on SB600/SB700"
>
> This reverts commit 5946fdaee4ba449e8fbb5d403e1ed69437f916e8.
>
> The original commit's assumption that the secondary port is
> unconnected turns out to be false.

Not entirely. I based my patch on information in AMD's document
43366_sb7xx_bdg_pub_1.00.p



> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Markku Pesonen <tourula@gmail.com>
> Fixes: 5946fdaee4ba ("pata_atiixp: Don't use unconnected secondary port
> on SB600/SB700") Cc: Darren Stevens <darren@stevens-zone.net>
> ---
>  drivers/ata/pata_atiixp.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/ata/pata_atiixp.c b/drivers/ata/pata_atiixp.c
> index 6c9aa95..49d705c 100644
> --- a/drivers/ata/pata_atiixp.c
> +++ b/drivers/ata/pata_atiixp.c
> @@ -278,11 +278,6 @@ static int atiixp_init_one(struct pci_dev *pdev,
>  const struct pci_device_id *id)  };
>   const struct ata_port_info *ppi[] = { &info, &info };
>  
> - /* SB600/700 don't have secondary port wired */
> - if ((pdev->device == PCI_DEVICE_ID_ATI_IXP600_IDE) ||
> -     (pdev->device == PCI_DEVICE_ID_ATI_IXP700_IDE))
> -     ppi[1] = &ata_dummy_port_info;
> -
>   return ata_pci_bmdma_init_one(pdev, ppi, &atiixp_sht, NULL,
>                     ATA_HOST_PARALLEL_SCAN);
>  }
Regards

Re: pata_atiixp regression in 4.11-rc1

[Markku Pesonen]

On 28.03.2017 11:24, Darren Stevens wrote:
> Hello Tejun
>
> On 27/03/2017, Tejun Heo wrote:
>> From a431ecd2d459da3c91a612061f09eb422ffe78e2 Mon Sep 17 00:00:00 2001
>> From: Tejun Heo <tj@kernel.org>
>> Date: Mon, 27 Mar 2017 13:52:00 -0400
>> Subject: [PATCH] Revert "pata_atiixp: Don't use unconnected secondary
>>  port on SB600/SB700"
>>
>> This reverts commit 5946fdaee4ba449e8fbb5d403e1ed69437f916e8.
>>
>> The original commit's assumption that the secondary port is
>> unconnected turns out to be false.
>
> Not entirely. I based my patch on information in AMD's document
> 43366_sb7xx_bdg_pub_1.00.p

I found this in the AMD SB710 Databook, rev 1.60, p. 56:
"The integrated parallel ATA controller contains a single channel but
can be configured as primary or secondary channel."

The IDE connector on my Asrock 980DE3/U3S3 motherboard is configured as
the secondary channel. The primary channel is used by the optional
SATA-IDE emulation mode.

I don't know much about kernel programming, southbridges, or ata 
controllers, but I hope this bit of information helps.

Re: pata_atiixp regression in 4.11-rc1

[Tejun Heo]

On Tue, Mar 28, 2017 at 12:57:42PM +0300, Markku Pesonen wrote:
> On 28.03.2017 11:24, Darren Stevens wrote:
> > Hello Tejun
> > 
> > On 27/03/2017, Tejun Heo wrote:
> > > From a431ecd2d459da3c91a612061f09eb422ffe78e2 Mon Sep 17 00:00:00 2001
> > > From: Tejun Heo <tj@kernel.org>
> > > Date: Mon, 27 Mar 2017 13:52:00 -0400
> > > Subject: [PATCH] Revert "pata_atiixp: Don't use unconnected secondary
> > >  port on SB600/SB700"
> > > 
> > > This reverts commit 5946fdaee4ba449e8fbb5d403e1ed69437f916e8.
> > > 
> > > The original commit's assumption that the secondary port is
> > > unconnected turns out to be false.
> > 
> > Not entirely. I based my patch on information in AMD's document
> > 43366_sb7xx_bdg_pub_1.00.p
> 
> I found this in the AMD SB710 Databook, rev 1.60, p. 56:
> "The integrated parallel ATA controller contains a single channel but
> can be configured as primary or secondary channel."
> 
> The IDE connector on my Asrock 980DE3/U3S3 motherboard is configured as
> the secondary channel. The primary channel is used by the optional
> SATA-IDE emulation mode.
> 
> I don't know much about kernel programming, southbridges, or ata
> controllers, but I hope this bit of information helps.

One way or the other, there's a reported case of secondary port being
used, so we can't mark it dummy no matter what the datasheet says.  If
whether the port is marked dummy or not matter much, we can try to do
finer grained blacklisting or try to do something smarter.

Thanks.

-- 
tejun

Re: pata_atiixp regression in 4.11-rc1

[Darren Stevens]

Hello Tejun

On 28/03/2017, Tejun Heo wrote:
> One way or the other, there's a reported case of secondary port being
> used, so we can't mark it dummy no matter what the datasheet says.  If
> whether the port is marked dummy or not matter much, we can try to do
> finer grained blacklisting or try to do something smarter.

The best option is just not to check the SB7xx series (patch to follow), as
AMD added this ability to these chips.

This is my bad, I thought the early SB700 docs I had were correct - sorry.

Regards
Darren

PS I've adjusted my mailer settings, hopefully this one will work.