From 09612aaf05ebb70975d7361c09039eb910f41cc3 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Mon, 11 Sep 2023 12:00:14 +0800
Subject: [PATCH 01/17] Support reading/writing module EEPROM data by page and
 offset

---
 sfputil/main.py       | 127 +++++++++++++++++++++++++++++++++++-------
 tests/sfputil_test.py |  93 ++++++++++++++++++++++++++++++-
 2 files changed, 198 insertions(+), 22 deletions(-)

diff --git a/sfputil/main.py b/sfputil/main.py
index 51d5af4895..bcfe344bae 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -793,33 +793,42 @@ def eeprom_hexdump_sff8636(port, physical_port, page):
 
     return output
 
+
 def convert_byte_to_valid_ascii_char(byte):
     if byte < 32 or 126 < byte:
         return '.'
     else:
         return chr(byte)
 
+
 def hexdump(indent, data, mem_address):
-    ascii_string = ''
-    result = ''
-    for byte in data:
-        ascii_string = ascii_string + convert_byte_to_valid_ascii_char(byte)
-        byte_string = "{:02x}".format(byte)
-        if mem_address % 16 == 0:
-            mem_address_string = "{:08x}".format(mem_address)
-            result += '\n{}{} '.format(indent, mem_address_string)
-            result += '{} '.format(byte_string)
-        elif mem_address % 16 == 15:
-            result += '{} '.format(byte_string)
-            result += '|{}|'.format(ascii_string)
-            ascii_string = ""
-        elif mem_address % 16 == 8:
-            result += ' {} '.format(byte_string)
+    size = len(data)
+    offset = 0
+    lines = []
+    while size > 0:
+        offset_str = "{}{:08x}".format(indent, mem_address)
+        if size >= 16:
+            first_half = ' '.join("{:02x}".format(x) for x in data[offset:offset + 8])
+            second_half = ' '.join("{:02x}".format(x) for x in data[offset + 8:offset + 16])
+            ascii_str = ''.join(convert_byte_to_valid_ascii_char(x) for x in data[offset:offset + 16])
+            lines.append(f'{offset_str} {first_half}  {second_half} |{ascii_str}|')
+        elif size > 8:
+            first_half = ' '.join("{:02x}".format(x) for x in data[offset:offset + 8])
+            second_half = ' '.join("{:02x}".format(x) for x in data[offset + 8:offset + size])
+            padding = '   ' * (16 - size)
+            ascii_str = ''.join(convert_byte_to_valid_ascii_char(x) for x in data[offset:offset + size])
+            lines.append(f'{offset_str} {first_half}  {second_half}{padding} |{ascii_str}|')
+            break
         else:
-            result += '{} '.format(byte_string)
-        mem_address += 1
-
-    return result
+            hex_part = ' '.join("{:02x}".format(x) for x in data[offset:offset + size])
+            padding = '   ' * (16 - size)
+            ascii_str = ''.join(convert_byte_to_valid_ascii_char(x) for x in data[offset:offset + size])
+            lines.append(f'{offset_str} {hex_part} {padding} |{ascii_str}|')
+            break
+        size -= 16
+        offset += 16
+        mem_address += 16
+    return '\n'.join(lines)
 
 # 'presence' subcommand
 @show.command()
@@ -1253,10 +1262,10 @@ def is_fw_switch_done(port_name):
                 status = -1 # Abnormal status.
             elif (ImageARunning == 1) and (ImageACommitted == 0):   # ImageA is running, but not committed.
                 click.echo("FW images switch successful : ImageA is running")
-                status = 1  # run_firmware is done. 
+                status = 1  # run_firmware is done.
             elif (ImageBRunning == 1) and (ImageBCommitted == 0):   # ImageB is running, but not committed.
                 click.echo("FW images switch successful : ImageB is running")
-                status = 1  # run_firmware is done. 
+                status = 1  # run_firmware is done.
             else:                                                   # No image is running, or running and committed image is same.
                 click.echo("FW info error : Failed to switch into uncommitted image!")
                 status = -1 # Failure for Switching images.
@@ -1514,6 +1523,82 @@ def unlock(port_name, password):
     else:
         click.echo("CDB: Host password NOT accepted! status = {}".format(status))
 
+
+# 'read-eeprom' subcommand
+@cli.command()
+@click.argument('port_name', metavar='<port_name>', required=True)
+@click.argument('page', metavar='<page>', type=click.INT, required=True)
+@click.argument('offset', metavar='<offset>', type=click.INT, required=True)
+@click.argument('size', metavar='<size>', type=click.INT, required=True)
+@click.option('--no-format', type=click.INT, is_flag=True, help="Display non formatted data")
+@click.option('--wire-addr', help="Wire address of sff8472")
+def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
+    """Read SFP EEPROM data"""
+    if is_port_type_rj45(port_name):
+        click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
+        sys.exit(EXIT_FAIL)
+
+    physical_port = logical_port_to_physical_port_index(port_name)
+    sfp = platform_chassis.get_sfp(physical_port)
+    if not sfp.get_presence():
+        click.echo("{}: SFP EEPROM not detected\n".format(port_name))
+        sys.exit(EXIT_FAIL)
+
+    try:
+        data = sfp.read_eeprom_by_page(page, offset, size, wire_addr)
+        if data is None:
+            click.echo("Error: Failed to read EEPROM!")
+            sys.exit(ERROR_NOT_IMPLEMENTED)
+        if no_format:
+            click.echo(''.join('{:02x}'.format(x) for x in data))
+        else:
+            click.echo(hexdump('', data, offset))
+    except NotImplementedError:
+        click.echo("This functionality is currently not implemented for this platform")
+        sys.exit(ERROR_NOT_IMPLEMENTED)
+    except ValueError as e:
+        click.echo("Error: {}".format(e))
+        sys.exit(EXIT_FAIL)
+
+
+# 'write-eeprom' subcommand
+@cli.command()
+@click.argument('port_name', metavar='<port_name>', required=True)
+@click.argument('page', metavar='<page>', type=click.INT, required=True)
+@click.argument('offset', metavar='<offset>', type=click.INT, required=True)
+@click.argument('data', metavar='<data>', required=True)
+@click.option('--wire-addr', help="Wire address of sff8472")
+def write_eeprom(port_name, page, offset, data, wire_addr):
+    """Write SFP EEPROM data"""
+    if is_port_type_rj45(port_name):
+        click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
+        sys.exit(EXIT_FAIL)
+
+    physical_port = logical_port_to_physical_port_index(port_name)
+    sfp = platform_chassis.get_sfp(physical_port)
+    if not sfp.get_presence():
+        click.echo("{}: SFP EEPROM not detected\n".format(port_name))
+        sys.exit(EXIT_FAIL)
+
+    try:
+        bytes = bytearray.fromhex(data)
+    except ValueError:
+        click.echo("Error: Data must be a hex string of even length!")
+        sys.exit(EXIT_FAIL)
+
+    try:
+        success = sfp.write_eeprom_by_page(page, offset, bytes, wire_addr)
+        if not success:
+            click.echo("Error: Failed to write EEPROM!")
+            sys.exit(ERROR_NOT_IMPLEMENTED)
+    except NotImplementedError:
+        click.echo("This functionality is currently not implemented for this platform")
+        sys.exit(ERROR_NOT_IMPLEMENTED)
+    except ValueError as e:
+        click.echo("Error: {}".format(e))
+        sys.exit(EXIT_FAIL)
+
+
 # 'version' subcommand
 @cli.command()
 def version():
diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index f8917d9c44..834c01f2fe 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -256,7 +256,7 @@ def test_convert_sfp_info_to_output_string(self, sfp_info_dict, expected_output)
                 Vcc: 3.2577Volts
         ModuleThresholdValues:
 '''
-        ), 
+        ),
         (
             'QSFP-DD Double Density 8X Pluggable Transceiver',
             {
@@ -950,3 +950,94 @@ def test_update_firmware_info_to_state_db(self, mock_chassis):
         mock_sfp.get_transceiver_info_firmware_versions.return_value = ['a.b.c', 'd.e.f']
 
         sfputil.update_firmware_info_to_state_db("Ethernet0")
+
+    @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=False))
+    @patch('sfputil.main.logical_port_to_physical_port_index', MagicMock(return_value=1))
+    @patch('sfputil.main.platform_chassis')
+    def test_read_eeprom(self, mock_chassis):
+        mock_sfp = MagicMock()
+        mock_chassis.get_sfp = MagicMock(return_value=mock_sfp)
+
+        mock_sfp.get_presence = MagicMock(return_value=False)
+        runner = CliRunner()
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
+        assert result.exit_code == EXIT_FAIL
+
+        mock_sfp.get_presence.return_value = True
+        mock_sfp.read_eeprom_by_page = MagicMock(return_value=None)
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
+        assert result.exit_code == ERROR_NOT_IMPLEMENTED
+
+        mock_sfp.read_eeprom_by_page.return_value = bytearray([0x00, 0x01])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '2', '--no-format'])
+        assert result.exit_code == 0
+        assert result.output == '0001\n'
+
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
+        assert result.exit_code == 0
+        expected_output = """00000005 00 01                                            |..|
+"""
+        print(result.output)
+        assert result.output == expected_output
+
+        mock_sfp.read_eeprom_by_page.side_effect = NotImplementedError
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
+        assert result.exit_code == ERROR_NOT_IMPLEMENTED
+
+        mock_sfp.read_eeprom_by_page.side_effect = ValueError
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
+        assert result.exit_code == EXIT_FAIL
+
+    @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=False))
+    @patch('sfputil.main.logical_port_to_physical_port_index', MagicMock(return_value=1))
+    @patch('sfputil.main.platform_chassis')
+    def test_write_eeprom(self, mock_chassis):
+        mock_sfp = MagicMock()
+        mock_chassis.get_sfp = MagicMock(return_value=mock_sfp)
+
+        mock_sfp.get_presence = MagicMock(return_value=False)
+        runner = CliRunner()
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '01'])
+        assert result.exit_code == EXIT_FAIL
+
+        # invalid hex string, hex string must have even length
+        mock_sfp.get_presence.return_value = True
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '1'])
+        assert result.exit_code == EXIT_FAIL
+
+        # invalid hex string
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '+0'])
+        assert result.exit_code == EXIT_FAIL
+
+        # write failed
+        mock_sfp.write_eeprom_by_page = MagicMock(return_value=False)
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
+        print(result.output)
+        assert result.exit_code == ERROR_NOT_IMPLEMENTED
+
+        # write success
+        mock_sfp.write_eeprom_by_page.return_value = True
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
+        assert result.exit_code == 0
+
+        # Not implemented
+        mock_sfp.write_eeprom_by_page.side_effect = NotImplementedError
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
+        assert result.exit_code == ERROR_NOT_IMPLEMENTED
+
+        # Value error
+        mock_sfp.write_eeprom_by_page.side_effect = ValueError
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
+        assert result.exit_code == EXIT_FAIL
+
+    @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=True))
+    def test_read_eeprom_rj45(self):
+        runner = CliRunner()
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
+        assert result.exit_code == EXIT_FAIL
+
+    @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=True))
+    def test_write_eeprom_rj45(self):
+        runner = CliRunner()
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '00'])
+        assert result.exit_code == EXIT_FAIL

From fbaaa34b171f36775ed17aae10a85f0f0f25f2cf Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Tue, 10 Oct 2023 12:10:34 +0300
Subject: [PATCH 02/17] Fix unit test issue

---
 sfputil/main.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sfputil/main.py b/sfputil/main.py
index bcfe344bae..7171bcd7ad 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -804,7 +804,7 @@ def convert_byte_to_valid_ascii_char(byte):
 def hexdump(indent, data, mem_address):
     size = len(data)
     offset = 0
-    lines = []
+    lines = ['']
     while size > 0:
         offset_str = "{}{:08x}".format(indent, mem_address)
         if size >= 16:

From ccf305703023092ead19411e0b584c56c2be7a9e Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Tue, 10 Oct 2023 12:33:18 +0300
Subject: [PATCH 03/17] Fix unit test issue

---
 tests/sfputil_test.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index 834c01f2fe..c5d77d9cb1 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -975,7 +975,8 @@ def test_read_eeprom(self, mock_chassis):
 
         result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
         assert result.exit_code == 0
-        expected_output = """00000005 00 01                                            |..|
+        expected_output = """
+00000005 00 01                                            |..|
 """
         print(result.output)
         assert result.output == expected_output

From 929ed19221c8e4665179ebce2a7e7d701060a911 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Wed, 18 Oct 2023 05:59:34 +0300
Subject: [PATCH 04/17] Fix review comment

---
 sfputil/main.py       | 10 ++++++++--
 tests/sfputil_test.py | 10 ++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/sfputil/main.py b/sfputil/main.py
index 7171bcd7ad..c066b102ac 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -1530,7 +1530,7 @@ def unlock(port_name, password):
 @click.argument('page', metavar='<page>', type=click.INT, required=True)
 @click.argument('offset', metavar='<offset>', type=click.INT, required=True)
 @click.argument('size', metavar='<size>', type=click.INT, required=True)
-@click.option('--no-format', type=click.INT, is_flag=True, help="Display non formatted data")
+@click.option('--no-format', is_flag=True, help="Display non formatted data")
 @click.option('--wire-addr', help="Wire address of sff8472")
 def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
     """Read SFP EEPROM data"""
@@ -1568,7 +1568,8 @@ def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
 @click.argument('offset', metavar='<offset>', type=click.INT, required=True)
 @click.argument('data', metavar='<data>', required=True)
 @click.option('--wire-addr', help="Wire address of sff8472")
-def write_eeprom(port_name, page, offset, data, wire_addr):
+@click.option('--verify', is_flag=True, help="Verify the data by reading back")
+def write_eeprom(port_name, page, offset, data, wire_addr, verify):
     """Write SFP EEPROM data"""
     if is_port_type_rj45(port_name):
         click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
@@ -1591,6 +1592,11 @@ def write_eeprom(port_name, page, offset, data, wire_addr):
         if not success:
             click.echo("Error: Failed to write EEPROM!")
             sys.exit(ERROR_NOT_IMPLEMENTED)
+        if verify:
+            read_data = sfp.read_eeprom_by_page(page, offset, len(bytes), wire_addr)
+            if read_data != bytes:
+                click.echo(f"Error: Write data failed! Write: {''.join('{:02x}'.format(x) for x in bytes)}, read: {''.join('{:02x}'.format(x) for x in read_data)}")
+                sys.exit(EXIT_FAIL)
     except NotImplementedError:
         click.echo("This functionality is currently not implemented for this platform")
         sys.exit(ERROR_NOT_IMPLEMENTED)
diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index c5d77d9cb1..cb71128c9d 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -1021,6 +1021,16 @@ def test_write_eeprom(self, mock_chassis):
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
         assert result.exit_code == 0
 
+        # write verify success
+        mock_sfp.read_eeprom_by_page = MagicMock(return_value=bytearray([16]))
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10', '--verify'])
+        assert result.exit_code == 0
+
+        # write verify failed
+        mock_sfp.read_eeprom_by_page = MagicMock(return_value=bytearray([10]))
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '11', '--verify'])
+        assert result.exit_code != 0
+
         # Not implemented
         mock_sfp.write_eeprom_by_page.side_effect = NotImplementedError
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])

From b7869d0280b47ea99d56c3ea9b369f31463dca35 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Thu, 19 Oct 2023 06:55:48 +0300
Subject: [PATCH 05/17] Update command ref

---
 doc/Command-Reference.md | 58 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md
index 429fe39bbf..d90acd2ebe 100644
--- a/doc/Command-Reference.md
+++ b/doc/Command-Reference.md
@@ -200,6 +200,9 @@
   * [MACsec config command](#macsec-config-command)
   * [MACsec show command](#macsec-show-command)
   * [MACsec clear command](#macsec-clear-command)
+* [SFP Utilities Commands](#sfp-utilities-commands)
+  * [SFP Utilities read command](#sfp-utilities-read-command)
+  * [SFP Utilities write command](#sfp-utilities-write-command)
 * [Static DNS Commands](#static-dns-commands)
   * [Static DNS config command](#static-dns-config-command)
   * [Static DNS show command](#static-dns-show-command)
@@ -12748,6 +12751,61 @@ Clear MACsec counters which is to reset all MACsec counters to ZERO.
 
 Go Back To [Beginning of the document](#) or [Beginning of this section](#macsec-commands)
 
+# SFP Utilities Commands
+
+This sub-section explains the list of commands available for SFP utilities feature.
+
+# SFP Utilities read command
+
+- Read SFP EEPROM data
+
+```
+admin@sonic:~$ sfputil read-eeprom --help
+Usage: sfputil read-eeprom [OPTIONS] <port_name> <page> <offset> <size>
+  Read SFP EEPROM data
+Options:
+  --no-format             Display non formatted data.
+  --wire-addr             Wire address of sff8472.
+  --help                  Show this 
+```
+
+```
+admin@sonic:~$ sfputil read-eeprom Ethernet0 0 100 2
+00000064 4a 44                                            |..|
+
+admin@sonic:~$ sfputil read-eeprom Ethernet0 0 0 32
+00000000 11 08 06 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
+00000010 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
+
+admin@sonic:~$ sfputil read-eeprom Ethernet0 0 100 2 --no-format
+4a44
+```
+
+# SFP Utilities write command
+
+- Write SFP EEPROM data
+
+```
+admin@sonic:~$ sfputil write-eeprom --help
+Usage: sfputil write-eeprom [OPTIONS] <port> <page> <offset> <data>
+
+  Write SFP EEPROM data
+
+Options:
+  --wire-addr             Wire address of sff8472.
+  --verify                Verify the data by reading back.
+  --help                  Show this message and exit.
+```
+
+```
+admin@sonic:~$ sfputil write-eeprom Ethernet0 0 100 4a44
+
+admin@sonic:~$ sfputil write-eeprom Etherent0 0 100 4a44 --verify
+Error: Write data failed! Write: 4a44, read: 0000.
+```
+
+Go Back To [Beginning of the document](#) or [Beginning of this section](#sfp-utilities-commands)
+
 # Static DNS Commands
 
 This sub-section explains the list of the configuration options available for static DNS feature.

From b6dc65b62fbcce5c3ca9f60f667d4e32d68e1792 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Thu, 19 Oct 2023 07:19:11 +0300
Subject: [PATCH 06/17] Fix format

---
 sfputil/main.py       | 6 ++----
 tests/sfputil_test.py | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/sfputil/main.py b/sfputil/main.py
index c066b102ac..e0522c98c8 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -793,14 +793,12 @@ def eeprom_hexdump_sff8636(port, physical_port, page):
 
     return output
 
-
 def convert_byte_to_valid_ascii_char(byte):
     if byte < 32 or 126 < byte:
         return '.'
     else:
         return chr(byte)
 
-
 def hexdump(indent, data, mem_address):
     size = len(data)
     offset = 0
@@ -1262,10 +1260,10 @@ def is_fw_switch_done(port_name):
                 status = -1 # Abnormal status.
             elif (ImageARunning == 1) and (ImageACommitted == 0):   # ImageA is running, but not committed.
                 click.echo("FW images switch successful : ImageA is running")
-                status = 1  # run_firmware is done.
+                status = 1  # run_firmware is done. 
             elif (ImageBRunning == 1) and (ImageBCommitted == 0):   # ImageB is running, but not committed.
                 click.echo("FW images switch successful : ImageB is running")
-                status = 1  # run_firmware is done.
+                status = 1  # run_firmware is done. 
             else:                                                   # No image is running, or running and committed image is same.
                 click.echo("FW info error : Failed to switch into uncommitted image!")
                 status = -1 # Failure for Switching images.
diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index cb71128c9d..bee4c3aa1c 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -256,7 +256,7 @@ def test_convert_sfp_info_to_output_string(self, sfp_info_dict, expected_output)
                 Vcc: 3.2577Volts
         ModuleThresholdValues:
 '''
-        ),
+        ), 
         (
             'QSFP-DD Double Density 8X Pluggable Transceiver',
             {

From 782efc1e3c92ac3f93958c1b7b6329df4c5f850d Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com>
Date: Thu, 19 Oct 2023 12:39:25 +0800
Subject: [PATCH 07/17] Update main.py

---
 sfputil/main.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sfputil/main.py b/sfputil/main.py
index e0522c98c8..415bb80be2 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -799,10 +799,10 @@ def convert_byte_to_valid_ascii_char(byte):
     else:
         return chr(byte)
 
-def hexdump(indent, data, mem_address):
+def hexdump(indent, data, mem_address, start_newline=True):
     size = len(data)
     offset = 0
-    lines = ['']
+    lines = [''] if start_newline else []
     while size > 0:
         offset_str = "{}{:08x}".format(indent, mem_address)
         if size >= 16:

From e5bcf0989d28cab67dcfb0d7b8d48580a7302d7c Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Fri, 20 Oct 2023 04:52:41 +0300
Subject: [PATCH 08/17] Fix review comments

---
 sfputil/main.py       | 10 ++++++++++
 tests/sfputil_test.py | 16 ++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/sfputil/main.py b/sfputil/main.py
index e0522c98c8..0a1b2e50eb 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -1532,6 +1532,11 @@ def unlock(port_name, password):
 @click.option('--wire-addr', help="Wire address of sff8472")
 def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
     """Read SFP EEPROM data"""
+    if platform_sfputil.is_logical_port(port_name) == 0:
+        click.echo("Error: invalid port {}".format(port_name))
+        print_all_valid_port_values()
+        sys.exit(ERROR_INVALID_PORT)
+        
     if is_port_type_rj45(port_name):
         click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
         sys.exit(EXIT_FAIL)
@@ -1569,6 +1574,11 @@ def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
 @click.option('--verify', is_flag=True, help="Verify the data by reading back")
 def write_eeprom(port_name, page, offset, data, wire_addr, verify):
     """Write SFP EEPROM data"""
+    if platform_sfputil.is_logical_port(port_name) == 0:
+        click.echo("Error: invalid port {}".format(port_name))
+        print_all_valid_port_values()
+        sys.exit(ERROR_INVALID_PORT)
+
     if is_port_type_rj45(port_name):
         click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
         sys.exit(EXIT_FAIL)
diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index bee4c3aa1c..741f9d40eb 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -953,6 +953,7 @@ def test_update_firmware_info_to_state_db(self, mock_chassis):
 
     @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=False))
     @patch('sfputil.main.logical_port_to_physical_port_index', MagicMock(return_value=1))
