Re: [PATCH net-next v3 1/2] ixgbe: register a mdiobus
On 12/3/18 11:44 AM, Steve Douthit wrote:
> On 12/3/18 2:07 PM, Florian Fainelli wrote:
>> On 12/3/18 10:55 AM, Steve Douthit wrote:
>>> Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
>>> via the MII interface.
>>>
>>> While this works for dsa devices, it will not work safely with Linux
>>> PHYs in all configurations since the firmware of the ixgbe device may
>>> be polling some PHY addresses in the background.
>>>
>>> Signed-off-by: Stephen Douthit
>>> ---
>>
>> [snip]
>>
>>> +/**
>>> + * ixgbe_mii_bus_write - Write a clause 22/45 register
>>> + * @hw: pointer to hardware structure
>>> + * @addr: address
>>> + * @regnum: register number
>>> + * @regnum: valueto write
>>
>> This should be @val to match the function parameters
>
> OK
>
>>> + **/
>>> +static s32 ixgbe_mii_bus_write(struct mii_bus *bus, int addr, int regnum,
>>> + u16 val)
>>> +{
>>> + struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)bus->priv;
>>
>> Nitpick: cast is not necessary since this is a void * pointer (for that
>> reason).
>
> OK, I'll clean up this and other unnecessary casts.
>
> I forgot Andrew's suggestion to squash the swfw semaphore masks from:
>
> + u32 gssr = hw->phy.phy_semaphore_mask | IXGBE_GSSR_TOKEN_SM;
> +
> + if (hw->bus.lan_id)
> + gssr |= IXGBE_GSSR_PHY1_SM;
> + else
> + gssr |= IXGBE_GSSR_PHY0_SM;
>
> to
>
> + u32 gssr = hw->phy.phy_semaphore_mask;
> + gssr |= IXGBE_GSSR_TOKEN_SM | IXGBE_GSSR_PHY0_SM;
>
> Is it ok to collect both of your 'Reviewed-by:'s with that additional
> change for v4?
I'd think so.
--
Florian
Re: [PATCH net-next v3 1/2] ixgbe: register a mdiobus
On 12/3/18 2:07 PM, Florian Fainelli wrote:
> On 12/3/18 10:55 AM, Steve Douthit wrote:
>> Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
>> via the MII interface.
>>
>> While this works for dsa devices, it will not work safely with Linux
>> PHYs in all configurations since the firmware of the ixgbe device may
>> be polling some PHY addresses in the background.
>>
>> Signed-off-by: Stephen Douthit
>> ---
>
> [snip]
>
>> +/**
>> + * ixgbe_mii_bus_write - Write a clause 22/45 register
>> + * @hw: pointer to hardware structure
>> + * @addr: address
>> + * @regnum: register number
>> + * @regnum: valueto write
>
> This should be @val to match the function parameters
OK
>> + **/
>> +static s32 ixgbe_mii_bus_write(struct mii_bus *bus, int addr, int regnum,
>> + u16 val)
>> +{
>> +struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)bus->priv;
>
> Nitpick: cast is not necessary since this is a void * pointer (for that
> reason).
OK, I'll clean up this and other unnecessary casts.
I forgot Andrew's suggestion to squash the swfw semaphore masks from:
+ u32 gssr = hw->phy.phy_semaphore_mask | IXGBE_GSSR_TOKEN_SM;
+
+ if (hw->bus.lan_id)
+ gssr |= IXGBE_GSSR_PHY1_SM;
+ else
+ gssr |= IXGBE_GSSR_PHY0_SM;
to
+ u32 gssr = hw->phy.phy_semaphore_mask;
+ gssr |= IXGBE_GSSR_TOKEN_SM | IXGBE_GSSR_PHY0_SM;
Is it ok to collect both of your 'Reviewed-by:'s with that additional
change for v4?
Re: [PATCH net-next v3 1/2] ixgbe: register a mdiobus
On 12/3/18 10:55 AM, Steve Douthit wrote:
> Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
> via the MII interface.
>
> While this works for dsa devices, it will not work safely with Linux
> PHYs in all configurations since the firmware of the ixgbe device may
> be polling some PHY addresses in the background.
>
> Signed-off-by: Stephen Douthit
> ---
[snip]
> +/**
> + * ixgbe_mii_bus_write - Write a clause 22/45 register
> + * @hw: pointer to hardware structure
> + * @addr: address
> + * @regnum: register number
> + * @regnum: valueto write
This should be @val to match the function parameters
> + **/
> +static s32 ixgbe_mii_bus_write(struct mii_bus *bus, int addr, int regnum,
> +u16 val)
> +{
> + struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)bus->priv;
Nitpick: cast is not necessary since this is a void * pointer (for that
reason).
> + struct ixgbe_hw *hw = >hw;
> + u32 gssr = hw->phy.phy_semaphore_mask;
> +
> + return ixgbe_mii_bus_write_generic(hw, addr, regnum, val, gssr);
> +}
> +
> +/**
> + * ixgbe_x550em_a_mii_bus_read - Read a clause 22/45 register on x550em_a
> + * @hw: pointer to hardware structure
> + * @addr: address
> + * @regnum: register number
> + **/
> +static s32 ixgbe_x550em_a_mii_bus_read(struct mii_bus *bus, int addr,
> +int regnum)
> +{
> + struct ixgbe_adapter *adapter = (struct ixgbe_adapter *)bus->priv;
Likewise, the cast is not necessary.
Everything else looked fine, so with that fixed:
Reviewed-by: Florian Fainelli
--
Florian
Re: [PATCH net-next v3 1/2] ixgbe: register a mdiobus
On Mon, Dec 03, 2018 at 06:55:22PM +, Steve Douthit wrote: > Most dsa devices expect a 'struct mii_bus' pointer to talk to switches > via the MII interface. > > While this works for dsa devices, it will not work safely with Linux > PHYs in all configurations since the firmware of the ixgbe device may > be polling some PHY addresses in the background. > > Signed-off-by: Stephen Douthit Reviewed-by: Andrew Lunn Andrew
[PATCH net-next v3 1/2] ixgbe: register a mdiobus
Most dsa devices expect a 'struct mii_bus' pointer to talk to switches
via the MII interface.
While this works for dsa devices, it will not work safely with Linux
PHYs in all configurations since the firmware of the ixgbe device may
be polling some PHY addresses in the background.
Signed-off-by: Stephen Douthit
---
drivers/net/ethernet/intel/Kconfig| 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 307 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.h | 2 +
5 files changed, 317 insertions(+)
diff --git a/drivers/net/ethernet/intel/Kconfig
b/drivers/net/ethernet/intel/Kconfig
index 59e1bc0f609e..79ad2c5860f0 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -159,6 +159,7 @@ config IXGBE
tristate "Intel(R) 10GbE PCI Express adapters support"
depends on PCI
select MDIO
+ select MII
imply PTP_1588_CLOCK
---help---
This driver supports Intel(R) 10GbE PCI Express family of
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 143bdd5ee2a0..08d85e336bd4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
@@ -561,6 +562,7 @@ struct ixgbe_adapter {
struct net_device *netdev;
struct bpf_prog *xdp_prog;
struct pci_dev *pdev;
+ struct mii_bus *mii_bus;
unsigned long state;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 49a4ea38eb07..82af3b24d222 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -39,6 +39,7 @@
#include "ixgbe.h"
#include "ixgbe_common.h"
#include "ixgbe_dcb_82599.h"
+#include "ixgbe_phy.h"
#include "ixgbe_sriov.h"
#include "ixgbe_model.h"
#include "ixgbe_txrx_common.h"
@@ -11120,6 +11121,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
IXGBE_LINK_SPEED_10GB_FULL | IXGBE_LINK_SPEED_1GB_FULL,
true);
+ ixgbe_mii_bus_init(hw);
+
return 0;
err_register:
@@ -11170,6 +11173,8 @@ static void ixgbe_remove(struct pci_dev *pdev)
set_bit(__IXGBE_REMOVING, >state);
cancel_work_sync(>service_task);
+ if (adapter->mii_bus)
+ mdiobus_unregister(adapter->mii_bus);
#ifdef CONFIG_IXGBE_DCA
if (adapter->flags & IXGBE_FLAG_DCA_ENABLED) {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
index 919a7af84b42..ccc19edaaf9c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
@@ -3,6 +3,7 @@
#include
#include
+#include
#include
#include "ixgbe.h"
@@ -658,6 +659,312 @@ s32 ixgbe_write_phy_reg_generic(struct ixgbe_hw *hw, u32
reg_addr,
return status;
}
+#define IXGBE_HW_READ_REG(addr) IXGBE_READ_REG(hw, addr)
+
+/**
+ * ixgbe_msca_cmd - Write the command register and poll for completion/timeout
+ * @hw: pointer to hardware structure
+ * @cmd: command register value to write
+ **/
+static s32 ixgbe_msca_cmd(struct ixgbe_hw *hw, u32 cmd)
+{
+ IXGBE_WRITE_REG(hw, IXGBE_MSCA, cmd);
+
+ return readx_poll_timeout(IXGBE_HW_READ_REG, IXGBE_MSCA, cmd,
+ !(cmd & IXGBE_MSCA_MDI_COMMAND), 10,
+ 10 * IXGBE_MDIO_COMMAND_TIMEOUT);
+}
+
+/**
+ * ixgbe_mii_bus_read_generic - Read a clause 22/45 register with gssr flags
+ * @hw: pointer to hardware structure
+ * @addr: address
+ * @regnum: register number
+ * @gssr: semaphore flags to acquire
+ **/
+static s32 ixgbe_mii_bus_read_generic(struct ixgbe_hw *hw, int addr,
+ int regnum, u32 gssr)
+{
+ u32 hwaddr, cmd;
+ s32 data;
+
+ if (hw->mac.ops.acquire_swfw_sync(hw, gssr))
+ return -EBUSY;
+
+ hwaddr = addr << IXGBE_MSCA_PHY_ADDR_SHIFT;
+ if (regnum & MII_ADDR_C45) {
+ hwaddr |= regnum & GENMASK(21, 0);
+ cmd = hwaddr | IXGBE_MSCA_ADDR_CYCLE | IXGBE_MSCA_MDI_COMMAND;
+ } else {
+ hwaddr |= (regnum & GENMASK(5, 0)) << IXGBE_MSCA_DEV_TYPE_SHIFT;
+ cmd = hwaddr | IXGBE_MSCA_OLD_PROTOCOL |
+ IXGBE_MSCA_READ_AUTOINC | IXGBE_MSCA_MDI_COMMAND;
+ }
+
+ data = ixgbe_msca_cmd(hw, cmd);
+ if (data < 0)
+ goto mii_bus_read_done;
+
+ /* For a clause 45 access the address cycle just completed, we still
+* need to do the read command, otherwise just get the data
+*/
+ if (!(regnum & MII_ADDR_C45))
+ goto
