> 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 <cyril...@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: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to