From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 178F2C00144 for ; Fri, 29 Jul 2022 13:13:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236698AbiG2NNS (ORCPT ); Fri, 29 Jul 2022 09:13:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35856 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236728AbiG2NM7 (ORCPT ); Fri, 29 Jul 2022 09:12:59 -0400 Received: from vps0.lunn.ch (vps0.lunn.ch [185.16.172.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9E41357234; Fri, 29 Jul 2022 06:12:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=1V1EPIx5SWj6VhkkBaKrSzdQ8VeFraOV5vkmO7pp+ZI=; b=cml+Ja7pPUqiLDpd+4o90bg8hR kt2rrlGhY32N0WuXoh6rEfMdftCIe9WJM+2BFwShQKVr+s/cZSmVP1U+h5hzeuF5DSiKsjpUhe6hu jlxfc9OymUbOCQuYV2lgWbZuoD/03bbe5bcFknsqNDAmU35falGqQKC3OHzYbNm4FooI=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1oHPmk-00Bv3L-FD; Fri, 29 Jul 2022 15:12:10 +0200 Date: Fri, 29 Jul 2022 15:12:10 +0200 From: Andrew Lunn To: Oleksij Rempel Cc: Woojung Huh , UNGLinuxDriver@microchip.com, Vivien Didelot , Florian Fainelli , Vladimir Oltean , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , kernel@pengutronix.de, linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: Re: [PATCH net v1 1/1] net: dsa: microchip: don't try do read Gbit registers on non Gbit chips Message-ID: References: <20220728131725.40492-1-o.rempel@pengutronix.de> <20220729090513.GA10850@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220729090513.GA10850@pengutronix.de> Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Jul 29, 2022 at 11:05:13AM +0200, Oleksij Rempel wrote: > On Thu, Jul 28, 2022 at 03:25:35PM +0200, Andrew Lunn wrote: > > On Thu, Jul 28, 2022 at 03:17:25PM +0200, Oleksij Rempel wrote: > > > Do not try to read not existing or wrong register on chips without > > > GBIT_SUPPORT. > > > > > > Fixes: c2e866911e25 ("net: dsa: microchip: break KSZ9477 DSA driver into two files") > > > Signed-off-by: Oleksij Rempel > > > --- > > > drivers/net/dsa/microchip/ksz9477.c | 8 +++++++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c > > > index c73bb6d383ad..f6bbd9646c85 100644 > > > --- a/drivers/net/dsa/microchip/ksz9477.c > > > +++ b/drivers/net/dsa/microchip/ksz9477.c > > > @@ -316,7 +316,13 @@ void ksz9477_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data) > > > break; > > > } > > > } else { > > > - ksz_pread16(dev, addr, 0x100 + (reg << 1), &val); > > > + /* No gigabit support. Do not read wrong registers. */ > > > + if (!(dev->features & GBIT_SUPPORT) && > > > + (reg == MII_CTRL1000 || reg == MII_ESTATUS || > > > + reg == MII_STAT1000)) > > > > Does this actually happen? > > > > If i remember this code correctly, it tries to make the oddly looking > > PHY look like a normal PHY. phylib is then used to drive the PHY? > > > > If i have that correct, why is phylib trying to read these registers? > > It should know there is no 1G support, and should skip them. > > It looks like currently undocumented silicon errata. According to the > data sheet, the BMSR_ESTATEN should not be set BMSR_ERCAP, but this bits > are set. > > The question is what is the proper place to implement it. There is same > PHYid for most KSZ switch PHYs, it is no possible to detect it by PHYid. > I have following options: > - add chips specific quirk in the ksz9477_r_phy(), just remove > BMSR_ESTATEN and BMSR_ERCAP. > - notify about errata over get_phy_flags and implement get_caps quirk in > the PHY driver. I would do the first. The DSA driver is already doing some emulation of a normal PHY, so it seems odd to push a workaround into the PHY driver when it can be part of the emulation. Andrew