diff mbox series

[v2,3/3] hwmon: (pmbus/core): Implement irq support

Message ID 20221128174715.1969957-3-Naresh.Solanki@9elements.com (mailing list archive)
State Changes Requested
Headers show
Series [v2,1/3] hwmon: (pmbus/core): Update regulator flag map | expand

Commit Message

Naresh Solanki Nov. 28, 2022, 5:47 p.m. UTC
From: Patrick Rudolph <patrick.rudolph@9elements.com>

Implement PMBUS irq handler to notify regulator events.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 drivers/hwmon/pmbus/pmbus.h      |   2 +-
 drivers/hwmon/pmbus/pmbus_core.c | 151 ++++++++++++++++++++++++++++---
 2 files changed, 137 insertions(+), 16 deletions(-)

Comments

Guenter Roeck Nov. 28, 2022, 11:09 p.m. UTC | #1
On 11/28/22 09:47, Naresh Solanki wrote:
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
> 
> Implement PMBUS irq handler to notify regulator events.
> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>

As I am sure I have mentioned before, this needs to primarily handle
sysfs notifications to hwmon status attributes and to generate kobject
events. Regulator events are secondary / optional.

Thanks,
Guenter
Naresh Solanki Nov. 29, 2022, 8:16 a.m. UTC | #2
Hi Guenter,

On 29-11-2022 04:39 am, Guenter Roeck wrote:
> On 11/28/22 09:47, Naresh Solanki wrote:
>> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>>
>> Implement PMBUS irq handler to notify regulator events.
>>
>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> 
> As I am sure I have mentioned before, this needs to primarily handle
> sysfs notifications to hwmon status attributes and to generate kobject
> events. Regulator events are secondary / optional.

Based on previous feedback, PMBus interrupt handler is made generic
Based on the use case I have in my machine, my application need to 
monitor regulator event as soon as they occur and hence the patch.

> 
> Thanks,
> Guenter
> 
Regards,
Naresh
Guenter Roeck Nov. 29, 2022, 3:29 p.m. UTC | #3
On 11/29/22 00:16, Naresh Solanki wrote:
> Hi Guenter,
> 
> On 29-11-2022 04:39 am, Guenter Roeck wrote:
>> On 11/28/22 09:47, Naresh Solanki wrote:
>>> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>>>
>>> Implement PMBUS irq handler to notify regulator events.
>>>
>>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>
>> As I am sure I have mentioned before, this needs to primarily handle
>> sysfs notifications to hwmon status attributes and to generate kobject
>> events. Regulator events are secondary / optional.
> 
> Based on previous feedback, PMBus interrupt handler is made generic
> Based on the use case I have in my machine, my application need to monitor regulator event as soon as they occur and hence the patch.
> 

I understand, but this isn't just about your specific use case. Your use case is
what triggers the change, and ensures that the code change is tested, but the
impact by far reaches beyond your specific use case and needs to address other
(much more common) use cases as well. Interrupt support is needed in the pmbus
code, but it needs to address the common use case first, and that is reporting
the status via sysfs notifications and kobject events.

Thanks,
Guenter
Naresh Solanki Nov. 30, 2022, 4:54 p.m. UTC | #4
Hi Guenter,

On 29-11-2022 08:59 pm, Guenter Roeck wrote:
> On 11/29/22 00:16, Naresh Solanki wrote:
>> Hi Guenter,
>>
>> On 29-11-2022 04:39 am, Guenter Roeck wrote:
>>> On 11/28/22 09:47, Naresh Solanki wrote:
>>>> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>>>>
>>>> Implement PMBUS irq handler to notify regulator events.
>>>>
>>>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>>
>>> As I am sure I have mentioned before, this needs to primarily handle
>>> sysfs notifications to hwmon status attributes and to generate kobject
>>> events. Regulator events are secondary / optional.
>>
>> Based on previous feedback, PMBus interrupt handler is made generic
>> Based on the use case I have in my machine, my application need to 
>> monitor regulator event as soon as they occur and hence the patch.
>>
> 
> I understand, but this isn't just about your specific use case. Your use 
> case is
> what triggers the change, and ensures that the code change is tested, 
> but the
> impact by far reaches beyond your specific use case and needs to address 
> other
> (much more common) use cases as well. Interrupt support is needed in the 
> pmbus
> code, but it needs to address the common use case first, and that is 
> reporting
> the status via sysfs notifications and kobject events.
Agree. I've done the implementation. Will submit the change as separate 
patch along with the series.
> 
> Thanks,
> Guenter
> 
Regards,
Naresh
diff mbox series

