Re: [PATCH net-next 4/5] net: ethernet: adi: adin1110: Add OA TC6 protocol support for ADIN1110

["Alvin Šipraga" <alvin.sipraga@analog.com>]

On Tue, Nov 03, 2026 at 06:09:17PM +0200, Iustin Stolniceanu wrote:
> From: Ciprian Regus <ciprian.regus@analog.com>
> 
> The ADIN1110 and ADIN2111 support two SPI host interface protocols,
> the generic SPI protocol and the OPEN Alliance TC6 protocol. The
> protocol is selected by the state of the SPI_CFG1 pin, which the device
> latches when it comes out of reset. The driver currently supports only
> the generic SPI protocol.
> 
> Add support for the OPEN Alliance TC6 protocol on the ADIN1110, using
> the oa_tc6 framework, when the adi,spi-oa-tc6 property is set. The
> framework handles the data transfers and the MDIO bus. Register
> accesses go through read_reg/write_reg callbacks, so the existing MAC
> configuration code is shared by both protocols. The ADIN1110 specific
> MAC registers are in MMS 1.
> 
> The framework does not append the FCS to transmitted frames, so set
> CRC_APPEND in CONFIG2 to let the MAC append it. Also set the port state
> to forwarding on open, as the generic SPI path does, otherwise the
> broadcast and multicast filters drop all received frames.
> 
> The ADIN2111 is not supported in OPEN Alliance TC6 mode yet, and probe
> fails with -EOPNOTSUPP.

Not sure -EOPNOTSUPP is correct, maybe -EIO or something?

