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
| ||
|
Message-ID: <56E60ADD.4080508@electromag.com.au> Date: Mon, 14 Mar 2016 08:50:37 +0800 From: Phil Reid <preid@...ctromag.com.au> To: Giuseppe CAVALLARO <peppe.cavallaro@...com>, netdev@...r.kernel.org Cc: gabriel.fernandez@...aro.org, afaerber@...e.de, fschaefer.oss@...glemail.com, dinh.linux@...il.com, davem@...emloft.net Subject: Re: [PATCHv3 (net.git) 2/2] stmmac: fix MDIO settings G'day Giuseppe, On 11/03/2016 11:32 PM, Giuseppe CAVALLARO wrote: > On 3/11/2016 4:14 PM, Phil Reid wrote: >> G'day Giuseppe, >> >> I wont be able to test until Monday. >> Concept looks ok to me except for comment below. >> >> On 11/03/2016 9:33 PM, Giuseppe Cavallaro wrote: >>> Initially the phy_bus_name was added to manipulate the >>> driver name but It was recently just used to manage the >>> fixed-link and then to take some decision at run-time >>> inside the main (for example to skip EEE). >>> So the patch uses the is_pseudo_fixed_link and removes >>> removes the phy_bus_name variable not necessary anymore. >>> >>> The driver can manage the mdio registration by using phy-handle, >>> dwmac-mdio and own parameter e.g. snps,phy-addr. >>> This patch takes care about all these possible configurations >>> and fixes the mdio registration in case of there is a real >>> transceiver or a switch (that needs to be managed by using >>> fixed-link). >>> >>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@...com> >>> Reviewed-by: Andreas Färber <afaerber@...e.de> >>> Tested-by: Frank Schäfer <fschaefer.oss@...glemail.com> >>> Cc: Gabriel Fernandez <gabriel.fernandez@...aro.org> >>> Cc: Dinh Nguyen <dinh.linux@...il.com> >>> Cc: David S. Miller <davem@...emloft.net> >>> Cc: Phil Reid <preid@...ctromag.com.au> >>> --- >>> >>> V2: use is_pseudo_fixed_link >>> V3: parse device-tree driver parameters to allocate PHY resources >>> considering >>> DSA case (+ fixed-link). >>> >>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 11 +-- >>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 19 +----- >>> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 77 >>> ++++++++++++++++---- >>> include/linux/stmmac.h | 2 +- >>> 4 files changed, 68 insertions(+), 41 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>> index c21015b..389d7d0 100644 >>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>> @@ -271,7 +271,6 @@ static void stmmac_eee_ctrl_timer(unsigned long arg) >>> */ >>> bool stmmac_eee_init(struct stmmac_priv *priv) >>> { >>> - char *phy_bus_name = priv->plat->phy_bus_name; >>> unsigned long flags; >>> bool ret = false; >>> >>> @@ -283,7 +282,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv) >>> goto out; >>> >>> /* Never init EEE in case of a switch is attached */ >>> - if (phy_bus_name && (!strcmp(phy_bus_name, "fixed"))) >>> + if (priv->phydev->is_pseudo_fixed_link) >>> goto out; >>> >>> /* MAC core supports the EEE feature. */ >>> @@ -820,12 +819,8 @@ static int stmmac_init_phy(struct net_device *dev) >>> phydev = of_phy_connect(dev, priv->plat->phy_node, >>> &stmmac_adjust_link, 0, interface); >>> } else { >>> - if (priv->plat->phy_bus_name) >>> - snprintf(bus_id, MII_BUS_ID_SIZE, "%s-%x", >>> - priv->plat->phy_bus_name, priv->plat->bus_id); >>> - else >>> - snprintf(bus_id, MII_BUS_ID_SIZE, "stmmac-%x", >>> - priv->plat->bus_id); >>> + snprintf(bus_id, MII_BUS_ID_SIZE, "stmmac-%x", >>> + priv->plat->bus_id); >>> >>> snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id, >>> priv->plat->phy_addr); >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c >>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c >>> index 0faf163..3f5512f 100644 >>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c >>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c >>> @@ -198,29 +198,12 @@ int stmmac_mdio_register(struct net_device *ndev) >>> struct mii_bus *new_bus; >>> struct stmmac_priv *priv = netdev_priv(ndev); >>> struct stmmac_mdio_bus_data *mdio_bus_data = >>> priv->plat->mdio_bus_data; >>> + struct device_node *mdio_node = priv->plat->mdio_node; >>> int addr, found; >>> - struct device_node *mdio_node = NULL; >>> - struct device_node *child_node = NULL; >>> >>> if (!mdio_bus_data) >>> return 0; >>> >>> - if (IS_ENABLED(CONFIG_OF)) { >>> - for_each_child_of_node(priv->device->of_node, child_node) { >>> - if (of_device_is_compatible(child_node, >>> - "snps,dwmac-mdio")) { >>> - mdio_node = child_node; >>> - break; >>> - } >>> - } >>> - >>> - if (mdio_node) { >>> - netdev_dbg(ndev, "FOUND MDIO subnode\n"); >>> - } else { >>> - netdev_warn(ndev, "No MDIO subnode found\n"); >>> - } >>> - } >>> - >>> new_bus = mdiobus_alloc(); >>> if (new_bus == NULL) >>> return -ENOMEM; >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c >>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c >>> index 6a52fa1..d2322e9 100644 >>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c >>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c >>> @@ -96,6 +96,66 @@ static int dwmac1000_validate_ucast_entries(int >>> ucast_entries) >>> } >>> >>> /** >>> + * stmmac_dt_phy - parse device-tree driver parameters to allocate >>> PHY resources >>> + * @plat: driver data platform structure >>> + * @np: device tree node >>> + * @dev: device pointer >>> + * Description: >>> + * The mdio bus will be allocated in case of a phy transceiver is on >>> board; >>> + * it will be NULL if the fixed-link is configured. >>> + * If there is the "snps,dwmac-mdio" sub-node the mdio will be allocated >>> + * in any case (for DSA, mdio must be registered even if fixed-link). >>> + * The table below sums the supported configurations: >>> + * ------------------------------- >>> + * snps,phy-addr | Y >>> + * ------------------------------- >>> + * phy-handle | Y >>> + * ------------------------------- >>> + * fixed-link | N >>> + * ------------------------------- >>> + * snps,dwmac-mdio | >>> + * even if | Y >>> + * fixed-link | >>> + * ------------------------------- >>> + * >>> + * It returns true in case of the mdio needs to be registered in the >>> main. >>> + */ >>> +static bool stmmac_dt_phy(struct plat_stmmacenet_data *plat, >>> + struct device_node *np, struct device *dev) >>> +{ >>> + bool ret = true; >>> + >>> + /* If phy-handle property is passed from DT, use it as the PHY */ >>> + plat->phy_node = of_parse_phandle(np, "phy-handle", 0); >>> + if (plat->phy_node) >>> + dev_dbg(dev, "Found phy-handle subnode\n"); >>> + >>> + /* If phy-handle is not specified, check if we have a fixed-phy */ >>> + if (!plat->phy_node && of_phy_is_fixed_link(np)) { >>> + if ((of_phy_register_fixed_link(np) < 0)) >>> + return -ENODEV; >>> + >>> + dev_dbg(dev, "Found fixed-link subnode\n"); >>> + plat->phy_node = of_node_get(np); >>> + >>> + ret = false; >>> + } >>> + >>> + /* If snps,dwmac-mdio is passed from DT, always register the MDIO */ >>> + for_each_child_of_node(np, plat->mdio_node) { >>> + if (of_device_is_compatible(plat->mdio_node, "snps,dwmac-mdio")) >>> + break; >>> + } >>> + >> >> Won't this always result in plat->mdio_node being assigned to something >> if np has a child. >> Regardless of the compatible string. >> Which is why I had the child_node temp. >> Still learning so may be missing something. > > hmm, i think so, let me know as soon as you test it so I can > rework the patch and send a v4. > Tested-by: Phil Reid <preid@...ctromag.com.au> Ignore my comment above. This works fine. Test with / without compatible flag and it works as expected. At the end of the loop for_each_child_of_node sets the node to NULL. > >> >>> + if (plat->mdio_node) { >>> + dev_dbg(dev, "Found MDIO subnode\n"); >>> + ret = true; >>> + } >>> + >>> + return ret; >>> +} >>> + >>> +/** >>> * stmmac_probe_config_dt - parse device-tree driver parameters >>> * @pdev: platform_device structure >>> * @plat: driver data platform structure >>> @@ -129,30 +189,19 @@ stmmac_probe_config_dt(struct platform_device >>> *pdev, const char **mac) >>> /* Default to phy auto-detection */ >>> plat->phy_addr = -1; >>> >>> - /* If we find a phy-handle property, use it as the PHY */ >>> - plat->phy_node = of_parse_phandle(np, "phy-handle", 0); >>> - >>> - /* If phy-handle is not specified, check if we have a fixed-phy */ >>> - if (!plat->phy_node && of_phy_is_fixed_link(np)) { >>> - if ((of_phy_register_fixed_link(np) < 0)) >>> - return ERR_PTR(-ENODEV); >>> - >>> - plat->phy_node = of_node_get(np); >>> - } >>> - >>> /* "snps,phy-addr" is not a standard property. Mark it as >>> deprecated >>> * and warn of its use. Remove this when phy node support is added. >>> */ >>> if (of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr) >>> == 0) >>> dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n"); >>> >>> - if ((plat->phy_node && !of_phy_is_fixed_link(np)) || >>> plat->phy_bus_name) >>> - plat->mdio_bus_data = NULL; >>> - else >>> + /* To Configure PHY by using all device-tree supported properties */ >>> + if (stmmac_dt_phy(plat, np, &pdev->dev)) { >>> plat->mdio_bus_data = >>> devm_kzalloc(&pdev->dev, >>> sizeof(struct stmmac_mdio_bus_data), >>> GFP_KERNEL); >>> + } >>> >>> of_property_read_u32(np, "tx-fifo-depth", &plat->tx_fifo_size); >>> >>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h >>> index eead8ab..8b1ff2b 100644 >>> --- a/include/linux/stmmac.h >>> +++ b/include/linux/stmmac.h >>> @@ -94,12 +94,12 @@ struct stmmac_dma_cfg { >>> }; >>> >>> struct plat_stmmacenet_data { >>> - char *phy_bus_name; >>> int bus_id; >>> int phy_addr; >>> int interface; >>> struct stmmac_mdio_bus_data *mdio_bus_data; >>> struct device_node *phy_node; >>> + struct device_node *mdio_node; >>> struct stmmac_dma_cfg *dma_cfg; >>> int clk_csr; >>> int has_gmac; >>> >> >> > > > -- Regards Phil Reid
Powered by blists - more mailing lists