Re: [dpdk-dev] [PATCH v4 1/5] ethdev: add access to eeprom

["Zijie Pan" <zijie.pan@6wind.com>]

Hi Thomas,

> > +/**
> > + * Placeholder for accessing plugin module eeprom
> > + */
> > +struct rte_dev_module_info {
> > + uint32_t type; /**< Type of plugin module eeprom */
> > + uint32_t eeprom_len; /**< Length of plugin module eeprom */
> > +};
>
> I am not sure "plugin module" is descriptive enough.
Here is the description when "man ethtool":
-m --dump-module-eeprom --module-info
    Retrieves  and if possible decodes the EEPROM from plugin modules, e.g SFP+, QSFP.
Is there any suggestion how to describe it?

> And I think the structure name should be rte_eth_dev_module_info
> to make clear that we are talking about NIC modules.
> Any better idea?
Yes, rte_eth_dev_module_info is better than rte_dev_module_info.
But the name of other structures should be also changed. And the file name
should be changed to rte_eth_dev_info.h.
Do we need to make a patch first to change the name of the file and structures?

> > +int __rte_experimental
> > +rte_eth_dev_get_module_eeprom(uint16_t port_id,
> > +       struct rte_dev_eeprom_info *info)
> > +{
> > + struct rte_eth_dev *dev;
> > +
> > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +
> > + dev = &rte_eth_devices[port_id];
> > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_module_eeprom, -ENOTSUP);
> > + return (*dev->dev_ops->get_module_eeprom)(dev, info);
> > +}
> > +
> >  int
> >  rte_eth_dev_get_dcb_info(uint16_t port_id,
>
> Please move this code after other EEPROM related functions.
I put the functions after rte_eth_dev_set_eeprom(). Please apply the patch and check it.

> > --- a/lib/librte_ether/rte_ethdev_version.map
> > +++ b/lib/librte_ether/rte_ethdev_version.map
> > @@ -229,5 +229,7 @@ EXPERIMENTAL {
> >  rte_mtr_policer_actions_update;
> >  rte_mtr_stats_read;
> >  rte_mtr_stats_update;
> > + rte_eth_dev_get_module_info;
> > + rte_eth_dev_get_module_eeprom;
>
> This must be inserted in alphabetical order.
I will update it in v5 patchset.

Thanks & Regards,
Zijie