Message ID | 20240409073528.13214-3-kobayashi.da-06@fujitsu.com |
---|---|
State | Superseded |
Headers | show |
Series | cxl: Export cxl1.1 device link status to sysfs | expand |
On 4/9/24 12:35 AM, Kobayashi,Daisuke wrote: > Add rcd_regs initialization at __rcrb_to_component() to cache the cxl1.1 > device link status information. Reduce access to the memory map area > where the RCRB is located by caching the cxl1.1 device link status information. > > Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com> > --- > drivers/cxl/core/regs.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > index 372786f80955..308eb951613e 100644 > --- a/drivers/cxl/core/regs.c > +++ b/drivers/cxl/core/regs.c > @@ -514,6 +514,8 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri > u32 bar0, bar1; > u16 cmd; > u32 id; > + u16 offset; > + u32 cap_hdr; > > if (which == CXL_RCRB_UPSTREAM) > rcrb += SZ_4K; > @@ -537,6 +539,22 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri > cmd = readw(addr + PCI_COMMAND); > bar0 = readl(addr + PCI_BASE_ADDRESS_0); > bar1 = readl(addr + PCI_BASE_ADDRESS_1); > + > + offset = readw(addr + PCI_CAPABILITY_LIST); > + offset &= 0x00ff; GENMASK(7,0) is preferred to 0x00ff. Although a properly defined mask would be nice. Also please consider using FIELD_GET(). > + cap_hdr = readl(addr + offset); > + while ((cap_hdr & 0x000000ff) != PCI_CAP_ID_EXP) { Same comment as above > + offset = (cap_hdr >> 8) & 0x000000ff; Also here > + if (offset == 0) // End of capability list Please use /* */ instead of // for Linux kernel code > + break; > + cap_hdr = readl(addr + offset); > + } > + if (offset) { > + ri->rcd_lnkcap = readl(addr + offset + PCI_EXP_LNKCAP); > + ri->rcd_lnkctrl = readl(addr + offset + PCI_EXP_LNKCTL); > + ri->rcd_lnkstatus = readl(addr + offset + PCI_EXP_LNKSTA); > + } > + > iounmap(addr); > release_mem_region(rcrb, SZ_4K); >
Dave Jiang wrote: > On 4/9/24 12:35 AM, Kobayashi,Daisuke wrote: > > Add rcd_regs initialization at __rcrb_to_component() to cache the > > cxl1.1 device link status information. Reduce access to the memory map > > area where the RCRB is located by caching the cxl1.1 device link status > information. > > > > Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com> > > --- > > drivers/cxl/core/regs.c | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index > > 372786f80955..308eb951613e 100644 > > --- a/drivers/cxl/core/regs.c > > +++ b/drivers/cxl/core/regs.c > > @@ -514,6 +514,8 @@ resource_size_t __rcrb_to_component(struct device > *dev, struct cxl_rcrb_info *ri > > u32 bar0, bar1; > > u16 cmd; > > u32 id; > > + u16 offset; > > + u32 cap_hdr; > > > > if (which == CXL_RCRB_UPSTREAM) > > rcrb += SZ_4K; > > @@ -537,6 +539,22 @@ resource_size_t __rcrb_to_component(struct device > *dev, struct cxl_rcrb_info *ri > > cmd = readw(addr + PCI_COMMAND); > > bar0 = readl(addr + PCI_BASE_ADDRESS_0); > > bar1 = readl(addr + PCI_BASE_ADDRESS_1); > > + > > + offset = readw(addr + PCI_CAPABILITY_LIST); > > + offset &= 0x00ff; > > GENMASK(7,0) is preferred to 0x00ff. Although a properly defined mask would > be nice. > Also please consider using FIELD_GET(). > Thank you for your feedback. I will update the patch to use those macros. > > + cap_hdr = readl(addr + offset); > > + while ((cap_hdr & 0x000000ff) != PCI_CAP_ID_EXP) { > > Same comment as above > > > > + offset = (cap_hdr >> 8) & 0x000000ff; > > Also here > > > + if (offset == 0) // End of capability list > > Please use /* */ instead of // for Linux kernel code > Also I will fix it. > > + break; > > + cap_hdr = readl(addr + offset); > > + } > > + if (offset) { > > + ri->rcd_lnkcap = readl(addr + offset + PCI_EXP_LNKCAP); > > + ri->rcd_lnkctrl = readl(addr + offset + PCI_EXP_LNKCTL); > > + ri->rcd_lnkstatus = readl(addr + offset + PCI_EXP_LNKSTA); > > + } > > + > > iounmap(addr); > > release_mem_region(rcrb, SZ_4K); > >
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index 372786f80955..308eb951613e 100644 --- a/drivers/cxl/core/regs.c +++ b/drivers/cxl/core/regs.c @@ -514,6 +514,8 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri u32 bar0, bar1; u16 cmd; u32 id; + u16 offset; + u32 cap_hdr; if (which == CXL_RCRB_UPSTREAM) rcrb += SZ_4K; @@ -537,6 +539,22 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri cmd = readw(addr + PCI_COMMAND); bar0 = readl(addr + PCI_BASE_ADDRESS_0); bar1 = readl(addr + PCI_BASE_ADDRESS_1); + + offset = readw(addr + PCI_CAPABILITY_LIST); + offset &= 0x00ff; + cap_hdr = readl(addr + offset); + while ((cap_hdr & 0x000000ff) != PCI_CAP_ID_EXP) { + offset = (cap_hdr >> 8) & 0x000000ff; + if (offset == 0) // End of capability list + break; + cap_hdr = readl(addr + offset); + } + if (offset) { + ri->rcd_lnkcap = readl(addr + offset + PCI_EXP_LNKCAP); + ri->rcd_lnkctrl = readl(addr + offset + PCI_EXP_LNKCTL); + ri->rcd_lnkstatus = readl(addr + offset + PCI_EXP_LNKSTA); + } + iounmap(addr); release_mem_region(rcrb, SZ_4K);
Add rcd_regs initialization at __rcrb_to_component() to cache the cxl1.1 device link status information. Reduce access to the memory map area where the RCRB is located by caching the cxl1.1 device link status information. Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com> --- drivers/cxl/core/regs.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)