pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems

If there is no PCI bus detected in drivers/ata/pata_legacy.c, it registers all the
common legacy PATA devices. This includes I/O ports (0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160)
and also their associated interrupts (14,15,11,10,8,12).

Unfortunately, on such systems those interrupt lines are at a premium because there is no
PCI alternative. This patch allows you to disable individual port/interrupt pairs by providing
a list of ports to skip allocating.

modprobe pata_legacy ignore_ports=0x1e8,0x168,0x1e0,0x160

Signed-off-by: Matthew Whitehead <tedheadster@gmail.com>
---
 drivers/ata/pata_legacy.c |   32 +++++++++++++++++++++++++-------
 1 files changed, 25 insertions(+), 7 deletions(-)

Hello, Matthew.

(cc'ing Bartlomiej)

On Tue, Nov 29, 2016 at 12:53:02PM -0500, Matthew Whitehead wrote:
> If there is no PCI bus detected in drivers/ata/pata_legacy.c, it registers all the
> common legacy PATA devices. This includes I/O ports (0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160)
> and also their associated interrupts (14,15,11,10,8,12).
> 
> Unfortunately, on such systems those interrupt lines are at a premium because there is no
> PCI alternative. This patch allows you to disable individual port/interrupt pairs by providing
> a list of ports to skip allocating.
> 
> modprobe pata_legacy ignore_ports=0x1e8,0x168,0x1e0,0x160

Can you please give a concrete example of a machine and situation
where this would be useful?

Thanks.

Hello Tejun,

On Tue, Nov 29, 2016 at 2:41 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Matthew.
>
> (cc'ing Bartlomiej)
>
> On Tue, Nov 29, 2016 at 12:53:02PM -0500, Matthew Whitehead wrote:
>> If there is no PCI bus detected in drivers/ata/pata_legacy.c, it registers all the
>> common legacy PATA devices. This includes I/O ports (0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160)
>> and also their associated interrupts (14,15,11,10,8,12).
>>
>> Unfortunately, on such systems those interrupt lines are at a premium because there is no
>> PCI alternative. This patch allows you to disable individual port/interrupt pairs by providing
>> a list of ports to skip allocating.
>>
>> modprobe pata_legacy ignore_ports=0x1e8,0x168,0x1e0,0x160

>
> Can you please give a concrete example of a machine and situation
> where this would be useful?
>


Sure, my regression testing i486 system has this problem. It only has
an EISA bus, no PCI. I had to apply the patch to be able to put even
2-3 cards in it. It had run out of IRQs.

This testing helped uncover a long time kernel bug that now has a patch.

http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fc0e81b2bea0ebceb71889b61d2240856141c9ee

Please check the linux-kernel mailing list thread "What exactly do
32-bit x86 exceptions push on the stack in the CS slot?" for more
details.

https://lkml.org/lkml/2016/11/19/308

- Matthew
--
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

Hello,

On Tue, Nov 29, 2016 at 03:12:31PM -0500, tedheadster wrote:
> > Can you please give a concrete example of a machine and situation
> > where this would be useful?
> >
> 
> 
> Sure, my regression testing i486 system has this problem. It only has
> an EISA bus, no PCI. I had to apply the patch to be able to put even
> 2-3 cards in it. It had run out of IRQs.
> 
> This testing helped uncover a long time kernel bug that now has a patch.
> 
> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fc0e81b2bea0ebceb71889b61d2240856141c9ee
> 
> Please check the linux-kernel mailing list thread "What exactly do
> 32-bit x86 exceptions push on the stack in the CS slot?" for more
> details.
> 
> https://lkml.org/lkml/2016/11/19/308

I see.  Yeah, I don't have any objections to the change although I do
wish it were easier / automatic.  That said, given how niche it is, it
most likely won't matter.  Bartlomiej, what do you think?

Thanks.

Hi,

On Tuesday, November 29, 2016 03:46:02 PM Tejun Heo wrote:
> Hello,
> 
> On Tue, Nov 29, 2016 at 03:12:31PM -0500, tedheadster wrote:
> > > Can you please give a concrete example of a machine and situation
> > > where this would be useful?
> > >
> > 
> > 
> > Sure, my regression testing i486 system has this problem. It only has
> > an EISA bus, no PCI. I had to apply the patch to be able to put even
> > 2-3 cards in it. It had run out of IRQs.
> > 
> > This testing helped uncover a long time kernel bug that now has a patch.
> > 
> > http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=fc0e81b2bea0ebceb71889b61d2240856141c9ee
> > 
> > Please check the linux-kernel mailing list thread "What exactly do
> > 32-bit x86 exceptions push on the stack in the CS slot?" for more
> > details.
> > 
> > https://lkml.org/lkml/2016/11/19/308
> 
> I see.  Yeah, I don't have any objections to the change although I do
> wish it were easier / automatic.  That said, given how niche it is, it
> most likely won't matter.  Bartlomiej, what do you think?

I would also prefer to have such systems detected automatically
(i.e. by using DMI, please check dmidecode output on this board).
If this is not possible I'm fine with the change, though I have some
review comments to the patch itself (please see the other mail).

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

--
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

Hello.

On 11/29/2016 08:53 PM, Matthew Whitehead wrote:

> If there is no PCI bus detected in drivers/ata/pata_legacy.c, it registers all the
> common legacy PATA devices. This includes I/O ports (0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160)
> and also their associated interrupts (14,15,11,10,8,12).
>
> Unfortunately, on such systems those interrupt lines are at a premium because there is no
> PCI alternative. This patch allows you to disable individual port/interrupt pairs by providing
> a list of ports to skip allocating.
>
> modprobe pata_legacy ignore_ports=0x1e8,0x168,0x1e0,0x160
>
> Signed-off-by: Matthew Whitehead <tedheadster@gmail.com>
> ---
>  drivers/ata/pata_legacy.c |   32 +++++++++++++++++++++++++-------
>  1 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
> index 4fe9d21..753c7ce 100644
> --- a/drivers/ata/pata_legacy.c
> +++ b/drivers/ata/pata_legacy.c
> @@ -130,6 +130,8 @@ static struct legacy_data legacy_data[NR_HOST];
>  static struct ata_host *legacy_host[NR_HOST];
>  static int nr_legacy_host;
>
> +static int ignore_ports[NR_HOST];
> +static int ignore_ports_count;
>
>  static int probe_all;		/* Set to check all ISA port ranges */
>  static int ht6560a;		/* HT 6560A on primary 1, second 2, both 3 */
> @@ -1168,6 +1170,17 @@ static __init void probe_qdi_vlb(void)
>  	}
>  }
>
> +int find_port(int port)

    *bool* instead please. And better rename it as port_ignored() I think.

