lists.openwall.net | 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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 20 Jul 2020 16:59:08 +0000 From: Ioana Ciornei <ioana.ciornei@....com> To: Russell King - ARM Linux admin <linux@...linux.org.uk> CC: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, Heiner Kallweit <hkallweit1@...il.com>, Vladimir Oltean <vladimir.oltean@....com>, Claudiu Manoil <claudiu.manoil@....com>, Alexandru Marginean <alexandru.marginean@....com>, "michael@...le.cc" <michael@...le.cc>, "olteanv@...il.com" <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> 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 <rmk+kernel@...linux.org.uk> >> >> Reviewed-by: Ioana Ciornei <ioana.ciornei@....com> >> >> 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: > > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=mcbin&id=593d56ef8c7f7d395626752f6810210471a5f5c1 > > 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 callback? 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. Ioana
Powered by blists - more mailing lists