RE: [PATCH v3 0/3] Display cxl1.1 device link status

["Daisuke Kobayashi (Fujitsu)" <kobayashi.da-06@fujitsu.com>]

Dan Williams wrote:
> Kobayashi,Daisuke wrote:
> > Hello.
> >
> > This patch series adds a feature that displays the link status
> > of the CXL1.1 device.
> 
> Good write up! One minor feedback going forward is to drop usage of
> "This patch" in a description as that is self evident.
> 
> > CXL devices are extensions of PCIe. Therefore, from CXL2.0 onwards,
> > the link status can be output in the same way as traditional PCIe.
> > However, unlike devices from CXL2.0 onwards, CXL1.1 requires a
> > different method to obtain the link status from traditional PCIe.
> > This is because the link status of the CXL1.1 device is not mapped
> > in the configuration space (as per cxl3.0 specification 8.1).
> > Instead, the configuration space containing the link status is mapped
> > to the memory mapped register region (as per cxl3.0 specification 8.2,
> > Table 8-18). Therefore, the current lspci has a problem where it does
> > not display the link status of the CXL1.1 device.
> > This patch solves these issues.
> 
> One common way to rewrite a "This patch..." sentence is in "imperative"
> tense, so for example:
> 
> "Solve these issues with sysfs attributes to expose the status
> registers hidden in the RCRB."
> 
> ...i.e. write the commit log as if the patch is commanding the code to
> change. You can find some more notes about this here:
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-
> submission-notes
> 
> ...but really, this cover letter is higher quality than most, so thanks
> for that!
> 

Thank you for your kind feedback.
In the updated patch, I will revise the text and submit it.

> > Kobayashi,Daisuke (3):
> >   Add sysfs attribute for CXL 1.1 device link status
> >   Remove conditional branch that is not suitable for cxl1.1 devices
> >
> >  drivers/cxl/acpi.c |   4 -
> >  drivers/cxl/pci.c  | 193
> +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 193 insertions(+), 4 deletions(-)
> >
> >   Add function to display cxl1.1 device link status
> >
> >  lib/access.c | 29 +++++++++++++++++++++
> >  lib/pci.h    |  2 ++
> >  ls-caps.c    | 73
> ++++++++++++++++++++++++++++++++++++++++++++++++
> ++++
> >  3 files changed, 104 insertions(+)
> 
> The typical way I handle cases where I am updating the kernel side and
> the user tooling side of a problem is to post the kernel changes and
> then separately post the user changes with a lore link to the kernel
> submission.
> 
> For PCI utils I believe you will need to send a separate pull request
> via github once the kernel changes are accepted.

I understand that it’s better to post separately. 
After the changes to the kernel are accepted, I will submit a patch to pciutils.