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;
Message-ID: <53bd4c46-17f9-91c1-fda5-bd2d09ae2ff7@gmail.com>
Date:   Thu, 10 Nov 2022 06:42:03 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH net-next v4 1/4] net: dsa: microchip: move max mtu to one
 location
Content-Language: en-US
To:     Oleksij Rempel <o.rempel@pengutronix.de>,
        Woojung Huh <woojung.huh@microchip.com>,
        UNGLinuxDriver@microchip.com, Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
Cc:     Arun Ramadoss <arun.ramadoss@microchip.com>, kernel@pengutronix.de,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org
References: <20221110122225.1283326-1-o.rempel@pengutronix.de>
 <20221110122225.1283326-2-o.rempel@pengutronix.de>
From:   Florian Fainelli <f.fainelli@gmail.com>
In-Reply-To: <20221110122225.1283326-2-o.rempel@pengutronix.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 11/10/2022 4:22 AM, Oleksij Rempel wrote:
> There are no HW specific registers, so we can process all of them
> in one location.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Tested-by: Arun Ramadoss <arun.ramadoss@microchip.com> (KSZ9893 and LAN937x)

This looks good to me, just one nit see below

[snip]

> @@ -2500,10 +2499,23 @@ static int ksz_max_mtu(struct dsa_switch *ds, int port)
>   {
>   	struct ksz_device *dev = ds->priv;
>   
> -	if (!dev->dev_ops->max_mtu)
> -		return -EOPNOTSUPP;
> +	switch (dev->chip_id) {
> +	case KSZ8563_CHIP_ID:
> +	case KSZ9477_CHIP_ID:
> +	case KSZ9563_CHIP_ID:
> +	case KSZ9567_CHIP_ID:
> +	case KSZ9893_CHIP_ID:
> +	case KSZ9896_CHIP_ID:
> +	case KSZ9897_CHIP_ID:
> +	case LAN9370_CHIP_ID:
> +	case LAN9371_CHIP_ID:
> +	case LAN9372_CHIP_ID:
> +	case LAN9373_CHIP_ID:
> +	case LAN9374_CHIP_ID:
> +		return KSZ9477_MAX_FRAME_SIZE - VLAN_ETH_HLEN - ETH_FCS_LEN;

Rename to KSZ_MAX_FRAME_SIZE to denote this is a common constant?

> +	}
>   
> -	return dev->dev_ops->max_mtu(dev, port);
> +	return -EOPNOTSUPP;
>   }
>   
>   static void ksz_set_xmii(struct ksz_device *dev, int port,
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index c6726cbd5465..27c26ee15af4 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -322,7 +322,6 @@ struct ksz_dev_ops {
>   	void (*get_caps)(struct ksz_device *dev, int port,
>   			 struct phylink_config *config);
>   	int (*change_mtu)(struct ksz_device *dev, int port, int mtu);
> -	int (*max_mtu)(struct ksz_device *dev, int port);
>   	void (*freeze_mib)(struct ksz_device *dev, int port, bool freeze);
>   	void (*port_init_cnt)(struct ksz_device *dev, int port);
>   	void (*phylink_mac_config)(struct ksz_device *dev, int port,
> @@ -588,6 +587,8 @@ static inline int is_lan937x(struct ksz_device *dev)
>   
>   #define PORT_SRC_PHY_INT		1
>   
> +#define KSZ9477_MAX_FRAME_SIZE		9000\

And here are as well.
-- 
Florian