Re: [PATCH v3 3/3] acpi/pci_root: negotiate CXL _OSC

[Bjorn Helgaas <helgaas@kernel.org>]

Don't just make up new prefixes for the subject line.  Previous ones
look like this:

  PCI/ACPI: Fix acpi_pci_osc_control_set() kernel-doc comment
  ACPI: Use acpi_fetch_acpi_dev() instead of acpi_bus_get_device()
  PCI/ACPI: Check for _OSC support in acpi_pci_osc_control_set()
  PCI/ACPI: Move _OSC query checks to separate function
  PCI/ACPI: Move supported and control calculations to separate functions
  PCI/ACPI: Remove OSC_PCI_SUPPORT_MASKS and OSC_PCI_CONTROL_MASKS
  ACPI: pci_root: Unify the message printing
  PCI/ACPI: Clarify message about _OSC failure
  PCI/ACPI: Remove unnecessary osc_lock
  PCI/ACPI: Make acpi_pci_osc_control_set() static
  PCI/ACPI: Replace open coded variant of resource_union()

So I think "PCI/ACPI: " would be a good choice.  Also capitalize the
next word as all the above do.

On Wed, Mar 30, 2022 at 12:14:34PM -0600, Vishal Verma wrote:
> Add full support for negotiating _OSC as defined in the CXL 2.0 spec, as

Please include a section reference.

> applicable to CXL-enabled platforms. Advertise support for the CXL
> features we support - 'CXL 2.0 port/device register access', 'Protocol
> Error Reporting', and 'CL Native Hot Plug'. Request control for 'CXL

"CL" looks like a typo for "CXL"?

> Memory Error Reporting'. The requests are dependent on CONFIG_* based
> pre-requisites, and prior PCI enabling, similar to how the standard PCI

s/pre-requisites/prerequisites/

> _OSC bits are determined.
> 
> The CXL specification does not define any additional constraints on
> the hotplug flow beyond PCIe native hotplug, so a kernel that supports
> native PCIe hotplug, supports CXL hotplug. For error handling protocol
> and link errors just use PCIe AER. There is nascent support for
> amending AER events with CXL specific status [1], but there's
> otherwise no additional OS responsibility for CXL errors beyond PCIe
> AER. CXL Memory Errors behave the same as typical memory errors so
> CONFIG_MEMORY_FAILURE is sufficient to indicate support to platform
> firmware.
> 
> [1]: https://lore.kernel.org/linux-cxl/164740402242.3912056.8303625392871313860.stgit@dwillia2-desk3.amr.corp.intel.com/
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Reported-by: kernel test robot <lkp@intel.com>

What was reported by the robot?  If it just complained about something
in v1 or v2, I think there's no point in mentioning this here.  It's
the same as ordinary review comments (like these I'm composing), and
they don't need to be acknowledged.  I think "Reported-by" is great
when giving credit for bug fixes, but that's not what's happening
here.

Bjorn