| Message ID | 20210213002356.2557207-1-robert.hancock@calian.com |
|---|---|
| Headers | show |
| Series | Xilinx axienet updates | expand |
Hello: This series was applied to netdev/net-next.git (refs/heads/master): On Fri, 12 Feb 2021 18:23:53 -0600 you wrote: > Updates to the Xilinx AXI Ethernet driver to add support for an additional > ethtool operation, and to support dynamic switching between 1000BaseX and > SGMII interface modes. > > Robert Hancock (3): > net: axienet: hook up nway_reset ethtool operation > dt-bindings: net: xilinx_axienet: add xlnx,switch-x-sgmii attribute > net: axienet: Support dynamic switching between 1000BaseX and SGMII > > [...] Here is the summary with links: - [net-next,1/3] net: axienet: hook up nway_reset ethtool operation https://git.kernel.org/netdev/net-next/c/66b51663cdd0 - [net-next,2/3] dt-bindings: net: xilinx_axienet: add xlnx,switch-x-sgmii attribute https://git.kernel.org/netdev/net-next/c/eceac9d2590b - [net-next,3/3] net: axienet: Support dynamic switching between 1000BaseX and SGMII https://git.kernel.org/netdev/net-next/c/6c8f06bb2e51 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On Fri, Feb 12, 2021 at 06:23:56PM -0600, Robert Hancock wrote: > Newer versions of the Xilinx AXI Ethernet core (specifically version 7.2 or > later) allow the core to be configured with a PHY interface mode of "Both", Hi Robert Is it possible to read the version of the core from a register? Is it possible to synthesizer a version 7.2 or > without this feature? I'm just wondering if the DT property is actually needed? > /** > * struct axidma_bd - Axi Dma buffer descriptor layout > * @next: MM2S/S2MM Next Descriptor Pointer > @@ -377,22 +381,29 @@ struct axidma_bd { > * @ndev: Pointer for net_device to which it will be attached. > * @dev: Pointer to device structure > * @phy_node: Pointer to device node structure > + * @phylink: Pointer to phylink instance > + * @phylink_config: phylink configuration settings > + * @pcs_phy: Reference to PCS/PMA PHY if used > + * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in the core > + * @clk: Clock for AXI bus > * @mii_bus: Pointer to MII bus structure > * @mii_clk_div: MII bus clock divider value > * @regs_start: Resource start for axienet device addresses > * @regs: Base address for the axienet_local device address space > * @dma_regs: Base address for the axidma device address space > - * @dma_err_tasklet: Tasklet structure to process Axi DMA errors > + * @dma_err_task: Work structure to process Axi DMA errors > * @tx_irq: Axidma TX IRQ number > * @rx_irq: Axidma RX IRQ number > + * @eth_irq: Ethernet core IRQ number > * @phy_mode: Phy type to identify between MII/GMII/RGMII/SGMII/1000 Base-X > * @options: AxiEthernet option word > - * @last_link: Phy link state in which the PHY was negotiated earlier > * @features: Stores the extended features supported by the axienet hw > * @tx_bd_v: Virtual address of the TX buffer descriptor ring > * @tx_bd_p: Physical address(start address) of the TX buffer descr. ring > + * @tx_bd_num: Size of TX buffer descriptor ring > * @rx_bd_v: Virtual address of the RX buffer descriptor ring > * @rx_bd_p: Physical address(start address) of the RX buffer descr. ring > + * @rx_bd_num: Size of RX buffer descriptor ring > * @tx_bd_ci: Stores the index of the Tx buffer descriptor in the ring being > * accessed currently. Used while alloc. BDs before a TX starts > * @tx_bd_tail: Stores the index of the Tx buffer descriptor in the ring being > @@ -414,23 +425,20 @@ struct axienet_local { > struct net_device *ndev; > struct device *dev; > > - /* Connection to PHY device */ > struct device_node *phy_node; > > struct phylink *phylink; > struct phylink_config phylink_config; > > - /* Reference to PCS/PMA PHY if used */ > struct mdio_device *pcs_phy; This really should of been two patches. One moving the comments around, and a second one adding the new fields. > +static int axienet_mac_prepare(struct phylink_config *config, unsigned int mode, > + phy_interface_t iface) > +{ > + struct net_device *ndev = to_net_dev(config->dev); > + struct axienet_local *lp = netdev_priv(ndev); > + int ret; > + > + switch (iface) { > + case PHY_INTERFACE_MODE_SGMII: > + case PHY_INTERFACE_MODE_1000BASEX: > + if (!lp->switch_x_sgmii) > + return 0; Maybe -EOPNOTSUPP would be better? Andrew
On Sat, 2021-02-13 at 17:43 +0100, Andrew Lunn wrote: > On Fri, Feb 12, 2021 at 06:23:56PM -0600, Robert Hancock wrote: > > Newer versions of the Xilinx AXI Ethernet core (specifically version 7.2 or > > later) allow the core to be configured with a PHY interface mode of "Both", > > Hi Robert > > Is it possible to read the version of the core from a register? Is it > possible to synthesizer a version 7.2 or > without this feature? I'm > just wondering if the DT property is actually needed? The core can still be synthesized with a fixed 1000Base-X or SGMII interface mode in addition to the "Both" option, and I'm not aware of a way to determine what mode has been used based on registers, so I don't think there's really another option. > > > /** > > * struct axidma_bd - Axi Dma buffer descriptor layout > > * @next: MM2S/S2MM Next Descriptor Pointer > > @@ -377,22 +381,29 @@ struct axidma_bd { > > * @ndev: Pointer for net_device to which it will be attached. > > * @dev: Pointer to device structure > > * @phy_node: Pointer to device node structure > > + * @phylink: Pointer to phylink instance > > + * @phylink_config: phylink configuration settings > > + * @pcs_phy: Reference to PCS/PMA PHY if used > > + * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in > > the core > > + * @clk: Clock for AXI bus > > * @mii_bus: Pointer to MII bus structure > > * @mii_clk_div: MII bus clock divider value > > * @regs_start: Resource start for axienet device addresses > > * @regs: Base address for the axienet_local device address space > > * @dma_regs: Base address for the axidma device address space > > - * @dma_err_tasklet: Tasklet structure to process Axi DMA errors > > + * @dma_err_task: Work structure to process Axi DMA errors > > * @tx_irq: Axidma TX IRQ number > > * @rx_irq: Axidma RX IRQ number > > + * @eth_irq: Ethernet core IRQ number > > * @phy_mode: Phy type to identify between MII/GMII/RGMII/SGMII/1000 > > Base-X > > * @options: AxiEthernet option word > > - * @last_link: Phy link state in which the PHY was negotiated earlier > > * @features: Stores the extended features supported by the axienet > > hw > > * @tx_bd_v: Virtual address of the TX buffer descriptor ring > > * @tx_bd_p: Physical address(start address) of the TX buffer descr. > > ring > > + * @tx_bd_num: Size of TX buffer descriptor ring > > * @rx_bd_v: Virtual address of the RX buffer descriptor ring > > * @rx_bd_p: Physical address(start address) of the RX buffer descr. > > ring > > + * @rx_bd_num: Size of RX buffer descriptor ring > > * @tx_bd_ci: Stores the index of the Tx buffer descriptor in the > > ring being > > * accessed currently. Used while alloc. BDs before a TX starts > > * @tx_bd_tail: Stores the index of the Tx buffer descriptor in the > > ring being > > @@ -414,23 +425,20 @@ struct axienet_local { > > struct net_device *ndev; > > struct device *dev; > > > > - /* Connection to PHY device */ > > struct device_node *phy_node; > > > > struct phylink *phylink; > > struct phylink_config phylink_config; > > > > - /* Reference to PCS/PMA PHY if used */ > > struct mdio_device *pcs_phy; > > This really should of been two patches. One moving the comments > around, and a second one adding the new fields. > > > +static int axienet_mac_prepare(struct phylink_config *config, unsigned int > > mode, > > + phy_interface_t iface) > > +{ > > + struct net_device *ndev = to_net_dev(config->dev); > > + struct axienet_local *lp = netdev_priv(ndev); > > + int ret; > > + > > + switch (iface) { > > + case PHY_INTERFACE_MODE_SGMII: > > + case PHY_INTERFACE_MODE_1000BASEX: > > + if (!lp->switch_x_sgmii) > > + return 0; > > Maybe -EOPNOTSUPP would be better? From my reading of the code it appears that this function is called on startup initially even if dynamic switching is not supported, so we would need to return 0 here for that case. The validate callback should trap cases where we attempt to switch modes and that isn't supported. > > Andrew