Re: [PATCH v3 3/5] platform/x86: int3472: Support multiple clock consumers

Hans de Goede
2022-09-22T08:49:24Z

Hi,

On 9/22/22 01:04, Daniel Scally wrote:
> At present, the tps68470.c only supports a single clock consumer when
> passing platform data to the clock driver. In some devices multiple
> sensors depend on the clock provided by a single TPS68470 and so all
> need to be able to acquire the clock. Support passing multiple
> consumers as platform data.
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Note this one already has a review + ack from Stephen Boyd for
merging this through the pdx86 tree (from v1 of the series):

https://lore.kernel.org/platform-driver-x86/20220225004943.AA8EDC340EF@smtp.kernel.org/

Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Acked-by: Stephen Boyd <sboyd@kernel.org>

Regards,

Hans

> ---
> Changes since v2:
>
> - None
>
> drivers/clk/clk-tps68470.c | 13 ++--
> drivers/platform/x86/intel/int3472/tps68470.c | 59 ++++++++++++++++---
> include/linux/platform_data/tps68470.h | 7 ++-
> 3 files changed, 67 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/clk-tps68470.c b/drivers/clk/clk-tps68470.c
> index e5fbefd6ac2d..38f44b5b9b1b 100644
> --- a/drivers/clk/clk-tps68470.c
> +++ b/drivers/clk/clk-tps68470.c
> @@ -200,7 +200,9 @@ static int tps68470_clk_probe(struct platform_device *pdev)
> .flags = CLK_SET_RATE_GATE,
> };
> struct tps68470_clkdata *tps68470_clkdata;
> + struct tps68470_clk_consumer *consumer;
> int ret;
> + int i;
>
> tps68470_clkdata = devm_kzalloc(&pdev->dev, sizeof(*tps68470_clkdata),
> GFP_KERNEL);
> @@ -223,10 +225,13 @@ static int tps68470_clk_probe(struct platform_device *pdev)
> return ret;
>
> if (pdata) {
> - ret = devm_clk_hw_register_clkdev(&pdev->dev,
> - &tps68470_clkdata->clkout_hw,
> - pdata->consumer_con_id,
> - pdata->consumer_dev_name);
> + for (i = 0; i < pdata->n_consumers; i++) {
> + consumer = &pdata->consumers[i];
> + ret = devm_clk_hw_register_clkdev(&pdev->dev,
> + &tps68470_clkdata->clkout_hw,
> + consumer->consumer_con_id,
> + consumer->consumer_dev_name);
> + }
> }
>
> return ret;
> diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
> index 22f61b47f9e5..8a684030933d 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Author: Dan Scally <djrscally@gmail.com> */
>
> +#include <linux/acpi.h>
> #include <linux/i2c.h>
> #include <linux/kernel.h>
> #include <linux/mfd/core.h>
> @@ -95,20 +96,64 @@ static int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
> return DESIGNED_FOR_WINDOWS;
> }
>
> +/*
> + * Return the size of the flexible array member, because we'll need that later
> + * on to pass .pdata_size to cells.
> + */
> +static int
> +skl_int3472_fill_clk_pdata(struct device *dev, struct tps68470_clk_platform_data **clk_pdata)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> + struct acpi_device *consumer;
> + unsigned int n_consumers = 0;
> + const char *sensor_name;
> + unsigned int i = 0;
> +
> + for_each_acpi_consumer_dev(adev, consumer)
> + n_consumers++;
> +
> + if (!n_consumers) {
> + dev_err(dev, "INT3472 seems to have no dependents\n");
> + return -ENODEV;
> + }
> +
> + *clk_pdata = devm_kzalloc(dev, struct_size(*clk_pdata, consumers, n_consumers),
> + GFP_KERNEL);
> + if (!*clk_pdata)
> + return -ENOMEM;
> +
> + (*clk_pdata)->n_consumers = n_consumers;
> + i = 0;
> +
> + for_each_acpi_consumer_dev(adev, consumer) {
> + sensor_name = devm_kasprintf(dev, GFP_KERNEL, I2C_DEV_NAME_FORMAT,
> + acpi_dev_name(consumer));
> + if (!sensor_name)
> + return -ENOMEM;
> +
> + (*clk_pdata)->consumers[i].consumer_dev_name = sensor_name;
> + i++;
> + }
> +
> + acpi_dev_put(consumer);
> +
> + return n_consumers;
> +}
> +
> static int skl_int3472_tps68470_probe(struct i2c_client *client)
> {
> struct acpi_device *adev = ACPI_COMPANION(&client->dev);
> const struct int3472_tps68470_board_data *board_data;
> - struct tps68470_clk_platform_data clk_pdata = {};
> + struct tps68470_clk_platform_data *clk_pdata;
> struct mfd_cell *cells;
> struct regmap *regmap;
> + int n_consumers;
> int device_type;
> int ret;
>
> - ret = skl_int3472_get_sensor_adev_and_name(&client->dev, NULL,
> - &clk_pdata.consumer_dev_name);
> - if (ret)
> - return ret;
> + n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
> + if (n_consumers < 0)
> + return n_consumers;
>
> regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
> if (IS_ERR(regmap)) {
> @@ -142,8 +187,8 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
> * the clk + regulators must be ready when this happens.
> */
> cells[0].name = "tps68470-clk";
> - cells[0].platform_data = &clk_pdata;
> - cells[0].pdata_size = sizeof(clk_pdata);
> + cells[0].platform_data = clk_pdata;
> + cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
> cells[1].name = "tps68470-regulator";
> cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
> cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
> diff --git a/include/linux/platform_data/tps68470.h b/include/linux/platform_data/tps68470.h
> index 126d082c3f2e..e605a2cab07f 100644
> --- a/include/linux/platform_data/tps68470.h
> +++ b/include/linux/platform_data/tps68470.h
> @@ -27,9 +27,14 @@ struct tps68470_regulator_platform_data {
> const struct regulator_init_data *reg_init_data[TPS68470_NUM_REGULATORS];
> };
>
> -struct tps68470_clk_platform_data {
> +struct tps68470_clk_consumer {
> const char *consumer_dev_name;
> const char *consumer_con_id;
> };
>
> +struct tps68470_clk_platform_data {
> + unsigned int n_consumers;
> + struct tps68470_clk_consumer consumers[];
> +};
> +
> #endif