lkml.org 
[lkml]   [2022]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [RFC PATCH v11 net-next 06/10] net: dsa: microchip: add support for phylink management
    On Fri, Mar 25, 2022 at 10:23:37PM +0530, Prasanna Vengateshan wrote:
    > phylink_get_caps() is implemented and reused KSZ commmon API for
    > phylink_mac_link_down() operation
    >
    > lan937x_phylink_mac_config configures the interface using
    > lan937x_mac_config and lan937x_phylink_mac_link_up configures
    > the speed/duplex/flow control.
    >
    > Currently SGMII & in-band neg are not supported & it will be
    > added later.

    If SGMII is also an option, then what you're doing in
    lan937x_parse_dt_rgmii_delay() looks wrong:

    /* skip for internal ports */
    if (lan937x_is_internal_phy_port(dev, p))
    continue;

    if (of_property_read_u32(port, "rx-internal-delay-ps", &val))
    val = 0;

    err = lan937x_set_rgmii_delay(dev, p, val, false);
    if (err)
    break;

    Right now you assume that "isn't internal PHY" is the same as "is RGMII",
    but this is not actually the future-proof way of doing things. Also
    please consider that the driver you write now may end up booting on a DT
    blob from the future. Do you do something sane when a port is configured
    for a phy-mode you don't recognize? For one thing, you try to configure
    RGMII delays on it, as far as I can see.

    > +static void lan937x_phylink_get_caps(struct dsa_switch *ds, int port,
    > + struct phylink_config *config)
    > +{
    > + struct ksz_device *dev = ds->priv;
    > +
    > + /* non legacy driver */
    > + config->legacy_pre_march2020 = false;
    > +
    > + 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 if (lan937x_is_rgmii_port(dev, port)) {
    > + /* MII/RMII/RGMII ports */
    > + config->mac_capabilities |= MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
    > + MAC_100HD | MAC_10 | MAC_1000FD;
    > + 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);
    > + }

    No supported_interfaces for the 100base-TX internal PHY port? Does it
    pass validation?

    > +}
    > +
    > const struct dsa_switch_ops lan937x_switch_ops = {
    > .get_tag_protocol = lan937x_get_tag_protocol,
    > .setup = lan937x_setup,
    > @@ -307,6 +369,10 @@ const struct dsa_switch_ops lan937x_switch_ops = {
    > .port_fast_age = ksz_port_fast_age,
    > .port_max_mtu = lan937x_get_max_mtu,
    > .port_change_mtu = lan937x_change_mtu,
    > + .phylink_get_caps = lan937x_phylink_get_caps,
    > + .phylink_mac_link_down = ksz_mac_link_down,
    > + .phylink_mac_config = lan937x_phylink_mac_config,
    > + .phylink_mac_link_up = lan937x_phylink_mac_link_up,
    > };
    >
    > int lan937x_switch_register(struct ksz_device *dev)
    > --
    > 2.30.2
    >

    \
     
     \ /
      Last update: 2022-04-09 01:42    [W:3.835 / U:0.200 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site