From: Dave Jiang <dave.jiang@intel.com>
To: "Daisuke Kobayashi (Fujitsu)" <kobayashi.da-06@fujitsu.com>,
"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>
Cc: "Yasunori Gotou (Fujitsu)" <y-goto@fujitsu.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"mj@ucw.cz" <mj@ucw.cz>,
"dan.j.williams@intel.com" <dan.j.williams@intel.com>
Subject: Re: [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component()
Date: Tue, 23 Apr 2024 08:24:02 -0700 [thread overview]
Message-ID: <c7a6bcb9-5727-46c0-8efb-6430604cd344@intel.com> (raw)
In-Reply-To: <OSAPR01MB71823658767F3088CDA09489BA112@OSAPR01MB7182.jpnprd01.prod.outlook.com>
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!
next prev parent reply other threads:[~2024-04-23 15:24 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-12 7:07 [PATCH v5 0/2] cxl: Export cxl1.1 device link status to sysfs Kobayashi,Daisuke
2024-04-12 7:07 ` [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component() Kobayashi,Daisuke
2024-04-12 17:47 ` Dave Jiang
2024-04-23 8:33 ` Daisuke Kobayashi (Fujitsu)
2024-04-23 15:24 ` Dave Jiang [this message]
2024-04-12 7:07 ` [PATCH v5 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status Kobayashi,Daisuke
2024-04-12 17:51 ` Dave Jiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c7a6bcb9-5727-46c0-8efb-6430604cd344@intel.com \
--to=dave.jiang@intel.com \
--cc=dan.j.williams@intel.com \
--cc=kobayashi.da-06@fujitsu.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mj@ucw.cz \
--cc=y-goto@fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).