* [PATCH] IOC3: Switch to pci refcounting safe APIs
@ 2007-04-23 14:06 Alan Cox
2007-04-23 14:09 ` Sergei Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Alan Cox @ 2007-04-23 14:06 UTC (permalink / raw)
To: ralf, linux-mips
Untested as I don't have any IOC3 hardware so if someone could give this
a check that would be great.
Signed-off-by: Alan Cox <alan@redhat.com>
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c linux-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c
--- linux.vanilla-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c 2007-04-12 14:15:04.000000000 +0100
+++ linux-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c 2007-04-23 11:49:32.708000752 +0100
@@ -1103,20 +1103,30 @@
* MiniDINs; all other subdevices are left swinging in the wind, leave
* them disabled.
*/
-static inline int ioc3_is_menet(struct pci_dev *pdev)
+
+static int ioc3_adjacent_is_ioc3(struct pci_dev *pdev, int dev)
+{
+ struct pci_dev *dev = pci_get_bus_and_slot(pdev->bus->number,
+ PCI_DEVFN(dev, 0));
+ int ret = 0;
+
+ if (dev) {
+ if (dev->vendor == PCI_VENDOR_ID_SGI &&
+ dev->device == PCI_DEVICE_ID_SGI_IOC3)
+ ret = 1;
+ pci_dev_put(dev);
+ }
+ return ret;
+}
+
+static int ioc3_is_menet(struct pci_dev *pdev)
{
struct pci_dev *dev;
- return pdev->bus->parent == NULL
- && (dev = pci_find_slot(pdev->bus->number, PCI_DEVFN(0, 0)))
- && dev->vendor == PCI_VENDOR_ID_SGI
- && dev->device == PCI_DEVICE_ID_SGI_IOC3
- && (dev = pci_find_slot(pdev->bus->number, PCI_DEVFN(1, 0)))
- && dev->vendor == PCI_VENDOR_ID_SGI
- && dev->device == PCI_DEVICE_ID_SGI_IOC3
- && (dev = pci_find_slot(pdev->bus->number, PCI_DEVFN(2, 0)))
- && dev->vendor == PCI_VENDOR_ID_SGI
- && dev->device == PCI_DEVICE_ID_SGI_IOC3;
+ return pdev->bus->parent == NULL &&
+ ioc3_adjacent_is_ioc3(pdev, 0) &&
+ ioc3_adjacent_is_ioc3(pdev, 1) &&
+ ioc3_adjacent_is_ioc3(pdev, 2));
}
#ifdef CONFIG_SERIAL_8250
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IOC3: Switch to pci refcounting safe APIs
2007-04-23 14:06 [PATCH] IOC3: Switch to pci refcounting safe APIs Alan Cox
@ 2007-04-23 14:09 ` Sergei Shtylyov
2007-04-23 14:19 ` Alan Cox
0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2007-04-23 14:09 UTC (permalink / raw)
To: Alan Cox; +Cc: ralf, linux-mips
Hello.
Alan Cox wrote:
> Untested as I don't have any IOC3 hardware so if someone could give this
> a check that would be great.
> Signed-off-by: Alan Cox <alan@redhat.com>
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c linux-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c
> --- linux.vanilla-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c 2007-04-12 14:15:04.000000000 +0100
> +++ linux-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c 2007-04-23 11:49:32.708000752 +0100
> @@ -1103,20 +1103,30 @@
> * MiniDINs; all other subdevices are left swinging in the wind, leave
> * them disabled.
> */
> -static inline int ioc3_is_menet(struct pci_dev *pdev)
> +
> +static int ioc3_adjacent_is_ioc3(struct pci_dev *pdev, int dev)
> +{
> + struct pci_dev *dev = pci_get_bus_and_slot(pdev->bus->number,
> + PCI_DEVFN(dev, 0));
The same question: isn't pci_get_bus() better in this case?
> + int ret = 0;
> +
> + if (dev) {
> + if (dev->vendor == PCI_VENDOR_ID_SGI &&
> + dev->device == PCI_DEVICE_ID_SGI_IOC3)
> + ret = 1;
> + pci_dev_put(dev);
> + }
> + return ret;
> +}
> +
> +static int ioc3_is_menet(struct pci_dev *pdev)
> {
> struct pci_dev *dev;
>
> - return pdev->bus->parent == NULL
> - && (dev = pci_find_slot(pdev->bus->number, PCI_DEVFN(0, 0)))
> - && dev->vendor == PCI_VENDOR_ID_SGI
> - && dev->device == PCI_DEVICE_ID_SGI_IOC3
> - && (dev = pci_find_slot(pdev->bus->number, PCI_DEVFN(1, 0)))
> - && dev->vendor == PCI_VENDOR_ID_SGI
> - && dev->device == PCI_DEVICE_ID_SGI_IOC3
> - && (dev = pci_find_slot(pdev->bus->number, PCI_DEVFN(2, 0)))
> - && dev->vendor == PCI_VENDOR_ID_SGI
> - && dev->device == PCI_DEVICE_ID_SGI_IOC3;
> + return pdev->bus->parent == NULL &&
> + ioc3_adjacent_is_ioc3(pdev, 0) &&
> + ioc3_adjacent_is_ioc3(pdev, 1) &&
> + ioc3_adjacent_is_ioc3(pdev, 2));
> }
I don't see the point of using refcounting API in such cases but well...
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IOC3: Switch to pci refcounting safe APIs
2007-04-23 14:09 ` Sergei Shtylyov
@ 2007-04-23 14:19 ` Alan Cox
2007-04-23 14:21 ` Sergei Shtylyov
2007-04-23 14:35 ` Ralf Baechle
0 siblings, 2 replies; 6+ messages in thread
From: Alan Cox @ 2007-04-23 14:19 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: ralf, linux-mips
> > +static int ioc3_adjacent_is_ioc3(struct pci_dev *pdev, int dev)
> > +{
> > + struct pci_dev *dev = pci_get_bus_and_slot(pdev->bus->number,
> > + PCI_DEVFN(dev, 0));
>
> The same question: isn't pci_get_bus() better in this case?
Makes no real difference, but if you know the MIPS tree never ends up
with pdev->bus = NULL for the root bus then its a trivial change
> I don't see the point of using refcounting API in such cases but well...
Two reasons
1. It makes the entire system more consistent
2. It means we can remove the (usually) unsafe pci_find_slot API
(and #3 sort of... it means the pci fake hotplug testing works with this
device too)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IOC3: Switch to pci refcounting safe APIs
2007-04-23 14:19 ` Alan Cox
@ 2007-04-23 14:21 ` Sergei Shtylyov
2007-04-23 14:35 ` Ralf Baechle
1 sibling, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2007-04-23 14:21 UTC (permalink / raw)
To: Alan Cox; +Cc: ralf, linux-mips
Alan Cox wrote:
>>>+static int ioc3_adjacent_is_ioc3(struct pci_dev *pdev, int dev)
>>>+{
>>>+ struct pci_dev *dev = pci_get_bus_and_slot(pdev->bus->number,
>>>+ PCI_DEVFN(dev, 0));
>>
>> The same question: isn't pci_get_bus() better in this case?
> Makes no real difference, but if you know the MIPS tree never ends up
pci_get_bus_and_slot() walks all device tree, pci_get_bus() walks only one bus.
> with pdev->bus = NULL for the root bus then its a trivial change
You'll get kernel oops in this case anyway as you're dereferencing pdev->bus.
>> I don't see the point of using refcounting API in such cases but well...
> Two reasons
>
> 1. It makes the entire system more consistent
> 2. It means we can remove the (usually) unsafe pci_find_slot API
> (and #3 sort of... it means the pci fake hotplug testing works with this
> device too)
Ah... Thanks for the explanation.
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IOC3: Switch to pci refcounting safe APIs
2007-04-23 14:19 ` Alan Cox
2007-04-23 14:21 ` Sergei Shtylyov
@ 2007-04-23 14:35 ` Ralf Baechle
2007-04-23 15:18 ` Alan Cox
1 sibling, 1 reply; 6+ messages in thread
From: Ralf Baechle @ 2007-04-23 14:35 UTC (permalink / raw)
To: Alan Cox; +Cc: Sergei Shtylyov, linux-mips
On Mon, Apr 23, 2007 at 03:19:18PM +0100, Alan Cox wrote:
> > > +static int ioc3_adjacent_is_ioc3(struct pci_dev *pdev, int dev)
> > > +{
> > > + struct pci_dev *dev = pci_get_bus_and_slot(pdev->bus->number,
> > > + PCI_DEVFN(dev, 0));
> >
> > The same question: isn't pci_get_bus() better in this case?
>
> Makes no real difference, but if you know the MIPS tree never ends up
> with pdev->bus = NULL for the root bus then its a trivial change
That's the case on MIPS.
> > I don't see the point of using refcounting API in such cases but well...
>
> Two reasons
>
> 1. It makes the entire system more consistent
> 2. It means we can remove the (usually) unsafe pci_find_slot API
>
> (and #3 sort of... it means the pci fake hotplug testing works with this
> device too)
The patch looks ok to me:
Acked-by: Ralf Baechle <ralf@linux-mips.org>
Longer term MENET should be handled differently but this patch certainly
doesn't make things worse.
Ralf
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IOC3: Switch to pci refcounting safe APIs
2007-04-23 14:35 ` Ralf Baechle
@ 2007-04-23 15:18 ` Alan Cox
0 siblings, 0 replies; 6+ messages in thread
From: Alan Cox @ 2007-04-23 15:18 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Sergei Shtylyov, linux-mips
> > Makes no real difference, but if you know the MIPS tree never ends up
> > with pdev->bus = NULL for the root bus then its a trivial change
>
> That's the case on MIPS.
Revised patch then
---
Convert the IOC3 driver to use ref counting pci interfaces so that we can
obsolete the (usually unsafe) pci_find_{slot/device} interfaces and avoid
future authors writing hotplug-unsafe device drivers.
Signed-off-by: Alan Cox <alan@redhat.com>
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c linux-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c
--- linux.vanilla-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c 2007-04-12 14:15:04.000000000 +0100
+++ linux-2.6.21-rc6-mm1/drivers/net/ioc3-eth.c 2007-04-23 15:58:21.431489816 +0100
@@ -1103,20 +1103,29 @@
* MiniDINs; all other subdevices are left swinging in the wind, leave
* them disabled.
*/
-static inline int ioc3_is_menet(struct pci_dev *pdev)
+
+static int ioc3_adjacent_is_ioc3(struct pci_dev *pdev, int dev)
+{
+ struct pci_dev *dev = pci_get_slot(pdev->bus, PCI_DEVFN(dev, 0));
+ int ret = 0;
+
+ if (dev) {
+ if (dev->vendor == PCI_VENDOR_ID_SGI &&
+ dev->device == PCI_DEVICE_ID_SGI_IOC3)
+ ret = 1;
+ pci_dev_put(dev);
+ }
+ return ret;
+}
+
+static int ioc3_is_menet(struct pci_dev *pdev)
{
struct pci_dev *dev;
- return pdev->bus->parent == NULL
- && (dev = pci_find_slot(pdev->bus->number, PCI_DEVFN(0, 0)))
- && dev->vendor == PCI_VENDOR_ID_SGI
- && dev->device == PCI_DEVICE_ID_SGI_IOC3
- && (dev = pci_find_slot(pdev->bus->number, PCI_DEVFN(1, 0)))
- && dev->vendor == PCI_VENDOR_ID_SGI
- && dev->device == PCI_DEVICE_ID_SGI_IOC3
- && (dev = pci_find_slot(pdev->bus->number, PCI_DEVFN(2, 0)))
- && dev->vendor == PCI_VENDOR_ID_SGI
- && dev->device == PCI_DEVICE_ID_SGI_IOC3;
+ return pdev->bus->parent == NULL &&
+ ioc3_adjacent_is_ioc3(pdev, 0) &&
+ ioc3_adjacent_is_ioc3(pdev, 1) &&
+ ioc3_adjacent_is_ioc3(pdev, 2));
}
#ifdef CONFIG_SERIAL_8250
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-04-23 15:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-23 14:06 [PATCH] IOC3: Switch to pci refcounting safe APIs Alan Cox
2007-04-23 14:09 ` Sergei Shtylyov
2007-04-23 14:19 ` Alan Cox
2007-04-23 14:21 ` Sergei Shtylyov
2007-04-23 14:35 ` Ralf Baechle
2007-04-23 15:18 ` Alan Cox
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.