> +{
> +	int i;
> +	for (i = 0 ; i < ignore_ports_count; i++) {
> +		if (port == ignore_ports[i])
> +			return 1;
> +	}
> +	return 0;
> +}
> +		
> +
>  /**
>   *	legacy_init		-	attach legacy interfaces
>   *
> @@ -1212,17 +1225,22 @@ static __init int legacy_init(void)
>  	if (winbond == 1)
>  		winbond = 0x130;	/* Default port, alt is 1B0 */
>
> -	if (primary == 0 || all)
> +	if ((primary == 0 || all) && (!find_port(0x1F0)))

    parens around !find_port() not needed.

>  		legacy_probe_add(0x1F0, 14, UNKNOWN, 0);
> -	if (secondary == 0 || all)
> +	if ((secondary == 0 || all) && (!find_port(0x170)))

    Same here.

>  		legacy_probe_add(0x170, 15, UNKNOWN, 0);
>
>  	if (probe_all || !pci_present) {
>  		/* ISA/VLB extra ports */
> -		legacy_probe_add(0x1E8, 11, UNKNOWN, 0);
> -		legacy_probe_add(0x168, 10, UNKNOWN, 0);
> -		legacy_probe_add(0x1E0, 8, UNKNOWN, 0);
> -		legacy_probe_add(0x160, 12, UNKNOWN, 0);
> +
> +		if (!find_port(0x1E0))

    0x1E8 maybe?

> +			legacy_probe_add(0x1E8, 11, UNKNOWN, 0);
> +		if (!find_port(0x168))
> +			legacy_probe_add(0x168, 10, UNKNOWN, 0);
> +		if (!find_port(0x1E0))
> +			legacy_probe_add(0x1E0, 8, UNKNOWN, 0);
> +		if (!find_port(0x160))
> +			legacy_probe_add(0x160, 12, UNKNOWN, 0);
>  	}
>
>  	if (opti82c46x)
[...]