+    @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1)))
     @patch('sfputil.main.platform_chassis')
     def test_read_eeprom(self, mock_chassis):
         mock_sfp = MagicMock()
@@ -991,6 +992,7 @@ def test_read_eeprom(self, mock_chassis):
 
     @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=False))
     @patch('sfputil.main.logical_port_to_physical_port_index', MagicMock(return_value=1))
+    @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1)))
     @patch('sfputil.main.platform_chassis')
     def test_write_eeprom(self, mock_chassis):
         mock_sfp = MagicMock()
@@ -1041,13 +1043,27 @@ def test_write_eeprom(self, mock_chassis):
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
         assert result.exit_code == EXIT_FAIL
 
+    @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=0)))
+    def test_read_eeprom_invalid_port(self):
+        runner = CliRunner()
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
+        assert result.exit_code == ERROR_INVALID_PORT
+
+    @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=0)))
+    def test_write_eeprom_invalid_port(self):
+        runner = CliRunner()
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '00'])
+        assert result.exit_code == ERROR_INVALID_PORT
+
     @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=True))
+    @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1)))
     def test_read_eeprom_rj45(self):
         runner = CliRunner()
         result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
         assert result.exit_code == EXIT_FAIL
 
     @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=True))
+    @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1)))
     def test_write_eeprom_rj45(self):
         runner = CliRunner()
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '00'])

From 8c69ad5490fe7d40c87505b767f5880cebc285cf Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Tue, 21 Nov 2023 04:55:54 +0200
Subject: [PATCH 09/17] Fix review comment

---
 doc/Command-Reference.md |  30 ++---
 sfputil/main.py          | 280 ++++++++++++++++++++++++++++-----------
 tests/sfputil_test.py    |  99 ++++++++++++--
 3 files changed, 301 insertions(+), 108 deletions(-)

diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md
index 9d91cf5e1c..aaf6ca7535 100644
--- a/doc/Command-Reference.md
+++ b/doc/Command-Reference.md
@@ -560,7 +560,7 @@ This command displays the current date and time configured on the system
   ```
 
 **config clock date**
-	
+
 This command will set the date-time of the systetm, given strings with date-time format <YYYY-MM-DD> <HH:MM:SS>
 
 - Usage:
@@ -578,7 +578,7 @@ This command will set the date-time of the systetm, given strings with date-time
   ```
 
 **config clock timezone**
-	
+
 This command will set the timezone of the systetm, given a string of a valid timezone.
 
 - Usage:
@@ -588,13 +588,13 @@ This command will set the timezone of the systetm, given a string of a valid tim
 
 - Parameters:
   - _timezone_: valid timezone to be configured
