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

[Daniel Axtens <dja@axtens.net>]

[-- Attachment #1: Type: text/plain, Size: 1540 bytes --]

> 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 <cyrilbur@gmail.com>
> 

> >  
> > +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.

> > +	/* 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 860 bytes --]