| 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: Fri, 11 Mar 2016 16:32:45 +0100
From: Giuseppe CAVALLARO <peppe.cavallaro@...com>
To: Phil Reid <preid@...ctromag.com.au>, <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
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.
Thanks
peppe
>
>> + 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;
>>
>
>
Powered by blists - more mailing lists