MBR, Sergei

--
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

Hi,

On Tuesday, November 29, 2016 12:53:02 PM Matthew Whitehead wrote:
> If there is no PCI bus detected in drivers/ata/pata_legacy.c, it registers all the
> common legacy PATA devices. This includes I/O ports (0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160)
> and also their associated interrupts (14,15,11,10,8,12).
> 
> Unfortunately, on such systems those interrupt lines are at a premium because there is no
> PCI alternative. This patch allows you to disable individual port/interrupt pairs by providing
> a list of ports to skip allocating.
> 
> modprobe pata_legacy ignore_ports=0x1e8,0x168,0x1e0,0x160
> 
> Signed-off-by: Matthew Whitehead <tedheadster@gmail.com>
> ---
>  drivers/ata/pata_legacy.c |   32 +++++++++++++++++++++++++-------
>  1 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
> index 4fe9d21..753c7ce 100644
> --- a/drivers/ata/pata_legacy.c
> +++ b/drivers/ata/pata_legacy.c
> @@ -130,6 +130,8 @@ static struct legacy_data legacy_data[NR_HOST];
>  static struct ata_host *legacy_host[NR_HOST];
>  static int nr_legacy_host;
>  
> +static int ignore_ports[NR_HOST];
> +static int ignore_ports_count;
>  
>  static int probe_all;		/* Set to check all ISA port ranges */
>  static int ht6560a;		/* HT 6560A on primary 1, second 2, both 3 */
> @@ -1168,6 +1170,17 @@ static __init void probe_qdi_vlb(void)
>  	}
>  }
>  
> +int find_port(int port)

this should be static

> +{
> +	int i;
> +	for (i = 0 ; i < ignore_ports_count; i++) {
> +		if (port == ignore_ports[i])
> +			return 1;
> +	}
> +	return 0;
> +}
> +		
> +
>  /**
>   *	legacy_init		-	attach legacy interfaces
>   *
> @@ -1212,17 +1225,22 @@ static __init int legacy_init(void)
>  	if (winbond == 1)
>  		winbond = 0x130;	/* Default port, alt is 1B0 */
>  
> -	if (primary == 0 || all)
> +	if ((primary == 0 || all) && (!find_port(0x1F0)))

extra parentheses are not necessary

>  		legacy_probe_add(0x1F0, 14, UNKNOWN, 0);
> -	if (secondary == 0 || all)
> +	if ((secondary == 0 || all) && (!find_port(0x170)))
>  		legacy_probe_add(0x170, 15, UNKNOWN, 0);

ditto

>  	if (probe_all || !pci_present) {
>  		/* ISA/VLB extra ports */
> -		legacy_probe_add(0x1E8, 11, UNKNOWN, 0);
> -		legacy_probe_add(0x168, 10, UNKNOWN, 0);
> -		legacy_probe_add(0x1E0, 8, UNKNOWN, 0);
> -		legacy_probe_add(0x160, 12, UNKNOWN, 0);
> +
> +		if (!find_port(0x1E0))

I believe it should be 0x1E8

> +			legacy_probe_add(0x1E8, 11, UNKNOWN, 0);
> +		if (!find_port(0x168))
> +			legacy_probe_add(0x168, 10, UNKNOWN, 0);
> +		if (!find_port(0x1E0))
> +			legacy_probe_add(0x1E0, 8, UNKNOWN, 0);
> +		if (!find_port(0x160))
> +			legacy_probe_add(0x160, 12, UNKNOWN, 0);
>  	}
>  
>  	if (opti82c46x)
> @@ -1272,6 +1290,6 @@ module_param(qdi, int, 0);
>  module_param(winbond, int, 0);
>  module_param(pio_mask, int, 0);
>  module_param(iordy_mask, int, 0);
> -
> +module_param_array(ignore_ports, int, &ignore_ports_count, 0444);

