| Message ID | 20240412070715.16160-2-kobayashi.da-06@fujitsu.com |
|---|---|
| State | New |
| Headers | show |
| Series | cxl: Export cxl1.1 device link status to sysfs | expand |
On 4/12/24 12:07 AM, Kobayashi,Daisuke wrote: > Add rcd_regs and its 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 | 16 ++++++++++++++++ > drivers/cxl/cxl.h | 3 +++ > 2 files changed, 19 insertions(+) > > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > index 372786f80955..e0e96be0ca7d 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,20 @@ 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 = FIELD_GET(GENMASK(7, 0), readw(addr + PCI_CAPABILITY_LIST)); Maybe #define PCI_RCRB_CAPABILITY_LIST_ID_MASK GENMASK(7, 0) > + cap_hdr = readl(addr + offset); > + while ((cap_hdr & GENMASK(7, 0)) != PCI_CAP_ID_EXP) { while ((FIELD_GET(PCI_RCRB_CAP_HDR_ID_MASK, cap_hdr) != PCI_CAP_ID_EXP) { Also I think you need to add a check and see if the loop went beyond SZ_4K that was mapped. > + offset = (cap_hdr >> 8) & GENMASK(7, 0); #define PCI_RCRB_CAP_HDR_NEXT_MASK GENMASK(15, 8); offset = FIELD_GET(PCI_RCRB_CAP_HDR_NEXT_MASK, cap_hdr); > + if (offset == 0) > + 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/cxl.h b/drivers/cxl/cxl.h > index 003feebab79b..2dc827c301a1 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -647,6 +647,9 @@ cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev) > struct cxl_rcrb_info { > resource_size_t base; > u16 aer_cap; > + u16 rcd_lnkctrl; > + u16 rcd_lnkstatus; > + u32 rcd_lnkcap; > }; > > /**
Dave Jiang wrote: > On 4/12/24 12:07 AM, Kobayashi,Daisuke wrote: > > Add rcd_regs and its 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 | 16 ++++++++++++++++ > > drivers/cxl/cxl.h | 3 +++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > > index 372786f80955..e0e96be0ca7d 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,20 @@ 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 = FIELD_GET(GENMASK(7, 0), readw(addr + > PCI_CAPABILITY_LIST)); > > Maybe > #define PCI_RCRB_CAPABILITY_LIST_ID_MASK GENMASK(7, 0) > > > + cap_hdr = readl(addr + offset); > > + while ((cap_hdr & GENMASK(7, 0)) != PCI_CAP_ID_EXP) { > > while ((FIELD_GET(PCI_RCRB_CAP_HDR_ID_MASK, cap_hdr) != > PCI_CAP_ID_EXP) { > > Also I think you need to add a check and see if the loop went beyond SZ_4K that > was mapped. > > > + offset = (cap_hdr >> 8) & GENMASK(7, 0); > > #define PCI_RCRB_CAP_HDR_NEXT_MASK GENMASK(15, 8); > offset = FIELD_GET(PCI_RCRB_CAP_HDR_NEXT_MASK, cap_hdr); Thank you for your comment. In the next patch I will define and use additional masks. > > + if (offset == 0) > > + 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/cxl.h b/drivers/cxl/cxl.h > > index 003feebab79b..2dc827c301a1 100644 > > --- a/drivers/cxl/cxl.h > > +++ b/drivers/cxl/cxl.h > > @@ -647,6 +647,9 @@ cxl_find_dport_by_dev(struct cxl_port *port, const > struct device *dport_dev) > > struct cxl_rcrb_info { > > resource_size_t base; > > u16 aer_cap; > > + u16 rcd_lnkctrl; > > + u16 rcd_lnkstatus; > > + u32 rcd_lnkcap; > > }; > > > > /** Please let me know if any revisions are necessary for merging this patch.
On 4/23/24 1:33 AM, Daisuke Kobayashi (Fujitsu) wrote: > Dave Jiang wrote: >> On 4/12/24 12:07 AM, Kobayashi,Daisuke wrote: >>> Add rcd_regs and its 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 | 16 ++++++++++++++++ >>> drivers/cxl/cxl.h | 3 +++ >>> 2 files changed, 19 insertions(+) >>> >>> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c >>> index 372786f80955..e0e96be0ca7d 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,20 @@ 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 = FIELD_GET(GENMASK(7, 0), readw(addr + >> PCI_CAPABILITY_LIST)); >> >> Maybe >> #define PCI_RCRB_CAPABILITY_LIST_ID_MASK GENMASK(7, 0) >> >>> + cap_hdr = readl(addr + offset); >>> + while ((cap_hdr & GENMASK(7, 0)) != PCI_CAP_ID_EXP) { >> >> while ((FIELD_GET(PCI_RCRB_CAP_HDR_ID_MASK, cap_hdr) != >> PCI_CAP_ID_EXP) { >> >> Also I think you need to add a check and see if the loop went beyond SZ_4K that >> was mapped. >> >>> + offset = (cap_hdr >> 8) & GENMASK(7, 0); >> >> #define PCI_RCRB_CAP_HDR_NEXT_MASK GENMASK(15, 8); >> offset = FIELD_GET(PCI_RCRB_CAP_HDR_NEXT_MASK, cap_hdr); > > Thank you for your comment. In the next patch I will define and use additional masks. > >>> + if (offset == 0) >>> + 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/cxl.h b/drivers/cxl/cxl.h >>> index 003feebab79b..2dc827c301a1 100644 >>> --- a/drivers/cxl/cxl.h >>> +++ b/drivers/cxl/cxl.h >>> @@ -647,6 +647,9 @@ cxl_find_dport_by_dev(struct cxl_port *port, const >> struct device *dport_dev) >>> struct cxl_rcrb_info { >>> resource_size_t base; >>> u16 aer_cap; >>> + u16 rcd_lnkctrl; >>> + u16 rcd_lnkstatus; >>> + u32 rcd_lnkcap; >>> }; >>> >>> /** > > Please let me know if any revisions are necessary for merging this patch. Yes please send out a revision with updates. I'd also like to see a review tag from one of the other maintainers before merging. Thanks!
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index 372786f80955..e0e96be0ca7d 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,20 @@ 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 = FIELD_GET(GENMASK(7, 0), readw(addr + PCI_CAPABILITY_LIST)); + cap_hdr = readl(addr + offset); + while ((cap_hdr & GENMASK(7, 0)) != PCI_CAP_ID_EXP) { + offset = (cap_hdr >> 8) & GENMASK(7, 0); + if (offset == 0) + 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/cxl.h b/drivers/cxl/cxl.h index 003feebab79b..2dc827c301a1 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -647,6 +647,9 @@ cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev) struct cxl_rcrb_info { resource_size_t base; u16 aer_cap; + u16 rcd_lnkctrl; + u16 rcd_lnkstatus; + u32 rcd_lnkcap; }; /**
Add rcd_regs and its 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 | 16 ++++++++++++++++ drivers/cxl/cxl.h | 3 +++ 2 files changed, 19 insertions(+)