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

Excerpts from andrew.donnellan's message of 2016-07-12 20:39:13 +1000:

Some comments below - with those addressed:

Reviewed-by: Andrew Donnellan <redacted>

Thanks for the review :)

quoted

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 :)

quoted

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

quoted

+EXPORT_SYMBOL(pnv_cxl_enable_phb_kernel_api);

...

quoted

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

quoted

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

quoted

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

quoted

+extern void pnv_pci_release_device(struct pci_dev *pdev);

Will also remove this declaration of pnv_pci_release_device.

quoted

+extern const struct pci_controller_ops pnv_pci_ioda_controller_ops;

See applicable comments about static.

Will remove.

Cheers,
-Ian