please leave a newline before module_init()

>  module_init(legacy_init);
>  module_exit(legacy_exit);

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

--
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

On 11/29/2016 08:53 PM, Matthew Whitehead wrote:

> If there is no PCI bus detected in drivers/ata/pata_legacy.c, it registers all the
> common legacy PATA devices. This includes I/O ports (0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160)
> and also their associated interrupts (14,15,11,10,8,12).
>
> Unfortunately, on such systems those interrupt lines are at a premium because there is no
> PCI alternative. This patch allows you to disable individual port/interrupt pairs by providing
> a list of ports to skip allocating.
>
> modprobe pata_legacy ignore_ports=0x1e8,0x168,0x1e0,0x160
>
> Signed-off-by: Matthew Whitehead <tedheadster@gmail.com>

    Also scripts/checkpatch.pl would complain about too long lines if you 
would have run the patch thru it.

MBR, Sergei

--
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

On Wed, 30 Nov 2016 16:11:39 +0300
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:

> Hello.
> 
> On 11/29/2016 08:53 PM, Matthew Whitehead wrote:
> 
> > If there is no PCI bus detected in drivers/ata/pata_legacy.c, it registers all the
> > common legacy PATA devices. This includes I/O ports (0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160)
> > and also their associated interrupts (14,15,11,10,8,12).
> >
> > Unfortunately, on such systems those interrupt lines are at a premium because there is no
> > PCI alternative. This patch allows you to disable individual port/interrupt pairs by providing
> > a list of ports to skip allocating.

In what situation do you actually hit this. The probes should fail so the
interrupt shouldn't end up allocated.

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

On Wed, Nov 30, 2016 at 9:53 AM, One Thousand Gnomes
<gnomes@lxorguk.ukuu.org.uk> wrote:
> On Wed, 30 Nov 2016 16:11:39 +0300
> Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
>
>> Hello.
>>
>> On 11/29/2016 08:53 PM, Matthew Whitehead wrote:
>>
>> > If there is no PCI bus detected in drivers/ata/pata_legacy.c, it registers all the
>> > common legacy PATA devices. This includes I/O ports (0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160)
>> > and also their associated interrupts (14,15,11,10,8,12).
>> >
>> > Unfortunately, on such systems those interrupt lines are at a premium because there is no
>> > PCI alternative. This patch allows you to disable individual port/interrupt pairs by providing
>> > a list of ports to skip allocating.
>
> In what situation do you actually hit this. The probes should fail so the
> interrupt shouldn't end up allocated.
>

Alan,
  on my hardware it grabbed all those interrupts, showing up in
/proc/interrupts very clearly.

- Matthew
--
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

Bartlomiej,

> I would also prefer to have such systems detected automatically
> (i.e. by using DMI, please check dmidecode output on this board).
> If this is not possible I'm fine with the change, though I have some
> review comments to the patch itself (please see the other mail).
>

I will try dmidecode when I return from travel on Friday. I predict it
will have nothing, but I will verify. Assuming that turns out to be
correct, I'm posting my revised patch next.

- Matthew
--
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

On Wed, 30 Nov 2016 10:03:47 -0500
tedheadster <tedheadster@gmail.com> wrote:

> On Wed, Nov 30, 2016 at 9:53 AM, One Thousand Gnomes
> <gnomes@lxorguk.ukuu.org.uk> wrote:
> > On Wed, 30 Nov 2016 16:11:39 +0300
> > Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
> >  
> >> Hello.
> >>
> >> On 11/29/2016 08:53 PM, Matthew Whitehead wrote:
> >>  
> >> > If there is no PCI bus detected in drivers/ata/pata_legacy.c, it registers all the
> >> > common legacy PATA devices. This includes I/O ports (0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160)
> >> > and also their associated interrupts (14,15,11,10,8,12).
> >> >
> >> > Unfortunately, on such systems those interrupt lines are at a premium because there is no
> >> > PCI alternative. This patch allows you to disable individual port/interrupt pairs by providing
> >> > a list of ports to skip allocating.  
> >
> > In what situation do you actually hit this. The probes should fail so the
> > interrupt shouldn't end up allocated.
> >  
> 
> Alan,
>   on my hardware it grabbed all those interrupts, showing up in
> /proc/interrupts very clearly.

