On Fri, Sep 08, 2023 at 07:59:18PM +0530, Parthiban Veerasooran wrote:
> The LAN8650/1 is designed to conform to the OPEN Alliance 10BASE‑T1x
> MAC‑PHY Serial Interface specification, Version 1.1. The IEEE Clause 4
> MAC integration provides the low pin count standard SPI interface to any
> microcontroller therefore providing Ethernet functionality without
> requiring MAC integration within the microcontroller. The LAN8650/1
> operates as an SPI client supporting SCLK clock rates up to a maximum of
> 25 MHz. This SPI interface supports the transfer of both data (Ethernet
> frames) and control (register access).
>
> By default, the chunk data payload is 64 bytes in size. A smaller payload
> data size of 32 bytes is also supported and may be configured in the
> Chunk Payload Size (CPS) field of the Configuration 0 (OA_CONFIG0)
> register. Changing the chunk payload size requires the LAN8650/1 be reset
> and shall not be done during normal operation.
>
> The Ethernet Media Access Controller (MAC) module implements a 10 Mbps
> half duplex Ethernet MAC, compatible with the IEEE 802.3 standard.
> 10BASE-T1S physical layer transceiver integrated into the LAN8650/1. The
> PHY and MAC are connected via an internal Media Independent Interface
> (MII).
>
> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@xxxxxxxxxxxxx>
Hi Parthiban,
thanks for your patches.
Some minor feedback on this one follows.
...
> diff --git a/drivers/net/ethernet/microchip/lan865x.c b/drivers/net/ethernet/microchip/lan865x.c
...
> +static int lan865x_set_hw_macaddr(struct net_device *netdev)
> +{
> + u32 regval;
> + bool ret;
> + struct lan865x_priv *priv = netdev_priv(netdev);
> + const u8 *mac = netdev->dev_addr;
Please arrange local variables in Networking code in reverse xmas tree
order - longest line to shortest.
This tool can be of assistance here:
https://github.com/ecree-solarflare/xmastree
...
> +static int lan865x_probe(struct spi_device *spi)
> +{
> + struct net_device *netdev;
> + struct lan865x_priv *priv;
> + u32 regval;
> + int ret;
> +
> + netdev = alloc_etherdev(sizeof(struct lan865x_priv));
> + if (!netdev)
> + return -ENOMEM;
> +
> + priv = netdev_priv(netdev);
> + priv->netdev = netdev;
> + priv->spi = spi;
> + priv->msg_enable = 0;
> + spi_set_drvdata(spi, priv);
> + SET_NETDEV_DEV(netdev, &spi->dev);
> +
> + ret = lan865x_get_dt_data(priv);
> + if (ret)
> + return ret;
> +
> + spi->rt = true;
> + spi_setup(spi);
> +
> + priv->tc6 = oa_tc6_init(spi, netdev);
> + if (!priv->tc6) {
> + ret = -ENOMEM;
> + goto error_oa_tc6_init;
> + }
> +
> + if (priv->cps == 32) {
> + regval = CCS_Q0_TX_CFG_32;
> + ret = oa_tc6_write_register(priv->tc6, CCS_Q0_TX_CFG, ®val, 1);
> + if (ret)
> + return ret;
> +
> + regval = CCS_Q0_RX_CFG_32;
> + ret = oa_tc6_write_register(priv->tc6, CCS_Q0_RX_CFG, ®val, 1);
> + if (ret)
> + return ret;
> + }
> +
> + if (oa_tc6_configure(priv->tc6, priv->cps, priv->protected, priv->txcte,
> + priv->rxcte))
> + goto err_macphy_config;
Jumping to err_macphy_config will result in this function returning ret.
However, ret will be 0 at this point. Perhaps it should be set to an
error value.
Flagged by Smatch.
> +
> + ret = lan865x_phy_init(priv);
> + if (ret)
> + goto error_phy;
> +
> + if (device_get_ethdev_address(&spi->dev, netdev))
> + eth_hw_addr_random(netdev);
> +
> + ret = lan865x_set_hw_macaddr(netdev);
> + if (ret) {
> + if (netif_msg_probe(priv))
> + dev_err(&spi->dev, "Failed to configure MAC");
> + goto error_set_mac;
> + }
> +
> + netdev->if_port = IF_PORT_10BASET;
> + netdev->irq = spi->irq;
> + netdev->netdev_ops = &lan865x_netdev_ops;
> + netdev->watchdog_timeo = TX_TIMEOUT;
> + netdev->ethtool_ops = &lan865x_ethtool_ops;
> + ret = register_netdev(netdev);
> + if (ret) {
> + if (netif_msg_probe(priv))
> + dev_err(&spi->dev, "Register netdev failed (ret = %d)",
> + ret);
> + goto error_netdev_register;
> + }
> +
> + phy_start(priv->phydev);
> + return 0;
> +
> +error_netdev_register:
> +error_set_mac:
> + phy_disconnect(priv->phydev);
> + mdiobus_unregister(priv->mdiobus);
> + mdiobus_free(priv->mdiobus);
> +error_phy:
> +err_macphy_config:
> + oa_tc6_deinit(priv->tc6);
> +error_oa_tc6_init:
> + free_netdev(priv->netdev);
> + return ret;
> +}
...