-	
-	
+
+
 - Example:
   ```
   admin@sonic:~$ config clock timezone Africa/Accra
 
-	
+
 **show clock timezones**
 
 This command Will display list of all valid timezones to be configured.
@@ -2571,7 +2571,7 @@ Once enabled, BGP will not advertise routes which aren't yet offloaded.
   admin@sonic:~$ sudo config suppress-fib-pending enabled
   ```
   ```
-  admin@sonic:~$ sudo config suppress-fib-pending disabled 
+  admin@sonic:~$ sudo config suppress-fib-pending disabled
   ```
 
 Go Back To [Beginning of the document](#) or [Beginning of this section](#bgp)
@@ -2851,7 +2851,7 @@ This command is used for downloading firmware tp upgrade the transciever module.
   ```
 **sfputil firmware run**
 
-This command is used to start and run a downloaded image. This command transfers control from the currently running firmware to a new firmware. 
+This command is used to start and run a downloaded image. This command transfers control from the currently running firmware to a new firmware.
 
 - Usage:
   ```
@@ -2876,7 +2876,7 @@ This command is used to start and run a downloaded image. This command transfers
 
 **sfputil firmware commit**
 
-This command to commit the running image so that the module will boot from it on future boots. 
+This command to commit the running image so that the module will boot from it on future boots.
 
 - Usage:
   ```
@@ -4597,7 +4597,7 @@ This command is to display the FEC status of the selected interfaces. If **inter
   show interfaces fec status [<interface_name>]
   ```
 
-- Example:  
+- Example:
 ```
   admin@sonic:~$ show interfaces fec status
   Interface    FEC Oper    FEC Admin
@@ -6849,7 +6849,7 @@ in order to detemine whether the health of the cable is Ok
 the following are checked
 - the vendor name is correct able to be read
 - the FW is correctly loaded for SerDes by reading the appropriate register val
-- the Counters for UART are displaying healthy status 
+- the Counters for UART are displaying healthy status
        i.e Error Counters , retry Counters for UART or internal xfer protocols are below a threshold
 
 
@@ -6905,7 +6905,7 @@ the result will be displayed like this, each item in the dictionary shows the he
            {
                "uart_stat1": "2",
                "uart_stat2": "1",
-                 
+
            }
     ```
 
@@ -12811,16 +12811,16 @@ Usage: sfputil read-eeprom [OPTIONS] <port_name> <page> <offset> <size>
 Options:
   --no-format             Display non formatted data.
   --wire-addr             Wire address of sff8472.
-  --help                  Show this 
+  --help                  Show this
 ```
 
 ```
 admin@sonic:~$ sfputil read-eeprom Ethernet0 0 100 2
-00000064 4a 44                                            |..|
+        00000064 4a 44                                            |..|
 
 admin@sonic:~$ sfputil read-eeprom Ethernet0 0 0 32
-00000000 11 08 06 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
-00000010 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
+        00000000 11 08 06 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
+        00000010 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
 
 admin@sonic:~$ sfputil read-eeprom Ethernet0 0 100 2 --no-format
 4a44
diff --git a/sfputil/main.py b/sfputil/main.py
index 1eeb10eb51..47d20368f0 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -48,8 +48,17 @@
 
 PAGE_SIZE = 128
 PAGE_OFFSET = 128
-
 SFF8472_A0_SIZE = 256
+MAX_EEPROM_PAGE = 255
+MAX_EEPROM_OFFSET = 255
+MIN_OFFSET_FOR_PAGE0 = 0
+MIN_OFFSET_FOR_OTHER_PAGE = 128
+MAX_OFFSET_FOR_A0H_UPPER_PAGE = 255
+MAX_OFFSET_FOR_A0H_LOWER_PAGE = 127
+MAX_OFFSET_FOR_A2H = 255
+PAGE_SIZE_FOR_A0H = 256
+
+EEPROM_DUMP_INDENT = ' ' * 8
 
 # TODO: We should share these maps and the formatting functions between sfputil and sfpshow
 QSFP_DD_DATA_MAP = {
@@ -793,6 +802,28 @@ def eeprom_hexdump_sff8636(port, physical_port, page):
 
     return output
 
+
+def eeprom_dump_general(physical_port, page, overall_offset, size, page_offset, no_format=False):
+    """
+    Dump module EEPROM for given pages in hex format. This function is designed for SFF8472 only.
+    Args:
+        logical_port_name: logical port name
+        pages: a list of pages to be dumped. The list always include a default page list and the target_page input by
+               user
+        target_page: user input page number, optional. target_page is only for display purpose
+    Returns:
+        tuple(0, dump string) if success else tuple(error_code, error_message)
+    """
+    sfp = platform_chassis.get_sfp(physical_port)
+    page_dump = sfp.read_eeprom(overall_offset, size)
+    if page_dump is None:
+        return ERROR_NOT_IMPLEMENTED, f'Error: Failed to read EEPROM for page {page:x}h, overall_offset {overall_offset}, page_offset {page_offset}, size {size}!'
+    if not no_format:
+        return 0, hexdump(EEPROM_DUMP_INDENT, page_dump, page_offset, start_newline=False)
+    else:
+        return 0, ''.join('{:02x}'.format(x) for x in page_dump)
+
+
 def convert_byte_to_valid_ascii_char(byte):
     if byte < 32 or 126 < byte:
         return '.'
@@ -1260,10 +1291,10 @@ def is_fw_switch_done(port_name):
                 status = -1 # Abnormal status.
             elif (ImageARunning == 1) and (ImageACommitted == 0):   # ImageA is running, but not committed.
                 click.echo("FW images switch successful : ImageA is running")
-                status = 1  # run_firmware is done. 
+                status = 1  # run_firmware is done.
             elif (ImageBRunning == 1) and (ImageBCommitted == 0):   # ImageB is running, but not committed.
                 click.echo("FW images switch successful : ImageB is running")
-                status = 1  # run_firmware is done. 
+                status = 1  # run_firmware is done.
             else:                                                   # No image is running, or running and committed image is same.
                 click.echo("FW info error : Failed to switch into uncommitted image!")
                 status = -1 # Failure for Switching images.
@@ -1527,6 +1558,50 @@ def unlock(port_name, password):
     else:
         click.echo("CDB: Host password NOT accepted! status = {}".format(status))
 
+# 'version' subcommand
+@cli.command()
+def version():
+    """Display version info"""
+    click.echo("sfputil version {0}".format(VERSION))
+
+# 'target' subcommand
+@firmware.command()
+@click.argument('port_name', required=True, default=None)
+@click.argument('target', type=click.IntRange(0, 2), required=True, default=None)
+def target(port_name, target):
+    """Select target end for firmware download 0-(local) \n
+                                               1-(remote-A) \n
+                                               2-(remote-B)
+    """
+    physical_port = logical_port_to_physical_port_index(port_name)
+    sfp = platform_chassis.get_sfp(physical_port)
+
+    if is_port_type_rj45(port_name):
+        click.echo("{}: This functionality is not applicable for RJ45 port".format(port_name))
+        sys.exit(EXIT_FAIL)
+
+    if not is_sfp_present(port_name):
+       click.echo("{}: SFP EEPROM not detected\n".format(port_name))
+       sys.exit(EXIT_FAIL)
+
+    try:
+        api = sfp.get_xcvr_api()
+    except NotImplementedError:
+        click.echo("{}: This functionality is currently not implemented for this module".format(port_name))
+        sys.exit(ERROR_NOT_IMPLEMENTED)
+
+    try:
+        status = api.set_firmware_download_target_end(target)
+    except AttributeError:
+        click.echo("{}: This functionality is not applicable for this module".format(port_name))
+        sys.exit(ERROR_NOT_IMPLEMENTED)
+
+    if status:
+        click.echo("Target Mode set to {}". format(target))
+    else:
+        click.echo("Target Mode set failed!")
+        sys.exit(EXIT_FAIL)
+
 
 # 'read-eeprom' subcommand
 @cli.command()
@@ -1538,35 +1613,40 @@ def unlock(port_name, password):
 @click.option('--wire-addr', help="Wire address of sff8472")
 def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
     """Read SFP EEPROM data"""
-    if platform_sfputil.is_logical_port(port_name) == 0:
-        click.echo("Error: invalid port {}".format(port_name))
-        print_all_valid_port_values()
-        sys.exit(ERROR_INVALID_PORT)
-        
-    if is_port_type_rj45(port_name):
-        click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
-        sys.exit(EXIT_FAIL)
+    try:
+        if platform_sfputil.is_logical_port(port_name) == 0:
+            click.echo("Error: invalid port {}".format(port_name))
+            print_all_valid_port_values()
+            sys.exit(ERROR_INVALID_PORT)
 
-    physical_port = logical_port_to_physical_port_index(port_name)
-    sfp = platform_chassis.get_sfp(physical_port)
-    if not sfp.get_presence():
-        click.echo("{}: SFP EEPROM not detected\n".format(port_name))
-        sys.exit(EXIT_FAIL)
+        if is_port_type_rj45(port_name):
+            click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
+            sys.exit(EXIT_FAIL)
 
-    try:
-        data = sfp.read_eeprom_by_page(page, offset, size, wire_addr)
-        if data is None:
-            click.echo("Error: Failed to read EEPROM!")
-            sys.exit(ERROR_NOT_IMPLEMENTED)
-        if no_format:
-            click.echo(''.join('{:02x}'.format(x) for x in data))
+        physical_port = logical_port_to_physical_port_index(port_name)
+        sfp = platform_chassis.get_sfp(physical_port)
+        if not sfp.get_presence():
+            click.echo("{}: SFP EEPROM not detected\n".format(port_name))
+            sys.exit(EXIT_FAIL)
+
+        from sonic_platform_base.sonic_xcvr.api.public import sff8472
+        api = sfp.get_xcvr_api()
+        if api is None:
+            click.echo('Error: SFP EEPROM not detected!')
+        if not isinstance(api, sff8472.Sff8472Api):
+            overall_offset = get_overall_offset_general(api, page, offset, size)
         else:
-            click.echo(hexdump('', data, offset))
+            overall_offset = get_overall_offset_sff8472(api, page, offset, size, wire_addr)
+        return_code, output = eeprom_dump_general(physical_port, page, overall_offset, size, offset, no_format)
+        if return_code != 0:
+            click.echo("Error: Failed to read EEPROM!")
+            sys.exit(return_code)
+        click.echo(output)
     except NotImplementedError:
         click.echo("This functionality is currently not implemented for this platform")
         sys.exit(ERROR_NOT_IMPLEMENTED)
     except ValueError as e:
-        click.echo("Error: {}".format(e))
+        click.echo(f"Error: {e}")
         sys.exit(EXIT_FAIL)
 
 
@@ -1580,34 +1660,44 @@ def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
 @click.option('--verify', is_flag=True, help="Verify the data by reading back")
 def write_eeprom(port_name, page, offset, data, wire_addr, verify):
     """Write SFP EEPROM data"""
-    if platform_sfputil.is_logical_port(port_name) == 0:
-        click.echo("Error: invalid port {}".format(port_name))
-        print_all_valid_port_values()
-        sys.exit(ERROR_INVALID_PORT)
+    try:
+        if platform_sfputil.is_logical_port(port_name) == 0:
+            click.echo("Error: invalid port {}".format(port_name))
+            print_all_valid_port_values()
+            sys.exit(ERROR_INVALID_PORT)
 
-    if is_port_type_rj45(port_name):
-        click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
-        sys.exit(EXIT_FAIL)
+        if is_port_type_rj45(port_name):
+            click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
+            sys.exit(EXIT_FAIL)
 
-    physical_port = logical_port_to_physical_port_index(port_name)
-    sfp = platform_chassis.get_sfp(physical_port)
-    if not sfp.get_presence():
-        click.echo("{}: SFP EEPROM not detected\n".format(port_name))
-        sys.exit(EXIT_FAIL)
+        physical_port = logical_port_to_physical_port_index(port_name)
+        sfp = platform_chassis.get_sfp(physical_port)
+        if not sfp.get_presence():
+            click.echo("{}: SFP EEPROM not detected\n".format(port_name))
+            sys.exit(EXIT_FAIL)
 
-    try:
-        bytes = bytearray.fromhex(data)
-    except ValueError:
-        click.echo("Error: Data must be a hex string of even length!")
-        sys.exit(EXIT_FAIL)
+        try:
+            bytes = bytearray.fromhex(data)
+        except ValueError:
+            click.echo("Error: Data must be a hex string of even length!")
+            sys.exit(EXIT_FAIL)
 
-    try:
-        success = sfp.write_eeprom_by_page(page, offset, bytes, wire_addr)
+        from sonic_platform_base.sonic_xcvr.api.public import sff8472
+        api = sfp.get_xcvr_api()
+        if api is None:
+            click.echo('Error: SFP EEPROM not detected!')
+            sys.exit(EXIT_FAIL)
+
+        if not isinstance(api, sff8472.Sff8472Api):
+            overall_offset = get_overall_offset_general(api, page, offset, len(bytes))
+        else:
+            overall_offset = get_overall_offset_sff8472(api, page, offset, len(bytes), wire_addr)
+        success = sfp.write_eeprom(overall_offset, len(bytes), bytes)
         if not success:
             click.echo("Error: Failed to write EEPROM!")
             sys.exit(ERROR_NOT_IMPLEMENTED)
         if verify:
-            read_data = sfp.read_eeprom_by_page(page, offset, len(bytes), wire_addr)
+            read_data = sfp.read_eeprom(overall_offset, len(bytes))
             if read_data != bytes:
                 click.echo(f"Error: Write data failed! Write: {''.join('{:02x}'.format(x) for x in bytes)}, read: {''.join('{:02x}'.format(x) for x in read_data)}")
                 sys.exit(EXIT_FAIL)
@@ -1619,49 +1709,79 @@ def write_eeprom(port_name, page, offset, data, wire_addr, verify):
         sys.exit(EXIT_FAIL)
 
 
-# 'version' subcommand
-@cli.command()
-def version():
-    """Display version info"""
-    click.echo("sfputil version {0}".format(VERSION))
+def get_overall_offset_general(api, page, offset, size):
+    """
+    Validate input parameter page, offset, size and translate them to overall offset
+    Args:
+        api: cable API object
+        page: module EEPROM page number.
+        offset: module EEPROM page offset.
+        size: number bytes of the data to be read/write
 
-# 'target' subcommand
-@firmware.command()
-@click.argument('port_name', required=True, default=None)
-@click.argument('target', type=click.IntRange(0, 2), required=True, default=None)
-def target(port_name, target):
-    """Select target end for firmware download 0-(local) \n
-                                               1-(remote-A) \n
-                                               2-(remote-B)
+    Returns:
+        The overall offset
     """
-    physical_port = logical_port_to_physical_port_index(port_name)
-    sfp = platform_chassis.get_sfp(physical_port)
+    max_page = 0 if api.is_flat_memory() else MAX_EEPROM_PAGE
+    if max_page == 0 and page != 0:
+        raise ValueError(f'Invalid page number {page}, only page 0 is supported')
 
-    if is_port_type_rj45(port_name):
-        click.echo("{}: This functionality is not applicable for RJ45 port".format(port_name))
-        sys.exit(EXIT_FAIL)
+    if page < 0 or page > max_page:
+        raise ValueError(f'Invalid page number {page}, valid range: [0, {max_page}]')
 
-    if not is_sfp_present(port_name):
-       click.echo("{}: SFP EEPROM not detected\n".format(port_name))
-       sys.exit(EXIT_FAIL)
+    if page == 0:
+        if offset < MIN_OFFSET_FOR_PAGE0 or offset > MAX_EEPROM_OFFSET:
+            raise ValueError(f'Invalid offset {offset} for page 0, valid range: [0, 255]')
+    else:
+        if offset < MIN_OFFSET_FOR_OTHER_PAGE or offset > MAX_EEPROM_OFFSET:
+            raise ValueError(f'Invalid offset {offset} for page {page}, valid range: [128, 255]')
 
-    try:
-        api = sfp.get_xcvr_api()
-    except NotImplementedError:
-        click.echo("{}: This functionality is currently not implemented for this module".format(port_name))
-        sys.exit(ERROR_NOT_IMPLEMENTED)
+    if size <= 0 or size + offset - 1 > MAX_EEPROM_OFFSET:
+        raise ValueError(f'Invalid size {size}, valid range: [1, {255 - offset + 1}]')
 
-    try:
-        status = api.set_firmware_download_target_end(target)
-    except AttributeError:
-        click.echo("{}: This functionality is not applicable for this module".format(port_name))
-        sys.exit(ERROR_NOT_IMPLEMENTED)
+    return page * PAGE_SIZE + offset
 
-    if status:
-        click.echo("Target Mode set to {}". format(target))
+
+def get_overall_offset_sff8472(api, page, offset, size, wire_addr):
+    """
+        Validate input parameter page, offset, size, wire_addr and translate them to overall offset
+        Args:
+            api: cable API object
+            page: module EEPROM page number.
+            offset: module EEPROM page offset.
+            size: number bytes of the data to be read/write
+            wire_addr: case-insensitive wire address string. Only valid for sff8472, a0h or a2h.
+
+        Returns:
+            The overall offset
+        """
+    if not wire_addr:
+        raise ValueError("Invalid wire address for sff8472, must a0h or a2h")
+
+    is_active_cable = not api.is_copper()
+    valid_wire_address = ('a0h', 'a2h') if is_active_cable else ('a0h',)
+    wire_addr = wire_addr.lower()
+    if wire_addr not in valid_wire_address:
+        raise ValueError(f"Invalid wire address {wire_addr} for sff8472, must be {' or '.join(valid_wire_address)}")
+
+    if wire_addr == 'a0h':
+        if page != 0:
+            raise ValueError(f'Invalid page number {page} for wire address {wire_addr}, only page 0 is supported')
+        max_offset = MAX_OFFSET_FOR_A0H_UPPER_PAGE if is_active_cable else MAX_OFFSET_FOR_A0H_LOWER_PAGE
+        if offset < 0 or offset > max_offset:
+            raise ValueError(f'Invalid offset {offset} for wire address {wire_addr}, valid range: [0, {max_offset}]')
+        if size <= 0 or size + offset - 1 > max_offset:
+            raise ValueError(
+                f'Invalid size {size} for wire address {wire_addr}, valid range: [1, {max_offset - offset + 1}]')
+        return offset
     else:
-        click.echo("Target Mode set failed!")
-        sys.exit(EXIT_FAIL)
+        if page < 0 or page > MAX_EEPROM_PAGE:
+            raise ValueError(f'Invalid page number {page} for wire address {wire_addr}, valid range: [0, 255]')
+        if offset < 0 or offset > MAX_OFFSET_FOR_A2H:
+            raise ValueError(f'Invalid offset {offset} for wire address {wire_addr}, valid range: [0, 255]')
+        if size <= 0 or size + offset - 1 > MAX_OFFSET_FOR_A2H:
+            raise ValueError(f'Invalid size {size} for wire address {wire_addr}, valid range: [1, {255 - offset + 1}]')
+        return page * PAGE_SIZE + offset + PAGE_SIZE_FOR_A0H
+
 
 if __name__ == '__main__':
     cli()
diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index d3a76935b9..c7ad00a68b 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -256,7 +256,7 @@ def test_convert_sfp_info_to_output_string(self, sfp_info_dict, expected_output)
                 Vcc: 3.2577Volts
         ModuleThresholdValues:
 '''
-        ), 
+        ),
         (
             'QSFP-DD Double Density 8X Pluggable Transceiver',
             {
@@ -982,28 +982,27 @@ def test_read_eeprom(self, mock_chassis):
         assert result.exit_code == EXIT_FAIL
 
         mock_sfp.get_presence.return_value = True
-        mock_sfp.read_eeprom_by_page = MagicMock(return_value=None)
+        mock_sfp.read_eeprom = MagicMock(return_value=None)
         result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
         assert result.exit_code == ERROR_NOT_IMPLEMENTED
 
-        mock_sfp.read_eeprom_by_page.return_value = bytearray([0x00, 0x01])
+        mock_sfp.read_eeprom.return_value = bytearray([0x00, 0x01])
         result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '2', '--no-format'])
         assert result.exit_code == 0
         assert result.output == '0001\n'
 
         result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
         assert result.exit_code == 0
-        expected_output = """
-00000005 00 01                                            |..|
+        expected_output = """        00000005 00 01                                            |..|
 """
         print(result.output)
         assert result.output == expected_output
 
-        mock_sfp.read_eeprom_by_page.side_effect = NotImplementedError
+        mock_sfp.read_eeprom.side_effect = NotImplementedError
         result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
         assert result.exit_code == ERROR_NOT_IMPLEMENTED
 
-        mock_sfp.read_eeprom_by_page.side_effect = ValueError
+        mock_sfp.read_eeprom.side_effect = ValueError
         result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
         assert result.exit_code == EXIT_FAIL
 
@@ -1030,33 +1029,33 @@ def test_write_eeprom(self, mock_chassis):
         assert result.exit_code == EXIT_FAIL
 
         # write failed
-        mock_sfp.write_eeprom_by_page = MagicMock(return_value=False)
+        mock_sfp.write_eeprom = MagicMock(return_value=False)
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
         print(result.output)
         assert result.exit_code == ERROR_NOT_IMPLEMENTED
 
         # write success
-        mock_sfp.write_eeprom_by_page.return_value = True
+        mock_sfp.write_eeprom.return_value = True
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
         assert result.exit_code == 0
 
         # write verify success
-        mock_sfp.read_eeprom_by_page = MagicMock(return_value=bytearray([16]))
+        mock_sfp.read_eeprom = MagicMock(return_value=bytearray([16]))
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10', '--verify'])
         assert result.exit_code == 0
 
         # write verify failed
-        mock_sfp.read_eeprom_by_page = MagicMock(return_value=bytearray([10]))
+        mock_sfp.read_eeprom = MagicMock(return_value=bytearray([10]))
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '11', '--verify'])
         assert result.exit_code != 0
 
         # Not implemented
