From: "Nelson, Shannon" <shannon.nelson@amd.com>
To: Dave Jiang <dave.jiang@intel.com>,
jgg@nvidia.com, andrew.gospodarek@broadcom.com,
aron.silverton@oracle.com, dan.j.williams@intel.com,
daniel.vetter@ffwll.ch, dsahern@kernel.org,
gregkh@linuxfoundation.org, hch@infradead.org,
itayavr@nvidia.com, jiri@nvidia.com, Jonathan.Cameron@huawei.com,
kuba@kernel.org, lbloch@nvidia.com, leonro@nvidia.com,
linux-cxl@vger.kernel.org, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org, saeedm@nvidia.com
Cc: brett.creeley@amd.com
Subject: Re: [PATCH v3 2/6] pds_core: specify auxiliary_device to be created
Date: Mon, 17 Mar 2025 15:30:43 -0700 [thread overview]
Message-ID: <6dc6a934-e896-45c3-b74d-6adf0ba1e258@amd.com> (raw)
In-Reply-To: <68c66882-3a91-4843-b08e-2f9d5d2d0290@intel.com>
On 3/10/2025 10:26 AM, Dave Jiang wrote:
>
> On 3/7/25 11:53 AM, Shannon Nelson wrote:
>> In preparation for adding a new auxiliary_device for the
>> PF, make the vif type an argument to pdsc_auxbus_dev_add().
>
>> We also now pass in the address to where we'll keep the new
>> padev pointer so that the caller can specify where to save it
>> but we can still change it under the mutex and keep the mutex
>> usage within the function.
>
> Please consider changing the commit log to use imperative language and avoid using pronouns such as "we".
>
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#describe-your-changes
> https://kernelnewbies.org/PatchPhilosophy#:~:text=Please%20read%20that%20whole%20section,it%20into%20the%20git%20history.
>
> Maybe something like:
> Pass in the address of the padev pointer so the caller can specify where to save it. ...
Done - thanks,
sln
>
>>
>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
>> ---
>> drivers/net/ethernet/amd/pds_core/auxbus.c | 37 ++++++++++-----------
>> drivers/net/ethernet/amd/pds_core/core.h | 7 ++--
>> drivers/net/ethernet/amd/pds_core/devlink.c | 5 +--
>> drivers/net/ethernet/amd/pds_core/main.c | 11 +++---
>> 4 files changed, 33 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
>> index 78fba368e797..563de9e7ce0a 100644
>> --- a/drivers/net/ethernet/amd/pds_core/auxbus.c
>> +++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
>> @@ -175,29 +175,32 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
>> return padev;
>> }
>>
>> -void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
>> +void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf,
>> + struct pds_auxiliary_dev **pd_ptr)
>> {
>> struct pds_auxiliary_dev *padev;
>>
>> + if (!*pd_ptr)
>> + return;
>> +
>> mutex_lock(&pf->config_lock);
>>
>> - padev = pf->vfs[cf->vf_id].padev;
>> - if (padev) {
>> - pds_client_unregister(pf, padev->client_id);
>> - auxiliary_device_delete(&padev->aux_dev);
>> - auxiliary_device_uninit(&padev->aux_dev);
>> - padev->client_id = 0;
>> - }
>> - pf->vfs[cf->vf_id].padev = NULL;
>> + padev = *pd_ptr;
>> + pds_client_unregister(pf, padev->client_id);
>> + auxiliary_device_delete(&padev->aux_dev);
>> + auxiliary_device_uninit(&padev->aux_dev);
>> + padev->client_id = 0;
>> + *pd_ptr = NULL;
>>
>> mutex_unlock(&pf->config_lock);
>> }
>>
>> -int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
>> +int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf,
>> + enum pds_core_vif_types vt,
>> + struct pds_auxiliary_dev **pd_ptr)
>> {
>> struct pds_auxiliary_dev *padev;
>> char devname[PDS_DEVNAME_LEN];
>> - enum pds_core_vif_types vt;
>> unsigned long mask;
>> u16 vt_support;
>> int client_id;
>> @@ -206,6 +209,9 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
>> if (!cf)
>> return -ENODEV;
>>
>> + if (vt >= PDS_DEV_TYPE_MAX)
>> + return -EINVAL;
>> +
>> mutex_lock(&pf->config_lock);
>>
>> mask = BIT_ULL(PDSC_S_FW_DEAD) |
>> @@ -217,17 +223,10 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
>> goto out_unlock;
>> }
>>
>> - /* We only support vDPA so far, so it is the only one to
>> - * be verified that it is available in the Core device and
>> - * enabled in the devlink param. In the future this might
>> - * become a loop for several VIF types.
>> - */
>> -
>> /* Verify that the type is supported and enabled. It is not
>> * an error if there is no auxbus device support for this
>> * VF, it just means something else needs to happen with it.
>> */
>> - vt = PDS_DEV_TYPE_VDPA;
>> vt_support = !!le16_to_cpu(pf->dev_ident.vif_types[vt]);
>> if (!(vt_support &&
>> pf->viftype_status[vt].supported &&
>> @@ -253,7 +252,7 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
>> err = PTR_ERR(padev);
>> goto out_unlock;
>> }
>> - pf->vfs[cf->vf_id].padev = padev;
>> + *pd_ptr = padev;
>>
>> out_unlock:
>> mutex_unlock(&pf->config_lock);
>> diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
>> index 631a59cfdd7e..f075e68c64db 100644
>> --- a/drivers/net/ethernet/amd/pds_core/core.h
>> +++ b/drivers/net/ethernet/amd/pds_core/core.h
>> @@ -303,8 +303,11 @@ void pdsc_health_thread(struct work_struct *work);
>> int pdsc_register_notify(struct notifier_block *nb);
>> void pdsc_unregister_notify(struct notifier_block *nb);
>> void pdsc_notify(unsigned long event, void *data);
>> -int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf);
>> -void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf);
>> +int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf,
>> + enum pds_core_vif_types vt,
>> + struct pds_auxiliary_dev **pd_ptr);
>> +void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf,
>> + struct pds_auxiliary_dev **pd_ptr);
>>
>> void pdsc_process_adminq(struct pdsc_qcq *qcq);
>> void pdsc_work_thread(struct work_struct *work);
>> diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
>> index 4e2b92ddef6f..c5c787df61a4 100644
>> --- a/drivers/net/ethernet/amd/pds_core/devlink.c
>> +++ b/drivers/net/ethernet/amd/pds_core/devlink.c
>> @@ -57,9 +57,10 @@ int pdsc_dl_enable_set(struct devlink *dl, u32 id,
>> struct pdsc *vf = pdsc->vfs[vf_id].vf;
>>
>> if (ctx->val.vbool)
>> - err = pdsc_auxbus_dev_add(vf, pdsc);
>> + err = pdsc_auxbus_dev_add(vf, pdsc, vt_entry->vif_id,
>> + &pdsc->vfs[vf_id].padev);
>> else
>> - pdsc_auxbus_dev_del(vf, pdsc);
>> + pdsc_auxbus_dev_del(vf, pdsc, &pdsc->vfs[vf_id].padev);
>> }
>>
>> return err;
>> diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c
>> index 660268ff9562..a3a68889137b 100644
>> --- a/drivers/net/ethernet/amd/pds_core/main.c
>> +++ b/drivers/net/ethernet/amd/pds_core/main.c
>> @@ -190,7 +190,8 @@ static int pdsc_init_vf(struct pdsc *vf)
>> devl_unlock(dl);
>>
>> pf->vfs[vf->vf_id].vf = vf;
>> - err = pdsc_auxbus_dev_add(vf, pf);
>> + err = pdsc_auxbus_dev_add(vf, pf, PDS_DEV_TYPE_VDPA,
>> + &pf->vfs[vf->vf_id].padev);
>> if (err) {
>> devl_lock(dl);
>> devl_unregister(dl);
>> @@ -417,7 +418,7 @@ static void pdsc_remove(struct pci_dev *pdev)
>>
>> pf = pdsc_get_pf_struct(pdsc->pdev);
>> if (!IS_ERR(pf)) {
>> - pdsc_auxbus_dev_del(pdsc, pf);
>> + pdsc_auxbus_dev_del(pdsc, pf, &pf->vfs[pdsc->vf_id].padev);
>> pf->vfs[pdsc->vf_id].vf = NULL;
>> }
>> } else {
>> @@ -482,7 +483,8 @@ static void pdsc_reset_prepare(struct pci_dev *pdev)
>>
>> pf = pdsc_get_pf_struct(pdsc->pdev);
>> if (!IS_ERR(pf))
>> - pdsc_auxbus_dev_del(pdsc, pf);
>> + pdsc_auxbus_dev_del(pdsc, pf,
>> + &pf->vfs[pdsc->vf_id].padev);
>> }
>>
>> pdsc_unmap_bars(pdsc);
>> @@ -527,7 +529,8 @@ static void pdsc_reset_done(struct pci_dev *pdev)
>>
>> pf = pdsc_get_pf_struct(pdsc->pdev);
>> if (!IS_ERR(pf))
>> - pdsc_auxbus_dev_add(pdsc, pf);
>> + pdsc_auxbus_dev_add(pdsc, pf, PDS_DEV_TYPE_VDPA,
>> + &pf->vfs[pdsc->vf_id].padev);
>> }
>> }
>>
>
next prev parent reply other threads:[~2025-03-17 22:30 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-07 18:53 [PATCH v3 0/6] pds_fwctl: fwctl for AMD/Pensando core devices Shannon Nelson
2025-03-07 18:53 ` [PATCH v3 1/6] pds_core: make pdsc_auxbus_dev_del() void Shannon Nelson
2025-03-10 16:58 ` Dave Jiang
2025-03-07 18:53 ` [PATCH v3 2/6] pds_core: specify auxiliary_device to be created Shannon Nelson
2025-03-10 17:26 ` Dave Jiang
2025-03-17 22:30 ` Nelson, Shannon [this message]
2025-03-07 18:53 ` [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device Shannon Nelson
2025-03-10 17:33 ` Dave Jiang
2025-03-17 22:37 ` Nelson, Shannon
2025-03-12 18:22 ` Jonathan Cameron
2025-03-07 18:53 ` [PATCH v3 4/6] pds_fwctl: initial driver framework Shannon Nelson
2025-03-10 18:28 ` Dave Jiang
2025-03-17 22:40 ` Nelson, Shannon
2025-03-12 18:25 ` Jonathan Cameron
2025-03-07 18:53 ` [PATCH v3 5/6] pds_fwctl: add rpc and query support Shannon Nelson
2025-03-07 23:38 ` Jason Gunthorpe
2025-03-17 22:24 ` Nelson, Shannon
2025-03-12 18:29 ` Jonathan Cameron
2025-03-07 18:53 ` [PATCH v3 6/6] pds_fwctl: add Documentation entries Shannon Nelson
2025-03-10 20:27 ` Dave Jiang
2025-03-17 22:41 ` Nelson, Shannon
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=6dc6a934-e896-45c3-b74d-6adf0ba1e258@amd.com \
--to=shannon.nelson@amd.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=andrew.gospodarek@broadcom.com \
--cc=aron.silverton@oracle.com \
--cc=brett.creeley@amd.com \
--cc=dan.j.williams@intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dave.jiang@intel.com \
--cc=dsahern@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=hch@infradead.org \
--cc=itayavr@nvidia.com \
--cc=jgg@nvidia.com \
--cc=jiri@nvidia.com \
--cc=kuba@kernel.org \
--cc=lbloch@nvidia.com \
--cc=leonro@nvidia.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=saeedm@nvidia.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 NNTP newsgroup(s).