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:   Sun, 24 Apr 2022 21:18:21 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Marek =?iso-8859-1?Q?Beh=FAn?= <kabel@kernel.org>
Cc:     Nathan Rossi <nathan@nathanrossi.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for
 port_base_addr
Message-ID: <YmWifeWNSlpNrJkl@lunn.ch>
References: <20220424141759.315303-1-nathan@nathanrossi.com>
 <20220424165228.4030aea6@thinkpad>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20220424165228.4030aea6@thinkpad>
Precedence: bulk

On Sun, Apr 24, 2022 at 04:52:28PM +0200, Marek Beh?n wrote:
> On Sun, 24 Apr 2022 14:17:59 +0000
> Nathan Rossi <nathan@nathanrossi.com> wrote:
> 
> > The other port_hidden functions rely on the port_read/port_write
> > functions to access the hidden control port. These functions apply the
> > offset for port_base_addr where applicable. Update port_hidden_wait to
> > use the port_wait_bit so that port_base_addr offsets are accounted for
> > when waiting for the busy bit to change.
> > 
> > Without the offset the port_hidden_wait function would timeout on
> > devices that have a non-zero port_base_addr (e.g. MV88E6141), however
> > devices that have a zero port_base_addr would operate correctly (e.g.
> > MV88E6390).
> 
> So basically the code is accessing the wrong register for devices with
> non-zero port_base_addr. This means that the patch should have a Fixes
> tag with the commit that introduced this bug, so that it gets
> backported to relevant stable versions.
> 
> Could you resend with Fixes tag?

I think that would be:

Fixes: 609070133aff ("net: dsa: mv88e6xxx: update code operating on hidden registers")

That patch moved the code out of chip.c into port_hidden. At the same
time, the functions got renamed from mv88e6390_hidden_read() to
mv886xxx_hidden_read(). 6390 does have zero-based ports, so was
correct before the rename. But the more generic name suggest it should
take into account devices which do not have zero based ports.

With the fixes tag added and based on met:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew