Re: [PATCH v3 2/2] Support NVIDIA BlueField-3 pinctrl driver

From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Asmaa Mnebhi <asmaa@nvidia.com>
Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-acpi@vger.kernel.org
Subject: Re: [PATCH v3 2/2] Support NVIDIA BlueField-3 pinctrl driver
Date: Sat, 11 Feb 2023 13:55:32 +0200	[thread overview]
Message-ID: <Y+eCNEHcmo4qapcI@smile.fi.intel.com> (raw)
In-Reply-To: <acd85e20d3e8d45ce3254e6ffe5ad2b5039b1a34.1676042188.git.asmaa@nvidia.com>

On Fri, Feb 10, 2023 at 10:39:41AM -0500, Asmaa Mnebhi wrote:
> This patch adds support to the BlueField-3 SoC pin controller.

Please read Submitting Patches on how to create better commit message.

> It allows muxing individual GPIOs to switch from the default
> hardware mode to software controlled mode.

...

> +config PINCTRL_MLXBF
> +	tristate "NVIDIA BlueField-3 SoC Pinctrl driver"
> +	depends on (MELLANOX_PLATFORM && ARM64 && ACPI)

This is wrong.
Please make sure you cover more testing.
Also, do you really need an ACPI dependency?

> +	select PINMUX
> +	select GPIOLIB
> +	select GPIOLIB_IRQCHIP
> +	select GPIO_MLXBF3
> +	help
> +	  This selects the pinctrl driver for BlueField-3 SoCs.

Same comment as per previous patch.

> +// SPDX-License-Identifier: GPL-2.0-only or BSD-3-Clause
> +
> +/*
> + * Copyright (C) 2022 NVIDIA CORPORATION & AFFILIATES
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/platform_device.h>

Same comments as per previous patch.

> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>

Based on the above, I stop here and wait for next version where comments given
anywhere are applied to all appropriate places.

...

> +	BUILD_BUG_ON(ARRAY_SIZE(mlxbf_pinctrl_single_group_names) !=
> +		     ARRAY_SIZE(mlxbf_pinctrl_single_group_pins));

static_assert().

-- 
With Best Regards,
Andy Shevchenko