linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/5] net: altera: tse: phylink conversion
@ 2022-09-01 14:35 Maxime Chevallier
  2022-09-01 14:35 ` [PATCH net-next v3 1/5] dt-bindings: net: Convert Altera TSE bindings to yaml Maxime Chevallier
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Maxime Chevallier @ 2022-09-01 14:35 UTC (permalink / raw)
  To: davem, Rob Herring
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Florian Fainelli, Heiner Kallweit, Russell King,
	linux-arm-kernel, Krzysztof Kozlowski, devicetree

This is V3 of a series converting the Altera TSE driver to phylink,
introducing a new PCS driver along the way.

The Altera TSE can be built with a SGMII/1000BaseX PCS, allowing to use
SFP ports with this MAC, which is the end goal of adding phylink support
and a proper PCS driver.

The PCS itself can either be mapped in the MAC's register space, in that
case, it's accessed through 32 bits registers, with the higher 16 bits
always 0. Alternatively, it can sit on its own register space, exposing
16 bits registers, some of which ressemble the standard PHY registers.

To tackle that rework, several things needs updating, starting by the DT
binding, since we add support for a new register range for the PCS.

Hence, the first patch of the series is a conversion to YAML of the
existing binding.

Then, patch 2 does a bit of simple cleanup to the TSE driver, using nice
reverse xmas tree definitions.

Patch 3 adds the actual PCS driver, as a standalone driver. Some future
series will then reuse that PCS driver from the dwmac-socfpga driver,
which implements support for this exact PCS too, allowing to share the
code nicely.

Patch 4 is then a phylink conversion of the altera_tse driver, to use
this new PCS driver.

Finally, patch 5 updates the newly converted DT binding to support the
pcs register range.

This series contains bits and pieces for this conversion, please tell me if
you want me to send it as individual patches.

Thanks,

Maxime

V3 Changes:
 - YAML binding conversion changes and PCS addition changes thanks to
   Krzysztof's reviews

V2 Changes :
 - Fixed the binding after the YAML conversion
 - Added a pcs_validate() callback
 - Introduced a comment to justify a soft reset for the PCS

Maxime Chevallier (5):
  dt-bindings: net: Convert Altera TSE bindings to yaml
  net: altera: tse: cosmetic change to use reverse xmas tree ordering
  net: pcs: add new PCS driver for altera TSE PCS
  net: altera: tse: convert to phylink
  dt-bindings: net: altera: tse: add an optional pcs register range

 .../devicetree/bindings/net/altera_tse.txt    | 113 -----
 .../devicetree/bindings/net/altr,tse.yaml     | 168 +++++++
 MAINTAINERS                                   |   7 +
 drivers/net/ethernet/altera/Kconfig           |   2 +
 drivers/net/ethernet/altera/altera_tse.h      |  19 +-
 .../net/ethernet/altera/altera_tse_ethtool.c  |  22 +-
 drivers/net/ethernet/altera/altera_tse_main.c | 453 +++++-------------
 drivers/net/pcs/Kconfig                       |   6 +
 drivers/net/pcs/Makefile                      |   1 +
 drivers/net/pcs/pcs-altera-tse.c              | 171 +++++++
 include/linux/pcs-altera-tse.h                |  17 +
 11 files changed, 532 insertions(+), 447 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/altera_tse.txt
 create mode 100644 Documentation/devicetree/bindings/net/altr,tse.yaml
 create mode 100644 drivers/net/pcs/pcs-altera-tse.c
 create mode 100644 include/linux/pcs-altera-tse.h

-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next v3 1/5] dt-bindings: net: Convert Altera TSE bindings to yaml
  2022-09-01 14:35 [PATCH net-next v3 0/5] net: altera: tse: phylink conversion Maxime Chevallier
@ 2022-09-01 14:35 ` Maxime Chevallier
  2022-09-01 14:35 ` [PATCH net-next v3 2/5] net: altera: tse: cosmetic change to use reverse xmas tree ordering Maxime Chevallier
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Maxime Chevallier @ 2022-09-01 14:35 UTC (permalink / raw)
  To: davem, Rob Herring
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Florian Fainelli, Heiner Kallweit, Russell King,
	linux-arm-kernel, Krzysztof Kozlowski, devicetree

Convert the bindings for the Altera Triple-Speed Ethernet to yaml.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V2->V3:
 - Moved allOf below required
 - Removed unnedded reg/reg-names in the properties section
 - Removed stray minItems

V1->V2:
 - Removed unnedded maxItems
 - Added missing minItems
 - Fixed typos in some properties names
 - Fixed the mdio subnode definition

 .../devicetree/bindings/net/altera_tse.txt    | 113 --------------
 .../devicetree/bindings/net/altr,tse.yaml     | 141 ++++++++++++++++++
 2 files changed, 141 insertions(+), 113 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/altera_tse.txt
 create mode 100644 Documentation/devicetree/bindings/net/altr,tse.yaml

diff --git a/Documentation/devicetree/bindings/net/altera_tse.txt b/Documentation/devicetree/bindings/net/altera_tse.txt
deleted file mode 100644
index 1d9148ff5130..000000000000
--- a/Documentation/devicetree/bindings/net/altera_tse.txt
+++ /dev/null
@@ -1,113 +0,0 @@
-* Altera Triple-Speed Ethernet MAC driver (TSE)
-
-Required properties:
-- compatible: Should be "altr,tse-1.0" for legacy SGDMA based TSE, and should
-		be "altr,tse-msgdma-1.0" for the preferred MSGDMA based TSE.
-		ALTR is supported for legacy device trees, but is deprecated.
-		altr should be used for all new designs.
-- reg: Address and length of the register set for the device. It contains
-  the information of registers in the same order as described by reg-names
-- reg-names: Should contain the reg names
-  "control_port": MAC configuration space region
-  "tx_csr":       xDMA Tx dispatcher control and status space region
-  "tx_desc":      MSGDMA Tx dispatcher descriptor space region
-  "rx_csr" :      xDMA Rx dispatcher control and status space region
-  "rx_desc":      MSGDMA Rx dispatcher descriptor space region
-  "rx_resp":      MSGDMA Rx dispatcher response space region
-  "s1":		  SGDMA descriptor memory
-- interrupts: Should contain the TSE interrupts and its mode.
-- interrupt-names: Should contain the interrupt names
-  "rx_irq":       xDMA Rx dispatcher interrupt
-  "tx_irq":       xDMA Tx dispatcher interrupt
-- rx-fifo-depth: MAC receive FIFO buffer depth in bytes
-- tx-fifo-depth: MAC transmit FIFO buffer depth in bytes
-- phy-mode: See ethernet.txt in the same directory.
-- phy-handle: See ethernet.txt in the same directory.
-- phy-addr: See ethernet.txt in the same directory. A configuration should
-		include phy-handle or phy-addr.
-- altr,has-supplementary-unicast:
-		If present, TSE supports additional unicast addresses.
-		Otherwise additional unicast addresses are not supported.
-- altr,has-hash-multicast-filter:
-		If present, TSE supports a hash based multicast filter.
-		Otherwise, hash-based multicast filtering is not supported.
-
-- mdio device tree subnode: When the TSE has a phy connected to its local
-		mdio, there must be device tree subnode with the following
-		required properties:
-
-	- compatible: Must be "altr,tse-mdio".
-	- #address-cells: Must be <1>.
-	- #size-cells: Must be <0>.
-
-	For each phy on the mdio bus, there must be a node with the following
-	fields:
-
-	- reg: phy id used to communicate to phy.
-	- device_type: Must be "ethernet-phy".
-
-The MAC address will be determined using the optional properties defined in
-ethernet.txt.
-
-Example:
-
-	tse_sub_0_eth_tse_0: ethernet@1,00000000 {
-		compatible = "altr,tse-msgdma-1.0";
-		reg =	<0x00000001 0x00000000 0x00000400>,
-			<0x00000001 0x00000460 0x00000020>,
-			<0x00000001 0x00000480 0x00000020>,
-			<0x00000001 0x000004A0 0x00000008>,
-			<0x00000001 0x00000400 0x00000020>,
-			<0x00000001 0x00000420 0x00000020>;
-		reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc";
-		interrupt-parent = <&hps_0_arm_gic_0>;
-		interrupts = <0 41 4>, <0 40 4>;
-		interrupt-names = "rx_irq", "tx_irq";
-		rx-fifo-depth = <2048>;
-		tx-fifo-depth = <2048>;
-		address-bits = <48>;
-		max-frame-size = <1500>;
-		local-mac-address = [ 00 00 00 00 00 00 ];
-		phy-mode = "gmii";
-		altr,has-supplementary-unicast;
-		altr,has-hash-multicast-filter;
-		phy-handle = <&phy0>;
-		mdio {
-			compatible = "altr,tse-mdio";
-			#address-cells = <1>;
-			#size-cells = <0>;
-			phy0: ethernet-phy@0 {
-				reg = <0x0>;
-				device_type = "ethernet-phy";
-			};
-
-			phy1: ethernet-phy@1 {
-				reg = <0x1>;
-				device_type = "ethernet-phy";
-			};
-
-		};
-	};
-
-	tse_sub_1_eth_tse_0: ethernet@1,00001000 {
-		compatible = "altr,tse-msgdma-1.0";
-		reg = 	<0x00000001 0x00001000 0x00000400>,
-			<0x00000001 0x00001460 0x00000020>,
-			<0x00000001 0x00001480 0x00000020>,
-			<0x00000001 0x000014A0 0x00000008>,
-			<0x00000001 0x00001400 0x00000020>,
-			<0x00000001 0x00001420 0x00000020>;
-		reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc";
-		interrupt-parent = <&hps_0_arm_gic_0>;
-		interrupts = <0 43 4>, <0 42 4>;
-		interrupt-names = "rx_irq", "tx_irq";
-		rx-fifo-depth = <2048>;
-		tx-fifo-depth = <2048>;
-		address-bits = <48>;
-		max-frame-size = <1500>;
-		local-mac-address = [ 00 00 00 00 00 00 ];
-		phy-mode = "gmii";
-		altr,has-supplementary-unicast;
-		altr,has-hash-multicast-filter;
-		phy-handle = <&phy1>;
-	};
diff --git a/Documentation/devicetree/bindings/net/altr,tse.yaml b/Documentation/devicetree/bindings/net/altr,tse.yaml
new file mode 100644
index 000000000000..78c7a2047910
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/altr,tse.yaml
@@ -0,0 +1,141 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/altr,tse.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Altera Triple Speed Ethernet MAC driver (TSE)
+
+maintainers:
+  - Maxime Chevallier <maxime.chevallier@bootlin.com>
+
+properties:
+  compatible:
+    oneOf:
+      - const: altr,tse-1.0
+      - const: ALTR,tse-1.0
+        deprecated: true
+      - const: altr,tse-msgdma-1.0
+
+  interrupts:
+    minItems: 2
+
+  interrupt-names:
+    items:
+      - const: rx_irq
+      - const: tx_irq
+
+  rx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Depth in bytes of the RX FIFO
+
+  tx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Depth in bytes of the TX FIFO
+
+  altr,has-supplementary-unicast:
+    type: boolean
+    description:
+      If present, TSE supports additional unicast addresses.
+
+  altr,has-hash-multicast-filter:
+    type: boolean
+    description:
+      If present, TSE supports hash based multicast filter.
+
+  mdio:
+    $ref: mdio.yaml#
+    unevaluatedProperties: false
+    description:
+      Creates and registers an MDIO bus.
+
+    properties:
+      compatible:
+        const: altr,tse-mdio
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - rx-fifo-depth
+  - tx-fifo-depth
+
+allOf:
+  - $ref: "ethernet-controller.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - const: altr,tse-1.0
+              - const: ALTR,tse-1.0
+    then:
+      properties:
+        reg:
+          minItems: 4
+        reg-names:
+          items:
+            - const: control_port
+            - const: rx_csr
+            - const: tx_csr
+            - const: s1
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - altr,tse-msgdma-1.0
+    then:
+      properties:
+        reg:
+          minItems: 6
+        reg-names:
+          items:
+            - const: control_port
+            - const: rx_csr
+            - const: rx_desc
+            - const: rx_resp
+            - const: tx_csr
+            - const: tx_desc
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    tse_sub_1_eth_tse_0: ethernet@1,00001000 {
+        compatible = "altr,tse-msgdma-1.0";
+        reg = <0x00001000 0x00000400>,
+              <0x00001460 0x00000020>,
+              <0x00001480 0x00000020>,
+              <0x000014A0 0x00000008>,
+              <0x00001400 0x00000020>,
+              <0x00001420 0x00000020>;
+        reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc";
+        interrupt-parent = <&hps_0_arm_gic_0>;
+        interrupts = <0 43 4>, <0 42 4>;
+        interrupt-names = "rx_irq", "tx_irq";
+        rx-fifo-depth = <2048>;
+        tx-fifo-depth = <2048>;
+        max-frame-size = <1500>;
+        local-mac-address = [ 00 00 00 00 00 00 ];
+        phy-mode = "gmii";
+        altr,has-supplementary-unicast;
+        altr,has-hash-multicast-filter;
+        phy-handle = <&phy1>;
+        mdio {
+            compatible = "altr,tse-mdio";
+            #address-cells = <1>;
+            #size-cells = <0>;
+            phy1: ethernet-phy@1 {
+                reg = <0x1>;
+            };
+        };
+    };
+
+...
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v3 2/5] net: altera: tse: cosmetic change to use reverse xmas tree ordering
  2022-09-01 14:35 [PATCH net-next v3 0/5] net: altera: tse: phylink conversion Maxime Chevallier
  2022-09-01 14:35 ` [PATCH net-next v3 1/5] dt-bindings: net: Convert Altera TSE bindings to yaml Maxime Chevallier
