Re: [PATCH 1/2] of_mdio: add new DT property 'link' for fixed-link

[Stas Sergeev]

10.07.2015 00:15, Florian Fainelli ÐÐÑÐÑ:
> On 09/07/15 13:43, Stas Sergeev wrote:
> > 09.07.2015 21:24, Florian Fainelli ÐÐÑÐÑ:
> > > (there is no such thing as linux-net@xxxxxxxxxxxxxxx, please remove it
> > > from your future submissions).
> > >
> > > On 09/07/15 10:38, Stas Sergeev wrote:
> > > > Currently for fixed-link the link state is always set to UP.
> > > Not quite true, this is always a driver decision to make.
> > But what about this part of of_mdio.c:of_phy_register_fixed_link():
> > ---
> >
> >       fixed_link_node = of_get_child_by_name(np, "fixed-link");
> >       if (fixed_link_node) {
> >          status.link = 1
> >
> > ---
> This seems like a logical consequence of finding a "fixed-link" property
> for the DT node of interest. If no such property exist, then we do not
> set anything.
>
> > > > This patch introduces the new property 'link' that accepts the
> > > > following string arguments: "up", "down" and "auto".
> > > > "down" may be needed if the link is physically unconnected.
> > > In which case you probably do not even care about inserting such a
> > > property in the first place, do you? What would be the value of forcibly
> > > having a link permanently down (not counting loopback)?
> > The DTs have a common parts that are included by other
> > parts. So if you include the definition of your SoC that have
> > all ethernets defined, and you only set up the external things
> > like PHYs, then I would see a potential use for "down".
> "down" is equivalent to using a status = "disabled", in fact the latter
> is much better since you can even conserve energy and resources by not
> enabling something which is not usable.
OK, agree.
So I'll probably go for something like
autoneg = 1 | 0;

> > This doesn't work.
> > It appears even if the driver supports it and wants to use it, the
> > PHY HW may simply not generate the inband status. This is actually
> > the whole point why we have a regression now. It is _currently_
> > a driver decision, and that doesn't work for some people.
> > The point of this patch set is to make it a DT decision instead.
> Then, if the in-band status indication is not reliable (which really
> should be completely understood),
Agree!
But this is not something I can help with.
Sebastien Rannou reports the problem, please ask him whatever
you see fits to get a better understanding of a problem.
The fact that his HW does not generate the inband status, is
_my own guess_.

>   you can just ignore the in-band status
> and use all the parameter in a 'fixed-link' property, should not we?
I don't think there is any way at all to find out if the inband stat is
usable or not. I think only the user (or manufacturer) can decide
on that. If there is any way to do a guess-work, that would be an
entirely different story.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/