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.
