Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 25 Apr 2023 05:20:35 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Jakub Kicinski <kuba@kernel.org>
Cc:     Daniel Golle <daniel@makrotopia.org>,
        Heiner Kallweit <hkallweit1@gmail.com>, netdev@vger.kernel.org,
        linux-mediatek@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Qingfang Deng <dqfext@gmail.com>,
        SkyLake Huang <SkyLake.Huang@mediatek.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
Subject: Re: [PATCH v2] net: phy: add driver for MediaTek SoC built-in GE PHYs
Message-ID: <651ef153-7555-4ae1-a068-cbe5f0da7e34@lunn.ch>
References: <ZEPU6oahOGwknkSc@makrotopia.org>
 <20230424183755.3fac65b0@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230424183755.3fac65b0@kernel.org>
Precedence: bulk

On Mon, Apr 24, 2023 at 06:37:55PM -0700, Jakub Kicinski wrote:
> On Sat, 22 Apr 2023 13:36:58 +0100 Daniel Golle wrote:
> > Some of MediaTek's Filogic SoCs come with built-in gigabit Ethernet
> > PHYs which require calibration data from the SoC's efuse.
> > Despite the similar design the driver doesn't share any code with the
> > existing mediatek-ge.c, so add support for these PHYs by introducing a
> > new driver for only MediaTek's ARM64 SoCs.
> 
> Andrew, Heiner, how do you feel about this driver?

It is 95% magic values in magic registers which nobody is every going
to understand without the datasheet. Assuming any of it is actually in
the data sheet. I really think the firmware in this PHY needs a
re-write to avoid exposing all this to the OS. But i don't know if we
have that level of NACK. Having said that, there is a nice quote in
LWN from Thomas Gleixer about this:

https://lwn.net/Articles/928946/

This does seem like a case of 'throw hardware/firmware over the fence
and let software folks deal with it.' There is no other PHY like it in
Linux. So i would like to reject it, but i then suspect we are
punishing the wrong people, Daniel not MediaTek.

I would also drop the LED configuration parts. Leave it at reset
defaults for the moment. It seems like the generic support for
controlling PHY LEDs is making progress and will be merged next cycle.

	    Andrew