Linux ATA/IDE development
 help / color / mirror / Atom feed
* [GIT PULL] libata fixes for v4.9-rc7
From: Tejun Heo @ 2016-11-28 21:06 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-ide, linux-kernel

Hello, Linus.

The recent changes in ahci MSI handling need one more fix.  Hopefully,
this restores parity with before.  The other two are minor fixes with
both low impact and risk.

Thanks.

The following changes since commit 0ce57f8af1782fd12d3a81872a4ab97244989802:

  ahci: fix the single MSI-X case in ahci_init_one (2016-10-25 11:43:07 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git for-4.9-fixes

for you to fetch changes up to 6929ef385e09c0065b87fda3e7b872a5070ac783:

  ahci: always fall back to single-MSI mode (2016-11-21 11:06:57 -0500)

----------------------------------------------------------------
Christoph Hellwig (1):
      ahci: always fall back to single-MSI mode

Hannes Reinecke (1):
      libata-scsi: Fixup ata_gen_passthru_sense()

Wei Yongjun (1):
      mvsas: fix error return code in mvs_task_prep()

 drivers/ata/ahci.c          | 7 -------
 drivers/ata/libata-scsi.c   | 2 +-
 drivers/scsi/mvsas/mv_sas.c | 4 +++-
 3 files changed, 4 insertions(+), 9 deletions(-)

^ permalink raw reply

* Re: [PATCH] ata: disable port while unloading ATA controller driver
From: Vladimir Zapolskiy @ 2016-11-28 23:51 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Bartlomiej Zolnierkiewicz, linux-ide
In-Reply-To: <20161128183425.GA19096@htj.duckdns.org>

Hello Tejun,

On 11/28/2016 08:34 PM, Tejun Heo wrote:
> Hello, Vladimir.
> 
> On Mon, Nov 28, 2016 at 01:18:56AM +0200, Vladimir Zapolskiy wrote:
>> While removing ATA controller driver ata_port_detach() sets 
>> ATA_PFLAG_UNLOADING flag and charges the error handler, however
>> actual port disabling does not happen due to unset
>> ATA_PFLAG_EH_PENDING flag.
>> 
>> To take care about clean port removal and ATA_PFLAG_EH_PENDING
>> flag setting it is sufficient to replace ata_port_schedule_eh()
>> call with ata_port_freeze().
> 
> Hmm... this explanation doesn't really make sense to me. 
> ATA_PFLAG_EH_PENDING is set by at_eh_set_pending() which is the same 
> for both ata_port_schedule_eh() and ata_port_freeze().

correct, ATA_PFLAG_EH_PENDING is set by ata_eh_set_pending(),
you caused me doubt, and my analysis is crap...

> There gotta me something else going on here.  Any chance you can
> track down why EH isn't running?
> 

I've tested the unmodified master branch with a different kernel config
and on another but similar board (SabreSD) powered by the same iMX6Q
SoC, and I can not reproduce this problem, but I still experience it
on the SabreAuto board, I'll trace the kernel on it over JTAG tomorrow.

Sorry for the noise, the reported problem is unlikely related to ATA
subsystem, because in parallel there are some other funny reported
issues like:

  Unable to handle kernel paging request at virtual address 70732f34
  pgd = e5c04000
  [70732f34] *pgd=00000000
  Internal error: Oops: 5 [#1] SMP ARM
  CPU: 3 PID: 218 Comm: udevd Not tainted 4.9.0-rc7 #5
  Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
  task: e6a48c40 task.stack: e5c02000
  PC is at __lock_acquire+0xd8/0x1ba0
  LR is at 0x1

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH] ata: sata_mv: check for errors when parsing nr-ports from dt
From: Uwe Kleine-König @ 2016-11-29 11:13 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide, kernel

If the nr-ports property is missing ata_host_alloc_pinfo is called with
n_ports = 0. This results in host->ports[0] = NULL which later makes
mv_init_host() oops when dereferencing this pointer.

Instead be a bit more cooperative and fail the probing with an error
message.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/ata/sata_mv.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index efc48bf89d51..823e938c9a78 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4090,7 +4090,20 @@ static int mv_platform_probe(struct platform_device *pdev)
 
 	/* allocate host */
 	if (pdev->dev.of_node) {
-		of_property_read_u32(pdev->dev.of_node, "nr-ports", &n_ports);
+		rc = of_property_read_u32(pdev->dev.of_node, "nr-ports",
+					   &n_ports);
+		if (rc) {
+			dev_err(&pdev->dev,
+				"error parsing nr-ports property: %d\n", rc);
+			return rc;
+		}
+
+		if (n_ports <= 0) {
+			dev_err(&pdev->dev, "nr-ports must be positive: %d\n",
+				n_ports);
+			return -EINVAL;
+		}
+
 		irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
 	} else {
 		mv_platform_data = dev_get_platdata(&pdev->dev);
-- 
2.10.2


^ permalink raw reply related

* Re: [PATCH] ata: sata_mv: check for errors when parsing nr-ports from dt
From: Tejun Heo @ 2016-11-29 16:36 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: linux-ide, kernel
In-Reply-To: <20161129111338.19389-1-u.kleine-koenig@pengutronix.de>

On Tue, Nov 29, 2016 at 12:13:38PM +0100, Uwe Kleine-König wrote:
> If the nr-ports property is missing ata_host_alloc_pinfo is called with
> n_ports = 0. This results in host->ports[0] = NULL which later makes
> mv_init_host() oops when dereferencing this pointer.
> 
> Instead be a bit more cooperative and fail the probing with an error
> message.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied to libata/for-4.9-fixes.

Thanks.

-- 
tejun

^ permalink raw reply

* [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Matthew Whitehead @ 2016-11-29 17:53 UTC (permalink / raw)
  To: linux-ide; +Cc: Matthew Whitehead

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)
+{
+	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)))
 		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);
 
 	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))