@ 2022-09-01 14:35 ` Maxime Chevallier
  2022-09-01 14:35 ` [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS Maxime Chevallier
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Maxime Chevallier @ 2022-09-01 14:35 UTC (permalink / raw)
  To: davem, Rob Herring
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Florian Fainelli, Heiner Kallweit, Russell King,
	linux-arm-kernel, Krzysztof Kozlowski, devicetree

This commit is a strictly cosmetic change, to use the reverse xmas tree
variable declaration ordering, and introduces no functional changes.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V2->V3 : No changes
V1->V2 : No changes

 .../net/ethernet/altera/altera_tse_ethtool.c  |  2 +-
 drivers/net/ethernet/altera/altera_tse_main.c | 43 ++++++++++---------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/altera/altera_tse_ethtool.c b/drivers/net/ethernet/altera/altera_tse_ethtool.c
index 3081e5874ac5..f0b11a278644 100644
--- a/drivers/net/ethernet/altera/altera_tse_ethtool.c
+++ b/drivers/net/ethernet/altera/altera_tse_ethtool.c
@@ -199,9 +199,9 @@ static int tse_reglen(struct net_device *dev)
 static void tse_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 			 void *regbuf)
 {
-	int i;
 	struct altera_tse_private *priv = netdev_priv(dev);
 	u32 *buf = regbuf;
+	int i;
 
 	/* Set version to a known value, so ethtool knows
 	 * how to do any special formatting of this data.
diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 8c5828582c21..930afc9ec833 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -141,10 +141,10 @@ static int altera_tse_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 static int altera_tse_mdio_create(struct net_device *dev, unsigned int id)
 {
 	struct altera_tse_private *priv = netdev_priv(dev);
-	int ret;
 	struct device_node *mdio_node = NULL;
-	struct mii_bus *mdio = NULL;
 	struct device_node *child_node = NULL;
+	struct mii_bus *mdio = NULL;
+	int ret;
 
 	for_each_child_of_node(priv->device->of_node, child_node) {
 		if (of_device_is_compatible(child_node, "altr,tse-mdio")) {
@@ -236,8 +236,8 @@ static int tse_init_rx_buffer(struct altera_tse_private *priv,
 static void tse_free_rx_buffer(struct altera_tse_private *priv,
 			       struct tse_buffer *rxbuffer)
 {
-	struct sk_buff *skb = rxbuffer->skb;
 	dma_addr_t dma_addr = rxbuffer->dma_addr;
+	struct sk_buff *skb = rxbuffer->skb;
 
 	if (skb != NULL) {
 		if (dma_addr)
@@ -358,6 +358,7 @@ static inline void tse_rx_vlan(struct net_device *dev, struct sk_buff *skb)
 {
 	struct ethhdr *eth_hdr;
 	u16 vid;
+
 	if ((dev->features & NETIF_F_HW_VLAN_CTAG_RX) &&
 	    !__vlan_get_tag(skb, &vid)) {
 		eth_hdr = (struct ethhdr *)skb->data;
@@ -371,10 +372,10 @@ static inline void tse_rx_vlan(struct net_device *dev, struct sk_buff *skb)
  */
 static int tse_rx(struct altera_tse_private *priv, int limit)
 {
-	unsigned int count = 0;
+	unsigned int entry = priv->rx_cons % priv->rx_ring_size;
 	unsigned int next_entry;
+	unsigned int count = 0;
 	struct sk_buff *skb;
-	unsigned int entry = priv->rx_cons % priv->rx_ring_size;
 	u32 rxstatus;
 	u16 pktlength;
 	u16 pktstatus;
@@ -448,10 +449,10 @@ static int tse_rx(struct altera_tse_private *priv, int limit)
 static int tse_tx_complete(struct altera_tse_private *priv)
 {
 	unsigned int txsize = priv->tx_ring_size;
-	u32 ready;
-	unsigned int entry;
 	struct tse_buffer *tx_buff;
+	unsigned int entry;
 	int txcomplete = 0;
+	u32 ready;
 
 	spin_lock(&priv->tx_lock);
 
@@ -497,8 +498,8 @@ static int tse_poll(struct napi_struct *napi, int budget)
 {
 	struct altera_tse_private *priv =
 			container_of(napi, struct altera_tse_private, napi);
-	int rxcomplete = 0;
 	unsigned long int flags;
+	int rxcomplete = 0;
 
 	tse_tx_complete(priv);
 
@@ -561,13 +562,13 @@ static irqreturn_t altera_isr(int irq, void *dev_id)
 static netdev_tx_t tse_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct altera_tse_private *priv = netdev_priv(dev);
+	unsigned int nopaged_len = skb_headlen(skb);
 	unsigned int txsize = priv->tx_ring_size;
-	unsigned int entry;
-	struct tse_buffer *buffer = NULL;
 	int nfrags = skb_shinfo(skb)->nr_frags;
-	unsigned int nopaged_len = skb_headlen(skb);
+	struct tse_buffer *buffer = NULL;
 	netdev_tx_t ret = NETDEV_TX_OK;
 	dma_addr_t dma_addr;
+	unsigned int entry;
 
 	spin_lock_bh(&priv->tx_lock);
 
@@ -696,8 +697,8 @@ static void altera_tse_adjust_link(struct net_device *dev)
 static struct phy_device *connect_local_phy(struct net_device *dev)
 {
 	struct altera_tse_private *priv = netdev_priv(dev);
-	struct phy_device *phydev = NULL;
 	char phy_id_fmt[MII_BUS_ID_SIZE + 3];
+	struct phy_device *phydev = NULL;
 
 	if (priv->phy_addr != POLL_PHY) {
 		snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT,
@@ -773,8 +774,8 @@ static int altera_tse_phy_get_addr_mdio_create(struct net_device *dev)
 static int init_phy(struct net_device *dev)
 {
 	struct altera_tse_private *priv = netdev_priv(dev);
-	struct phy_device *phydev;
 	struct device_node *phynode;
+	struct phy_device *phydev;
 	bool fixed_link = false;
 	int rc = 0;
 
@@ -1012,8 +1013,8 @@ static int tse_change_mtu(struct net_device *dev, int new_mtu)
 static void altera_tse_set_mcfilter(struct net_device *dev)
 {
 	struct altera_tse_private *priv = netdev_priv(dev);
-	int i;
 	struct netdev_hw_addr *ha;
+	int i;
 
 	/* clear the hash filter */
 	for (i = 0; i < 64; i++)
@@ -1152,9 +1153,9 @@ static int init_sgmii_pcs(struct net_device *dev)
 static int tse_open(struct net_device *dev)
 {
 	struct altera_tse_private *priv = netdev_priv(dev);
+	unsigned long flags;
 	int ret = 0;
 	int i;
-	unsigned long int flags;
 
 	/* Reset and configure TSE MAC and probe associated PHY */
 	ret = priv->dmaops->init_dma(priv);
@@ -1265,8 +1266,8 @@ static int tse_open(struct net_device *dev)
 static int tse_shutdown(struct net_device *dev)
 {
 	struct altera_tse_private *priv = netdev_priv(dev);
-	int ret;
 	unsigned long int flags;
+	int ret;
 
 	/* Stop the PHY */
 	if (dev->phydev)
@@ -1320,8 +1321,8 @@ static struct net_device_ops altera_tse_netdev_ops = {
 static int request_and_map(struct platform_device *pdev, const char *name,
 			   struct resource **res, void __iomem **ptr)
 {
-	struct resource *region;
 	struct device *device = &pdev->dev;
+	struct resource *region;
 
 	*res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
 	if (*res == NULL) {
@@ -1350,13 +1351,13 @@ static int request_and_map(struct platform_device *pdev, const char *name,
  */
 static int altera_tse_probe(struct platform_device *pdev)
 {
-	struct net_device *ndev;
-	int ret = -ENODEV;
+	const struct of_device_id *of_id = NULL;
+	struct altera_tse_private *priv;
 	struct resource *control_port;
 	struct resource *dma_res;
-	struct altera_tse_private *priv;
+	struct net_device *ndev;
 	void __iomem *descmap;
-	const struct of_device_id *of_id = NULL;
+	int ret = -ENODEV;
 
 	ndev = alloc_etherdev(sizeof(struct altera_tse_private));
 	if (!ndev) {
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS
  2022-09-01 14:35 [PATCH net-next v3 0/5] net: altera: tse: phylink conversion Maxime Chevallier
  2022-09-01 14:35 ` [PATCH net-next v3 1/5] dt-bindings: net: Convert Altera TSE bindings to yaml Maxime Chevallier
  2022-09-01 14:35 ` [PATCH net-next v3 2/5] net: altera: tse: cosmetic change to use reverse xmas tree ordering Maxime Chevallier
@ 2022-09-01 14:35 ` Maxime Chevallier
  2022-10-09  5:38   ` Sean Anderson
  2022-09-01 14:35 ` [PATCH net-next v3 4/5] net: altera: tse: convert to phylink Maxime Chevallier
  2022-09-01 14:35 ` [PATCH net-next v3 5/5] dt-bindings: net: altera: tse: add an optional pcs register range Maxime Chevallier
  4 siblings, 1 reply; 12+ messages in thread
From: Maxime Chevallier @ 2022-09-01 14:35 UTC (permalink / raw)
  To: davem, Rob Herring
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Florian Fainelli, Heiner Kallweit, Russell King,
	linux-arm-kernel, Krzysztof Kozlowski, devicetree

The Altera Triple Speed Ethernet has a SGMII/1000BaseC PCS that can be
integrated in several ways. It can either be part of the TSE MAC's
address space, accessed through 32 bits accesses on the mapped mdio
device 0, or through a dedicated 16 bits register set.

This driver allows using the TSE PCS outside of altera TSE's driver,
since it can be used standalone by other MACs.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V2->V3 :
  - No changes
V1->V2 :
  - Added a pcs_validate() callback to filter interface modes
  - Added comments on the need for a soft reset at an_restart

 MAINTAINERS                      |   7 ++
 drivers/net/pcs/Kconfig          |   6 ++
 drivers/net/pcs/Makefile         |   1 +
 drivers/net/pcs/pcs-altera-tse.c | 171 +++++++++++++++++++++++++++++++
 include/linux/pcs-altera-tse.h   |  17 +++
 5 files changed, 202 insertions(+)
 create mode 100644 drivers/net/pcs/pcs-altera-tse.c
 create mode 100644 include/linux/pcs-altera-tse.h

diff --git a/MAINTAINERS b/MAINTAINERS
index fe484e7d36dc..576c01576a5f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -878,6 +878,13 @@ L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/ethernet/altera/
 
+ALTERA TSE PCS
+M:	Maxime Chevallier <maxime.chevallier@bootlin.com>
+L:	netdev@vger.kernel.org
+S:	Supported
+F:	drivers/net/pcs/pcs-altera-tse.c
+F:	include/linux/pcs-altera-tse.h
+
 ALTERA UART/JTAG UART SERIAL DRIVERS
 M:	Tobias Klauser <tklauser@distanz.ch>
 L:	linux-serial@vger.kernel.org
diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index 6289b7c765f1..6e7e6c346a3e 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -26,4 +26,10 @@ config PCS_RZN1_MIIC
 	  on RZ/N1 SoCs. This PCS converts MII to RMII/RGMII or can be set in
 	  pass-through mode for MII.
 
+config PCS_ALTERA_TSE
+	tristate
+	help
+	  This module provides helper functions for the Altera Triple Speed
+	  Ethernet SGMII PCS, that can be found on the Intel Socfpga family.
+
 endmenu
diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
index 0ff5388fcdea..4c780d8f2e98 100644
--- a/drivers/net/pcs/Makefile
+++ b/drivers/net/pcs/Makefile
@@ -6,3 +6,4 @@ pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-nxp.o
 obj-$(CONFIG_PCS_XPCS)		+= pcs_xpcs.o
 obj-$(CONFIG_PCS_LYNX)		+= pcs-lynx.o
 obj-$(CONFIG_PCS_RZN1_MIIC)	+= pcs-rzn1-miic.o
+obj-$(CONFIG_PCS_ALTERA_TSE)	+= pcs-altera-tse.o
diff --git a/drivers/net/pcs/pcs-altera-tse.c b/drivers/net/pcs/pcs-altera-tse.c
new file mode 100644
index 000000000000..ae7688ffc4d7
--- /dev/null
+++ b/drivers/net/pcs/pcs-altera-tse.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Bootlin
+ *
+ * Maxime Chevallier <maxime.chevallier@bootlin.com>
+ */
+
+#include <linux/netdevice.h>
+#include <linux/phy.h>
+#include <linux/phylink.h>
+#include <linux/pcs-altera-tse.h>
+
+/* SGMII PCS register addresses
+ */
+#define SGMII_PCS_SCRATCH	0x10
+#define SGMII_PCS_REV		0x11
+#define SGMII_PCS_LINK_TIMER_0	0x12
+#define   SGMII_PCS_LINK_TIMER_REG(x)		(0x12 + (x))
+#define SGMII_PCS_LINK_TIMER_1	0x13
+#define SGMII_PCS_IF_MODE	0x14
+#define   PCS_IF_MODE_SGMII_ENA		BIT(0)
+#define   PCS_IF_MODE_USE_SGMII_AN	BIT(1)
+#define   PCS_IF_MODE_SGMI_SPEED_MASK	GENMASK(3, 2)
+#define   PCS_IF_MODE_SGMI_SPEED_10	(0 << 2)
+#define   PCS_IF_MODE_SGMI_SPEED_100	(1 << 2)
+#define   PCS_IF_MODE_SGMI_SPEED_1000	(2 << 2)
+#define   PCS_IF_MODE_SGMI_HALF_DUPLEX	BIT(4)
+#define   PCS_IF_MODE_SGMI_PHY_AN	BIT(5)
+#define SGMII_PCS_DIS_READ_TO	0x15
+#define SGMII_PCS_READ_TO	0x16
+#define SGMII_PCS_SW_RESET_TIMEOUT 100 /* usecs */
+
+struct altera_tse_pcs {
+	struct phylink_pcs pcs;
+	void __iomem *base;
+	int reg_width;
+};
+
+static struct altera_tse_pcs *phylink_pcs_to_tse_pcs(struct phylink_pcs *pcs)
+{
+	return container_of(pcs, struct altera_tse_pcs, pcs);
+}
+
+static u16 tse_pcs_read(struct altera_tse_pcs *tse_pcs, int regnum)
+{
+	if (tse_pcs->reg_width == 4)
+		return readl(tse_pcs->base + regnum * 4);
+	else
+		return readw(tse_pcs->base + regnum * 2);
+}
+
+static void tse_pcs_write(struct altera_tse_pcs *tse_pcs, int regnum,
+			  u16 value)
+{
+	if (tse_pcs->reg_width == 4)
+		writel(value, tse_pcs->base + regnum * 4);
+	else
+		writew(value, tse_pcs->base + regnum * 2);
+}
+
+static int tse_pcs_reset(struct altera_tse_pcs *tse_pcs)
+{
+	int i = 0;
+	u16 bmcr;
+
+	/* Reset PCS block */
+	bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
+	bmcr |= BMCR_RESET;
+	tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
+
+	for (i = 0; i < SGMII_PCS_SW_RESET_TIMEOUT; i++) {
+		if (!(tse_pcs_read(tse_pcs, MII_BMCR) & BMCR_RESET))
+			return 0;
+		udelay(1);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int alt_tse_pcs_validate(struct phylink_pcs *pcs,
+				unsigned long *supported,
+				const struct phylink_link_state *state)
+{
+	if (state->interface == PHY_INTERFACE_MODE_SGMII ||
+	    state->interface == PHY_INTERFACE_MODE_1000BASEX)
+		return 1;
+
+	return -EINVAL;
+}
+
+static int alt_tse_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
+			      phy_interface_t interface,
+			      const unsigned long *advertising,
+			      bool permit_pause_to_mac)
+{
+	struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs);
+	u32 ctrl, if_mode;
+
+	ctrl = tse_pcs_read(tse_pcs, MII_BMCR);
+	if_mode = tse_pcs_read(tse_pcs, SGMII_PCS_IF_MODE);
+
+	/* Set link timer to 1.6ms, as per the MegaCore Function User Guide */
+	tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_0, 0x0D40);
+	tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_1, 0x03);
+
+	if (interface == PHY_INTERFACE_MODE_SGMII) {
+		if_mode |= PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA;
+	} else if (interface == PHY_INTERFACE_MODE_1000BASEX) {
+		if_mode &= ~(PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA);
+		if_mode |= PCS_IF_MODE_SGMI_SPEED_1000;
+	}
+
+	ctrl |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE);
+
+	tse_pcs_write(tse_pcs, MII_BMCR, ctrl);
+	tse_pcs_write(tse_pcs, SGMII_PCS_IF_MODE, if_mode);
+
+	return tse_pcs_reset(tse_pcs);
+}
+
+static void alt_tse_pcs_get_state(struct phylink_pcs *pcs,
+				  struct phylink_link_state *state)
+{
+	struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs);
+	u16 bmsr, lpa;
+
+	bmsr = tse_pcs_read(tse_pcs, MII_BMSR);
+	lpa = tse_pcs_read(tse_pcs, MII_LPA);
+
+	phylink_mii_c22_pcs_decode_state(state, bmsr, lpa);
+}
+
+static void alt_tse_pcs_an_restart(struct phylink_pcs *pcs)
+{
+	struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs);
+	u16 bmcr;
+
+	bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
+	bmcr |= BMCR_ANRESTART;
+	tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
+
+	/* This PCS seems to require a soft reset to re-sync the AN logic */
+	tse_pcs_reset(tse_pcs);
+}
+
+static const struct phylink_pcs_ops alt_tse_pcs_ops = {
+	.pcs_validate = alt_tse_pcs_validate,
+	.pcs_get_state = alt_tse_pcs_get_state,
+	.pcs_config = alt_tse_pcs_config,
+	.pcs_an_restart = alt_tse_pcs_an_restart,
+};
+
+struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev,
+				       void __iomem *pcs_base, int reg_width)
+{
+	struct altera_tse_pcs *tse_pcs;
+
+	if (reg_width != 4 && reg_width != 2)
+		return ERR_PTR(-EINVAL);
+
+	tse_pcs = devm_kzalloc(&ndev->dev, sizeof(*tse_pcs), GFP_KERNEL);
+	if (!tse_pcs)
+		return ERR_PTR(-ENOMEM);
+
+	tse_pcs->pcs.ops = &alt_tse_pcs_ops;
+	tse_pcs->base = pcs_base;
+	tse_pcs->reg_width = reg_width;
+
+	return &tse_pcs->pcs;
+}
+EXPORT_SYMBOL_GPL(alt_tse_pcs_create);
diff --git a/include/linux/pcs-altera-tse.h b/include/linux/pcs-altera-tse.h
new file mode 100644
index 000000000000..92ab9f08e835
--- /dev/null
+++ b/include/linux/pcs-altera-tse.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 Bootlin
+ *
+ * Maxime Chevallier <maxime.chevallier@bootlin.com>
+ */
+
+#ifndef __LINUX_PCS_ALTERA_TSE_H
+#define __LINUX_PCS_ALTERA_TSE_H
+
+struct phylink_pcs;
+struct net_device;
+
+struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev,
+				       void __iomem *pcs_base, int reg_width);
+
+#endif /* __LINUX_PCS_ALTERA_TSE_H */
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v3 4/5] net: altera: tse: convert to phylink
  2022-09-01 14:35 [PATCH net-next v3 0/5] net: altera: tse: phylink conversion Maxime Chevallier
                   ` (2 preceding siblings ...)
  2022-09-01 14:35 ` [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS Maxime Chevallier
@ 2022-09-01 14:35 ` Maxime Chevallier
  2022-09-02  4:10   ` Jakub Kicinski
  2022-09-01 14:35 ` [PATCH net-next v3 5/5] dt-bindings: net: altera: tse: add an optional pcs register range Maxime Chevallier
  4 siblings, 1 reply; 12+ messages in thread
From: Maxime Chevallier @ 2022-09-01 14:35 UTC (permalink / raw)
  To: davem, Rob Herring
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Florian Fainelli, Heiner Kallweit, Russell King,
	linux-arm-kernel, Krzysztof Kozlowski, devicetree

This commit converts the Altera Triple Speed Ethernet Controller to
phylink. This controller supports MII, GMII and RGMII with its MAC, and
SGMII + 1000BaseX through a small embedded PCS.

The PCS itself has a register set very similar to what is found in a
typical 802.3 ethernet PHY, but this register set memory-mapped instead
of lying on an mdio bus.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V2->V3 : No Changes
V1->V2 : Removed stray DT makefile patching that had nothing to do with
this patch

 drivers/net/ethernet/altera/Kconfig           |   2 +
 drivers/net/ethernet/altera/altera_tse.h      |  19 +-
 .../net/ethernet/altera/altera_tse_ethtool.c  |  20 +-
 drivers/net/ethernet/altera/altera_tse_main.c | 414 +++++-------------
 4 files changed, 141 insertions(+), 314 deletions(-)

diff --git a/drivers/net/ethernet/altera/Kconfig b/drivers/net/ethernet/altera/Kconfig
index 914e56b91467..dd7fd41ccde5 100644
--- a/drivers/net/ethernet/altera/Kconfig
+++ b/drivers/net/ethernet/altera/Kconfig
@@ -3,6 +3,8 @@ config ALTERA_TSE
 	tristate "Altera Triple-Speed Ethernet MAC support"
 	depends on HAS_DMA
 	select PHYLIB
+	select PHYLINK
+	select PCS_ALTERA_TSE
 	help
 	  This driver supports the Altera Triple-Speed (TSE) Ethernet MAC.
 
diff --git a/drivers/net/ethernet/altera/altera_tse.h b/drivers/net/ethernet/altera/altera_tse.h
index f17acfb579a0..db5eed06e92d 100644
--- a/drivers/net/ethernet/altera/altera_tse.h
+++ b/drivers/net/ethernet/altera/altera_tse.h
@@ -27,6 +27,7 @@
 #include <linux/list.h>
 #include <linux/netdevice.h>
 #include <linux/phy.h>
+#include <linux/phylink.h>
 
 #define ALTERA_TSE_SW_RESET_WATCHDOG_CNTR	10000
 #define ALTERA_TSE_MAC_FIFO_WIDTH		4	/* TX/RX FIFO width in
@@ -109,17 +110,6 @@
 #define MAC_CMDCFG_DISABLE_READ_TIMEOUT_GET(v)	GET_BIT_VALUE(v, 27)
 #define MAC_CMDCFG_CNT_RESET_GET(v)		GET_BIT_VALUE(v, 31)
 
-/* SGMII PCS register addresses
- */
-#define SGMII_PCS_SCRATCH	0x10
-#define SGMII_PCS_REV		0x11
-#define SGMII_PCS_LINK_TIMER_0	0x12
-#define SGMII_PCS_LINK_TIMER_1	0x13
-#define SGMII_PCS_IF_MODE	0x14
-#define SGMII_PCS_DIS_READ_TO	0x15
-#define SGMII_PCS_READ_TO	0x16
-#define SGMII_PCS_SW_RESET_TIMEOUT 100 /* usecs */
-
 /* MDIO registers within MAC register Space
  */
 struct altera_tse_mdio {
@@ -423,6 +413,9 @@ struct altera_tse_private {
 	void __iomem *tx_dma_csr;
 	void __iomem *tx_dma_desc;
 
+	/* SGMII PCS address space */
+	void __iomem *pcs_base;
+
 	/* Rx buffers queue */
 	struct tse_buffer *rx_ring;
 	u32 rx_cons;
@@ -480,6 +473,10 @@ struct altera_tse_private {
 	u32 msg_enable;
 
 	struct altera_dmaops *dmaops;
+
+	struct phylink *phylink;
+	struct phylink_config phylink_config;
+	struct phylink_pcs *pcs;
 };
 
 /* Function prototypes
diff --git a/drivers/net/ethernet/altera/altera_tse_ethtool.c b/drivers/net/ethernet/altera/altera_tse_ethtool.c
index f0b11a278644..81313c85833e 100644
--- a/drivers/net/ethernet/altera/altera_tse_ethtool.c
+++ b/drivers/net/ethernet/altera/altera_tse_ethtool.c
@@ -221,6 +221,22 @@ static void tse_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 		buf[i] = csrrd32(priv->mac_dev, i * 4);
 }
 
+static int tse_ethtool_set_link_ksettings(struct net_device *dev,
+					  const struct ethtool_link_ksettings *cmd)
+{
+	struct altera_tse_private *priv = netdev_priv(dev);
+
+	return phylink_ethtool_ksettings_set(priv->phylink, cmd);
+}
+
+static int tse_ethtool_get_link_ksettings(struct net_device *dev,
+					  struct ethtool_link_ksettings *cmd)
+{
+	struct altera_tse_private *priv = netdev_priv(dev);
+
+	return phylink_ethtool_ksettings_get(priv->phylink, cmd);
+}
+
 static const struct ethtool_ops tse_ethtool_ops = {
 	.get_drvinfo = tse_get_drvinfo,
 	.get_regs_len = tse_reglen,
@@ -231,8 +247,8 @@ static const struct ethtool_ops tse_ethtool_ops = {
 	.get_ethtool_stats = tse_fill_stats,
 	.get_msglevel = tse_get_msglevel,
 	.set_msglevel = tse_set_msglevel,
-	.get_link_ksettings = phy_ethtool_get_link_ksettings,
-	.set_link_ksettings = phy_ethtool_set_link_ksettings,
+	.get_link_ksettings = tse_ethtool_get_link_ksettings,
+	.set_link_ksettings = tse_ethtool_set_link_ksettings,
 	.get_ts_info = ethtool_op_get_ts_info,
 };
 
diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 930afc9ec833..89ae6d1623aa 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -32,6 +32,7 @@
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include <linux/of_platform.h>
+#include <linux/pcs-altera-tse.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 #include <linux/skbuff.h>
@@ -86,27 +87,6 @@ static inline u32 tse_tx_avail(struct altera_tse_private *priv)
 	return priv->tx_cons + priv->tx_ring_size - priv->tx_prod - 1;
 }
 
-/* PCS Register read/write functions
- */
-static u16 sgmii_pcs_read(struct altera_tse_private *priv, int regnum)
-{
-	return csrrd32(priv->mac_dev,
-		       tse_csroffs(mdio_phy0) + regnum * 4) & 0xffff;
-}
-
-static void sgmii_pcs_write(struct altera_tse_private *priv, int regnum,
-				u16 value)
-{
-	csrwr32(value, priv->mac_dev, tse_csroffs(mdio_phy0) + regnum * 4);
-}
-
-/* Check PCS scratch memory */
-static int sgmii_pcs_scratch_test(struct altera_tse_private *priv, u16 value)
-{
-	sgmii_pcs_write(priv, SGMII_PCS_SCRATCH, value);
-	return (sgmii_pcs_read(priv, SGMII_PCS_SCRATCH) == value);
-}
-
 /* MDIO specific functions
  */
 static int altera_tse_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
@@ -620,117 +600,6 @@ static netdev_tx_t tse_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return ret;
 }
 
-/* Called every time the controller might need to be made
- * aware of new link state.  The PHY code conveys this
- * information through variables in the phydev structure, and this
- * function converts those variables into the appropriate
- * register values, and can bring down the device if needed.
- */
-static void altera_tse_adjust_link(struct net_device *dev)
-{
-	struct altera_tse_private *priv = netdev_priv(dev);
-	struct phy_device *phydev = dev->phydev;
-	int new_state = 0;
-
-	/* only change config if there is a link */
-	spin_lock(&priv->mac_cfg_lock);
-	if (phydev->link) {
-		/* Read old config */
-		u32 cfg_reg = ioread32(&priv->mac_dev->command_config);
-
-		/* Check duplex */
-		if (phydev->duplex != priv->oldduplex) {
-			new_state = 1;
-			if (!(phydev->duplex))
-				cfg_reg |= MAC_CMDCFG_HD_ENA;
-			else
-				cfg_reg &= ~MAC_CMDCFG_HD_ENA;
-
-			netdev_dbg(priv->dev, "%s: Link duplex = 0x%x\n",
-				   dev->name, phydev->duplex);
-
-			priv->oldduplex = phydev->duplex;
-		}
-
-		/* Check speed */
-		if (phydev->speed != priv->oldspeed) {
-			new_state = 1;
-			switch (phydev->speed) {
-			case 1000:
-				cfg_reg |= MAC_CMDCFG_ETH_SPEED;
-				cfg_reg &= ~MAC_CMDCFG_ENA_10;
-				break;
-			case 100:
-				cfg_reg &= ~MAC_CMDCFG_ETH_SPEED;
-				cfg_reg &= ~MAC_CMDCFG_ENA_10;
-				break;
-			case 10:
-				cfg_reg &= ~MAC_CMDCFG_ETH_SPEED;
-				cfg_reg |= MAC_CMDCFG_ENA_10;
-				break;
-			default:
-				if (netif_msg_link(priv))
-					netdev_warn(dev, "Speed (%d) is not 10/100/1000!\n",
-						    phydev->speed);
-				break;
-			}
-			priv->oldspeed = phydev->speed;
-		}
-		iowrite32(cfg_reg, &priv->mac_dev->command_config);
-
-		if (!priv->oldlink) {
-			new_state = 1;
-			priv->oldlink = 1;
-		}
-	} else if (priv->oldlink) {
-		new_state = 1;
-		priv->oldlink = 0;
-		priv->oldspeed = 0;
-		priv->oldduplex = -1;
-	}
-
-	if (new_state && netif_msg_link(priv))
-		phy_print_status(phydev);
-
-	spin_unlock(&priv->mac_cfg_lock);
-}
-static struct phy_device *connect_local_phy(struct net_device *dev)
-{
-	struct altera_tse_private *priv = netdev_priv(dev);
-	char phy_id_fmt[MII_BUS_ID_SIZE + 3];
-	struct phy_device *phydev = NULL;
-
-	if (priv->phy_addr != POLL_PHY) {
-		snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT,
-			 priv->mdio->id, priv->phy_addr);
-
-		netdev_dbg(dev, "trying to attach to %s\n", phy_id_fmt);
-
-		phydev = phy_connect(dev, phy_id_fmt, &altera_tse_adjust_link,
-				     priv->phy_iface);
-		if (IS_ERR(phydev)) {
-			netdev_err(dev, "Could not attach to PHY\n");
-			phydev = NULL;
-		}
-
-	} else {
-		int ret;
-		phydev = phy_find_first(priv->mdio);
-		if (phydev == NULL) {
-			netdev_err(dev, "No PHY found\n");
-			return phydev;
-		}
-
-		ret = phy_connect_direct(dev, phydev, &altera_tse_adjust_link,
-				priv->phy_iface);
-		if (ret != 0) {
-			netdev_err(dev, "Could not attach to PHY\n");
-			phydev = NULL;
-		}
-	}
-	return phydev;
-}
-
 static int altera_tse_phy_get_addr_mdio_create(struct net_device *dev)
 {
 	struct altera_tse_private *priv = netdev_priv(dev);
@@ -769,91 +638,6 @@ static int altera_tse_phy_get_addr_mdio_create(struct net_device *dev)
 	return 0;
 }
 
-/* Initialize driver's PHY state, and attach to the PHY
- */
-static int init_phy(struct net_device *dev)
-{
-	struct altera_tse_private *priv = netdev_priv(dev);
-	struct device_node *phynode;
-	struct phy_device *phydev;
-	bool fixed_link = false;
-	int rc = 0;
-
-	/* Avoid init phy in case of no phy present */
-	if (!priv->phy_iface)
-		return 0;
-
-	priv->oldlink = 0;
-	priv->oldspeed = 0;
-	priv->oldduplex = -1;
-
-	phynode = of_parse_phandle(priv->device->of_node, "phy-handle", 0);
-
-	if (!phynode) {
-		/* check if a fixed-link is defined in device-tree */
-		if (of_phy_is_fixed_link(priv->device->of_node)) {
-			rc = of_phy_register_fixed_link(priv->device->of_node);
-			if (rc < 0) {
-				netdev_err(dev, "cannot register fixed PHY\n");
-				return rc;
-			}
-
-			/* In the case of a fixed PHY, the DT node associated
-			 * to the PHY is the Ethernet MAC DT node.
-			 */
-			phynode = of_node_get(priv->device->of_node);
-			fixed_link = true;
-
-			netdev_dbg(dev, "fixed-link detected\n");
-			phydev = of_phy_connect(dev, phynode,
-						&altera_tse_adjust_link,
-						0, priv->phy_iface);
-		} else {
-			netdev_dbg(dev, "no phy-handle found\n");
-			if (!priv->mdio) {
-				netdev_err(dev, "No phy-handle nor local mdio specified\n");
-				return -ENODEV;
-			}
-			phydev = connect_local_phy(dev);
-		}
-	} else {
-		netdev_dbg(dev, "phy-handle found\n");
-		phydev = of_phy_connect(dev, phynode,
-			&altera_tse_adjust_link, 0, priv->phy_iface);
-	}
-	of_node_put(phynode);
-
-	if (!phydev) {
-		netdev_err(dev, "Could not find the PHY\n");
-		if (fixed_link)
-			of_phy_deregister_fixed_link(priv->device->of_node);
-		return -ENODEV;
-	}
-
-	/* Stop Advertising 1000BASE Capability if interface is not GMII
-	 */
-	if ((priv->phy_iface == PHY_INTERFACE_MODE_MII) ||
-	    (priv->phy_iface == PHY_INTERFACE_MODE_RMII))
-		phy_set_max_speed(phydev, SPEED_100);
-
-	/* Broken HW is sometimes missing the pull-up resistor on the
-	 * MDIO line, which results in reads to non-existent devices returning
-	 * 0 rather than 0xffff. Catch this here and treat 0 as a non-existent
-	 * device as well. If a fixed-link is used the phy_id is always 0.
-	 * Note: phydev->phy_id is the result of reading the UID PHY registers.
-	 */
-	if ((phydev->phy_id == 0) && !fixed_link) {
-		netdev_err(dev, "Bad PHY UID 0x%08x\n", phydev->phy_id);
-		phy_disconnect(phydev);
-		return -ENODEV;
-	}
-
-	netdev_dbg(dev, "attached to PHY %d UID 0x%08x Link = %d\n",
-		   phydev->mdio.addr, phydev->phy_id, phydev->link);
-
-	return 0;
-}
-
 static void tse_update_mac_addr(struct altera_tse_private *priv, const u8 *addr)
 {
 	u32 msb;
@@ -1088,66 +872,6 @@ static void tse_set_rx_mode(struct net_device *dev)
 	spin_unlock(&priv->mac_cfg_lock);
 }
 
-/* Initialise (if necessary) the SGMII PCS component
- */
-static int init_sgmii_pcs(struct net_device *dev)
-{
-	struct altera_tse_private *priv = netdev_priv(dev);
-	int n;
-	unsigned int tmp_reg = 0;
-
-	if (priv->phy_iface != PHY_INTERFACE_MODE_SGMII)
-		return 0; /* Nothing to do, not in SGMII mode */
-
-	/* The TSE SGMII PCS block looks a little like a PHY, it is
-	 * mapped into the zeroth MDIO space of the MAC and it has
-	 * ID registers like a PHY would.  Sadly this is often
-	 * configured to zeroes, so don't be surprised if it does
-	 * show 0x00000000.
-	 */
-
-	if (sgmii_pcs_scratch_test(priv, 0x0000) &&
-		sgmii_pcs_scratch_test(priv, 0xffff) &&
-		sgmii_pcs_scratch_test(priv, 0xa5a5) &&
-		sgmii_pcs_scratch_test(priv, 0x5a5a)) {
-		netdev_info(dev, "PCS PHY ID: 0x%04x%04x\n",
-				sgmii_pcs_read(priv, MII_PHYSID1),
-				sgmii_pcs_read(priv, MII_PHYSID2));
-	} else {
-		netdev_err(dev, "SGMII PCS Scratch memory test failed.\n");
-		return -ENOMEM;
-	}
-
-	/* Starting on page 5-29 of the MegaCore Function User Guide
-	 * Set SGMII Link timer to 1.6ms
-	 */
-	sgmii_pcs_write(priv, SGMII_PCS_LINK_TIMER_0, 0x0D40);
-	sgmii_pcs_write(priv, SGMII_PCS_LINK_TIMER_1, 0x03);
-
-	/* Enable SGMII Interface and Enable SGMII Auto Negotiation */
-	sgmii_pcs_write(priv, SGMII_PCS_IF_MODE, 0x3);
-
-	/* Enable Autonegotiation */
-	tmp_reg = sgmii_pcs_read(priv, MII_BMCR);
-	tmp_reg |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE);
-	sgmii_pcs_write(priv, MII_BMCR, tmp_reg);
-
-	/* Reset PCS block */
-	tmp_reg |= BMCR_RESET;
-	sgmii_pcs_write(priv, MII_BMCR, tmp_reg);
-	for (n = 0; n < SGMII_PCS_SW_RESET_TIMEOUT; n++) {
-		if (!(sgmii_pcs_read(priv, MII_BMCR) & BMCR_RESET)) {
-			netdev_info(dev, "SGMII PCS block initialised OK\n");
-			return 0;
-		}
-		udelay(1);
-	}
-
-	/* We failed to reset the block, return a timeout */
-	netdev_err(dev, "SGMII PCS block reset failed.\n");
-	return -ETIMEDOUT;
-}
-
 /* Open and initialize the interface
  */
 static int tse_open(struct net_device *dev)
@@ -1172,14 +896,6 @@ static int tse_open(struct net_device *dev)
 		netdev_warn(dev, "TSE revision %x\n", priv->revision);
 
 	spin_lock(&priv->mac_cfg_lock);
-	/* no-op if MAC not operating in SGMII mode*/
-	ret = init_sgmii_pcs(dev);
-	if (ret) {
-		netdev_err(dev,
-			   "Cannot init the SGMII PCS (error: %d)\n", ret);
-		spin_unlock(&priv->mac_cfg_lock);
-		goto phy_error;
-	}
 
 	ret = reset_mac(priv);
 	/* Note that reset_mac will fail if the clocks are gated by the PHY
@@ -1237,8 +953,12 @@ static int tse_open(struct net_device *dev)
 
 	spin_unlock_irqrestore(&priv->rxdma_irq_lock, flags);
 
-	if (dev->phydev)
-		phy_start(dev->phydev);
+	ret = phylink_of_phy_connect(priv->phylink, priv->device->of_node, 0);
+	if (ret) {
+		netdev_err(dev, "could not connect phylink (%d)\n", ret);
+		goto tx_request_irq_error;
+	}
+	phylink_start(priv->phylink);
 
 	napi_enable(&priv->napi);
 	netif_start_queue(dev);
@@ -1269,10 +989,7 @@ static int tse_shutdown(struct net_device *dev)
 	unsigned long int flags;
 	int ret;
 
-	/* Stop the PHY */
-	if (dev->phydev)
-		phy_stop(dev->phydev);
-
+	phylink_stop(priv->phylink);
 	netif_stop_queue(dev);
 	napi_disable(&priv->napi);
 
@@ -1318,6 +1035,74 @@ static struct net_device_ops altera_tse_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 };
 
+static void alt_tse_mac_an_restart(struct phylink_config *config)
+{
+}
+
+static void alt_tse_mac_config(struct phylink_config *config, unsigned int mode,
+			       const struct phylink_link_state *state)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct altera_tse_private *priv = netdev_priv(ndev);
+
+	spin_lock(&priv->mac_cfg_lock);
+	reset_mac(priv);
+	tse_set_mac(priv, true);
+	spin_unlock(&priv->mac_cfg_lock);
+}
+
+static void alt_tse_mac_link_down(struct phylink_config *config,
+				  unsigned int mode, phy_interface_t interface)
+{
+}
+
+static void alt_tse_mac_link_up(struct phylink_config *config,
+				struct phy_device *phy, unsigned int mode,
+				phy_interface_t interface, int speed,
+				int duplex, bool tx_pause, bool rx_pause)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct altera_tse_private *priv = netdev_priv(ndev);
+	u32 ctrl;
+
+	ctrl = csrrd32(priv->mac_dev, tse_csroffs(command_config));
+	ctrl &= ~(MAC_CMDCFG_ENA_10 | MAC_CMDCFG_ETH_SPEED | MAC_CMDCFG_HD_ENA);
+
+	if (duplex == DUPLEX_HALF)
+		ctrl |= MAC_CMDCFG_HD_ENA;
+
+	if (speed == SPEED_1000)
+		ctrl |= MAC_CMDCFG_ETH_SPEED;
+	else if (speed == SPEED_10)
+		ctrl |= MAC_CMDCFG_ENA_10;
+
+	spin_lock(&priv->mac_cfg_lock);
+	csrwr32(ctrl, priv->mac_dev, tse_csroffs(command_config));
+	spin_unlock(&priv->mac_cfg_lock);
+}
+
+static struct phylink_pcs *alt_tse_select_pcs(struct phylink_config *config,
+					      phy_interface_t interface)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct altera_tse_private *priv = netdev_priv(ndev);
+
+	if (interface == PHY_INTERFACE_MODE_SGMII ||
+	    interface == PHY_INTERFACE_MODE_1000BASEX)
+		return priv->pcs;
+	else
+		return NULL;
+}
+
+static const struct phylink_mac_ops alt_tse_phylink_ops = {
+	.validate = phylink_generic_validate,
+	.mac_an_restart = alt_tse_mac_an_restart,
+	.mac_config = alt_tse_mac_config,
+	.mac_link_down = alt_tse_mac_link_down,
+	.mac_link_up = alt_tse_mac_link_up,
+	.mac_select_pcs = alt_tse_select_pcs,
+};
+
 static int request_and_map(struct platform_device *pdev, const char *name,
 			   struct resource **res, void __iomem **ptr)
 {
@@ -1355,8 +1140,10 @@ static int altera_tse_probe(struct platform_device *pdev)
 	struct altera_tse_private *priv;
 	struct resource *control_port;
 	struct resource *dma_res;
+	struct resource *pcs_res;
 	struct net_device *ndev;
 	void __iomem *descmap;
+	int pcs_reg_width = 2;
 	int ret = -ENODEV;
 
 	ndev = alloc_etherdev(sizeof(struct altera_tse_private));
@@ -1468,6 +1255,17 @@ static int altera_tse_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_free_netdev;
 
+	/* SGMII PCS address space. The location can vary depending on how the
+	 * IP is integrated. We can have a resource dedicated to it at a specific
+	 * address space, but if it's not the case, we fallback to the mdiophy0
+	 * from the MAC's address space
+	 */
+	ret = request_and_map(pdev, "pcs", &pcs_res,
+			      &priv->pcs_base);
+	if (ret) {
+		priv->pcs_base = priv->mac_dev + tse_csroffs(mdio_phy0);
+		pcs_reg_width = 4;
+	}
 
 	/* Rx IRQ */
 	priv->rx_irq = platform_get_irq_byname(pdev, "rx_irq");
@@ -1591,11 +1389,31 @@ static int altera_tse_probe(struct platform_device *pdev)
 			 (unsigned long) control_port->start, priv->rx_irq,
 			 priv->tx_irq);
 
-	ret = init_phy(ndev);
-	if (ret != 0) {
-		netdev_err(ndev, "Cannot attach to PHY (error: %d)\n", ret);
+	priv->pcs = alt_tse_pcs_create(ndev, priv->pcs_base, pcs_reg_width);
+
+	priv->phylink_config.dev = &ndev->dev;
+	priv->phylink_config.type = PHYLINK_NETDEV;
+	priv->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_10 |
+						MAC_100 | MAC_1000FD;
+
+	phy_interface_set_rgmii(priv->phylink_config.supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_MII,
+		  priv->phylink_config.supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_GMII,
+		  priv->phylink_config.supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_SGMII,
+		  priv->phylink_config.supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_1000BASEX,
+		  priv->phylink_config.supported_interfaces);
+
+	priv->phylink = phylink_create(&priv->phylink_config,
+				       of_fwnode_handle(priv->device->of_node),
+				       priv->phy_iface, &alt_tse_phylink_ops);
+	if (IS_ERR(priv->phylink)) {
+		dev_err(&pdev->dev, "failed to create phylink\n");
 		goto err_init_phy;
 	}
+
 	return 0;
 
 err_init_phy:
@@ -1615,16 +1433,10 @@ static int altera_tse_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct altera_tse_private *priv = netdev_priv(ndev);
 
-	if (ndev->phydev) {
-		phy_disconnect(ndev->phydev);
-
-		if (of_phy_is_fixed_link(priv->device->of_node))
-			of_phy_deregister_fixed_link(priv->device->of_node);
-	}
-
 	platform_set_drvdata(pdev, NULL);
 	altera_tse_mdio_destroy(ndev);
 	unregister_netdev(ndev);
+	phylink_destroy(priv->phylink);
 	free_netdev(ndev);
 
 	return 0;
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v3 5/5] dt-bindings: net: altera: tse: add an optional pcs register range
  2022-09-01 14:35 [PATCH net-next v3 0/5] net: altera: tse: phylink conversion Maxime Chevallier
                   ` (3 preceding siblings ...)
  2022-09-01 14:35 ` [PATCH net-next v3 4/5] net: altera: tse: convert to phylink Maxime Chevallier
@ 2022-09-01 14:35 ` Maxime Chevallier
  4 siblings, 0 replies; 12+ messages in thread
From: Maxime Chevallier @ 2022-09-01 14:35 UTC (permalink / raw)
  To: davem, Rob Herring
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Florian Fainelli, Heiner Kallweit, Russell King,
	linux-arm-kernel, Krzysztof Kozlowski, devicetree

Some implementations of the TSE have their PCS as an external bloc,
exposed at its own register range. Document this, and add a new example
showing a case using the pcs and the new phylink conversion to connect
an sfp port to a TSE mac.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V2->V3 :
 - Fixed construct of reg/reg-names
 - Fixed example to use an all-zero mac-addr
V1->V2 :
 - Fixed example

 .../devicetree/bindings/net/altr,tse.yaml     | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/altr,tse.yaml b/Documentation/devicetree/bindings/net/altr,tse.yaml
index 78c7a2047910..8d1d94494349 100644
--- a/Documentation/devicetree/bindings/net/altr,tse.yaml
+++ b/Documentation/devicetree/bindings/net/altr,tse.yaml
@@ -95,7 +95,9 @@ allOf:
       properties:
         reg:
           minItems: 6
+          maxItems: 7
         reg-names:
+          minItems: 6
           items:
             - const: control_port
             - const: rx_csr
@@ -103,10 +105,35 @@ allOf:
             - const: rx_resp
             - const: tx_csr
             - const: tx_desc
+            - const: pcs
 
 unevaluatedProperties: false
 
 examples:
+  - |
+    tse_sub_0: ethernet@c0100000 {
+        compatible = "altr,tse-msgdma-1.0";
+        reg = <0xc0100000 0x00000400>,
+              <0xc0101000 0x00000020>,
+              <0xc0102000 0x00000020>,
+              <0xc0103000 0x00000008>,
+              <0xc0104000 0x00000020>,
+              <0xc0105000 0x00000020>,
+              <0xc0106000 0x00000100>;
+        reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc", "pcs";
+        interrupt-parent = <&intc>;
+        interrupts = <0 44 4>,<0 45 4>;
+        interrupt-names = "rx_irq","tx_irq";
+        rx-fifo-depth = <2048>;
+        tx-fifo-depth = <2048>;
+        max-frame-size = <1500>;
+        local-mac-address = [ 00 00 00 00 00 00 ];
+        altr,has-supplementary-unicast;
+        altr,has-hash-multicast-filter;
+        sfp = <&sfp0>;
+        phy-mode = "sgmii";
+        managed = "in-band-status";
+    };
   - |
     tse_sub_1_eth_tse_0: ethernet@1,00001000 {
         compatible = "altr,tse-msgdma-1.0";
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v3 4/5] net: altera: tse: convert to phylink
  2022-09-01 14:35 ` [PATCH net-next v3 4/5] net: altera: tse: convert to phylink Maxime Chevallier
@ 2022-09-02  4:10   ` Jakub Kicinski
  2022-09-02  7:57     ` Maxime Chevallier
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2022-09-02  4:10 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: davem, Rob Herring, netdev, linux-kernel, thomas.petazzoni,
	Andrew Lunn, Eric Dumazet, Paolo Abeni, Florian Fainelli,
	Heiner Kallweit, Russell King, linux-arm-kernel,
	Krzysztof Kozlowski, devicetree

On Thu,  1 Sep 2022 16:35:42 +0200 Maxime Chevallier wrote:
> This commit converts the Altera Triple Speed Ethernet Controller to
> phylink. This controller supports MII, GMII and RGMII with its MAC, and
> SGMII + 1000BaseX through a small embedded PCS.
> 
> The PCS itself has a register set very similar to what is found in a
> typical 802.3 ethernet PHY, but this register set memory-mapped instead
> of lying on an mdio bus.

allmodconfig builds report:

ERROR: modpost: missing MODULE_LICENSE() in drivers/net/pcs/pcs-altera-tse.o

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v3 4/5] net: altera: tse: convert to phylink
  2022-09-02  4:10   ` Jakub Kicinski
@ 2022-09-02  7:57     ` Maxime Chevallier
  0 siblings, 0 replies; 12+ messages in thread
From: Maxime Chevallier @ 2022-09-02  7:57 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, Rob Herring, netdev, linux-kernel, thomas.petazzoni,
	Andrew Lunn, Eric Dumazet, Paolo Abeni, Florian Fainelli,
	Heiner Kallweit, Russell King, linux-arm-kernel,
	Krzysztof Kozlowski, devicetree

Hello Jakub,

On Thu, 1 Sep 2022 21:10:21 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Thu,  1 Sep 2022 16:35:42 +0200 Maxime Chevallier wrote:
> > This commit converts the Altera Triple Speed Ethernet Controller to
> > phylink. This controller supports MII, GMII and RGMII with its MAC,
> > and SGMII + 1000BaseX through a small embedded PCS.
> > 
> > The PCS itself has a register set very similar to what is found in a
> > typical 802.3 ethernet PHY, but this register set memory-mapped
> > instead of lying on an mdio bus.  
> 
> allmodconfig builds report:
> 
> ERROR: modpost: missing MODULE_LICENSE() in
> drivers/net/pcs/pcs-altera-tse.o

Ah you're right, I forgot to specify it. I'll address that in the next
version, thanks !

Maxime

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS
  2022-09-01 14:35 ` [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS Maxime Chevallier
@ 2022-10-09  5:38   ` Sean Anderson
  2022-10-26  9:37     ` Maxime Chevallier
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Anderson @ 2022-10-09  5:38 UTC (permalink / raw)
  To: Maxime Chevallier, davem, Rob Herring
  Cc: netdev, linux-kernel, thomas.petazzoni, Andrew Lunn,
	Jakub Kicinski, Eric Dumazet, Paolo Abeni, Florian Fainelli,
	Heiner Kallweit, Russell King, linux-arm-kernel,
	Krzysztof Kozlowski, devicetree

On 9/1/22 10:35, Maxime Chevallier wrote:
> The Altera Triple Speed Ethernet has a SGMII/1000BaseC PCS that can be
> integrated in several ways. It can either be part of the TSE MAC's
> address space, accessed through 32 bits accesses on the mapped mdio
> device 0, or through a dedicated 16 bits register set.
> 
> This driver allows using the TSE PCS outside of altera TSE's driver,
> since it can be used standalone by other MACs.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> V2->V3 :
>    - No changes
> V1->V2 :
>    - Added a pcs_validate() callback to filter interface modes
>    - Added comments on the need for a soft reset at an_restart
> 
>   MAINTAINERS                      |   7 ++
>   drivers/net/pcs/Kconfig          |   6 ++
>   drivers/net/pcs/Makefile         |   1 +
>   drivers/net/pcs/pcs-altera-tse.c | 171 +++++++++++++++++++++++++++++++
>   include/linux/pcs-altera-tse.h   |  17 +++
>   5 files changed, 202 insertions(+)
>   create mode 100644 drivers/net/pcs/pcs-altera-tse.c
>   create mode 100644 include/linux/pcs-altera-tse.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fe484e7d36dc..576c01576a5f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -878,6 +878,13 @@ L:	netdev@vger.kernel.org
>   S:	Maintained
>   F:	drivers/net/ethernet/altera/
>   
> +ALTERA TSE PCS
> +M:	Maxime Chevallier <maxime.chevallier@bootlin.com>
> +L:	netdev@vger.kernel.org
> +S:	Supported
> +F:	drivers/net/pcs/pcs-altera-tse.c
> +F:	include/linux/pcs-altera-tse.h
> +
>   ALTERA UART/JTAG UART SERIAL DRIVERS
>   M:	Tobias Klauser <tklauser@distanz.ch>
>   L:	linux-serial@vger.kernel.org
> diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
> index 6289b7c765f1..6e7e6c346a3e 100644
> --- a/drivers/net/pcs/Kconfig
> +++ b/drivers/net/pcs/Kconfig
> @@ -26,4 +26,10 @@ config PCS_RZN1_MIIC
>   	  on RZ/N1 SoCs. This PCS converts MII to RMII/RGMII or can be set in
>   	  pass-through mode for MII.
>   
> +config PCS_ALTERA_TSE
> +	tristate
> +	help
> +	  This module provides helper functions for the Altera Triple Speed
> +	  Ethernet SGMII PCS, that can be found on the Intel Socfpga family.
> +
>   endmenu
> diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
> index 0ff5388fcdea..4c780d8f2e98 100644
> --- a/drivers/net/pcs/Makefile
> +++ b/drivers/net/pcs/Makefile
> @@ -6,3 +6,4 @@ pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-nxp.o
>   obj-$(CONFIG_PCS_XPCS)		+= pcs_xpcs.o
>   obj-$(CONFIG_PCS_LYNX)		+= pcs-lynx.o
>   obj-$(CONFIG_PCS_RZN1_MIIC)	+= pcs-rzn1-miic.o
> +obj-$(CONFIG_PCS_ALTERA_TSE)	+= pcs-altera-tse.o
> diff --git a/drivers/net/pcs/pcs-altera-tse.c b/drivers/net/pcs/pcs-altera-tse.c
> new file mode 100644
> index 000000000000..ae7688ffc4d7
> --- /dev/null
> +++ b/drivers/net/pcs/pcs-altera-tse.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Bootlin
> + *
> + * Maxime Chevallier <maxime.chevallier@bootlin.com>
> + */
> +
> +#include <linux/netdevice.h>
> +#include <linux/phy.h>
> +#include <linux/phylink.h>
> +#include <linux/pcs-altera-tse.h>
> +
> +/* SGMII PCS register addresses
> + */
> +#define SGMII_PCS_SCRATCH	0x10
> +#define SGMII_PCS_REV		0x11
> +#define SGMII_PCS_LINK_TIMER_0	0x12
> +#define   SGMII_PCS_LINK_TIMER_REG(x)		(0x12 + (x))

Not used.

> +#define SGMII_PCS_LINK_TIMER_1	0x13
> +#define SGMII_PCS_IF_MODE	0x14
> +#define   PCS_IF_MODE_SGMII_ENA		BIT(0)
> +#define   PCS_IF_MODE_USE_SGMII_AN	BIT(1)
> +#define   PCS_IF_MODE_SGMI_SPEED_MASK	GENMASK(3, 2)
> +#define   PCS_IF_MODE_SGMI_SPEED_10	(0 << 2)
> +#define   PCS_IF_MODE_SGMI_SPEED_100	(1 << 2)
> +#define   PCS_IF_MODE_SGMI_SPEED_1000	(2 << 2)

You can use FIELD_PREP if you're so inclined. I assume SGMI is from the datasheet.

> +#define   PCS_IF_MODE_SGMI_HALF_DUPLEX	BIT(4)
> +#define   PCS_IF_MODE_SGMI_PHY_AN	BIT(5)
> +#define SGMII_PCS_DIS_READ_TO	0x15
> +#define SGMII_PCS_READ_TO	0x16
> +#define SGMII_PCS_SW_RESET_TIMEOUT 100 /* usecs */
> +
> +struct altera_tse_pcs {
> +	struct phylink_pcs pcs;
> +	void __iomem *base;
> +	int reg_width;
> +};
> +
> +static struct altera_tse_pcs *phylink_pcs_to_tse_pcs(struct phylink_pcs *pcs)
> +{
> +	return container_of(pcs, struct altera_tse_pcs, pcs);
> +}
> +
> +static u16 tse_pcs_read(struct altera_tse_pcs *tse_pcs, int regnum)
> +{
> +	if (tse_pcs->reg_width == 4)
> +		return readl(tse_pcs->base + regnum * 4);
> +	else
> +		return readw(tse_pcs->base + regnum * 2);
> +}
> +
> +static void tse_pcs_write(struct altera_tse_pcs *tse_pcs, int regnum,
> +			  u16 value)
> +{
> +	if (tse_pcs->reg_width == 4)
> +		writel(value, tse_pcs->base + regnum * 4);
> +	else
> +		writew(value, tse_pcs->base + regnum * 2);
> +}
> +
> +static int tse_pcs_reset(struct altera_tse_pcs *tse_pcs)
> +{
> +	int i = 0;
> +	u16 bmcr;
> +
> +	/* Reset PCS block */
> +	bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
> +	bmcr |= BMCR_RESET;
> +	tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
> +
> +	for (i = 0; i < SGMII_PCS_SW_RESET_TIMEOUT; i++) {
> +		if (!(tse_pcs_read(tse_pcs, MII_BMCR) & BMCR_RESET))
> +			return 0;
> +		udelay(1);
> +	}

read_poll_timeout?

> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int alt_tse_pcs_validate(struct phylink_pcs *pcs,
> +				unsigned long *supported,
> +				const struct phylink_link_state *state)
> +{
> +	if (state->interface == PHY_INTERFACE_MODE_SGMII ||
> +	    state->interface == PHY_INTERFACE_MODE_1000BASEX)
> +		return 1;
> +
> +	return -EINVAL;
> +}
> +
> +static int alt_tse_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
> +			      phy_interface_t interface,
> +			      const unsigned long *advertising,
> +			      bool permit_pause_to_mac)
> +{
> +	struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs);
> +	u32 ctrl, if_mode;
> +
> +	ctrl = tse_pcs_read(tse_pcs, MII_BMCR);
> +	if_mode = tse_pcs_read(tse_pcs, SGMII_PCS_IF_MODE);
> +
> +	/* Set link timer to 1.6ms, as per the MegaCore Function User Guide */
> +	tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_0, 0x0D40);
> +	tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_1, 0x03);

Shouldn't this be different for SGMII vs 1000BASE-X?

> +
> +	if (interface == PHY_INTERFACE_MODE_SGMII) {
> +		if_mode |= PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA;

I think PCS_IF_MODE_USE_SGMII_AN should be cleared if mode=MLO_AN_FIXED.

> +	} else if (interface == PHY_INTERFACE_MODE_1000BASEX) {
> +		if_mode &= ~(PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA);
> +		if_mode |= PCS_IF_MODE_SGMI_SPEED_1000;

I don't think you need to set this for 1000BASE-X.

> +	}
> +
> +	ctrl |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE);

BMCR_FULLDPLX is read-only, so you don't have to set it. Same for the
speed.

> +
> +	tse_pcs_write(tse_pcs, MII_BMCR, ctrl);
> +	tse_pcs_write(tse_pcs, SGMII_PCS_IF_MODE, if_mode);
> +
> +	return tse_pcs_reset(tse_pcs);
> +}
> +
> +static void alt_tse_pcs_get_state(struct phylink_pcs *pcs,
> +				  struct phylink_link_state *state)
> +{
> +	struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs);
> +	u16 bmsr, lpa;
> +
> +	bmsr = tse_pcs_read(tse_pcs, MII_BMSR);
> +	lpa = tse_pcs_read(tse_pcs, MII_LPA);
> +
> +	phylink_mii_c22_pcs_decode_state(state, bmsr, lpa);
> +}
> +
> +static void alt_tse_pcs_an_restart(struct phylink_pcs *pcs)
> +{
> +	struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs);
> +	u16 bmcr;
> +
> +	bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
> +	bmcr |= BMCR_ANRESTART;
> +	tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
> +
> +	/* This PCS seems to require a soft reset to re-sync the AN logic */
> +	tse_pcs_reset(tse_pcs);

This is kinda strange since c22 phys are supposed to reset the other
registers to default values when BMCR_RESET is written. Good thing this
is a PCS...

> +}
> +
> +static const struct phylink_pcs_ops alt_tse_pcs_ops = {
> +	.pcs_validate = alt_tse_pcs_validate,
> +	.pcs_get_state = alt_tse_pcs_get_state,
> +	.pcs_config = alt_tse_pcs_config,
> +	.pcs_an_restart = alt_tse_pcs_an_restart,
> +};

Don't you need link_up to set the speed/duplex for MLO_AN_FIXED?

> +
> +struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev,
> +				       void __iomem *pcs_base, int reg_width)
> +{
> +	struct altera_tse_pcs *tse_pcs;
> +
> +	if (reg_width != 4 && reg_width != 2)
> +		return ERR_PTR(-EINVAL);
> +
> +	tse_pcs = devm_kzalloc(&ndev->dev, sizeof(*tse_pcs), GFP_KERNEL);
> +	if (!tse_pcs)
> +		return ERR_PTR(-ENOMEM);
> +
> +	tse_pcs->pcs.ops = &alt_tse_pcs_ops;
> +	tse_pcs->base = pcs_base;
> +	tse_pcs->reg_width = reg_width;
> +
> +	return &tse_pcs->pcs;
> +}
> +EXPORT_SYMBOL_GPL(alt_tse_pcs_create);
> diff --git a/include/linux/pcs-altera-tse.h b/include/linux/pcs-altera-tse.h
> new file mode 100644
> index 000000000000..92ab9f08e835
> --- /dev/null
> +++ b/include/linux/pcs-altera-tse.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 Bootlin
> + *
> + * Maxime Chevallier <maxime.chevallier@bootlin.com>
> + */
> +
> +#ifndef __LINUX_PCS_ALTERA_TSE_H
> +#define __LINUX_PCS_ALTERA_TSE_H
> +
> +struct phylink_pcs;
> +struct net_device;
> +
> +struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev,
> +				       void __iomem *pcs_base, int reg_width);
> +
> +#endif /* __LINUX_PCS_ALTERA_TSE_H */

--Sean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS
  2022-10-09  5:38   ` Sean Anderson
@ 2022-10-26  9:37     ` Maxime Chevallier
  2022-10-26 12:05       ` Andrew Lunn
  2022-10-26 12:47       ` Russell King (Oracle)
  0 siblings, 2 replies; 12+ messages in thread
From: Maxime Chevallier @ 2022-10-26  9:37 UTC (permalink / raw)
  To: Sean Anderson
  Cc: davem, Rob Herring, netdev, linux-kernel, thomas.petazzoni,
	Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Florian Fainelli, Heiner Kallweit, Russell King,
	linux-arm-kernel, Krzysztof Kozlowski, devicetree

Hello Sean,

On Sun, 9 Oct 2022 01:38:15 -0400
Sean Anderson <seanga2@gmail.com> wrote:


> > +#define   SGMII_PCS_LINK_TIMER_REG(x)		(0x12 + (x))  
> 
> Not used.

Right, I'll remove that in a followup patch

> > +#define SGMII_PCS_LINK_TIMER_1	0x13
> > +#define SGMII_PCS_IF_MODE	0x14
> > +#define   PCS_IF_MODE_SGMII_ENA		BIT(0)
> > +#define   PCS_IF_MODE_USE_SGMII_AN	BIT(1)
> > +#define   PCS_IF_MODE_SGMI_SPEED_MASK	GENMASK(3, 2)
> > +#define   PCS_IF_MODE_SGMI_SPEED_10	(0 << 2)
> > +#define   PCS_IF_MODE_SGMI_SPEED_100	(1 << 2)
> > +#define   PCS_IF_MODE_SGMI_SPEED_1000	(2 << 2)  
> 
> You can use FIELD_PREP if you're so inclined. I assume SGMI is from
> the datasheet.

Will do ! thanks :)

> > +#define   PCS_IF_MODE_SGMI_HALF_DUPLEX	BIT(4)
> > +#define   PCS_IF_MODE_SGMI_PHY_AN	BIT(5)
> > +#define SGMII_PCS_DIS_READ_TO	0x15
> > +#define SGMII_PCS_READ_TO	0x16
> > +#define SGMII_PCS_SW_RESET_TIMEOUT 100 /* usecs */
> > +
> > +struct altera_tse_pcs {
> > +	struct phylink_pcs pcs;
> > +	void __iomem *base;
> > +	int reg_width;
> > +};
> > +
> > +static struct altera_tse_pcs *phylink_pcs_to_tse_pcs(struct
> > phylink_pcs *pcs) +{
> > +	return container_of(pcs, struct altera_tse_pcs, pcs);
> > +}
> > +
> > +static u16 tse_pcs_read(struct altera_tse_pcs *tse_pcs, int regnum)
> > +{
> > +	if (tse_pcs->reg_width == 4)
> > +		return readl(tse_pcs->base + regnum * 4);
> > +	else
> > +		return readw(tse_pcs->base + regnum * 2);
> > +}
> > +
> > +static void tse_pcs_write(struct altera_tse_pcs *tse_pcs, int
> > regnum,
> > +			  u16 value)
> > +{
> > +	if (tse_pcs->reg_width == 4)
> > +		writel(value, tse_pcs->base + regnum * 4);
> > +	else
> > +		writew(value, tse_pcs->base + regnum * 2);
> > +}
> > +
> > +static int tse_pcs_reset(struct altera_tse_pcs *tse_pcs)
> > +{
> > +	int i = 0;
> > +	u16 bmcr;
> > +
> > +	/* Reset PCS block */
> > +	bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
> > +	bmcr |= BMCR_RESET;
> > +	tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
> > +
> > +	for (i = 0; i < SGMII_PCS_SW_RESET_TIMEOUT; i++) {
> > +		if (!(tse_pcs_read(tse_pcs, MII_BMCR) &
> > BMCR_RESET))
> > +			return 0;
> > +		udelay(1);
> > +	}  
> 
> read_poll_timeout?

Oh yeah definitely, I didn't know about this helper.

> > +
> > +	return -ETIMEDOUT;
> > +}
> > +
> > +static int alt_tse_pcs_validate(struct phylink_pcs *pcs,
> > +				unsigned long *supported,
> > +				const struct phylink_link_state
> > *state) +{
> > +	if (state->interface == PHY_INTERFACE_MODE_SGMII ||
> > +	    state->interface == PHY_INTERFACE_MODE_1000BASEX)
> > +		return 1;
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static int alt_tse_pcs_config(struct phylink_pcs *pcs, unsigned
> > int mode,
> > +			      phy_interface_t interface,
> > +			      const unsigned long *advertising,
> > +			      bool permit_pause_to_mac)
> > +{
> > +	struct altera_tse_pcs *tse_pcs =
> > phylink_pcs_to_tse_pcs(pcs);
> > +	u32 ctrl, if_mode;
> > +
> > +	ctrl = tse_pcs_read(tse_pcs, MII_BMCR);
> > +	if_mode = tse_pcs_read(tse_pcs, SGMII_PCS_IF_MODE);
> > +
> > +	/* Set link timer to 1.6ms, as per the MegaCore Function
> > User Guide */
> > +	tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_0, 0x0D40);
> > +	tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_1, 0x03);  
> 
> Shouldn't this be different for SGMII vs 1000BASE-X?

I've dug a bit and indeed you're right. The value of 1.6ms works for
SGMII, but for 1000BaseX it should be set to 10ms. I'll send a fix for
this too.

> > +
> > +	if (interface == PHY_INTERFACE_MODE_SGMII) {
> > +		if_mode |= PCS_IF_MODE_USE_SGMII_AN |
> > PCS_IF_MODE_SGMII_ENA;  
> 
> I think PCS_IF_MODE_USE_SGMII_AN should be cleared if
> mode=MLO_AN_FIXED.

Correct.

> > +	} else if (interface == PHY_INTERFACE_MODE_1000BASEX) {
> > +		if_mode &= ~(PCS_IF_MODE_USE_SGMII_AN |
> > PCS_IF_MODE_SGMII_ENA);
> > +		if_mode |= PCS_IF_MODE_SGMI_SPEED_1000;  
> 
> I don't think you need to set this for 1000BASE-X.

You're correct too.

> > +	}
> > +
> > +	ctrl |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE);  
> 
> BMCR_FULLDPLX is read-only, so you don't have to set it. Same for the
> speed.

