Re: [PATCH v3 08/14] RDMA/ionic: Register auxiliary module for ionic ethernet adapter

Abhijit Gangurde
2025-06-27T10:18:30Z

On 6/26/25 12:56, Leon Romanovsky wrote:
> On Tue, Jun 24, 2025 at 05:43:09PM +0530, Abhijit Gangurde wrote:
>> Register auxiliary module to create ibdevice for ionic
>> ethernet adapter.
>>
>> Co-developed-by: Andrew Boyer <andrew.boyer@amd.com>
>> Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
>> Co-developed-by: Allen Hubbe <allen.hubbe@amd.com>
>> Signed-off-by: Allen Hubbe <allen.hubbe@amd.com>
>> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
>> ---
>> v1->v2
>> - Removed netdev references from ionic RDMA driver
>> - Moved to ionic_lif* instead of void* to convey information between
>> aux devices and drivers.
>>
>> drivers/infiniband/hw/ionic/ionic_ibdev.c | 133 ++++++++++++++++++++
>> drivers/infiniband/hw/ionic/ionic_ibdev.h | 21 ++++
>> drivers/infiniband/hw/ionic/ionic_lif_cfg.c | 118 +++++++++++++++++
>> drivers/infiniband/hw/ionic/ionic_lif_cfg.h | 65 ++++++++++
>> 4 files changed, 337 insertions(+)
>> create mode 100644 drivers/infiniband/hw/ionic/ionic_ibdev.c
>> create mode 100644 drivers/infiniband/hw/ionic/ionic_ibdev.h
>> create mode 100644 drivers/infiniband/hw/ionic/ionic_lif_cfg.c
>> create mode 100644 drivers/infiniband/hw/ionic/ionic_lif_cfg.h
> <...>
>
>> + rc = ionic_version_check(&ionic_adev->adev.dev, ionic_adev->lif);
>> + if (rc)
>> + return ERR_PTR(rc);
> <...>
>
>> +struct net_device *ionic_lif_netdev(struct ionic_lif *lif)
>> +{
>> + return lif->netdev;
>> +}
> Why do you need to store netdev pointer?
> Why can't you use existing ib_device_get_netdev/ib_device_set_netdev?

We are not storing the netdev in RDMA driver. This function is accessing
the ethernet copy to set netdev and guid. I can add dev_hold() here
till netdev is registered with ib device.

>
>> +
>> +int ionic_version_check(const struct device *dev, struct ionic_lif *lif)
>> +{
>> + union ionic_lif_identity *ident = &lif->ionic->ident.lif;
>> +
>> + if (ident->rdma.version < IONIC_MIN_RDMA_VERSION ||
>> + ident->rdma.version > IONIC_MAX_RDMA_VERSION) {
>> + dev_err_probe(dev, -EINVAL,
>> + "ionic_rdma: incompatible version, fw ver %u\n",
>> + ident->rdma.version);
>> + dev_err_probe(dev, -EINVAL,
>> + "ionic_rdma: Driver Min Version %u\n",
>> + IONIC_MIN_RDMA_VERSION);
>> + dev_err_probe(dev, -EINVAL,
>> + "ionic_rdma: Driver Max Version %u\n",
>> + IONIC_MAX_RDMA_VERSION);
>> + }
>> +
>> + return 0;
>> +}
> Upstream code has all subsystems in sync, and RDMA driver is always
> compatible with its netdev counterpart. Please remove this part.
>
> This is not full review yet, please wait till next week, so we will
> review more deeply.
>
> Thanks
I will remove this.

Thanks,
Abhijit