| Message ID | 20240227084722.27110-1-hkelam@marvell.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,PatchV3] octeontx2-pf: Add support to read eeprom information | expand |
Tue, Feb 27, 2024 at 09:47:22AM CET, hkelam@marvell.com wrote: >Add support to read/decode EEPROM module information using ethtool. >Usage: ethtool -m <interface> > >Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> >Signed-off-by: Sunil Goutham <sgoutham@marvell.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
On Tue, Feb 27, 2024 at 02:17:22PM +0530, Hariprasad Kelam wrote: > Add support to read/decode EEPROM module information using ethtool. > Usage: ethtool -m <interface> > > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> > Signed-off-by: Sunil Goutham <sgoutham@marvell.com> > --- > V3 * remove redundant checks as stack is already doing it. So still only access to the first 256 bytes, using the old internal API. Disappointing. And the Signed-of-by: appear to be in the wrong order. Andrew
> On Tue, Feb 27, 2024 at 02:17:22PM +0530, Hariprasad Kelam wrote: > > Add support to read/decode EEPROM module information using ethtool. > > Usage: ethtool -m <interface> > > > > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> > > Signed-off-by: Sunil Goutham <sgoutham@marvell.com> > > --- > > V3 * remove redundant checks as stack is already doing it. > > So still only access to the first 256 bytes, using the old internal API. > Firmware and Kernel shares the data over a shared memory region. Firmware <-- Shared memory --> Kernel As per our design, firmware updates the shared memory region by reading eeprom data from the MAC. Upon receiving ethtool request to read eeprom data, Kernel maps this shared memory and copies the eeprom data to the user. Currently firmware supports updating only the first page of eeprom. Due to this we are limited to support the first page. , using the old internal API. While copying the data, the current patch does considers the offset/length fields. ethtool -m ethx offset x length x Could please point us what are we missing here? Thanks, Hariprasad k > Disappointing. > > And the Signed-of-by: appear to be in the wrong order. > > Andrew
On Tue, 2024-02-27 at 16:47 +0100, Andrew Lunn wrote: > On Tue, Feb 27, 2024 at 02:17:22PM +0530, Hariprasad Kelam wrote: > > Add support to read/decode EEPROM module information using ethtool. > > Usage: ethtool -m <interface> > > > > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> > > Signed-off-by: Sunil Goutham <sgoutham@marvell.com> > > --- > > V3 * remove redundant checks as stack is already doing it. > > So still only access to the first 256 bytes, using the old internal > API. > > Disappointing. > > And the Signed-of-by: appear to be in the wrong order. To clarify: If Sunil is the author of this patch, you should put his SoB first and add a From: tag before the changelog, see: https://elixir.bootlin.com/linux/v6.8-rc6/source/Documentation/process/6.Followthrough.rst#L199 @Andrew: it looks like the firmware interface is going to be limited for the time being, are ok with that? Cheers, Paolo
> @Andrew: it looks like the firmware interface is going to be limited > for the time being, are ok with that? Yes, i'm O.K. with it, just disappointed. Maybe the next version of the firmware will allow more? Andrew
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c index 7f786de61014..55ec0792038f 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c @@ -1184,6 +1184,38 @@ static void otx2_get_link_mode_info(u64 link_mode_bmap, otx2_link_modes); } +static int otx2_get_module_info(struct net_device *netdev, + struct ethtool_modinfo *modinfo) +{ + struct otx2_nic *pfvf = netdev_priv(netdev); + struct cgx_fw_data *rsp; + + rsp = otx2_get_fwdata(pfvf); + if (IS_ERR(rsp)) + return PTR_ERR(rsp); + + modinfo->type = rsp->fwdata.sfp_eeprom.sff_id; + modinfo->eeprom_len = SFP_EEPROM_SIZE; + return 0; +} + +static int otx2_get_module_eeprom(struct net_device *netdev, + struct ethtool_eeprom *ee, + u8 *data) +{ + struct otx2_nic *pfvf = netdev_priv(netdev); + struct cgx_fw_data *rsp; + + memset(data, 0, ee->len); + + rsp = otx2_get_fwdata(pfvf); + if (IS_ERR(rsp)) + return PTR_ERR(rsp); + + memcpy(data, &rsp->fwdata.sfp_eeprom.buf + ee->offset, ee->len); + return 0; +} + static int otx2_get_link_ksettings(struct net_device *netdev, struct ethtool_link_ksettings *cmd) { @@ -1342,6 +1374,8 @@ static const struct ethtool_ops otx2_ethtool_ops = { .set_fecparam = otx2_set_fecparam, .get_link_ksettings = otx2_get_link_ksettings, .set_link_ksettings = otx2_set_link_ksettings, + .get_module_info = otx2_get_module_info, + .get_module_eeprom = otx2_get_module_eeprom, }; void otx2_set_ethtool_ops(struct net_device *netdev)