lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 20 Jul 2020 16:59:08 +0000
From:   Ioana Ciornei <>
To:     Russell King - ARM Linux admin <>
CC:     Andrew Lunn <>,
        Florian Fainelli <>,
        Heiner Kallweit <>,
        Vladimir Oltean <>,
        Claudiu Manoil <>,
        Alexandru Marginean <>,
        "" <>,
        "" <>,
        "David S. Miller" <>,
        Jakub Kicinski <>,
        "" <>
Subject: Re: [PATCH RFC net-next 12/13] net: phylink: add struct phylink_pcs

On 7/20/20 7:26 PM, Russell King - ARM Linux admin wrote:
> On Mon, Jul 20, 2020 at 03:50:57PM +0000, Ioana Ciornei wrote:
>> On 6/30/20 5:29 PM, Russell King wrote:
>>> Add a way for MAC PCS to have private data while keeping independence
>>> from struct phylink_config, which is used for the MAC itself. We need
>>> this independence as we will have stand-alone code for PCS that is
>>> independent of the MAC.  Introduce struct phylink_pcs, which is
>>> designed to be embedded in a driver private data structure.
>>> This structure does not include a mdio_device as there are PCS
>>> implementations such as the Marvell DSA and network drivers where this
>>> is not necessary.
>>> Signed-off-by: Russell King <>
>> Reviewed-by: Ioana Ciornei <>
>> I integrated and used the phylink_pcs structure into the Lynx PCS just
>> to see how everything fits. Pasting below the main parts so that we can
>> catch early any possible different opinions on how to integrate this:
>> The basic Lynx structure looks like below and the main idea is just to
>> encapsulate the phylink_pcs structure and the mdio device (which in some
>> other cases might not be needed).
>> struct lynx_pcs {
>>          struct phylink_pcs pcs;
>>          struct mdio_device *mdio;
>>          phy_interface_t interface;
>> };
>> The lynx_pcs structure is requested by the MAC driver with:
>> struct lynx_pcs *lynx_pcs_create(struct mdio_device *mdio)
>> {
>> (...)
>>          lynx_pcs->mdio = mdio;
>>          lynx_pcs->pcs.ops = &lynx_pcs_phylink_ops;
>>          lynx_pcs->pcs.poll = true;
>>          return lynx_pcs;
>> }
>> And then passed to phylink with something like:
>> phylink_set_pcs(pl, &lynx_pcs->pcs);
>> For DSA it's a bit less straightforward because the .setup() callback
>> from the dsa_switch_ops is run before any phylink structure has been
>> created internally. For this, a new DSA helper can be created that just
>> stores the phylink_pcs structure per port:
>> void dsa_port_phylink_set_pcs(struct dsa_switch *ds, int port,
>>                                struct phylink_pcs *pcs)
>> {
>>          struct dsa_port *dp = dsa_to_port(ds, port);
>>          dp->pcs = pcs;                                         but I do
>> }
>> and at the appropriate time, from dsa_slave_setup, it can really install
>> the phylink_pcs with phylink_set_pcs.
>> The other option would be to add a new dsa_switch ops that requests the
>> phylink_pcs for a specific port - something like phylink_get_pcs.
> It is entirely possible to set the PCS in the mac_prepare() or
> mac_config() callbacks - but DSA doesn't yet support the mac_prepare()
> callback (because it needs to be propagated through the DSA way of
> doing things.)
> An example of this can be found at:
> where we choose between the XLGMAC and GMAC pcs_ops structures
> depending on the interface mode we are configuring for.  Note that
> this is one of the devices that the PCS does not appear as a
> distinctly separate entity in the register set, at least in the
> GMAC side of things.

Thanks for the info, I didn't get that this is possible by reading the 
previous patch. Maybe this would be useful in the documentation of the 

Back to the DSA, I don't feel like we gain much by setting up the 
phylink_pcs from another callback: we somehow force the driver to 
implement a phylink_mac_prepare dsa_switch_ops just so that it sets up 
the phylink_pcs, which for me at least would not be intuitive.


Powered by blists - more mailing lists