On Fri, Jul 31, 2020 at 06:54:04PM +0000, Asmaa Mnebhi wrote:
Hi Asmaa
Please don't send HTML obfusticated emails to mailing lists.
> > +static int mlxbf_gige_mdio_read(struct mii_bus *bus, int phy_add, int
>
> > +phy_reg) {
>
> > + struct mlxbf_gige *priv = bus->priv;
>
> > + u32 cmd;
>
> > + u32 ret;
>
> > +
>
> > + /* If the lock is held by something else, drop the request.
>
> > + * If the lock is cleared, that means the busy bit was cleared.
>
> > + */
>
>
>
> How can this happen? The mdio core has a mutex which prevents parallel access?
>
>
>
> This is a HW Lock. It is an actual register. So another HW entity can be
> holding that lock and reading/changing the values in the HW registers.
You have not explains how that can happen? Is there something in the
driver i missed which takes a backdoor to read/write MDIO
transactions?
> > + ret = mlxbf_gige_mdio_poll_bit(priv,
> > MLXBF_GIGE_MDIO_GW_LOCK_MASK);
>
> > + if (ret)
>
> > + return -EBUSY;
>
>
>
> PHY drivers are not going to like that. They are not going to retry. What is
> likely to happen is that phylib moves into the ERROR state, and the PHY driver
> grinds to a halt.
>
>
>
> This is a fairly quick HW transaction. So I don’t think it would cause and
> issue for the PHY drivers. In this case, we use the micrel KSZ9031. We haven’t
> seen issues.
So you have happy to debug hard to find and reproduce issues when it
does happen? Or would you like to spend a little bit of time now and
just prevent it happening at all?
Andrew
