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:   Mon, 22 May 2023 13:38:45 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     Justin Chen <justin.chen@broadcom.com>
Cc:     netdev@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
        dri-devel@lists.freedesktop.org,
        bcm-kernel-feedback-list@broadcom.com, justinpopo6@gmail.com,
        f.fainelli@gmail.com, davem@davemloft.net,
        florian.fainelli@broadcom.com, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, opendmb@gmail.com,
        andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk,
        richardcochran@gmail.com, sumit.semwal@linaro.org,
        christian.koenig@amd.com
Subject: Re: [PATCH net-next v3 3/6] net: bcmasp: Add support for ASP2.0
 Ethernet controller
Message-ID: <ZGtURVrdqz536ai7@corigine.com>
References: <1684531184-14009-1-git-send-email-justin.chen@broadcom.com>
 <1684531184-14009-4-git-send-email-justin.chen@broadcom.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1684531184-14009-4-git-send-email-justin.chen@broadcom.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?PV4dodSeGbVkN93XpPG8pG4kzgcYRssQIgLgSkuhQDAMqIBxzEPFNKQTA07m?=
 =?us-ascii?Q?J9kT6PznBpTTMxYnG7mNbgvc75zL2g1TMcIziJjnGZaTotCgoR2c+Udk4FRQ?=
 =?us-ascii?Q?C8E0tvWM4aKV0CrGEI5z7w0A6fcclrycZqwtbBTjspG7SKvp4G4ylMM/dEtA?=
 =?us-ascii?Q?XD/PsjsGpKi9SjQYPFCsuTSRZ1YByZWUMFgVN7r0ZaueFa/kH9SqcF9OaofJ?=
 =?us-ascii?Q?lnn5V86RbGPyFzsDCWs+ggAyf3UHMziydbOdPBWclpC5cDUM9J1b+LUR6hYK?=
 =?us-ascii?Q?XwGrNVw+Ka55eg5G96W09WbrzLiujaAOSnSZOriW8Wl7VcBcODWy0ttu41GS?=
 =?us-ascii?Q?1MWTSqSiWzAyE+b9ooLF9Ykh/cl/0cJTt3v9eqymGxhcBYC6Ew1w6kAIOGpo?=
 =?us-ascii?Q?uIGxENf4X3e4i0BdFy9RZsheKMZMTGReSNrwGSgqUYxPu98X5q26tqsyumkE?=
 =?us-ascii?Q?IpEX7v7ifzntk2xiCs8wzLZaQ8nXdd4T6qEKvJiySYjX22RotI/bP08Beft8?=
 =?us-ascii?Q?xA0Rn/FUalGIzw1pOhMTvGvzP/Xa2m+YoIHNNpbGje0PbXVFy2WLauzreJ4Q?=
 =?us-ascii?Q?7aVKS1HVWQn4t5DY3mzPyEtAlNrzjCR3PDE+wNaU3Qx2afpb3rR1DdAGkWaO?=
 =?us-ascii?Q?twK+lOO3fHVIS7K8up12cy0M0VWQVmDqH9z3mBvI9uzsHpBRWEtUDd79fmvW?=
 =?us-ascii?Q?6qist4Hqx/GA8Zp/mKnK9HJiMCdCxNz0/FPTkdYO7QM0bfFWUxFbzRlDLiDu?=
 =?us-ascii?Q?sYMT17R9DHzfk5zfyUejvqOtDcgRIJoQZpwgdKtJC7Vee/EEoC0VURZmWDKg?=
 =?us-ascii?Q?cEDTGWBAymd2VzXj8wyPABTgoTsDT1ACIc3glQ/gYf5WsZab5HQsPNP5lWMs?=
 =?us-ascii?Q?+9lgNNBYUluvDo5rlsB5JQ3/j+XyH8nP/LMNaANxU+m2IfKOkqsmvwl3aq5g?=
 =?us-ascii?Q?BR7lWAKW2rZQATzE6i2vVo1uKw1T/yI4HVl6DcCy42svVS2xn7UIQ2WStyt9?=
 =?us-ascii?Q?r/HJ+eq6ZeKdxgZ1HmU+5nuHwK3ZbBaaoDLS+PVdl68bA3L5Wc9LUdXTjYiP?=
 =?us-ascii?Q?4L7m6VZg3OredY/kj6T0DE6oszuNZpHpCgDAlDqHnmvYyZS9tOPvKFvbn+u3?=
 =?us-ascii?Q?KtZ9l8g6kkztI9zpZezYf1f6l4uLIbrg0X6Kb9dBg/zmGp/+M833uBToj3tR?=
 =?us-ascii?Q?kdtFw+LqFQDfwrD3LMNBxtW7XsqjU6CN0BJe2WN7mkmYNOJWYGFTJbuvaAVn?=
 =?us-ascii?Q?zv7cDs2IggmeSH8sBEFKeNPbOU8/nZ3uC4m6S/Ir8LkMbrdGDI7WJynfAmhw?=
 =?us-ascii?Q?TTKvnysQOJ3w2eoSG7J91crW6+QVdXtwxjY7E4qov06Axu1IKkBB3DVcjv5W?=
 =?us-ascii?Q?nKCVufMcGE0KFE1n04HtCR/HNwyXAeh9iSNOqszGKpSRE9aRCG9odovcp+J+?=
 =?us-ascii?Q?6CM4Y4YSWvOu3gWT8pzB9+et8of/+SfJXI73XU907JpF7V1cRZEI5t3JGb+w?=
 =?us-ascii?Q?7Wbg0zWLQeAtlDdq7Scs9jVMNxeFNEnevVKuW4JMfvlvOm4SU0MBG/CMA0oo?=
 =?us-ascii?Q?ZsWEM7V6YcRsBQ1zRABVgbmqtgX6+lUaIifKQP61cJt85dVN1fx0/VqmbYhn?=
 =?us-ascii?Q?++J4pOtpWvMbmoxMkL2r6KOEuxIAVri0fHxjcRI1JMpxaOMOc3RoOmhvG50K?=
 =?us-ascii?Q?4GsP5A=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cebcf2f8-acfe-4b79-2a4f-08db5ab91ee0
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 May 2023 11:38:53.9077
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: NG+iAL61qcf3MrUlwfeHQzAk3riFhZD2GXbyjX6ov7whkOCFejN6romeE0YFKstOvip3g7yiptkcYMrdAtmr4Q9AWVe9QJnkRzOEwnZ81AM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR13MB4700
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, May 19, 2023 at 02:19:41PM -0700, Justin Chen wrote:
> Add support for the Broadcom ASP 2.0 Ethernet controller which is first
> introduced with 72165. This controller features two distinct Ethernet
> ports that can be independently operated.
> 
> This patch supports:
> 
> - Wake-on-LAN using magic packets
> - basic ethtool operations (link, counters, message level)
> - MAC destination address filtering (promiscuous, ALL_MULTI, etc.)
> 
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> Signed-off-by: Justin Chen <justin.chen@broadcom.com>

