* [net-next PATCH v9 0/4] net: dsa: Add Airoha AN8855 support
@ 2024-12-05 14:51 Christian Marangi
2024-12-05 14:51 ` [net-next PATCH v9 1/4] net: dsa: add devm_dsa_register_switch() Christian Marangi
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 14:51 UTC (permalink / raw)
To: Christian Marangi, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, Matthias Brugger, AngeloGioacchino Del Regno,
linux-arm-kernel, linux-mediatek, netdev, devicetree,
linux-kernel, upstream
This small series add the initial support for the Airoha AN8855 Switch.
It's a 5 port Gigabit Switch with SGMII/HSGMII upstream port.
This is starting to get in the wild and there are already some router
having this switch chip.
It's conceptually similar to mediatek switch but register and bits
are different. And there is that massive Hell that is the PCS
configuration.
Saddly for that part we have absolutely NO documentation currently.
There is this special thing where PHY needs to be calibrated with values
from the switch efuse. (the thing have a whole cpu timer and MCU)
From v8 Driver is now evaluated with Kernel selftest scripts for DSA:
Output local_termination.sh
TEST: lan2: Unicast IPv4 to primary MAC address [ OK ]
TEST: lan2: Unicast IPv4 to macvlan MAC address [ OK ]
TEST: lan2: Unicast IPv4 to unknown MAC address [ OK ]
TEST: lan2: Unicast IPv4 to unknown MAC address, promisc [ OK ]
TEST: lan2: Unicast IPv4 to unknown MAC address, allmulti [ OK ]
TEST: lan2: Multicast IPv4 to joined group [ OK ]
TEST: lan2: Multicast IPv4 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: lan2: Multicast IPv4 to unknown group, promisc [ OK ]
TEST: lan2: Multicast IPv4 to unknown group, allmulti [ OK ]
TEST: lan2: Multicast IPv6 to joined group [ OK ]
TEST: lan2: Multicast IPv6 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: lan2: Multicast IPv6 to unknown group, promisc [ OK ]
TEST: lan2: Multicast IPv6 to unknown group, allmulti [ OK ]
TEST: lan2: 1588v2 over L2 transport, Sync [ OK ]
TEST: lan2: 1588v2 over L2 transport, Follow-Up [ OK ]
TEST: lan2: 1588v2 over L2 transport, Peer Delay Request [ OK ]
TEST: lan2: 1588v2 over IPv4, Sync [FAIL]
reception failed
TEST: lan2: 1588v2 over IPv4, Follow-Up [FAIL]
reception failed
TEST: lan2: 1588v2 over IPv4, Peer Delay Request [FAIL]
reception failed
TEST: lan2: 1588v2 over IPv6, Sync [FAIL]
reception failed
TEST: lan2: 1588v2 over IPv6, Follow-Up [FAIL]
reception failed
TEST: lan2: 1588v2 over IPv6, Peer Delay Request [FAIL]
reception failed
TEST: vlan_filtering=0 bridge: Unicast IPv4 to primary MAC address [ OK ]
TEST: vlan_filtering=0 bridge: Unicast IPv4 to macvlan MAC address [ OK ]
TEST: vlan_filtering=0 bridge: Unicast IPv4 to unknown MAC address [ OK ]
TEST: vlan_filtering=0 bridge: Unicast IPv4 to unknown MAC address, promisc [ OK ]
TEST: vlan_filtering=0 bridge: Unicast IPv4 to unknown MAC address, allmulti [ OK ]
TEST: vlan_filtering=0 bridge: Multicast IPv4 to joined group [ OK ]
TEST: vlan_filtering=0 bridge: Multicast IPv4 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: vlan_filtering=0 bridge: Multicast IPv4 to unknown group, promisc [ OK ]
TEST: vlan_filtering=0 bridge: Multicast IPv4 to unknown group, allmulti [ OK ]
TEST: vlan_filtering=0 bridge: Multicast IPv6 to joined group [ OK ]
TEST: vlan_filtering=0 bridge: Multicast IPv6 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: vlan_filtering=0 bridge: Multicast IPv6 to unknown group, promisc [ OK ]
TEST: vlan_filtering=0 bridge: Multicast IPv6 to unknown group, allmulti [ OK ]
TEST: vlan_filtering=1 bridge: Unicast IPv4 to primary MAC address [ OK ]
TEST: vlan_filtering=1 bridge: Unicast IPv4 to macvlan MAC address [ OK ]
TEST: vlan_filtering=1 bridge: Unicast IPv4 to unknown MAC address [ OK ]
TEST: vlan_filtering=1 bridge: Unicast IPv4 to unknown MAC address, promisc [ OK ]
TEST: vlan_filtering=1 bridge: Unicast IPv4 to unknown MAC address, allmulti [ OK ]
TEST: vlan_filtering=1 bridge: Multicast IPv4 to joined group [ OK ]
TEST: vlan_filtering=1 bridge: Multicast IPv4 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: vlan_filtering=1 bridge: Multicast IPv4 to unknown group, promisc [ OK ]
TEST: vlan_filtering=1 bridge: Multicast IPv4 to unknown group, allmulti [ OK ]
TEST: vlan_filtering=1 bridge: Multicast IPv6 to joined group [ OK ]
TEST: vlan_filtering=1 bridge: Multicast IPv6 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: vlan_filtering=1 bridge: Multicast IPv6 to unknown group, promisc [ OK ]
TEST: vlan_filtering=1 bridge: Multicast IPv6 to unknown group, allmulti [ OK ]
TEST: VLAN upper: Unicast IPv4 to primary MAC address [ OK ]
TEST: VLAN upper: Unicast IPv4 to macvlan MAC address [ OK ]
TEST: VLAN upper: Unicast IPv4 to unknown MAC address [ OK ]
TEST: VLAN upper: Unicast IPv4 to unknown MAC address, promisc [ OK ]
TEST: VLAN upper: Unicast IPv4 to unknown MAC address, allmulti [ OK ]
TEST: VLAN upper: Multicast IPv4 to joined group [ OK ]
TEST: VLAN upper: Multicast IPv4 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN upper: Multicast IPv4 to unknown group, promisc [ OK ]
TEST: VLAN upper: Multicast IPv4 to unknown group, allmulti [ OK ]
TEST: VLAN upper: Multicast IPv6 to joined group [ OK ]
TEST: VLAN upper: Multicast IPv6 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN upper: Multicast IPv6 to unknown group, promisc [ OK ]
TEST: VLAN upper: Multicast IPv6 to unknown group, allmulti [ OK ]
TEST: VLAN upper: 1588v2 over L2 transport, Sync [ OK ]
TEST: VLAN upper: 1588v2 over L2 transport, Follow-Up [FAIL]
reception failed
TEST: VLAN upper: 1588v2 over L2 transport, Peer Delay Request [ OK ]
TEST: VLAN upper: 1588v2 over IPv4, Sync [FAIL]
reception failed
;TEST: VLAN upper: 1588v2 over IPv4, Follow-Up [FAIL]
reception failed
TEST: VLAN upper: 1588v2 over IPv4, Peer Delay Request [FAIL]
reception failed
TEST: VLAN upper: 1588v2 over IPv6, Sync [FAIL]
reception failed
TEST: VLAN upper: 1588v2 over IPv6, Follow-Up [FAIL]
reception failed
TEST: VLAN upper: 1588v2 over IPv6, Peer Delay Request [FAIL]
reception failed
TEST: VLAN over vlan_filtering=0 bridged port: Unicast IPv4 to primary MAC address [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Unicast IPv4 to macvlan MAC address [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Unicast IPv4 to unknown MAC address [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Unicast IPv4 to unknown MAC address, promisc [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Unicast IPv4 to unknown MAC address, allmulti [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Multicast IPv4 to joined group [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Multicast IPv4 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=0 bridged port: Multicast IPv4 to unknown group, promisc [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Multicast IPv4 to unknown group, allmulti [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Multicast IPv6 to joined group [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Multicast IPv6 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=0 bridged port: Multicast IPv6 to unknown group, promisc [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: Multicast IPv6 to unknown group, allmulti [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: 1588v2 over L2 transport, Sync [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: 1588v2 over L2 transport, Follow-Up [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: 1588v2 over L2 transport, Peer Delay Request [ OK ]
TEST: VLAN over vlan_filtering=0 bridged port: 1588v2 over IPv4, Sync [FAIL]
reception failed
TEST: VLAN over vlan_filtering=0 bridged port: 1588v2 over IPv4, Follow-Up [FAIL]
reception failed
TEST: VLAN over vlan_filtering=0 bridged port: 1588v2 over IPv4, Peer Delay Request [FAIL]
reception failed
TEST: VLAN over vlan_filtering=0 bridged port: 1588v2 over IPv6, Sync [FAIL]
reception failed
TEST: VLAN over vlan_filtering=0 bridged port: 1588v2 over IPv6, Follow-Up [FAIL]
reception failed
TEST: VLAN over vlan_filtering=0 bridged port: 1588v2 over IPv6, Peer Delay Request [FAIL]
reception failed
TEST: VLAN over vlan_filtering=1 bridged port: Unicast IPv4 to primary MAC address [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: Unicast IPv4 to macvlan MAC address [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: Unicast IPv4 to unknown MAC address [FAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=1 bridged port: Unicast IPv4 to unknown MAC address, promisc [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: Unicast IPv4 to unknown MAC address, allmulti [FAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=1 bridged port: Multicast IPv4 to joined group [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: Multicast IPv4 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=1 bridged port: Multicast IPv4 to unknown group, promisc [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: Multicast IPv4 to unknown group, allmulti [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: Multicast IPv6 to joined group [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: Multicast IPv6 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=1 bridged port: Multicast IPv6 to unknown group, promisc [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: Multicast IPv6 to unknown group, allmulti [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: 1588v2 over L2 transport, Sync [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: 1588v2 over L2 transport, Follow-Up [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: 1588v2 over L2 transport, Peer Delay Request [ OK ]
TEST: VLAN over vlan_filtering=1 bridged port: 1588v2 over IPv4, Sync [FAIL]
reception failed
TEST: VLAN over vlan_filtering=1 bridged port: 1588v2 over IPv4, Follow-Up [FAIL]
reception failed
TEST: VLAN over vlan_filtering=1 bridged port: 1588v2 over IPv4, Peer Delay Request [FAIL]
reception failed
TEST: VLAN over vlan_filtering=1 bridged port: 1588v2 over IPv6, Sync [FAIL]
reception failed
TEST: VLAN over vlan_filtering=1 bridged port: 1588v2 over IPv6, Follow-Up [FAIL]
reception failed
TEST: VLAN over vlan_filtering=1 bridged port: 1588v2 over IPv6, Peer Delay Request [FAIL]
reception failed
TEST: VLAN over vlan_filtering=0 bridge: Unicast IPv4 to primary MAC address [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Unicast IPv4 to macvlan MAC address [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Unicast IPv4 to unknown MAC address [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Unicast IPv4 to unknown MAC address, promisc [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Unicast IPv4 to unknown MAC address, allmulti [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Multicast IPv4 to joined group [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Multicast IPv4 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=0 bridge: Multicast IPv4 to unknown group, promisc [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Multicast IPv4 to unknown group, allmulti [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Multicast IPv6 to joined group [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Multicast IPv6 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=0 bridge: Multicast IPv6 to unknown group, promisc [ OK ]
TEST: VLAN over vlan_filtering=0 bridge: Multicast IPv6 to unknown group, allmulti [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Unicast IPv4 to primary MAC address [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Unicast IPv4 to macvlan MAC address [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Unicast IPv4 to unknown MAC address [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Unicast IPv4 to unknown MAC address, promisc [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Unicast IPv4 to unknown MAC address, allmulti [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Multicast IPv4 to joined group [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Multicast IPv4 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=1 bridge: Multicast IPv4 to unknown group, promisc [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Multicast IPv4 to unknown group, allmulti [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Multicast IPv6 to joined group [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Multicast IPv6 to unknown group [XFAIL]
reception succeeded, but should have failed
TEST: VLAN over vlan_filtering=1 bridge: Multicast IPv6 to unknown group, promisc [ OK ]
TEST: VLAN over vlan_filtering=1 bridge: Multicast IPv6 to unknown group, allmulti [ OK ]
Output bridge_vlan_unaware.sh
TEST: ping [ OK ]
TEST: ping6 [ OK ]
TEST: FDB learning [ OK ]
TEST: Unknown unicast flood [ OK ]
TEST: Unregistered multicast flood [ OK ]
Output bridge_vlan_aware.sh
TEST: ping [ OK ]
TEST: ping6 [ OK ]
TEST: FDB learning [ OK ]
TEST: Unknown unicast flood [ OK ]
TEST: Unregistered multicast flood [ OK ]
INFO: Add and delete a VLAN on bridge port lan2
TEST: ping [ OK ]
TEST: ping6 [ OK ]
TEST: Externally learned FDB entry - ageing & roaming [ OK ]
TEST: FDB entry in PVID for VLAN-tagged with other TPID [FAIL]
FDB entry was not learned when it should
TEST: Reception of VLAN with other TPID as untagged [FAIL]
Packet was not forwarded when it should
TEST: Reception of VLAN with other TPID as untagged (no PVID) [FAIL]
Packet was forwarded when should not
Changes v9:
- Error out on using 5G speed as currently not supported
- Add missing MAC_2500FD in phylink mac_capabilities
- Add comment and improve if condition for an8855_phylink_mac_config
Changes v8:
- Add port Fast Age support
- Add support for Port Isolation
- Use correct register for Learning Disable
- Add support for Ageing Time OP
- Set default PVID to 0 by default
- Add mdb OPs
- Add port change MTU
- Fix support for Upper VLAN
Changes v7:
- Fix devm_dsa_register_switch wrong export symbol
Changes v6:
- Drop standard MIB and handle with ethtool OPs (as requested by Jakub)
- Cosmetic: use bool instead of 0 or 1
Changes v5:
- Add devm_dsa_register_switch() patch
- Add Reviewed-by tag for DT patch
Changes v4:
- Set regmap readable_table static (mute compilation warning)
- Add support for port_bridge flags (LEARNING, FLOOD)
- Reset fdb struct in fdb_dump
- Drop support_asym_pause in port_enable
- Add define for get_phy_flags
- Fix bug for port not inititially part of a bridge
(in an8855_setup the port matrix was always cleared but
the CPU port was never initially added)
- Disable learning and flood for user port by default
- Set CPU port to flood and learning by default
- Correctly AND force duplex and flow control in an8855_phylink_mac_link_up
- Drop RGMII from pcs_config
- Check ret in "Disable AN if not in autoneg"
- Use devm_mutex_init
- Fix typo for AN8855_PORT_CHECK_MODE
- Better define AN8855_STP_LISTENING = AN8855_STP_BLOCKING
- Fix typo in AN8855_PHY_EN_DOWN_SHIFT
- Use paged helper for PHY
- Skip calibration in config_init if priv not defined
Changes v3:
- Out of RFC
- Switch PHY code to select_page API
- Better describe masks and bits in PHY driver for ADC register
- Drop raw values and use define for mii read/write
- Switch to absolute PHY address
- Replace raw values with mask and bits for pcs_config
- Fix typo for ext-surge property name
- Drop support for relocating Switch base PHY address on the bus
Changes v2:
- Drop mutex guard patch
- Drop guard usage in DSA driver
- Use __mdiobus_write/read
- Check return condition and return errors for mii read/write
- Fix wrong logic for EEE
- Fix link_down (don't force link down with autoneg)
- Fix forcing speed on sgmii autoneg
- Better document link speed for sgmii reg
- Use standard define for sgmii reg
- Imlement nvmem support to expose switch EFUSE
- Rework PHY calibration with the use of NVMEM producer/consumer
- Update DT with new NVMEM property
- Move aneg validation for 2500-basex in pcs_config
- Move r50Ohm table and function to PHY driver
Christian Marangi (4):
net: dsa: add devm_dsa_register_switch()
dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation
net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
.../bindings/net/dsa/airoha,an8855.yaml | 242 ++
MAINTAINERS | 11 +
drivers/net/dsa/Kconfig | 9 +
drivers/net/dsa/Makefile | 1 +
drivers/net/dsa/an8855.c | 2671 +++++++++++++++++
drivers/net/dsa/an8855.h | 810 +++++
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/air_an8855.c | 267 ++
include/net/dsa.h | 1 +
net/dsa/dsa.c | 19 +
11 files changed, 4037 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
create mode 100644 drivers/net/dsa/an8855.c
create mode 100644 drivers/net/dsa/an8855.h
create mode 100644 drivers/net/phy/air_an8855.c
--
2.45.2
^ permalink raw reply [flat|nested] 21+ messages in thread
* [net-next PATCH v9 1/4] net: dsa: add devm_dsa_register_switch()
2024-12-05 14:51 [net-next PATCH v9 0/4] net: dsa: Add Airoha AN8855 support Christian Marangi
@ 2024-12-05 14:51 ` Christian Marangi
2024-12-05 16:03 ` Vladimir Oltean
2024-12-05 14:51 ` [net-next PATCH v9 2/4] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation Christian Marangi
` (2 subsequent siblings)
3 siblings, 1 reply; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 14:51 UTC (permalink / raw)
To: Christian Marangi, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, Matthias Brugger, AngeloGioacchino Del Regno,
linux-arm-kernel, linux-mediatek, netdev, devicetree,
linux-kernel, upstream
Some DSA driver can be simplified if devres takes care of unregistering
the DSA switch. This permits to effectively drop the remove OP from
driver that just execute the dsa_unregister_switch() and nothing else.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
include/net/dsa.h | 1 +
net/dsa/dsa.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 72ae65e7246a..c703d5dc3fb0 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1355,6 +1355,7 @@ static inline void dsa_tag_generic_flow_dissect(const struct sk_buff *skb,
void dsa_unregister_switch(struct dsa_switch *ds);
int dsa_register_switch(struct dsa_switch *ds);
+int devm_dsa_register_switch(struct device *dev, struct dsa_switch *ds);
void dsa_switch_shutdown(struct dsa_switch *ds);
struct dsa_switch *dsa_switch_find(int tree_index, int sw_index);
void dsa_flush_workqueue(void);
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 5a7c0e565a89..aca6aee68248 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -1544,6 +1544,25 @@ int dsa_register_switch(struct dsa_switch *ds)
}
EXPORT_SYMBOL_GPL(dsa_register_switch);
+static void devm_dsa_unregister_switch(void *data)
+{
+ struct dsa_switch *ds = data;
+
+ dsa_unregister_switch(ds);
+}
+
+int devm_dsa_register_switch(struct device *dev, struct dsa_switch *ds)
+{
+ int err;
+
+ err = dsa_register_switch(ds);
+ if (err)
+ return err;
+
+ return devm_add_action_or_reset(dev, devm_dsa_unregister_switch, ds);
+}
+EXPORT_SYMBOL_GPL(devm_dsa_register_switch);
+
static void dsa_switch_remove(struct dsa_switch *ds)
{
struct dsa_switch_tree *dst = ds->dst;
--
2.45.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [net-next PATCH v9 2/4] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation
2024-12-05 14:51 [net-next PATCH v9 0/4] net: dsa: Add Airoha AN8855 support Christian Marangi
2024-12-05 14:51 ` [net-next PATCH v9 1/4] net: dsa: add devm_dsa_register_switch() Christian Marangi
@ 2024-12-05 14:51 ` Christian Marangi
2024-12-05 14:51 ` [net-next PATCH v9 4/4] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY Christian Marangi
[not found] ` <20241205145142.29278-4-ansuelsmth@gmail.com>
3 siblings, 0 replies; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 14:51 UTC (permalink / raw)
To: Christian Marangi, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, Matthias Brugger, AngeloGioacchino Del Regno,
linux-arm-kernel, linux-mediatek, netdev, devicetree,
linux-kernel, upstream
Add Airoha AN8855 5 port Gigabit Switch documentation.
The switch node requires an additional mdio node to describe each internal
PHY absolute address on the bus.
Calibration values might be stored in switch EFUSE and internal PHY
might need to be calibrated, in such case, airoha,ext-surge needs to be
enabled and relative NVMEM cells needs to be defined in nvmem-layout
node.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
.../bindings/net/dsa/airoha,an8855.yaml | 242 ++++++++++++++++++
1 file changed, 242 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
diff --git a/Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml b/Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
new file mode 100644
index 000000000000..8ea2fadbab85
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
@@ -0,0 +1,242 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/airoha,an8855.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha AN8855 Gigabit switch
+
+maintainers:
+ - Christian Marangi <ansuelsmth@gmail.com>
+
+description: >
+ Airoha AN8855 is a 5-port Gigabit Switch.
+
+ The switch node requires an additional mdio node to describe each internal
+ PHY relative offset as the PHY address for the switch match the one for
+ the PHY ports. On top of internal PHY address, the switch base PHY address
+ is added.
+
+ Also the switch base PHY address can be configured and changed after the
+ first initialization. On reset, the switch PHY address is ALWAYS 1.
+
+properties:
+ compatible:
+ const: airoha,an8855
+
+ reg:
+ maxItems: 1
+
+ reset-gpios:
+ description:
+ GPIO to be used to reset the whole device
+ maxItems: 1
+
+ airoha,ext-surge:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Calibrate the internal PHY with the calibration values stored in EFUSE
+ for the r50Ohm values.
+
+ '#nvmem-cell-cells':
+ const: 0
+
+ nvmem-layout:
+ $ref: /schemas/nvmem/layouts/nvmem-layout.yaml
+ description:
+ NVMEM Layout for exposed EFUSE. (for example to propagate calibration
+ value for r50Ohm for internal PHYs)
+
+ mdio:
+ $ref: /schemas/net/mdio.yaml#
+ unevaluatedProperties: false
+ description:
+ Define the absolute address of the internal PHY for each port.
+
+$ref: dsa.yaml#
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch@1 {
+ compatible = "airoha,an8855";
+ reg = <1>;
+ reset-gpios = <&pio 39 0>;
+
+ airoha,ext-surge;
+
+ #nvmem-cell-cells = <0>;
+
+ nvmem-layout {
+ compatible = "fixed-layout";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ shift_sel_port0_tx_a: shift-sel-port0-tx-a@c {
+ reg = <0xc 0x4>;
+ };
+
+ shift_sel_port0_tx_b: shift-sel-port0-tx-b@10 {
+ reg = <0x10 0x4>;
+ };
+
+ shift_sel_port0_tx_c: shift-sel-port0-tx-c@14 {
+ reg = <0x14 0x4>;
+ };
+
+ shift_sel_port0_tx_d: shift-sel-port0-tx-d@18 {
+ reg = <0x18 0x4>;
+ };
+
+ shift_sel_port1_tx_a: shift-sel-port1-tx-a@1c {
+ reg = <0x1c 0x4>;
+ };
+
+ shift_sel_port1_tx_b: shift-sel-port1-tx-b@20 {
+ reg = <0x20 0x4>;
+ };
+
+ shift_sel_port1_tx_c: shift-sel-port1-tx-c@24 {
+ reg = <0x24 0x4>;
+ };
+
+ shift_sel_port1_tx_d: shift-sel-port1-tx-d@28 {
+ reg = <0x28 0x4>;
+ };
+
+ shift_sel_port2_tx_a: shift-sel-port2-tx-a@2c {
+ reg = <0x2c 0x4>;
+ };
+
+ shift_sel_port2_tx_b: shift-sel-port2-tx-b@30 {
+ reg = <0x30 0x4>;
+ };
+
+ shift_sel_port2_tx_c: shift-sel-port2-tx-c@34 {
+ reg = <0x34 0x4>;
+ };
+
+ shift_sel_port2_tx_d: shift-sel-port2-tx-d@38 {
+ reg = <0x38 0x4>;
+ };
+
+ shift_sel_port3_tx_a: shift-sel-port3-tx-a@4c {
+ reg = <0x4c 0x4>;
+ };
+
+ shift_sel_port3_tx_b: shift-sel-port3-tx-b@50 {
+ reg = <0x50 0x4>;
+ };
+
+ shift_sel_port3_tx_c: shift-sel-port3-tx-c@54 {
+ reg = <0x54 0x4>;
+ };
+
+ shift_sel_port3_tx_d: shift-sel-port3-tx-d@58 {
+ reg = <0x58 0x4>;
+ };
+ };
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ label = "lan1";
+ phy-mode = "internal";
+ phy-handle = <&internal_phy1>;
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "lan2";
+ phy-mode = "internal";
+ phy-handle = <&internal_phy2>;
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "lan3";
+ phy-mode = "internal";
+ phy-handle = <&internal_phy3>;
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "lan4";
+ phy-mode = "internal";
+ phy-handle = <&internal_phy4>;
+ };
+
+ port@5 {
+ reg = <5>;
+ label = "cpu";
+ ethernet = <&gmac0>;
+ phy-mode = "2500base-x";
+
+ fixed-link {
+ speed = <2500>;
+ full-duplex;
+ pause;
+ };
+ };
+ };
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ internal_phy1: phy@1 {
+ reg = <1>;
+
+ nvmem-cells = <&shift_sel_port0_tx_a>,
+ <&shift_sel_port0_tx_b>,
+ <&shift_sel_port0_tx_c>,
+ <&shift_sel_port0_tx_d>;
+ nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
+ };
+
+ internal_phy2: phy@2 {
+ reg = <2>;
+
+ nvmem-cells = <&shift_sel_port1_tx_a>,
+ <&shift_sel_port1_tx_b>,
+ <&shift_sel_port1_tx_c>,
+ <&shift_sel_port1_tx_d>;
+ nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
+ };
+
+ internal_phy3: phy@3 {
+ reg = <3>;
+
+ nvmem-cells = <&shift_sel_port2_tx_a>,
+ <&shift_sel_port2_tx_b>,
+ <&shift_sel_port2_tx_c>,
+ <&shift_sel_port2_tx_d>;
+ nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
+ };
+
+ internal_phy4: phy@4 {
+ reg = <4>;
+
+ nvmem-cells = <&shift_sel_port3_tx_a>,
+ <&shift_sel_port3_tx_b>,
+ <&shift_sel_port3_tx_c>,
+ <&shift_sel_port3_tx_d>;
+ nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
+ };
+ };
+ };
+ };
--
2.45.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [net-next PATCH v9 4/4] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY
2024-12-05 14:51 [net-next PATCH v9 0/4] net: dsa: Add Airoha AN8855 support Christian Marangi
2024-12-05 14:51 ` [net-next PATCH v9 1/4] net: dsa: add devm_dsa_register_switch() Christian Marangi
2024-12-05 14:51 ` [net-next PATCH v9 2/4] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation Christian Marangi
@ 2024-12-05 14:51 ` Christian Marangi
[not found] ` <20241205145142.29278-4-ansuelsmth@gmail.com>
3 siblings, 0 replies; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 14:51 UTC (permalink / raw)
To: Christian Marangi, Andrew Lunn, Florian Fainelli, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, Matthias Brugger, AngeloGioacchino Del Regno,
linux-arm-kernel, linux-mediatek, netdev, devicetree,
linux-kernel, upstream
Add support for Airoha AN8855 Internal Switch Gigabit PHY.
This is a simple PHY driver to configure and calibrate the PHY for the
AN8855 Switch with the use of NVMEM cells.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
MAINTAINERS | 1 +
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/air_an8855.c | 267 +++++++++++++++++++++++++++++++++++
4 files changed, 274 insertions(+)
create mode 100644 drivers/net/phy/air_an8855.c
diff --git a/MAINTAINERS b/MAINTAINERS
index e3077d9feee2..cf34add2a0bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -726,6 +726,7 @@ S: Maintained
F: Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
F: drivers/net/dsa/an8855.c
F: drivers/net/dsa/an8855.h
+F: drivers/net/phy/air_an8855.c
AIROHA ETHERNET DRIVER
M: Lorenzo Bianconi <lorenzo@kernel.org>
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index ee3ea0b56d48..1d474038ea7f 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -79,6 +79,11 @@ config SFP
comment "MII PHY device drivers"
+config AIR_AN8855_PHY
+ tristate "Airoha AN8855 Internal Gigabit PHY"
+ help
+ Currently supports the internal Airoha AN8855 Switch PHY.
+
config AIR_EN8811H_PHY
tristate "Airoha EN8811H 2.5 Gigabit PHY"
help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 90f886844381..baba7894785b 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -35,6 +35,7 @@ obj-y += $(sfp-obj-y) $(sfp-obj-m)
obj-$(CONFIG_ADIN_PHY) += adin.o
obj-$(CONFIG_ADIN1100_PHY) += adin1100.o
+obj-$(CONFIG_AIR_AN8855_PHY) += air_an8855.o
obj-$(CONFIG_AIR_EN8811H_PHY) += air_en8811h.o
obj-$(CONFIG_AMD_PHY) += amd.o
obj-$(CONFIG_AMCC_QT2025_PHY) += qt2025.o
diff --git a/drivers/net/phy/air_an8855.c b/drivers/net/phy/air_an8855.c
new file mode 100644
index 000000000000..7ede6674994f
--- /dev/null
+++ b/drivers/net/phy/air_an8855.c
@@ -0,0 +1,267 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2024 Christian Marangi <ansuelsmth@gmail.com>
+ */
+
+#include <linux/phy.h>
+#include <linux/module.h>
+#include <linux/bitfield.h>
+#include <linux/nvmem-consumer.h>
+
+#define AN8855_PHY_SELECT_PAGE 0x1f
+#define AN8855_PHY_PAGE GENMASK(2, 0)
+#define AN8855_PHY_PAGE_STANDARD FIELD_PREP_CONST(AN8855_PHY_PAGE, 0x0)
+#define AN8855_PHY_PAGE_EXTENDED_1 FIELD_PREP_CONST(AN8855_PHY_PAGE, 0x1)
+
+/* MII Registers Page 1 */
+#define AN8855_PHY_EXT_REG_14 0x14
+#define AN8855_PHY_EN_DOWN_SHIFT BIT(4)
+
+/* R50 Calibration regs in MDIO_MMD_VEND1 */
+#define AN8855_PHY_R500HM_RSEL_TX_AB 0x174
+#define AN8855_PHY_R50OHM_RSEL_TX_A_EN BIT(15)
+#define AN8855_PHY_R50OHM_RSEL_TX_A GENMASK(14, 8)
+#define AN8855_PHY_R50OHM_RSEL_TX_B_EN BIT(7)
+#define AN8855_PHY_R50OHM_RSEL_TX_B GENMASK(6, 0)
+#define AN8855_PHY_R500HM_RSEL_TX_CD 0x175
+#define AN8855_PHY_R50OHM_RSEL_TX_C_EN BIT(15)
+#define AN8855_PHY_R50OHM_RSEL_TX_C GENMASK(14, 8)
+#define AN8855_PHY_R50OHM_RSEL_TX_D_EN BIT(7)
+#define AN8855_PHY_R50OHM_RSEL_TX_D GENMASK(6, 0)
+
+#define AN8855_SWITCH_EFUSE_R50O GENMASK(30, 24)
+
+/* PHY TX PAIR DELAY SELECT Register */
+#define AN8855_PHY_TX_PAIR_DLY_SEL_GBE 0x013
+#define AN8855_PHY_CR_DA_TX_PAIR_DELKAY_SEL_A_GBE GENMASK(14, 12)
+#define AN8855_PHY_CR_DA_TX_PAIR_DELKAY_SEL_B_GBE GENMASK(10, 8)
+#define AN8855_PHY_CR_DA_TX_PAIR_DELKAY_SEL_C_GBE GENMASK(6, 4)
+#define AN8855_PHY_CR_DA_TX_PAIR_DELKAY_SEL_D_GBE GENMASK(2, 0)
+/* PHY ADC Register */
+#define AN8855_PHY_RXADC_CTRL 0x0d8
+#define AN8855_PHY_RG_AD_SAMNPLE_PHSEL_A BIT(12)
+#define AN8855_PHY_RG_AD_SAMNPLE_PHSEL_B BIT(8)
+#define AN8855_PHY_RG_AD_SAMNPLE_PHSEL_C BIT(4)
+#define AN8855_PHY_RG_AD_SAMNPLE_PHSEL_D BIT(0)
+#define AN8855_PHY_RXADC_REV_0 0x0d9
+#define AN8855_PHY_RG_AD_RESERVE0_A GENMASK(15, 8)
+#define AN8855_PHY_RG_AD_RESERVE0_B GENMASK(7, 0)
+#define AN8855_PHY_RXADC_REV_1 0x0da
+#define AN8855_PHY_RG_AD_RESERVE0_C GENMASK(15, 8)
+#define AN8855_PHY_RG_AD_RESERVE0_D GENMASK(7, 0)
+
+#define AN8855_PHY_ID 0xc0ff0410
+
+#define AN8855_PHY_FLAGS_EN_CALIBRATION BIT(0)
+
+struct air_an8855_priv {
+ u8 calibration_data[4];
+};
+
+static const u8 dsa_r50ohm_table[] = {
+ 127, 127, 127, 127, 127, 127, 127, 127, 127, 127,
+ 127, 127, 127, 127, 127, 127, 127, 126, 122, 117,
+ 112, 109, 104, 101, 97, 94, 90, 88, 84, 80,
+ 78, 74, 72, 68, 66, 64, 61, 58, 56, 53,
+ 51, 48, 47, 44, 42, 40, 38, 36, 34, 32,
+ 31, 28, 27, 24, 24, 22, 20, 18, 16, 16,
+ 14, 12, 11, 9
+};
+
+static int en8855_get_r50ohm_val(struct device *dev, const char *calib_name,
+ u8 *dest)
+{
+ u32 shift_sel, val;
+ int ret;
+ int i;
+
+ ret = nvmem_cell_read_u32(dev, calib_name, &val);
+ if (ret)
+ return ret;
+
+ shift_sel = FIELD_GET(AN8855_SWITCH_EFUSE_R50O, val);
+ for (i = 0; i < ARRAY_SIZE(dsa_r50ohm_table); i++)
+ if (dsa_r50ohm_table[i] == shift_sel)
+ break;
+
+ if (i < 8 || i >= ARRAY_SIZE(dsa_r50ohm_table))
+ *dest = dsa_r50ohm_table[25];
+ else
+ *dest = dsa_r50ohm_table[i - 8];
+
+ return 0;
+}
+
+static int an8855_probe(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ struct device_node *node = dev->of_node;
+ struct air_an8855_priv *priv;
+ int ret;
+
+ /* If we don't have a node, skip get calib */
+ if (!node)
+ return 0;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ ret = en8855_get_r50ohm_val(dev, "tx_a", &priv->calibration_data[0]);
+ if (ret)
+ return ret;
+
+ ret = en8855_get_r50ohm_val(dev, "tx_b", &priv->calibration_data[1]);
+ if (ret)
+ return ret;
+
+ ret = en8855_get_r50ohm_val(dev, "tx_c", &priv->calibration_data[2]);
+ if (ret)
+ return ret;
+
+ ret = en8855_get_r50ohm_val(dev, "tx_d", &priv->calibration_data[3]);
+ if (ret)
+ return ret;
+
+ phydev->priv = priv;
+
+ return 0;
+}
+
+static int an8855_get_downshift(struct phy_device *phydev, u8 *data)
+{
+ int val;
+
+ val = phy_read_paged(phydev, AN8855_PHY_PAGE_EXTENDED_1, AN8855_PHY_EXT_REG_14);
+ if (val < 0)
+ return val;
+
+ *data = val & AN8855_PHY_EN_DOWN_SHIFT ? DOWNSHIFT_DEV_DEFAULT_COUNT :
+ DOWNSHIFT_DEV_DISABLE;
+
+ return 0;
+}
+
+static int an8855_set_downshift(struct phy_device *phydev, u8 cnt)
+{
+ u16 ds = cnt != DOWNSHIFT_DEV_DISABLE ? AN8855_PHY_EN_DOWN_SHIFT : 0;
+
+ return phy_modify_paged(phydev, AN8855_PHY_PAGE_EXTENDED_1,
+ AN8855_PHY_EXT_REG_14, AN8855_PHY_EN_DOWN_SHIFT,
+ ds);
+}
+
+static int an8855_config_init(struct phy_device *phydev)
+{
+ struct air_an8855_priv *priv = phydev->priv;
+ int ret;
+
+ /* Enable HW auto downshift */
+ ret = an8855_set_downshift(phydev, DOWNSHIFT_DEV_DEFAULT_COUNT);
+ if (ret)
+ return ret;
+
+ /* Apply calibration values, if needed.
+ * AN8855_PHY_FLAGS_EN_CALIBRATION signal this.
+ */
+ if (priv && phydev->dev_flags & AN8855_PHY_FLAGS_EN_CALIBRATION) {
+ u8 *calibration_data = priv->calibration_data;
+
+ ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_R500HM_RSEL_TX_AB,
+ AN8855_PHY_R50OHM_RSEL_TX_A | AN8855_PHY_R50OHM_RSEL_TX_B,
+ FIELD_PREP(AN8855_PHY_R50OHM_RSEL_TX_A, calibration_data[0]) |
+ FIELD_PREP(AN8855_PHY_R50OHM_RSEL_TX_B, calibration_data[1]));
+ if (ret)
+ return ret;
+ ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_R500HM_RSEL_TX_CD,
+ AN8855_PHY_R50OHM_RSEL_TX_C | AN8855_PHY_R50OHM_RSEL_TX_D,
+ FIELD_PREP(AN8855_PHY_R50OHM_RSEL_TX_C, calibration_data[2]) |
+ FIELD_PREP(AN8855_PHY_R50OHM_RSEL_TX_D, calibration_data[3]));
+ if (ret)
+ return ret;
+ }
+
+ /* Apply values to reduce signal noise */
+ ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_TX_PAIR_DLY_SEL_GBE,
+ FIELD_PREP(AN8855_PHY_CR_DA_TX_PAIR_DELKAY_SEL_A_GBE, 0x4) |
+ FIELD_PREP(AN8855_PHY_CR_DA_TX_PAIR_DELKAY_SEL_C_GBE, 0x4));
+ if (ret)
+ return ret;
+ ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_RXADC_CTRL,
+ AN8855_PHY_RG_AD_SAMNPLE_PHSEL_A |
+ AN8855_PHY_RG_AD_SAMNPLE_PHSEL_C);
+ if (ret)
+ return ret;
+ ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_RXADC_REV_0,
+ FIELD_PREP(AN8855_PHY_RG_AD_RESERVE0_A, 0x1));
+ if (ret)
+ return ret;
+ ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, AN8855_PHY_RXADC_REV_1,
+ FIELD_PREP(AN8855_PHY_RG_AD_RESERVE0_C, 0x1));
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int an8855_get_tunable(struct phy_device *phydev,
+ struct ethtool_tunable *tuna, void *data)
+{
+ switch (tuna->id) {
+ case ETHTOOL_PHY_DOWNSHIFT:
+ return an8855_get_downshift(phydev, data);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int an8855_set_tunable(struct phy_device *phydev,
+ struct ethtool_tunable *tuna, const void *data)
+{
+ switch (tuna->id) {
+ case ETHTOOL_PHY_DOWNSHIFT:
+ return an8855_set_downshift(phydev, *(const u8 *)data);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int an8855_read_page(struct phy_device *phydev)
+{
+ return __phy_read(phydev, AN8855_PHY_SELECT_PAGE);
+}
+
+static int an8855_write_page(struct phy_device *phydev, int page)
+{
+ return __phy_write(phydev, AN8855_PHY_SELECT_PAGE, page);
+}
+
+static struct phy_driver an8855_driver[] = {
+{
+ PHY_ID_MATCH_EXACT(AN8855_PHY_ID),
+ .name = "Airoha AN8855 internal PHY",
+ /* PHY_GBIT_FEATURES */
+ .flags = PHY_IS_INTERNAL,
+ .probe = an8855_probe,
+ .config_init = an8855_config_init,
+ .soft_reset = genphy_soft_reset,
+ .get_tunable = an8855_get_tunable,
+ .set_tunable = an8855_set_tunable,
+ .suspend = genphy_suspend,
+ .resume = genphy_resume,
+ .read_page = an8855_read_page,
+ .write_page = an8855_write_page,
+}, };
+
+module_phy_driver(an8855_driver);
+
+static struct mdio_device_id __maybe_unused an8855_tbl[] = {
+ { PHY_ID_MATCH_EXACT(AN8855_PHY_ID) },
+ { }
+};
+
+MODULE_DEVICE_TABLE(mdio, an8855_tbl);
+
+MODULE_DESCRIPTION("Airoha AN8855 PHY driver");
+MODULE_AUTHOR("Christian Marangi <ansuelsmth@gmail.com>");
+MODULE_LICENSE("GPL");
--
2.45.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 1/4] net: dsa: add devm_dsa_register_switch()
2024-12-05 14:51 ` [net-next PATCH v9 1/4] net: dsa: add devm_dsa_register_switch() Christian Marangi
@ 2024-12-05 16:03 ` Vladimir Oltean
0 siblings, 0 replies; 21+ messages in thread
From: Vladimir Oltean @ 2024-12-05 16:03 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 03:51:31PM +0100, Christian Marangi wrote:
> Some DSA driver can be simplified if devres takes care of unregistering
> the DSA switch. This permits to effectively drop the remove OP from
> driver that just execute the dsa_unregister_switch() and nothing else.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
The premise is false. *No* DSA drivers can safely be simplified if
devres is let to take care of calling dsa_unregister_switch().
See, no remove() method of a DSA driver calls dsa_unregister_switch()
directly, but instead they all test dev_get_drvdata() against NULL first.
See this patch set for a full explanation:
https://lore.kernel.org/netdev/20210917133436.553995-1-vladimir.oltean@nxp.com/
but the short explanation is that the parent bus can implement its own
.shutdown() as .remove(), which for the DSA switch device means that
during shutdown/reboot, both .shutdown() *and* .remove() will be called.
The DSA framework is only prepared for either dsa_unregister_switch()
*or* dsa_switch_shutdown() to be called. It doesn't work if *both* are
called, so we have this mechanism where .shutdown() will set the device
drvdata to NULL, so that .remove() will become a no-op. But that mechanism
will become void if we start to drop the driver's remove() and rely on
devres to call dsa_unregister_switch().
Demo for sja1105 driver with the spi-fsl-dspi.c controller driver as parent.
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index f8454f3b6f9c..b9c92a5e5f5f 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -3404,17 +3404,7 @@ static int sja1105_probe(struct spi_device *spi)
return -ENOMEM;
}
- return dsa_register_switch(priv->ds);
-}
-
-static void sja1105_remove(struct spi_device *spi)
-{
- struct sja1105_private *priv = spi_get_drvdata(spi);
-
- if (!priv)
- return;
-
- dsa_unregister_switch(priv->ds);
+ return devm_dsa_register_switch(dev, priv->ds);
}
static void sja1105_shutdown(struct spi_device *spi)
@@ -3466,7 +3456,6 @@ static struct spi_driver sja1105_driver = {
},
.id_table = sja1105_spi_ids,
.probe = sja1105_probe,
- .remove = sja1105_remove,
.shutdown = sja1105_shutdown,
};
root@debian:~# reboot
[ 52.421866] watchdog: watchdog0: watchdog did not stop!
[ 52.515700] systemd-shutdown[1]: Using hardware watchdog 'sp805-wdt', version 0, device /dev/watchdog0
[ 52.525256] systemd-shutdown[1]: Watchdog running with a timeout of 5min 44s.
[ 52.977392] systemd-shutdown[1]: Syncing filesystems and block devices.
[ 53.041107] systemd-shutdown[1]: Sending SIGTERM to remaining processes...
[ 53.070259] systemd-journald[277]: Received SIGTERM from PID 1 (systemd-shutdow).
[ 53.123590] systemd-shutdown[1]: Sending SIGKILL to remaining processes...
[ 53.156518] systemd-shutdown[1]: Unmounting file systems.
[ 53.170735] (sd-remount)[632]: Remounting '/' read-only with options ''.
[ 53.229253] EXT4-fs (mmcblk0p2): re-mounted e092e674-ed6c-4216-b216-58d8390ae85d ro. Quota mode: none.
[ 53.313634] systemd-shutdown[1]: All filesystems unmounted.
[ 53.319334] systemd-shutdown[1]: Deactivating swaps.
[ 53.324625] systemd-shutdown[1]: All swaps deactivated.
[ 53.329943] systemd-shutdown[1]: Detaching loop devices.
[ 53.342596] systemd-shutdown[1]: All loop devices detached.
[ 53.348263] systemd-shutdown[1]: Stopping MD devices.
[ 53.354180] systemd-shutdown[1]: All MD devices stopped.
[ 53.359633] systemd-shutdown[1]: Detaching DM devices.
[ 53.365699] systemd-shutdown[1]: All DM devices detached.
[ 53.371178] systemd-shutdown[1]: All filesystems, swaps, loop devices, MD devices and DM devices detached.
[ 53.381033] watchdog: watchdog0: watchdog did not stop!
[ 53.424144] systemd-shutdown[1]: Syncing filesystems and block devices.
[ 53.431313] systemd-shutdown[1]: Rebooting.
[ 53.458710] sja1105 spi2.0 sw0p0: Link is Down
[ 53.477004] mscc_felix 0000:00:00.5 swp0: Link is Down
[ 53.486054] fsl_enetc 0000:00:00.2 eno2: Link is Down
[ 53.518865] Unable to handle kernel NULL pointer dereference at virtual address 000000000000002c
[ 53.527921] Mem abort info:
[ 53.530776] ESR = 0x0000000096000004
[ 53.534612] EC = 0x25: DABT (current EL), IL = 32 bits
[ 53.539988] SET = 0, FnV = 0
[ 53.543124] EA = 0, S1PTW = 0
[ 53.546315] FSC = 0x04: level 0 translation fault
[ 53.551282] Data abort info:
[ 53.554211] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[ 53.559792] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 53.564904] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 53.570476] user pgtable: 4k pages, 48-bit VAs, pgdp=0000002083f73000
[ 53.577029] [000000000000002c] pgd=0000000000000000, p4d=0000000000000000
[ 53.584079] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[ 53.590374] Modules linked in:
[ 53.593442] CPU: 0 UID: 0 PID: 1 Comm: systemd-shutdow Tainted: G N 6.12.0-10714-gc118f6e3b41e-dirty #2585
[ 53.604532] Tainted: [N]=TEST
[ 53.613010] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 53.619999] pc : dsa_tree_conduit_admin_state_change+0x44/0xf8
[ 53.625864] lr : dsa_unregister_switch+0x194/0x2a8
[ 53.630674] sp : ffff80008002b940
[ 53.633997] x29: ffff80008002b960 x28: ffff330a40938000 x27: ffff330a40cff818
[ 53.641168] x26: ffffba4f5bb05000 x25: 0000000000000001 x24: ffff330a42e19400
[ 53.648338] x23: ffff330a42e13668 x22: ffff330a435a5890 x21: ffff330a42dc7000
[ 53.655507] x20: ffff330a42e5e080 x19: ffff330a435a5880 x18: 0000000000000000
[ 53.662676] x17: ffffba4f5be23118 x16: ffffba4f5bb0d088 x15: 0000000000000108
[ 53.669845] x14: ffffba4f5c02b550 x13: 0000000000000004 x12: ffff330a40938908
[ 53.677014] x11: ffffba4f5b2ae418 x10: 0000000000000000 x9 : 0000000000000000
[ 53.684183] x8 : 0000000000000000 x7 : ffffba4f5917fbe0 x6 : 0000000000000000
[ 53.691352] x5 : 0000000000000020 x4 : ffff80008002b620 x3 : 0000000000000000
[ 53.698521] x2 : 0000000000000000 x1 : ffff330a42dc7000 x0 : ffff330a435a5880
[ 53.705691] Call trace:
[ 53.708142] dsa_tree_conduit_admin_state_change+0x44/0xf8 (P)
[ 53.714001] dsa_unregister_switch+0x194/0x2a8 (L)
[ 53.718811] dsa_unregister_switch+0x194/0x2a8
[ 53.723272] devm_dsa_unregister_switch+0x1c/0x30
[ 53.727994] devm_action_release+0x20/0x38
[ 53.732107] devres_release_all+0xc4/0x130
[ 53.736217] device_release_driver_internal+0x1d0/0x280
[ 53.741464] device_release_driver+0x24/0x38
[ 53.745751] bus_remove_device+0x154/0x170
[ 53.749862] device_del+0x1f8/0x3e8
[ 53.753361] spi_unregister_device+0x90/0xe8
[ 53.757646] __unregister+0x1c/0x38
[ 53.761147] device_for_each_child+0x6c/0xc8
[ 53.765432] spi_unregister_controller+0x50/0x158
[ 53.770153] dspi_remove+0x28/0x98
[ 53.773567] dspi_shutdown+0x1c/0x30
[ 53.777154] platform_shutdown+0x30/0x48
[ 53.781089] device_shutdown+0x174/0x238
[ 53.785025] kernel_restart+0x4c/0x128
[ 53.788788] __arm64_sys_reboot+0x200/0x2e8
[ 53.792987] invoke_syscall+0x4c/0x110
[ 53.796752] el0_svc_common+0xb8/0xf0
[ 53.800429] do_el0_svc+0x28/0x40
[ 53.803757] el0_svc+0x4c/0xc0
[ 53.806823] el0t_64_sync_handler+0x84/0x108
[ 53.811109] el0t_64_sync+0x198/0x1a0
[ 53.814786] Code: 0a490949 37000489 37b00468 f9421828 (b9402d09)
[ 53.820901] ---[ end trace 0000000000000000 ]---
[ 53.825600] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 53.833306] Kernel Offset: 0x3a4ed7800000 from 0xffff800080000000
[ 53.839420] PHYS_OFFSET: 0xfff0cd1640000000
[ 53.843615] CPU features: 0x080,0002012c,00800000,8200421b
[ 53.849120] Memory Limit: none
[ 53.852184] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
This will need a lot more thought before it makes its appearance as a
tool in the DSA toolbox. Otherwise it is just an avoidable source of
problems.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
[not found] ` <20241205145142.29278-4-ansuelsmth@gmail.com>
@ 2024-12-05 16:12 ` Russell King (Oracle)
2024-12-05 17:44 ` Christian Marangi
2024-12-05 16:27 ` Vladimir Oltean
2024-12-05 17:06 ` Vladimir Oltean
2 siblings, 1 reply; 21+ messages in thread
From: Russell King (Oracle) @ 2024-12-05 16:12 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Matthias Brugger, AngeloGioacchino Del Regno, linux-arm-kernel,
linux-mediatek, netdev, devicetree, linux-kernel, upstream
hi Christian,
On Thu, Dec 05, 2024 at 03:51:33PM +0100, Christian Marangi wrote:
> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> index 2d10b4d6cfbb..6b6d0b7bae72 100644
> --- a/drivers/net/dsa/Kconfig
> +++ b/drivers/net/dsa/Kconfig
> @@ -24,6 +24,15 @@ config NET_DSA_LOOP
> This enables support for a fake mock-up switch chip which
> exercises the DSA APIs.
>
> +
Niggle - no need for the extra blank line.
> +static int an8855_set_mac_eee(struct dsa_switch *ds, int port,
> + struct ethtool_keee *eee)
> +{
> + struct an8855_priv *priv = ds->priv;
> + u32 reg;
> + int ret;
> +
> + if (eee->eee_enabled) {
> + ret = regmap_read(priv->regmap, AN8855_PMCR_P(port), ®);
> + if (ret)
> + return ret;
> + /* Force enable EEE if force mode and LINK */
> + if (reg & AN8855_PMCR_FORCE_MODE &&
> + reg & AN8855_PMCR_FORCE_LNK) {
> + switch (reg & AN8855_PMCR_FORCE_SPEED) {
> + case AN8855_PMCR_FORCE_SPEED_1000:
> + reg |= AN8855_PMCR_FORCE_EEE1G;
> + break;
> + case AN8855_PMCR_FORCE_SPEED_100:
> + reg |= AN8855_PMCR_FORCE_EEE100;
> + break;
> + default:
> + break;
> + }
Does this forcing force EEE to be enabled for the port, irrespective
of the negotiation?
> + ret = regmap_write(priv->regmap, AN8855_PMCR_P(port), reg);
> + if (ret)
> + return ret;
> + }
> + ret = regmap_update_bits(priv->regmap, AN8855_PMEEECR_P(port),
> + AN8855_LPI_MODE_EN,
> + eee->tx_lpi_enabled ? AN8855_LPI_MODE_EN : 0);
> + if (ret)
> + return ret;
> + } else {
> + ret = regmap_clear_bits(priv->regmap, AN8855_PMCR_P(port),
> + AN8855_PMCR_FORCE_EEE1G |
> + AN8855_PMCR_FORCE_EEE100);
> + if (ret)
> + return ret;
> +
> + ret = regmap_clear_bits(priv->regmap, AN8855_PMEEECR_P(port),
> + AN8855_LPI_MODE_EN);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int an8855_get_mac_eee(struct dsa_switch *ds, int port,
> + struct ethtool_keee *eee)
> +{
> + struct an8855_priv *priv = ds->priv;
> + u32 reg;
> + int ret;
> +
> + ret = regmap_read(priv->regmap, AN8855_PMEEECR_P(port), ®);
> + if (ret)
> + return ret;
> + eee->tx_lpi_enabled = reg & AN8855_LPI_MODE_EN;
> +
> + ret = regmap_read(priv->regmap, AN8855_CKGCR, ®);
> + if (ret)
> + return ret;
> + /* Global LPI TXIDLE Threshold, default 60ms (unit 2us) */
> + eee->tx_lpi_timer = FIELD_GET(AN8855_LPI_TXIDLE_THD_MASK, reg) / 500;
There is no point setting tx_lpi_enabled and tx_lpi_timer, as phylib
will immediately overwrite then in its phy_ethtool_get_eee() method.
In fact, there's no point to the DSA get_mac_eee() method.
> +
> + ret = regmap_read(priv->regmap, AN8855_PMSR_P(port), ®);
> + if (ret)
> + return ret;
What's the purpose of this read?
> +
> + return 0;
> +}
Overall, I would suggest not implementing EEE just yet - I sent out a
22 patch RFC patch set last week which implements EEE support in
phylink, and I have another bunch of patches that clean up DSA that
I didn't send, which includes getting rid of the DSA get_mac_eee()
method.
Now that the bulk of the phylink in-band changes have been merged, my
plan is to work through getting the RFC patch set merged - I've sent
out the first four patches today for final review and merging.
I stated some questions in the RFC cover message that everyone ignored,
maybe you could look at that and give your views on the points I raised
there?
> +static struct phylink_pcs *
> +an8855_phylink_mac_select_pcs(struct phylink_config *config,
> + phy_interface_t interface)
> +{
> + struct dsa_port *dp = dsa_phylink_to_port(config);
> + struct an8855_priv *priv = dp->ds->priv;
> +
> + switch (interface) {
> + case PHY_INTERFACE_MODE_SGMII:
> + case PHY_INTERFACE_MODE_2500BASEX:
> + return &priv->pcs;
> + default:
> + return NULL;
> + }
Great, exactly what I like to see in mac_select_pcs()!
> +}
> +
> +static void
> +an8855_phylink_mac_config(struct phylink_config *config, unsigned int mode,
> + const struct phylink_link_state *state)
> +{
> + struct dsa_port *dp = dsa_phylink_to_port(config);
> + struct dsa_switch *ds = dp->ds;
> + struct an8855_priv *priv;
> + int port = dp->index;
> +
> + priv = ds->priv;
> +
> + /* Nothing to configure for internal ports */
> + if (port != 5)
> + return;
> +
> + regmap_update_bits(priv->regmap, AN8855_PMCR_P(port),
> + AN8855_PMCR_IFG_XMIT | AN8855_PMCR_MAC_MODE |
> + AN8855_PMCR_BACKOFF_EN | AN8855_PMCR_BACKPR_EN,
> + FIELD_PREP(AN8855_PMCR_IFG_XMIT, 0x1) |
> + AN8855_PMCR_MAC_MODE | AN8855_PMCR_BACKOFF_EN |
> + AN8855_PMCR_BACKPR_EN);
> +}
This looks much better, thanks.
> +
> +static void an8855_phylink_get_caps(struct dsa_switch *ds, int port,
> + struct phylink_config *config)
> +{
> + switch (port) {
> + case 0:
> + case 1:
> + case 2:
> + case 3:
> + case 4:
> + __set_bit(PHY_INTERFACE_MODE_GMII,
> + config->supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_INTERNAL,
> + config->supported_interfaces);
> + break;
> + case 5:
> + phy_interface_set_rgmii(config->supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_SGMII,
> + config->supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_2500BASEX,
> + config->supported_interfaces);
> + break;
> + }
> +
> + config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> + MAC_10 | MAC_100 | MAC_1000FD | MAC_2500FD;
> +}
> +
> +static void
> +an8855_phylink_mac_link_down(struct phylink_config *config, unsigned int mode,
> + phy_interface_t interface)
> +{
> + struct dsa_port *dp = dsa_phylink_to_port(config);
> + struct an8855_priv *priv = dp->ds->priv;
> +
> + /* With autoneg just disable TX/RX else also force link down */
> + if (phylink_autoneg_inband(mode)) {
> + regmap_clear_bits(priv->regmap, AN8855_PMCR_P(dp->index),
> + AN8855_PMCR_TX_EN | AN8855_PMCR_RX_EN);
> + } else {
> + regmap_update_bits(priv->regmap, AN8855_PMCR_P(dp->index),
> + AN8855_PMCR_TX_EN | AN8855_PMCR_RX_EN |
> + AN8855_PMCR_FORCE_MODE | AN8855_PMCR_FORCE_LNK,
> + AN8855_PMCR_FORCE_MODE);
> + }
> +}
> +
> +static void
> +an8855_phylink_mac_link_up(struct phylink_config *config,
> + struct phy_device *phydev, unsigned int mode,
> + phy_interface_t interface, int speed, int duplex,
> + bool tx_pause, bool rx_pause)
> +{
> + struct dsa_port *dp = dsa_phylink_to_port(config);
> + struct an8855_priv *priv = dp->ds->priv;
> + int port = dp->index;
> + u32 reg;
> +
> + reg = regmap_read(priv->regmap, AN8855_PMCR_P(port), ®);
> + if (phylink_autoneg_inband(mode)) {
> + reg &= ~AN8855_PMCR_FORCE_MODE;
> + } else {
> + reg |= AN8855_PMCR_FORCE_MODE | AN8855_PMCR_FORCE_LNK;
> +
> + reg &= ~AN8855_PMCR_FORCE_SPEED;
> + switch (speed) {
> + case SPEED_10:
> + reg |= AN8855_PMCR_FORCE_SPEED_10;
> + break;
> + case SPEED_100:
> + reg |= AN8855_PMCR_FORCE_SPEED_100;
> + break;
> + case SPEED_1000:
> + reg |= AN8855_PMCR_FORCE_SPEED_1000;
> + break;
> + case SPEED_2500:
> + reg |= AN8855_PMCR_FORCE_SPEED_2500;
> + break;
> + case SPEED_5000:
> + dev_err(priv->dev, "Missing support for 5G speed. Aborting...\n");
> + return;
> + }
> +
> + reg &= ~AN8855_PMCR_FORCE_FDX;
> + if (duplex == DUPLEX_FULL)
> + reg |= AN8855_PMCR_FORCE_FDX;
> +
> + reg &= ~AN8855_PMCR_RX_FC_EN;
> + if (rx_pause || dsa_port_is_cpu(dp))
> + reg |= AN8855_PMCR_RX_FC_EN;
> +
> + reg &= ~AN8855_PMCR_TX_FC_EN;
> + if (rx_pause || dsa_port_is_cpu(dp))
> + reg |= AN8855_PMCR_TX_FC_EN;
> +
> + /* Disable any EEE options */
> + reg &= ~(AN8855_PMCR_FORCE_EEE5G | AN8855_PMCR_FORCE_EEE2P5G |
> + AN8855_PMCR_FORCE_EEE1G | AN8855_PMCR_FORCE_EEE100);
> + }
> +
> + reg |= AN8855_PMCR_TX_EN | AN8855_PMCR_RX_EN;
> +
> + regmap_write(priv->regmap, AN8855_PMCR_P(port), reg);
> +}
All the above looks fine to me.
> +
> +static void an8855_pcs_get_state(struct phylink_pcs *pcs,
> + struct phylink_link_state *state)
> +{
> + struct an8855_priv *priv = container_of(pcs, struct an8855_priv, pcs);
> + u32 val;
> + int ret;
> +
> + ret = regmap_read(priv->regmap, AN8855_PMSR_P(AN8855_CPU_PORT), &val);
> + if (ret < 0) {
> + state->link = false;
> + return;
> + }
> +
> + state->link = !!(val & AN8855_PMSR_LNK);
> + state->an_complete = state->link;
> + state->duplex = (val & AN8855_PMSR_DPX) ? DUPLEX_FULL :
> + DUPLEX_HALF;
> +
> + switch (val & AN8855_PMSR_SPEED) {
> + case AN8855_PMSR_SPEED_10:
> + state->speed = SPEED_10;
> + break;
> + case AN8855_PMSR_SPEED_100:
> + state->speed = SPEED_100;
> + break;
> + case AN8855_PMSR_SPEED_1000:
> + state->speed = SPEED_1000;
> + break;
> + case AN8855_PMSR_SPEED_2500:
> + state->speed = SPEED_2500;
> + break;
> + case AN8855_PMSR_SPEED_5000:
> + dev_err(priv->dev, "Missing support for 5G speed. Setting Unknown.\n");
> + fallthrough;
> + default:
> + state->speed = SPEED_UNKNOWN;
> + break;
> + }
> +
> + if (val & AN8855_PMSR_RX_FC)
> + state->pause |= MLO_PAUSE_RX;
> + if (val & AN8855_PMSR_TX_FC)
> + state->pause |= MLO_PAUSE_TX;
> +}
> +
> +static int an8855_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
> + phy_interface_t interface,
> + const unsigned long *advertising,
> + bool permit_pause_to_mac)
> +{
> + struct an8855_priv *priv = container_of(pcs, struct an8855_priv, pcs);
> + u32 val;
> + int ret;
> +
> + switch (interface) {
> + case PHY_INTERFACE_MODE_SGMII:
> + break;
> + case PHY_INTERFACE_MODE_2500BASEX:
> + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
> + dev_err(priv->dev, "in-band negotiation unsupported");
> + return -EINVAL;
> + }
> + break;
Now that we have the phylink in-band changes merged, along with a new
PCS .pcs_inband_caps() method, please implement this method to indicate
that in-band is not supported in 2500BASE-X mode. Phylink will issue a
warning if it attempts to go against that (particularly when e.g. the
PHY demands in-band but the PCS doesn't support it - a case that likely
will never work in practice.)
> + default:
> + return -EINVAL;
> + }
> +
> + /* !!! WELCOME TO HELL !!! */
> +
> + /* TX FIR - improve TX EYE */
> + ret = regmap_update_bits(priv->regmap, AN8855_INTF_CTRL_10,
> + AN8855_RG_DA_QP_TX_FIR_C2_SEL |
> + AN8855_RG_DA_QP_TX_FIR_C2_FORCE |
> + AN8855_RG_DA_QP_TX_FIR_C1_SEL |
> + AN8855_RG_DA_QP_TX_FIR_C1_FORCE,
> + AN8855_RG_DA_QP_TX_FIR_C2_SEL |
> + FIELD_PREP(AN8855_RG_DA_QP_TX_FIR_C2_FORCE, 0x4) |
> + AN8855_RG_DA_QP_TX_FIR_C1_SEL |
> + FIELD_PREP(AN8855_RG_DA_QP_TX_FIR_C1_FORCE, 0x0));
> + if (ret)
> + return ret;
> +
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = 0x0;
> + else
> + val = 0xd;
> + ret = regmap_update_bits(priv->regmap, AN8855_INTF_CTRL_11,
> + AN8855_RG_DA_QP_TX_FIR_C0B_SEL |
> + AN8855_RG_DA_QP_TX_FIR_C0B_FORCE,
> + AN8855_RG_DA_QP_TX_FIR_C0B_SEL |
> + FIELD_PREP(AN8855_RG_DA_QP_TX_FIR_C0B_FORCE, val));
> + if (ret)
> + return ret;
> +
> + /* RX CDR - improve RX Jitter Tolerance */
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = 0x5;
> + else
> + val = 0x6;
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_LPF_BOT_LIM,
> + AN8855_RG_QP_CDR_LPF_KP_GAIN |
> + AN8855_RG_QP_CDR_LPF_KI_GAIN,
> + FIELD_PREP(AN8855_RG_QP_CDR_LPF_KP_GAIN, val) |
> + FIELD_PREP(AN8855_RG_QP_CDR_LPF_KI_GAIN, val));
> + if (ret)
> + return ret;
> +
> + /* PLL */
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = 0x1;
> + else
> + val = 0x0;
> + ret = regmap_update_bits(priv->regmap, AN8855_QP_DIG_MODE_CTRL_1,
> + AN8855_RG_TPHY_SPEED,
> + FIELD_PREP(AN8855_RG_TPHY_SPEED, val));
> + if (ret)
> + return ret;
> +
> + /* PLL - LPF */
> + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> + AN8855_RG_DA_QP_PLL_RICO_SEL_INTF |
> + AN8855_RG_DA_QP_PLL_FBKSEL_INTF |
> + AN8855_RG_DA_QP_PLL_BR_INTF |
> + AN8855_RG_DA_QP_PLL_BPD_INTF |
> + AN8855_RG_DA_QP_PLL_BPA_INTF |
> + AN8855_RG_DA_QP_PLL_BC_INTF,
> + AN8855_RG_DA_QP_PLL_RICO_SEL_INTF |
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_FBKSEL_INTF, 0x0) |
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_BR_INTF, 0x3) |
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_BPD_INTF, 0x0) |
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_BPA_INTF, 0x5) |
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_BC_INTF, 0x1));
> + if (ret)
> + return ret;
> +
> + /* PLL - ICO */
> + ret = regmap_set_bits(priv->regmap, AN8855_PLL_CTRL_4,
> + AN8855_RG_DA_QP_PLL_ICOLP_EN_INTF);
> + if (ret)
> + return ret;
> + ret = regmap_clear_bits(priv->regmap, AN8855_PLL_CTRL_2,
> + AN8855_RG_DA_QP_PLL_ICOIQ_EN_INTF);
> + if (ret)
> + return ret;
> +
> + /* PLL - CHP */
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = 0x6;
> + else
> + val = 0x4;
> + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> + AN8855_RG_DA_QP_PLL_IR_INTF,
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_IR_INTF, val));
> + if (ret)
> + return ret;
> +
> + /* PLL - PFD */
> + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> + AN8855_RG_DA_QP_PLL_PFD_OFFSET_EN_INTRF |
> + AN8855_RG_DA_QP_PLL_PFD_OFFSET_INTF |
> + AN8855_RG_DA_QP_PLL_KBAND_PREDIV_INTF,
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_PFD_OFFSET_INTF, 0x1) |
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_KBAND_PREDIV_INTF, 0x1));
> + if (ret)
> + return ret;
> +
> + /* PLL - POSTDIV */
> + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> + AN8855_RG_DA_QP_PLL_POSTDIV_EN_INTF |
> + AN8855_RG_DA_QP_PLL_PHY_CK_EN_INTF |
> + AN8855_RG_DA_QP_PLL_PCK_SEL_INTF,
> + AN8855_RG_DA_QP_PLL_PCK_SEL_INTF);
> + if (ret)
> + return ret;
> +
> + /* PLL - SDM */
> + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> + AN8855_RG_DA_QP_PLL_SDM_HREN_INTF,
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_SDM_HREN_INTF, 0x0));
> + if (ret)
> + return ret;
> + ret = regmap_clear_bits(priv->regmap, AN8855_PLL_CTRL_2,
> + AN8855_RG_DA_QP_PLL_SDM_IFM_INTF);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_SS_LCPLL_PWCTL_SETTING_2,
> + AN8855_RG_NCPO_ANA_MSB,
> + FIELD_PREP(AN8855_RG_NCPO_ANA_MSB, 0x1));
> + if (ret)
> + return ret;
> +
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = 0x7a000000;
> + else
> + val = 0x48000000;
> + ret = regmap_write(priv->regmap, AN8855_SS_LCPLL_TDC_FLT_2,
> + FIELD_PREP(AN8855_RG_LCPLL_NCPO_VALUE, val));
> + if (ret)
> + return ret;
> + ret = regmap_write(priv->regmap, AN8855_SS_LCPLL_TDC_PCW_1,
> + FIELD_PREP(AN8855_RG_LCPLL_PON_HRDDS_PCW_NCPO_GPON, val));
> + if (ret)
> + return ret;
> +
> + ret = regmap_clear_bits(priv->regmap, AN8855_SS_LCPLL_TDC_FLT_5,
> + AN8855_RG_LCPLL_NCPO_CHG);
> + if (ret)
> + return ret;
> + ret = regmap_clear_bits(priv->regmap, AN8855_PLL_CK_CTRL_0,
> + AN8855_RG_DA_QP_PLL_SDM_DI_EN_INTF);
> + if (ret)
> + return ret;
> +
> + /* PLL - SS */
> + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_3,
> + AN8855_RG_DA_QP_PLL_SSC_DELTA_INTF,
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_SSC_DELTA_INTF, 0x0));
> + if (ret)
> + return ret;
> + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_4,
> + AN8855_RG_DA_QP_PLL_SSC_DIR_DLY_INTF,
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_SSC_DIR_DLY_INTF, 0x0));
> + if (ret)
> + return ret;
> + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_3,
> + AN8855_RG_DA_QP_PLL_SSC_PERIOD_INTF,
> + FIELD_PREP(AN8855_RG_DA_QP_PLL_SSC_PERIOD_INTF, 0x0));
> + if (ret)
> + return ret;
> +
> + /* PLL - TDC */
> + ret = regmap_clear_bits(priv->regmap, AN8855_PLL_CK_CTRL_0,
> + AN8855_RG_DA_QP_PLL_TDC_TXCK_SEL_INTF);
> + if (ret)
> + return ret;
> +
> + ret = regmap_set_bits(priv->regmap, AN8855_RG_QP_PLL_SDM_ORD,
> + AN8855_RG_QP_PLL_SSC_TRI_EN);
> + if (ret)
> + return ret;
> + ret = regmap_set_bits(priv->regmap, AN8855_RG_QP_PLL_SDM_ORD,
> + AN8855_RG_QP_PLL_SSC_PHASE_INI);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_RX_DAC_EN,
> + AN8855_RG_QP_SIGDET_HF,
> + FIELD_PREP(AN8855_RG_QP_SIGDET_HF, 0x2));
> + if (ret)
> + return ret;
> +
> + /* TCL Disable (only for Co-SIM) */
> + ret = regmap_clear_bits(priv->regmap, AN8855_PON_RXFEDIG_CTRL_0,
> + AN8855_RG_QP_EQ_RX500M_CK_SEL);
> + if (ret)
> + return ret;
> +
> + /* TX Init */
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = 0x4;
> + else
> + val = 0x0;
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_TX_MODE,
> + AN8855_RG_QP_TX_RESERVE |
> + AN8855_RG_QP_TX_MODE_16B_EN,
> + FIELD_PREP(AN8855_RG_QP_TX_RESERVE, val));
> + if (ret)
> + return ret;
> +
> + /* RX Control/Init */
> + ret = regmap_set_bits(priv->regmap, AN8855_RG_QP_RXAFE_RESERVE,
> + AN8855_RG_QP_CDR_PD_10B_EN);
> + if (ret)
> + return ret;
> +
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = 0x1;
> + else
> + val = 0x2;
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_LPF_MJV_LIM,
> + AN8855_RG_QP_CDR_LPF_RATIO,
> + FIELD_PREP(AN8855_RG_QP_CDR_LPF_RATIO, val));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_LPF_SETVALUE,
> + AN8855_RG_QP_CDR_PR_BUF_IN_SR |
> + AN8855_RG_QP_CDR_PR_BETA_SEL,
> + FIELD_PREP(AN8855_RG_QP_CDR_PR_BUF_IN_SR, 0x6) |
> + FIELD_PREP(AN8855_RG_QP_CDR_PR_BETA_SEL, 0x1));
> + if (ret)
> + return ret;
> +
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = 0xf;
> + else
> + val = 0xc;
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_PR_CKREF_DIV1,
> + AN8855_RG_QP_CDR_PR_DAC_BAND,
> + FIELD_PREP(AN8855_RG_QP_CDR_PR_DAC_BAND, val));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_PR_KBAND_DIV_PCIE,
> + AN8855_RG_QP_CDR_PR_KBAND_PCIE_MODE |
> + AN8855_RG_QP_CDR_PR_KBAND_DIV_PCIE_MASK,
> + FIELD_PREP(AN8855_RG_QP_CDR_PR_KBAND_DIV_PCIE_MASK, 0x19));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_FORCE_IBANDLPF_R_OFF,
> + AN8855_RG_QP_CDR_PHYCK_SEL |
> + AN8855_RG_QP_CDR_PHYCK_RSTB |
> + AN8855_RG_QP_CDR_PHYCK_DIV,
> + FIELD_PREP(AN8855_RG_QP_CDR_PHYCK_SEL, 0x2) |
> + FIELD_PREP(AN8855_RG_QP_CDR_PHYCK_DIV, 0x21));
> + if (ret)
> + return ret;
> +
> + ret = regmap_clear_bits(priv->regmap, AN8855_RG_QP_CDR_PR_KBAND_DIV_PCIE,
> + AN8855_RG_QP_CDR_PR_XFICK_EN);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_PR_CKREF_DIV1,
> + AN8855_RG_QP_CDR_PR_KBAND_DIV,
> + FIELD_PREP(AN8855_RG_QP_CDR_PR_KBAND_DIV, 0x4));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_26,
> + AN8855_RG_QP_EQ_RETRAIN_ONLY_EN |
> + AN8855_RG_LINK_NE_EN |
> + AN8855_RG_LINK_ERRO_EN,
> + AN8855_RG_QP_EQ_RETRAIN_ONLY_EN |
> + AN8855_RG_LINK_ERRO_EN);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RX_DLY_0,
> + AN8855_RG_QP_RX_SAOSC_EN_H_DLY |
> + AN8855_RG_QP_RX_PI_CAL_EN_H_DLY,
> + FIELD_PREP(AN8855_RG_QP_RX_SAOSC_EN_H_DLY, 0x3f) |
> + FIELD_PREP(AN8855_RG_QP_RX_PI_CAL_EN_H_DLY, 0x6f));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_42,
> + AN8855_RG_QP_EQ_EN_DLY,
> + FIELD_PREP(AN8855_RG_QP_EQ_EN_DLY, 0x150));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_2,
> + AN8855_RG_QP_RX_EQ_EN_H_DLY,
> + FIELD_PREP(AN8855_RG_QP_RX_EQ_EN_H_DLY, 0x150));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_PON_RXFEDIG_CTRL_9,
> + AN8855_RG_QP_EQ_LEQOSC_DLYCNT,
> + FIELD_PREP(AN8855_RG_QP_EQ_LEQOSC_DLYCNT, 0x1));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_8,
> + AN8855_RG_DA_QP_SAOSC_DONE_TIME |
> + AN8855_RG_DA_QP_LEQOS_EN_TIME,
> + FIELD_PREP(AN8855_RG_DA_QP_SAOSC_DONE_TIME, 0x200) |
> + FIELD_PREP(AN8855_RG_DA_QP_LEQOS_EN_TIME, 0xfff));
> + if (ret)
> + return ret;
> +
> + /* Frequency meter */
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = 0x10;
> + else
> + val = 0x28;
> + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_5,
> + AN8855_RG_FREDET_CHK_CYCLE,
> + FIELD_PREP(AN8855_RG_FREDET_CHK_CYCLE, val));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_6,
> + AN8855_RG_FREDET_GOLDEN_CYCLE,
> + FIELD_PREP(AN8855_RG_FREDET_GOLDEN_CYCLE, 0x64));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_7,
> + AN8855_RG_FREDET_TOLERATE_CYCLE,
> + FIELD_PREP(AN8855_RG_FREDET_TOLERATE_CYCLE, 0x2710));
> + if (ret)
> + return ret;
> +
> + ret = regmap_set_bits(priv->regmap, AN8855_PLL_CTRL_0,
> + AN8855_RG_PHYA_AUTO_INIT);
> + if (ret)
> + return ret;
> +
> + /* PCS Init */
> + if (interface == PHY_INTERFACE_MODE_SGMII &&
> + neg_mode == PHYLINK_PCS_NEG_INBAND_DISABLED) {
> + ret = regmap_clear_bits(priv->regmap, AN8855_QP_DIG_MODE_CTRL_0,
> + AN8855_RG_SGMII_MODE | AN8855_RG_SGMII_AN_EN);
> + if (ret)
> + return ret;
> + }
> +
> + ret = regmap_clear_bits(priv->regmap, AN8855_RG_HSGMII_PCS_CTROL_1,
> + AN8855_RG_TBI_10B_MODE);
> + if (ret)
> + return ret;
> +
> + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
> + /* Set AN Ability - Interrupt */
> + ret = regmap_set_bits(priv->regmap, AN8855_SGMII_REG_AN_FORCE_CL37,
> + AN8855_RG_FORCE_AN_DONE);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap, AN8855_SGMII_REG_AN_13,
> + AN8855_SGMII_REMOTE_FAULT_DIS |
> + AN8855_SGMII_IF_MODE,
> + AN8855_SGMII_REMOTE_FAULT_DIS |
> + FIELD_PREP(AN8855_SGMII_IF_MODE, 0xb));
> + if (ret)
> + return ret;
> + }
> +
> + /* Rate Adaption - GMII path config. */
> + if (interface == PHY_INTERFACE_MODE_2500BASEX) {
> + ret = regmap_clear_bits(priv->regmap, AN8855_RATE_ADP_P0_CTRL_0,
> + AN8855_RG_P0_DIS_MII_MODE);
> + if (ret)
> + return ret;
> + } else {
> + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
> + ret = regmap_set_bits(priv->regmap, AN8855_MII_RA_AN_ENABLE,
> + AN8855_RG_P0_RA_AN_EN);
> + if (ret)
> + return ret;
> + } else {
> + ret = regmap_update_bits(priv->regmap, AN8855_RG_AN_SGMII_MODE_FORCE,
> + AN8855_RG_FORCE_CUR_SGMII_MODE |
> + AN8855_RG_FORCE_CUR_SGMII_SEL,
> + AN8855_RG_FORCE_CUR_SGMII_SEL);
> + if (ret)
> + return ret;
> +
> + ret = regmap_clear_bits(priv->regmap, AN8855_RATE_ADP_P0_CTRL_0,
> + AN8855_RG_P0_MII_RA_RX_EN |
> + AN8855_RG_P0_MII_RA_TX_EN |
> + AN8855_RG_P0_MII_RA_RX_MODE |
> + AN8855_RG_P0_MII_RA_TX_MODE);
> + if (ret)
> + return ret;
> + }
> +
> + ret = regmap_set_bits(priv->regmap, AN8855_RATE_ADP_P0_CTRL_0,
> + AN8855_RG_P0_MII_MODE);
> + if (ret)
> + return ret;
> + }
> +
> + ret = regmap_set_bits(priv->regmap, AN8855_RG_RATE_ADAPT_CTRL_0,
> + AN8855_RG_RATE_ADAPT_RX_BYPASS |
> + AN8855_RG_RATE_ADAPT_TX_BYPASS |
> + AN8855_RG_RATE_ADAPT_RX_EN |
> + AN8855_RG_RATE_ADAPT_TX_EN);
> + if (ret)
> + return ret;
> +
> + /* Disable AN if not in autoneg */
> + ret = regmap_update_bits(priv->regmap, AN8855_SGMII_REG_AN0, BMCR_ANENABLE,
> + neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED ? BMCR_ANENABLE :
> + 0);
> + if (ret)
> + return ret;
> +
> + if (interface == PHY_INTERFACE_MODE_SGMII &&
> + neg_mode == PHYLINK_PCS_NEG_INBAND_DISABLED) {
> + ret = regmap_set_bits(priv->regmap, AN8855_PHY_RX_FORCE_CTRL_0,
> + AN8855_RG_FORCE_TXC_SEL);
> + if (ret)
> + return ret;
> + }
> +
> + /* Force Speed with fixed-link or 2500base-x as doesn't support aneg */
> + if (interface == PHY_INTERFACE_MODE_2500BASEX ||
> + neg_mode != PHYLINK_PCS_NEG_INBAND_ENABLED) {
> + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> + val = AN8855_RG_LINK_MODE_P0_SPEED_2500;
> + else
> + val = AN8855_RG_LINK_MODE_P0_SPEED_1000;
> + ret = regmap_update_bits(priv->regmap, AN8855_SGMII_STS_CTRL_0,
> + AN8855_RG_LINK_MODE_P0 |
> + AN8855_RG_FORCE_SPD_MODE_P0,
> + val | AN8855_RG_FORCE_SPD_MODE_P0);
> + if (ret)
> + return ret;
> + }
> +
> + /* bypass flow control to MAC */
> + ret = regmap_write(priv->regmap, AN8855_MSG_RX_LIK_STS_0,
> + AN8855_RG_DPX_STS_P3 | AN8855_RG_DPX_STS_P2 |
> + AN8855_RG_DPX_STS_P1 | AN8855_RG_TXFC_STS_P0 |
> + AN8855_RG_RXFC_STS_P0 | AN8855_RG_DPX_STS_P0);
> + if (ret)
> + return ret;
> + ret = regmap_write(priv->regmap, AN8855_MSG_RX_LIK_STS_2,
> + AN8855_RG_RXFC_AN_BYPASS_P3 |
> + AN8855_RG_RXFC_AN_BYPASS_P2 |
> + AN8855_RG_RXFC_AN_BYPASS_P1 |
> + AN8855_RG_TXFC_AN_BYPASS_P3 |
> + AN8855_RG_TXFC_AN_BYPASS_P2 |
> + AN8855_RG_TXFC_AN_BYPASS_P1 |
> + AN8855_RG_DPX_AN_BYPASS_P3 |
> + AN8855_RG_DPX_AN_BYPASS_P2 |
> + AN8855_RG_DPX_AN_BYPASS_P1 |
> + AN8855_RG_DPX_AN_BYPASS_P0);
> + if (ret)
> + return ret;
Is the above disruptive to the link if it is executed when the link is
already up? Do you need to re-execute it even when "interface" hasn't
changed?
This gets called to possibly change the PCS advertisement in response
to ethtool -s, so ought not have any link disrupting effects if
nothing has changed.
> +
> + return 0;
> +}
> +
> +static void an8855_pcs_an_restart(struct phylink_pcs *pcs)
> +{
> + struct an8855_priv *priv = container_of(pcs, struct an8855_priv, pcs);
> +
> + regmap_set_bits(priv->regmap, AN8855_SGMII_REG_AN0, BMCR_ANRESTART);
This won't get called for SGMII - Cisco SGMII is merely the PHY telling
the PCS/MAC what was negotiated at the media and how it's going to be
operating the SGMII link. AN restart is a media side thing, so that
would happen at the PHY in that case. I suggest this becomes an empty
no-op function.
Thanks!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
[not found] ` <20241205145142.29278-4-ansuelsmth@gmail.com>
2024-12-05 16:12 ` [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver Russell King (Oracle)
@ 2024-12-05 16:27 ` Vladimir Oltean
2024-12-05 17:17 ` Christian Marangi
2024-12-05 17:06 ` Vladimir Oltean
2 siblings, 1 reply; 21+ messages in thread
From: Vladimir Oltean @ 2024-12-05 16:27 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 03:51:33PM +0100, Christian Marangi wrote:
> +static int an8855_efuse_read(void *context, unsigned int offset,
> + void *val, size_t bytes)
> +{
> + struct an8855_priv *priv = context;
> +
> + return regmap_bulk_read(priv->regmap, AN8855_EFUSE_DATA0 + offset,
> + val, bytes / sizeof(u32));
> +}
> +
> +static struct nvmem_config an8855_nvmem_config = {
> + .name = "an8855-efuse",
> + .size = AN8855_EFUSE_CELL * sizeof(u32),
> + .stride = sizeof(u32),
> + .word_size = sizeof(u32),
> + .reg_read = an8855_efuse_read,
> +};
> +
> +static int an8855_sw_register_nvmem(struct an8855_priv *priv)
> +{
> + struct nvmem_device *nvmem;
> +
> + an8855_nvmem_config.priv = priv;
> + an8855_nvmem_config.dev = priv->dev;
> + nvmem = devm_nvmem_register(priv->dev, &an8855_nvmem_config);
> + if (IS_ERR(nvmem))
> + return PTR_ERR(nvmem);
> +
> + return 0;
> +}
At some point we should enforce the rule that new drivers for switch
SoCs with complex peripherals should use MFD and move all non-networking
peripherals to drivers handled by their respective subsystems.
I don't have the expertise to review a nvmem driver, and the majority of
them are in drivers/nvmem, with a dedicated subsystem and maintainer.
In general I want to make sure it is clear that I don't encourage the
model where DSA owns the entire mdio_device.
What other peripherals are there on this SoC other than an MDIO bus and
an EFUSE? IRQCHIP, GPIOs, LED controller, sensors?
You can take a look at drivers/mfd/ocelot* and
Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml for an example on
how to use mfd for the top-level MDIO device, and DSA as just the driver
for the Ethernet switch component (which will be represented as a
platform_device).
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
[not found] ` <20241205145142.29278-4-ansuelsmth@gmail.com>
2024-12-05 16:12 ` [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver Russell King (Oracle)
2024-12-05 16:27 ` Vladimir Oltean
@ 2024-12-05 17:06 ` Vladimir Oltean
2024-12-05 17:26 ` Christian Marangi
2 siblings, 1 reply; 21+ messages in thread
From: Vladimir Oltean @ 2024-12-05 17:06 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 03:51:33PM +0100, Christian Marangi wrote:
> + .port_fdb_add = an8855_port_fdb_add,
> + .port_fdb_del = an8855_port_fdb_del,
> + .port_fdb_dump = an8855_port_fdb_dump,
> + .port_mdb_add = an8855_port_mdb_add,
> + .port_mdb_del = an8855_port_mdb_del,
Please handle the "struct dsa_db" argument of these functions, so that
you can turn on ds->fdb_isolation. It is likely that instead of a single
AN8855_FID_BRIDGED, there needs to be a unique FID allocated for each
VLAN-unaware bridge in order for their FDBs to be isolated from each
other, and so that the same MAC address could live under both bridges.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 16:27 ` Vladimir Oltean
@ 2024-12-05 17:17 ` Christian Marangi
2024-12-05 18:05 ` Vladimir Oltean
0 siblings, 1 reply; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 17:17 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 06:27:59PM +0200, Vladimir Oltean wrote:
> On Thu, Dec 05, 2024 at 03:51:33PM +0100, Christian Marangi wrote:
> > +static int an8855_efuse_read(void *context, unsigned int offset,
> > + void *val, size_t bytes)
> > +{
> > + struct an8855_priv *priv = context;
> > +
> > + return regmap_bulk_read(priv->regmap, AN8855_EFUSE_DATA0 + offset,
> > + val, bytes / sizeof(u32));
> > +}
> > +
> > +static struct nvmem_config an8855_nvmem_config = {
> > + .name = "an8855-efuse",
> > + .size = AN8855_EFUSE_CELL * sizeof(u32),
> > + .stride = sizeof(u32),
> > + .word_size = sizeof(u32),
> > + .reg_read = an8855_efuse_read,
> > +};
> > +
> > +static int an8855_sw_register_nvmem(struct an8855_priv *priv)
> > +{
> > + struct nvmem_device *nvmem;
> > +
> > + an8855_nvmem_config.priv = priv;
> > + an8855_nvmem_config.dev = priv->dev;
> > + nvmem = devm_nvmem_register(priv->dev, &an8855_nvmem_config);
> > + if (IS_ERR(nvmem))
> > + return PTR_ERR(nvmem);
> > +
> > + return 0;
> > +}
>
> At some point we should enforce the rule that new drivers for switch
> SoCs with complex peripherals should use MFD and move all non-networking
> peripherals to drivers handled by their respective subsystems.
>
> I don't have the expertise to review a nvmem driver, and the majority of
> them are in drivers/nvmem, with a dedicated subsystem and maintainer.
> In general I want to make sure it is clear that I don't encourage the
> model where DSA owns the entire mdio_device.
>
> What other peripherals are there on this SoC other than an MDIO bus and
> an EFUSE? IRQCHIP, GPIOs, LED controller, sensors?
>
> You can take a look at drivers/mfd/ocelot* and
> Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml for an example on
> how to use mfd for the top-level MDIO device, and DSA as just the driver
> for the Ethernet switch component (which will be represented as a
> platform_device).
Hi Vladimir,
I checked the examples and one problems that comes to me is how to model
this if only MDIO is used as a comunication method. Ocelot have PCIE or
SPI but this switch only comunicate with MDIO on his address. So where
should I place the SoC or MFD node? In the switch root node?
Also the big problem is how to model accessing the register with MDIO
with an MFD implementation.
Anyway just to make sure the Switch SoC doesn't expose an actualy MDIO
bus, that is just to solve the problem with the Switch Address shared
with one of the port. (Switch Address can be accessed by every switch
port with a specific page set)
But yes the problem is there... Function is not implemented but the
switch have i2c interface, minimal CPU, GPIO and Timer in it.
Happy to make the required changes, just very confused on how the final
DT node structure.
--
Ansuel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 17:06 ` Vladimir Oltean
@ 2024-12-05 17:26 ` Christian Marangi
2024-12-05 18:34 ` Vladimir Oltean
0 siblings, 1 reply; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 17:26 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 07:06:29PM +0200, Vladimir Oltean wrote:
> On Thu, Dec 05, 2024 at 03:51:33PM +0100, Christian Marangi wrote:
> > + .port_fdb_add = an8855_port_fdb_add,
> > + .port_fdb_del = an8855_port_fdb_del,
> > + .port_fdb_dump = an8855_port_fdb_dump,
> > + .port_mdb_add = an8855_port_mdb_add,
> > + .port_mdb_del = an8855_port_mdb_del,
>
> Please handle the "struct dsa_db" argument of these functions, so that
> you can turn on ds->fdb_isolation. It is likely that instead of a single
> AN8855_FID_BRIDGED, there needs to be a unique FID allocated for each
> VLAN-unaware bridge in order for their FDBs to be isolated from each
> other, and so that the same MAC address could live under both bridges.
Mh ok, I hoped we could first have the base DSA driver merged before
starting to applying these kind of feature.
Concept looks handy, ideally I can just assign one ID for each port
like:
port 1 -> FIB 1
port 2 -> FIB 1
port 3 -> FIB 2
Question:
Ports of the same bridge should have the same FIB?
What I need to check is how the switch handle this for learning. Does
the switch correctly create FDB entry with the right FIB? If that's not
the case then I think assisted_learning is needed and HW Learn can't be
used?
(I still need to check if I can assign a default FIB for a port...
Currently the STP register are 2 bit for each FIB id, so 16 different
FIB are possible)
Also do we have a script for selft tests? I remember there was one back
in the days for fdb isolation?
--
Ansuel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 16:12 ` [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver Russell King (Oracle)
@ 2024-12-05 17:44 ` Christian Marangi
0 siblings, 0 replies; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 17:44 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Matthias Brugger, AngeloGioacchino Del Regno, linux-arm-kernel,
linux-mediatek, netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 04:12:03PM +0000, Russell King (Oracle) wrote:
> hi Christian,
>
> On Thu, Dec 05, 2024 at 03:51:33PM +0100, Christian Marangi wrote:
> > diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> > index 2d10b4d6cfbb..6b6d0b7bae72 100644
> > --- a/drivers/net/dsa/Kconfig
> > +++ b/drivers/net/dsa/Kconfig
> > @@ -24,6 +24,15 @@ config NET_DSA_LOOP
> > This enables support for a fake mock-up switch chip which
> > exercises the DSA APIs.
> >
> > +
>
> Niggle - no need for the extra blank line.
>
Will drop!
> > +static int an8855_set_mac_eee(struct dsa_switch *ds, int port,
> > + struct ethtool_keee *eee)
> > +{
> > + struct an8855_priv *priv = ds->priv;
> > + u32 reg;
> > + int ret;
> > +
> > + if (eee->eee_enabled) {
> > + ret = regmap_read(priv->regmap, AN8855_PMCR_P(port), ®);
> > + if (ret)
> > + return ret;
> > + /* Force enable EEE if force mode and LINK */
> > + if (reg & AN8855_PMCR_FORCE_MODE &&
> > + reg & AN8855_PMCR_FORCE_LNK) {
> > + switch (reg & AN8855_PMCR_FORCE_SPEED) {
> > + case AN8855_PMCR_FORCE_SPEED_1000:
> > + reg |= AN8855_PMCR_FORCE_EEE1G;
> > + break;
> > + case AN8855_PMCR_FORCE_SPEED_100:
> > + reg |= AN8855_PMCR_FORCE_EEE100;
> > + break;
> > + default:
> > + break;
> > + }
>
> Does this forcing force EEE to be enabled for the port, irrespective
> of the negotiation?
>
This apply only if autoneg is off. In that case EEE is forced. I think
you already had this comment at times for this exact question. If
autoneg is on, EEE is not forced if EEE is turned on.
> > + ret = regmap_write(priv->regmap, AN8855_PMCR_P(port), reg);
> > + if (ret)
> > + return ret;
> > + }
> > + ret = regmap_update_bits(priv->regmap, AN8855_PMEEECR_P(port),
> > + AN8855_LPI_MODE_EN,
> > + eee->tx_lpi_enabled ? AN8855_LPI_MODE_EN : 0);
> > + if (ret)
> > + return ret;
> > + } else {
> > + ret = regmap_clear_bits(priv->regmap, AN8855_PMCR_P(port),
> > + AN8855_PMCR_FORCE_EEE1G |
> > + AN8855_PMCR_FORCE_EEE100);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_clear_bits(priv->regmap, AN8855_PMEEECR_P(port),
> > + AN8855_LPI_MODE_EN);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int an8855_get_mac_eee(struct dsa_switch *ds, int port,
> > + struct ethtool_keee *eee)
> > +{
> > + struct an8855_priv *priv = ds->priv;
> > + u32 reg;
> > + int ret;
> > +
> > + ret = regmap_read(priv->regmap, AN8855_PMEEECR_P(port), ®);
> > + if (ret)
> > + return ret;
> > + eee->tx_lpi_enabled = reg & AN8855_LPI_MODE_EN;
> > +
> > + ret = regmap_read(priv->regmap, AN8855_CKGCR, ®);
> > + if (ret)
> > + return ret;
> > + /* Global LPI TXIDLE Threshold, default 60ms (unit 2us) */
> > + eee->tx_lpi_timer = FIELD_GET(AN8855_LPI_TXIDLE_THD_MASK, reg) / 500;
>
> There is no point setting tx_lpi_enabled and tx_lpi_timer, as phylib
> will immediately overwrite then in its phy_ethtool_get_eee() method.
> In fact, there's no point to the DSA get_mac_eee() method.
>
> > +
> > + ret = regmap_read(priv->regmap, AN8855_PMSR_P(port), ®);
> > + if (ret)
> > + return ret;
>
> What's the purpose of this read?
>
> > +
> > + return 0;
> > +}
>
> Overall, I would suggest not implementing EEE just yet - I sent out a
> 22 patch RFC patch set last week which implements EEE support in
> phylink, and I have another bunch of patches that clean up DSA that
> I didn't send, which includes getting rid of the DSA get_mac_eee()
> method.
>
> Now that the bulk of the phylink in-band changes have been merged, my
> plan is to work through getting the RFC patch set merged - I've sent
> out the first four patches today for final review and merging.
>
> I stated some questions in the RFC cover message that everyone ignored,
> maybe you could look at that and give your views on the points I raised
> there?
>
Ok to drop these OPs and reimplement later once we have a better
implementation? (really wants the base driver merged so I don't have to
send big patch every time, hope you understand the problem)
> > +static struct phylink_pcs *
> > +an8855_phylink_mac_select_pcs(struct phylink_config *config,
> > + phy_interface_t interface)
> > +{
> > + struct dsa_port *dp = dsa_phylink_to_port(config);
> > + struct an8855_priv *priv = dp->ds->priv;
> > +
> > + switch (interface) {
> > + case PHY_INTERFACE_MODE_SGMII:
> > + case PHY_INTERFACE_MODE_2500BASEX:
> > + return &priv->pcs;
> > + default:
> > + return NULL;
> > + }
>
> Great, exactly what I like to see in mac_select_pcs()!
>
> > +}
> > +
> > +static void
> > +an8855_phylink_mac_config(struct phylink_config *config, unsigned int mode,
> > + const struct phylink_link_state *state)
> > +{
> > + struct dsa_port *dp = dsa_phylink_to_port(config);
> > + struct dsa_switch *ds = dp->ds;
> > + struct an8855_priv *priv;
> > + int port = dp->index;
> > +
> > + priv = ds->priv;
> > +
> > + /* Nothing to configure for internal ports */
> > + if (port != 5)
> > + return;
> > +
> > + regmap_update_bits(priv->regmap, AN8855_PMCR_P(port),
> > + AN8855_PMCR_IFG_XMIT | AN8855_PMCR_MAC_MODE |
> > + AN8855_PMCR_BACKOFF_EN | AN8855_PMCR_BACKPR_EN,
> > + FIELD_PREP(AN8855_PMCR_IFG_XMIT, 0x1) |
> > + AN8855_PMCR_MAC_MODE | AN8855_PMCR_BACKOFF_EN |
> > + AN8855_PMCR_BACKPR_EN);
> > +}
>
> This looks much better, thanks.
>
> > +
> > +static void an8855_phylink_get_caps(struct dsa_switch *ds, int port,
> > + struct phylink_config *config)
> > +{
> > + switch (port) {
> > + case 0:
> > + case 1:
> > + case 2:
> > + case 3:
> > + case 4:
> > + __set_bit(PHY_INTERFACE_MODE_GMII,
> > + config->supported_interfaces);
> > + __set_bit(PHY_INTERFACE_MODE_INTERNAL,
> > + config->supported_interfaces);
> > + break;
> > + case 5:
> > + phy_interface_set_rgmii(config->supported_interfaces);
> > + __set_bit(PHY_INTERFACE_MODE_SGMII,
> > + config->supported_interfaces);
> > + __set_bit(PHY_INTERFACE_MODE_2500BASEX,
> > + config->supported_interfaces);
> > + break;
> > + }
> > +
> > + config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> > + MAC_10 | MAC_100 | MAC_1000FD | MAC_2500FD;
> > +}
> > +
> > +static void
> > +an8855_phylink_mac_link_down(struct phylink_config *config, unsigned int mode,
> > + phy_interface_t interface)
> > +{
> > + struct dsa_port *dp = dsa_phylink_to_port(config);
> > + struct an8855_priv *priv = dp->ds->priv;
> > +
> > + /* With autoneg just disable TX/RX else also force link down */
> > + if (phylink_autoneg_inband(mode)) {
> > + regmap_clear_bits(priv->regmap, AN8855_PMCR_P(dp->index),
> > + AN8855_PMCR_TX_EN | AN8855_PMCR_RX_EN);
> > + } else {
> > + regmap_update_bits(priv->regmap, AN8855_PMCR_P(dp->index),
> > + AN8855_PMCR_TX_EN | AN8855_PMCR_RX_EN |
> > + AN8855_PMCR_FORCE_MODE | AN8855_PMCR_FORCE_LNK,
> > + AN8855_PMCR_FORCE_MODE);
> > + }
> > +}
> > +
> > +static void
> > +an8855_phylink_mac_link_up(struct phylink_config *config,
> > + struct phy_device *phydev, unsigned int mode,
> > + phy_interface_t interface, int speed, int duplex,
> > + bool tx_pause, bool rx_pause)
> > +{
> > + struct dsa_port *dp = dsa_phylink_to_port(config);
> > + struct an8855_priv *priv = dp->ds->priv;
> > + int port = dp->index;
> > + u32 reg;
> > +
> > + reg = regmap_read(priv->regmap, AN8855_PMCR_P(port), ®);
> > + if (phylink_autoneg_inband(mode)) {
> > + reg &= ~AN8855_PMCR_FORCE_MODE;
> > + } else {
> > + reg |= AN8855_PMCR_FORCE_MODE | AN8855_PMCR_FORCE_LNK;
> > +
> > + reg &= ~AN8855_PMCR_FORCE_SPEED;
> > + switch (speed) {
> > + case SPEED_10:
> > + reg |= AN8855_PMCR_FORCE_SPEED_10;
> > + break;
> > + case SPEED_100:
> > + reg |= AN8855_PMCR_FORCE_SPEED_100;
> > + break;
> > + case SPEED_1000:
> > + reg |= AN8855_PMCR_FORCE_SPEED_1000;
> > + break;
> > + case SPEED_2500:
> > + reg |= AN8855_PMCR_FORCE_SPEED_2500;
> > + break;
> > + case SPEED_5000:
> > + dev_err(priv->dev, "Missing support for 5G speed. Aborting...\n");
> > + return;
> > + }
> > +
> > + reg &= ~AN8855_PMCR_FORCE_FDX;
> > + if (duplex == DUPLEX_FULL)
> > + reg |= AN8855_PMCR_FORCE_FDX;
> > +
> > + reg &= ~AN8855_PMCR_RX_FC_EN;
> > + if (rx_pause || dsa_port_is_cpu(dp))
> > + reg |= AN8855_PMCR_RX_FC_EN;
> > +
> > + reg &= ~AN8855_PMCR_TX_FC_EN;
> > + if (rx_pause || dsa_port_is_cpu(dp))
> > + reg |= AN8855_PMCR_TX_FC_EN;
> > +
> > + /* Disable any EEE options */
> > + reg &= ~(AN8855_PMCR_FORCE_EEE5G | AN8855_PMCR_FORCE_EEE2P5G |
> > + AN8855_PMCR_FORCE_EEE1G | AN8855_PMCR_FORCE_EEE100);
> > + }
> > +
> > + reg |= AN8855_PMCR_TX_EN | AN8855_PMCR_RX_EN;
> > +
> > + regmap_write(priv->regmap, AN8855_PMCR_P(port), reg);
> > +}
>
> All the above looks fine to me.
>
> > +
> > +static void an8855_pcs_get_state(struct phylink_pcs *pcs,
> > + struct phylink_link_state *state)
> > +{
> > + struct an8855_priv *priv = container_of(pcs, struct an8855_priv, pcs);
> > + u32 val;
> > + int ret;
> > +
> > + ret = regmap_read(priv->regmap, AN8855_PMSR_P(AN8855_CPU_PORT), &val);
> > + if (ret < 0) {
> > + state->link = false;
> > + return;
> > + }
> > +
> > + state->link = !!(val & AN8855_PMSR_LNK);
> > + state->an_complete = state->link;
> > + state->duplex = (val & AN8855_PMSR_DPX) ? DUPLEX_FULL :
> > + DUPLEX_HALF;
> > +
> > + switch (val & AN8855_PMSR_SPEED) {
> > + case AN8855_PMSR_SPEED_10:
> > + state->speed = SPEED_10;
> > + break;
> > + case AN8855_PMSR_SPEED_100:
> > + state->speed = SPEED_100;
> > + break;
> > + case AN8855_PMSR_SPEED_1000:
> > + state->speed = SPEED_1000;
> > + break;
> > + case AN8855_PMSR_SPEED_2500:
> > + state->speed = SPEED_2500;
> > + break;
> > + case AN8855_PMSR_SPEED_5000:
> > + dev_err(priv->dev, "Missing support for 5G speed. Setting Unknown.\n");
> > + fallthrough;
> > + default:
> > + state->speed = SPEED_UNKNOWN;
> > + break;
> > + }
> > +
> > + if (val & AN8855_PMSR_RX_FC)
> > + state->pause |= MLO_PAUSE_RX;
> > + if (val & AN8855_PMSR_TX_FC)
> > + state->pause |= MLO_PAUSE_TX;
> > +}
> > +
> > +static int an8855_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
> > + phy_interface_t interface,
> > + const unsigned long *advertising,
> > + bool permit_pause_to_mac)
> > +{
> > + struct an8855_priv *priv = container_of(pcs, struct an8855_priv, pcs);
> > + u32 val;
> > + int ret;
> > +
> > + switch (interface) {
> > + case PHY_INTERFACE_MODE_SGMII:
> > + break;
> > + case PHY_INTERFACE_MODE_2500BASEX:
> > + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
> > + dev_err(priv->dev, "in-band negotiation unsupported");
> > + return -EINVAL;
> > + }
> > + break;
>
> Now that we have the phylink in-band changes merged, along with a new
> PCS .pcs_inband_caps() method, please implement this method to indicate
> that in-band is not supported in 2500BASE-X mode. Phylink will issue a
> warning if it attempts to go against that (particularly when e.g. the
> PHY demands in-band but the PCS doesn't support it - a case that likely
> will never work in practice.)
>
Thanks for the hint of the new OPs, yes will reimplement this.
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + /* !!! WELCOME TO HELL !!! */
> > +
> > + /* TX FIR - improve TX EYE */
> > + ret = regmap_update_bits(priv->regmap, AN8855_INTF_CTRL_10,
> > + AN8855_RG_DA_QP_TX_FIR_C2_SEL |
> > + AN8855_RG_DA_QP_TX_FIR_C2_FORCE |
> > + AN8855_RG_DA_QP_TX_FIR_C1_SEL |
> > + AN8855_RG_DA_QP_TX_FIR_C1_FORCE,
> > + AN8855_RG_DA_QP_TX_FIR_C2_SEL |
> > + FIELD_PREP(AN8855_RG_DA_QP_TX_FIR_C2_FORCE, 0x4) |
> > + AN8855_RG_DA_QP_TX_FIR_C1_SEL |
> > + FIELD_PREP(AN8855_RG_DA_QP_TX_FIR_C1_FORCE, 0x0));
> > + if (ret)
> > + return ret;
> > +
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = 0x0;
> > + else
> > + val = 0xd;
> > + ret = regmap_update_bits(priv->regmap, AN8855_INTF_CTRL_11,
> > + AN8855_RG_DA_QP_TX_FIR_C0B_SEL |
> > + AN8855_RG_DA_QP_TX_FIR_C0B_FORCE,
> > + AN8855_RG_DA_QP_TX_FIR_C0B_SEL |
> > + FIELD_PREP(AN8855_RG_DA_QP_TX_FIR_C0B_FORCE, val));
> > + if (ret)
> > + return ret;
> > +
> > + /* RX CDR - improve RX Jitter Tolerance */
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = 0x5;
> > + else
> > + val = 0x6;
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_LPF_BOT_LIM,
> > + AN8855_RG_QP_CDR_LPF_KP_GAIN |
> > + AN8855_RG_QP_CDR_LPF_KI_GAIN,
> > + FIELD_PREP(AN8855_RG_QP_CDR_LPF_KP_GAIN, val) |
> > + FIELD_PREP(AN8855_RG_QP_CDR_LPF_KI_GAIN, val));
> > + if (ret)
> > + return ret;
> > +
> > + /* PLL */
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = 0x1;
> > + else
> > + val = 0x0;
> > + ret = regmap_update_bits(priv->regmap, AN8855_QP_DIG_MODE_CTRL_1,
> > + AN8855_RG_TPHY_SPEED,
> > + FIELD_PREP(AN8855_RG_TPHY_SPEED, val));
> > + if (ret)
> > + return ret;
> > +
> > + /* PLL - LPF */
> > + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> > + AN8855_RG_DA_QP_PLL_RICO_SEL_INTF |
> > + AN8855_RG_DA_QP_PLL_FBKSEL_INTF |
> > + AN8855_RG_DA_QP_PLL_BR_INTF |
> > + AN8855_RG_DA_QP_PLL_BPD_INTF |
> > + AN8855_RG_DA_QP_PLL_BPA_INTF |
> > + AN8855_RG_DA_QP_PLL_BC_INTF,
> > + AN8855_RG_DA_QP_PLL_RICO_SEL_INTF |
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_FBKSEL_INTF, 0x0) |
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_BR_INTF, 0x3) |
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_BPD_INTF, 0x0) |
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_BPA_INTF, 0x5) |
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_BC_INTF, 0x1));
> > + if (ret)
> > + return ret;
> > +
> > + /* PLL - ICO */
> > + ret = regmap_set_bits(priv->regmap, AN8855_PLL_CTRL_4,
> > + AN8855_RG_DA_QP_PLL_ICOLP_EN_INTF);
> > + if (ret)
> > + return ret;
> > + ret = regmap_clear_bits(priv->regmap, AN8855_PLL_CTRL_2,
> > + AN8855_RG_DA_QP_PLL_ICOIQ_EN_INTF);
> > + if (ret)
> > + return ret;
> > +
> > + /* PLL - CHP */
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = 0x6;
> > + else
> > + val = 0x4;
> > + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> > + AN8855_RG_DA_QP_PLL_IR_INTF,
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_IR_INTF, val));
> > + if (ret)
> > + return ret;
> > +
> > + /* PLL - PFD */
> > + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> > + AN8855_RG_DA_QP_PLL_PFD_OFFSET_EN_INTRF |
> > + AN8855_RG_DA_QP_PLL_PFD_OFFSET_INTF |
> > + AN8855_RG_DA_QP_PLL_KBAND_PREDIV_INTF,
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_PFD_OFFSET_INTF, 0x1) |
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_KBAND_PREDIV_INTF, 0x1));
> > + if (ret)
> > + return ret;
> > +
> > + /* PLL - POSTDIV */
> > + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> > + AN8855_RG_DA_QP_PLL_POSTDIV_EN_INTF |
> > + AN8855_RG_DA_QP_PLL_PHY_CK_EN_INTF |
> > + AN8855_RG_DA_QP_PLL_PCK_SEL_INTF,
> > + AN8855_RG_DA_QP_PLL_PCK_SEL_INTF);
> > + if (ret)
> > + return ret;
> > +
> > + /* PLL - SDM */
> > + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_2,
> > + AN8855_RG_DA_QP_PLL_SDM_HREN_INTF,
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_SDM_HREN_INTF, 0x0));
> > + if (ret)
> > + return ret;
> > + ret = regmap_clear_bits(priv->regmap, AN8855_PLL_CTRL_2,
> > + AN8855_RG_DA_QP_PLL_SDM_IFM_INTF);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_SS_LCPLL_PWCTL_SETTING_2,
> > + AN8855_RG_NCPO_ANA_MSB,
> > + FIELD_PREP(AN8855_RG_NCPO_ANA_MSB, 0x1));
> > + if (ret)
> > + return ret;
> > +
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = 0x7a000000;
> > + else
> > + val = 0x48000000;
> > + ret = regmap_write(priv->regmap, AN8855_SS_LCPLL_TDC_FLT_2,
> > + FIELD_PREP(AN8855_RG_LCPLL_NCPO_VALUE, val));
> > + if (ret)
> > + return ret;
> > + ret = regmap_write(priv->regmap, AN8855_SS_LCPLL_TDC_PCW_1,
> > + FIELD_PREP(AN8855_RG_LCPLL_PON_HRDDS_PCW_NCPO_GPON, val));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_clear_bits(priv->regmap, AN8855_SS_LCPLL_TDC_FLT_5,
> > + AN8855_RG_LCPLL_NCPO_CHG);
> > + if (ret)
> > + return ret;
> > + ret = regmap_clear_bits(priv->regmap, AN8855_PLL_CK_CTRL_0,
> > + AN8855_RG_DA_QP_PLL_SDM_DI_EN_INTF);
> > + if (ret)
> > + return ret;
> > +
> > + /* PLL - SS */
> > + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_3,
> > + AN8855_RG_DA_QP_PLL_SSC_DELTA_INTF,
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_SSC_DELTA_INTF, 0x0));
> > + if (ret)
> > + return ret;
> > + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_4,
> > + AN8855_RG_DA_QP_PLL_SSC_DIR_DLY_INTF,
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_SSC_DIR_DLY_INTF, 0x0));
> > + if (ret)
> > + return ret;
> > + ret = regmap_update_bits(priv->regmap, AN8855_PLL_CTRL_3,
> > + AN8855_RG_DA_QP_PLL_SSC_PERIOD_INTF,
> > + FIELD_PREP(AN8855_RG_DA_QP_PLL_SSC_PERIOD_INTF, 0x0));
> > + if (ret)
> > + return ret;
> > +
> > + /* PLL - TDC */
> > + ret = regmap_clear_bits(priv->regmap, AN8855_PLL_CK_CTRL_0,
> > + AN8855_RG_DA_QP_PLL_TDC_TXCK_SEL_INTF);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_set_bits(priv->regmap, AN8855_RG_QP_PLL_SDM_ORD,
> > + AN8855_RG_QP_PLL_SSC_TRI_EN);
> > + if (ret)
> > + return ret;
> > + ret = regmap_set_bits(priv->regmap, AN8855_RG_QP_PLL_SDM_ORD,
> > + AN8855_RG_QP_PLL_SSC_PHASE_INI);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_RX_DAC_EN,
> > + AN8855_RG_QP_SIGDET_HF,
> > + FIELD_PREP(AN8855_RG_QP_SIGDET_HF, 0x2));
> > + if (ret)
> > + return ret;
> > +
> > + /* TCL Disable (only for Co-SIM) */
> > + ret = regmap_clear_bits(priv->regmap, AN8855_PON_RXFEDIG_CTRL_0,
> > + AN8855_RG_QP_EQ_RX500M_CK_SEL);
> > + if (ret)
> > + return ret;
> > +
> > + /* TX Init */
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = 0x4;
> > + else
> > + val = 0x0;
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_TX_MODE,
> > + AN8855_RG_QP_TX_RESERVE |
> > + AN8855_RG_QP_TX_MODE_16B_EN,
> > + FIELD_PREP(AN8855_RG_QP_TX_RESERVE, val));
> > + if (ret)
> > + return ret;
> > +
> > + /* RX Control/Init */
> > + ret = regmap_set_bits(priv->regmap, AN8855_RG_QP_RXAFE_RESERVE,
> > + AN8855_RG_QP_CDR_PD_10B_EN);
> > + if (ret)
> > + return ret;
> > +
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = 0x1;
> > + else
> > + val = 0x2;
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_LPF_MJV_LIM,
> > + AN8855_RG_QP_CDR_LPF_RATIO,
> > + FIELD_PREP(AN8855_RG_QP_CDR_LPF_RATIO, val));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_LPF_SETVALUE,
> > + AN8855_RG_QP_CDR_PR_BUF_IN_SR |
> > + AN8855_RG_QP_CDR_PR_BETA_SEL,
> > + FIELD_PREP(AN8855_RG_QP_CDR_PR_BUF_IN_SR, 0x6) |
> > + FIELD_PREP(AN8855_RG_QP_CDR_PR_BETA_SEL, 0x1));
> > + if (ret)
> > + return ret;
> > +
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = 0xf;
> > + else
> > + val = 0xc;
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_PR_CKREF_DIV1,
> > + AN8855_RG_QP_CDR_PR_DAC_BAND,
> > + FIELD_PREP(AN8855_RG_QP_CDR_PR_DAC_BAND, val));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_PR_KBAND_DIV_PCIE,
> > + AN8855_RG_QP_CDR_PR_KBAND_PCIE_MODE |
> > + AN8855_RG_QP_CDR_PR_KBAND_DIV_PCIE_MASK,
> > + FIELD_PREP(AN8855_RG_QP_CDR_PR_KBAND_DIV_PCIE_MASK, 0x19));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_FORCE_IBANDLPF_R_OFF,
> > + AN8855_RG_QP_CDR_PHYCK_SEL |
> > + AN8855_RG_QP_CDR_PHYCK_RSTB |
> > + AN8855_RG_QP_CDR_PHYCK_DIV,
> > + FIELD_PREP(AN8855_RG_QP_CDR_PHYCK_SEL, 0x2) |
> > + FIELD_PREP(AN8855_RG_QP_CDR_PHYCK_DIV, 0x21));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_clear_bits(priv->regmap, AN8855_RG_QP_CDR_PR_KBAND_DIV_PCIE,
> > + AN8855_RG_QP_CDR_PR_XFICK_EN);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_QP_CDR_PR_CKREF_DIV1,
> > + AN8855_RG_QP_CDR_PR_KBAND_DIV,
> > + FIELD_PREP(AN8855_RG_QP_CDR_PR_KBAND_DIV, 0x4));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_26,
> > + AN8855_RG_QP_EQ_RETRAIN_ONLY_EN |
> > + AN8855_RG_LINK_NE_EN |
> > + AN8855_RG_LINK_ERRO_EN,
> > + AN8855_RG_QP_EQ_RETRAIN_ONLY_EN |
> > + AN8855_RG_LINK_ERRO_EN);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RX_DLY_0,
> > + AN8855_RG_QP_RX_SAOSC_EN_H_DLY |
> > + AN8855_RG_QP_RX_PI_CAL_EN_H_DLY,
> > + FIELD_PREP(AN8855_RG_QP_RX_SAOSC_EN_H_DLY, 0x3f) |
> > + FIELD_PREP(AN8855_RG_QP_RX_PI_CAL_EN_H_DLY, 0x6f));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_42,
> > + AN8855_RG_QP_EQ_EN_DLY,
> > + FIELD_PREP(AN8855_RG_QP_EQ_EN_DLY, 0x150));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_2,
> > + AN8855_RG_QP_RX_EQ_EN_H_DLY,
> > + FIELD_PREP(AN8855_RG_QP_RX_EQ_EN_H_DLY, 0x150));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_PON_RXFEDIG_CTRL_9,
> > + AN8855_RG_QP_EQ_LEQOSC_DLYCNT,
> > + FIELD_PREP(AN8855_RG_QP_EQ_LEQOSC_DLYCNT, 0x1));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_8,
> > + AN8855_RG_DA_QP_SAOSC_DONE_TIME |
> > + AN8855_RG_DA_QP_LEQOS_EN_TIME,
> > + FIELD_PREP(AN8855_RG_DA_QP_SAOSC_DONE_TIME, 0x200) |
> > + FIELD_PREP(AN8855_RG_DA_QP_LEQOS_EN_TIME, 0xfff));
> > + if (ret)
> > + return ret;
> > +
> > + /* Frequency meter */
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = 0x10;
> > + else
> > + val = 0x28;
> > + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_5,
> > + AN8855_RG_FREDET_CHK_CYCLE,
> > + FIELD_PREP(AN8855_RG_FREDET_CHK_CYCLE, val));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_6,
> > + AN8855_RG_FREDET_GOLDEN_CYCLE,
> > + FIELD_PREP(AN8855_RG_FREDET_GOLDEN_CYCLE, 0x64));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_RX_CTRL_7,
> > + AN8855_RG_FREDET_TOLERATE_CYCLE,
> > + FIELD_PREP(AN8855_RG_FREDET_TOLERATE_CYCLE, 0x2710));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_set_bits(priv->regmap, AN8855_PLL_CTRL_0,
> > + AN8855_RG_PHYA_AUTO_INIT);
> > + if (ret)
> > + return ret;
> > +
> > + /* PCS Init */
> > + if (interface == PHY_INTERFACE_MODE_SGMII &&
> > + neg_mode == PHYLINK_PCS_NEG_INBAND_DISABLED) {
> > + ret = regmap_clear_bits(priv->regmap, AN8855_QP_DIG_MODE_CTRL_0,
> > + AN8855_RG_SGMII_MODE | AN8855_RG_SGMII_AN_EN);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + ret = regmap_clear_bits(priv->regmap, AN8855_RG_HSGMII_PCS_CTROL_1,
> > + AN8855_RG_TBI_10B_MODE);
> > + if (ret)
> > + return ret;
> > +
> > + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
> > + /* Set AN Ability - Interrupt */
> > + ret = regmap_set_bits(priv->regmap, AN8855_SGMII_REG_AN_FORCE_CL37,
> > + AN8855_RG_FORCE_AN_DONE);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(priv->regmap, AN8855_SGMII_REG_AN_13,
> > + AN8855_SGMII_REMOTE_FAULT_DIS |
> > + AN8855_SGMII_IF_MODE,
> > + AN8855_SGMII_REMOTE_FAULT_DIS |
> > + FIELD_PREP(AN8855_SGMII_IF_MODE, 0xb));
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + /* Rate Adaption - GMII path config. */
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX) {
> > + ret = regmap_clear_bits(priv->regmap, AN8855_RATE_ADP_P0_CTRL_0,
> > + AN8855_RG_P0_DIS_MII_MODE);
> > + if (ret)
> > + return ret;
> > + } else {
> > + if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
> > + ret = regmap_set_bits(priv->regmap, AN8855_MII_RA_AN_ENABLE,
> > + AN8855_RG_P0_RA_AN_EN);
> > + if (ret)
> > + return ret;
> > + } else {
> > + ret = regmap_update_bits(priv->regmap, AN8855_RG_AN_SGMII_MODE_FORCE,
> > + AN8855_RG_FORCE_CUR_SGMII_MODE |
> > + AN8855_RG_FORCE_CUR_SGMII_SEL,
> > + AN8855_RG_FORCE_CUR_SGMII_SEL);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_clear_bits(priv->regmap, AN8855_RATE_ADP_P0_CTRL_0,
> > + AN8855_RG_P0_MII_RA_RX_EN |
> > + AN8855_RG_P0_MII_RA_TX_EN |
> > + AN8855_RG_P0_MII_RA_RX_MODE |
> > + AN8855_RG_P0_MII_RA_TX_MODE);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + ret = regmap_set_bits(priv->regmap, AN8855_RATE_ADP_P0_CTRL_0,
> > + AN8855_RG_P0_MII_MODE);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + ret = regmap_set_bits(priv->regmap, AN8855_RG_RATE_ADAPT_CTRL_0,
> > + AN8855_RG_RATE_ADAPT_RX_BYPASS |
> > + AN8855_RG_RATE_ADAPT_TX_BYPASS |
> > + AN8855_RG_RATE_ADAPT_RX_EN |
> > + AN8855_RG_RATE_ADAPT_TX_EN);
> > + if (ret)
> > + return ret;
> > +
> > + /* Disable AN if not in autoneg */
> > + ret = regmap_update_bits(priv->regmap, AN8855_SGMII_REG_AN0, BMCR_ANENABLE,
> > + neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED ? BMCR_ANENABLE :
> > + 0);
> > + if (ret)
> > + return ret;
> > +
> > + if (interface == PHY_INTERFACE_MODE_SGMII &&
> > + neg_mode == PHYLINK_PCS_NEG_INBAND_DISABLED) {
> > + ret = regmap_set_bits(priv->regmap, AN8855_PHY_RX_FORCE_CTRL_0,
> > + AN8855_RG_FORCE_TXC_SEL);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + /* Force Speed with fixed-link or 2500base-x as doesn't support aneg */
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX ||
> > + neg_mode != PHYLINK_PCS_NEG_INBAND_ENABLED) {
> > + if (interface == PHY_INTERFACE_MODE_2500BASEX)
> > + val = AN8855_RG_LINK_MODE_P0_SPEED_2500;
> > + else
> > + val = AN8855_RG_LINK_MODE_P0_SPEED_1000;
> > + ret = regmap_update_bits(priv->regmap, AN8855_SGMII_STS_CTRL_0,
> > + AN8855_RG_LINK_MODE_P0 |
> > + AN8855_RG_FORCE_SPD_MODE_P0,
> > + val | AN8855_RG_FORCE_SPD_MODE_P0);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + /* bypass flow control to MAC */
> > + ret = regmap_write(priv->regmap, AN8855_MSG_RX_LIK_STS_0,
> > + AN8855_RG_DPX_STS_P3 | AN8855_RG_DPX_STS_P2 |
> > + AN8855_RG_DPX_STS_P1 | AN8855_RG_TXFC_STS_P0 |
> > + AN8855_RG_RXFC_STS_P0 | AN8855_RG_DPX_STS_P0);
> > + if (ret)
> > + return ret;
> > + ret = regmap_write(priv->regmap, AN8855_MSG_RX_LIK_STS_2,
> > + AN8855_RG_RXFC_AN_BYPASS_P3 |
> > + AN8855_RG_RXFC_AN_BYPASS_P2 |
> > + AN8855_RG_RXFC_AN_BYPASS_P1 |
> > + AN8855_RG_TXFC_AN_BYPASS_P3 |
> > + AN8855_RG_TXFC_AN_BYPASS_P2 |
> > + AN8855_RG_TXFC_AN_BYPASS_P1 |
> > + AN8855_RG_DPX_AN_BYPASS_P3 |
> > + AN8855_RG_DPX_AN_BYPASS_P2 |
> > + AN8855_RG_DPX_AN_BYPASS_P1 |
> > + AN8855_RG_DPX_AN_BYPASS_P0);
> > + if (ret)
> > + return ret;
>
> Is the above disruptive to the link if it is executed when the link is
> already up? Do you need to re-execute it even when "interface" hasn't
> changed?
>
> This gets called to possibly change the PCS advertisement in response
> to ethtool -s, so ought not have any link disrupting effects if
> nothing has changed.
>
No these change only after a switch reset, so they don't need to be
re-executed and doesn't interrupt the link.
> > +
> > + return 0;
> > +}
> > +
> > +static void an8855_pcs_an_restart(struct phylink_pcs *pcs)
> > +{
> > + struct an8855_priv *priv = container_of(pcs, struct an8855_priv, pcs);
> > +
> > + regmap_set_bits(priv->regmap, AN8855_SGMII_REG_AN0, BMCR_ANRESTART);
>
> This won't get called for SGMII - Cisco SGMII is merely the PHY telling
> the PCS/MAC what was negotiated at the media and how it's going to be
> operating the SGMII link. AN restart is a media side thing, so that
> would happen at the PHY in that case. I suggest this becomes an empty
> no-op function.
>
Soo this function won't ever be called in the current implementation?
I would like to keep the same PCS config of the original driver (just to
be sure). This is called for SGMII configuration in autoneg right after
BMCR_ANENABLE, ok for you to move this call in pcs_config or do you
think I should drop it entirely?
--
Ansuel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 17:17 ` Christian Marangi
@ 2024-12-05 18:05 ` Vladimir Oltean
2024-12-05 18:29 ` Christian Marangi
0 siblings, 1 reply; 21+ messages in thread
From: Vladimir Oltean @ 2024-12-05 18:05 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 06:17:18PM +0100, Christian Marangi wrote:
> I checked the examples and one problems that comes to me is how to model
> this if only MDIO is used as a comunication method. Ocelot have PCIE or
> SPI but this switch only comunicate with MDIO on his address.
I don't see why this matters. There will be a top-level device driver,
which in this case will be an mdio_driver and will use mdiobus_{read,write}
to physically access registers. This driver will create regmaps and add
them to devres using devm_regmap_init(). From devres, DSA and other child
drivers can use dev_get_regmap(dev->parent) and perform their I/O through
regmap.
This driver is already written for regmap, so part of the work can
already be reused.
> So where should I place the SoC or MFD node? In the switch root node?
The SoC should be placed on the host MDIO bus. And the Ethernet switch
component should be a child of the SoC. Ideally, so should be all other
switch peripherals: on the same level as the Ethernet switch.
> Also the big problem is how to model accessing the register with MDIO
> with an MFD implementation.
>
> Anyway just to make sure the Switch SoC doesn't expose an actualy MDIO
> bus, that is just to solve the problem with the Switch Address shared
> with one of the port. (Switch Address can be accessed by every switch
> port with a specific page set)
Sorry, I don't understand this, can you explain more? "Switch Address
can be accessed by every switch port with a specific page set"
In the code, I see that the priv->bus and priv->phy_base are used to
perform MDIO accesses for anything related to the switch. That's perfect,
it means that all switch registers are concentrated on a single MDIO
address, behind a single mdio_device. If that weren't the case, things
would get messy, because the Linux device model associates an MDIO device
with a single address on its bus.
And then we have an8855_phy_read() and an8855_phy_write(), which in my
understanding are the ops of a fake MDIO controller, one which has no
registers or MDIO address space of its own, but is just a passthrough
towards the host MDIO bus's address space. I have no idea why you don't
just put a phy-handle from the switch user ports to PHYs located on the
host MDIO bus directly, and why you go through this middle entity, but I
expect you will clarify. Creating an MDIO bus from DSA for internal PHYs
is completely optional if no special handling is required.
To explain again: In the MFD proposal, there is only one driver who has
access to the mdio_device from the host bus: the MFD driver. Depending
on how it implements the regmaps it presents to the children, it can
control page switching, etc etc. The child devices only operate with
regmaps, and have no idea of the underlying hardware access method.
> But yes the problem is there... Function is not implemented but the
> switch have i2c interface, minimal CPU, GPIO and Timer in it.
>
> Happy to make the required changes, just very confused on how the final
> DT node structure.
>
> --
> Ansuel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 18:05 ` Vladimir Oltean
@ 2024-12-05 18:29 ` Christian Marangi
2024-12-05 18:50 ` Vladimir Oltean
0 siblings, 1 reply; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 18:29 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 08:05:39PM +0200, Vladimir Oltean wrote:
> On Thu, Dec 05, 2024 at 06:17:18PM +0100, Christian Marangi wrote:
> > I checked the examples and one problems that comes to me is how to model
> > this if only MDIO is used as a comunication method. Ocelot have PCIE or
> > SPI but this switch only comunicate with MDIO on his address.
>
> I don't see why this matters. There will be a top-level device driver,
> which in this case will be an mdio_driver and will use mdiobus_{read,write}
> to physically access registers. This driver will create regmaps and add
> them to devres using devm_regmap_init(). From devres, DSA and other child
> drivers can use dev_get_regmap(dev->parent) and perform their I/O through
> regmap.
>
> This driver is already written for regmap, so part of the work can
> already be reused.
>
> > So where should I place the SoC or MFD node? In the switch root node?
>
> The SoC should be placed on the host MDIO bus. And the Ethernet switch
> component should be a child of the SoC. Ideally, so should be all other
> switch peripherals: on the same level as the Ethernet switch.
>
Ohhhh ok, wasn't clear to me the MFD driver had to be placed in the mdio
node.
To make it clear this would be an implementation.
mdio_bus: mdio-bus {
#address-cells = <1>;
#size-cells = <0>;
...
mfd@1 {
compatible = "airoha,an8855-mfd";
reg = <1>;
nvmem_node {
...
};
switch_node {
...
};
};
};
Consider tho that recently I faced some problem with such structure with
DT mainatiners asking to keep everything in the MFD node. But lets see
how it goes.
Well aware of the MFD API, (had to have some fun recently) so the only
confusing part was the node placement.
> > Also the big problem is how to model accessing the register with MDIO
> > with an MFD implementation.
> >
> > Anyway just to make sure the Switch SoC doesn't expose an actualy MDIO
> > bus, that is just to solve the problem with the Switch Address shared
> > with one of the port. (Switch Address can be accessed by every switch
> > port with a specific page set)
>
> Sorry, I don't understand this, can you explain more? "Switch Address
> can be accessed by every switch port with a specific page set"
>
> In the code, I see that the priv->bus and priv->phy_base are used to
> perform MDIO accesses for anything related to the switch. That's perfect,
> it means that all switch registers are concentrated on a single MDIO
> address, behind a single mdio_device. If that weren't the case, things
> would get messy, because the Linux device model associates an MDIO device
> with a single address on its bus.
>
> And then we have an8855_phy_read() and an8855_phy_write(), which in my
> understanding are the ops of a fake MDIO controller, one which has no
> registers or MDIO address space of its own, but is just a passthrough
> towards the host MDIO bus's address space. I have no idea why you don't
> just put a phy-handle from the switch user ports to PHYs located on the
> host MDIO bus directly, and why you go through this middle entity, but I
> expect you will clarify. Creating an MDIO bus from DSA for internal PHYs
> is completely optional if no special handling is required.
The difficulties I found (and maybe is very easy to solve and I'm
missing something here) is that switch and internal PHY port have the
same address and conflicts.
Switch will be at address 1 (or 2 3 4 5... every port can access switch
register with page 0x4)
DSA port 0 will be at address 1, that is already occupied by the switch.
Defining the DSA port node on the host MDIO bus works correctly for
every port but for port 0 (the one at address 1), the kernel complains
and is not init. (as it does conflict with the switch that is at the
same address) (can't remember the exact warning)
>
> To explain again: In the MFD proposal, there is only one driver who has
> access to the mdio_device from the host bus: the MFD driver. Depending
> on how it implements the regmaps it presents to the children, it can
> control page switching, etc etc. The child devices only operate with
> regmaps, and have no idea of the underlying hardware access method.
>
> > But yes the problem is there... Function is not implemented but the
> > switch have i2c interface, minimal CPU, GPIO and Timer in it.
> >
> > Happy to make the required changes, just very confused on how the final
> > DT node structure.
> >
> > --
> > Ansuel
>
--
Ansuel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 17:26 ` Christian Marangi
@ 2024-12-05 18:34 ` Vladimir Oltean
2024-12-05 19:16 ` Christian Marangi
0 siblings, 1 reply; 21+ messages in thread
From: Vladimir Oltean @ 2024-12-05 18:34 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 06:26:01PM +0100, Christian Marangi wrote:
> Concept looks handy, ideally I can just assign one ID for each port
> like:
> port 1 -> FIB 1
> port 2 -> FIB 1
> port 3 -> FIB 2
>
> Question:
> Ports of the same bridge should have the same FIB?
The answer, as well as many other explanations, is under the "Address
databases" section of Documentation/networking/dsa/dsa.rst. Please read
it through before starting to implement anything.
> What I need to check is how the switch handle this for learning. Does
> the switch correctly create FDB entry with the right FIB?
You're asking me how an8855 behaves? I have no idea, I never interacted
with it :-|
The idea as far as the DSA API is concerned would be to learn addresses
in the bridge database (DSA_DB_BRIDGE) that the user port is configured
for, and not learn any addresses in the port-private database (DSA_DB_PORT).
> If that's not the case then I think assisted_learning is needed and HW
> Learn can't be used?
ds->assisted_learning_on_cpu_port applies, as suggested by its name,
only on the CPU port. On user ports, address learning should work normally.
As you will find in the documentation, the CPU port is not like a user
port, in the sense that it is not configured to service a single address
database, but all of them. So, source learning on the CPU port will not
work unless the switch knows which address database should each packet
be associated with.
In principle, one way could be to pass, during tagger xmit, the database ID,
so that the switch knows that it must learn the MAC SA of this packet in
this FID. I don't have the full image of the Mediatek DSA tag format,
but if an8855 is anything like mt7530, this option isn't available.
Thus, like mt7530, it needs to set ds->assisted_learning_on_cpu_port, so
that software will call port_fdb_add() on the CPU port with the correct
dsa_db (for the right bridge) as argument. But I don't think that is
going to pose any sort of issue.
> (I still need to check if I can assign a default FIB for a port...
> Currently the STP register are 2 bit for each FIB id, so 16 different
> FIB are possible)
>
> Also do we have a script for selft tests? I remember there was one back
> in the days for fdb isolation?
I don't remember right now, I don't think we do. I'll try to come up
with something in the following days.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 18:29 ` Christian Marangi
@ 2024-12-05 18:50 ` Vladimir Oltean
2024-12-05 19:36 ` Christian Marangi
0 siblings, 1 reply; 21+ messages in thread
From: Vladimir Oltean @ 2024-12-05 18:50 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 07:29:53PM +0100, Christian Marangi wrote:
> Ohhhh ok, wasn't clear to me the MFD driver had to be placed in the mdio
> node.
>
> To make it clear this would be an implementation.
>
> mdio_bus: mdio-bus {
> #address-cells = <1>;
> #size-cells = <0>;
>
> ...
>
> mfd@1 {
> compatible = "airoha,an8855-mfd";
> reg = <1>;
>
> nvmem_node {
> ...
> };
>
> switch_node {
> ...
> };
> };
> };
I mean, I did mention Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml
in my initial reply, which has an example with exactly this layout...
> The difficulties I found (and maybe is very easy to solve and I'm
> missing something here) is that switch and internal PHY port have the
> same address and conflicts.
>
> Switch will be at address 1 (or 2 3 4 5... every port can access switch
> register with page 0x4)
>
> DSA port 0 will be at address 1, that is already occupied by the switch.
>
> Defining the DSA port node on the host MDIO bus works correctly for
> every port but for port 0 (the one at address 1), the kernel complains
> and is not init. (as it does conflict with the switch that is at the
> same address) (can't remember the exact warning)
Can any of these MDIO addresses (switch or ports) be changed through registers?
I guess the non-hack solution would be to permit MDIO buses to have
#size-cells = 1, and MDIO devices to acquire a range of the address
space, rather than just one address. Though take this with a grain of
salt, I have a lot more to learn.
If neither of those are options, in principle the hack with just
selecting, randomly, one of the N internal PHY addresses as the central
MDIO address should work equally fine regardless of whether we are
talking about the DSA switch's MDIO address here, or the MFD device's
MDIO address.
With MFD you still have the option of creating a fake MDIO controller
child device, which has mdio-parent-bus = <&host_bus>, and redirecting
all user port phy-handles to children of this bus. Since all regmap I/O
of this fake MDIO bus goes to the MFD driver, you can implement there
your hacks with page switching etc etc, and it should be equally safe.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 18:34 ` Vladimir Oltean
@ 2024-12-05 19:16 ` Christian Marangi
0 siblings, 0 replies; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 19:16 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 08:34:03PM +0200, Vladimir Oltean wrote:
> On Thu, Dec 05, 2024 at 06:26:01PM +0100, Christian Marangi wrote:
> > Concept looks handy, ideally I can just assign one ID for each port
> > like:
> > port 1 -> FIB 1
> > port 2 -> FIB 1
> > port 3 -> FIB 2
> >
> > Question:
> > Ports of the same bridge should have the same FIB?
>
> The answer, as well as many other explanations, is under the "Address
> databases" section of Documentation/networking/dsa/dsa.rst. Please read
> it through before starting to implement anything.
>
Ok sorry, will read.
> > What I need to check is how the switch handle this for learning. Does
> > the switch correctly create FDB entry with the right FIB?
>
> You're asking me how an8855 behaves? I have no idea, I never interacted
> with it :-|
>
Noo it wasn't a question for you, it was really to describe a problem
that might be present if the switch doesn't account for that and that I
need to check.
> The idea as far as the DSA API is concerned would be to learn addresses
> in the bridge database (DSA_DB_BRIDGE) that the user port is configured
> for, and not learn any addresses in the port-private database (DSA_DB_PORT).
>
> > If that's not the case then I think assisted_learning is needed and HW
> > Learn can't be used?
>
> ds->assisted_learning_on_cpu_port applies, as suggested by its name,
> only on the CPU port. On user ports, address learning should work normally.
>
> As you will find in the documentation, the CPU port is not like a user
> port, in the sense that it is not configured to service a single address
> database, but all of them. So, source learning on the CPU port will not
> work unless the switch knows which address database should each packet
> be associated with.
Ok so in such case, learning on CPU needs to be disabled and assisted
learning enabled.
>
> In principle, one way could be to pass, during tagger xmit, the database ID,
> so that the switch knows that it must learn the MAC SA of this packet in
> this FID. I don't have the full image of the Mediatek DSA tag format,
> but if an8855 is anything like mt7530, this option isn't available.
> Thus, like mt7530, it needs to set ds->assisted_learning_on_cpu_port, so
> that software will call port_fdb_add() on the CPU port with the correct
> dsa_db (for the right bridge) as argument. But I don't think that is
> going to pose any sort of issue.
>
In theory I might have found just this option. Tagger documentation is
totally missing but there are some c and header API that define some
interesting option of the tagger.
It seems the tagger can work in 3 way:
- portmap (what we currently use)
- portid
- lookup result
Now the last 2 mode seems very interesting.
The naming is very confusing but maybe with portid they refer to the
FIB. I need to check this. If that's the case then it's exactly what we
need. They set an int so it's definetly an ID.
I assume lookup result is to only use FDB to decide where the packet
should go. In this mode no ID or port are defined.
So in short lots of tests to do, maybe this can be handled in the
tagger.
> > (I still need to check if I can assign a default FIB for a port...
> > Currently the STP register are 2 bit for each FIB id, so 16 different
> > FIB are possible)
> >
> > Also do we have a script for selft tests? I remember there was one back
> > in the days for fdb isolation?
>
> I don't remember right now, I don't think we do. I'll try to come up
> with something in the following days.
Yes that would be handy.
--
Ansuel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 18:50 ` Vladimir Oltean
@ 2024-12-05 19:36 ` Christian Marangi
2024-12-05 23:57 ` Vladimir Oltean
0 siblings, 1 reply; 21+ messages in thread
From: Christian Marangi @ 2024-12-05 19:36 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 08:50:37PM +0200, Vladimir Oltean wrote:
> On Thu, Dec 05, 2024 at 07:29:53PM +0100, Christian Marangi wrote:
> > Ohhhh ok, wasn't clear to me the MFD driver had to be placed in the mdio
> > node.
> >
> > To make it clear this would be an implementation.
> >
> > mdio_bus: mdio-bus {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > ...
> >
> > mfd@1 {
> > compatible = "airoha,an8855-mfd";
> > reg = <1>;
> >
> > nvmem_node {
> > ...
> > };
> >
> > switch_node {
> > ...
> > };
> > };
> > };
>
> I mean, I did mention Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml
> in my initial reply, which has an example with exactly this layout...
>
> > The difficulties I found (and maybe is very easy to solve and I'm
> > missing something here) is that switch and internal PHY port have the
> > same address and conflicts.
> >
> > Switch will be at address 1 (or 2 3 4 5... every port can access switch
> > register with page 0x4)
> >
> > DSA port 0 will be at address 1, that is already occupied by the switch.
> >
> > Defining the DSA port node on the host MDIO bus works correctly for
> > every port but for port 0 (the one at address 1), the kernel complains
> > and is not init. (as it does conflict with the switch that is at the
> > same address) (can't remember the exact warning)
>
> Can any of these MDIO addresses (switch or ports) be changed through registers?
No, it can only be changed the BASE address that change the address of
each port.
port 0 is base address
port 1 is base address + 1
port 2 is base address + 2...
>
> I guess the non-hack solution would be to permit MDIO buses to have
> #size-cells = 1, and MDIO devices to acquire a range of the address
> space, rather than just one address. Though take this with a grain of
> salt, I have a lot more to learn.
I remember this was an idea when PHY Package API were proposed and was
rejected as we wanted PHY to be single reg.
>
> If neither of those are options, in principle the hack with just
> selecting, randomly, one of the N internal PHY addresses as the central
> MDIO address should work equally fine regardless of whether we are
> talking about the DSA switch's MDIO address here, or the MFD device's
> MDIO address.
>
> With MFD you still have the option of creating a fake MDIO controller
> child device, which has mdio-parent-bus = <&host_bus>, and redirecting
> all user port phy-handles to children of this bus. Since all regmap I/O
> of this fake MDIO bus goes to the MFD driver, you can implement there
> your hacks with page switching etc etc, and it should be equally
> safe.
I wonder if a node like this would be more consistent and descriptive?
mdio_bus: mdio-bus {
#address-cells = <1>;
#size-cells = <0>;
...
mfd@1 {
compatible = "airoha,an8855-mfd";
reg = <1>;
nvmem_node {
...
};
switch_node {
ports {
port@0 {
phy-handle = <&phy>;
};
port@1 {
phy-handle = <&phy_2>;
}
};
};
phy: phy_node {
};
};
phy_2: phy@2 {
reg = <2>;
}
phy@3 {
reg = <3>;
}
..
};
No idea how to register that single phy in mfd... I guess a fake mdio is
needed anyway... What do you think of this node example? Or not worth it
and better have the fake MDIO with all the switch PHY in it?
--
Ansuel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 19:36 ` Christian Marangi
@ 2024-12-05 23:57 ` Vladimir Oltean
2024-12-07 12:11 ` Christian Marangi
0 siblings, 1 reply; 21+ messages in thread
From: Vladimir Oltean @ 2024-12-05 23:57 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Thu, Dec 05, 2024 at 08:36:30PM +0100, Christian Marangi wrote:
> > I guess the non-hack solution would be to permit MDIO buses to have
> > #size-cells = 1, and MDIO devices to acquire a range of the address
> > space, rather than just one address. Though take this with a grain of
> > salt, I have a lot more to learn.
>
> I remember this was an idea when PHY Package API were proposed and was
> rejected as we wanted PHY to be single reg.
Would that effort have helped with MDIO devices, in the way it was proposed?
Why did it die out?
> > If neither of those are options, in principle the hack with just
> > selecting, randomly, one of the N internal PHY addresses as the central
> > MDIO address should work equally fine regardless of whether we are
> > talking about the DSA switch's MDIO address here, or the MFD device's
> > MDIO address.
> >
> > With MFD you still have the option of creating a fake MDIO controller
> > child device, which has mdio-parent-bus = <&host_bus>, and redirecting
> > all user port phy-handles to children of this bus. Since all regmap I/O
> > of this fake MDIO bus goes to the MFD driver, you can implement there
> > your hacks with page switching etc etc, and it should be equally
> > safe.
>
> I wonder if a node like this would be more consistent and descriptive?
>
> mdio_bus: mdio-bus {
> #address-cells = <1>;
> #size-cells = <0>;
>
> ...
>
> mfd@1 {
> compatible = "airoha,an8855-mfd";
> reg = <1>;
>
> nvmem_node {
> ...
> };
>
> switch_node {
> ports {
> port@0 {
> phy-handle = <&phy>;
> };
>
> port@1 {
> phy-handle = <&phy_2>;
> }
> };
> };
>
> phy: phy_node {
>
> };
> };
>
> phy_2: phy@2 {
> reg = <2>;
> }
>
> phy@3 {
> reg = <3>;
> }
>
> ..
> };
>
> No idea how to register that single phy in mfd... I guess a fake mdio is
> needed anyway... What do you think of this node example? Or not worth it
> and better have the fake MDIO with all the switch PHY in it?
Could you work with something like this? dtc seems to swallow it without
any warnings...
mdio_bus: mdio {
#address-cells = <1>;
#size-cells = <0>;
soc@1 {
compatible = "airoha,an8855";
reg = <1>, <2>, <3>, <4>;
reg-names = "phy0", "phy1", "phy2", "phy3";
nvmem {
compatible = "airoha,an8855-nvmem";
};
ethernet-switch {
compatible = "airoha,an8855-switch";
ethernet-ports {
#address-cells = <1>;
#size-cells = <0>;
ethernet-port@0 {
reg = <0>;
phy-handle = <&phy0>;
phy-mode = "internal";
};
ethernet-port@1 {
reg = <1>;
phy-handle = <&phy1>;
phy-mode = "internal";
};
ethernet-port@2 {
reg = <2>;
phy-handle = <&phy2>;
phy-mode = "internal";
};
ethernet-port@3 {
reg = <3>;
phy-handle = <&phy3>;
phy-mode = "internal";
};
};
};
mdio {
compatible = "airoha,an8855-mdio";
mdio-parent-bus = <&host_mdio>;
#address-cells = <1>;
#size-cells = <0>;
phy0: ethernet-phy@1 {
reg = <1>;
};
phy1: ethernet-phy@2 {
reg = <2>;
};
phy2: ethernet-phy@3 {
reg = <3>;
};
phy3: ethernet-phy@4 {
reg = <4>;
};
};
};
};
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-05 23:57 ` Vladimir Oltean
@ 2024-12-07 12:11 ` Christian Marangi
2024-12-10 20:31 ` Vladimir Oltean
0 siblings, 1 reply; 21+ messages in thread
From: Christian Marangi @ 2024-12-07 12:11 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Fri, Dec 06, 2024 at 01:57:09AM +0200, Vladimir Oltean wrote:
> On Thu, Dec 05, 2024 at 08:36:30PM +0100, Christian Marangi wrote:
> > > I guess the non-hack solution would be to permit MDIO buses to have
> > > #size-cells = 1, and MDIO devices to acquire a range of the address
> > > space, rather than just one address. Though take this with a grain of
> > > salt, I have a lot more to learn.
> >
> > I remember this was an idea when PHY Package API were proposed and was
> > rejected as we wanted PHY to be single reg.
>
> Would that effort have helped with MDIO devices, in the way it was proposed?
> Why did it die out?
>
> > > If neither of those are options, in principle the hack with just
> > > selecting, randomly, one of the N internal PHY addresses as the central
> > > MDIO address should work equally fine regardless of whether we are
> > > talking about the DSA switch's MDIO address here, or the MFD device's
> > > MDIO address.
> > >
> > > With MFD you still have the option of creating a fake MDIO controller
> > > child device, which has mdio-parent-bus = <&host_bus>, and redirecting
> > > all user port phy-handles to children of this bus. Since all regmap I/O
> > > of this fake MDIO bus goes to the MFD driver, you can implement there
> > > your hacks with page switching etc etc, and it should be equally
> > > safe.
> >
> > I wonder if a node like this would be more consistent and descriptive?
> >
> > mdio_bus: mdio-bus {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > ...
> >
> > mfd@1 {
> > compatible = "airoha,an8855-mfd";
> > reg = <1>;
> >
> > nvmem_node {
> > ...
> > };
> >
> > switch_node {
> > ports {
> > port@0 {
> > phy-handle = <&phy>;
> > };
> >
> > port@1 {
> > phy-handle = <&phy_2>;
> > }
> > };
> > };
> >
> > phy: phy_node {
> >
> > };
> > };
> >
> > phy_2: phy@2 {
> > reg = <2>;
> > }
> >
> > phy@3 {
> > reg = <3>;
> > }
> >
> > ..
> > };
> >
> > No idea how to register that single phy in mfd... I guess a fake mdio is
> > needed anyway... What do you think of this node example? Or not worth it
> > and better have the fake MDIO with all the switch PHY in it?
>
> Could you work with something like this? dtc seems to swallow it without
> any warnings...
>
> mdio_bus: mdio {
> #address-cells = <1>;
> #size-cells = <0>;
>
> soc@1 {
> compatible = "airoha,an8855";
> reg = <1>, <2>, <3>, <4>;
> reg-names = "phy0", "phy1", "phy2", "phy3";
>
> nvmem {
> compatible = "airoha,an8855-nvmem";
> };
>
> ethernet-switch {
> compatible = "airoha,an8855-switch";
>
> ethernet-ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> ethernet-port@0 {
> reg = <0>;
> phy-handle = <&phy0>;
> phy-mode = "internal";
> };
>
> ethernet-port@1 {
> reg = <1>;
> phy-handle = <&phy1>;
> phy-mode = "internal";
> };
>
> ethernet-port@2 {
> reg = <2>;
> phy-handle = <&phy2>;
> phy-mode = "internal";
> };
>
> ethernet-port@3 {
> reg = <3>;
> phy-handle = <&phy3>;
> phy-mode = "internal";
> };
> };
> };
>
> mdio {
> compatible = "airoha,an8855-mdio";
> mdio-parent-bus = <&host_mdio>;
> #address-cells = <1>;
> #size-cells = <0>;
>
> phy0: ethernet-phy@1 {
> reg = <1>;
> };
>
> phy1: ethernet-phy@2 {
> reg = <2>;
> };
>
> phy2: ethernet-phy@3 {
> reg = <3>;
> };
>
> phy3: ethernet-phy@4 {
> reg = <4>;
> };
> };
> };
> };
I finished testing and this works, I'm not using mdio-parent-bus tho as
the mdio-mux driver seems overkill for the task and problematic for PAGE
handling. (mdio-mux doesn't provide a way to give the current addr that
is being accessed)
My big concern is dt_binding_check and how Rob might take this
implementation. We recently had another case with a MFD node and Rob
found some problems in having subnode with compatible but maybe for this
particular complex case it will be O.K.
Still have to check if it's ok to have multiple reg in the mfd root node
(for mdio schema)
--
Ansuel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-07 12:11 ` Christian Marangi
@ 2024-12-10 20:31 ` Vladimir Oltean
2024-12-10 20:56 ` Christian Marangi
0 siblings, 1 reply; 21+ messages in thread
From: Vladimir Oltean @ 2024-12-10 20:31 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Sat, Dec 07, 2024 at 01:11:23PM +0100, Christian Marangi wrote:
> I finished testing and this works, I'm not using mdio-parent-bus tho as
> the mdio-mux driver seems overkill for the task and problematic for PAGE
> handling. (mdio-mux doesn't provide a way to give the current addr that
> is being accessed)
The use of mdio-parent-bus doesn't necessarily imply an mdio-mux. For
example, you'll also see it used in net/dsa/microchip,ksz.yaml.
You say this switch is also accessible over I2C. How are the internal
PHYs accessed in that case? Still over MDIO? If so, it would be nice to
have a unified scheme for both I2C-controlled switch and MDIO-controlled
switch, which is something that mdio-parent-bus would permit.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver
2024-12-10 20:31 ` Vladimir Oltean
@ 2024-12-10 20:56 ` Christian Marangi
0 siblings, 0 replies; 21+ messages in thread
From: Christian Marangi @ 2024-12-10 20:56 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiner Kallweit, Russell King, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev, devicetree, linux-kernel, upstream
On Tue, Dec 10, 2024 at 10:31:48PM +0200, Vladimir Oltean wrote:
> On Sat, Dec 07, 2024 at 01:11:23PM +0100, Christian Marangi wrote:
> > I finished testing and this works, I'm not using mdio-parent-bus tho as
> > the mdio-mux driver seems overkill for the task and problematic for PAGE
> > handling. (mdio-mux doesn't provide a way to give the current addr that
> > is being accessed)
>
> The use of mdio-parent-bus doesn't necessarily imply an mdio-mux. For
> example, you'll also see it used in net/dsa/microchip,ksz.yaml.
>
> You say this switch is also accessible over I2C. How are the internal
> PHYs accessed in that case? Still over MDIO? If so, it would be nice to
> have a unified scheme for both I2C-controlled switch and MDIO-controlled
> switch, which is something that mdio-parent-bus would permit.
What is not clear to me is where that property should be used in the
code or it's just for reference in DT to describe what is the parent?
Given MFD implementation, I pass the bus by accessing the MFD priv
struct so having the parent property is redundant.
--
Ansuel
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2024-12-10 21:01 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-05 14:51 [net-next PATCH v9 0/4] net: dsa: Add Airoha AN8855 support Christian Marangi
2024-12-05 14:51 ` [net-next PATCH v9 1/4] net: dsa: add devm_dsa_register_switch() Christian Marangi
2024-12-05 16:03 ` Vladimir Oltean
2024-12-05 14:51 ` [net-next PATCH v9 2/4] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation Christian Marangi
2024-12-05 14:51 ` [net-next PATCH v9 4/4] net: phy: Add Airoha AN8855 Internal Switch Gigabit PHY Christian Marangi
[not found] ` <20241205145142.29278-4-ansuelsmth@gmail.com>
2024-12-05 16:12 ` [net-next PATCH v9 3/4] net: dsa: Add Airoha AN8855 5-Port Gigabit DSA Switch driver Russell King (Oracle)
2024-12-05 17:44 ` Christian Marangi
2024-12-05 16:27 ` Vladimir Oltean
2024-12-05 17:17 ` Christian Marangi
2024-12-05 18:05 ` Vladimir Oltean
2024-12-05 18:29 ` Christian Marangi
2024-12-05 18:50 ` Vladimir Oltean
2024-12-05 19:36 ` Christian Marangi
2024-12-05 23:57 ` Vladimir Oltean
2024-12-07 12:11 ` Christian Marangi
2024-12-10 20:31 ` Vladimir Oltean
2024-12-10 20:56 ` Christian Marangi
2024-12-05 17:06 ` Vladimir Oltean
2024-12-05 17:26 ` Christian Marangi
2024-12-05 18:34 ` Vladimir Oltean
2024-12-05 19:16 ` Christian Marangi
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).