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, 8 Nov 2022 11:21:16 +0200
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     Woojung Huh <woojung.huh@microchip.com>,
        UNGLinuxDriver@microchip.com, Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, kernel@pengutronix.de,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        Arun.Ramadoss@microchip.com
Subject: Re: [PATCH net-next v3 3/3] net: dsa: microchip: ksz8: add MTU
 configuration support
Message-ID: <20221108092116.fpwvlxz3bbvkha66@skbuf>
References: <20221108054336.4165931-1-o.rempel@pengutronix.de>
 <20221108054336.4165931-4-o.rempel@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221108054336.4165931-4-o.rempel@pengutronix.de>
Precedence: bulk

On Tue, Nov 08, 2022 at 06:43:36AM +0100, Oleksij Rempel wrote:
> +static int ksz8863_change_mtu(struct ksz_device *dev, int port, int max_frame)

Don't pass "int port" if you're not going to use it.

> +{
> +	u8 ctrl2 = 0;
> +
> +	if (max_frame <= KSZ8_LEGAL_PACKET_SIZE)
> +		ctrl2 |= KSZ8863_LEGAL_PACKET_ENABLE;
> +	else if (max_frame > KSZ8863_NORMAL_PACKET_SIZE)
> +		ctrl2 |= KSZ8863_HUGE_PACKET_ENABLE;
> +
> +	return ksz_rmw8(dev, REG_SW_CTRL_2, KSZ8863_LEGAL_PACKET_ENABLE
> +			| KSZ8863_HUGE_PACKET_ENABLE, ctrl2);

Coding conventions are to not start a new line with an operator, but to
put it at the end of the previous line:

	return ksz_rmw8(dev, REG_SW_CTRL_2, KSZ8863_LEGAL_PACKET_ENABLE |
			KSZ8863_HUGE_PACKET_ENABLE, ctrl2);

> +}
> +
> +static int ksz8795_change_mtu(struct ksz_device *dev, int port, int max_frame)

Same.

> +{
> +	u8 ctrl1 = 0, ctrl2 = 0;
> +	int ret;
> +
> +	if (max_frame > KSZ8_LEGAL_PACKET_SIZE)
> +		ctrl2 |= SW_LEGAL_PACKET_DISABLE;
> +	else if (max_frame > KSZ8863_NORMAL_PACKET_SIZE)
> +		ctrl1 |= SW_HUGE_PACKET;
> +
> +	ret = ksz_rmw8(dev, REG_SW_CTRL_1, SW_HUGE_PACKET, ctrl1);
> +	if (ret)
> +		return ret;
> +
> +	return ksz_rmw8(dev, REG_SW_CTRL_2, SW_LEGAL_PACKET_DISABLE, ctrl2);
> +}
> +
> +int ksz8_change_mtu(struct ksz_device *dev, int port, int mtu)
> +{
> +	u16 frame_size, max_frame = 0;
> +	int i;
> +
> +	frame_size = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
> +
> +	/* Cache the per-port MTU setting */
> +	dev->ports[port].max_frame = frame_size;
> +
> +	for (i = 0; i < dev->info->port_cnt; i++)
> +		max_frame = max(max_frame, dev->ports[i].max_frame);

You can do what other switches do, and instead of caching into an array,
"return 0" for everything except the CPU port. The CPU port will always
be programmed with the largest MTU.

> +
> +	switch (dev->chip_id) {
> +	case KSZ8795_CHIP_ID:
> +	case KSZ8794_CHIP_ID:
> +	case KSZ8765_CHIP_ID:
> +		return ksz8795_change_mtu(dev, port, max_frame);
> +	case KSZ8830_CHIP_ID:
> +		return ksz8863_change_mtu(dev, port, max_frame);
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +