lkml.org 
[lkml]   [2023]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] net: mdio: force deassert MDIO reset signal
    > Specifying the ID as part of the compatible string works for clause 22 PHYs,
    > but for clause 45 PHYs it does not work. The code always wants to read the
    > ID from the PHY itself. But I do not understand things well enough to tell
    > whether that's a fundamental restriction of C45 or just our implementation
    > and the implementation can be changed to fix it.
    >
    > Do you have some thoughts on this?

    Do you have more details about what goes wrong? Which PHY driver is
    it? What compatibles do you put into DT for the PHY?

    To some extent, the ID is only used to find the driver. A C45 device
    has a lot of ID register, and all of them are used by phy_bus_match()
    to see if a driver matches. So you need to be careful which ID you
    pick, it needs to match the driver.

    It is the driver which decides to use C22 or C45 to talk to the PHY.
    However, we do have:

    static int phy_probe(struct device *dev)
    {
    ...
    else if (phydev->is_c45)
    err = genphy_c45_pma_read_abilities(phydev);
    else
    err = genphy_read_abilities(phydev);

    so it could be a C45 PHY probed using an ID does not have
    phydev->is_c45 set, and so it looks in the wrong place for the
    capabilities. Make sure you also have the compatible
    "ethernet-phy-ieee802.3-c45" which i think should cause is_c45 to be
    set.

    There is no fundamental restriction that i know of here, it probably
    just needs somebody to debug it and find where it goes wrong.

    Ah!

    int fwnode_mdiobus_register_phy(struct mii_bus *bus,
    struct fwnode_handle *child, u32 addr)
    {
    ...
    rc = fwnode_property_match_string(child, "compatible",
    "ethernet-phy-ieee802.3-c45");
    if (rc >= 0)
    is_c45 = true;

    if (is_c45 || fwnode_get_phy_id(child, &phy_id))
    phy = get_phy_device(bus, addr, is_c45);
    else
    phy = phy_device_create(bus, addr, phy_id, 0, NULL);


    So compatible "ethernet-phy-ieee802.3-c45" results in is_c45 being set
    true. The if (is_c45 || is then true, so it does not need to call
    fwnode_get_phy_id(child, &phy_id) so ignores whatever ID is in DT and
    asks the PHY.

    Try this, totally untested:

    diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
    index b782c35c4ac1..13be23f8ac97 100644
    --- a/drivers/net/mdio/fwnode_mdio.c
    +++ b/drivers/net/mdio/fwnode_mdio.c
    @@ -134,10 +134,10 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
    if (rc >= 0)
    is_c45 = true;

    - if (is_c45 || fwnode_get_phy_id(child, &phy_id))
    + if (fwnode_get_phy_id (child, &phy_id))
    phy = get_phy_device(bus, addr, is_c45);
    else
    - phy = phy_device_create(bus, addr, phy_id, 0, NULL);
    + phy = phy_device_create(bus, addr, phy_id, is_c45, NULL);
    if (IS_ERR(phy)) {
    rc = PTR_ERR(phy);
    goto clean_mii_ts;

    Andrew

    \
     
     \ /
      Last update: 2023-03-26 23:42    [W:5.696 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site