Received-SPF: pass (google.com: domain of linux-kernel+bounces-141372-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 11 Apr 2024 18:03:44 +0100
From: Lee Jones <lee@kernel.org>
To: Bhargav Raviprakash <bhargav.r@ltts.com>
Cc: linux-kernel@vger.kernel.org, m.nirmaladevi@ltts.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	conor+dt@kernel.org, jpanis@baylibre.com,
	devicetree@vger.kernel.org, arnd@arndb.de,
	gregkh@linuxfoundation.org, lgirdwood@gmail.com, broonie@kernel.org,
	linus.walleij@linaro.org, linux-gpio@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, nm@ti.com, vigneshr@ti.com,
	kristo@kernel.org, eblanc@baylibre.com
Subject: Re: [PATCH v6 03/11] mfd: tps6594: add regmap config in match data
Message-ID: <20240411170344.GK2399047@google.com>
References: <20240408124047.191895-1-bhargav.r@ltts.com>
 <20240408124047.191895-4-bhargav.r@ltts.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240408124047.191895-4-bhargav.r@ltts.com>

On Mon, 08 Apr 2024, Bhargav Raviprakash wrote:

> Introduces a new struct tps6594_match_data. This struct holds fields for
> chip id and regmap config. Using this struct in of_device_id data field.
> This helps in adding support for TPS65224 PMIC.
> 
> Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
> Acked-by: Julien Panis <jpanis@baylibre.com>
> ---
>  drivers/mfd/tps6594-i2c.c   | 24 ++++++++++++++++--------
>  drivers/mfd/tps6594-spi.c   | 24 ++++++++++++++++--------
>  include/linux/mfd/tps6594.h | 11 +++++++++++
>  3 files changed, 43 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mfd/tps6594-i2c.c b/drivers/mfd/tps6594-i2c.c
> index c125b474b..9e2ed48b7 100644
> --- a/drivers/mfd/tps6594-i2c.c
> +++ b/drivers/mfd/tps6594-i2c.c
> @@ -192,10 +192,16 @@ static const struct regmap_config tps6594_i2c_regmap_config = {
>  	.write = tps6594_i2c_write,
>  };
>  
> +static const struct tps6594_match_data match_data[] = {
> +	[TPS6594] = {TPS6594, &tps6594_i2c_regmap_config},
> +	[TPS6593] = {TPS6593, &tps6594_i2c_regmap_config},
> +	[LP8764] = {LP8764, &tps6594_i2c_regmap_config},

Nit: There should be spaces after the '{' and before the '}'.

> +};
> +
>  static const struct of_device_id tps6594_i2c_of_match_table[] = {
> -	{ .compatible = "ti,tps6594-q1", .data = (void *)TPS6594, },
> -	{ .compatible = "ti,tps6593-q1", .data = (void *)TPS6593, },
> -	{ .compatible = "ti,lp8764-q1",  .data = (void *)LP8764,  },
> +	{ .compatible = "ti,tps6594-q1", .data = &match_data[TPS6594], },
> +	{ .compatible = "ti,tps6593-q1", .data = &match_data[TPS6593], },
> +	{ .compatible = "ti,lp8764-q1",  .data = &match_data[LP8764], },

Not keen on this.  Why do you pass the regmap data through here and
leave everything else to be matched on device ID?  It would be better to
keep passing the device ID through and match everything off of that.


-- 
Lee Jones [李琼斯]