lkml.org 
[lkml]   [2022]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v7 net-next 06/10] net: dsa: microchip: add support for phylink management
    Hi,

    On Fri, Feb 04, 2022 at 11:14:56PM +0530, Prasanna Vengateshan wrote:
    > +static void lan937x_phylink_mac_link_up(struct dsa_switch *ds, int port,
    > + unsigned int mode,
    > + phy_interface_t interface,
    > + struct phy_device *phydev,
    > + int speed, int duplex,
    > + bool tx_pause, bool rx_pause)
    > +{
    > + struct ksz_device *dev = ds->priv;
    > +
    > + /* Internal PHYs */
    > + if (lan937x_is_internal_phy_port(dev, port))
    > + return;
    > +
    > + if (phylink_autoneg_inband(mode)) {
    > + dev_err(ds->dev, "In-band AN not supported!\n");
    > + return;
    > + }

    No need to check this in the link_up() method - if this were true,
    you've already printed an error in the mac_config() method and this
    one doesn't provide any additional useful information.

    > +
    > + lan937x_config_interface(dev, port, speed, duplex,
    > + tx_pause, rx_pause);
    > +}
    > +
    > +static void lan937x_phylink_get_caps(struct dsa_switch *ds, int port,
    > + struct phylink_config *config)
    > +{
    > + struct ksz_device *dev = ds->priv;
    > +
    > + config->mac_capabilities = MAC_100FD;
    > +
    > + /* internal T1 PHY */
    > + if (lan937x_is_internal_base_t1_phy_port(dev, port)) {
    > + __set_bit(PHY_INTERFACE_MODE_INTERNAL,
    > + config->supported_interfaces);
    > + } else {
    > + /* MII/RMII/RGMII ports */
    > + if (!lan937x_is_internal_base_tx_phy_port(dev, port)) {

    Please consider:

    } else if (!lan937x_is_internal_base_tx_phy_port(dev, port)) {
    /* MII/RMII/RGMII ports */
    ...
    }

    to avoid needing two tabs to indent, which probably makes:

    > + config->mac_capabilities |= MAC_100HD | MAC_SYM_PAUSE |
    > + MAC_ASYM_PAUSE | MAC_10 |
    > + MAC_1000FD;

    able to be laid out with the two pause modes first followed by the
    speeds.

    > + phy_interface_set_rgmii(config->supported_interfaces);
    > +
    > + __set_bit(PHY_INTERFACE_MODE_MII,
    > + config->supported_interfaces);
    > + __set_bit(PHY_INTERFACE_MODE_RMII,
    > + config->supported_interfaces);
    > + }
    > + }

    You seem to be a non-legacy driver in this patch (good!) so please also
    add:

    config->legacy_pre_march2020 = false;

    while DSA is transitioned over. Thanks.

    > +}
    > +
    > const struct dsa_switch_ops lan937x_switch_ops = {
    > .get_tag_protocol = lan937x_get_tag_protocol,
    > .setup = lan937x_setup,

    Thanks.

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

    \
     
     \ /
      Last update: 2022-02-04 19:15    [W:3.381 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site