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, 24 Jul 2023 16:41:57 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     nick.hawkins@hpe.com
Cc:     verdun@hpe.com, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
        netdev@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 2/5] net: hpe: Add GXP UMAC MDIO
Message-ID: <ZL6Ntazi84za5YX+@corigine.com>
References: <20230721212044.59666-1-nick.hawkins@hpe.com>
 <20230721212044.59666-3-nick.hawkins@hpe.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230721212044.59666-3-nick.hawkins@hpe.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?EntDJg4asg3y6SQLEtqaPVEPgUTS/L0AlJ5lnSjFMvjSshXQk9LfHg+i+6hT?=
 =?us-ascii?Q?TdwmdAkc0kALh+I8B4G53O4CvHN6G8zB+eCNo+aO3r1Jmpam5AmeoEa9cfTc?=
 =?us-ascii?Q?2cUR+6TPfApSix9l6NUlkMyKCDCfTx5JxDjPoAQcqm3v941XCj/BuOtnpbqw?=
 =?us-ascii?Q?ieB8kZHZiO1p7Q91+84OoNHidDt30Lc74CpqzPmZMW94Ld24Hc2HnKR/2y9c?=
 =?us-ascii?Q?o6dVsxNJQxLHsdXRX8uCaqwFjISxojsiN+luTAjkOHA/dX0I9ZvanlvUrVH3?=
 =?us-ascii?Q?FtvdreXsBszDYuSX5WN6F1komK5T8VHEP+JJ1ZZNfGVdKqvpETg9dAMsF8ZK?=
 =?us-ascii?Q?4yDCP6Ne4g4mpxlYa/cE1y6P2GlCQi9SfExnsYY4Qdx16bJ/IVXQH508jUFM?=
 =?us-ascii?Q?FrOk04N8ZZ2PCG3B+36nRLgzlBGG9jMG9Fk/to96odV4ts2l12WXKvyAh4KB?=
 =?us-ascii?Q?xX5Md16vF07Bzty5jz2If1iO4mJsgGJrg/Xuv2qv84K7r4OpjFC58DzdAnD2?=
 =?us-ascii?Q?sh/7Md9FOi8wbaVyPsTHQdCi2IjxLk1fwfuX3SGCxIAIoWKzjwtx9pov9V5E?=
 =?us-ascii?Q?BOIAkUdc71e8hzp2eQcVzsw3nhhmoCoEzspe2Gcz/Ho8zxN/8Wi5Wf9IMDd4?=
 =?us-ascii?Q?yj9bGuSqZ7F1R/vTiymsm4Ti+XZpU98OREIgxty+vSKBmkdX5S1rHmo4zhjk?=
 =?us-ascii?Q?rMtgYUqwOFV34W2khGXxlQYD/iZhBadVOemuejpPTX1LGMCT+kbOirwS6/w0?=
 =?us-ascii?Q?2QCivdOyMrx78Am3sxmAQ2juKU54MvjrhsQOSMo6zpBb/QDUIjPNSt953n8t?=
 =?us-ascii?Q?RPC+nMRbQSc7foJNQU5pDbZzXCBXdn9xdqHWwpj8r5FJUtdIYPmUw6l6/pUd?=
 =?us-ascii?Q?1aYtCtceW02Lntg7jF348tP4r4H6/zNIdZejGGesWfm1vri1ijvKPtv3Yc2P?=
 =?us-ascii?Q?JiGXd6qcQjXtPqoIgYth30jIeo1H17Zvl69OmQ2ALglRqBp0xtThT447m/5u?=
 =?us-ascii?Q?auFGkej3dntze72jghwj3RAsJ/hx4WgySxO/EqawYaVWN6X3JmHfq2UrLI28?=
 =?us-ascii?Q?fbSgR05kNm+PQkIOOwnd8d/ywt1HH8dg9Y2Hb8wKte4d4yEJ8hRaqz3aG+HM?=
 =?us-ascii?Q?YuI2mK1/fDtPSCRs7LjNbi4KfKFBxeFoH32OtLuPjbLDxb5YSavxFWOEOfO7?=
 =?us-ascii?Q?zdOrJLo148ceS7l9+wzLV6LOjQhsjgfxO35gS5O+hfJbUBjJDePUOs0JioKt?=
 =?us-ascii?Q?1wFWoVsLygfbDkAI2QY2Mho8KBwd7RW8QKpUXWBbtBXLWjr2TXj/mT0Arwgx?=
 =?us-ascii?Q?eV6wfPSijIhxSTJ1JsGVn/cCVT3e5XtJ/1LmOlCe7PtmdHwKKVX3i2emAaci?=
 =?us-ascii?Q?OkGioAfdWVBgPXEBeq9jkZ8BNW8i37e3ouz9W/uv4GsOBqMRT5Tj5e09StDH?=
 =?us-ascii?Q?hsLt9Wznm28VT+gES67YPJlE3CO5aGt1MGMrEasogPdtGrKmIKNgGHBOaMZQ?=
 =?us-ascii?Q?5aoGUo8PmN1W8jgK2a/0IfewUZZ3lhcq6OWAZgJRHmOUhXXuPzZWpP6/BA1U?=
 =?us-ascii?Q?CmJrzhABz7WaV/Z1zJYd7oXVrXuF8GKdJKhnIzVVBYAnemXkGG1hMgsZ2Hi6?=
 =?us-ascii?Q?7kJOwY0Nhwb38C1OUJHml42s7P0uTX79QZTtLJde22wobfXvCp6Sta4DZGBU?=
 =?us-ascii?Q?PU6RMQ=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 866ba56a-6c07-47d0-e388-08db8c5425d1
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Jul 2023 14:42:04.4106
 (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: fxxeCqF/oMzfyccTjJRsJkwcHBGT327y9ZSw5x1JHLJbwIbHt96+T/E4JCsHoDvSNS9qkterTBb/88rmbE8lZQeVKX9RRfU0pY8iKqsMjSU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR13MB6184
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, Jul 21, 2023 at 04:20:41PM -0500, nick.hawkins@hpe.com wrote:

...

Hi Nick,

> +static int umac_mdio_read(struct mii_bus *bus, int phy_id, int reg)
> +{
> +	struct umac_mdio_priv *umac_mdio = bus->priv;
> +	unsigned int value;
> +	unsigned int status;

Please use reverse xmas tree - longest line to shortest - for
local variable declarations in new Networking code.

...

> +static int umac_mdio_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct mii_bus *bus;
> +	struct umac_mdio_priv *umac_mdio;

Ditto.

> +
> +	int ret;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "fail to get resource\n");
> +		return -ENODEV;
> +	}
> +
> +	bus = devm_mdiobus_alloc_size(&pdev->dev,
> +				      sizeof(struct umac_mdio_priv));
> +	if (!bus) {
> +		dev_err(&pdev->dev, "failed to alloc mii bus\n");
> +		return -ENOMEM;
> +	}
> +
> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(&pdev->dev));
> +
> +	bus->name	= dev_name(&pdev->dev);
> +	bus->read	= umac_mdio_read,
> +	bus->write	= umac_mdio_write,

Should the comas (',') on the two lines above be semicolons (';') ?

> +	bus->parent	= &pdev->dev;
> +	umac_mdio = bus->priv;
> +	umac_mdio->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (!umac_mdio->base) {
> +		dev_err(&pdev->dev, "failed to do ioremap\n");
> +		return -ENODEV;
> +	}
> +
> +	platform_set_drvdata(pdev, umac_mdio);
> +
> +	ret = of_mdiobus_register(bus, pdev->dev.of_node);
> +
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}

...