Linux-GPIO Archive mirror
 help / color / mirror / Atom feed
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
Date: Thu, 11 Apr 2024 18:03:44 +0100	[thread overview]
Message-ID: <20240411170344.GK2399047@google.com> (raw)
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 [李琼斯]

  reply	other threads:[~2024-04-11 17:03 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-08 12:40 [PATCH v6 00/11] Add support for TI TPS65224 PMIC Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 01/11] mfd: tps6594: Add register definitions " Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 02/11] mfd: tps6594: use volatile_table instead of volatile_reg Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 03/11] mfd: tps6594: add regmap config in match data Bhargav Raviprakash
2024-04-11 17:03   ` Lee Jones [this message]
2024-04-15 13:10     ` [RESEND PATCH v1 05/13] mfd: tps6594-spi: Add TI TPS65224 PMIC SPI Bhargav Raviprakash
2024-04-16 12:25       ` Lee Jones
2024-04-17 10:35     ` [PATCH v6 03/11] mfd: tps6594: add regmap config in match data Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 04/11] dt-bindings: mfd: ti,tps6594: Add TI TPS65224 PMIC Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 05/11] mfd: tps6594-i2c: Add TI TPS65224 PMIC I2C Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 06/11] mfd: tps6594-spi: Add TI TPS65224 PMIC SPI Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 07/11] mfd: tps6594-core: Add TI TPS65224 PMIC core Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 08/11] misc: tps6594-pfsm: Add TI TPS65224 PMIC PFSM Bhargav Raviprakash
2024-04-11 13:03   ` Greg KH
2024-04-08 12:40 ` [PATCH v6 09/11] regulator: tps6594-regulator: Add TI TPS65224 PMIC regulators Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 10/11] pinctrl: pinctrl-tps6594: Add TPS65224 PMIC pinctrl and GPIO Bhargav Raviprakash
2024-04-12  8:52   ` Esteban Blanc
2024-04-17 10:39     ` Bhargav Raviprakash
2024-04-08 12:40 ` [PATCH v6 11/11] arch: arm64: dts: ti: k3-am62p5-sk: Add TPS65224 PMIC support in AM62P dts Bhargav Raviprakash

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240411170344.GK2399047@google.com \
    --to=lee@kernel.org \
    --cc=arnd@arndb.de \
    --cc=bhargav.r@ltts.com \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=eblanc@baylibre.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jpanis@baylibre.com \
    --cc=kristo@kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.nirmaladevi@ltts.com \
    --cc=nm@ti.com \
    --cc=robh+dt@kernel.org \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).