-        mock_sfp.write_eeprom_by_page.side_effect = NotImplementedError
+        mock_sfp.write_eeprom.side_effect = NotImplementedError
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
         assert result.exit_code == ERROR_NOT_IMPLEMENTED
 
         # Value error
-        mock_sfp.write_eeprom_by_page.side_effect = ValueError
+        mock_sfp.write_eeprom.side_effect = ValueError
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
         assert result.exit_code == EXIT_FAIL
 
@@ -1086,6 +1085,80 @@ def test_write_eeprom_rj45(self):
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '00'])
         assert result.exit_code == EXIT_FAIL
 
+    def test_get_overall_offset_general(self):
+        api = MagicMock()
+        api.is_flat_memory = MagicMock(return_value=False)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_general(api, -1, 0, 1)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_general(api, 256, 0, 1)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_general(api, 0, -1, 1)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_general(api, 0, 256, 1)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_general(api, 1, 127, 1)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_general(api, 1, 256, 1)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_general(api, 0, 0, 0)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_general(api, 0, 0, 257)
+
+        assert sfputil.get_overall_offset_general(api, 0, 1, 1) == 1
+
+    def test_get_overall_offset_sff8472(self):
+        api = MagicMock()
+        api.is_copper = MagicMock(return_value=False)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 0, 0, 1, None)
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 0, 0, 1, wire_addr='invalid')
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 1, 0, 1, wire_addr='a0h')
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 0, -1, 1, wire_addr='A0h')
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 0, 256, 1, wire_addr='A0h')
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 0, 0, 0, wire_addr='A0h')
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 0, 0, 257, wire_addr='A0h')
+
+        assert sfputil.get_overall_offset_sff8472(api, 0, 2, 2, wire_addr='A0h') == 2
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, -1, 0, 1, wire_addr='a2h')
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 256, 0, 1, wire_addr='a2h')
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 0, -1, 1, wire_addr='a2h')
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 0, 0, 0, wire_addr='A2h')
+
+        with pytest.raises(ValueError):
+            sfputil.get_overall_offset_sff8472(api, 0, 0, 257, wire_addr='A2h')
+
+        assert sfputil.get_overall_offset_sff8472(api, 0, 2, 2, wire_addr='A2h') == 258
+
     @patch('sfputil.main.platform_chassis')
     @patch('sfputil.main.logical_port_to_physical_port_index', MagicMock(return_value=1))
     def test_target_firmware(self, mock_chassis):

From 447b238fc4e7067ae41a054c215d35ca479d869d Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Tue, 28 Nov 2023 10:40:31 +0200
Subject: [PATCH 10/17] Remove un-intended change

---
 doc/Command-Reference.md | 22 +++++++++++-----------
 sfputil/main.py          |  5 +++--
 tests/sfputil_test.py    |  2 +-
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md
index aaf6ca7535..298afc1fa3 100644
--- a/doc/Command-Reference.md
+++ b/doc/Command-Reference.md
@@ -560,7 +560,7 @@ This command displays the current date and time configured on the system
   ```
 
 **config clock date**
-
+	
 This command will set the date-time of the systetm, given strings with date-time format <YYYY-MM-DD> <HH:MM:SS>
 
 - Usage:
@@ -578,7 +578,7 @@ This command will set the date-time of the systetm, given strings with date-time
   ```
 
 **config clock timezone**
-
+	
 This command will set the timezone of the systetm, given a string of a valid timezone.
 
 - Usage:
@@ -588,13 +588,13 @@ This command will set the timezone of the systetm, given a string of a valid tim
 
 - Parameters:
   - _timezone_: valid timezone to be configured