+			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);
 module_init(legacy_init);
 module_exit(legacy_exit);
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH] ata: disable port while unloading ATA controller driver
From: Vladimir Zapolskiy @ 2016-11-29 18:54 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Bartlomiej Zolnierkiewicz, linux-ide
In-Reply-To: <a498fb9f-822e-45aa-aac1-c7afae7a44e3@mleia.com>

Hello Tejun,

On 11/29/2016 01:51 AM, Vladimir Zapolskiy wrote:
> Hello Tejun,
> 
> On 11/28/2016 08:34 PM, Tejun Heo wrote:
>> Hello, Vladimir.
>>
>> On Mon, Nov 28, 2016 at 01:18:56AM +0200, Vladimir Zapolskiy wrote:
>>> While removing ATA controller driver ata_port_detach() sets 
>>> ATA_PFLAG_UNLOADING flag and charges the error handler, however
>>> actual port disabling does not happen due to unset
>>> ATA_PFLAG_EH_PENDING flag.
>>>
>>> To take care about clean port removal and ATA_PFLAG_EH_PENDING
>>> flag setting it is sufficient to replace ata_port_schedule_eh()
>>> call with ata_port_freeze().
>>
>> Hmm... this explanation doesn't really make sense to me. 
>> ATA_PFLAG_EH_PENDING is set by at_eh_set_pending() which is the same 
>> for both ata_port_schedule_eh() and ata_port_freeze().
> 
> correct, ATA_PFLAG_EH_PENDING is set by ata_eh_set_pending(),
> you caused me doubt, and my analysis is crap...
> 
>> There gotta me something else going on here.  Any chance you can
>> track down why EH isn't running?
>>
> 
> I've tested the unmodified master branch with a different kernel config
> and on another but similar board (SabreSD) powered by the same iMX6Q
> SoC, and I can not reproduce this problem, but I still experience it
> on the SabreAuto board, I'll trace the kernel on it over JTAG tomorrow.
> 

tracing on the board shows a race between driver initialization and
deinitialization, when async_port_probe() is scheduled after driver
removal, this causes the reported problem.

Since it is a race, it should be possible to fuzz the kernel by
introducing a delay (e.g. in ata_port_probe()) to get enough time
to reproduce the problem reliably and to verify a fix.

imx_ahci_probe()
  ahci_platform_init_host()
    ata_host_alloc_pinfo()
      ata_host_alloc()
        ata_port_alloc()    ---> sets ATA_PFLAG_INITIALIZING flag
          ata_link_init()
          ....
    ahci_host_activate()
      ata_host_activate()
        ata_host_start()
          ata_eh_freeze_port()
        ata_port_desc()
        ata_host_register() ---> schedules async_port_probe()
  ....

*** at this point the driver probe is completed, thus it can be removed ***

