Thread (29 messages) 29 messages, 3 authors, 2015-08-12

Re: [PATCH v2 01/10] cxl: Drop commands if the PCI channel is not in normal state

From: Daniel Axtens <hidden>
Date: 2015-08-11 04:13:53

Hey Daniel, keeping in mind I can't exactly test it, and that I don't know the
ins and outs of CAPI, the code looks nice and it makes sence to me.
Thanks!
Some very minor points,

Otherwise

Acked-by: Cyril Bur <redacted>
quoted
 
+static inline bool cxl_adapter_link_ok(struct cxl *cxl)
+{
+	struct pci_dev *pdev;
+
+	pdev = to_pci_dev(cxl->dev.parent);
+	return (pdev->error_state == pci_channel_io_normal);
+}
+
In the process of reviewing these patches I read the style guide in furthur
detail and (it doesn't 100% commit one way or the other but) it suggests it may
be wise to get GCC choose if it should inline or not, unless you have an reason 
(the macro replacment below being a good example)... Just a thought.
This sits in a bunch of hot paths and tight loops... and it's a
glorified macro. I will bear this in mind for the future: I hadn't
thought through the tradeoffs.

So a birdy has informed me these are going to become inlines, you can
therefore disregard those comments. I am much more in favour of inlines!
Yep, they're going to become inlines. yay for inlines.
quoted
+	/* We could be asked to terminate when the hw is down.  That
                                                               ^
I'm notoriously bad for having a low care threshhold but I do have a high care
threshhold for consistency. Double space after period? Are you sure?
Fixed.

-- 
Regards,
Daniel

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help