[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <0d459686-85e0-4ba2-ae5a-058a5c1be437@ti.com>
Date: Tue, 14 Jan 2025 12:17:57 -0600
From: Shree Ramamoorthy <s-ramamoorthy@...com>
To: Andrew Davis <afd@...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
Hi,
On 1/13/25 6:49 PM, Andrew Davis wrote:
> 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
>
Thank you for reviewing, will correct this for the next version!
>> +
>> 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;
--
Best,
Shree Ramamoorthy
PMIC Software Engineer
Powered by blists - more mailing lists