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

[Abhijit Gangurde <abhijit.gangurde@amd.com>]

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