* [PATCH] PCI: Fix legacy IRQ assignment execution stage
From: Lorenzo Pieralisi @ 2017-09-28 11:37 UTC (permalink / raw)
To: linux-pci
Cc: linux-kernel, linux-arm-kernel, linux-ide, Lorenzo Pieralisi,
Bjorn Helgaas, Richard Henderson, Ivan Kokshaysky,
David S. Miller, Guenter Roeck, Matt Turner
Through struct pci_host_bridge->{map/swizzle}_irq() hooks is now
possible to define IRQ mapping functions on a per PCI host bridge basis.
Actual IRQ allocation is carried out by the pci_assign_irq() function in
pci_device_probe() - to make sure a device is assigned an IRQ only if it
is probed (ie match a driver); it retrieves a struct pci_host_bridge*
for a given PCI device and through {map/swizzle}_irq() hooks it carries
out the PCI IRQ allocation.
As it turned out, some legacy drivers (eg IDE layer) require that a
device allocates IRQ as soon as it is added so that its actual IRQ
settings are available early in the boot process. With current code
calling pci_assign_irq() in pci_device_probe() IDE IRQ probing fails
for some drivers:
ide0: disabled, no IRQ
ide0: failed to initialize IDE interface
ide0: disabling port
cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
cmd64x 0000:00:02.0: can't reserve resources
CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
ide_generic: please use "probe_mask=0x3f" module parameter for probing
all legacy ISA IDE ports
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
sysfs: cannot create duplicate filename '/class/ide_port/ide0'
...
Trace:
[<fffffc00003308a0>] __warn+0x160/0x190
[<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
[<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
[<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
[<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
[<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
[<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
[<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
[<fffffc00005b9d64>] device_add+0x2a4/0x7c0
[<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
[<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
[<fffffc00005ba690>] device_create+0x50/0x70
[<fffffc00005df36c>] ide_host_register+0x48c/0xa00
[<fffffc00005df330>] ide_host_register+0x450/0xa00
[<fffffc00005ba2a0>] device_register+0x20/0x50
[<fffffc00005df330>] ide_host_register+0x450/0xa00
[<fffffc00005df944>] ide_host_add+0x64/0xe0
[<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
[<fffffc0000310288>] do_one_initcall+0x68/0x260
[<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
[<fffffc00007b13a0>] kernel_init+0x0/0x1a0
[<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
[<fffffc00007b13a0>] kernel_init+0x0/0x1a0
---[ end trace 24a70433c3e4d374 ]---
ide0: disabling port
Fix the IRQ allocation issue by moving the pci_assign_irq() call from
pci_device_probe() to pci_device_add(), so that IRQs for a given PCI
device are allocated as soon as it is scanned by the PCI enumeration.
This has the drawback of allocating an IRQ even for PCI devices that
do not have a matching kernel driver but it should be safe to carry
out in all configurations.
Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in
pci_device_probe()")
Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reported-by: Guenter Roeck <linux@roeck-us.net>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: David S. Miller <davem@davemloft.net>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Matt Turner <mattst88@gmail.com>
---
drivers/pci/pci-driver.c | 2 --
drivers/pci/probe.c | 3 +++
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 11bd267..750d688 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -415,8 +415,6 @@ static int pci_device_probe(struct device *dev)
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
- pci_assign_irq(pci_dev);
-
error = pcibios_alloc_irq(pci_dev);
if (error < 0)
return error;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ff94b69..ba4e466 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2103,6 +2103,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
/* Initialize various capabilities */
pci_init_capabilities(dev);
+ /* Assign device IRQ number */
+ pci_assign_irq(dev);
+
/*
* Add the device to our list of discovered devices
* and the bus list for fixup functions, etc.
--
2.4.12
^ permalink raw reply related
* Good day
From: Adams @ 2017-09-28 10:52 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: My dear.JPG --]
[-- Type: image/jpeg, Size: 161604 bytes --]
^ permalink raw reply
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
From: Lorenzo Pieralisi @ 2017-09-28 9:25 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Guenter Roeck, Bjorn Helgaas, linux-kernel@vger.kernel.org,
Richard Henderson, Ivan Kokshaysky, Matt Turner, alpha, linux-pci,
David S. Miller, linux-ide
In-Reply-To: <20170927195502.GA11925@bhelgaas-glaptop.roam.corp.google.com>
On Wed, Sep 27, 2017 at 02:55:02PM -0500, Bjorn Helgaas wrote:
> [+cc linux-pci, linux-ide, DaveM]
>
> On Wed, Sep 27, 2017 at 11:30:34AM +0100, Lorenzo Pieralisi wrote:
> > Bjorn, Guenter,
> >
> > On Wed, Sep 20, 2017 at 12:31:04PM +0100, Lorenzo Pieralisi wrote:
> > > On Tue, Sep 19, 2017 at 11:02:33AM -0700, Guenter Roeck wrote:
> > > > On Tue, Sep 19, 2017 at 10:12:32AM +0100, Lorenzo Pieralisi wrote:
> > > > > On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> > > > > > Hi,
> > > > > >
> > > > > > I see the following runtime warnings in mainline when running alpha images in qemu.
> > > > > >
> > > > > >
> > > > > > Floppy drive(s): fd0 is 2.88M
> > > > > > ide0: disabled, no IRQ
> > > > > > ide0: failed to initialize IDE interface
> > > > > > ide0: disabling port
> > > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
> > > > > > cmd64x 0000:00:02.0: can't reserve resources
> > > > > > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
> > > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > > ------------[ cut here ]------------
> > > > > > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
> > > > > > sysfs: cannot create duplicate filename '/class/ide_port/ide0'
> > > > > > ...
> > > > > >
> > > > > > Trace:
> > > > > > [<fffffc00003308a0>] __warn+0x160/0x190
> > > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
> > > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
> > > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
> > > > > > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0
> > > > > > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
> > > > > > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
> > > > > > [<fffffc00005ba690>] device_create+0x50/0x70
> > > > > > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00
> > > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > > [<fffffc00005ba2a0>] device_register+0x20/0x50
> > > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > > [<fffffc00005df944>] ide_host_add+0x64/0xe0
> > > > > > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
> > > > > > [<fffffc0000310288>] do_one_initcall+0x68/0x260
> > > > > > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
> > > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > > > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
> > > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > > >
> > > > > > ---[ end trace 24a70433c3e4d374 ]---
> > > > > > ide0: disabling port
> > > > > >
> > > > > > [ multiple times ]
> > > > > >
> > > > > > A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
> > > > > >
> > > > > > Prior to the offending commit, the kernel log looks as follows.
> > > > > >
> > > > > > ...
> > > > > > Uniform Multi-Platform E-IDE driver
> > > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > > cmd64x 0000:00:02.0: IDE port disabled
> > > > > > cmd64x 0000:00:02.0: 100% native mode on irq 28
> > > > > > PCI: Setting latency timer of device 0000:00:02.0 to 64
> > > > > > ide0: BM-DMA at 0x8040-0x8047
> > > > > > Floppy drive(s): fd0 is 2.88M
> > > > > > ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> > > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > > ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> > > > > > ide2 at 0x170-0x177,0x376 on irq 15
> > > > > > ide-gd driver 1.18
> > > > > > ide-cd driver 5.00
> > > > > > ...
> > > > > >
> > > > > > Reverting the commit is not possible due to context changes.
> > > > > >
> > > > > > Bisect log is attached.
> > > > >
> > > > > Ok thanks. Can you please check if the diff below fixes the issue ?
> > > > >
> > > > It does. Feel free to add
> > > >
> > > > Tested-by: Guenter Roeck <linux@roeck-us.net>
> > > >
> > > > to the actual patch.
> > > >
> > > > > I think the problem is that now we allocate the IRQ at device_probe
> > > > > instead of device_add time, if the patch below fixes the issue the
> > > > > question is whether we want to move pci_assign_irq() to pci_device_add()
> > > > > for ALL PCI devices or just fix this for IDE subsytem.
> > > > >
> > > >
> > > > That I can not answer ...
> > >
> > > It is not a clean-cut answer. Moving pci_assign_irq() from
> > >
> > > pci_device_probe()
> > >
> > > to
> > >
> > > pci_device_add()
> > >
> > > would fix this issue too and just map/swizzle() irqs for ALL PCI devices
> > > earlier (even ones that are not even probed); it should have no side
> > > effects (famous last words) and would be a cleaner fix.
>
> Hmmm, this is ugly. I *think* this is related to ide_pci_init_two().
> We already call pci_assign_irq() from pci_device_probe(), where we try
> to bind one device to a driver. But ide_pci_init_two() deals with
> *two* devices: the one pci_device_probe() is trying to bind, and
> another random one found via pci_get_slot(). We have not called
> pci_assign_irq() for this second device.
>
> That breaks the model the PCI core expects, where we call the driver
> probe method for device X, and that method deals only with device X.
>
> We can fix this by doing the pci_assign_irq() either in the PCI core's
> device add path or in the driver's probe path (ide_pci_init_two(), as
> in the patch below). Doing it in the PCI core is sort of ugly because
> there's no obvious reason why we should map/swizzle the IRQ if there's
> no driver.
>
> Doing it in the driver is also sort of ugly because this is something
> that should be confined to the core (pci_assign_irq() probably should
> be declared in drivers/pci/pci.h so it's not available outside the
> core).
>
> I think the patch below means we call pci_assign_irq() *twice* on the
> device we're binding: once from pci_device_probe() and again from
> do_ide_setup_pci_device(). And I guess we might call it twice for the
> second device, too: once from do_ide_setup_pci_device() and again if
> pci_device_probe() tries to bind it. It's probably idempotent, but it
> does seem a little ugly.
>
> I guess I agree that calling pci_assign_irq() from pci_device_add() is
> the lesser evil. That will do it unnecessarily in cases where a
> device doesn't have a driver, but it should be safe. It sets
> pdev->irq but probably doesn't touch the device itself.
I will do that, actually this would reinstate the exact boot stage at
which pci_fixup_irqs() was called (except that pci_assign_irq() does
that on a per host-bridge basis and in core code) because
pci_fixup_irqs() was called after enumeration before PCI devices were
added (ie before calling pci_bus_add_devices()), it *should* be safe.
> We do more IRQ setup at pci_enable_device()-time (or probe-time for
> arm64) via acpi_pci_irq_enable(). This also updates pdev->irq. I'd
> rather not do that part at device add-time, because it does things
> like allocate and enable interrupt links, and we shouldn't do that
> until we know we have a driver for the device.
>
> But this is potentially another problem for this "second-device" thing
> IDE does. We've called pci_assign_irq() but not acpi_pci_irq_enable()
> for the second device, so we may still get the wrong IRQ for it.
I have not touched the ACPI irq assignment path with my patches on
purpose. Put it differently, PNP0A03 host bridges do not have any
{map/swizzle}_irq() hook (yet) so pci_assign_irq() does nothing on
them. ACPI based platforms never used pci_fixup_irqs() so I changed
nothing on them (and I doubt I can given how complicated x86 legacy
IRQ assignment code is in arch code).
I will send the fix shortly - even though I am not sure I can add
a sensible Fixes: tag to it, I think:
Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in
pci_device_probe()")
is what makes most sense.
Thanks,
Lorenzo
> > > Fix below is fine but I am not a big fan of it, I would like to
> > > get Bjorn's opinion before I send a fix out.
> >
> > I think that for now the fix below should be ok, I would like to
> > send it out before -rc3, the only question is related to the
> > "Fixes:" tag, I am reluctant to add:
> >
> > Fixes: 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host bridge mapping hooks")
> >
> > since this is not necessarily just an alpha/PCI bug (ie it is related to
> > IDE legacy IRQ routing - which Guenter hit on Alpha), the tag can be
> > misleading in that sense.
> >
> > Please let me know what you think, I will send out the patch then.
> >
> > Thanks,
> > Lorenzo
> >
> > > Thanks,
> > > Lorenzo
> > >
> > > > Guenter
> > > >
> > > > > Lorenzo
> > > > >
> > > > > -- >8 --
> > > > > diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> > > > > index 112d2fe..94ca9cc 100644
> > > > > --- a/drivers/ide/setup-pci.c
> > > > > +++ b/drivers/ide/setup-pci.c
> > > > > @@ -502,6 +502,8 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
> > > > > {
> > > > > int pciirq, ret;
> > > > >
> > > > > + pci_assign_irq(dev);
> > > > > +
> > > > > /*
> > > > > * Can we trust the reported IRQ?
> > > > > */
^ permalink raw reply
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
From: Bjorn Helgaas @ 2017-09-27 19:55 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Guenter Roeck, Bjorn Helgaas, linux-kernel@vger.kernel.org,
Richard Henderson, Ivan Kokshaysky, Matt Turner, alpha, linux-pci,
David S. Miller, linux-ide
In-Reply-To: <20170927103034.GA13763@red-moon>
[+cc linux-pci, linux-ide, DaveM]
On Wed, Sep 27, 2017 at 11:30:34AM +0100, Lorenzo Pieralisi wrote:
> Bjorn, Guenter,
>
> On Wed, Sep 20, 2017 at 12:31:04PM +0100, Lorenzo Pieralisi wrote:
> > On Tue, Sep 19, 2017 at 11:02:33AM -0700, Guenter Roeck wrote:
> > > On Tue, Sep 19, 2017 at 10:12:32AM +0100, Lorenzo Pieralisi wrote:
> > > > On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> > > > > Hi,
> > > > >
> > > > > I see the following runtime warnings in mainline when running alpha images in qemu.
> > > > >
> > > > >
> > > > > Floppy drive(s): fd0 is 2.88M
> > > > > ide0: disabled, no IRQ
> > > > > ide0: failed to initialize IDE interface
> > > > > ide0: disabling port
> > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057]
> > > > > cmd64x 0000:00:02.0: can't reserve resources
> > > > > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16
> > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > ------------[ cut here ]------------
> > > > > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0
> > > > > sysfs: cannot create duplicate filename '/class/ide_port/ide0'
> > > > > ...
> > > > >
> > > > > Trace:
> > > > > [<fffffc00003308a0>] __warn+0x160/0x190
> > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70
> > > > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0
> > > > > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60
> > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0
> > > > > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120
> > > > > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0
> > > > > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170
> > > > > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170
> > > > > [<fffffc00005ba690>] device_create+0x50/0x70
> > > > > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00
> > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > [<fffffc00005ba2a0>] device_register+0x20/0x50
> > > > > [<fffffc00005df330>] ide_host_register+0x450/0xa00
> > > > > [<fffffc00005df944>] ide_host_add+0x64/0xe0
> > > > > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710
> > > > > [<fffffc0000310288>] do_one_initcall+0x68/0x260
> > > > > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0
> > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20
> > > > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0
> > > > >
> > > > > ---[ end trace 24a70433c3e4d374 ]---
> > > > > ide0: disabling port
> > > > >
> > > > > [ multiple times ]
> > > > >
> > > > > A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
> > > > >
> > > > > Prior to the offending commit, the kernel log looks as follows.
> > > > >
> > > > > ...
> > > > > Uniform Multi-Platform E-IDE driver
> > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > cmd64x 0000:00:02.0: IDE port disabled
> > > > > cmd64x 0000:00:02.0: 100% native mode on irq 28
> > > > > PCI: Setting latency timer of device 0000:00:02.0 to 64
> > > > > ide0: BM-DMA at 0x8040-0x8047
> > > > > Floppy drive(s): fd0 is 2.88M
> > > > > ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> > > > > ide2 at 0x170-0x177,0x376 on irq 15
> > > > > ide-gd driver 1.18
> > > > > ide-cd driver 5.00
> > > > > ...
> > > > >
> > > > > Reverting the commit is not possible due to context changes.
> > > > >
> > > > > Bisect log is attached.
> > > >
> > > > Ok thanks. Can you please check if the diff below fixes the issue ?
> > > >
> > > It does. Feel free to add
> > >
> > > Tested-by: Guenter Roeck <linux@roeck-us.net>
> > >
> > > to the actual patch.
> > >
> > > > I think the problem is that now we allocate the IRQ at device_probe
> > > > instead of device_add time, if the patch below fixes the issue the
> > > > question is whether we want to move pci_assign_irq() to pci_device_add()
> > > > for ALL PCI devices or just fix this for IDE subsytem.
> > > >
> > >
> > > That I can not answer ...
> >
> > It is not a clean-cut answer. Moving pci_assign_irq() from
> >
> > pci_device_probe()
> >
> > to
> >
> > pci_device_add()
> >
> > would fix this issue too and just map/swizzle() irqs for ALL PCI devices
> > earlier (even ones that are not even probed); it should have no side
> > effects (famous last words) and would be a cleaner fix.
Hmmm, this is ugly. I *think* this is related to ide_pci_init_two().
We already call pci_assign_irq() from pci_device_probe(), where we try
to bind one device to a driver. But ide_pci_init_two() deals with
*two* devices: the one pci_device_probe() is trying to bind, and
another random one found via pci_get_slot(). We have not called
pci_assign_irq() for this second device.
That breaks the model the PCI core expects, where we call the driver
probe method for device X, and that method deals only with device X.
We can fix this by doing the pci_assign_irq() either in the PCI core's
device add path or in the driver's probe path (ide_pci_init_two(), as
in the patch below). Doing it in the PCI core is sort of ugly because
there's no obvious reason why we should map/swizzle the IRQ if there's
no driver.
Doing it in the driver is also sort of ugly because this is something
that should be confined to the core (pci_assign_irq() probably should
be declared in drivers/pci/pci.h so it's not available outside the
core).
I think the patch below means we call pci_assign_irq() *twice* on the
device we're binding: once from pci_device_probe() and again from
do_ide_setup_pci_device(). And I guess we might call it twice for the
second device, too: once from do_ide_setup_pci_device() and again if
pci_device_probe() tries to bind it. It's probably idempotent, but it
does seem a little ugly.
I guess I agree that calling pci_assign_irq() from pci_device_add() is
the lesser evil. That will do it unnecessarily in cases where a
device doesn't have a driver, but it should be safe. It sets
pdev->irq but probably doesn't touch the device itself.
We do more IRQ setup at pci_enable_device()-time (or probe-time for
arm64) via acpi_pci_irq_enable(). This also updates pdev->irq. I'd
rather not do that part at device add-time, because it does things
like allocate and enable interrupt links, and we shouldn't do that
until we know we have a driver for the device.
But this is potentially another problem for this "second-device" thing
IDE does. We've called pci_assign_irq() but not acpi_pci_irq_enable()
for the second device, so we may still get the wrong IRQ for it.
> > Fix below is fine but I am not a big fan of it, I would like to
> > get Bjorn's opinion before I send a fix out.
>
> I think that for now the fix below should be ok, I would like to
> send it out before -rc3, the only question is related to the
> "Fixes:" tag, I am reluctant to add:
>
> Fixes: 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host bridge mapping hooks")
>
> since this is not necessarily just an alpha/PCI bug (ie it is related to
> IDE legacy IRQ routing - which Guenter hit on Alpha), the tag can be
> misleading in that sense.
>
> Please let me know what you think, I will send out the patch then.
>
> Thanks,
> Lorenzo
>
> > Thanks,
> > Lorenzo
> >
> > > Guenter
> > >
> > > > Lorenzo
> > > >
> > > > -- >8 --
> > > > diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> > > > index 112d2fe..94ca9cc 100644
> > > > --- a/drivers/ide/setup-pci.c
> > > > +++ b/drivers/ide/setup-pci.c
> > > > @@ -502,6 +502,8 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
> > > > {
> > > > int pciirq, ret;
> > > >
> > > > + pci_assign_irq(dev);
> > > > +
> > > > /*
> > > > * Can we trust the reported IRQ?
> > > > */
^ permalink raw reply
* (unknown),
From: dengx @ 2017-09-22 1:55 UTC (permalink / raw)
To: linux-ide
[-- Attachment #1: 240154917498495.doc --]
[-- Type: application/msword, Size: 54325 bytes --]
^ permalink raw reply
* Re: hitting a WARN() in ata_host_activate on 4.13
From: Tejun Heo @ 2017-09-20 22:23 UTC (permalink / raw)
To: Robert Richter, Tycho Andersen; +Cc: linux-ide
In-Reply-To: <20170920021120.ldddwe3pd3ynizjd@smitten>
Hello,
(cc'ing Robert and quoting entire message)
Robert, can you please take a look? Is cavium ahci device expected to
get registered without irq?
Thanks.
On Tue, Sep 19, 2017 at 08:11:20PM -0600, Tycho Andersen wrote:
> Hi all,
>
> I've been playing with a Cavium ThunderX arm64 board on a stock 4.13 kernel and
> I've hit the following warning:
>
> [ 13.784242] ------------[ cut here ]------------
> [ 13.784247] WARNING: CPU: 48 PID: 556 at drivers/ata/libata-core.c:6554 ata_host_activate+0x148/0x160
> [ 13.784248] Modules linked in: drm(+) ahci(+) libahci thunder_bgx(+) thunder_xcv mdio_thunder mdio_cavium
> [ 13.784260] CPU: 48 PID: 556 Comm: kworker/48:1 Tainted: G W 4.13.0+ #2
> [ 13.784261] Hardware name: FOXCONN C2U1N/C2U1N_MB, BIOS G31FB12A 10/26/2016
> [ 13.784265] Workqueue: events work_for_cpu_fn
> [ 13.784268] task: ffff810f87ac4b00 task.stack: ffff810f8ec28000
> [ 13.784270] PC is at ata_host_activate+0x148/0x160
> [ 13.784272] LR is at ata_host_activate+0x70/0x160
> [ 13.784274] pc : [<ffff00000875b378>] lr : [<ffff00000875b2a0>] pstate: 60400045
> [ 13.784275] sp : ffff810f8ec2bbe0
> [ 13.784276] x29: ffff810f8ec2bbe0 x28: ffff000000b26a48
> [ 13.784279] x27: ffff000000b271e8 x26: ffff800f952930a0
> [ 13.784283] x25: 0000000000000001 x24: 0000000000000080
> [ 13.784286] x23: ffff000000b271e8 x22: ffff000000b203d8
> [ 13.784289] x21: 0000000000000000 x20: ffff810f837d0f18
> [ 13.784292] x19: 0000000000000000 x18: ffffffffffffffff
> [ 13.784295] x17: ffff000000a897e8 x16: ffff000000a897e0
> [ 13.784298] x15: ffff0000092f8c08 x14: 0088000000000000
> [ 13.784301] x13: ffff000009481b79 x12: ffff00009317f000
> [ 13.784304] x11: 0000000000000001 x10: 0000000000000001
> [ 13.784307] x9 : 0000000000000000 x8 : ffff000053195500
> [ 13.784310] x7 : 0000000000000000 x6 : 000000000000007f
> [ 13.784313] x5 : 0000000000000080 x4 : ffff810f870c5600
> [ 13.784316] x3 : ffff810f870c5b00 x2 : ffff800f95293330
> [ 13.784319] x1 : 0000000000000040 x0 : 0000000000000000
> [ 13.784323] Call trace:
> [ 13.784325] Exception stack(0xffff810f8ec2b9f0 to 0xffff810f8ec2bb20)
> [ 13.784327] b9e0: 0000000000000000 0001000000000000
> [ 13.784330] ba00: 0000000002359000 ffff00000875b378 0000000060400045 ffff0000086d4780
> [ 13.784333] ba20: ffff810f8ec2ba50 ffff0000086d4808 ffff810f870c5780 0000000000000008
> [ 13.784335] ba40: ffff810f8ec2bad0 ffff0000086c4b0c ffff800f952930a0 0000000000000004
> [ 13.784338] ba60: ffff800f95293170 0000000000000040 ffff0000092f8c08 0000000000016500
> [ 13.784341] ba80: 0000000000000100 ffff800f952930a0 ffff810f8ec2bad0 ffff000000a8591c
> [ 13.784343] baa0: ffff810f837d4000 0000000000040d00 0000000000000000 0000000000000040
> [ 13.784346] bac0: ffff800f95293330 ffff810f870c5b00 ffff810f870c5600 0000000000000080
> [ 13.784349] bae0: 000000000000007f 0000000000000000 ffff000053195500 0000000000000000
> [ 13.784351] bb00: 0000000000000001 0000000000000001 ffff00009317f000 ffff000009481b79
> [ 13.784354] [<ffff00000875b378>] ata_host_activate+0x148/0x160
> [ 13.784365] [<ffff000000a87680>] ahci_host_activate+0x148/0x1c8 [libahci]
> [ 13.784376] [<ffff000000b211b4>] ahci_init_one+0x7d4/0xda0 [ahci]
> [ 13.784379] [<ffff00000856029c>] local_pci_probe+0x44/0xb0
> [ 13.784382] [<ffff0000080ef0c0>] work_for_cpu_fn+0x20/0x30
> [ 13.784385] [<ffff0000080f2850>] process_one_work+0x1e8/0x430
> [ 13.784387] [<ffff0000080f2ce4>] worker_thread+0x24c/0x440
> [ 13.784389] [<ffff0000080f9a08>] kthread+0x108/0x138
> [ 13.784392] [<ffff0000080838e0>] ret_from_fork+0x10/0x30
> [ 13.784393] ---[ end trace 6b8dd2026acce7bc ]---
>
> lspci for the controller is below. Is this the right place to report the bug?
> Any other information I can collect that would be useful?
>
> Cheers,
>
> Tycho
>
> 0001:00:08.0 SATA controller: Cavium, Inc. THUNDERX AHCI SATA Controller (rev 09) (prog-if 01 [AHCI 1.0])
> Subsystem: Cavium, Inc. CN88XX AHCI SATA Controller
> Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
> Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> Latency: 0
> Region 0: [virtual] Memory at 814000000000 (32-bit, non-prefetchable) [size=2M]
> Region 4: [virtual] Memory at 814000200000 (32-bit, non-prefetchable) [size=1M]
> Capabilities: [40] Express (v2) Endpoint, MSI 00
> DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, L1 <1us
> ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
> DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
> RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
> MaxPayload 128 bytes, MaxReadReq 128 bytes
> DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend-
> LnkCap: Port #0, Speed unknown, Width x0, ASPM not supported, Exit Latency L0s <64ns, L1 <1us
> ClockPM- Surprise- LLActRep- BwNot- ASPMOptComp-
> LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk-
> ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
> LnkSta: Speed unknown, Width x0, TrErr- Train- SlotClk- DLActive- BWMgmt- ABWMgmt-
> DevCap2: Completion Timeout: Not Supported, TimeoutDis-, LTR-, OBFF Not Supported
> DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
> LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
> EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
> Capabilities: [80] MSI-X: Enable+ Count=4 Masked-
> Vector table: BAR=4 offset=00000000
> PBA: BAR=4 offset=000f0000
> Capabilities: [98] #14 [0002]
> Capabilities: [100 v1] Alternative Routing-ID Interpretation (ARI)
> ARICap: MFVC- ACS-, Next Function: 0
> ARICtl: MFVC- ACS-, Function Group: 0
> Capabilities: [108 v1] Vendor Specific Information: ID=00a0 Rev=1 Len=040 <?>
> Kernel driver in use: ahci
> Kernel modules: ahci
>
> --
> 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
^ permalink raw reply
* hitting a WARN() in ata_host_activate on 4.13
From: Tycho Andersen @ 2017-09-20 2:11 UTC (permalink / raw)
To: linux-ide
Hi all,
I've been playing with a Cavium ThunderX arm64 board on a stock 4.13 kernel and
I've hit the following warning:
[ 13.784242] ------------[ cut here ]------------
[ 13.784247] WARNING: CPU: 48 PID: 556 at drivers/ata/libata-core.c:6554 ata_host_activate+0x148/0x160
[ 13.784248] Modules linked in: drm(+) ahci(+) libahci thunder_bgx(+) thunder_xcv mdio_thunder mdio_cavium
[ 13.784260] CPU: 48 PID: 556 Comm: kworker/48:1 Tainted: G W 4.13.0+ #2
[ 13.784261] Hardware name: FOXCONN C2U1N/C2U1N_MB, BIOS G31FB12A 10/26/2016
[ 13.784265] Workqueue: events work_for_cpu_fn
[ 13.784268] task: ffff810f87ac4b00 task.stack: ffff810f8ec28000
[ 13.784270] PC is at ata_host_activate+0x148/0x160
[ 13.784272] LR is at ata_host_activate+0x70/0x160
[ 13.784274] pc : [<ffff00000875b378>] lr : [<ffff00000875b2a0>] pstate: 60400045
[ 13.784275] sp : ffff810f8ec2bbe0
[ 13.784276] x29: ffff810f8ec2bbe0 x28: ffff000000b26a48
[ 13.784279] x27: ffff000000b271e8 x26: ffff800f952930a0
[ 13.784283] x25: 0000000000000001 x24: 0000000000000080
[ 13.784286] x23: ffff000000b271e8 x22: ffff000000b203d8
[ 13.784289] x21: 0000000000000000 x20: ffff810f837d0f18
[ 13.784292] x19: 0000000000000000 x18: ffffffffffffffff
[ 13.784295] x17: ffff000000a897e8 x16: ffff000000a897e0
[ 13.784298] x15: ffff0000092f8c08 x14: 0088000000000000
[ 13.784301] x13: ffff000009481b79 x12: ffff00009317f000
[ 13.784304] x11: 0000000000000001 x10: 0000000000000001
[ 13.784307] x9 : 0000000000000000 x8 : ffff000053195500
[ 13.784310] x7 : 0000000000000000 x6 : 000000000000007f
[ 13.784313] x5 : 0000000000000080 x4 : ffff810f870c5600
[ 13.784316] x3 : ffff810f870c5b00 x2 : ffff800f95293330
[ 13.784319] x1 : 0000000000000040 x0 : 0000000000000000
[ 13.784323] Call trace:
[ 13.784325] Exception stack(0xffff810f8ec2b9f0 to 0xffff810f8ec2bb20)
[ 13.784327] b9e0: 0000000000000000 0001000000000000
[ 13.784330] ba00: 0000000002359000 ffff00000875b378 0000000060400045 ffff0000086d4780
[ 13.784333] ba20: ffff810f8ec2ba50 ffff0000086d4808 ffff810f870c5780 0000000000000008
[ 13.784335] ba40: ffff810f8ec2bad0 ffff0000086c4b0c ffff800f952930a0 0000000000000004
[ 13.784338] ba60: ffff800f95293170 0000000000000040 ffff0000092f8c08 0000000000016500
[ 13.784341] ba80: 0000000000000100 ffff800f952930a0 ffff810f8ec2bad0 ffff000000a8591c
[ 13.784343] baa0: ffff810f837d4000 0000000000040d00 0000000000000000 0000000000000040
[ 13.784346] bac0: ffff800f95293330 ffff810f870c5b00 ffff810f870c5600 0000000000000080
[ 13.784349] bae0: 000000000000007f 0000000000000000 ffff000053195500 0000000000000000
[ 13.784351] bb00: 0000000000000001 0000000000000001 ffff00009317f000 ffff000009481b79
[ 13.784354] [<ffff00000875b378>] ata_host_activate+0x148/0x160
[ 13.784365] [<ffff000000a87680>] ahci_host_activate+0x148/0x1c8 [libahci]
[ 13.784376] [<ffff000000b211b4>] ahci_init_one+0x7d4/0xda0 [ahci]
[ 13.784379] [<ffff00000856029c>] local_pci_probe+0x44/0xb0
[ 13.784382] [<ffff0000080ef0c0>] work_for_cpu_fn+0x20/0x30
[ 13.784385] [<ffff0000080f2850>] process_one_work+0x1e8/0x430
[ 13.784387] [<ffff0000080f2ce4>] worker_thread+0x24c/0x440
[ 13.784389] [<ffff0000080f9a08>] kthread+0x108/0x138
[ 13.784392] [<ffff0000080838e0>] ret_from_fork+0x10/0x30
[ 13.784393] ---[ end trace 6b8dd2026acce7bc ]---
lspci for the controller is below. Is this the right place to report the bug?
Any other information I can collect that would be useful?
Cheers,
Tycho
0001:00:08.0 SATA controller: Cavium, Inc. THUNDERX AHCI SATA Controller (rev 09) (prog-if 01 [AHCI 1.0])
Subsystem: Cavium, Inc. CN88XX AHCI SATA Controller
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0
Region 0: [virtual] Memory at 814000000000 (32-bit, non-prefetchable) [size=2M]
Region 4: [virtual] Memory at 814000200000 (32-bit, non-prefetchable) [size=1M]
Capabilities: [40] Express (v2) Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, L1 <1us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 128 bytes
DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend-
LnkCap: Port #0, Speed unknown, Width x0, ASPM not supported, Exit Latency L0s <64ns, L1 <1us
ClockPM- Surprise- LLActRep- BwNot- ASPMOptComp-
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk-
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed unknown, Width x0, TrErr- Train- SlotClk- DLActive- BWMgmt- ABWMgmt-
DevCap2: Completion Timeout: Not Supported, TimeoutDis-, LTR-, OBFF Not Supported
DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
Capabilities: [80] MSI-X: Enable+ Count=4 Masked-
Vector table: BAR=4 offset=00000000
PBA: BAR=4 offset=000f0000
Capabilities: [98] #14 [0002]
Capabilities: [100 v1] Alternative Routing-ID Interpretation (ARI)
ARICap: MFVC- ACS-, Next Function: 0
ARICtl: MFVC- ACS-, Function Group: 0
Capabilities: [108 v1] Vendor Specific Information: ID=00a0 Rev=1 Len=040 <?>
Kernel driver in use: ahci
Kernel modules: ahci
^ permalink raw reply
* Re: [PATCH][V3] libata: make static arrays const, reduces object code size
From: Tejun Heo @ 2017-09-19 18:54 UTC (permalink / raw)
To: Colin King; +Cc: linux-ide, kernel-janitors, linux-kernel
In-Reply-To: <20170919083952.5648-1-colin.king@canonical.com>
On Tue, Sep 19, 2017 at 09:39:52AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Don't populate const arrayis on the stack, instead make them static.
> Makes the object code smaller by over 260 bytes:
>
> Before:
> text data bss dec hex filename
> 64864 5948 4128 74940 124bc drivers/ata/libata-scsi.o
>
> After:
> text data bss dec hex filename
> 64183 6364 4128 74675 123b3 drivers/ata/libata-scsi.o
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to libata/for-4.15.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH][V3] libata: make static arrays const, reduces object code size
From: Colin King @ 2017-09-19 8:39 UTC (permalink / raw)
To: Tejun Heo, linux-ide; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Don't populate const arrayis on the stack, instead make them static.
Makes the object code smaller by over 260 bytes:
Before:
text data bss dec hex filename
64864 5948 4128 74940 124bc drivers/ata/libata-scsi.o
After:
text data bss dec hex filename
64183 6364 4128 74675 123b3 drivers/ata/libata-scsi.o
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/ata/libata-scsi.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 44ba292f2cd7..c3c483fc2145 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2145,7 +2145,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
*/
static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
{
- const u8 versions[] = {
+ static const u8 versions[] = {
0x00,
0x60, /* SAM-3 (no version claimed) */
@@ -2155,7 +2155,7 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
0x03,
0x00 /* SPC-3 (no version claimed) */
};
- const u8 versions_zbc[] = {
+ static const u8 versions_zbc[] = {
0x00,
0xA0, /* SAM-5 (no version claimed) */
@@ -2227,7 +2227,7 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
{
int num_pages;
- const u8 pages[] = {
+ static const u8 pages[] = {
0x00, /* page 0x00, this page */
0x80, /* page 0x80, unit serial no page */
0x83, /* page 0x83, device ident page */
@@ -2258,7 +2258,7 @@ static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
*/
static unsigned int ata_scsiop_inq_80(struct ata_scsi_args *args, u8 *rbuf)
{
- const u8 hdr[] = {
+ static const u8 hdr[] = {
0,
0x80, /* this page code */
0,
@@ -2580,7 +2580,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
{
struct ata_device *dev = args->dev;
u8 *scsicmd = args->cmd->cmnd, *p = rbuf;
- const u8 sat_blk_desc[] = {
+ static const u8 sat_blk_desc[] = {
0, 0, 0, 0, /* number of blocks: sat unspecified */
0,
0, 0x2, 0x0 /* block length: 512 bytes */
--
2.14.1
^ permalink raw reply related
* Re: [PATCH] ata_piix: Add Fujitsu-Siemens Lifebook S6120 to short cable IDs
From: Tejun Heo @ 2017-09-19 3:32 UTC (permalink / raw)
To: Ville Syrjala; +Cc: linux-ide
In-Reply-To: <20170918200008.16139-1-ville.syrjala@linux.intel.com>
On Mon, Sep 18, 2017 at 11:00:08PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Fujitsu-Siemens Lifebook S6120 misdetects the cable type for some
> drives. The problematic one in this case is an mSATA SSD hooked up via a
> mSATA->PATA bridge. With regular hard disks the detection seems to work
> correctly.
>
> Strangely an older Lifebook model (S6020) detects the cable as 80c
> with the mSATA SSD, even if using the exact same flex cable.
>
> Cc: Tejun Heo <tj@kernel.org>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Applied to libata/for-4.14-fixes.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] libata: make static arrays const, reduces object code size
From: Tejun Heo @ 2017-09-19 3:29 UTC (permalink / raw)
To: Colin King
Cc: Mark Fasheh, Joel Becker, Andrew Morton, piaojun, linux-ide,
ocfs2-devel, kernel-janitors, linux-kernel
In-Reply-To: <20170918133140.31220-1-colin.king@canonical.com>
On Mon, Sep 18, 2017 at 02:31:40PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Don't populate const arrayis on the stack, instead make them static.
> Makes the object code smaller by over 260 bytes:
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
...
> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> index 74407c6dd592..31a61e8e0152 100644
> --- a/fs/ocfs2/dlm/dlmrecovery.c
> +++ b/fs/ocfs2/dlm/dlmrecovery.c
> @@ -1129,7 +1129,6 @@ static int dlm_send_mig_lockres_msg(struct dlm_ctxt *dlm,
> (mres->num_locks * sizeof(struct dlm_migratable_lock));
>
> /* add an all-done flag if we reached the last lock */
> - orig_flags = mres->flags;
> BUG_ON(total_locks > mres_total_locks);
> if (total_locks == mres_total_locks)
> mres->flags |= DLM_MRES_ALL_DONE;
Looks like patch contamination.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] ata: pata_artop: remove redundant initialization of pio
From: Tejun Heo @ 2017-09-19 3:24 UTC (permalink / raw)
To: Colin King
Cc: Bartlomiej Zolnierkiewicz, linux-ide, kernel-janitors,
linux-kernel
In-Reply-To: <20170914185418.13790-1-colin.king@canonical.com>
On Thu, Sep 14, 2017 at 07:54:18PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> pio is initialized and the data is never read, instead it is almost
> immediately being updated to a new value. Fix this by removing the
> initialization.
>
> Detected by scan-build:
> "warning: Value stored to 'pio' during its initialization is never read"
>
> Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to libata/for-4.15.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] libata: Add new med_power_with_dipm link_power_management_policy setting
From: Tejun Heo @ 2017-09-19 3:23 UTC (permalink / raw)
To: Hans de Goede; +Cc: Matthew Garrett, linux-ide, linux-kernel
In-Reply-To: <20170914103536.7650-2-hdegoede@redhat.com>
On Thu, Sep 14, 2017 at 12:35:36PM +0200, Hans de Goede wrote:
> As described by Matthew Garret quite a while back:
> https://mjg59.dreamwidth.org/34868.html
>
> Intel CPUs starting with the Haswell generation need SATA links to power
> down for the "package" part of the CPU to reach low power-states like
> PC7 / P8 which bring a significant power-saving with them.
>
> The default max_performance lpm policy does not allow for these high
> PC states, both the medium_power and min_power policies do allow this.
>
> The min_power policy saves significantly more power, but there are some
> reports of some disks / SSDs not liking min_power leading to system
> crashes and in some cases even data corruption has been reported.
>
> Matthew has found a document documenting the default settings of
> Intel's IRST Windows driver with which most laptops ship:
> https://www-ssl.intel.com/content/dam/doc/reference-guide/sata-devices-implementation-recommendations.pdf
>
> Matthew wrote a patch changing med_power to match those defaults, but
> that never got anywhere as some people where reporting issues with the
> patch-set that patch was a part of.
>
> This commit is another attempt to make the default IRST driver settings
> available under Linux, but instead of changing medium_power and
> potentially introducing regressions, this commit adds a new
> med_power_with_dipm setting which is identical to the existing
> medium_power accept that it enables dipm on top, which makes it match
> the Windows IRST driver settings, which should hopefully be safe to
> use on most devices.
>
> The med_power_with_dipm setting is close to min_power, except that:
> a) It does not use host-initiated slumber mode (ASP not set),
> but it does allow device-initiated slumber
> b) It does not enable DevSlp mode
>
> On my T440s test laptop I get the following power savings when idle:
> medium_power 0.9W
> med_power_with_dipm 1.2W
> min_power 1.2W
>
> Suggested-by: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied to libata/for-4.15.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH] ata_piix: Add Fujitsu-Siemens Lifebook S6120 to short cable IDs
From: Ville Syrjala @ 2017-09-18 20:00 UTC (permalink / raw)
To: linux-ide; +Cc: Tejun Heo, Ville Syrjälä
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fujitsu-Siemens Lifebook S6120 misdetects the cable type for some
drives. The problematic one in this case is an mSATA SSD hooked up via a
mSATA->PATA bridge. With regular hard disks the detection seems to work
correctly.
Strangely an older Lifebook model (S6020) detects the cable as 80c
with the mSATA SSD, even if using the exact same flex cable.
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/ata/ata_piix.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 8401c3b5be92..b702c20fbc2b 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -492,6 +492,7 @@ static const struct ich_laptop ich_laptop[] = {
{ 0x27DF, 0x152D, 0x0778 }, /* ICH7 on unknown Intel */
{ 0x24CA, 0x1025, 0x0061 }, /* ICH4 on ACER Aspire 2023WLMi */
{ 0x24CA, 0x1025, 0x003d }, /* ICH4 on ACER TM290 */
+ { 0x24CA, 0x10CF, 0x11AB }, /* ICH4M on Fujitsu-Siemens Lifebook S6120 */
{ 0x266F, 0x1025, 0x0066 }, /* ICH6 on ACER Aspire 1694WLMi */
{ 0x2653, 0x1043, 0x82D8 }, /* ICH6M on Asus Eee 701 */
{ 0x27df, 0x104d, 0x900e }, /* ICH7 on Sony TZ-90 */
--
2.13.5
^ permalink raw reply related
* Re: [PATCH][V2] libata: make const arrays static, reduces object code size
From: Sergei Shtylyov @ 2017-09-18 16:54 UTC (permalink / raw)
To: Colin King, Tejun Heo, Mark Fasheh, Joel Becker, Andrew Morton,
piaojun, linux-ide, ocfs2-devel
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20170918133219.31293-1-colin.king@canonical.com>
Hello!
On 09/18/2017 04:32 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Don't populate const arrays on the stack, instead make them static.
> Makes the object code smaller by over 260 bytes:
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/ata/libata-scsi.c | 10 +++++-----
> fs/ocfs2/dlm/dlmrecovery.c | 1 -
> 2 files changed, 5 insertions(+), 6 deletions(-)
[...]
> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> index 74407c6dd592..31a61e8e0152 100644
> --- a/fs/ocfs2/dlm/dlmrecovery.c
> +++ b/fs/ocfs2/dlm/dlmrecovery.c
> @@ -1129,7 +1129,6 @@ static int dlm_send_mig_lockres_msg(struct dlm_ctxt *dlm,
> (mres->num_locks * sizeof(struct dlm_migratable_lock));
>
> /* add an all-done flag if we reached the last lock */
> - orig_flags = mres->flags;
What's that? :-/
> BUG_ON(total_locks > mres_total_locks);
> if (total_locks == mres_total_locks)
> mres->flags |= DLM_MRES_ALL_DONE;
MBR, Sergei
^ permalink raw reply
* [PATCH][V2] libata: make const arrays static, reduces object code size
From: Colin King @ 2017-09-18 13:32 UTC (permalink / raw)
To: Tejun Heo, Mark Fasheh, Joel Becker, Andrew Morton, piaojun,
linux-ide, ocfs2-devel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Don't populate const arrays on the stack, instead make them static.
Makes the object code smaller by over 260 bytes:
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/ata/libata-scsi.c | 10 +++++-----
fs/ocfs2/dlm/dlmrecovery.c | 1 -
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 44ba292f2cd7..c3c483fc2145 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2145,7 +2145,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
*/
static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
{
- const u8 versions[] = {
+ static const u8 versions[] = {
0x00,
0x60, /* SAM-3 (no version claimed) */
@@ -2155,7 +2155,7 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
0x03,
0x00 /* SPC-3 (no version claimed) */
};
- const u8 versions_zbc[] = {
+ static const u8 versions_zbc[] = {
0x00,
0xA0, /* SAM-5 (no version claimed) */
@@ -2227,7 +2227,7 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
{
int num_pages;
- const u8 pages[] = {
+ static const u8 pages[] = {
0x00, /* page 0x00, this page */
0x80, /* page 0x80, unit serial no page */
0x83, /* page 0x83, device ident page */
@@ -2258,7 +2258,7 @@ static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
*/
static unsigned int ata_scsiop_inq_80(struct ata_scsi_args *args, u8 *rbuf)
{
- const u8 hdr[] = {
+ static const u8 hdr[] = {
0,
0x80, /* this page code */
0,
@@ -2580,7 +2580,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
{
struct ata_device *dev = args->dev;
u8 *scsicmd = args->cmd->cmnd, *p = rbuf;
- const u8 sat_blk_desc[] = {
+ static const u8 sat_blk_desc[] = {
0, 0, 0, 0, /* number of blocks: sat unspecified */
0,
0, 0x2, 0x0 /* block length: 512 bytes */
diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
index 74407c6dd592..31a61e8e0152 100644
--- a/fs/ocfs2/dlm/dlmrecovery.c
+++ b/fs/ocfs2/dlm/dlmrecovery.c
@@ -1129,7 +1129,6 @@ static int dlm_send_mig_lockres_msg(struct dlm_ctxt *dlm,
(mres->num_locks * sizeof(struct dlm_migratable_lock));
/* add an all-done flag if we reached the last lock */
- orig_flags = mres->flags;
BUG_ON(total_locks > mres_total_locks);
if (total_locks == mres_total_locks)
mres->flags |= DLM_MRES_ALL_DONE;
--
2.14.1
^ permalink raw reply related
* [PATCH] libata: make static arrays const, reduces object code size
From: Colin King @ 2017-09-18 13:31 UTC (permalink / raw)
To: Tejun Heo, Mark Fasheh, Joel Becker, Andrew Morton, piaojun,
linux-ide, ocfs2-devel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Don't populate const arrayis on the stack, instead make them static.
Makes the object code smaller by over 260 bytes:
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/ata/libata-scsi.c | 10 +++++-----
fs/ocfs2/dlm/dlmrecovery.c | 1 -
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 44ba292f2cd7..c3c483fc2145 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2145,7 +2145,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
*/
static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
{
- const u8 versions[] = {
+ static const u8 versions[] = {
0x00,
0x60, /* SAM-3 (no version claimed) */
@@ -2155,7 +2155,7 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
0x03,
0x00 /* SPC-3 (no version claimed) */
};
- const u8 versions_zbc[] = {
+ static const u8 versions_zbc[] = {
0x00,
0xA0, /* SAM-5 (no version claimed) */
@@ -2227,7 +2227,7 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
{
int num_pages;
- const u8 pages[] = {
+ static const u8 pages[] = {
0x00, /* page 0x00, this page */
0x80, /* page 0x80, unit serial no page */
0x83, /* page 0x83, device ident page */
@@ -2258,7 +2258,7 @@ static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
*/
static unsigned int ata_scsiop_inq_80(struct ata_scsi_args *args, u8 *rbuf)
{
- const u8 hdr[] = {
+ static const u8 hdr[] = {
0,
0x80, /* this page code */
0,
@@ -2580,7 +2580,7 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
{
struct ata_device *dev = args->dev;
u8 *scsicmd = args->cmd->cmnd, *p = rbuf;
- const u8 sat_blk_desc[] = {
+ static const u8 sat_blk_desc[] = {
0, 0, 0, 0, /* number of blocks: sat unspecified */
0,
0, 0x2, 0x0 /* block length: 512 bytes */
diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
index 74407c6dd592..31a61e8e0152 100644
--- a/fs/ocfs2/dlm/dlmrecovery.c
+++ b/fs/ocfs2/dlm/dlmrecovery.c
@@ -1129,7 +1129,6 @@ static int dlm_send_mig_lockres_msg(struct dlm_ctxt *dlm,
(mres->num_locks * sizeof(struct dlm_migratable_lock));
/* add an all-done flag if we reached the last lock */
- orig_flags = mres->flags;
BUG_ON(total_locks > mres_total_locks);
if (total_locks == mres_total_locks)
mres->flags |= DLM_MRES_ALL_DONE;
--
2.14.1
^ permalink raw reply related
* [PATCH] ata: pata_artop: remove redundant initialization of pio
From: Colin King @ 2017-09-14 18:54 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz, Tejun Heo, linux-ide
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
pio is initialized and the data is never read, instead it is almost
immediately being updated to a new value. Fix this by removing the
initialization.
Detected by scan-build:
"warning: Value stored to 'pio' during its initialization is never read"
Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/ata/pata_artop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/pata_artop.c b/drivers/ata/pata_artop.c
index 96c05c9494fa..6b3355343542 100644
--- a/drivers/ata/pata_artop.c
+++ b/drivers/ata/pata_artop.c
@@ -242,7 +242,7 @@ static void artop6210_set_dmamode (struct ata_port *ap, struct ata_device *adev)
static void artop6260_set_dmamode (struct ata_port *ap, struct ata_device *adev)
{
- unsigned int pio = adev->pio_mode - XFER_PIO_0;
+ unsigned int pio;
struct pci_dev *pdev = to_pci_dev(ap->host->dev);
u8 ultra;
--
2.14.1
^ permalink raw reply related
* [PATCH] libata: Add new med_power_with_dipm link_power_management_policy setting
From: Hans de Goede @ 2017-09-14 10:35 UTC (permalink / raw)
To: Tejun Heo; +Cc: Hans de Goede, Matthew Garrett, linux-ide, linux-kernel
In-Reply-To: <20170914103536.7650-1-hdegoede@redhat.com>
As described by Matthew Garret quite a while back:
https://mjg59.dreamwidth.org/34868.html
Intel CPUs starting with the Haswell generation need SATA links to power
down for the "package" part of the CPU to reach low power-states like
PC7 / P8 which bring a significant power-saving with them.
The default max_performance lpm policy does not allow for these high
PC states, both the medium_power and min_power policies do allow this.
The min_power policy saves significantly more power, but there are some
reports of some disks / SSDs not liking min_power leading to system
crashes and in some cases even data corruption has been reported.
Matthew has found a document documenting the default settings of
Intel's IRST Windows driver with which most laptops ship:
https://www-ssl.intel.com/content/dam/doc/reference-guide/sata-devices-implementation-recommendations.pdf
Matthew wrote a patch changing med_power to match those defaults, but
that never got anywhere as some people where reporting issues with the
patch-set that patch was a part of.
This commit is another attempt to make the default IRST driver settings
available under Linux, but instead of changing medium_power and
potentially introducing regressions, this commit adds a new
med_power_with_dipm setting which is identical to the existing
medium_power accept that it enables dipm on top, which makes it match
the Windows IRST driver settings, which should hopefully be safe to
use on most devices.
The med_power_with_dipm setting is close to min_power, except that:
a) It does not use host-initiated slumber mode (ASP not set),
but it does allow device-initiated slumber
b) It does not enable DevSlp mode
On my T440s test laptop I get the following power savings when idle:
medium_power 0.9W
med_power_with_dipm 1.2W
min_power 1.2W
Suggested-by: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/ata/libata-core.c | 1 +
drivers/ata/libata-eh.c | 10 +++++-----
drivers/ata/libata-scsi.c | 9 +++++----
include/linux/libata.h | 1 +
4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 1945a8ea2099..f165d95c780f 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3964,6 +3964,7 @@ int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
scontrol &= ~(0x1 << 8);
scontrol |= (0x6 << 8);
break;
+ case ATA_LPM_MED_POWER_WITH_DIPM:
case ATA_LPM_MIN_POWER:
if (ata_link_nr_enabled(link) > 0)
/* no restrictions on LPM transitions */
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 3dbd05532c09..aefe9a9971ad 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -3456,9 +3456,9 @@ static int ata_eh_maybe_retry_flush(struct ata_device *dev)
* @r_failed_dev: out parameter for failed device
*
* Enable SATA Interface power management. This will enable
- * Device Interface Power Management (DIPM) for min_power
- * policy, and then call driver specific callbacks for
- * enabling Host Initiated Power management.
+ * Device Interface Power Management (DIPM) for min_power and
+ * medium_power_with_dipm policies, and then call driver specific
+ * callbacks for enabling Host Initiated Power management.
*
* LOCKING:
* EH context.
@@ -3504,7 +3504,7 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
hints &= ~ATA_LPM_HIPM;
/* disable DIPM before changing link config */
- if (policy != ATA_LPM_MIN_POWER && dipm) {
+ if (policy < ATA_LPM_MED_POWER_WITH_DIPM && dipm) {
err_mask = ata_dev_set_feature(dev,
SETFEATURES_SATA_DISABLE, SATA_DIPM);
if (err_mask && err_mask != AC_ERR_DEV) {
@@ -3547,7 +3547,7 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
/* host config updated, enable DIPM if transitioning to MIN_POWER */
ata_for_each_dev(dev, link, ENABLED) {
- if (policy == ATA_LPM_MIN_POWER && !no_dipm &&
+ if (policy >= ATA_LPM_MED_POWER_WITH_DIPM && !no_dipm &&
ata_id_has_dipm(dev->id)) {
err_mask = ata_dev_set_feature(dev,
SETFEATURES_SATA_ENABLE, SATA_DIPM);
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 44ba292f2cd7..673e72f438eb 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -106,10 +106,11 @@ static const u8 def_control_mpage[CONTROL_MPAGE_LEN] = {
};
static const char *ata_lpm_policy_names[] = {
- [ATA_LPM_UNKNOWN] = "max_performance",
- [ATA_LPM_MAX_POWER] = "max_performance",
- [ATA_LPM_MED_POWER] = "medium_power",
- [ATA_LPM_MIN_POWER] = "min_power",
+ [ATA_LPM_UNKNOWN] = "max_performance",
+ [ATA_LPM_MAX_POWER] = "max_performance",
+ [ATA_LPM_MED_POWER] = "medium_power",
+ [ATA_LPM_MED_POWER_WITH_DIPM] = "med_power_with_dipm",
+ [ATA_LPM_MIN_POWER] = "min_power",
};
static ssize_t ata_scsi_lpm_store(struct device *device,
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 931c32f1f18d..ed9826b21c5e 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -522,6 +522,7 @@ enum ata_lpm_policy {
ATA_LPM_UNKNOWN,
ATA_LPM_MAX_POWER,
ATA_LPM_MED_POWER,
+ ATA_LPM_MED_POWER_WITH_DIPM, /* Med power + DIPM as win IRST does */
ATA_LPM_MIN_POWER,
};
--
2.14.1
^ permalink raw reply related
* [PATCH 0/1] libata: Add new med_power_with_dipm link_power_management_policy setting
From: Hans de Goede @ 2017-09-14 10:35 UTC (permalink / raw)
To: Tejun Heo; +Cc: Hans de Goede, Matthew Garrett, linux-ide, linux-kernel
Hi All,
As described by Matthew Garret quite a while back:
https://mjg59.dreamwidth.org/34868.html
On Intel CPUs starting with the Haswell generation SATA link power
management can save a significant amount of power.
Previous attempts to try and enable SATA LPM by default have gotten stuck
on ome reports of some disks / SSDs not liking min_power leading to system
crashes and in some cases even data corruption has been reported.
This patch is another attempt to make the default Windows IRST driver
settings Matthew found available under Linux, but instead of changing
medium_power and potentially introducing regressions, this commit adds
a new med_power_with_dipm setting.
I've done a blog post asking people to test this new settings on laptops:
https://hansdegoede.livejournal.com/18412.html
As mentioned there my goal is to enable med_power_with_dipm as default
LPM setting for laptops for Fedora 28 (to be released aprox. May 2018).
How exactly this will be done is still up for debate, one option would
be a kernel patch which recognizes the mobile variant of Intel's chipset
and changes the default on those, another option is punting this to
userspace.
Regards,
Hans
^ permalink raw reply
* Travelers List
From: Nancy McCarthy @ 2017-09-08 22:22 UTC (permalink / raw)
To: linux-ide
Hi,
Greeting of the day!
I hope this note finds you well!
Would you be interested in acquiring an email list of "Travelers" from USA?
We also have data for Frequent Travelers List, Camping and Outdoor List, Scuba Divers List, Cruise Travelers List, Vacation Travelers List,Boat Owners List, Fishing Enthusiasts List, Apparel Buyers List, Luxury Brand Buyers List, Gift buyers List and many more.
Each record in the list contains Contact Name (First, Middle and Last Name), Mailing Address, List type and Opt-in email address.
All the contacts are opt-in verified, Complete permission based and can be used for unlimited multi-channel marketing.
Let me know if you'd be interested in hearing more about it.
Waiting for your valuable and sincere reply.
Best Regards,
Nancy McCarthy
Business Development & Data Specialist
^ permalink raw reply
* Re: [PATCH] [RESEND] ata: avoid gcc-7 warning in ata_timing_quantize
From: Tejun Heo @ 2017-09-07 20:34 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-ide, linux-kernel
In-Reply-To: <20170906214552.2111904-1-arnd@arndb.de>
On Wed, Sep 06, 2017 at 11:45:34PM +0200, Arnd Bergmann wrote:
> gcc-7 warns about the result of a constant multiplication used as
> a boolean:
>
> drivers/ata/libata-core.c: In function 'ata_timing_quantize':
> drivers/ata/libata-core.c:3164:30: warning: '*' in boolean context, suggest '&&' instead [-Wint-in-bool-context]
>
> This slightly rearranges the macro to simplify the code and avoid
> the warning at the same time.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Hi Tejun,
>
> I just noticed this patch among others I had previously sent
> but that were not applied. It seems I forgot to reply to your
> Ack after it had become clear that the serious would not
> get merged in one chunk. Please apply this for 4.14.
My bad. I'm sorry. Applied to libata/for-4.14-fixes.
Thanks.
--
tejun
^ permalink raw reply
* (unknown),
From: dengx @ 2017-09-07 4:02 UTC (permalink / raw)
To: linux-ide
[-- Attachment #1: 977913748072031.doc --]
[-- Type: application/msword, Size: 39607 bytes --]
^ permalink raw reply
* [PATCH] [RESEND] ata: avoid gcc-7 warning in ata_timing_quantize
From: Arnd Bergmann @ 2017-09-06 21:45 UTC (permalink / raw)
To: Tejun Heo; +Cc: Arnd Bergmann, linux-ide, linux-kernel
gcc-7 warns about the result of a constant multiplication used as
a boolean:
drivers/ata/libata-core.c: In function 'ata_timing_quantize':
drivers/ata/libata-core.c:3164:30: warning: '*' in boolean context, suggest '&&' instead [-Wint-in-bool-context]
This slightly rearranges the macro to simplify the code and avoid
the warning at the same time.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Hi Tejun,
I just noticed this patch among others I had previously sent
but that were not applied. It seems I forgot to reply to your
Ack after it had become clear that the serious would not
get merged in one chunk. Please apply this for 4.14.
Link: https://patchwork.kernel.org/patch/9840269/
---
drivers/ata/libata-core.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 1945a8ea2099..ee4c1ec9dca0 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3234,19 +3234,19 @@ static const struct ata_timing ata_timing[] = {
};
#define ENOUGH(v, unit) (((v)-1)/(unit)+1)
-#define EZ(v, unit) ((v)?ENOUGH(v, unit):0)
+#define EZ(v, unit) ((v)?ENOUGH(((v) * 1000), unit):0)
static void ata_timing_quantize(const struct ata_timing *t, struct ata_timing *q, int T, int UT)
{
- q->setup = EZ(t->setup * 1000, T);
- q->act8b = EZ(t->act8b * 1000, T);
- q->rec8b = EZ(t->rec8b * 1000, T);
- q->cyc8b = EZ(t->cyc8b * 1000, T);
- q->active = EZ(t->active * 1000, T);
- q->recover = EZ(t->recover * 1000, T);
- q->dmack_hold = EZ(t->dmack_hold * 1000, T);
- q->cycle = EZ(t->cycle * 1000, T);
- q->udma = EZ(t->udma * 1000, UT);
+ q->setup = EZ(t->setup, T);
+ q->act8b = EZ(t->act8b, T);
+ q->rec8b = EZ(t->rec8b, T);
+ q->cyc8b = EZ(t->cyc8b, T);
+ q->active = EZ(t->active, T);
+ q->recover = EZ(t->recover, T);
+ q->dmack_hold = EZ(t->dmack_hold, T);
+ q->cycle = EZ(t->cycle, T);
+ q->udma = EZ(t->udma, UT);
}
void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,
--
2.9.0
^ permalink raw reply related
* Re: [GIT PULL] libata changes for v4.14-rc1
From: Tejun Heo @ 2017-09-06 14:46 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-ide, Bartlomiej Zolnierkiewicz, Hans de Goede, linux-kernel
In-Reply-To: <20170905142959.GG1774378@devbig577.frc2.facebook.com>
Hello, Linus.
I added the following two patches to the branch. The ahci patch fixes
a boot issue, so it needs some prioritizing and I applied the zpodd
one while at it.
795ef78 libata: zpodd: make arrays cdb static, reduces object code size
f723fa4 ahci: don't use MSI for devices with the silly Intel NVMe remapping scheme
Except for the ahci fix, nothing major in this pull request. Some new
platform controller support and device specific changes.
Thanks.
The following changes since commit 74cbd96bc2e00f5daa805e2ebf49e998f7045062:
Merge tag 'md/4.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/shli/md (2017-07-18 11:51:08 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git for-4.14
for you to fetch changes up to 795ef788145ed2fa023efdf11e8d5d7bedc21462:
libata: zpodd: make arrays cdb static, reduces object code size (2017-09-06 07:40:28 -0700)
----------------------------------------------------------------
Andrey Korolyov (1):
cs5536: add support for IDE controller variant
Christoph Hellwig (1):
ahci: don't use MSI for devices with the silly Intel NVMe remapping scheme
Colin Ian King (1):
libata: zpodd: make arrays cdb static, reduces object code size
Julia Lawall (1):
ata: Drop unnecessary static
Linus Walleij (2):
ata: sata_gemini: Retire custom pin control
ata: sata_gemini: Introduce explicit IDE pin control
Nate Watterson (1):
ata: ahci_platform: Add shutdown handler
Philipp Zabel (1):
ata: sata_gemini: explicitly request exclusive reset control
Rob Herring (1):
ata: Convert to using %pOF instead of full_name
Ryder Lee (2):
ata: mediatek: add support for MediaTek SATA controller
dt-bindings: ata: add DT bindings for MediaTek SATA controller
Sergei Shtylyov (1):
pata_octeon_cf: use of_property_read_{bool|u32}()
Documentation/devicetree/bindings/ata/ahci-mtk.txt | 51 ++++++
drivers/ata/Kconfig | 10 ++
drivers/ata/Makefile | 1 +
drivers/ata/ahci.c | 9 +-
drivers/ata/ahci_mtk.c | 196 +++++++++++++++++++++
drivers/ata/ahci_platform.c | 1 +
drivers/ata/libahci_platform.c | 34 ++++
drivers/ata/libata-zpodd.c | 4 +-
drivers/ata/pata_amd.c | 1 +
drivers/ata/pata_cs5536.c | 1 +
drivers/ata/pata_octeon_cf.c | 10 +-
drivers/ata/sata_gemini.c | 67 +++----
drivers/ata/sata_svw.c | 2 +-
include/linux/ahci_platform.h | 2 +
include/linux/pci_ids.h | 1 +
15 files changed, 347 insertions(+), 43 deletions(-)
create mode 100644 Documentation/devicetree/bindings/ata/ahci-mtk.txt
create mode 100644 drivers/ata/ahci_mtk.c
--
tejun
^ 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