From: Wei Huang <wei.huang2@amd.com>
To: Simon Horman <horms@kernel.org>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org, netdev@vger.kernel.org,
bhelgaas@google.com, corbet@lwn.net, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
alex.williamson@redhat.com, gospo@broadcom.com,
michael.chan@broadcom.com, ajit.khaparde@broadcom.com,
manoj.panicker2@amd.com, Eric.VanTassell@amd.com
Subject: Re: [PATCH V1 5/9] PCI/TPH: Introduce API functions to get/set steering tags
Date: Mon, 13 May 2024 08:29:45 -0500 [thread overview]
Message-ID: <4810bb32-6a9f-4108-8728-da39fd9d81a4@amd.com> (raw)
In-Reply-To: <20240511201554.GV2347895@kernel.org>
On 5/11/24 15:15, Simon Horman wrote:
> On Thu, May 09, 2024 at 11:27:37AM -0500, Wei Huang wrote:
>> This patch introduces two API functions, pcie_tph_get_st() and
>> pcie_tph_set_st(), for a driver to retrieve or configure device's
>> steering tags. There are two possible locations for steering tag
>> table and the code automatically figure out the right location to
>> set the tags if pcie_tph_set_st() is called. Note the tag value is
>> always zero currently and will be extended in the follow-up patches.
>>
>> Co-developed-by: Eric Van Tassell <Eric.VanTassell@amd.com>
>> Signed-off-by: Eric Van Tassell <Eric.VanTassell@amd.com>
>> Signed-off-by: Wei Huang <wei.huang2@amd.com>
>
> Hi Eric and Wei,
>
> I noticed a few minor problems flagged by Sparse
> which I'd like to bring to your attention.
>
>> ---
>> drivers/pci/pcie/tph.c | 383 ++++++++++++++++++++++++++++++++++++++++
>> include/linux/pci-tph.h | 19 ++
>> 2 files changed, 402 insertions(+)
>>
>> diff --git a/drivers/pci/pcie/tph.c b/drivers/pci/pcie/tph.c
>
> ...
>
>> +/*
>> + * For a given device, return a pointer to the MSI table entry at msi_index.
>> + */
>> +static void __iomem *tph_msix_table_entry(struct pci_dev *dev,
>> + __le16 msi_index)
>> +{
>> + void *entry;
>> + u16 tbl_sz;
>> + int ret;
>> +
>> + ret = tph_get_table_size(dev, &tbl_sz);
>> + if (ret || msi_index > tbl_sz)
>
> While tbl_sz is a host-byte order integer value, msi_index is little endian.
> So maths operations involving the latter doesn't seem right.
Thanks, will take care of it in the next revision.
>
> Flagged by Sparse.
>
>> + return NULL;
>> +
>> + entry = dev->msix_base + msi_index * PCI_MSIX_ENTRY_SIZE;
>
> Likewise, there seem to be endian problems here here.
>
> Also, entry is used on the line above and below in a way
> where an __iomem annotation is expected, but entry doesn't have one.
>
> Also flagged by Sparse.
Will fix
>
>> +
>> + return entry;
>> +}
>
> ...
>
>> +/* Write the steering tag to MSI-X vector control register */
>> +static void tph_write_tag_to_msix(struct pci_dev *dev, int msix_nr, u16 tag)
>> +{
>> + u32 val;
>> + void __iomem *vec_ctrl;
>> + struct msi_desc *msi_desc;
>> +
>> + msi_desc = tph_msix_index_to_desc(dev, msix_nr);
>> + if (!msi_desc) {
>> + pr_err("MSI-X descriptor for #%d not found\n", msix_nr);
>> + return;
>> + }
>> +
>> + vec_ctrl = tph_msix_vector_control(dev, msi_desc->msi_index);
>
> According to Sparse, the type of msi_desc->msi_index is unsigned short.
> But tph_msix_vector_control expects it's second argument to be __le16.
Will fix
>
>> +
>> + val = readl(vec_ctrl);
>> + val &= 0xffff;
>> + val |= (tag << 16);
>> + writel(val, vec_ctrl);
>> +
>> + /* read back to flush the update */
>> + val = readl(vec_ctrl);
>> + msi_unlock_descs(&dev->dev);
>> +}
>
> ...
next prev parent reply other threads:[~2024-05-13 13:29 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-09 16:27 [PATCH V1 0/9] PCIe TPH and cache direct injection support Wei Huang
2024-05-09 16:27 ` [PATCH V1 1/9] PCI: Introduce PCIe TPH support framework Wei Huang
2024-05-09 16:27 ` [PATCH V1 2/9] PCI: Add TPH related register definition Wei Huang
2024-05-09 16:27 ` [PATCH V1 3/9] PCI/TPH: Implement a command line option to disable TPH Wei Huang
2024-05-09 16:27 ` [PATCH V1 4/9] PCI/TPH: Implement a command line option to force No ST Mode Wei Huang
2024-05-09 16:27 ` [PATCH V1 5/9] PCI/TPH: Introduce API functions to get/set steering tags Wei Huang
2024-05-10 3:07 ` kernel test robot
2024-05-11 20:15 ` Simon Horman
2024-05-13 13:29 ` Wei Huang [this message]
2024-05-09 16:27 ` [PATCH V1 6/9] PCI/TPH: Retrieve steering tag from ACPI _DSM Wei Huang
2024-05-10 4:20 ` kernel test robot
2024-05-10 5:24 ` kernel test robot
2024-05-09 16:27 ` [PATCH V1 7/9] PCI/TPH: Add TPH documentation Wei Huang
2024-05-15 12:11 ` Bagas Sanjaya
2024-05-09 16:27 ` [PATCH V1 8/9] bnxt_en: Add TPH support in BNXT driver Wei Huang
2024-05-09 21:50 ` Vadim Fedorenko
2024-05-10 3:55 ` Ajit Khaparde
2024-05-10 10:35 ` Vadim Fedorenko
2024-05-10 15:23 ` Andy Gospodarek
2024-05-10 20:03 ` David Wei
2024-05-10 20:33 ` Andy Gospodarek
2024-05-10 20:33 ` Vadim Fedorenko
2024-05-10 20:37 ` Andy Gospodarek
2024-05-10 3:10 ` Somnath Kotur
2024-05-09 16:27 ` [PATCH V1 9/9] bnxt_en: Pass NQ ID to the FW when allocating RX/RX AGG rings Wei Huang
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=4810bb32-6a9f-4108-8728-da39fd9d81a4@amd.com \
--to=wei.huang2@amd.com \
--cc=Eric.VanTassell@amd.com \
--cc=ajit.khaparde@broadcom.com \
--cc=alex.williamson@redhat.com \
--cc=bhelgaas@google.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gospo@broadcom.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=manoj.panicker2@amd.com \
--cc=michael.chan@broadcom.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.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).