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:34:51 +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: <ZGtTW1sNDT8si9/r@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?RvJ4BDMYilDy+dKJRBfGWo4/cq29YG31ogOJRJbfIJ5wHUQOSt8WX8nAlK0H?=
 =?us-ascii?Q?aUPXP4/rOWO3nvS6RUCrnRk+XD7QZhiNIR76TTEMzZR7IugqNQF1MBLM/gVL?=
 =?us-ascii?Q?WTOKAo/0M5Nwn866SQVMK9wo0LF1foUVwJbvj9YVyFvUbSnDKqsINprng4f8?=
 =?us-ascii?Q?jAd5iO46toMmbN3f2+5EgRSOiYE/AT5ZW0By8U1qucFkY+HYn4CpQzXywdQY?=
 =?us-ascii?Q?HuXxSV2JesKFUhRynl3+2kEJ8FyCyTXx/RiQB+j5LVr5e3/93MVd8XrM/Fo7?=
 =?us-ascii?Q?UTg6KVftuuwA/Pjz2FoHXkXMilGvnFR1ig8chvpPrUHBu4jrvXJGLN4PSwHn?=
 =?us-ascii?Q?p6PvIp5DPRRs1cMlvEadprhAj71i4jk5Jn4T+lOQIvylF7iMU6QCrMT6cIE7?=
 =?us-ascii?Q?TQq5MeDyUFwlOf+t+sH5Zjyje5IcMzgecuxsSXrF5gJRjroX6vvfWnNzlo0x?=
 =?us-ascii?Q?p0N6PX7nACqBa/FrtO2x2LhtiA/l+hb8Wiz1GO1e8nQWzspaBE8kdwifMgZt?=
 =?us-ascii?Q?O1IvE6Uupy+wReztTQX47+4HtZx7KKOQYNc3+a+MRYcJdao5ds7cFZAZNk/4?=
 =?us-ascii?Q?WlrBOiQGgs5M0/A10wtc9VhkmwlaSQI0gX9D7rBsQfsWJBF94FAVATlwz+ct?=
 =?us-ascii?Q?zImGdZ65MhN/lb5cnggM6xePB21tvzm9/tua3tIqQXJOkK5upi2u40k1Bgnt?=
 =?us-ascii?Q?3ValUyV6ZaH1wELG5GpYecETFtjt9hnuIDRrKpmDELxPdRI9DSzfifTPiXeV?=
 =?us-ascii?Q?MGf+xTk13bVcCsRyjkMXJbEaTmptV59UWaou2aucyMy/kkg5D3DUnwCg2QqM?=
 =?us-ascii?Q?wPl1EBLEAydoGIQER/vzMA9pcJaYHwtoqYR+LE3eNFOY2sGge8B2lDJcuUhq?=
 =?us-ascii?Q?EhGgCEJjfnuKGPJgEjSO6XuT+KKaHGWo1hf86MtcuzlzIYdD350DlvkTd7rI?=
 =?us-ascii?Q?UB81A+RMJSNVC2Xk4MnocjgspxZf9LCBqooGeQjnTgLQOCiu3I/IURFqwALG?=
 =?us-ascii?Q?1FNtwkTfFFe4Mp93wIExLfj9pSkA77bzqT87ejErqHgCkt0rh0qBa2D+4CR4?=
 =?us-ascii?Q?C8O9FI6S3xVIpTTdeDpffxjIWjaGAK1kYfFPxbVvzSQ3LM0XRKnvb4rcxVNU?=
 =?us-ascii?Q?xrG/k0JmN/B70Pq6QQPoiv+AsStZpLbzFxnUnJqwjTwBeiyjT6CnrB0l+Oi3?=
 =?us-ascii?Q?rCB8JCktJFo1MZAeGiCBkCezCleuMfd2qAviREFVtu/Zp+NHDdgq2sgLBZF2?=
 =?us-ascii?Q?0tRecPQa3VrF34iDH9zyIMIt69lXYGou0/gEglRn/pTMsN7ksfQc6BJ4mx0v?=
 =?us-ascii?Q?XjyMtrMD8HrpEgC+JoJYW/bRKA31uS1M7FTZa3GSbz2GMcWZO+rEee5VQ4PD?=
 =?us-ascii?Q?eo22o1hi2PYIs8xQNVWZi7fQsNR9tS1yqZotvfLliBL+upNQMPbO52zRW0Sw?=
 =?us-ascii?Q?mDai3zZKcS7f8r9fJ+uxMNHuWQFCG7Kw8c8w0RCimbkfEqaO5qKHC4thFczW?=
 =?us-ascii?Q?TzZeAe1sW5p6G2qB2Ah39cOqPvVLRXDSeQFfNpVhKtMsP02ZEakAgdgNimvW?=
 =?us-ascii?Q?hqCkXD3Yrq8wYOprftK3lOqRJ6dzq013s+t+TvOsJQd9HfE8lJ9la2GhpLlm?=
 =?us-ascii?Q?VTQlLvdUawrpq208u6mj8SZaSiUkasMeA9iMyU1z8+jQhoDIcIkSOFm4uKvD?=
 =?us-ascii?Q?+soO1A=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d166d279-0e37-4591-ccf4-08db5ab8940e
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:35:00.9243
 (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: v1aAzN53XF683NTOmyymWa7C6e3F87HOeHb5GzzTuzJ1LZsVnQUB+ALxNTfq2IeIVMfhHZML2eGNIGSX3cpaRid2xTFSdAyhW5CsYPHMXrk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR13MB5176
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 netdev_tx_t bcmasp_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct bcmasp_intf *intf = netdev_priv(dev);
> +	struct device *kdev = &intf->parent->pdev->dev;
> +	int spb_index, nr_frags, ret, i, j;
> +	unsigned int total_bytes, size;
> +	struct bcmasp_tx_cb *txcb;
> +	dma_addr_t mapping, valid;
> +	struct bcmasp_desc *desc;
> +	bool csum_hw = false;
> +	skb_frag_t *frag;

Hi Justin,

Please use reverse xmas tree order - lognest line to shortest - for local
variables, even in cases of assignment such as this one.

In this case I would suggest splitting the declarations and assignment
of kdev. Something line this:

	struct bcmasp_intf *intf = netdev_priv(dev);
	int spb_index, nr_frags, ret, i, j;
	unsigned int total_bytes, size;
	struct bcmasp_tx_cb *txcb;
	dma_addr_t mapping, valid;
	struct bcmasp_desc *desc;
	bool csum_hw = false;
	struct device *kdev;
	skb_frag_t *frag;

	kdev = &intf->parent->pdev->dev;

...