Thanks, that's true

> > +
> > +	tse_pcs_write(tse_pcs, MII_BMCR, ctrl);
> > +	tse_pcs_write(tse_pcs, SGMII_PCS_IF_MODE, if_mode);
> > +
> > +	return tse_pcs_reset(tse_pcs);
> > +}
> > +
> > +static void alt_tse_pcs_get_state(struct phylink_pcs *pcs,
> > +				  struct phylink_link_state *state)
> > +{
> > +	struct altera_tse_pcs *tse_pcs =
> > phylink_pcs_to_tse_pcs(pcs);
> > +	u16 bmsr, lpa;
> > +
> > +	bmsr = tse_pcs_read(tse_pcs, MII_BMSR);
> > +	lpa = tse_pcs_read(tse_pcs, MII_LPA);
> > +
> > +	phylink_mii_c22_pcs_decode_state(state, bmsr, lpa);
> > +}
> > +
> > +static void alt_tse_pcs_an_restart(struct phylink_pcs *pcs)
> > +{
> > +	struct altera_tse_pcs *tse_pcs =
> > phylink_pcs_to_tse_pcs(pcs);
> > +	u16 bmcr;
> > +
> > +	bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
> > +	bmcr |= BMCR_ANRESTART;
> > +	tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
> > +
> > +	/* This PCS seems to require a soft reset to re-sync the
> > AN logic */
> > +	tse_pcs_reset(tse_pcs);  
> 
> This is kinda strange since c22 phys are supposed to reset the other
> registers to default values when BMCR_RESET is written. Good thing
> this is a PCS...

Indeed. This soft reset will not affect the register configuration, it
will only reset all internal state machines.

The datasheet actually recommends performing a reset after any
configuration change...

That's one thing with this IP, it tries to re-use the C22 register
layout but it's not fully consistent with it...

> > +}
> > +
> > +static const struct phylink_pcs_ops alt_tse_pcs_ops = {
> > +	.pcs_validate = alt_tse_pcs_validate,
> > +	.pcs_get_state = alt_tse_pcs_get_state,
> > +	.pcs_config = alt_tse_pcs_config,
> > +	.pcs_an_restart = alt_tse_pcs_an_restart,
> > +};  
> 
> Don't you need link_up to set the speed/duplex for MLO_AN_FIXED?

