diff mbox series

[net-next,PatchV3] octeontx2-pf: Add support to read eeprom information

Message ID 20240227084722.27110-1-hkelam@marvell.com
State New
Headers
Series [net-next,PatchV3] octeontx2-pf: Add support to read eeprom information |

Commit Message

Hariprasad Kelam Feb. 27, 2024, 8:47 a.m. UTC 
  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.
V2 * consider length and offset fields while copying
     the eeprom data.

 .../marvell/octeontx2/nic/otx2_ethtool.c      | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

--
2.17.1

Comments

Jiri Pirko Feb. 27, 2024, 12:42 p.m. UTC | #1 
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>
Andrew Lunn Feb. 27, 2024, 3:47 p.m. UTC | #2 
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
Hariprasad Kelam Feb. 28, 2024, 10:45 a.m. UTC | #3 
> 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
Paolo Abeni Feb. 29, 2024, 9:14 a.m. UTC | #4 
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 Lunn Feb. 29, 2024, 3:26 p.m. UTC | #5 
> @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 mbox series

Patch

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)