Patch

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 10fb17879f8e..6b2e6cf93b19 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -26,7 +26,7 @@  enum pmbus_regs {
 
 	PMBUS_CAPABILITY		= 0x19,
 	PMBUS_QUERY			= 0x1A,
-
+	PMBUS_SMBALERT_MASK		= 0x1B,
 	PMBUS_VOUT_MODE			= 0x20,
 	PMBUS_VOUT_COMMAND		= 0x21,
 	PMBUS_VOUT_TRIM			= 0x22,
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 060e9d0a55bd..e1f84fa127ba 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -81,6 +81,7 @@  struct pmbus_label {
 struct pmbus_data {
 	struct device *dev;
 	struct device *hwmon_dev;
+	struct regulator_dev **rdevs;
 
 	u32 flags;		/* from platform data */
 
@@ -2823,7 +2824,8 @@  static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] =
 	},
 };
 
-static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags)
+static int pmbus_regulator_get_flags(struct regulator_dev *rdev, unsigned int *error,
+				    unsigned int *event)
 {
 	int i, status;
 	const struct pmbus_regulator_status_category *cat;
@@ -2834,7 +2836,8 @@  static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
 	u8 page = rdev_get_id(rdev);
 	int func = data->info->func[page];
 
-	*flags = 0;
+	*error = 0;
+	*event = 0;
 
 	mutex_lock(&data->update_lock);
 
@@ -2850,8 +2853,10 @@  static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
 		}
 
 		for (bit = cat->bits; bit->pflag; bit++) {
-			if (status & bit->pflag)
-				*flags |= bit->rflag;
+			if (status & bit->pflag) {
+				*error |= bit->rflag;
+				*event |= bit->eflag;
+			}
 		}
 	}
 
@@ -2870,11 +2875,15 @@  static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
 		return status;
 
 	if (pmbus_regulator_is_enabled(rdev)) {
-		if (status & PB_STATUS_OFF)
-			*flags |= REGULATOR_ERROR_FAIL;
+		if (status & PB_STATUS_OFF) {
+			*error |= REGULATOR_ERROR_FAIL;
+			*event |= REGULATOR_EVENT_FAIL;
+		}
 
-		if (status & PB_STATUS_POWER_GOOD_N)
-			*flags |= REGULATOR_ERROR_REGULATION_OUT;
+		if (status & PB_STATUS_POWER_GOOD_N) {
+			*error |= REGULATOR_ERROR_REGULATION_OUT;
+			*event |= REGULATOR_EVENT_REGULATION_OUT;
+		}
 	}
 
 	/*
@@ -2882,13 +2891,22 @@  static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
 	 * PMBUS_STATUS_TEMPERATURE, map PB_STATUS_TEMPERATURE to a warning as
 	 * a (conservative) best-effort interpretation.
 	 */
-	if (!(*flags & (REGULATOR_ERROR_OVER_TEMP | REGULATOR_ERROR_OVER_TEMP_WARN)) &&
-	    (status & PB_STATUS_TEMPERATURE))
-		*flags |= REGULATOR_ERROR_OVER_TEMP_WARN;
+	if (!(*error & (REGULATOR_ERROR_OVER_TEMP | REGULATOR_ERROR_OVER_TEMP_WARN)) &&
+	    (status & PB_STATUS_TEMPERATURE)) {
+		*error |= REGULATOR_ERROR_OVER_TEMP_WARN;
+		*event |= REGULATOR_EVENT_OVER_TEMP_WARN;
+	}
 
 	return 0;
 }
 
+static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags)
+{
+	unsigned int event;
+
+	return pmbus_regulator_get_flags(rdev, flags, &event);
+}
+
 static int pmbus_regulator_get_status(struct regulator_dev *rdev)
 {
 	struct device *dev = rdev_get_dev(rdev);
@@ -3079,14 +3097,61 @@  const struct regulator_ops pmbus_regulator_ops = {
 };
 EXPORT_SYMBOL_NS_GPL(pmbus_regulator_ops, PMBUS);
 
+static int pmbus_write_smbalert_mask(struct i2c_client *client, u8 page, u8 reg, u8 val)
+{
+	return pmbus_write_word_data(client, page, PMBUS_SMBALERT_MASK, reg | (val << 8));
+}
+
+static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
+{
+	struct pmbus_data *data = pdata;
+	struct i2c_client *client = to_i2c_client(data->dev);
+	int i, ret = IRQ_NONE, status, event;
+	u8 page;
+
+	for (i = 0; i < data->info->num_regulators; i++) {
+
+		if (!data->rdevs[i])
+			continue;
+
+		ret = pmbus_regulator_get_flags(data->rdevs[i], &status, &event);
+		if (ret)
+			return ret;
+
+		if (event) {
+			regulator_notifier_call_chain(data->rdevs[i], event, NULL);
+			ret = IRQ_HANDLED;
+		}
+
+		page = rdev_get_id(data->rdevs[i]);
+		mutex_lock(&data->update_lock);
+		status = pmbus_read_status_word(client, page);
+		if (status < 0) {
+			mutex_unlock(&data->update_lock);
+			return status;
+		}
+
+		if (status & ~(PB_STATUS_OFF | PB_STATUS_BUSY | PB_STATUS_POWER_GOOD_N))
+			pmbus_clear_fault_page(client, page);
+
+		mutex_unlock(&data->update_lock);
+	}
+
+	return ret;
+}
+
 static int pmbus_regulator_register(struct pmbus_data *data)
 {
 	struct device *dev = data->dev;
 	const struct pmbus_driver_info *info = data->info;
 	const struct pmbus_platform_data *pdata = dev_get_platdata(dev);
-	struct regulator_dev *rdev;
 	int i;
 
+	data->rdevs = devm_kzalloc(dev, sizeof(struct regulator_dev *) * info->num_regulators,
+				  GFP_KERNEL);
+	if (!data->rdevs)
+		return -ENOMEM;
+
 	for (i = 0; i < info->num_regulators; i++) {
 		struct regulator_config config = { };
 
@@ -3096,21 +3161,71 @@  static int pmbus_regulator_register(struct pmbus_data *data)
 		if (pdata && pdata->reg_init_data)
 			config.init_data = &pdata->reg_init_data[i];
 
-		rdev = devm_regulator_register(dev, &info->reg_desc[i],
+		data->rdevs[i] = devm_regulator_register(dev, &info->reg_desc[i],
 					       &config);
-		if (IS_ERR(rdev))
-			return dev_err_probe(dev, PTR_ERR(rdev),
+		if (IS_ERR(data->rdevs[i]))
+			return dev_err_probe(dev, PTR_ERR(data->rdevs[i]),
 					     "Failed to register %s regulator\n",
 					     info->reg_desc[i].name);
 	}
 
 	return 0;
 }
+
+static int pmbus_irq_setup(struct i2c_client *client, struct pmbus_data *data)
+{
+	struct device *dev = &client->dev;
+	const struct pmbus_regulator_status_category *cat;
+	const struct pmbus_regulator_status_assoc *bit;
+	int i, j, err, ret;
+	u8 mask;
+	int func;
+
+	for (i = 0; i < data->info->pages; i++) {
+		func = data->info->func[i];
+
+		for (j = 0; j < ARRAY_SIZE(pmbus_regulator_flag_map); j++) {
+			cat = &pmbus_regulator_flag_map[i];
+			if (!(func & cat->func))
+				continue;
+			mask = 0;
+			for (bit = cat->bits; bit->pflag; bit++)
+				mask |= bit->pflag;
+
+			err = pmbus_write_smbalert_mask(client, i, cat->reg, ~mask);
+			if (err)
+				dev_err(dev, "Failed to set smbalert for reg 0x%02x\n",	cat->reg);
+		}
+
+		pmbus_write_smbalert_mask(client, i, PMBUS_STATUS_CML, 0xff);
+		pmbus_write_smbalert_mask(client, i, PMBUS_STATUS_OTHER, 0xff);
+		pmbus_write_smbalert_mask(client, i, PMBUS_STATUS_MFR_SPECIFIC, 0xff);
+		if (data->info->func[i] & PMBUS_HAVE_FAN12)
+			pmbus_write_smbalert_mask(client, i, PMBUS_STATUS_FAN_12, 0xff);
+		if (data->info->func[i] & PMBUS_HAVE_FAN34)
+			pmbus_write_smbalert_mask(client, i, PMBUS_STATUS_FAN_34, 0xff);
+
+	}
+
+	/* Register notifiers - can fail if IRQ is not given */
+	ret = devm_request_threaded_irq(dev, client->irq, NULL, pmbus_fault_handler,
+			      0, "pmbus-irq", data);
+	if (ret) {
+		dev_warn(dev, "IRQ disabled %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
 #else
 static int pmbus_regulator_register(struct pmbus_data *data)
 {
 	return 0;
 }
+static int pmbus_irq_setup(struct i2c_client *client, struct pmbus_data *data)
+{
+	return 0;
+}
 #endif
 
 static struct dentry *pmbus_debugfs_dir;	/* pmbus debugfs directory */
@@ -3475,6 +3590,12 @@  int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info)
 	if (ret)
 		return ret;
 
+	if (client->irq > 0) {
+		ret = pmbus_irq_setup(client, data);
+		if (ret)
+			return ret;
+	}
+
 	ret = pmbus_init_debugfs(client, data);
 	if (ret)
 		dev_warn(dev, "Failed to register debugfs\n");