Re: [PATCH 0/6] mvneta: SGMII-based in-band link state signaling

2015-03-27 7:20 GMT-07:00 Stas Sergeev [off-list ref]:

27.03.2015 16:59, Andrew Lunn пишет:

quoted

quoted

But there is no MDIO, because SGMII AFAIK doesn't need MDIO.
SGMII has in-band status, but for some reason it seems currently
linux is not ready for such setup - this is what my patch addresses.

Hacking the fixed-link driver feels wrong to me. You probably want to
implement an SGMII-link driver. With some refactoring you can probably
re-use some of the fixed-link driver code, but it should be a separate
driver, with its own DT binding.

Well I took the path of "the least resistance" of course...
and the one that doesn't look unreasonable to me, but the separate
driver is an option too. Though it will really be just the "same
fixed-link with 3 added functions to change properties". Do we really
need 2 mostly similar drivers? Esp when one includes the functionality
of the other, and just adds a bit on top? Maybe having just one, with
all the capabilities added?
For example, maybe someone will later want the ability to alter the
properties of fixed-link with ethtool or alike? Will this also require
"just another fixed link driver"?
Having one fully-featured driver will allow us to add the "sgmii-link"
DT binding as an alias to "fixed-link" and some function like
of_phy_fixed_link_is_sgmii() that will tell us whether we need an
in-band status or not.

quoted

But i'm no export here. So lets see what others say.

Right. If people will be negative to an addition to fixed-link driver,
I'll take a look into adding a new DT binding - something I'd really
like to avoid doing. :) More work, more code, and yet even a new config
option - this have to be justified.

See my other reply, I think that by register a fixed_link_update
callback and adjusting fixed_phy_status based on the SGMII in-band
decoded data you would get all you want to work without major surgery
to the current code. If we are missing information in fixed_phy_status
to support SGMII-specific properties, maybe we could add those.
-- 
Florian