Re: [PATCH 07/15] powerpc/powernv: Add support for the cxl kernel api on the real phb

[Ian Munsie <imunsie@au1.ibm.com>]

Excerpts from andrew.donnellan's message of 2016-07-12 20:39:13 +1000:
> Some comments below - with those addressed:
> 
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Thanks for the review :)

> > V1->V2:
> >     - Add an explanation of the peer model to the commit message,
> >       and a comment above the pnv_cxl_enable_device_hook function.
> 
> The comments are good!

Thanks :)

> > +/*
> > + * Sets flags and switches the controller ops to enable the cxl kernel api.
> > + * Original the cxl kernel API operated on a virtual PHB, but certain cards
> 
> Originally

Will fix.

> > +EXPORT_SYMBOL(pnv_cxl_enable_phb_kernel_api);
...
> > +EXPORT_SYMBOL(pnv_pci_on_cxl_phb);
> 
> Should these two exports be _GPL as well?

Yep, they should be - will fix in V3.

Looks like we have several more in this file that should be _GPL as
well, but since they weren't introduced in this series (just moved from
another file) I might send a separate patch for those after this has
been merged.

> > -static void pnv_pci_release_device(struct pci_dev *pdev)
> > +void pnv_pci_release_device(struct pci_dev *pdev)
> 
> Why is this being unstatic-ified? I don't see us introducing any new 
> uses of it.

Left over from an earlier cut - will undo this for V3.

> > -static const struct pci_controller_ops pnv_pci_ioda_controller_ops = {
> > +const struct pci_controller_ops pnv_pci_ioda_controller_ops = {
> 
> It looks like we don't currently use this - is this being unstatic-ised 
> in view of allowing a switch back to regular mode in future?

Oops, again left over from an earlier cut (for that reason). Will undo
this for V3 - if we allow switching back in the future we can remove the
static then.

> > +extern void pnv_pci_release_device(struct pci_dev *pdev);

Will also remove this declaration of pnv_pci_release_device.

> > +extern const struct pci_controller_ops pnv_pci_ioda_controller_ops;
> 
> See applicable comments about static.

Will remove.

Cheers,
-Ian