-
-
+	
+	
 - Example:
   ```
   admin@sonic:~$ config clock timezone Africa/Accra
 
-
+	
 **show clock timezones**
 
 This command Will display list of all valid timezones to be configured.
@@ -2571,7 +2571,7 @@ Once enabled, BGP will not advertise routes which aren't yet offloaded.
   admin@sonic:~$ sudo config suppress-fib-pending enabled
   ```
   ```
-  admin@sonic:~$ sudo config suppress-fib-pending disabled
+  admin@sonic:~$ sudo config suppress-fib-pending disabled 
   ```
 
 Go Back To [Beginning of the document](#) or [Beginning of this section](#bgp)
@@ -2851,7 +2851,7 @@ This command is used for downloading firmware tp upgrade the transciever module.
   ```
 **sfputil firmware run**
 
-This command is used to start and run a downloaded image. This command transfers control from the currently running firmware to a new firmware.
+This command is used to start and run a downloaded image. This command transfers control from the currently running firmware to a new firmware. 
 
 - Usage:
   ```
@@ -2876,7 +2876,7 @@ This command is used to start and run a downloaded image. This command transfers
 
 **sfputil firmware commit**
 
-This command to commit the running image so that the module will boot from it on future boots.
+This command to commit the running image so that the module will boot from it on future boots. 
 
 - Usage:
   ```
@@ -4597,7 +4597,7 @@ This command is to display the FEC status of the selected interfaces. If **inter
   show interfaces fec status [<interface_name>]
   ```
 
-- Example:
+- Example:  
 ```
   admin@sonic:~$ show interfaces fec status
   Interface    FEC Oper    FEC Admin
@@ -6849,7 +6849,7 @@ in order to detemine whether the health of the cable is Ok
 the following are checked
 - the vendor name is correct able to be read
 - the FW is correctly loaded for SerDes by reading the appropriate register val
-- the Counters for UART are displaying healthy status
+- the Counters for UART are displaying healthy status 
        i.e Error Counters , retry Counters for UART or internal xfer protocols are below a threshold
 
 
@@ -6905,7 +6905,7 @@ the result will be displayed like this, each item in the dictionary shows the he
            {
                "uart_stat1": "2",
                "uart_stat2": "1",
-
+                 
            }
     ```
 
diff --git a/sfputil/main.py b/sfputil/main.py
index 47d20368f0..71d017b0a4 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -48,6 +48,7 @@
 
 PAGE_SIZE = 128
 PAGE_OFFSET = 128
+
 SFF8472_A0_SIZE = 256
 MAX_EEPROM_PAGE = 255
 MAX_EEPROM_OFFSET = 255
@@ -1291,10 +1292,10 @@ def is_fw_switch_done(port_name):
                 status = -1 # Abnormal status.
             elif (ImageARunning == 1) and (ImageACommitted == 0):   # ImageA is running, but not committed.
                 click.echo("FW images switch successful : ImageA is running")
-                status = 1  # run_firmware is done.
+                status = 1  # run_firmware is done. 
             elif (ImageBRunning == 1) and (ImageBCommitted == 0):   # ImageB is running, but not committed.
                 click.echo("FW images switch successful : ImageB is running")
-                status = 1  # run_firmware is done.
+                status = 1  # run_firmware is done. 
             else:                                                   # No image is running, or running and committed image is same.
                 click.echo("FW info error : Failed to switch into uncommitted image!")
                 status = -1 # Failure for Switching images.
diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index c7ad00a68b..77664c8a3a 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -256,7 +256,7 @@ def test_convert_sfp_info_to_output_string(self, sfp_info_dict, expected_output)
                 Vcc: 3.2577Volts
         ModuleThresholdValues:
 '''
-        ),
+        ), 
         (
             'QSFP-DD Double Density 8X Pluggable Transceiver',
             {

From 62bf0b342da9e7c8a3c24ab75da7e2b0e6829a13 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com>
Date: Wed, 29 Nov 2023 11:09:40 +0800
Subject: [PATCH 11/17] Update Command-Reference.md

---
 doc/Command-Reference.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md
index 298afc1fa3..4cbcf4b334 100644
--- a/doc/Command-Reference.md
+++ b/doc/Command-Reference.md
@@ -12845,6 +12845,10 @@ Options:
 ```
 admin@sonic:~$ sfputil write-eeprom Ethernet0 0 100 4a44
 
+# Write success
+admin@sonic:~$ sfputil write-eeprom Etherent0 0 100 0000 --verify
+
+# Write fail
 admin@sonic:~$ sfputil write-eeprom Etherent0 0 100 4a44 --verify
 Error: Write data failed! Write: 4a44, read: 0000.
 ```

From ee78e3c863cdc28670a9e34d4b349c9f6ab55509 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com>
Date: Wed, 29 Nov 2023 11:11:44 +0800
Subject: [PATCH 12/17] Update Command-Reference.md

---
 doc/Command-Reference.md | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md
index 4cbcf4b334..668ae35823 100644
--- a/doc/Command-Reference.md
+++ b/doc/Command-Reference.md
@@ -12842,13 +12842,16 @@ Options:
   --help                  Show this message and exit.
 ```
 
+- Write success
 ```
 admin@sonic:~$ sfputil write-eeprom Ethernet0 0 100 4a44
 
-# Write success
 admin@sonic:~$ sfputil write-eeprom Etherent0 0 100 0000 --verify
 
-# Write fail
+```
+
+- Write fail
+```
 admin@sonic:~$ sfputil write-eeprom Etherent0 0 100 4a44 --verify
 Error: Write data failed! Write: 4a44, read: 0000.
 ```

From 6c1437675e7acc66a79cf8eac7fa18e5c820286b Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com>
Date: Wed, 29 Nov 2023 11:39:42 +0800
Subject: [PATCH 13/17] Fix review comments

---
 sfputil/main.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sfputil/main.py b/sfputil/main.py
index 71d017b0a4..0425ea047b 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -53,7 +53,7 @@
 MAX_EEPROM_PAGE = 255
 MAX_EEPROM_OFFSET = 255
 MIN_OFFSET_FOR_PAGE0 = 0
-MIN_OFFSET_FOR_OTHER_PAGE = 128
+MIN_OFFSET_FOR_NON_PAGE0  = 128
 MAX_OFFSET_FOR_A0H_UPPER_PAGE = 255
 MAX_OFFSET_FOR_A0H_LOWER_PAGE = 127
 MAX_OFFSET_FOR_A2H = 255
@@ -806,7 +806,7 @@ def eeprom_hexdump_sff8636(port, physical_port, page):
 
 def eeprom_dump_general(physical_port, page, overall_offset, size, page_offset, no_format=False):
     """
-    Dump module EEPROM for given pages in hex format. This function is designed for SFF8472 only.
+    Dump module EEPROM for given pages in hex format. This function is designed for non-SFF8472.
     Args:
         logical_port_name: logical port name
         pages: a list of pages to be dumped. The list always include a default page list and the target_page input by
@@ -1733,7 +1733,7 @@ def get_overall_offset_general(api, page, offset, size):
         if offset < MIN_OFFSET_FOR_PAGE0 or offset > MAX_EEPROM_OFFSET:
             raise ValueError(f'Invalid offset {offset} for page 0, valid range: [0, 255]')
     else:
-        if offset < MIN_OFFSET_FOR_OTHER_PAGE or offset > MAX_EEPROM_OFFSET:
+        if offset < MIN_OFFSET_FOR_NON_PAGE0 or offset > MAX_EEPROM_OFFSET:
             raise ValueError(f'Invalid offset {offset} for page {page}, valid range: [128, 255]')
 
     if size <= 0 or size + offset - 1 > MAX_EEPROM_OFFSET:

From ff1052a15f4beb4ef0339af80f527773b1b79f38 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com>
Date: Thu, 30 Nov 2023 11:29:52 +0800
Subject: [PATCH 14/17] Fix review comments

---
 sfputil/main.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sfputil/main.py b/sfputil/main.py
index 0425ea047b..29b0ee1e7e 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -806,7 +806,7 @@ def eeprom_hexdump_sff8636(port, physical_port, page):
 
 def eeprom_dump_general(physical_port, page, overall_offset, size, page_offset, no_format=False):
     """
-    Dump module EEPROM for given pages in hex format. This function is designed for non-SFF8472.
+    Dump module EEPROM for given pages in hex format.
     Args:
         logical_port_name: logical port name
         pages: a list of pages to be dumped. The list always include a default page list and the target_page input by

From b1591818bd47a3f892a7052c86ba7ca520666394 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Tue, 5 Dec 2023 13:09:19 +0200
Subject: [PATCH 15/17] User click.IntRange to avoid duplicate validation

---
 sfputil/main.py       |  47 +++++--------
 tests/sfputil_test.py | 156 +++++++++++++++++++++++++-----------------
 2 files changed, 114 insertions(+), 89 deletions(-)

diff --git a/sfputil/main.py b/sfputil/main.py
index 29b0ee1e7e..1f018a8027 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -52,7 +52,6 @@
 SFF8472_A0_SIZE = 256
 MAX_EEPROM_PAGE = 255
 MAX_EEPROM_OFFSET = 255
-MIN_OFFSET_FOR_PAGE0 = 0
 MIN_OFFSET_FOR_NON_PAGE0  = 128
 MAX_OFFSET_FOR_A0H_UPPER_PAGE = 255
 MAX_OFFSET_FOR_A0H_LOWER_PAGE = 127
@@ -1606,14 +1605,16 @@ def target(port_name, target):
 
 # 'read-eeprom' subcommand
 @cli.command()
-@click.argument('port_name', metavar='<port_name>', required=True)
-@click.argument('page', metavar='<page>', type=click.INT, required=True)
-@click.argument('offset', metavar='<offset>', type=click.INT, required=True)
-@click.argument('size', metavar='<size>', type=click.INT, required=True)
+@click.argument('port_name', metavar='<logical_port_name>', required=True)
+@click.argument('page', metavar='<page>', type=click.IntRange(0, MAX_EEPROM_PAGE), required=True)
+@click.argument('offset', metavar='<offset>', type=click.IntRange(0, MAX_EEPROM_OFFSET), required=True)
+@click.argument('size', metavar='<size>', type=click.IntRange(1, MAX_EEPROM_OFFSET + 1), required=True)
 @click.option('--no-format', is_flag=True, help="Display non formatted data")
 @click.option('--wire-addr', help="Wire address of sff8472")
 def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
-    """Read SFP EEPROM data"""
+    """Read SFP EEPROM data
+       <port_name>: 
+    """
     try:
         if platform_sfputil.is_logical_port(port_name) == 0:
             click.echo("Error: invalid port {}".format(port_name))
@@ -1653,9 +1654,9 @@ def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
 
 # 'write-eeprom' subcommand
 @cli.command()
-@click.argument('port_name', metavar='<port_name>', required=True)
-@click.argument('page', metavar='<page>', type=click.INT, required=True)
-@click.argument('offset', metavar='<offset>', type=click.INT, required=True)
+@click.argument('port_name', metavar='<logical_port_name>', required=True)
+@click.argument('page', metavar='<page>', type=click.IntRange(0, MAX_EEPROM_PAGE), required=True)
+@click.argument('offset', metavar='<offset>', type=click.IntRange(0, MAX_EEPROM_OFFSET), required=True)
 @click.argument('data', metavar='<data>', required=True)
 @click.option('--wire-addr', help="Wire address of sff8472")
 @click.option('--verify', is_flag=True, help="Verify the data by reading back")
@@ -1722,21 +1723,15 @@ def get_overall_offset_general(api, page, offset, size):
     Returns:
         The overall offset
     """
-    max_page = 0 if api.is_flat_memory() else MAX_EEPROM_PAGE
-    if max_page == 0 and page != 0:
-        raise ValueError(f'Invalid page number {page}, only page 0 is supported')
-
-    if page < 0 or page > max_page:
-        raise ValueError(f'Invalid page number {page}, valid range: [0, {max_page}]')
+    if api.is_flat_memory():
+        if page != 0:
+            raise ValueError(f'Invalid page number {page}, only page 0 is supported')
 
-    if page == 0:
-        if offset < MIN_OFFSET_FOR_PAGE0 or offset > MAX_EEPROM_OFFSET:
-            raise ValueError(f'Invalid offset {offset} for page 0, valid range: [0, 255]')
-    else:
-        if offset < MIN_OFFSET_FOR_NON_PAGE0 or offset > MAX_EEPROM_OFFSET:
+    if page != 0:
+        if offset < MIN_OFFSET_FOR_NON_PAGE0:
             raise ValueError(f'Invalid offset {offset} for page {page}, valid range: [128, 255]')
 
-    if size <= 0 or size + offset - 1 > MAX_EEPROM_OFFSET:
+    if size + offset - 1 > MAX_EEPROM_OFFSET:
         raise ValueError(f'Invalid size {size}, valid range: [1, {255 - offset + 1}]')
 
     return page * PAGE_SIZE + offset
@@ -1768,18 +1763,14 @@ def get_overall_offset_sff8472(api, page, offset, size, wire_addr):
         if page != 0:
             raise ValueError(f'Invalid page number {page} for wire address {wire_addr}, only page 0 is supported')
         max_offset = MAX_OFFSET_FOR_A0H_UPPER_PAGE if is_active_cable else MAX_OFFSET_FOR_A0H_LOWER_PAGE
-        if offset < 0 or offset > max_offset:
+        if offset > max_offset:
             raise ValueError(f'Invalid offset {offset} for wire address {wire_addr}, valid range: [0, {max_offset}]')
-        if size <= 0 or size + offset - 1 > max_offset:
+        if size + offset - 1 > max_offset:
             raise ValueError(
                 f'Invalid size {size} for wire address {wire_addr}, valid range: [1, {max_offset - offset + 1}]')
         return offset
     else:
-        if page < 0 or page > MAX_EEPROM_PAGE:
-            raise ValueError(f'Invalid page number {page} for wire address {wire_addr}, valid range: [0, 255]')
-        if offset < 0 or offset > MAX_OFFSET_FOR_A2H:
-            raise ValueError(f'Invalid offset {offset} for wire address {wire_addr}, valid range: [0, 255]')
-        if size <= 0 or size + offset - 1 > MAX_OFFSET_FOR_A2H:
+        if size + offset - 1 > MAX_OFFSET_FOR_A2H:
             raise ValueError(f'Invalid size {size} for wire address {wire_addr}, valid range: [1, {255 - offset + 1}]')
         return page * PAGE_SIZE + offset + PAGE_SIZE_FOR_A0H
 
diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index 77664c8a3a..86ed9f23f2 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -1085,78 +1085,112 @@ def test_write_eeprom_rj45(self):
         result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '00'])
         assert result.exit_code == EXIT_FAIL
 
-    def test_get_overall_offset_general(self):
+    @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=False))
+    @patch('sfputil.main.logical_port_to_physical_port_index', MagicMock(return_value=1))
+    @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1)))
+    @patch('sfputil.main.platform_chassis')
+    def test_get_overall_offset_general(self, mock_chassis):
         api = MagicMock()
         api.is_flat_memory = MagicMock(return_value=False)
+        mock_sfp = MagicMock()
+        mock_chassis.get_sfp = MagicMock(return_value=mock_sfp)
 
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_general(api, -1, 0, 1)
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_general(api, 256, 0, 1)
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_general(api, 0, -1, 1)
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_general(api, 0, 256, 1)
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_general(api, 1, 127, 1)
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_general(api, 1, 256, 1)
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_general(api, 0, 0, 0)
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_general(api, 0, 0, 257)
-
-        assert sfputil.get_overall_offset_general(api, 0, 1, 1) == 1
+        mock_sfp.get_presence = MagicMock(return_value=True)
+        mock_sfp.get_xcvr_api = MagicMock(return_value=api)
+        
+        runner = CliRunner()
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '-1', '0', '01'])
+        assert result.exit_code != 0
+        
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '256', '0', '01'])
+        assert result.exit_code != 0
+        
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '0', '-1', '01'])
+        assert result.exit_code != 0
+        
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '0', '256', '01'])
+        assert result.exit_code != 0
+        
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '1', '127', '01'])
+        assert result.exit_code != 0
+        
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '1', '256', '01'])
+        assert result.exit_code != 0
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--', '0', '0', '0'])
+        assert result.exit_code != 0
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--', '0', '0', '257'])
+        assert result.exit_code != 0
+        
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '0', '1', '01'])
+        assert result.exit_code == 0
 
-    def test_get_overall_offset_sff8472(self):
+    @patch('sfputil.main.isinstance', MagicMock(return_value=True))
+    @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=False))
+    @patch('sfputil.main.logical_port_to_physical_port_index', MagicMock(return_value=1))
+    @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1)))
+    @patch('sfputil.main.platform_chassis')
+    def test_get_overall_offset_sff8472(self, mock_chassis):
         api = MagicMock()
         api.is_copper = MagicMock(return_value=False)
+        mock_sfp = MagicMock()
+        mock_chassis.get_sfp = MagicMock(return_value=mock_sfp)
 
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 0, 0, 1, None)
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 0, 0, 1, wire_addr='invalid')
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 1, 0, 1, wire_addr='a0h')
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 0, -1, 1, wire_addr='A0h')
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 0, 256, 1, wire_addr='A0h')
+        mock_sfp.get_presence = MagicMock(return_value=True)
+        mock_sfp.get_xcvr_api = MagicMock(return_value=api)
+        
+        runner = CliRunner()
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--', '0', '0', '1'])
+        assert result.exit_code != 0
+        print(result.output)
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'invalid', '0', '0', '1'])
+        assert result.exit_code != 0
+        print(result.output)
 
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 0, 0, 0, wire_addr='A0h')
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a0h', '1', '0', '1'])
+        assert result.exit_code != 0
+        print(result.output)
 
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 0, 0, 257, wire_addr='A0h')
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'A0h', '--', '0', '-1', '1'])
+        assert result.exit_code != 0
+        print(result.output)
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'A0h', '--', '0', '256', '1'])
+        assert result.exit_code != 0
+        print(result.output)
 
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'A0h', '--', '0', '0', '0'])
+        assert result.exit_code != 0
+        print(result.output)
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'A0h', '--', '0', '0', '257'])
+        assert result.exit_code != 0
+        print(result.output)
+        
         assert sfputil.get_overall_offset_sff8472(api, 0, 2, 2, wire_addr='A0h') == 2
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, -1, 0, 1, wire_addr='a2h')
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 256, 0, 1, wire_addr='a2h')
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 0, -1, 1, wire_addr='a2h')
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 0, 0, 0, wire_addr='A2h')
-
-        with pytest.raises(ValueError):
-            sfputil.get_overall_offset_sff8472(api, 0, 0, 257, wire_addr='A2h')
-
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '-1', '0', '1'])
+        assert result.exit_code != 0
+        print(result.output)
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '256', '0', '1'])
+        assert result.exit_code != 0
+        print(result.output)
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '0', '-1', '1'])
+        assert result.exit_code != 0
+        print(result.output)
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '0', '0', '0'])
+        assert result.exit_code != 0
+        print(result.output)
+        
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '0', '0', '257'])
+        assert result.exit_code != 0
+        print(result.output)
+        
         assert sfputil.get_overall_offset_sff8472(api, 0, 2, 2, wire_addr='A2h') == 258
 
     @patch('sfputil.main.platform_chassis')

From 8aeb52875891675eb6b5a7b11a82ae5524d32908 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <junchao@nvidia.com>
Date: Thu, 7 Dec 2023 09:11:38 +0200
Subject: [PATCH 16/17] Fix review comments

---
 doc/Command-Reference.md |  38 ++++++++-----
 sfputil/main.py          |  45 +++++++--------
 tests/sfputil_test.py    | 120 ++++++++++++++++++++++++++-------------
 3 files changed, 126 insertions(+), 77 deletions(-)

diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md
index 668ae35823..df46f6da80 100644
--- a/doc/Command-Reference.md
+++ b/doc/Command-Reference.md
@@ -12806,23 +12806,29 @@ This sub-section explains the list of commands available for SFP utilities featu
 
 ```
 admin@sonic:~$ sfputil read-eeprom --help
-Usage: sfputil read-eeprom [OPTIONS] <port_name> <page> <offset> <size>
+Usage: sfputil read-eeprom [OPTIONS]
+
   Read SFP EEPROM data
+
 Options:
-  --no-format             Display non formatted data.
-  --wire-addr             Wire address of sff8472.
-  --help                  Show this
+  -p, --port <logical_port_name>  Logical port name  [required]
+  -n, --page <page>               EEPROM page number  [required]
+  -o, --offset <offset>           EEPROM offset within the page  [required]
+  -s, --size <size>               Size of byte to be read  [required]
+  --no-format                     Display non formatted data
+  --wire-addr TEXT                Wire address of sff8472
+  --help                          Show this message and exit.
 ```
 
 ```
-admin@sonic:~$ sfputil read-eeprom Ethernet0 0 100 2
+admin@sonic:~$ sfputil read-eeprom -p Ethernet0 -n 0 -o 100 -s 2
         00000064 4a 44                                            |..|
 
-admin@sonic:~$ sfputil read-eeprom Ethernet0 0 0 32
+admin@sonic:~$ sfputil read-eeprom --port Ethernet0 --page 0 --offset 0 --size 32
         00000000 11 08 06 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
         00000010 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
 
-admin@sonic:~$ sfputil read-eeprom Ethernet0 0 100 2 --no-format
+admin@sonic:~$ sfputil read-eeprom --port Ethernet0 --page 0 --offset 100 --size 2 --no-format
 4a44
 ```
 
@@ -12832,27 +12838,31 @@ admin@sonic:~$ sfputil read-eeprom Ethernet0 0 100 2 --no-format
 
 ```
 admin@sonic:~$ sfputil write-eeprom --help
-Usage: sfputil write-eeprom [OPTIONS] <port> <page> <offset> <data>
+Usage: sfputil write-eeprom [OPTIONS]
 
   Write SFP EEPROM data
 
 Options:
-  --wire-addr             Wire address of sff8472.
-  --verify                Verify the data by reading back.
-  --help                  Show this message and exit.
+  -p, --port <logical_port_name>  Logical port name  [required]
+  -n, --page <page>               EEPROM page number  [required]
+  -o, --offset <offset>           EEPROM offset within the page  [required]
+  -d, --data <data>               Hex string EEPROM data  [required]
+  --wire-addr TEXT                Wire address of sff8472
+  --verify                        Verify the data by reading back
+  --help                          Show this message and exit.
 ```
 
 - Write success
 ```
-admin@sonic:~$ sfputil write-eeprom Ethernet0 0 100 4a44
+admin@sonic:~$ sfputil write-eeprom -p Ethernet0 -n 0 -o 100 -d 4a44
 
-admin@sonic:~$ sfputil write-eeprom Etherent0 0 100 0000 --verify
+admin@sonic:~$ sfputil write-eeprom --port Etherent0 --page 0 --offset 100 --data 0000 --verify
 
 ```
 
 - Write fail
 ```
-admin@sonic:~$ sfputil write-eeprom Etherent0 0 100 4a44 --verify
+admin@sonic:~$ sfputil write-eeprom -p Etherent0 -n 0 -o 100 -d 4a44 --verify
 Error: Write data failed! Write: 4a44, read: 0000.
 ```
 
diff --git a/sfputil/main.py b/sfputil/main.py
index 1f018a8027..ad227e4163 100644
--- a/sfputil/main.py
+++ b/sfputil/main.py
@@ -1605,30 +1605,29 @@ def target(port_name, target):
 
 # 'read-eeprom' subcommand
 @cli.command()
-@click.argument('port_name', metavar='<logical_port_name>', required=True)
-@click.argument('page', metavar='<page>', type=click.IntRange(0, MAX_EEPROM_PAGE), required=True)
-@click.argument('offset', metavar='<offset>', type=click.IntRange(0, MAX_EEPROM_OFFSET), required=True)
-@click.argument('size', metavar='<size>', type=click.IntRange(1, MAX_EEPROM_OFFSET + 1), required=True)
+@click.option('-p', '--port', metavar='<logical_port_name>', help="Logical port name", required=True)
+@click.option('-n', '--page', metavar='<page>', type=click.IntRange(0, MAX_EEPROM_PAGE), help="EEPROM page number", required=True)
+@click.option('-o', '--offset', metavar='<offset>', type=click.IntRange(0, MAX_EEPROM_OFFSET), help="EEPROM offset within the page", required=True)
+@click.option('-s', '--size', metavar='<size>', type=click.IntRange(1, MAX_EEPROM_OFFSET + 1), help="Size of byte to be read", required=True)
 @click.option('--no-format', is_flag=True, help="Display non formatted data")
 @click.option('--wire-addr', help="Wire address of sff8472")
-def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
+def read_eeprom(port, page, offset, size, no_format, wire_addr):
     """Read SFP EEPROM data
-       <port_name>: 
     """
     try:
-        if platform_sfputil.is_logical_port(port_name) == 0:
-            click.echo("Error: invalid port {}".format(port_name))
+        if platform_sfputil.is_logical_port(port) == 0:
+            click.echo("Error: invalid port {}".format(port))
             print_all_valid_port_values()
             sys.exit(ERROR_INVALID_PORT)
 
-        if is_port_type_rj45(port_name):
-            click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
+        if is_port_type_rj45(port):
+            click.echo("This functionality is not applicable for RJ45 port {}.".format(port))
             sys.exit(EXIT_FAIL)
 
-        physical_port = logical_port_to_physical_port_index(port_name)
+        physical_port = logical_port_to_physical_port_index(port)
         sfp = platform_chassis.get_sfp(physical_port)
         if not sfp.get_presence():
-            click.echo("{}: SFP EEPROM not detected\n".format(port_name))
+            click.echo("{}: SFP EEPROM not detected\n".format(port))
             sys.exit(EXIT_FAIL)
 
         from sonic_platform_base.sonic_xcvr.api.public import sff8472
@@ -1654,28 +1653,28 @@ def read_eeprom(port_name, page, offset, size, no_format, wire_addr):
 
 # 'write-eeprom' subcommand
 @cli.command()
-@click.argument('port_name', metavar='<logical_port_name>', required=True)
-@click.argument('page', metavar='<page>', type=click.IntRange(0, MAX_EEPROM_PAGE), required=True)
-@click.argument('offset', metavar='<offset>', type=click.IntRange(0, MAX_EEPROM_OFFSET), required=True)
-@click.argument('data', metavar='<data>', required=True)
+@click.option('-p', '--port', metavar='<logical_port_name>', help="Logical port name", required=True)
+@click.option('-n', '--page', metavar='<page>', type=click.IntRange(0, MAX_EEPROM_PAGE), help="EEPROM page number", required=True)
+@click.option('-o', '--offset', metavar='<offset>', type=click.IntRange(0, MAX_EEPROM_OFFSET), help="EEPROM offset within the page", required=True)
+@click.option('-d', '--data', metavar='<data>', help="Hex string EEPROM data", required=True)
 @click.option('--wire-addr', help="Wire address of sff8472")
 @click.option('--verify', is_flag=True, help="Verify the data by reading back")
-def write_eeprom(port_name, page, offset, data, wire_addr, verify):
+def write_eeprom(port, page, offset, data, wire_addr, verify):
     """Write SFP EEPROM data"""
     try:
-        if platform_sfputil.is_logical_port(port_name) == 0:
-            click.echo("Error: invalid port {}".format(port_name))
+        if platform_sfputil.is_logical_port(port) == 0:
+            click.echo("Error: invalid port {}".format(port))
             print_all_valid_port_values()
             sys.exit(ERROR_INVALID_PORT)
 
-        if is_port_type_rj45(port_name):
-            click.echo("This functionality is not applicable for RJ45 port {}.".format(port_name))
+        if is_port_type_rj45(port):
+            click.echo("This functionality is not applicable for RJ45 port {}.".format(port))
             sys.exit(EXIT_FAIL)
 
-        physical_port = logical_port_to_physical_port_index(port_name)
+        physical_port = logical_port_to_physical_port_index(port)
         sfp = platform_chassis.get_sfp(physical_port)
         if not sfp.get_presence():
-            click.echo("{}: SFP EEPROM not detected\n".format(port_name))
+            click.echo("{}: SFP EEPROM not detected\n".format(port))
             sys.exit(EXIT_FAIL)
 
         try:
diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index 86ed9f23f2..3e7d9f0034 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -978,20 +978,24 @@ def test_read_eeprom(self, mock_chassis):
 
         mock_sfp.get_presence = MagicMock(return_value=False)
         runner = CliRunner()
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-s', '1'])
         assert result.exit_code == EXIT_FAIL
 
         mock_sfp.get_presence.return_value = True
         mock_sfp.read_eeprom = MagicMock(return_value=None)
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-s', '1'])
         assert result.exit_code == ERROR_NOT_IMPLEMENTED
 
         mock_sfp.read_eeprom.return_value = bytearray([0x00, 0x01])
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '2', '--no-format'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-s', '2', '--no-format'])
         assert result.exit_code == 0
         assert result.output == '0001\n'
 
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '5', '-s', '2'])
         assert result.exit_code == 0
         expected_output = """        00000005 00 01                                            |..|
 """
@@ -999,11 +1003,13 @@ def test_read_eeprom(self, mock_chassis):
         assert result.output == expected_output
 
         mock_sfp.read_eeprom.side_effect = NotImplementedError
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '5', '-s', '2'])
         assert result.exit_code == ERROR_NOT_IMPLEMENTED
 
         mock_sfp.read_eeprom.side_effect = ValueError
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '5', '2'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '5', '-s', '2'])
         assert result.exit_code == EXIT_FAIL
 
     @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=False))
@@ -1016,73 +1022,86 @@ def test_write_eeprom(self, mock_chassis):
 
         mock_sfp.get_presence = MagicMock(return_value=False)
         runner = CliRunner()
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '01'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '01'])
         assert result.exit_code == EXIT_FAIL
 
         # invalid hex string, hex string must have even length
         mock_sfp.get_presence.return_value = True
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '1'])
         assert result.exit_code == EXIT_FAIL
 
         # invalid hex string
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '+0'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '+0'])
         assert result.exit_code == EXIT_FAIL
 
         # write failed
         mock_sfp.write_eeprom = MagicMock(return_value=False)
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '10'])
         print(result.output)
         assert result.exit_code == ERROR_NOT_IMPLEMENTED
 
         # write success
         mock_sfp.write_eeprom.return_value = True
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '10'])
         assert result.exit_code == 0
 
         # write verify success
         mock_sfp.read_eeprom = MagicMock(return_value=bytearray([16]))
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10', '--verify'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '10', '--verify'])
         assert result.exit_code == 0
 
         # write verify failed
         mock_sfp.read_eeprom = MagicMock(return_value=bytearray([10]))
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '11', '--verify'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '11', '--verify'])
         assert result.exit_code != 0
 
         # Not implemented
         mock_sfp.write_eeprom.side_effect = NotImplementedError
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '10'])
         assert result.exit_code == ERROR_NOT_IMPLEMENTED
 
         # Value error
         mock_sfp.write_eeprom.side_effect = ValueError
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '10'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '10'])
         assert result.exit_code == EXIT_FAIL
 
     @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=0)))
     def test_read_eeprom_invalid_port(self):
         runner = CliRunner()
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '1'])
         assert result.exit_code == ERROR_INVALID_PORT
 
     @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=0)))
     def test_write_eeprom_invalid_port(self):
         runner = CliRunner()
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '00'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '00'])
         assert result.exit_code == ERROR_INVALID_PORT
 
     @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=True))
     @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1)))
     def test_read_eeprom_rj45(self):
         runner = CliRunner()
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '0', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-s', '1'])
         assert result.exit_code == EXIT_FAIL
 
     @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=True))
     @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1)))
     def test_write_eeprom_rj45(self):
         runner = CliRunner()
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '0', '0', '00'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '00'])
         assert result.exit_code == EXIT_FAIL
 
     @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=False))
@@ -1099,31 +1118,40 @@ def test_get_overall_offset_general(self, mock_chassis):
         mock_sfp.get_xcvr_api = MagicMock(return_value=api)
         
         runner = CliRunner()
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '-1', '0', '01'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '-1', '-o', '0', '-d', '01'])
         assert result.exit_code != 0
         
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '256', '0', '01'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '256', '-o', '0', '-d', '01'])
         assert result.exit_code != 0
         
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '0', '-1', '01'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '-1', '-d', '01'])
         assert result.exit_code != 0
         
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '0', '256', '01'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '256', '-d', '01'])
         assert result.exit_code != 0
         
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '1', '127', '01'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '1', '-o', '127', '-d', '01'])
         assert result.exit_code != 0
         
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '1', '256', '01'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '1', '-o', '256', '-d', '01'])
         assert result.exit_code != 0
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--', '0', '0', '0'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-s', '0'])
         assert result.exit_code != 0
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--', '0', '0', '257'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-s', '257'])
         assert result.exit_code != 0
         
-        result = runner.invoke(sfputil.cli.commands['write-eeprom'], ["Ethernet0", '--', '0', '1', '01'])
+        result = runner.invoke(sfputil.cli.commands['write-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '1', '-d', '01'])
         assert result.exit_code == 0
 
     @patch('sfputil.main.isinstance', MagicMock(return_value=True))
@@ -1141,53 +1169,65 @@ def test_get_overall_offset_sff8472(self, mock_chassis):
         mock_sfp.get_xcvr_api = MagicMock(return_value=api)
         
         runner = CliRunner()
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--', '0', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-s', '1'])
         assert result.exit_code != 0
         print(result.output)
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'invalid', '0', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'invalid', '-n', '0', '-o', '0', '-s', '1'])
         assert result.exit_code != 0
         print(result.output)
 
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a0h', '1', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'a0h', '-n', '1', '-o', '0', '-s', '1'])
         assert result.exit_code != 0
         print(result.output)
 
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'A0h', '--', '0', '-1', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'A0h', '-n', '0', '-o', '-1', '-s', '1'])
         assert result.exit_code != 0
         print(result.output)
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'A0h', '--', '0', '256', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'A0h', '-n', '0', '-o', '256', '-s', '1'])
         assert result.exit_code != 0
         print(result.output)
 
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'A0h', '--', '0', '0', '0'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'], 
+                               ['-p', "Ethernet0", '--wire-addr', 'A0h', '-n', '0', '-o', '0', '-s', '0'])
         assert result.exit_code != 0
         print(result.output)
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'A0h', '--', '0', '0', '257'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'A0h', '-n', '0', '-o', '0', '-s', '257'])
         assert result.exit_code != 0
         print(result.output)
         
         assert sfputil.get_overall_offset_sff8472(api, 0, 2, 2, wire_addr='A0h') == 2
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '-1', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'a2h', '-n', '-1', '-o', '0', '-s', '1'])
         assert result.exit_code != 0
         print(result.output)
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '256', '0', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'a2h', '-n', '256', '-o', '0', '-s', '1'])
         assert result.exit_code != 0
         print(result.output)
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '0', '-1', '1'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'a2h', '-n', '0', '-o', '-1', '-s', '1'])
         assert result.exit_code != 0
         print(result.output)
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '0', '0', '0'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'a2h', '-n', '0', '-o', '0', '-s', '0'])
         assert result.exit_code != 0
         print(result.output)
         
-        result = runner.invoke(sfputil.cli.commands['read-eeprom'], ["Ethernet0", '--wire-addr', 'a2h', '--', '0', '0', '257'])
+        result = runner.invoke(sfputil.cli.commands['read-eeprom'],
+                               ['-p', "Ethernet0", '--wire-addr', 'a2h', '-n', '0', '-o', '0', '-s', '257'])
         assert result.exit_code != 0
         print(result.output)
         

From dc01dc83a0bb10bb91a34153c3a3c7d26d14d6f8 Mon Sep 17 00:00:00 2001
From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com>
Date: Thu, 7 Dec 2023 16:40:04 +0800
Subject: [PATCH 17/17] Update sfputil_test.py

---
 tests/sfputil_test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py
index 3e7d9f0034..10ca69240b 100644
--- a/tests/sfputil_test.py
+++ b/tests/sfputil_test.py
@@ -1078,7 +1078,7 @@ def test_write_eeprom(self, mock_chassis):
     def test_read_eeprom_invalid_port(self):
         runner = CliRunner()
         result = runner.invoke(sfputil.cli.commands['read-eeprom'],
-                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-d', '1'])
+                               ['-p', "Ethernet0", '-n', '0', '-o', '0', '-s', '1'])
         assert result.exit_code == ERROR_INVALID_PORT
 
     @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=0)))