I'll give it a test and confirm it

> > +
> > +struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev,
> > +				       void __iomem *pcs_base, int
> > reg_width) +{
> > +	struct altera_tse_pcs *tse_pcs;
> > +
> > +	if (reg_width != 4 && reg_width != 2)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	tse_pcs = devm_kzalloc(&ndev->dev, sizeof(*tse_pcs),
> > GFP_KERNEL);
> > +	if (!tse_pcs)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	tse_pcs->pcs.ops = &alt_tse_pcs_ops;
> > +	tse_pcs->base = pcs_base;
> > +	tse_pcs->reg_width = reg_width;
> > +
> > +	return &tse_pcs->pcs;
> > +}
> > +EXPORT_SYMBOL_GPL(alt_tse_pcs_create);
> > diff --git a/include/linux/pcs-altera-tse.h
> > b/include/linux/pcs-altera-tse.h new file mode 100644
> > index 000000000000..92ab9f08e835
> > --- /dev/null
> > +++ b/include/linux/pcs-altera-tse.h
> > @@ -0,0 +1,17 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2022 Bootlin
> > + *
> > + * Maxime Chevallier <maxime.chevallier@bootlin.com>
> > + */
> > +
> > +#ifndef __LINUX_PCS_ALTERA_TSE_H
> > +#define __LINUX_PCS_ALTERA_TSE_H
> > +
> > +struct phylink_pcs;
> > +struct net_device;
> > +
> > +struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev,
> > +				       void __iomem *pcs_base, int
> > reg_width); +
> > +#endif /* __LINUX_PCS_ALTERA_TSE_H */  
> 
> --Sean