...

> +static int bcmasp_netfilt_get_reg_offset(struct bcmasp_priv *priv,
> +					 struct bcmasp_net_filter *nfilt,
> +					 enum asp_netfilt_reg_type reg_type,
> +					 u32 offset)
> +{
> +	u32 block_index, filter_sel;
> +
> +	if (offset < 32) {
> +		block_index = ASP_RX_FILTER_NET_L2;
> +		filter_sel = nfilt->hw_index;
> +	} else if (offset < 64) {
> +		block_index = ASP_RX_FILTER_NET_L2;
> +		filter_sel = nfilt->hw_index + 1;
> +	} else if (offset < 96) {
> +		block_index = ASP_RX_FILTER_NET_L3_0;
> +		filter_sel = nfilt->hw_index;
> +	} else if (offset < 128) {
> +		block_index = ASP_RX_FILTER_NET_L3_0;
> +		filter_sel = nfilt->hw_index + 1;
> +	} else if (offset < 160) {
> +		block_index = ASP_RX_FILTER_NET_L3_1;
> +		filter_sel = nfilt->hw_index;
> +	} else if (offset < 192) {
> +		block_index = ASP_RX_FILTER_NET_L3_1;
> +		filter_sel = nfilt->hw_index + 1;
> +	} else if (offset < 224) {
> +		block_index = ASP_RX_FILTER_NET_L4;
> +		filter_sel = nfilt->hw_index;
> +	} else if (offset < 256) {
> +		block_index = ASP_RX_FILTER_NET_L4;
> +		filter_sel = nfilt->hw_index + 1;
> +	}

Hi Justin,

Perhaps it is not possible.
But it seems to me that it would be nice to have:

	else {
		return -EINVAL;
	}

Otherwise, if that case does occur, block_index and filter_sel
will be used uninitialised.

> +
> +	switch (reg_type) {
> +	case ASP_NETFILT_MATCH:
> +		return ASP_RX_FILTER_NET_PAT(filter_sel, block_index,
> +					     (offset % 32));
> +	case ASP_NETFILT_MASK:
> +		return ASP_RX_FILTER_NET_MASK(filter_sel, block_index,
> +					      (offset % 32));
> +	default:
> +		return -EINVAL;
> +	}
> +}

...