[linux-sunxi] Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i

On Tue, Jun 27, 2017 at 6:17 PM, Icenowy Zheng [off-list ref] wrote:

? 2017?6?27? GMT+08:00 ??6:11:47, Chen-Yu Tsai [off-list ref] ??:

quoted

On Tue, Jun 27, 2017 at 5:41 PM, Maxime Ripard
[off-list ref] wrote:

quoted

On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:

quoted

Hi,

(CC:ing some people from that Rockchip dmwac series)

On 27/06/17 09:21, Corentin Labbe wrote:

quoted

On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:

quoted

On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe
[off-list ref] wrote:

quoted

On Mon, Jun 26, 2017 at 01:18:23AM +0100, Andr? Przywara wrote:

quoted

On 31/05/17 08:18, Corentin Labbe wrote:

quoted

The dwmac-sun8i is a heavy hacked version of stmmac hardware

by

quoted

quoted

quoted

quoted

quoted

quoted

quoted

allwinner.
In fact the only common part is the descriptor management and

the first

quoted

quoted

quoted

quoted

quoted

quoted

quoted

register function.

Hi,

I know I am a bit late with this, but while adapting the U-Boot

driver

quoted

quoted

quoted

quoted

quoted

quoted

to the new binding I was wondering about the internal PHY

detection:

quoted

quoted

quoted

quoted

quoted

quoted

So here you seem to deduce the usage of the internal PHY by the

PHY

quoted

quoted

quoted

quoted

quoted

quoted

interface specified in the DT (MII = internal, RGMII =

external).

quoted

quoted

quoted

quoted

quoted

quoted

I think I raised this question before, but isn't it perfectly

legal for

quoted

quoted

quoted

quoted

quoted

quoted

a board to use MII with an external PHY even on those SoCs that

feature

quoted

quoted

quoted

quoted

quoted

quoted

an internal PHY?
On the first glance that does not make too much sense, but

apart from

quoted

quoted

quoted

quoted

quoted

quoted

not being the correct binding to describe all of the SoCs

features I see

quoted

quoted

quoted

quoted

quoted

quoted

two scenarios:
1) A board vendor might choose to not use the internal PHY

because it

quoted

quoted

quoted

quoted

quoted

quoted

has bugs, lacks features (configurability) or has other issues.

For

quoted

quoted

quoted

quoted

quoted

quoted

instance I have heard reports that the internal PHY makes the

SoC go

quoted

quoted

quoted

quoted

quoted

quoted

rather hot, possibly limiting the CPU frequency. By using an

external

quoted

quoted

quoted

quoted

quoted

quoted

MII PHY (which are still cheaper than RGMII PHYs) this can be

avoided.

quoted

quoted

quoted

quoted

quoted

quoted

2) A PHY does not necessarily need to be directly connected to
magnetics. Indeed quite some boards use (RG)MII to connect to a

switch

quoted

quoted

quoted

quoted

quoted

quoted

IC or some other network circuitry, for instance fibre

connectors.

quoted

quoted

quoted

quoted

quoted

quoted

So I was wondering if we would need an explicit:
      allwinner,use-internal-phy;
boolean DT property to signal the usage of the internal PHY?
Alternatively we could go with the negative version:
      allwinner,disable-internal-phy;

Or what about introducing a new "allwinner,internal-mii-phy"

compatible

quoted

quoted

quoted

quoted

quoted

quoted

string for the *PHY* node and use that?

I just want to avoid that we introduce a binding that causes us
headaches later. I think we can still fix this with a followup

patch

quoted

quoted

quoted

quoted

quoted

quoted

before the driver and its binding hit a release kernel.

Cheers,
Andre.

I just see some patch, where "phy-mode = internal" is valid.
I will try to find a way to use it

Can you provide a link?

https://lkml.org/lkml/2017/6/23/479

quoted

I'm not a fan of using phy-mode for this. There's no guarantee

what

quoted

quoted

quoted

quoted

mode the internal PHY uses. That's what phy-mode is for.

I can understand Chen-Yu's concerns, but ...

quoted

For each soc the internal PHY mode is know and setted in

emac_variant/internal_phy

quoted

quoted

quoted

So its not a problem.

that is true as well, at least for now.

So while I agree that having a separate property to indicate the

usage

quoted

quoted

of the internal PHY would be nice, I am bit tempted to use this

easier

quoted

quoted

approach and piggy back on the existing phy-mode property.

We're trying to fix an issue that works for now too.

If we want to consider future weird cases, then we must consider all
of them. And the phy mode changing is definitely not really far
fetched.

I guess the issue is whether it's likely that the vendor puts 2
internal
PHYs in one SoC, and they use different modes and can be switched
around.
Otherwise it's fixed for a given SoC, and we can just handle that with
the per-SoC GMAC compatible.

Maybe Florian could tell us if this was one of the intended use cases
for the "internal" phy mode.

As for Rockchip, AFAIK they have 2 MACs, one is connected to the
internal
PHY, while the other is connected to the external interface, and there
is
no muxing involved, unlike Allwinner's solution.

quoted

I agree with Chen-Yu, and I really feel like the compatible solution
you suggested would cover both your concerns, and ours.

If using a PHY compatible is the solution, we could just use the
"ethernet-phy-idAAAA.BBBB" style one, and put in the bogus ID that
Allwinner used.

Care must be taken to put this at the board level for boards using
the internal PHY, or we'd have to delete or override the property
in all other boards.

Ideally I think the internal PHY device node should _not_ be in
the SoC level .dtsi file. If we select the external interface, then
there's no connection to the internal PHY, and that device node becomes
unusable and bogus. This is something I think should be fixed
regardless
of the phy-mode discussion above.

I think it should be in the SoC DTSI, as it's part of the SoC.

But it makes sense to set status to disabled defaultly.

Then you end up stepping on it when you add an external PHY
with the same address, and start wondering why things don't
work.

ChenYu