> 
> Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
> Co-developed-by: Iustin Stolniceanu <iustin.stolniceanu@analog.com>
> Signed-off-by: Iustin Stolniceanu <iustin.stolniceanu@analog.com>
> ---
>  drivers/net/ethernet/adi/Kconfig    |   1 +
>  drivers/net/ethernet/adi/adin1110.c | 186 +++++++++++++++++++++++++++-
>  include/linux/oa_tc6.h              |   1 +
>  3 files changed, 186 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/adi/Kconfig b/drivers/net/ethernet/adi/Kconfig
> index bdb8ff7d15da..c35f68814c21 100644
> --- a/drivers/net/ethernet/adi/Kconfig
> +++ b/drivers/net/ethernet/adi/Kconfig
> @@ -22,6 +22,7 @@ config ADIN1110
>  	depends on SPI && NET_SWITCHDEV
>  	select CRC8
>  	select PHYLIB
> +	select OA_TC6
>  	help
>  	  Say yes here to build support for Analog Devices ADIN1110
>  	  Low Power 10BASE-T1L Ethernet MAC-PHY.
> diff --git a/drivers/net/ethernet/adi/adin1110.c b/drivers/net/ethernet/adi/adin1110.c
> index 53fac3dc8be6..d08b165bf7c4 100644
> --- a/drivers/net/ethernet/adi/adin1110.c
> +++ b/drivers/net/ethernet/adi/adin1110.c
> @@ -19,6 +19,7 @@
>  #include <linux/mii.h>
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
> +#include <linux/oa_tc6.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/phy.h>
>  #include <linux/property.h>
> @@ -114,6 +115,8 @@
>  #define ADIN_MAC_P2_ADDR_SLOT			3
>  #define ADIN_MAC_FDB_ADDR_SLOT			4
>  
> +#define ADIN1110_MMS0_LIMIT			0x30
> +
>  DECLARE_CRC8_TABLE(adin1110_crc_table);
>  
>  enum adin1110_chips_id {
> @@ -145,6 +148,7 @@ struct adin1110_port_priv {
>  	u32				nr;
>  	u32				state;
>  	struct adin1110_cfg		*cfg;
> +	struct oa_tc6			*tc6;
>  };
>  
>  struct adin1110_priv {
> @@ -161,6 +165,9 @@ struct adin1110_priv {
>  	struct adin1110_port_priv	*ports[ADIN_MAC_MAX_PORTS];
>  	char				mii_bus_name[MII_BUS_ID_SIZE];
>  	u8				data[ADIN1110_MAX_BUFF] ____cacheline_aligned;
> +
> +	int (*read_reg)(struct adin1110_priv *priv, u16 reg, u32 *val);
> +	int (*write_reg)(struct adin1110_priv *priv, u16 reg, u32 val);
>  };
>  
>  struct adin1110_switchdev_event_work {
> @@ -192,7 +199,18 @@ static u8 adin1110_crc_data(u8 *data, u32 len)
>  	return crc8(adin1110_crc_table, data, len, 0);
>  }
>  
> -static int adin1110_read_reg(struct adin1110_priv *priv, u16 reg, u32 *val)
> +static int adin1110_read_reg_tc6(struct adin1110_priv *priv, u16 reg, u32 *val)
> +{
> +	struct adin1110_port_priv *port_priv = priv->ports[0];
> +
> +	if (reg >= ADIN1110_MMS0_LIMIT)
> +		return oa_tc6_read_register_mms(port_priv->tc6, OA_TC6_MAC_MMS1,
> +						 reg, val);
> +
> +	return oa_tc6_read_register(port_priv->tc6, reg, val);
> +}
> +
> +static int adin1110_read_reg_generic_spi(struct adin1110_priv *priv, u16 reg, u32 *val)
>  {
>  	u32 header_len = ADIN1110_RD_HEADER_LEN;
>  	u32 read_len = ADIN1110_REG_LEN;
> @@ -240,7 +258,23 @@ static int adin1110_read_reg(struct adin1110_priv *priv, u16 reg, u32 *val)
>  	return ret;
>  }
>  
> -static int adin1110_write_reg(struct adin1110_priv *priv, u16 reg, u32 val)
> +static int adin1110_read_reg(struct adin1110_priv *priv, u16 reg, u32 *val)
> +{
> +	return priv->read_reg(priv, reg, val);
> +}
> +
> +static int adin1110_write_reg_tc6(struct adin1110_priv *priv, u16 reg, u32 val)
> +{
> +	struct adin1110_port_priv *port_priv = priv->ports[0];
> +
> +	if (reg >= ADIN1110_MMS0_LIMIT)
> +		return oa_tc6_write_register_mms(port_priv->tc6, OA_TC6_MAC_MMS1,
> +						  reg, val);
> +
> +	return oa_tc6_write_register(port_priv->tc6, reg, val);
> +}
> +
> +static int adin1110_write_reg_generic_spi(struct adin1110_priv *priv, u16 reg, u32 val)
>  {
>  	u32 header_len = ADIN1110_WR_HEADER_LEN;
>  	u32 write_len = ADIN1110_REG_LEN;
> @@ -263,6 +297,11 @@ static int adin1110_write_reg(struct adin1110_priv *priv, u16 reg, u32 val)
>  	return spi_write(priv->spidev, &priv->data[0], header_len + write_len);
>  }
>  
> +static int adin1110_write_reg(struct adin1110_priv *priv, u16 reg, u32 val)
> +{
> +	return priv->write_reg(priv, reg, val);
> +}
> +
>  static int adin1110_set_bits(struct adin1110_priv *priv, u16 reg,
>  			     unsigned long mask, unsigned long val)
>  {
> @@ -881,6 +920,22 @@ static void adin1110_set_rx_mode(struct net_device *dev)
>  	spin_unlock(&priv->state_lock);
>  }
>  
> +static int adin1110_set_rx_mode_tc6(struct net_device *dev,
> +				    struct netdev_hw_addr_list *uc,
> +				    struct netdev_hw_addr_list *mc)
> +{
> +	struct adin1110_port_priv *port_priv = netdev_priv(dev);
> +	struct adin1110_priv *priv = port_priv->priv;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);

guard(mutex)(&priv->lock)?

OK, but the rest of the driver doesn't use that, so I can see why you
refrained from going that way.

> +	port_priv->flags = dev->flags;
> +	ret = adin1110_setup_rx_mode(port_priv);
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
>  static int adin1110_net_open(struct net_device *net_dev)
>  {
>  	struct adin1110_port_priv *port_priv = netdev_priv(net_dev);
> @@ -938,6 +993,31 @@ static int adin1110_net_open(struct net_device *net_dev)
>  	return 0;
>  }
>  
> +static int adin1110_net_open_tc6(struct net_device *net_dev)
> +{
> +	struct adin1110_port_priv *port_priv = netdev_priv(net_dev);
> +	struct adin1110_priv *priv = port_priv->priv;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +
> +	/* Needed for the broadcast/multicast filters to forward to the host */
> +	port_priv->state = BR_STATE_FORWARDING;
> +
> +	ret = adin1110_set_mac_address(net_dev, net_dev->dev_addr);
> +	mutex_unlock(&priv->lock);
> +	if (ret < 0) {
> +		netdev_err(net_dev, "Could not set MAC address: %pM, %d\n",
> +			   net_dev->dev_addr, ret);
> +		return ret;
> +	}

For the above, it wasn't quite clear to me why you don't call
adin1110_port_set_forwarding_state()?

> +
> +	phy_start(net_dev->phydev);
> +	netif_start_queue(net_dev);
> +
> +	return 0;
> +}
> +
>  static int adin1110_net_stop(struct net_device *net_dev)
>  {
>  	struct adin1110_port_priv *port_priv = netdev_priv(net_dev);
> @@ -961,6 +1041,14 @@ static int adin1110_net_stop(struct net_device *net_dev)
>  	return 0;
>  }
>  
> +static int adin1110_net_stop_tc6(struct net_device *net_dev)
> +{
> +	netif_stop_queue(net_dev);
> +	phy_stop(net_dev->phydev);
> +
> +	return 0;
> +}
> +
>  static void adin1110_tx_work(struct work_struct *work)
>  {
>  	struct adin1110_port_priv *port_priv;
> @@ -1006,6 +1094,18 @@ static netdev_tx_t adin1110_start_xmit(struct sk_buff *skb, struct net_device *d
>  	return netdev_ret;
>  }
>  
> +static netdev_tx_t adin1110_start_xmit_tc6(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct adin1110_port_priv *port_priv = netdev_priv(dev);
> +
> +	if (skb_put_padto(skb, ETH_ZLEN)) {
> +		dev->stats.tx_dropped++;
> +		return NETDEV_TX_OK;
> +	}
> +
> +	return oa_tc6_start_xmit(port_priv->tc6, skb);
> +}
> +
>  static void adin1110_ndo_get_stats64(struct net_device *dev,
>  				     struct rtnl_link_stats64 *storage)
>  {
> @@ -1056,6 +1156,16 @@ static const struct net_device_ops adin1110_netdev_ops = {
>  	.ndo_get_phys_port_name	= adin1110_ndo_get_phys_port_name,
>  };
>  
> +static const struct net_device_ops adin1110_tc6_netdev_ops = {
> +	.ndo_open		= adin1110_net_open_tc6,
> +	.ndo_stop		= adin1110_net_stop_tc6,
> +	.ndo_eth_ioctl		= adin1110_ioctl,
> +	.ndo_start_xmit		= adin1110_start_xmit_tc6,
> +	.ndo_set_mac_address	= adin1110_ndo_set_mac_address,
> +	.ndo_set_rx_mode_async	= adin1110_set_rx_mode_tc6,

Would it make sense to use the async mode for the non-tc6 case as well?
Then you can drop the work there and have the same ndo callback for both.

> +	.ndo_validate_addr	= eth_validate_addr,
> +};
> +
>  static void adin1110_get_drvinfo(struct net_device *dev,
>  				 struct ethtool_drvinfo *di)
>  {
> @@ -1123,6 +1233,69 @@ static int adin1110_check_spi(struct adin1110_priv *priv)
>  	return 0;
>  }
>  
> +static void adin1110_oa_tc6_remove(void *data)
> +{
> +	oa_tc6_exit(data);
> +}
> +
> +static int adin1110_oa_tc6_probe(struct adin1110_priv *priv)
> +{
> +	struct adin1110_port_priv *port_priv;
> +	struct oa_tc6_quirks quirks = {0};
> +	struct net_device *netdev;
> +	struct device *dev;
> +	int ret;
> +
> +	dev = &priv->spidev->dev;
> +
> +	if (priv->cfg->id == ADIN2111_MAC)
> +		return dev_err_probe(dev, -EOPNOTSUPP,
> +				     "OA TC6 isn't supported for ADIN2111\n");
> +
> +	netdev = devm_alloc_etherdev(dev, sizeof(*port_priv));
> +	if (!netdev)
> +		return -ENOMEM;
> +
> +	port_priv = netdev_priv(netdev);
> +	port_priv->netdev = netdev;
> +	port_priv->priv = priv;
> +	port_priv->cfg = priv->cfg;
> +	port_priv->nr = 0;
> +	priv->ports[0] = port_priv;
> +	SET_NETDEV_DEV(netdev, dev);
> +
> +	quirks.reset_settle_us = 50000;
> +
> +	port_priv->tc6 = oa_tc6_init(priv->spidev, netdev, &quirks);
> +	if (IS_ERR(port_priv->tc6))
> +		return PTR_ERR(port_priv->tc6);
> +
> +	priv->read_reg = adin1110_read_reg_tc6;
> +	priv->write_reg = adin1110_write_reg_tc6;
> +
> +	ret = devm_add_action_or_reset(dev, adin1110_oa_tc6_remove,
> +				       port_priv->tc6);

Spurious newline above.

> +	if (ret)
> +		return ret;
> +
> +	/* The OA TC6 framework doesn't append the FCS, let the MAC do it. */
> +	ret = adin1110_set_bits(priv, ADIN1110_CONFIG2, ADIN1110_CRC_APPEND,
> +				ADIN1110_CRC_APPEND);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (device_get_ethdev_address(dev, netdev))
> +		eth_hw_addr_random(netdev);
> +
> +	netdev->if_port = IF_PORT_10BASET;
> +	netdev->netdev_ops = &adin1110_tc6_netdev_ops;
> +	netdev->ethtool_ops = &adin1110_ethtool_ops;
> +	netdev->priv_flags |= IFF_UNICAST_FLT;
> +	netdev->netns_immutable = true;
> +
> +	return devm_register_netdev(dev, netdev);
> +}
> +
>  static int adin1110_hw_forwarding(struct adin1110_priv *priv, bool enable)
>  {
>  	int ret;
> @@ -1649,6 +1822,7 @@ static int adin1110_probe(struct spi_device *spi)
>  	const struct spi_device_id *dev_id = spi_get_device_id(spi);
>  	struct device *dev = &spi->dev;
>  	struct adin1110_priv *priv;
> +	bool oa_tc6_protocol;
>  	int ret;
>  
>  	priv = devm_kzalloc(dev, sizeof(struct adin1110_priv), GFP_KERNEL);
> @@ -1663,11 +1837,19 @@ static int adin1110_probe(struct spi_device *spi)
>  	mutex_init(&priv->lock);
>  	spin_lock_init(&priv->state_lock);
>  
> +	oa_tc6_protocol = device_property_read_bool(dev, "adi,spi-oa-tc6");
> +

Spurious newline.

> +	if (oa_tc6_protocol)
> +		return adin1110_oa_tc6_probe(priv);
> +
>  	/* use of CRC on control and data transactions is pin dependent */
>  	priv->append_crc = device_property_read_bool(dev, "adi,spi-crc");
>  	if (priv->append_crc)
>  		crc8_populate_msb(adin1110_crc_table, 0x7);
>  
> +	priv->read_reg = adin1110_read_reg_generic_spi;
> +	priv->write_reg = adin1110_write_reg_generic_spi;
> +
>  	ret = adin1110_check_spi(priv);
>  	if (ret < 0) {
>  		dev_err(dev, "Probe SPI Read check failed: %d\n", ret);
> diff --git a/include/linux/oa_tc6.h b/include/linux/oa_tc6.h
> index e0627b99e569..2fd051e1c604 100644
> --- a/include/linux/oa_tc6.h
> +++ b/include/linux/oa_tc6.h
> @@ -79,6 +79,7 @@
>  #define OA_TC6_PHY_C45_AUTO_NEG_MMS5		5	/* MMD 7 */
>  #define OA_TC6_PHY_C45_POWER_UNIT_MMS6		6	/* MMD 13 */
>  
> +struct mii_bus;

A mistake here? Doesn't seem related.

>  struct oa_tc6;
>  
>  enum oa_tc6_quirk_flag {
> -- 
> 2.43.0
>