Do you have a dmesg of the boot ?

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

Alan,

>> Alan,
>>   on my hardware it grabbed all those interrupts, showing up in
>> /proc/interrupts very clearly.
>
> Do you have a dmesg of the boot ?
>

Here is the relevant dmesg information and also the /proc/interrupts results:

dmesg info:

[   22.534587] scsi host0: pata_legacy
[   22.566618] ata1: PATA max PIO4 cmd 0x1f0 ctl 0x3f6 irq 14
[   22.730639] ata1.00: ATA-4: QUANTUM FIREBALL CR8.4A, A5U.1200, max UDMA/66
[   22.730639] ata1.00: 16514064 sectors, multi 0: LBA
[   22.730639] ata1.00: configured for PIO
[   22.738601] scsi 0:0:0:0: Direct-Access     ATA      QUANTUM
FIREBALL 1200 PQ: 0 ANSI: 5
[   22.798627] sd 0:0:0:0: [sda] 16514064 512-byte logical blocks:
(8.45 GB/7.87 GiB)
[   22.818640] sd 0:0:0:0: [sda] Write Protect is off
[   22.818640] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
[   22.830629] sd 0:0:0:0: [sda] Write cache: enabled, read cache:
enabled, doesn't support DPO or FUA
[   22.934729]  sda: sda1 sda2 sda3 sda4
[   23.062648] sd 0:0:0:0: [sda] Attached SCSI disk
[   23.162640] scsi host1: pata_legacy
[   23.186648] ata2: PATA max PIO4 cmd 0x170 ctl 0x376 irq 15
[   23.494643] scsi host2: pata_legacy
[   23.518656] ata3: PATA max PIO4 cmd 0x1e8 ctl 0x3ee irq 11
[   23.830691] scsi host3: pata_legacy
[   23.854692] ata4: PATA max PIO4 cmd 0x168 ctl 0x36e irq 10
[   24.086711] genirq: Flags mismatch irq 8. 00000000 (pata_legacy.4)
vs. 00000000 (rtc0)
[   24.190692] scsi host4: pata_legacy
[   24.214697] ata5: PATA max PIO4 cmd 0x160 ctl 0x366 irq 12

/proc/interrupts info :

           CPU0
  0:      49060    XT-PIC  timer
...
 10:          0    XT-PIC  pata_legacy.3
 11:          0    XT-PIC  pata_legacy.2
 12:          0    XT-PIC  pata_legacy.5
 14:      41233    XT-PIC  pata_legacy.0
 15:          0    XT-PIC  pata_legacy.1
...

Tejun: I will build a kernel with your patches today and let you know
the results.

- Matthew
--
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

On Fri, 2 Dec 2016 08:37:04 -0500
tedheadster <tedheadster@gmail.com> wrote:

> Alan,
> 
> >> Alan,
> >>   on my hardware it grabbed all those interrupts, showing up in
> >> /proc/interrupts very clearly.  
> >
> > Do you have a dmesg of the boot ?
> >  
> 
> Here is the relevant dmesg information and also the /proc/interrupts results:

That's very strange. The legacy code should be seeing that no devices are
present and then dropping the controller

        /* Nothing found means we drop the port as its probably not there */

        ret = -ENODEV;
        ata_for_each_dev(dev, &ap->link, ALL) {
                if (!ata_dev_absent(dev)) {
                        legacy_host[probe->slot] = host;
                        ld->platform_dev = pdev;
                        return 0;
                }
        }
        ata_host_detach(host);


so this ought to be triggered and free up the interface and the IRQ

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