Re: [PATCH 14/14] cxl: Add cxl_check_and_switch_mode() API to switch bi-modal cards

Thanks for the review Fred!

On 07/07/16 04:51, Frederic Barrat wrote:

quoted

+    rc = CXL_READ_VSEC_MODE_CONTROL(dev, switch_work->vsec, &val);
+    if (rc) {
+        dev_err(&bus->dev, "cxl: Failed to read CAPI mode control:
%i\n", rc);
+        pci_dev_put(dev);
+        goto err_free_work;
+    }
+    devfn = dev->devfn;
+    pci_dev_put(dev);

This is to balance the 'get' done in cxl_check_and_switch_mode(), right?
A comment wouldn't hurt. I think we're missing the 'put' on the first
error path above (!bridge).

Yep, it's to balance the pci_dev_get() in cxl_check_and_switch_mode() - 
you're right, a comment to that effect wouldn't hurt.

You're also right about the error path. Will fix in V2.

I was half-expecting to see a new entry in the cxl_pci_tbl pci ID table
for the Mellanox entry, but no such thing. By what magic is cxl_probe()
called after the switch? Because of the device class?

It matches against the class, as function 0 of the device after reset 
comes up as a class 1200 processing accelerator.

Perhaps we should be a bit more explicit though...

Out of curiosity, could you tell me what the 3rd pci function looks like
(vendor ID, device ID, ....)?

Before:

root@io163:~# lspci -vnn
0000:00:00.0 PCI bridge [0604]: IBM Device [1014:03dc] (prog-if 00 
[Normal decode])
         Flags: fast devsel
         Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
         I/O behind bridge: 00000000-00000fff
         Capabilities: [40] Power Management version 3
         Capabilities: [48] Express Root Port (Slot-), MSI 00
         Capabilities: [100] Advanced Error Reporting
         Capabilities: [148] #19

0000:01:00.0 Infiniband controller [0207]: Mellanox Technologies MT27700 
Family [ConnectX-4] [15b3:1013]
         Subsystem: IBM Device [1014:04f4]
         Flags: fast devsel, IRQ 502
         Memory at 200000000000 (64-bit, prefetchable) [disabled] [size=32M]
         Capabilities: [60] Express Endpoint, MSI 00
         Capabilities: [48] Vital Product Data
         Capabilities: [9c] MSI-X: Enable- Count=128 Masked-
         Capabilities: [c0] Vendor Specific Information: Len=18 <?>
         Capabilities: [40] Power Management version 3
         Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe
         Capabilities: [160] Vendor Specific Information: ID=1280 Rev=0 
Len=080 <?>
         Capabilities: [240] #19

0000:01:00.1 Infiniband controller [0207]: Mellanox Technologies MT27700 
Family [ConnectX-4] [15b3:1013]
         Subsystem: IBM Device [1014:04f4]
         Flags: fast devsel, IRQ 502
         Memory at 200002000000 (64-bit, prefetchable) [disabled] [size=32M]
         Capabilities: [60] Express Endpoint, MSI 00
         Capabilities: [48] Vital Product Data
         Capabilities: [9c] MSI-X: Enable- Count=128 Masked-
         Capabilities: [40] Power Management version 3
         Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe

After:

root@io163:~# lspci -vnn
0000:00:00.0 PCI bridge [0604]: IBM Device [1014:03dc] (prog-if 00 
[Normal decode])
         Flags: bus master, fast devsel, latency 0
         Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
         I/O behind bridge: 00000000-00000fff
         Capabilities: [40] Power Management version 3
         Capabilities: [48] Express Root Port (Slot-), MSI 00
         Capabilities: [100] Advanced Error Reporting
         Capabilities: [148] #19

0000:01:00.0 Processing accelerators [1200]: Mellanox Technologies 
MT27700 Family [ConnectX-4] [15b3:1013]
         Subsystem: IBM Device [1014:04f4]
         Physical Slot: Slot3
         Flags: bus master, fast devsel, latency 0, IRQ 502
         Memory at 200004000000 (64-bit, prefetchable) [size=128K]
         Memory at 200004020000 (64-bit, prefetchable) [size=128K]
         Memory at <ignored> (64-bit, prefetchable) [size=256T]
         Capabilities: [60] Express Endpoint, MSI 00
         Capabilities: [48] Vital Product Data
         Capabilities: [9c] MSI-X: Enable- Count=128 Masked-
         Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe
         Capabilities: [160] Vendor Specific Information: ID=1280 Rev=0 
Len=080 <?>
         Kernel driver in use: cxl-pci

0000:01:00.1 Infiniband controller [0207]: Mellanox Technologies MT27700 
Family [ConnectX-4] [15b3:1013]
         Subsystem: IBM Device [1014:04f4]
         Physical Slot: Slot3
         Flags: bus master, fast devsel, latency 0, IRQ 502
         Memory at 200000000000 (64-bit, prefetchable) [size=32M]
         Capabilities: [60] Express Endpoint, MSI 00
         Capabilities: [48] Vital Product Data
         Capabilities: [9c] MSI-X: Enable+ Count=128 Masked-
         Capabilities: [40] Power Management version 3
         Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe
         Kernel driver in use: mlx5_core

0000:01:00.2 Infiniband controller [0207]: Mellanox Technologies MT27700 
Family [ConnectX-4] [15b3:1013]
         Subsystem: IBM Device [1014:04f4]
         Physical Slot: Slot3
         Flags: bus master, fast devsel, latency 0, IRQ 502
         Memory at 200002000000 (64-bit, prefetchable) [size=32M]
         Capabilities: [60] Express Endpoint, MSI 00
         Capabilities: [48] Vital Product Data
         Capabilities: [9c] MSI-X: Enable+ Count=128 Masked-
         Capabilities: [40] Power Management version 3
         Capabilities: [100] Device Serial Number ba-da-ce-55-de-ad-ca-fe
         Kernel driver in use: mlx5_core

Andrew

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited