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;
Date:   Tue, 29 Oct 2019 13:38:54 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     Mark Rutland <mark.rutland@arm.com>, devicetree@vger.kernel.org,
        Jay Cliburn <jcliburn@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Chris Snook <chris.snook@gmail.com>,
        linux-kernel@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>,
        "David S. Miller" <davem@davemloft.net>,
        Paul Burton <paul.burton@mips.com>,
        Rob Herring <robh+dt@kernel.org>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        James Hogan <jhogan@kernel.org>,
        Russell King <linux@armlinux.org.uk>, netdev@vger.kernel.org,
        linux-mips@vger.kernel.org,
        Vivien Didelot <vivien.didelot@gmail.com>
Subject: Re: [PATCH v4 5/5] net: dsa: add support for Atheros AR9331 build-in
 switch
Message-ID: <20191029123854.GN15259@lunn.ch>
References: <20191022055743.6832-1-o.rempel@pengutronix.de>
 <20191022055743.6832-6-o.rempel@pengutronix.de>
 <20191023005850.GG5707@lunn.ch>
 <20191029071404.pl34q4rmadusc2u5@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191029071404.pl34q4rmadusc2u5@pengutronix.de>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Oleksij

> > > +static void ar9331_sw_port_disable(struct dsa_switch *ds, int port)
> > > +{
> > > +	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
> > > +	struct regmap *regmap = priv->regmap;
> > > +	int ret;
> > > +
> > > +	ret = regmap_write(regmap, AR9331_SW_REG_PORT_STATUS(port), 0);
> > > +	if (ret)
> > > +		dev_err_ratelimited(priv->dev, "%s: %i\n", __func__, ret);
> > > +}
> > 
> > I've asked this before, but i don't remember the answer. Why are
> > port_enable and port_disable the same?
> 
> I have only MAC TX/RX enable bit. This bit is set by phylink_mac_link_up and
> removed by phylink_mac_link_down.
> The port enable I use only to set predictable state of the port
> register: all bits cleared. May be i should just drop port enable
> function? What do you think? 

At minimum, it needs a comment about why enable and disable are the
same. If i keep asking, others will as well.

If there is nothing useful to do, then drop it.

   Andrew