ata_platform_remove_one()    ==  imx_ahci_driver.remove()
  ata_port_detach()
    ata_port_schedule_eh()
      ata_std_sched_eh()    ---> return, ATA_PFLAG_EH_PENDING flag is not set
    ata_port_wait_eh()      ---> return, port cleanup work is not done

*** warning is printed out ***

async_port_probe()          ---- scheduled too late
  ata_port_probe()
    __ata_port_probe()      ---> now ATA_PFLAG_INITIALIZING flag unset
      ata_port_schedule_eh()
        ata_std_sched_eh()


It also explains why ata_port_schedule_eh() inside ata_port_detach()
replaced by ata_port_abort() with unconditional ATA_PFLAG_EH_PENDING
flag setting does not produce the warning, but still I'm not sure
that resource and state clean-ups are done correctly under the race.

If you buy this analysis sketch, it may take another day or two for
me to prepare a proper fix, or, if you have enough time and desire,
you may implement the fix on your own.

--
With best wishes,
Vladimir


^ permalink raw reply

* Re: [PATCH] ata: disable port while unloading ATA controller driver
From: Tejun Heo @ 2016-11-29 19:00 UTC (permalink / raw)
  To: Vladimir Zapolskiy; +Cc: Bartlomiej Zolnierkiewicz, linux-ide
In-Reply-To: <09c7866c-ecd4-f48a-5112-6cf3c6786cd9@mleia.com>

Hello, Vladimir.

On Tue, Nov 29, 2016 at 08:54:11PM +0200, Vladimir Zapolskiy wrote:
> tracing on the board shows a race between driver initialization and
> deinitialization, when async_port_probe() is scheduled after driver
> removal, this causes the reported problem.
> 
> Since it is a race, it should be possible to fuzz the kernel by
> introducing a delay (e.g. in ata_port_probe()) to get enough time
> to reproduce the problem reliably and to verify a fix.
> 
> imx_ahci_probe()
>   ahci_platform_init_host()
>     ata_host_alloc_pinfo()
>       ata_host_alloc()
>         ata_port_alloc()    ---> sets ATA_PFLAG_INITIALIZING flag
>           ata_link_init()
>           ....
>     ahci_host_activate()
>       ata_host_activate()
>         ata_host_start()
>           ata_eh_freeze_port()
>         ata_port_desc()
>         ata_host_register() ---> schedules async_port_probe()
>   ....
> 
> *** at this point the driver probe is completed, thus it can be removed ***

Not really.  Is this from the unloading test config?  Control doesn't
get passed to userland until async probings are flushed, so this
shouldn't normally be possible.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Tejun Heo @ 2016-11-29 19:41 UTC (permalink / raw)
  To: Matthew Whitehead; +Cc: linux-ide, Bartlomiej Zolnierkiewicz
In-Reply-To: <1480441982-20992-1-git-send-email-tedheadster@gmail.com>

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.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] ata: disable port while unloading ATA controller driver
From: Vladimir Zapolskiy @ 2016-11-29 20:04 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Bartlomiej Zolnierkiewicz, linux-ide
In-Reply-To: <20161129190025.GC22330@htj.duckdns.org>

On 11/29/2016 09:00 PM, Tejun Heo wrote:
> Hello, Vladimir.
> 
> On Tue, Nov 29, 2016 at 08:54:11PM +0200, Vladimir Zapolskiy wrote:
>> tracing on the board shows a race between driver initialization and
>> deinitialization, when async_port_probe() is scheduled after driver
>> removal, this causes the reported problem.
>>
>> Since it is a race, it should be possible to fuzz the kernel by
>> introducing a delay (e.g. in ata_port_probe()) to get enough time
>> to reproduce the problem reliably and to verify a fix.
>>
>> imx_ahci_probe()
>>   ahci_platform_init_host()
>>     ata_host_alloc_pinfo()
>>       ata_host_alloc()
>>         ata_port_alloc()    ---> sets ATA_PFLAG_INITIALIZING flag
>>           ata_link_init()
>>           ....
>>     ahci_host_activate()
>>       ata_host_activate()
>>         ata_host_start()
>>           ata_eh_freeze_port()
>>         ata_port_desc()
>>         ata_host_register() ---> schedules async_port_probe()
>>   ....
>>
>> *** at this point the driver probe is completed, thus it can be removed ***
> 
> Not really.  Is this from the unloading test config?

Correct, I always get the warning with CONFIG_DEBUG_TEST_DRIVER_REMOVE
option enabled.

