2024-03-20 10:27:19

by Bhargav Raviprakash

[permalink] [raw]
Subject: [PATCH v4 03/11] mfd: tps6594: add regmap config in match data

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 <[email protected]>
Acked-by: Julien Panis <[email protected]>
---
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},
+};
+
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], },
{}
};
MODULE_DEVICE_TABLE(of, tps6594_i2c_of_match_table);
@@ -205,6 +211,7 @@ static int tps6594_i2c_probe(struct i2c_client *client)
struct device *dev = &client->dev;
struct tps6594 *tps;
const struct of_device_id *match;
+ const struct tps6594_match_data *mdata;

tps = devm_kzalloc(dev, sizeof(*tps), GFP_KERNEL);
if (!tps)
@@ -216,14 +223,15 @@ static int tps6594_i2c_probe(struct i2c_client *client)
tps->reg = client->addr;
tps->irq = client->irq;

- tps->regmap = devm_regmap_init(dev, NULL, client, &tps6594_i2c_regmap_config);
- if (IS_ERR(tps->regmap))
- return dev_err_probe(dev, PTR_ERR(tps->regmap), "Failed to init regmap\n");
-
match = of_match_device(tps6594_i2c_of_match_table, dev);
if (!match)
return dev_err_probe(dev, -EINVAL, "Failed to find matching chip ID\n");
- tps->chip_id = (unsigned long)match->data;
+ mdata = (struct tps6594_match_data *)match->data;
+ tps->chip_id = mdata->chip_id;
+
+ tps->regmap = devm_regmap_init(dev, NULL, client, mdata->config);
+ if (IS_ERR(tps->regmap))
+ return dev_err_probe(dev, PTR_ERR(tps->regmap), "Failed to init regmap\n");

crc8_populate_msb(tps6594_i2c_crc_table, TPS6594_CRC8_POLYNOMIAL);

diff --git a/drivers/mfd/tps6594-spi.c b/drivers/mfd/tps6594-spi.c
index 5afb1736f..82a1c02e3 100644
--- a/drivers/mfd/tps6594-spi.c
+++ b/drivers/mfd/tps6594-spi.c
@@ -77,10 +77,16 @@ static const struct regmap_config tps6594_spi_regmap_config = {
.use_single_write = true,
};

+static const struct tps6594_match_data match_data[] = {
+ [TPS6594] = {TPS6594, &tps6594_spi_regmap_config},
+ [TPS6593] = {TPS6593, &tps6594_spi_regmap_config},
+ [LP8764] = {LP8764, &tps6594_spi_regmap_config},
+};
+
static const struct of_device_id tps6594_spi_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], },
{}
};
MODULE_DEVICE_TABLE(of, tps6594_spi_of_match_table);
@@ -90,6 +96,7 @@ static int tps6594_spi_probe(struct spi_device *spi)
struct device *dev = &spi->dev;
struct tps6594 *tps;
const struct of_device_id *match;
+ const struct tps6594_match_data *mdata;

tps = devm_kzalloc(dev, sizeof(*tps), GFP_KERNEL);
if (!tps)
@@ -101,14 +108,15 @@ static int tps6594_spi_probe(struct spi_device *spi)
tps->reg = spi_get_chipselect(spi, 0);
tps->irq = spi->irq;

- tps->regmap = devm_regmap_init(dev, NULL, spi, &tps6594_spi_regmap_config);
- if (IS_ERR(tps->regmap))
- return dev_err_probe(dev, PTR_ERR(tps->regmap), "Failed to init regmap\n");
-
match = of_match_device(tps6594_spi_of_match_table, dev);
if (!match)
return dev_err_probe(dev, -EINVAL, "Failed to find matching chip ID\n");
- tps->chip_id = (unsigned long)match->data;
+ mdata = (struct tps6594_match_data *)match->data;
+ tps->chip_id = mdata->chip_id;
+
+ tps->regmap = devm_regmap_init(dev, NULL, spi, mdata->config);
+ if (IS_ERR(tps->regmap))
+ return dev_err_probe(dev, PTR_ERR(tps->regmap), "Failed to init regmap\n");

crc8_populate_msb(tps6594_spi_crc_table, TPS6594_CRC8_POLYNOMIAL);

diff --git a/include/linux/mfd/tps6594.h b/include/linux/mfd/tps6594.h
index 16543fd4d..d781e0fe3 100644
--- a/include/linux/mfd/tps6594.h
+++ b/include/linux/mfd/tps6594.h
@@ -1337,6 +1337,17 @@ struct tps6594 {
struct regmap_irq_chip_data *irq_data;
};

+/**
+ * struct tps6594_match_data - of match data of PMIC
+ *
+ * @chip_id: chip ID of PMIC
+ * @config: regmap config of PMIC
+ */
+struct tps6594_match_data {
+ unsigned long chip_id;
+ const struct regmap_config *config;
+};
+
extern const struct regmap_access_table tps6594_volatile_table;
extern const struct regmap_access_table tps65224_volatile_table;

--
2.25.1



2024-03-28 11:37:42

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 03/11] mfd: tps6594: add regmap config in match data

On Wed, 20 Mar 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 <[email protected]>
> Acked-by: Julien Panis <[email protected]>
> ---
> 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(-)

Mostly fine, just a couple of nits.

Firstly the subject lines for the entire set are inconsistent.

