lkml.org 
[lkml]   [2023]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH net-next V1 6/7] net: lan743x: Add support to the phylink framework
    On Tue, Oct 17, 2023 at 03:12:07PM +0530, Raju Lakkaraju wrote:
    > +static void lan743x_phylink_mac_config(struct phylink_config *config,
    > + unsigned int link_an_mode,
    > + const struct phylink_link_state *state)
    > +{
    > + struct net_device *netdev = to_net_dev(config->dev);
    > + struct lan743x_adapter *adapter = netdev_priv(netdev);
    > + bool status;
    > + int ret;
    > +
    > + lan743x_mac_cfg_update(adapter, state->link, state->speed,
    > + state->advertising);

    Another case of not reading the phylink documentation... :( I *really*
    don't see why we bother to write documentation, from my experience, it
    seems to be totally a write-only thing... no one seems to bother
    reading it.

    /**
    * mac_config() - configure the MAC for the selected mode and state
    * @config: a pointer to a &struct phylink_config.
    * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
    * @state: a pointer to a &struct phylink_link_state.
    *
    * Note - not all members of @state are valid. In particular,
    * @state->lp_advertising, @state->link, @state->an_complete are never
    * guaranteed to be correct, and so any mac_config() implementation must
    * never reference these fields.
    ...
    * The action performed depends on the currently selected mode:
    *
    * %MLO_AN_FIXED, %MLO_AN_PHY:
    ...
    * Valid state members: interface, advertising.
    * Deprecated state members: speed, duplex, pause.
    ...
    * %MLO_AN_INBAND:
    ...
    * Valid state members: interface, an_enabled, pause, advertising.

    So "link" and "speed" are not valid. They're certainly not valid now
    that the pre-March 2020 legacy support has been removed.

    The only time that you get to know what speed the link is operating at
    is after the link has been negotiated and is up - and you will be told
    via the mac_link_up() function (or in the case of a PCS, via the
    pcs_link_up() function.) You can't know before that point what speed
    the link will be operating at because its dependent on the results of
    link negotiation.

    I haven't bothered to look more deeply at this patch, because I regard
    any patch that gets this wrong to be utter trash (sorry that's a bit
    hard, but I see people not bothering to read documentation all too
    often and it pisses me off that I've gone to the bother of writing it
    and it just gets ignored.)

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

    \
     
     \ /
      Last update: 2023-10-17 12:46    [W:6.761 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site