Thanks a lot for the review ! I'll do a round of tests with the
comments and send follow-up patches.

Best regards,

Maxime


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS
  2022-10-26  9:37     ` Maxime Chevallier
@ 2022-10-26 12:05       ` Andrew Lunn
  2022-10-26 12:47       ` Russell King (Oracle)
  1 sibling, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2022-10-26 12:05 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Sean Anderson, davem, Rob Herring, netdev, linux-kernel,
	thomas.petazzoni, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Florian Fainelli, Heiner Kallweit, Russell King,
	linux-arm-kernel, Krzysztof Kozlowski, devicetree

> > > +	/* This PCS seems to require a soft reset to re-sync the
> > > AN logic */
> > > +	tse_pcs_reset(tse_pcs);  
> > 
> > This is kinda strange since c22 phys are supposed to reset the other
> > registers to default values when BMCR_RESET is written. Good thing
> > this is a PCS...
> 
> Indeed. This soft reset will not affect the register configuration, it
> will only reset all internal state machines.
> 
> The datasheet actually recommends performing a reset after any
> configuration change...

The Marvell PHYs work like this. Many of its registers won't take
effect until you do a soft reset. I think the thinking behind this is
that changing many registers is disruptive to the link and slow. It
takes over a second to perform auto-neg etc. So ideally you want to
make all your register changes, and then trigger them into operation.
And a soft reset is this trigger.

    Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS
  2022-10-26  9:37     ` Maxime Chevallier
  2022-10-26 12:05       ` Andrew Lunn
@ 2022-10-26 12:47       ` Russell King (Oracle)
  1 sibling, 0 replies; 12+ messages in thread
