[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d3c16f4c-8141-40af-b7cb-d717b7443344@ti.com>
Date: Mon, 13 Jan 2025 18:49:43 -0600
From: Andrew Davis <afd@...com>
To: Shree Ramamoorthy <s-ramamoorthy@...com>, <lgirdwood@...il.com>,
<broonie@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>, <aaro.koskinen@....fi>, <andreas@...nade.info>,
<khilman@...libre.com>, <rogerq@...nel.org>, <tony@...mide.com>,
<jerome.neanne@...libre.com>, <linux-omap@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>
CC: <m-leonard@...com>, <praneeth@...com>, <christophe.jaillet@...adoo.fr>
Subject: Re: [PATCH v3 4/4] regulator: tps65215: Add support for TPS65215
Regulator IRQs
On 1/13/25 5:10 PM, Shree Ramamoorthy wrote:
> Isolate all changes involving regulator IRQ types:
> - Adding in TPS65215 resources
> - Organize what resources are common vs device-specific
> - How the chip_data uses these resource structs
> - Restructure the probe() for multi-PMIC support.
>
> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@...com>
> ---
> drivers/regulator/tps65219-regulator.c | 68 +++++++++++++++++++-------
> 1 file changed, 51 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/regulator/tps65219-regulator.c b/drivers/regulator/tps65219-regulator.c
> index cfb2ab6dbab4..732e28c213c3 100644
> --- a/drivers/regulator/tps65219-regulator.c
> +++ b/drivers/regulator/tps65219-regulator.c
> @@ -29,6 +29,8 @@ struct tps65219_regulator_irq_type {
> unsigned long event;
> };
>
> +static struct tps65219_regulator_irq_type tps65215_regulator_irq_types[] = { 0 };
This creates a struct array of length 1, guessing you wanted an empty
array here, so { 0 }; -> { };
But might be even easier if you have no extra irqs for this device to
drop this empty array then below in the device definition simply do this:
+ .irq_types = NULL,
+ .dev_irq_size = 0,
Andrew
> +
> static struct tps65219_regulator_irq_type tps65219_regulator_irq_types[] = {
> { "LDO3_SCG", "LDO3", "short circuit to ground", REGULATOR_EVENT_REGULATION_OUT },
> { "LDO3_OC", "LDO3", "overcurrent", REGULATOR_EVENT_OVER_CURRENT },
> @@ -36,6 +38,14 @@ static struct tps65219_regulator_irq_type tps65219_regulator_irq_types[] = {
> { "LDO4_SCG", "LDO4", "short circuit to ground", REGULATOR_EVENT_REGULATION_OUT },
> { "LDO4_OC", "LDO4", "overcurrent", REGULATOR_EVENT_OVER_CURRENT },
> { "LDO4_UV", "LDO4", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> + { "LDO3_RV", "LDO3", "residual voltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { "LDO4_RV", "LDO4", "residual voltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { "LDO3_RV_SD", "LDO3", "residual voltage on shutdown", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> + { "LDO4_RV_SD", "LDO4", "residual voltage on shutdown", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> +};
> +
> +/* All of TPS65215's irq types are the same as common_regulator_irq_types */
> +static struct tps65219_regulator_irq_type common_regulator_irq_types[] = {
> { "LDO1_SCG", "LDO1", "short circuit to ground", REGULATOR_EVENT_REGULATION_OUT },
> { "LDO1_OC", "LDO1", "overcurrent", REGULATOR_EVENT_OVER_CURRENT },
> { "LDO1_UV", "LDO1", "undervoltage", REGULATOR_EVENT_UNDER_VOLTAGE },
> @@ -59,8 +69,6 @@ static struct tps65219_regulator_irq_type tps65219_regulator_irq_types[] = {
> { "BUCK3_RV", "BUCK3", "residual voltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> { "LDO1_RV", "LDO1", "residual voltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> { "LDO2_RV", "LDO2", "residual voltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> - { "LDO3_RV", "LDO3", "residual voltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> - { "LDO4_RV", "LDO4", "residual voltage", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> { "BUCK1_RV_SD", "BUCK1", "residual voltage on shutdown",
> REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> { "BUCK2_RV_SD", "BUCK2", "residual voltage on shutdown",
> @@ -69,8 +77,6 @@ static struct tps65219_regulator_irq_type tps65219_regulator_irq_types[] = {
> REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> { "LDO1_RV_SD", "LDO1", "residual voltage on shutdown", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> { "LDO2_RV_SD", "LDO2", "residual voltage on shutdown", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> - { "LDO3_RV_SD", "LDO3", "residual voltage on shutdown", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> - { "LDO4_RV_SD", "LDO4", "residual voltage on shutdown", REGULATOR_EVENT_OVER_VOLTAGE_WARN },
> { "SENSOR_3_WARM", "SENSOR3", "warm temperature", REGULATOR_EVENT_OVER_TEMP_WARN},
> { "SENSOR_2_WARM", "SENSOR2", "warm temperature", REGULATOR_EVENT_OVER_TEMP_WARN },
> { "SENSOR_1_WARM", "SENSOR1", "warm temperature", REGULATOR_EVENT_OVER_TEMP_WARN },
> @@ -313,8 +319,12 @@ static irqreturn_t tps65219_regulator_irq_handler(int irq, void *data)
> struct tps65219_chip_data {
> size_t rdesc_size;
> size_t common_rdesc_size;
> + size_t dev_irq_size;
> + size_t common_irq_size;
> const struct regulator_desc *rdesc;
> const struct regulator_desc *common_rdesc;
> + struct tps65219_regulator_irq_type *irq_types;
> + struct tps65219_regulator_irq_type *common_irq_types;
> };
>
> static struct tps65219_chip_data chip_info_table[] = {
> @@ -323,12 +333,20 @@ static struct tps65219_chip_data chip_info_table[] = {
> .rdesc_size = ARRAY_SIZE(tps65215_regs),
> .common_rdesc = common_regs,
> .common_rdesc_size = ARRAY_SIZE(common_regs),
> + .irq_types = tps65215_regulator_irq_types,
> + .dev_irq_size = ARRAY_SIZE(tps65215_regulator_irq_types),
> + .common_irq_types = common_regulator_irq_types,
> + .common_irq_size = ARRAY_SIZE(common_regulator_irq_types),
> },
> [TPS65219] = {
> .rdesc = tps65219_regs,
> .rdesc_size = ARRAY_SIZE(tps65219_regs),
> .common_rdesc = common_regs,
> .common_rdesc_size = ARRAY_SIZE(common_regs),
> + .irq_types = tps65219_regulator_irq_types,
> + .dev_irq_size = ARRAY_SIZE(tps65219_regulator_irq_types),
> + .common_irq_types = common_regulator_irq_types,
> + .common_irq_size = ARRAY_SIZE(common_regulator_irq_types),
> },
> };
>
> @@ -336,7 +354,6 @@ static int tps65219_regulator_probe(struct platform_device *pdev)
> {
> struct tps65219_regulator_irq_data *irq_data;
> struct tps65219_regulator_irq_type *irq_type;
> -
> struct tps65219_chip_data *pmic;
> struct regulator_dev *rdev;
> int error;
> @@ -370,33 +387,50 @@ static int tps65219_regulator_probe(struct platform_device *pdev)
> pmic->rdesc[i].name);
> }
>
> - irq_data = devm_kmalloc(tps->dev,
> - ARRAY_SIZE(tps65219_regulator_irq_types) *
> - sizeof(struct tps65219_regulator_irq_data),
> - GFP_KERNEL);
> + irq_data = devm_kmalloc(tps->dev, pmic->common_irq_size, GFP_KERNEL);
> if (!irq_data)
> return -ENOMEM;
>
> - for (i = 0; i < ARRAY_SIZE(tps65219_regulator_irq_types); ++i) {
> - irq_type = &tps65219_regulator_irq_types[i];
> -
> + for (i = 0; i < pmic->common_irq_size; ++i) {
> + irq_type = &pmic->common_irq_types[i];
> irq = platform_get_irq_byname(pdev, irq_type->irq_name);
> if (irq < 0)
> return -EINVAL;
>
> irq_data[i].dev = tps->dev;
> irq_data[i].type = irq_type;
> + error = devm_request_threaded_irq(tps->dev, irq, NULL,
> + tps65219_regulator_irq_handler,
> + IRQF_ONESHOT,
> + irq_type->irq_name,
> + &irq_data[i]);
> + if (error)
> + return dev_err_probe(tps->dev, PTR_ERR(rdev),
> + "Failed to request %s IRQ %d: %d\n",
> + irq_type->irq_name, irq, error);
> + }
> +
> + irq_data = devm_kmalloc(tps->dev, pmic->dev_irq_size, GFP_KERNEL);
> + if (!irq_data)
> + return -ENOMEM;
>
> + for (i = 0; i < pmic->dev_irq_size; ++i) {
> + irq_type = &pmic->irq_types[i];
> + irq = platform_get_irq_byname(pdev, irq_type->irq_name);
> + if (irq < 0)
> + return -EINVAL;
> +
> + irq_data[i].dev = tps->dev;
> + irq_data[i].type = irq_type;
> error = devm_request_threaded_irq(tps->dev, irq, NULL,
> tps65219_regulator_irq_handler,
> IRQF_ONESHOT,
> irq_type->irq_name,
> &irq_data[i]);
> - if (error) {
> - dev_err(tps->dev, "failed to request %s IRQ %d: %d\n",
> - irq_type->irq_name, irq, error);
> - return error;
> - }
> + if (error)
> + return dev_err_probe(tps->dev, PTR_ERR(rdev),
> + "Failed to request %s IRQ %d: %d\n",
> + irq_type->irq_name, irq, error);
> }
>
> return 0;
Powered by blists - more mailing lists