| Message ID | 1441347624-9787-1-git-send-email-andrew.donnellan@au1.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Excerpts from andrew.donnellan's message of 2015-09-04 16:20:24 +1000: > + if (!cxl_adapter_link_ok(afu->adapter)) > + dev_warn(&dev->dev, "%s: Device link is down, refusing to enable AFU\n", __func__); > + return false; Based on the indentation it looks like you meant to add some curly brackets here. -Ian
On 07/09/15 09:58, Ian Munsie wrote: > Excerpts from andrew.donnellan's message of 2015-09-04 16:20:24 +1000: >> + if (!cxl_adapter_link_ok(afu->adapter)) >> + dev_warn(&dev->dev, "%s: Device link is down, refusing to enable AFU\n", __func__); >> + return false; > > Based on the indentation it looks like you meant to add some curly > brackets here. Indeed I did, and I'd even thought I'd tested it... this is why I shouldn't be allowed to submit patches at 4pm on Fridays... Will submit V2. (FWIW, I'm a firm believer in braces around single statements in conditionals to avoid mistakes like this, but CodingStyle disagrees...) Andrew
No worries. I've been caught out by something similar in the past when I assumed that something like this: if (foo) /* * A big long * multiline * block comment! */ do_something() would surely already have curly brackets around it ;-) Excerpts from andrew.donnellan's message of 2015-09-07 10:22:31 +1000: > > On 07/09/15 09:58, Ian Munsie wrote: > > Excerpts from andrew.donnellan's message of 2015-09-04 16:20:24 +1000: > >> + if (!cxl_adapter_link_ok(afu->adapter)) > >> + dev_warn(&dev->dev, "%s: Device link is down, refusing to enable AFU\n", __func__); > >> + return false; > > > > Based on the indentation it looks like you meant to add some curly > > brackets here. > > Indeed I did, and I'd even thought I'd tested it... this is why I > shouldn't be allowed to submit patches at 4pm on Fridays... > > Will submit V2. > > (FWIW, I'm a firm believer in braces around single statements in > conditionals to avoid mistakes like this, but CodingStyle disagrees...) > > > Andrew >
On Mon, 2015-09-07 at 16:33 +1000, Ian Munsie wrote: > No worries. I've been caught out by something similar in the past when I > assumed that something like this: > > if (foo) > /* > * A big long > * multiline > * block comment! > */ > do_something() > > would surely already have curly brackets around it ;-) You guys can do whatever you like for the cxl code. cheers
diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c index 6dd16a6..1972fb6 100644 --- a/drivers/misc/cxl/vphb.c +++ b/drivers/misc/cxl/vphb.c @@ -48,6 +48,11 @@ static bool cxl_pci_enable_device_hook(struct pci_dev *dev) phb = pci_bus_to_host(dev->bus); afu = (struct cxl_afu *)phb->private_data; + + if (!cxl_adapter_link_ok(afu->adapter)) + dev_warn(&dev->dev, "%s: Device link is down, refusing to enable AFU\n", __func__); + return false; + set_dma_ops(&dev->dev, &dma_direct_ops); set_dma_offset(&dev->dev, PAGE_OFFSET);
cxl_pci_enable_device_hook() is called when attempting to enable an AFU sitting on a vPHB. At present, the state of the underlying CXL card's PCI channel is only checked when it calls cxl_afu_check_and_enable() at the very end, after it has already set DMA options and initialised a default context. Check the CXL card's link status before setting DMA options or initialising a default context. If the link is down, print a warning and return immediately. Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> --- drivers/misc/cxl/vphb.c | 5 +++++ 1 file changed, 5 insertions(+)