I understand that a working solution might be just to disable the
option rather than handle this case, however because it is wanted
to test other drivers for potential errors also (e.g. the same ATA
controller driver regardless of the false positive in the ATA core), 
in my opinion the issue should not be ignored.

> Control doesn't get passed to userland until async probings are
> flushed, so this shouldn't normally be possible.
> 

I'm not an expert in ATA, can you please show me the synchronization
point?

In parallel I'll test the fuzzed kernel with an added artificial
delay in ata_port_probe() to get a runtime confirmation.

--
With best wishes,
Vladimir

^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: tedheadster @ 2016-11-29 20:12 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide, Bartlomiej Zolnierkiewicz
In-Reply-To: <20161129194158.GB4959@htj.duckdns.org>

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

^ permalink raw reply

* Re: [PATCH] ata: disable port while unloading ATA controller driver
From: Tejun Heo @ 2016-11-29 20:44 UTC (permalink / raw)
  To: Vladimir Zapolskiy; +Cc: Bartlomiej Zolnierkiewicz, linux-ide
In-Reply-To: <cf257dae-7811-0338-3b59-050f34c1ae29@mleia.com>

Hello, Vladimir.

On Tue, Nov 29, 2016 at 10:04:14PM +0200, Vladimir Zapolskiy wrote:
> > Not really.  Is this from the unloading test config?
> 
> Correct, I always get the warning with CONFIG_DEBUG_TEST_DRIVER_REMOVE
> option enabled.
> 
> I understand that a working solution might be just to disable the
> option rather than handle this case, however because it is wanted
> to test other drivers for potential errors also (e.g. the same ATA
> controller driver regardless of the false positive in the ATA core), 
> in my opinion the issue should not be ignored.

So, the problem is that CONFIG_DEBUG_TEST_DRIVER_REMOVE is introducing
a condition which isn't otherwise possible, so it's triggering pseudo
bugs.  The solution here is to make CONFIG_DEBUG_TEST_DRIVER_REMOVE
flush async calls before trying to remove the driver.

> > Control doesn't get passed to userland until async probings are
> > flushed, so this shouldn't normally be possible.
> > 
> 
> I'm not an expert in ATA, can you please show me the synchronization
> point?

async_synchronize_full() call in kernel_init() for booting and in
delete_module() for module unloading.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Tejun Heo @ 2016-11-29 20:46 UTC (permalink / raw)
  To: whiteheadm; +Cc: linux-ide, Bartlomiej Zolnierkiewicz
In-Reply-To: <CAP8WD_ZxS0TMzHB+=3Gohw39+S2hQbm-pucuYPYK6JUhFWVd5A@mail.gmail.com>

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.

-- 
tejun

^ permalink raw reply

* Donation
From: Lopez Omar @ 2016-11-29 19:54 UTC (permalink / raw)
  To: linux-ide

Hello, My name is Gloria C. Mackenzie, i have a Monetary Donation to make for less privilege and yourself and your organization, am writing you with a friend's email, please contact me on g_mackenzie@rogers.com

^ permalink raw reply

* Re: [PATCH] ata: disable port while unloading ATA controller driver
From: Vladimir Zapolskiy @ 2016-11-29 22:15 UTC (permalink / raw)
  To: Tejun Heo, Rob Herring; +Cc: Bartlomiej Zolnierkiewicz, linux-ide
In-Reply-To: <20161129204413.GC4959@htj.duckdns.org>

Hello Tejun,

I'm adding Rob (author of TEST_DRIVER_REMOVE) to Cc in case if he
has interest in the topic.

On 11/29/2016 10:44 PM, Tejun Heo wrote:
> Hello, Vladimir.
> 
> On Tue, Nov 29, 2016 at 10:04:14PM +0200, Vladimir Zapolskiy wrote:
>>> Not really.  Is this from the unloading test config?
>>
>> Correct, I always get the warning with CONFIG_DEBUG_TEST_DRIVER_REMOVE
>> option enabled.
>>
>> I understand that a working solution might be just to disable the
>> option rather than handle this case, however because it is wanted
>> to test other drivers for potential errors also (e.g. the same ATA
>> controller driver regardless of the false positive in the ATA core), 
>> in my opinion the issue should not be ignored.
> 
> So, the problem is that CONFIG_DEBUG_TEST_DRIVER_REMOVE is introducing
> a condition which isn't otherwise possible, so it's triggering pseudo
> bugs.  The solution here is to make CONFIG_DEBUG_TEST_DRIVER_REMOVE
> flush async calls before trying to remove the driver.
> 

in case if I haven't tired you out yet, I verified you solution and it
works perfectly, there is no problem in ATA subsystem which I can point
out:

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d76cd97..a4feecf 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -384,6 +384,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 	if (test_remove) {
 		test_remove = false;
 
+		async_synchronize_full();
+
 		if (dev->bus->remove)
 			dev->bus->remove(dev);
 		else if (drv->remove)

In my understanding the code under "if (test_remove)" branch should be
close to the code of __device_release_driver() function, but here it
is slightly different on purpose --- driver_allows_async_probing(drv)
returns false in case of the ATA controller driver(s), here async
probing is not a functional part of a driver, but it is embedded into
the ATA subsystem by means of generic async_port_probe(). Not sure if
__device_release_driver() or driver_allows_async_probing() should be
corrected respecting this case, and hence I'm not going to touch it.

>>> Control doesn't get passed to userland until async probings are
>>> flushed, so this shouldn't normally be possible.
>>>
>>
>> I'm not an expert in ATA, can you please show me the synchronization
>> point?
> 
> async_synchronize_full() call in kernel_init() for booting and in
> delete_module() for module unloading.
> 

Thank you for discussion and support.

--
With best wishes,
Vladimir

^ permalink raw reply related

* Re: [PATCH] ata: disable port while unloading ATA controller driver
From: Tejun Heo @ 2016-11-29 22:29 UTC (permalink / raw)
  To: Vladimir Zapolskiy; +Cc: Rob Herring, Bartlomiej Zolnierkiewicz, linux-ide
In-Reply-To: <b33224b4-0f20-9c8d-38e3-2e6b2e8882a9@mleia.com>

Hello,

On Wed, Nov 30, 2016 at 12:15:50AM +0200, Vladimir Zapolskiy wrote:
> in case if I haven't tired you out yet, I verified you solution and it
> works perfectly, there is no problem in ATA subsystem which I can point
> out:
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index d76cd97..a4feecf 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -384,6 +384,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>  	if (test_remove) {
>  		test_remove = false;
>  
> +		async_synchronize_full();
> +
>  		if (dev->bus->remove)
>  			dev->bus->remove(dev);
>  		else if (drv->remove)

Yeah, this should do it.

> In my understanding the code under "if (test_remove)" branch should be
> close to the code of __device_release_driver() function, but here it
> is slightly different on purpose --- driver_allows_async_probing(drv)
> returns false in case of the ATA controller driver(s), here async
> probing is not a functional part of a driver, but it is embedded into
> the ATA subsystem by means of generic async_port_probe(). Not sure if
> __device_release_driver() or driver_allows_async_probing() should be
> corrected respecting this case, and hence I'm not going to touch it.

Currently, we depend on the fact that there is guaranteed to be a
synchronization point before unloading, so adding that to test code
seems like the right thing to do to me.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Bartlomiej Zolnierkiewicz @ 2016-11-30 13:06 UTC (permalink / raw)
  To: Tejun Heo; +Cc: whiteheadm, linux-ide
In-Reply-To: <20161129204602.GD4959@htj.duckdns.org>


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


^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Sergei Shtylyov @ 2016-11-30 13:11 UTC (permalink / raw)
  To: Matthew Whitehead, linux-ide
In-Reply-To: <1480441982-20992-1-git-send-email-tedheadster@gmail.com>

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


^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Bartlomiej Zolnierkiewicz @ 2016-11-30 13:12 UTC (permalink / raw)
  To: Matthew Whitehead; +Cc: linux-ide, Tejun Heo
In-Reply-To: <1480441982-20992-1-git-send-email-tedheadster@gmail.com>


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


^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Sergei Shtylyov @ 2016-11-30 13:15 UTC (permalink / raw)
  To: Matthew Whitehead, linux-ide
In-Reply-To: <1480441982-20992-1-git-send-email-tedheadster@gmail.com>

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


^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: One Thousand Gnomes @ 2016-11-30 14:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Matthew Whitehead, linux-ide
In-Reply-To: <84a95190-9e30-81ac-5f23-744243ba7fce@cogentembedded.com>

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

^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: tedheadster @ 2016-11-30 15:03 UTC (permalink / raw)
  To: One Thousand Gnomes; +Cc: Sergei Shtylyov, linux-ide
In-Reply-To: <20161130145322.252f7403@lxorguk.ukuu.org.uk>

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

^ permalink raw reply

* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: tedheadster @ 2016-11-30 17:13 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: Tejun Heo, linux-ide
In-Reply-To: <2368047.lDgGYdnaMc@amdc3058>

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

^ permalink raw reply

* [PATCH,v2] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Matthew Whitehead @ 2016-11-30 17:20 UTC (permalink / raw)
  To: linux-ide, tj, b.zolnierkie, sergei.shtylyov, gnomes; +Cc: Matthew Whitehead
In-Reply-To: <1480440386-20400-1-git-send-email-tedheadster@gmail.com>

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 |   30 ++++++++++++++++++++++++------
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index 4fe9d21..65a33b4 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,16 @@ static __init void probe_qdi_vlb(void)
 	}
 }
 
