On Thu, Feb 13, 2014 at 05:29:52AM +0000, Florian Fainelli wrote:
> This patch adds support for configuring the port multiplexer hardware
> which resides in front of the GENET Ethernet MAC controller. This allows
> us to support:
>
> - internal PHYs (using drivers/net/phy/bcm7xxx.c)
> - MoCA PHYs which are an entirely separate hardware block not covered
> here
> - external PHYs and switches
>
> Note that MoCA and switches are currently supported using the emulated
> "fixed PHY" driver.
>
> Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
> ---
> Changes since v1:
> - fixed MDIO crash/warning when Device Tree probing fails
> - removed the use of priv->phy_type and use priv->phy_interface
> directly
>
> drivers/net/ethernet/broadcom/genet/bcmmii.c | 481 +++++++++++++++++++++++++++
> 1 file changed, 481 insertions(+)
> create mode 100644 drivers/net/ethernet/broadcom/genet/bcmmii.c
[...]
> +static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
> +{
> + struct device_node *dn = priv->pdev->dev.of_node;
> + struct device *kdev = &priv->pdev->dev;
> + struct device_node *mdio_dn;
> + const __be32 *fixed_link;
This looks a bit odd. Could we not have a common parser for fixed-link
properties?
> + u32 propval;
> + int ret, sz;
> +
> + mdio_dn = of_get_next_child(dn, NULL);
> + if (!mdio_dn) {
> + dev_err(kdev, "unable to find MDIO bus node\n");
> + return -ENODEV;
> + }
Could you please check that this is the node you expect (by looking at
the compatible string list).
> +
> + ret = of_mdiobus_register(priv->mii_bus, mdio_dn);
> + if (ret) {
> + dev_err(kdev, "failed to register MDIO bus\n");
> + return ret;
> + }
> +
> + /* Check if we have an internal or external PHY */
> + priv->phy_dn = of_parse_phandle(dn, "phy-handle", 0);
> + if (priv->phy_dn) {
> + if (!of_property_read_u32(priv->phy_dn, "max-speed", &propval))
> + priv->phy_speed = propval;
Is there no way to find this out without reading values directly off of
the PHY? It seems like something we should have an abstraction for.
> + } else {
> + /* Read the link speed from the fixed-link property */
> + fixed_link = of_get_property(dn, "fixed-link", &sz);
> + if (!fixed_link || sz < sizeof(*fixed_link)) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + priv->phy_speed = be32_to_cpu(fixed_link[2]);
Similarly can we not have a common fixed-link parser? Or abstraction
such that you query the phy regardless of what it is and how its binding
represents this?
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
