| Message ID | 1429302240-654-1-git-send-email-jtoppins@cumulusnetworks.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On 4/17/15 4:23 PM, Jonathan Toppins wrote: > From: Alan Liebthal <alanl@cumulusnetworks.com> > > The Quanta LY8 Ethernet management port uses a Broadcom 5461S chip for > the PHY layer. This adds support for this PHY to the Intel igb driver. > > Signed-off-by: Alan Liebthal <alanl@cumulusnetworks.com> > Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> > --- > drivers/net/ethernet/intel/igb/e1000_82575.c | 20 +++++- > drivers/net/ethernet/intel/igb/e1000_defines.h | 1 + > drivers/net/ethernet/intel/igb/e1000_hw.h | 1 + > drivers/net/ethernet/intel/igb/e1000_phy.c | 79 ++++++++++++++++++++++++ > drivers/net/ethernet/intel/igb/e1000_phy.h | 2 + > 5 files changed, 102 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c > index 2dcc808..e222804 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_82575.c > +++ b/drivers/net/ethernet/intel/igb/e1000_82575.c > @@ -178,7 +178,7 @@ static s32 igb_init_phy_params_82575(struct e1000_hw *hw) > > ctrl_ext = rd32(E1000_CTRL_EXT); > > - if (igb_sgmii_active_82575(hw)) { > + if (igb_sgmii_active_82575(hw) && phy->type != e1000_phy_bcm5461s) { > phy->ops.reset = igb_phy_hw_reset_sgmii_82575; > ctrl_ext |= E1000_CTRL_I2C_ENA; > } else { > @@ -289,6 +289,11 @@ static s32 igb_init_phy_params_82575(struct e1000_hw *hw) > case BCM54616_E_PHY_ID: > phy->type = e1000_phy_bcm54616; > break; > + case BCM5461S_E_PHY_ID: > + phy->type = e1000_phy_bcm5461s; > + phy->ops.get_phy_info = igb_get_phy_info_5461s; > + phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_82580; > + break; > default: > ret_val = -E1000_ERR_PHY; > goto out; > @@ -841,6 +846,15 @@ static s32 igb_get_phy_id_82575(struct e1000_hw *hw) > goto out; > } > ret_val = igb_get_phy_id(hw); > + if (ret_val && hw->mac.type == e1000_i354) { > + /* We do a special check for bcm5461s phy by setting > + * the phy->addr to 5 and doing the phy check again. > + * This call will succeed and retrieve a valid phy > + * id if we have the bcm5461s phy. > + */ > + phy->addr = 5; > + ret_val = igb_get_phy_id(hw); > + } > goto out; > } > > @@ -1221,6 +1235,9 @@ static s32 igb_get_cfg_done_82575(struct e1000_hw *hw) > (hw->phy.type == e1000_phy_igp_3)) > igb_phy_init_script_igp3(hw); > > + if (hw->phy.type == e1000_phy_bcm5461s) > + igb_phy_init_script_5461s(hw); > + > return 0; > } > > @@ -1597,6 +1614,7 @@ static s32 igb_setup_copper_link_82575(struct e1000_hw *hw) > ret_val = igb_copper_link_setup_82580(hw); > break; > case e1000_phy_bcm54616: > + case e1000_phy_bcm5461s: > break; > default: > ret_val = -E1000_ERR_PHY; > diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h > index 50d51e4..787224d 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_defines.h > +++ b/drivers/net/ethernet/intel/igb/e1000_defines.h > @@ -861,6 +861,7 @@ > #define I210_I_PHY_ID 0x01410C00 > #define M88E1543_E_PHY_ID 0x01410EA0 > #define BCM54616_E_PHY_ID 0x03625D10 > +#define BCM5461S_E_PHY_ID 0x002060C0 > > /* M88E1000 Specific Registers */ > #define M88E1000_PHY_SPEC_CTRL 0x10 /* PHY Specific Control Register */ > diff --git a/drivers/net/ethernet/intel/igb/e1000_hw.h b/drivers/net/ethernet/intel/igb/e1000_hw.h > index d82c96b..408a3e5 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_hw.h > +++ b/drivers/net/ethernet/intel/igb/e1000_hw.h > @@ -129,6 +129,7 @@ enum e1000_phy_type { > e1000_phy_82580, > e1000_phy_i210, > e1000_phy_bcm54616, > + e1000_phy_bcm5461s, > }; > > enum e1000_bus_type { > diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c > index c1bb64d..0f5a55b 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_phy.c > +++ b/drivers/net/ethernet/intel/igb/e1000_phy.c > @@ -148,6 +148,13 @@ s32 igb_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data) > * Control register. The MAC will take care of interfacing with the > * PHY to retrieve the desired data. > */ > + if (phy->type == e1000_phy_bcm5461s) { > + mdic = rd32(E1000_MDICNFG); > + mdic &= ~E1000_MDICNFG_PHY_MASK; > + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); > + wr32(E1000_MDICNFG, mdic); > + } > + > mdic = ((offset << E1000_MDIC_REG_SHIFT) | > (phy->addr << E1000_MDIC_PHY_SHIFT) | > (E1000_MDIC_OP_READ)); > @@ -204,6 +211,13 @@ s32 igb_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data) > * Control register. The MAC will take care of interfacing with the > * PHY to retrieve the desired data. > */ > + if (phy->type == e1000_phy_bcm5461s) { > + mdic = rd32(E1000_MDICNFG); > + mdic &= ~E1000_MDICNFG_PHY_MASK; > + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); > + wr32(E1000_MDICNFG, mdic); > + } > + > mdic = (((u32)data) | > (offset << E1000_MDIC_REG_SHIFT) | > (phy->addr << E1000_MDIC_PHY_SHIFT) | > @@ -2509,3 +2523,68 @@ static s32 igb_set_master_slave_mode(struct e1000_hw *hw) > > return hw->phy.ops.write_reg(hw, PHY_1000T_CTRL, phy_data); > } > + > +/** > + * igb_phy_init_script_5461s - Inits the BCM5461S PHY > + * @hw: pointer to the HW structure > + * > + * Initializes a Broadcom Gigabit PHY. > + **/ > +s32 igb_phy_init_script_5461s(struct e1000_hw *hw) > +{ > + u16 mii_reg_led = 0; > + > + /* 1. Speed LED (Set the Link LED mode), Shadow 00010, 0x1C.bit2=1 */ > + hw->phy.ops.write_reg(hw, 0x1C, 0x0800); > + hw->phy.ops.read_reg(hw, 0x1C, &mii_reg_led); > + mii_reg_led |= 0x0004; > + hw->phy.ops.write_reg(hw, 0x1C, mii_reg_led | 0x8000); > + > + /* 2. Active LED (Set the Link LED mode), Shadow 01001, 0x1C.bit4=1, > + * 0x10.bit5=0 > + */ > + hw->phy.ops.write_reg(hw, 0x1C, 0x2400); > + hw->phy.ops.read_reg(hw, 0x1C, &mii_reg_led); > + mii_reg_led |= 0x0010; > + hw->phy.ops.write_reg(hw, 0x1C, mii_reg_led | 0x8000); > + hw->phy.ops.read_reg(hw, 0x10, &mii_reg_led); > + mii_reg_led &= 0xffdf; > + hw->phy.ops.write_reg(hw, 0x10, mii_reg_led); > + > + return 0; > +} > + > +/** > + * igb_get_phy_info_5461s - Retrieve 5461s PHY information > + * @hw: pointer to the HW structure > + * > + * Read PHY status to determine if link is up. If link is up, then > + * set/determine 10base-T extended distance and polarity correction. Read > + * PHY port status to determine MDI/MDIx and speed. Based on the speed, > + * determine on the cable length, local and remote receiver. > + **/ > +s32 igb_get_phy_info_5461s(struct e1000_hw *hw) > +{ > + struct e1000_phy_info *phy = &hw->phy; > + s32 ret_val; > + bool link; > + > + ret_val = igb_phy_has_link(hw, 1, 0, &link); > + if (ret_val) > + goto out; > + > + if (!link) { > + ret_val = -E1000_ERR_CONFIG; > + goto out; > + } > + > + phy->polarity_correction = true; > + > + phy->is_mdix = true; > + phy->cable_length = E1000_CABLE_LENGTH_UNDEFINED; > + phy->local_rx = e1000_1000t_rx_status_ok; > + phy->remote_rx = e1000_1000t_rx_status_ok; > + > +out: > + return ret_val; > +} > diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.h b/drivers/net/ethernet/intel/igb/e1000_phy.h > index 7af4ffa..fcdd84c 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_phy.h > +++ b/drivers/net/ethernet/intel/igb/e1000_phy.h > @@ -61,6 +61,8 @@ s32 igb_phy_has_link(struct e1000_hw *hw, u32 iterations, > void igb_power_up_phy_copper(struct e1000_hw *hw); > void igb_power_down_phy_copper(struct e1000_hw *hw); > s32 igb_phy_init_script_igp3(struct e1000_hw *hw); > +s32 igb_phy_init_script_5461s(struct e1000_hw *hw); > +s32 igb_get_phy_info_5461s(struct e1000_hw *hw); > s32 igb_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data); > s32 igb_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data); > s32 igb_read_phy_reg_i2c(struct e1000_hw *hw, u32 offset, u16 *data); > Simple keepalive to make sure these have not been forgotten, the patchworks entries are still in "new" state. http://patchwork.ozlabs.org/patch/462222/ http://patchwork.ozlabs.org/patch/462223/ Regards, -Jon -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] > On Behalf Of Jonathan Toppins > Sent: Monday, April 27, 2015 8:45 AM > To: Kirsher, Jeffrey T > Cc: Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald > C; Vick, Matthew; Ronciak, John; Williams, Mitch A; intel-wired- > lan@lists.osuosl.org; netdev@vger.kernel.org; gospo@cumulusnetworks.com; > shm@cumulusnetworks.com; Alan Liebthal > Subject: Re: [PATCH v1 net-next 1/2] igb: add PHY support for Broadcom > 5461S > > On 4/17/15 4:23 PM, Jonathan Toppins wrote: > > From: Alan Liebthal <alanl@cumulusnetworks.com> > > > > The Quanta LY8 Ethernet management port uses a Broadcom 5461S chip for > > the PHY layer. This adds support for this PHY to the Intel igb driver. > > > > Signed-off-by: Alan Liebthal <alanl@cumulusnetworks.com> > > Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> > > --- > > drivers/net/ethernet/intel/igb/e1000_82575.c | 20 +++++- > > drivers/net/ethernet/intel/igb/e1000_defines.h | 1 + > > drivers/net/ethernet/intel/igb/e1000_hw.h | 1 + > > drivers/net/ethernet/intel/igb/e1000_phy.c | 79 > ++++++++++++++++++++++++ > > drivers/net/ethernet/intel/igb/e1000_phy.h | 2 + > > 5 files changed, 102 insertions(+), 1 deletion(-) > > I don't have access to the Broadcom 5461s, but will assume Jonathan / Alan checked this on it on their end. My set of regression systems did not have any problems with this, so... Tested-by: Aaron Brown <aaron.f.brown@intel.com> ... ... > Simple keepalive to make sure these have not been forgotten, the > patchworks entries are still in "new" state. > > http://patchwork.ozlabs.org/patch/462222/ > http://patchwork.ozlabs.org/patch/462223/ > > Regards, > -Jon Nope, not forgotten, I've just been really busy with other things. I'll take a closer look at that other patch (2/2) next early next week. > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 17, 2015 at 1:23 PM, Jonathan Toppins <jtoppins@cumulusnetworks.com> wrote: > From: Alan Liebthal <alanl@cumulusnetworks.com> > > The Quanta LY8 Ethernet management port uses a Broadcom 5461S chip for > the PHY layer. This adds support for this PHY to the Intel igb driver. > > Signed-off-by: Alan Liebthal <alanl@cumulusnetworks.com> > Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> > --- <snip> > --- a/drivers/net/ethernet/intel/igb/e1000_phy.c > +++ b/drivers/net/ethernet/intel/igb/e1000_phy.c > @@ -148,6 +148,13 @@ s32 igb_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data) > * Control register. The MAC will take care of interfacing with the > * PHY to retrieve the desired data. > */ > + if (phy->type == e1000_phy_bcm5461s) { > + mdic = rd32(E1000_MDICNFG); > + mdic &= ~E1000_MDICNFG_PHY_MASK; > + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); > + wr32(E1000_MDICNFG, mdic); > + } > + > mdic = ((offset << E1000_MDIC_REG_SHIFT) | > (phy->addr << E1000_MDIC_PHY_SHIFT) | > (E1000_MDIC_OP_READ)); > @@ -204,6 +211,13 @@ s32 igb_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data) > * Control register. The MAC will take care of interfacing with the > * PHY to retrieve the desired data. > */ > + if (phy->type == e1000_phy_bcm5461s) { > + mdic = rd32(E1000_MDICNFG); > + mdic &= ~E1000_MDICNFG_PHY_MASK; > + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); > + wr32(E1000_MDICNFG, mdic); > + } > + > mdic = (((u32)data) | > (offset << E1000_MDIC_REG_SHIFT) | > (phy->addr << E1000_MDIC_PHY_SHIFT) | > @@ -2509,3 +2523,68 @@ static s32 igb_set_master_slave_mode(struct e1000_hw *hw) > > return hw->phy.ops.write_reg(hw, PHY_1000T_CTRL, phy_data); > } Jonathan, Is this bcm5461s attached to an i210/i211? These changes look a lot like some changes I'm trying to upstream to add support for i210/i211 which require the phy address in the MDICNFG register. If this is the case, then I think the right approach is to check for hw->mac.type = e1000_i210/e1000_i211 and I can submit my patch for review. Regards, Tim -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5/7/15 12:18 PM, Tim Harvey wrote: > On Fri, Apr 17, 2015 at 1:23 PM, Jonathan Toppins > <jtoppins@cumulusnetworks.com> wrote: >> From: Alan Liebthal <alanl@cumulusnetworks.com> >> >> The Quanta LY8 Ethernet management port uses a Broadcom 5461S chip for >> the PHY layer. This adds support for this PHY to the Intel igb driver. >> >> Signed-off-by: Alan Liebthal <alanl@cumulusnetworks.com> >> Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> >> --- > <snip> >> --- a/drivers/net/ethernet/intel/igb/e1000_phy.c >> +++ b/drivers/net/ethernet/intel/igb/e1000_phy.c >> @@ -148,6 +148,13 @@ s32 igb_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data) >> * Control register. The MAC will take care of interfacing with the >> * PHY to retrieve the desired data. >> */ >> + if (phy->type == e1000_phy_bcm5461s) { >> + mdic = rd32(E1000_MDICNFG); >> + mdic &= ~E1000_MDICNFG_PHY_MASK; >> + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); >> + wr32(E1000_MDICNFG, mdic); >> + } >> + >> mdic = ((offset << E1000_MDIC_REG_SHIFT) | >> (phy->addr << E1000_MDIC_PHY_SHIFT) | >> (E1000_MDIC_OP_READ)); >> @@ -204,6 +211,13 @@ s32 igb_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data) >> * Control register. The MAC will take care of interfacing with the >> * PHY to retrieve the desired data. >> */ >> + if (phy->type == e1000_phy_bcm5461s) { >> + mdic = rd32(E1000_MDICNFG); >> + mdic &= ~E1000_MDICNFG_PHY_MASK; >> + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); >> + wr32(E1000_MDICNFG, mdic); >> + } >> + >> mdic = (((u32)data) | >> (offset << E1000_MDIC_REG_SHIFT) | >> (phy->addr << E1000_MDIC_PHY_SHIFT) | >> @@ -2509,3 +2523,68 @@ static s32 igb_set_master_slave_mode(struct e1000_hw *hw) >> >> return hw->phy.ops.write_reg(hw, PHY_1000T_CTRL, phy_data); >> } > > Jonathan, > > Is this bcm5461s attached to an i210/i211? These changes look a lot > like some changes I'm trying to upstream to add support for i210/i211 > which require the phy address in the MDICNFG register. If this is the > case, then I think the right approach is to check for hw->mac.type = > e1000_i210/e1000_i211 and I can submit my patch for review. > > Regards, > > Tim > Hi Tim, The MAC in question are the ones integrated with Intel's Atom processor, I don't recall the series off hand, output of lspci and /proc/cpuinfo below. If you think your change may apply to this controller as well I would be more than happy to apply your change and test. Thanks, -Jon Supplementary Information. Processor info (8 processors total): root@qct-ly8-01:~# uname -a Linux qct-ly8-01 3.2.60-1+deb7u1+cl2.5+1 #3.2.60-1+deb7u1+cl2.5+1 SMP Mon Apr 13 23:18:31 PDT 2015 x86_64 GNU/Linux root@qct-ly8-01:~# cat /proc/cpuinfo | grep "processor" | wc -l 8 root@qct-ly8-01:~# cat /proc/cpuinfo processor : 0 vendor_id : GenuineIntel cpu family : 6 model : 77 model name : Intel(R) Atom(TM) CPU C2758 @ 2.40GHz stepping : 8 microcode : 0x11d cpu MHz : 2400.191 cache size : 1024 KB physical id : 0 siblings : 8 core id : 0 cpu cores : 8 apicid : 0 initial apicid : 0 fpu : yes fpu_exception : yes cpuid level : 11 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr pdcm sse4_1 sse4_2 movbe popcnt tsc_deadline_timer aes rdrand lahf_lm 3dnowprefetch arat epb dtherm tpr_shadow vnmi flexpriority ept vpid smep erms bogomips : 4800.38 clflush size : 64 cache_alignment : 64 address sizes : 36 bits physical, 48 bits virtual power management: ...trimmed... The PCI info for the controller: root@qct-ly8-01:~# lspci -vvx -s 00:14.0 00:14.0 Ethernet controller: Intel Corporation Device 1f41 (rev 03) Subsystem: Intel Corporation Device 1f41 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0, Cache Line Size: 64 bytes Interrupt: pin A routed to IRQ 20 Region 0: Memory at dff60000 (64-bit, non-prefetchable) [size=128K] Region 2: I/O ports at 1000 [size=32] Region 4: Memory at dff84000 (64-bit, non-prefetchable) [size=16K] Capabilities: [40] Power Management version 3 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=1 PME- Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+ Address: 0000000000000000 Data: 0000 Masking: 00000000 Pending: 00000000 Capabilities: [70] MSI-X: Enable+ Count=10 Masked- Vector table: BAR=4 offset=00000000 PBA: BAR=4 offset=00002000 Capabilities: [a0] Express (v2) Root Complex Integrated Endpoint, MSI 00 DevCap: MaxPayload 512 bytes, PhantFunc 0, Latency L0s <64ns, L1 <1us ExtTag- RBE+ FLReset+ DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend- LnkCap: Port #0, Speed unknown, Width x0, ASPM unknown, Latency L0 <64ns, L1 <1us ClockPM- Surprise- LLActRep- BwNot- LnkCtl: ASPM Disabled; Disabled- Retrain- CommClk- ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed unknown, Width x0, TrErr- Train- SlotClk- DLActive- BWMgmt- ABWMgmt- DevCap2: Completion Timeout: Range ABCD, TimeoutDis+ DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-, Selectable De-emphasis: -6dB Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1- EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest- Kernel driver in use: igb 00: 86 80 41 1f 07 04 10 00 03 00 00 02 10 00 00 00 10: 04 00 f6 df 00 00 00 00 01 10 00 00 00 00 00 00 20: 04 40 f8 df 00 00 00 00 00 00 00 00 86 80 41 1f 30: 00 00 00 00 40 00 00 00 00 00 00 00 0b 01 00 00 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/17/2015 01:23 PM, Jonathan Toppins wrote: > From: Alan Liebthal <alanl@cumulusnetworks.com> > > The Quanta LY8 Ethernet management port uses a Broadcom 5461S chip for > the PHY layer. This adds support for this PHY to the Intel igb driver. > > Signed-off-by: Alan Liebthal <alanl@cumulusnetworks.com> > Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> Feedback below. I would say you need to talk to Quanta, and have them talk to Intel about getting their EEPROM configuration fixed. It looks like the part you have is lacking some EEPROM configuration since most of this patch contains the typical stuff one would do to workaround such an issue. I'm assuming this isn't an SPF pluggable PHY since you are using MDIO for the interface, so they really should be doing some of the initialization in the EEPROM rather than pushing it off to the driver code. > --- > drivers/net/ethernet/intel/igb/e1000_82575.c | 20 +++++- > drivers/net/ethernet/intel/igb/e1000_defines.h | 1 + > drivers/net/ethernet/intel/igb/e1000_hw.h | 1 + > drivers/net/ethernet/intel/igb/e1000_phy.c | 79 ++++++++++++++++++++++++ > drivers/net/ethernet/intel/igb/e1000_phy.h | 2 + > 5 files changed, 102 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c > index 2dcc808..e222804 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_82575.c > +++ b/drivers/net/ethernet/intel/igb/e1000_82575.c > @@ -178,7 +178,7 @@ static s32 igb_init_phy_params_82575(struct e1000_hw *hw) > > ctrl_ext = rd32(E1000_CTRL_EXT); > > - if (igb_sgmii_active_82575(hw)) { > + if (igb_sgmii_active_82575(hw) && phy->type != e1000_phy_bcm5461s) { > phy->ops.reset = igb_phy_hw_reset_sgmii_82575; > ctrl_ext |= E1000_CTRL_I2C_ENA; > } else { This doesn't look right to me. Why do you need a special exception for this part? It seems like we should probably be using this patch assuming you are using an external PHY as the I2C enable is what enables external MII or I2C. Is it possible that the EEPROM is missing the bits for setting up external MDIO correct? You might take a look at the code related to igb_sgmii_uses_mdio_82575(). > @@ -289,6 +289,11 @@ static s32 igb_init_phy_params_82575(struct e1000_hw *hw) > case BCM54616_E_PHY_ID: > phy->type = e1000_phy_bcm54616; > break; > + case BCM5461S_E_PHY_ID: > + phy->type = e1000_phy_bcm5461s; > + phy->ops.get_phy_info = igb_get_phy_info_5461s; > + phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_82580; > + break; > default: > ret_val = -E1000_ERR_PHY; > goto out; > @@ -841,6 +846,15 @@ static s32 igb_get_phy_id_82575(struct e1000_hw *hw) > goto out; > } > ret_val = igb_get_phy_id(hw); > + if (ret_val && hw->mac.type == e1000_i354) { > + /* We do a special check for bcm5461s phy by setting > + * the phy->addr to 5 and doing the phy check again. > + * This call will succeed and retrieve a valid phy > + * id if we have the bcm5461s phy. > + */ > + phy->addr = 5; > + ret_val = igb_get_phy_id(hw); > + } > goto out; > } > This looks like a bad EEPROM to me. The phy ID should already be stored in the EEPROM or be discoverable. > @@ -1221,6 +1235,9 @@ static s32 igb_get_cfg_done_82575(struct e1000_hw *hw) > (hw->phy.type == e1000_phy_igp_3)) > igb_phy_init_script_igp3(hw); > > + if (hw->phy.type == e1000_phy_bcm5461s) > + igb_phy_init_script_5461s(hw); > + > return 0; > } > Yuck, okay so this platform definitely has an EEPROM issue. All of this init stuff should really be in the EEPROM for the part. You should probably have Quanta talk to Intel about getting the correct EEPROM built for this thing. > @@ -1597,6 +1614,7 @@ static s32 igb_setup_copper_link_82575(struct e1000_hw *hw) > ret_val = igb_copper_link_setup_82580(hw); > break; > case e1000_phy_bcm54616: > + case e1000_phy_bcm5461s: > break; > default: > ret_val = -E1000_ERR_PHY; > diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h > index 50d51e4..787224d 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_defines.h > +++ b/drivers/net/ethernet/intel/igb/e1000_defines.h > @@ -861,6 +861,7 @@ > #define I210_I_PHY_ID 0x01410C00 > #define M88E1543_E_PHY_ID 0x01410EA0 > #define BCM54616_E_PHY_ID 0x03625D10 > +#define BCM5461S_E_PHY_ID 0x002060C0 > > /* M88E1000 Specific Registers */ > #define M88E1000_PHY_SPEC_CTRL 0x10 /* PHY Specific Control Register */ > diff --git a/drivers/net/ethernet/intel/igb/e1000_hw.h b/drivers/net/ethernet/intel/igb/e1000_hw.h > index d82c96b..408a3e5 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_hw.h > +++ b/drivers/net/ethernet/intel/igb/e1000_hw.h > @@ -129,6 +129,7 @@ enum e1000_phy_type { > e1000_phy_82580, > e1000_phy_i210, > e1000_phy_bcm54616, > + e1000_phy_bcm5461s, > }; > > enum e1000_bus_type { > diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c > index c1bb64d..0f5a55b 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_phy.c > +++ b/drivers/net/ethernet/intel/igb/e1000_phy.c > @@ -148,6 +148,13 @@ s32 igb_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data) > * Control register. The MAC will take care of interfacing with the > * PHY to retrieve the desired data. > */ > + if (phy->type == e1000_phy_bcm5461s) { > + mdic = rd32(E1000_MDICNFG); > + mdic &= ~E1000_MDICNFG_PHY_MASK; > + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); > + wr32(E1000_MDICNFG, mdic); > + } > + > mdic = ((offset << E1000_MDIC_REG_SHIFT) | > (phy->addr << E1000_MDIC_PHY_SHIFT) | > (E1000_MDIC_OP_READ)); > @@ -204,6 +211,13 @@ s32 igb_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data) > * Control register. The MAC will take care of interfacing with the > * PHY to retrieve the desired data. > */ > + if (phy->type == e1000_phy_bcm5461s) { > + mdic = rd32(E1000_MDICNFG); > + mdic &= ~E1000_MDICNFG_PHY_MASK; > + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); > + wr32(E1000_MDICNFG, mdic); > + } > + > mdic = (((u32)data) | > (offset << E1000_MDIC_REG_SHIFT) | > (phy->addr << E1000_MDIC_PHY_SHIFT) | This bit is already done for you if you have a valid EEPROM. > @@ -2509,3 +2523,68 @@ static s32 igb_set_master_slave_mode(struct e1000_hw *hw) > > return hw->phy.ops.write_reg(hw, PHY_1000T_CTRL, phy_data); > } > + > +/** > + * igb_phy_init_script_5461s - Inits the BCM5461S PHY > + * @hw: pointer to the HW structure > + * > + * Initializes a Broadcom Gigabit PHY. > + **/ > +s32 igb_phy_init_script_5461s(struct e1000_hw *hw) > +{ > + u16 mii_reg_led = 0; > + > + /* 1. Speed LED (Set the Link LED mode), Shadow 00010, 0x1C.bit2=1 */ > + hw->phy.ops.write_reg(hw, 0x1C, 0x0800); > + hw->phy.ops.read_reg(hw, 0x1C, &mii_reg_led); > + mii_reg_led |= 0x0004; > + hw->phy.ops.write_reg(hw, 0x1C, mii_reg_led | 0x8000); > + > + /* 2. Active LED (Set the Link LED mode), Shadow 01001, 0x1C.bit4=1, > + * 0x10.bit5=0 > + */ > + hw->phy.ops.write_reg(hw, 0x1C, 0x2400); > + hw->phy.ops.read_reg(hw, 0x1C, &mii_reg_led); > + mii_reg_led |= 0x0010; > + hw->phy.ops.write_reg(hw, 0x1C, mii_reg_led | 0x8000); > + hw->phy.ops.read_reg(hw, 0x10, &mii_reg_led); > + mii_reg_led &= 0xffdf; > + hw->phy.ops.write_reg(hw, 0x10, mii_reg_led); > + > + return 0; > +} > + This belongs in the device EEPROM, not in the driver. This is why Jeff is suggesting PHYlib. At least if it is there then there only needs to be one copy per device this is attached to while lacking an EEPROM configuration. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c index 2dcc808..e222804 100644 --- a/drivers/net/ethernet/intel/igb/e1000_82575.c +++ b/drivers/net/ethernet/intel/igb/e1000_82575.c @@ -178,7 +178,7 @@ static s32 igb_init_phy_params_82575(struct e1000_hw *hw) ctrl_ext = rd32(E1000_CTRL_EXT); - if (igb_sgmii_active_82575(hw)) { + if (igb_sgmii_active_82575(hw) && phy->type != e1000_phy_bcm5461s) { phy->ops.reset = igb_phy_hw_reset_sgmii_82575; ctrl_ext |= E1000_CTRL_I2C_ENA; } else { @@ -289,6 +289,11 @@ static s32 igb_init_phy_params_82575(struct e1000_hw *hw) case BCM54616_E_PHY_ID: phy->type = e1000_phy_bcm54616; break; + case BCM5461S_E_PHY_ID: + phy->type = e1000_phy_bcm5461s; + phy->ops.get_phy_info = igb_get_phy_info_5461s; + phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_82580; + break; default: ret_val = -E1000_ERR_PHY; goto out; @@ -841,6 +846,15 @@ static s32 igb_get_phy_id_82575(struct e1000_hw *hw) goto out; } ret_val = igb_get_phy_id(hw); + if (ret_val && hw->mac.type == e1000_i354) { + /* We do a special check for bcm5461s phy by setting + * the phy->addr to 5 and doing the phy check again. + * This call will succeed and retrieve a valid phy + * id if we have the bcm5461s phy. + */ + phy->addr = 5; + ret_val = igb_get_phy_id(hw); + } goto out; } @@ -1221,6 +1235,9 @@ static s32 igb_get_cfg_done_82575(struct e1000_hw *hw) (hw->phy.type == e1000_phy_igp_3)) igb_phy_init_script_igp3(hw); + if (hw->phy.type == e1000_phy_bcm5461s) + igb_phy_init_script_5461s(hw); + return 0; } @@ -1597,6 +1614,7 @@ static s32 igb_setup_copper_link_82575(struct e1000_hw *hw) ret_val = igb_copper_link_setup_82580(hw); break; case e1000_phy_bcm54616: + case e1000_phy_bcm5461s: break; default: ret_val = -E1000_ERR_PHY; diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h index 50d51e4..787224d 100644 --- a/drivers/net/ethernet/intel/igb/e1000_defines.h +++ b/drivers/net/ethernet/intel/igb/e1000_defines.h @@ -861,6 +861,7 @@ #define I210_I_PHY_ID 0x01410C00 #define M88E1543_E_PHY_ID 0x01410EA0 #define BCM54616_E_PHY_ID 0x03625D10 +#define BCM5461S_E_PHY_ID 0x002060C0 /* M88E1000 Specific Registers */ #define M88E1000_PHY_SPEC_CTRL 0x10 /* PHY Specific Control Register */ diff --git a/drivers/net/ethernet/intel/igb/e1000_hw.h b/drivers/net/ethernet/intel/igb/e1000_hw.h index d82c96b..408a3e5 100644 --- a/drivers/net/ethernet/intel/igb/e1000_hw.h +++ b/drivers/net/ethernet/intel/igb/e1000_hw.h @@ -129,6 +129,7 @@ enum e1000_phy_type { e1000_phy_82580, e1000_phy_i210, e1000_phy_bcm54616, + e1000_phy_bcm5461s, }; enum e1000_bus_type { diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c index c1bb64d..0f5a55b 100644 --- a/drivers/net/ethernet/intel/igb/e1000_phy.c +++ b/drivers/net/ethernet/intel/igb/e1000_phy.c @@ -148,6 +148,13 @@ s32 igb_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data) * Control register. The MAC will take care of interfacing with the * PHY to retrieve the desired data. */ + if (phy->type == e1000_phy_bcm5461s) { + mdic = rd32(E1000_MDICNFG); + mdic &= ~E1000_MDICNFG_PHY_MASK; + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); + wr32(E1000_MDICNFG, mdic); + } + mdic = ((offset << E1000_MDIC_REG_SHIFT) | (phy->addr << E1000_MDIC_PHY_SHIFT) | (E1000_MDIC_OP_READ)); @@ -204,6 +211,13 @@ s32 igb_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data) * Control register. The MAC will take care of interfacing with the * PHY to retrieve the desired data. */ + if (phy->type == e1000_phy_bcm5461s) { + mdic = rd32(E1000_MDICNFG); + mdic &= ~E1000_MDICNFG_PHY_MASK; + mdic |= (phy->addr << E1000_MDICNFG_PHY_SHIFT); + wr32(E1000_MDICNFG, mdic); + } + mdic = (((u32)data) | (offset << E1000_MDIC_REG_SHIFT) | (phy->addr << E1000_MDIC_PHY_SHIFT) | @@ -2509,3 +2523,68 @@ static s32 igb_set_master_slave_mode(struct e1000_hw *hw) return hw->phy.ops.write_reg(hw, PHY_1000T_CTRL, phy_data); } + +/** + * igb_phy_init_script_5461s - Inits the BCM5461S PHY + * @hw: pointer to the HW structure + * + * Initializes a Broadcom Gigabit PHY. + **/ +s32 igb_phy_init_script_5461s(struct e1000_hw *hw) +{ + u16 mii_reg_led = 0; + + /* 1. Speed LED (Set the Link LED mode), Shadow 00010, 0x1C.bit2=1 */ + hw->phy.ops.write_reg(hw, 0x1C, 0x0800); + hw->phy.ops.read_reg(hw, 0x1C, &mii_reg_led); + mii_reg_led |= 0x0004; + hw->phy.ops.write_reg(hw, 0x1C, mii_reg_led | 0x8000); + + /* 2. Active LED (Set the Link LED mode), Shadow 01001, 0x1C.bit4=1, + * 0x10.bit5=0 + */ + hw->phy.ops.write_reg(hw, 0x1C, 0x2400); + hw->phy.ops.read_reg(hw, 0x1C, &mii_reg_led); + mii_reg_led |= 0x0010; + hw->phy.ops.write_reg(hw, 0x1C, mii_reg_led | 0x8000); + hw->phy.ops.read_reg(hw, 0x10, &mii_reg_led); + mii_reg_led &= 0xffdf; + hw->phy.ops.write_reg(hw, 0x10, mii_reg_led); + + return 0; +} + +/** + * igb_get_phy_info_5461s - Retrieve 5461s PHY information + * @hw: pointer to the HW structure + * + * Read PHY status to determine if link is up. If link is up, then + * set/determine 10base-T extended distance and polarity correction. Read + * PHY port status to determine MDI/MDIx and speed. Based on the speed, + * determine on the cable length, local and remote receiver. + **/ +s32 igb_get_phy_info_5461s(struct e1000_hw *hw) +{ + struct e1000_phy_info *phy = &hw->phy; + s32 ret_val; + bool link; + + ret_val = igb_phy_has_link(hw, 1, 0, &link); + if (ret_val) + goto out; + + if (!link) { + ret_val = -E1000_ERR_CONFIG; + goto out; + } + + phy->polarity_correction = true; + + phy->is_mdix = true; + phy->cable_length = E1000_CABLE_LENGTH_UNDEFINED; + phy->local_rx = e1000_1000t_rx_status_ok; + phy->remote_rx = e1000_1000t_rx_status_ok; + +out: + return ret_val; +} diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.h b/drivers/net/ethernet/intel/igb/e1000_phy.h index 7af4ffa..fcdd84c 100644 --- a/drivers/net/ethernet/intel/igb/e1000_phy.h +++ b/drivers/net/ethernet/intel/igb/e1000_phy.h @@ -61,6 +61,8 @@ s32 igb_phy_has_link(struct e1000_hw *hw, u32 iterations, void igb_power_up_phy_copper(struct e1000_hw *hw); void igb_power_down_phy_copper(struct e1000_hw *hw); s32 igb_phy_init_script_igp3(struct e1000_hw *hw); +s32 igb_phy_init_script_5461s(struct e1000_hw *hw); +s32 igb_get_phy_info_5461s(struct e1000_hw *hw); s32 igb_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data); s32 igb_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data); s32 igb_read_phy_reg_i2c(struct e1000_hw *hw, u32 offset, u16 *data);