On 2/8/24 00:15, Ernesto Castellotti wrote:
Added support for 1000BASE-BX, i.e. Gigabit Ethernet over single strand
of single-mode fiber.
The initialization of a 1000BASE-BX SFP is the same as 1000BASE-SX/LX
with the only difference that the Bit Rate Nominal Value must be
checked to make sure it is a Gigabit Ethernet transceiver, as described
by the SFF-8472 specification.


Thank you for your patch!, and sorry for long break in review process.

[...]


Signed-off-by: Ernesto Castellotti <[email protected]>
---
v2: Fixed indentation/style issues as per review by Tony Nguyen
v1: 
https://lore.kernel.org/intel-wired-lan/[email protected]/
---
  .../net/ethernet/intel/ixgbe/ixgbe_82599.c    |  4 ++-
  .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  |  2 ++
  drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 32 ++++++++++++++++---
  drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h  |  2 ++
  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  3 ++
  5 files changed, 38 insertions(+), 5 deletions(-)


[...]

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
index ef72729d7c93..b5bc60916402 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h
@@ -17,6 +17,7 @@
  #define IXGBE_SFF_1GBE_COMP_CODES     0x6
  #define IXGBE_SFF_10GBE_COMP_CODES    0x3
  #define IXGBE_SFF_CABLE_TECHNOLOGY    0x8
+#define IXGBE_SFF_BITRATE_NOMINAL      0xC
  #define IXGBE_SFF_CABLE_SPEC_COMP     0x3C
  #define IXGBE_SFF_SFF_8472_SWAP               0x5C
  #define IXGBE_SFF_SFF_8472_COMP               0x5E
@@ -39,6 +40,7 @@
  #define IXGBE_SFF_1GBASESX_CAPABLE            0x1
  #define IXGBE_SFF_1GBASELX_CAPABLE            0x2
  #define IXGBE_SFF_1GBASET_CAPABLE             0x8
+#define IXGBE_SFF_BASEBX10_CAPABLE             0x64

this define looks off - perhaps it should be named
IXGBE_SFF_1GBASEBX_CAPABLE?

  #define IXGBE_SFF_10GBASESR_CAPABLE           0x10
  #define IXGBE_SFF_10GBASELR_CAPABLE           0x20
  #define IXGBE_SFF_SOFT_RS_SELECT_MASK         0x8

[...]

code looks fine (as much as our ixgbe will go ;))

just one nitpick, so:
Reviewed-by: Przemek Kitszel <[email protected]>

Reply via email to