+static bool port_ignored(int port)
+{
+	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 +1224,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) && !port_ignored(0x1F0))
 		legacy_probe_add(0x1F0, 14, UNKNOWN, 0);
-	if (secondary == 0 || all)
+	if ((secondary == 0 || all) && !port_ignored(0x170))
 		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 (!port_ignored(0x1E8))
+			legacy_probe_add(0x1E8, 11, UNKNOWN, 0);
+		if (!port_ignored(0x168))
+			legacy_probe_add(0x168, 10, UNKNOWN, 0);
+		if (!port_ignored(0x1E0))
+			legacy_probe_add(0x1E0, 8, UNKNOWN, 0);
+		if (!port_ignored(0x160))
+			legacy_probe_add(0x160, 12, UNKNOWN, 0);
 	}
 
 	if (opti82c46x)
@@ -1272,6 +1289,7 @@ 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);
 
 module_init(legacy_init);
 module_exit(legacy_exit);
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH,v2] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Sergei Shtylyov @ 2016-11-30 17:24 UTC (permalink / raw)
  To: Matthew Whitehead, linux-ide, tj, b.zolnierkie, gnomes
In-Reply-To: <1480526456-15797-1-git-send-email-tedheadster@gmail.com>

Hello.

On 11/30/2016 08:20 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 |   30 ++++++++++++++++++++++++------
>  1 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
> index 4fe9d21..65a33b4 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;

    It's ignore_port_count if my grammar serves still. :-)

>
>  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,16 @@ static __init void probe_qdi_vlb(void)
>  	}
>  }
>
> +static bool port_ignored(int port)
> +{
> +	int i;

    Need empty line here.

> +	for (i = 0; i < ignore_ports_count; i++) {
> +		if (port == ignore_ports[i])
> +			return 1;

    s/1/true/.

> +	}
> +	return 0;

    s/0/false/.

[...]

MBR, Sergei


^ permalink raw reply

* [PATCH,v3] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: Matthew Whitehead @ 2016-11-30 18:14 UTC (permalink / raw)
  To: linux-ide, tj, b.zolnierkie, sergei.shtylyov, gnomes; +Cc: Matthew Whitehead
In-Reply-To: <1480440386-20400-1-git-send-email-tedheadster@gmail.com>

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 |   31 +++++++++++++++++++++++++------
 1 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index 4fe9d21..b9b49db 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)
 	}
 }
 
+static bool port_ignored(int port)
+{
+	int i;
+
+	for (i = 0; i < ignore_ports_count; i++) {
+		if (port == ignore_ports[i])
+			return true;
+	}
+	return false;
+}
+
 /**
  *	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) && !port_ignored(0x1F0))
 		legacy_probe_add(0x1F0, 14, UNKNOWN, 0);
-	if (secondary == 0 || all)
+	if ((secondary == 0 || all) && !port_ignored(0x170))
 		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 (!port_ignored(0x1E8))
+			legacy_probe_add(0x1E8, 11, UNKNOWN, 0);
+		if (!port_ignored(0x168))
+			legacy_probe_add(0x168, 10, UNKNOWN, 0);
+		if (!port_ignored(0x1E0))
+			legacy_probe_add(0x1E0, 8, UNKNOWN, 0);
+		if (!port_ignored(0x160))
+			legacy_probe_add(0x160, 12, UNKNOWN, 0);
 	}
 
 	if (opti82c46x)
@@ -1272,6 +1290,7 @@ 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);
 
 module_init(legacy_init);
 module_exit(legacy_exit);
-- 
1.7.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox