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:   Wed, 16 Nov 2022 01:02:07 +0200
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Sean Anderson <sean.anderson@seco.com>
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>, netdev@vger.kernel.org,
        Eric Dumazet <edumazet@google.com>,
        Tim Harvey <tharvey@gateworks.com>,
        "David S . Miller" <davem@davemloft.net>,
        linux-kernel@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>
Subject: Re: [PATCH] phy: aquantia: Configure SERDES mode by default
Message-ID: <20221115230207.2e77pifwruzkexbr@skbuf>
References: <20221114210740.3332937-1-sean.anderson@seco.com>
 <20221114210740.3332937-1-sean.anderson@seco.com>
 <20221115223732.ctvzjbpeaxulnm5l@skbuf>
 <3771f5be-3deb-06f9-d0a0-c3139d098bf0@seco.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3771f5be-3deb-06f9-d0a0-c3139d098bf0@seco.com>
Precedence: bulk

On Tue, Nov 15, 2022 at 05:46:54PM -0500, Sean Anderson wrote:
> On 11/15/22 17:37, Vladimir Oltean wrote:
> > Was this patch tested and confirmed to do something sane on any platform
> > at all?
> 
> This was mainly intended for Tim to test and see if it fixed his problem.

And that is stated where? Does Tim know he should test it?
If you don't have the certainty that it works, do maintainers know not
to apply it, as many times unfortunately happens when there is no review
comment and the change looks innocuous?

Even if the change works, why would it be a good idea to overwrite some
random registers which are supposed to be configured correctly by the
firmware provided for the board? If the Linux fixup works for one board
with one firmware, how do we know it also works for another board with
the same PHY, but different firmware? Are you willing to take the risk
to break someone's system to find out?

As long as the Aquantia PHY driver doesn't contain all the necessary
steps for bringing the PHY up from a clean slate, but works on top of
what the firmware has done, changes like this make me very uncomfortable
to add any PHY ID to the Aquantia driver. I'd rather leave them with the
Generic C45 driver, even if that means I'll lose interrupt support, rate
matching and things like that.