From: Russell King (Oracle) @ 2022-10-26 12:47 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Sean Anderson, davem, Rob Herring, netdev, linux-kernel,
	thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet,
	Paolo Abeni, Florian Fainelli, Heiner Kallweit, linux-arm-kernel,
	Krzysztof Kozlowski, devicetree

On Wed, Oct 26, 2022 at 11:37:11AM +0200, Maxime Chevallier wrote:
> Hello Sean,
> 
> On Sun, 9 Oct 2022 01:38:15 -0400
> Sean Anderson <seanga2@gmail.com> wrote:
> 
> 
> > > +#define   SGMII_PCS_LINK_TIMER_REG(x)		(0x12 + (x))  
> > 
> > Not used.
> 
> Right, I'll remove that in a followup patch
> 
> > > +#define SGMII_PCS_LINK_TIMER_1	0x13
> > > +#define SGMII_PCS_IF_MODE	0x14
> > > +#define   PCS_IF_MODE_SGMII_ENA		BIT(0)
> > > +#define   PCS_IF_MODE_USE_SGMII_AN	BIT(1)
> > > +#define   PCS_IF_MODE_SGMI_SPEED_MASK	GENMASK(3, 2)
> > > +#define   PCS_IF_MODE_SGMI_SPEED_10	(0 << 2)
> > > +#define   PCS_IF_MODE_SGMI_SPEED_100	(1 << 2)
> > > +#define   PCS_IF_MODE_SGMI_SPEED_1000	(2 << 2)  
> > 
> > You can use FIELD_PREP if you're so inclined. I assume SGMI is from
> > the datasheet.
> 
> Will do ! thanks :)
> 
> > > +#define   PCS_IF_MODE_SGMI_HALF_DUPLEX	BIT(4)
> > > +#define   PCS_IF_MODE_SGMI_PHY_ANi	BIT(5)