Please use the style set by the subsystem:

`git log --oneline -- <subsystem>` is your friend.

> 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},

Spaces after the '{' and before the '}' please.
> +};
> +
> 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], },
> {}
> };
> MODULE_DEVICE_TABLE(of, tps6594_i2c_of_match_table);
> @@ -205,6 +211,7 @@ static int tps6594_i2c_probe(struct i2c_client *client)
> struct device *dev = &client->dev;
> struct tps6594 *tps;
> const struct of_device_id *match;
> + const struct tps6594_match_data *mdata;
>
> tps = devm_kzalloc(dev, sizeof(*tps), GFP_KERNEL);
> if (!tps)
> @@ -216,14 +223,15 @@ static int tps6594_i2c_probe(struct i2c_client *client)
> tps->reg = client->addr;
> tps->irq = client->irq;
>
> - tps->regmap = devm_regmap_init(dev, NULL, client, &tps6594_i2c_regmap_config);
> - if (IS_ERR(tps->regmap))
> - return dev_err_probe(dev, PTR_ERR(tps->regmap), "Failed to init regmap\n");
> -
> match = of_match_device(tps6594_i2c_of_match_table, dev);
> if (!match)
> return dev_err_probe(dev, -EINVAL, "Failed to find matching chip ID\n");
> - tps->chip_id = (unsigned long)match->data;
> + mdata = (struct tps6594_match_data *)match->data;

What happens when you drop this case?

I was under the impression this was not required when casting from (void *)

> + tps->chip_id = mdata->chip_id;
> +
> + tps->regmap = devm_regmap_init(dev, NULL, client, mdata->config);
> + if (IS_ERR(tps->regmap))
> + return dev_err_probe(dev, PTR_ERR(tps->regmap), "Failed to init regmap\n");

"initialise"

>
> crc8_populate_msb(tps6594_i2c_crc_table, TPS6594_CRC8_POLYNOMIAL);
>
> diff --git a/drivers/mfd/tps6594-spi.c b/drivers/mfd/tps6594-spi.c
> index 5afb1736f..82a1c02e3 100644
> --- a/drivers/mfd/tps6594-spi.c
> +++ b/drivers/mfd/tps6594-spi.c
> @@ -77,10 +77,16 @@ static const struct regmap_config tps6594_spi_regmap_config = {
> .use_single_write = true,
> };
>
> +static const struct tps6594_match_data match_data[] = {
> + [TPS6594] = {TPS6594, &tps6594_spi_regmap_config},
> + [TPS6593] = {TPS6593, &tps6594_spi_regmap_config},
> + [LP8764] = {LP8764, &tps6594_spi_regmap_config},

As above.

> +};
> +
> static const struct of_device_id tps6594_spi_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], },
> {}
> };
> MODULE_DEVICE_TABLE(of, tps6594_spi_of_match_table);
> @@ -90,6 +96,7 @@ static int tps6594_spi_probe(struct spi_device *spi)
> struct device *dev = &spi->dev;
> struct tps6594 *tps;
> const struct of_device_id *match;
> + const struct tps6594_match_data *mdata;
>
> tps = devm_kzalloc(dev, sizeof(*tps), GFP_KERNEL);
> if (!tps)
> @@ -101,14 +108,15 @@ static int tps6594_spi_probe(struct spi_device *spi)
> tps->reg = spi_get_chipselect(spi, 0);
> tps->irq = spi->irq;
>
> - tps->regmap = devm_regmap_init(dev, NULL, spi, &tps6594_spi_regmap_config);
> - if (IS_ERR(tps->regmap))
> - return dev_err_probe(dev, PTR_ERR(tps->regmap), "Failed to init regmap\n");
> -
> match = of_match_device(tps6594_spi_of_match_table, dev);
> if (!match)
> return dev_err_probe(dev, -EINVAL, "Failed to find matching chip ID\n");
> - tps->chip_id = (unsigned long)match->data;
> + mdata = (struct tps6594_match_data *)match->data;
> + tps->chip_id = mdata->chip_id;
> +
> + tps->regmap = devm_regmap_init(dev, NULL, spi, mdata->config);
> + if (IS_ERR(tps->regmap))
> + return dev_err_probe(dev, PTR_ERR(tps->regmap), "Failed to init regmap\n");
>
> crc8_populate_msb(tps6594_spi_crc_table, TPS6594_CRC8_POLYNOMIAL);
>
> diff --git a/include/linux/mfd/tps6594.h b/include/linux/mfd/tps6594.h
> index 16543fd4d..d781e0fe3 100644
> --- a/include/linux/mfd/tps6594.h
> +++ b/include/linux/mfd/tps6594.h
> @@ -1337,6 +1337,17 @@ struct tps6594 {
> struct regmap_irq_chip_data *irq_data;
> };
>
> +/**
> + * struct tps6594_match_data - of match data of PMIC
> + *
> + * @chip_id: chip ID of PMIC
> + * @config: regmap config of PMIC
> + */
> +struct tps6594_match_data {
> + unsigned long chip_id;
> + const struct regmap_config *config;
> +};
> +
> extern const struct regmap_access_table tps6594_volatile_table;
> extern const struct regmap_access_table tps65224_volatile_table;
>
> --
> 2.25.1
>

--
Lee Jones [李琼斯]