Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
IronPort-SDR: sZB/W3tX2EDNQIDG5CobiUoShToM0PuH8HJmpWULERpmgYPZ8+UaMGWL+b6t98wAYzEUtQDAq/
 1eSwGjkW5bpN7ipa/cExrMcyHCEn4QtAibrfBoC6aT9D/JFZXCY9eY7i3KwwDkR8KtBmTcYNjG
 qkd+rK8faIgwKALGdqKaZcw7CoBUii40SnJLoQAoeKvy9ZO+TyEgtIldlz9Ti7/1f19JVO/+kw
 c8VWOIrv7rIVB2UnZuSD0X06b/+/CXD8zulySRHvD1Nn0q5om+OpXQ/3V9aQBnFakg210a3og0
 C0Q=
IronPort-SDR: BNIMxixaKYbDbaNh/qECj2bdl5U81jLwJ28F61hn/AKoqlxNFn8XZ6uA3Y2g+Md3D3flcA54+w
 NTKCoH4ywraeauwofQxJd71PozGlxaKy7cSw0wLYgQqGJJhOtGzNjZWcLB+DyJtx/vWs2yEXtU
 UsB8kEhOewiLKe1y7ppmassKMADNTUgBpnS6IM6+9F6Hh51xgOinshRXhVH4OiBKVpaM2gXgnE
 gFvI8tjeRDItH2Aa2ah4tbF0EtVHuOuVbzOyu9/iMdIpmy28mbIw8iLMKa/U05NoalZVNy0uQc
 110=
Message-ID: <a796bf7cfb1f72a888522050320624546950c281.camel@ew.tq-group.com>
Subject: Re: (EXT) Re: [PATCH net-next 1/4] net: dsa: allow switch drivers
 to override default slave PHY addresses
From:   Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
To:     Florian Fainelli <f.fainelli@gmail.com>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        andrew@lunn.ch, vivien.didelot@gmail.com, davem@davemloft.net,
        kuba@kernel.org
Date:   Tue, 31 Mar 2020 11:09:32 +0200
In-Reply-To: <6a306ad4-c029-03a3-7a1c-0fdadc13d386@gmail.com>
References: <20200330135345.4361-1-matthias.schiffer@ew.tq-group.com>
         <6a306ad4-c029-03a3-7a1c-0fdadc13d386@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 2020-03-30 at 20:04 -0700, Florian Fainelli wrote:
> 
> On 3/30/2020 6:53 AM, Matthias Schiffer wrote:
> > Avoid having to define a PHY for every physical port when PHY
> > addresses
> > are fixed, but port index != PHY address.
> > 
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com
> > >
> 
> You could do this much more elegantly by doing this with Device Tree
> and
> specifying the built-in PHYs to be hanging off the switch's internal
> MDIO bus and specifying the port to PHY address mapping, you would
> only
> patch #4 then.

This does work indeed, but it seems we have different ideas on
elegance.

I'm not happy about the fact that an implementor needs to study the
switch manual in great detail to find out about things like the PHY
address offsets when the driver could just to the right thing by
default. Requiring this only for some switch configurations, while
others work fine with the defaults, doesn't make this any less
confusing (I'd even argue that it would be better if there weren't any
default PHY and IRQ mappings for the switch ports, but I also
understand that this can't easily be removed at this point...)

In particular when PHY IRQ support is desired (not implemented on the
PHY driver side for this switch yet; not sure if my current project
will require it), indices are easy to get wrong - which might not be
noticed as long as there is no PHY driver with IRQ support for the port
PHYs, potentially breaking existing Device Trees with future kernel
updates. For this reason, I think at least patch #2 should be
considered even if #1 and #3 are rejected.

Kind regards,
Matthias