The definitions up to here look very similar to pcs-lynx.c when it comes
to 1000base-X and SGMII. I wonder whether regmap can help here to
abstract the register access differences and then maybe code can be
shared.

What value is in registers 2 and 3 (the ID registers) for this PCS?

On the link timer value setting, I have a patch to add a phylink helper
which returns the link timer in nanoseconds. May be a good idea if we
get that queued up so drivers can make use of it rather than hard-coding
a register value everywhere.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-10-26 12:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-01 14:35 [PATCH net-next v3 0/5] net: altera: tse: phylink conversion Maxime Chevallier
2022-09-01 14:35 ` [PATCH net-next v3 1/5] dt-bindings: net: Convert Altera TSE bindings to yaml Maxime Chevallier
2022-09-01 14:35 ` [PATCH net-next v3 2/5] net: altera: tse: cosmetic change to use reverse xmas tree ordering Maxime Chevallier
2022-09-01 14:35 ` [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS Maxime Chevallier
2022-10-09  5:38   ` Sean Anderson
2022-10-26  9:37     ` Maxime Chevallier
2022-10-26 12:05       ` Andrew Lunn
2022-10-26 12:47       ` Russell King (Oracle)
2022-09-01 14:35 ` [PATCH net-next v3 4/5] net: altera: tse: convert to phylink Maxime Chevallier
2022-09-02  4:10   ` Jakub Kicinski
2022-09-02  7:57     ` Maxime Chevallier
2022-09-01 14:35 ` [PATCH net-next v3 5/5] dt-bindings: net: altera: tse: add an optional pcs register range Maxime Chevallier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).