The PMBus status flag map(pmbus_regulator_status_flag_map) is moved
outside of the regulator #if block and the associated variable/struct
name updated to reflect as generic PMBus status.
This will make the PMBus status flag map more versatile and easier to
incorporate into different contexts and functions.
Signed-off-by: Naresh Solanki <[email protected]>
---
drivers/hwmon/pmbus/pmbus_core.c | 94 ++++++++++++++++----------------
1 file changed, 47 insertions(+), 47 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 95e95783972a..1b70cf3be313 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2692,6 +2692,49 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,
return 0;
}
+/* A PMBus status flag and the corresponding REGULATOR_ERROR_* flag */
+struct pmbus_status_assoc {
+ int pflag, rflag;
+};
+
+/* PMBus->regulator bit mappings for a PMBus status register */
+struct pmbus_status_category {
+ int func;
+ int reg;
+ const struct pmbus_status_assoc *bits; /* zero-terminated */
+};
+
+static const struct pmbus_status_category __maybe_unused pmbus_status_flag_map[] = {
+ {
+ .func = PMBUS_HAVE_STATUS_VOUT,
+ .reg = PMBUS_STATUS_VOUT,
+ .bits = (const struct pmbus_status_assoc[]) {
+ { PB_VOLTAGE_UV_WARNING, REGULATOR_ERROR_UNDER_VOLTAGE_WARN },
+ { PB_VOLTAGE_UV_FAULT, REGULATOR_ERROR_UNDER_VOLTAGE },
+ { PB_VOLTAGE_OV_WARNING, REGULATOR_ERROR_OVER_VOLTAGE_WARN },
+ { PB_VOLTAGE_OV_FAULT, REGULATOR_ERROR_REGULATION_OUT },
+ { },
+ },
+ }, {
+ .func = PMBUS_HAVE_STATUS_IOUT,
+ .reg = PMBUS_STATUS_IOUT,
+ .bits = (const struct pmbus_status_assoc[]) {
+ { PB_IOUT_OC_WARNING, REGULATOR_ERROR_OVER_CURRENT_WARN },
+ { PB_IOUT_OC_FAULT, REGULATOR_ERROR_OVER_CURRENT },
+ { PB_IOUT_OC_LV_FAULT, REGULATOR_ERROR_OVER_CURRENT },
+ { },
+ },
+ }, {
+ .func = PMBUS_HAVE_STATUS_TEMP,
+ .reg = PMBUS_STATUS_TEMPERATURE,
+ .bits = (const struct pmbus_status_assoc[]) {
+ { PB_TEMP_OT_WARNING, REGULATOR_ERROR_OVER_TEMP_WARN },
+ { PB_TEMP_OT_FAULT, REGULATOR_ERROR_OVER_TEMP },
+ { },
+ },
+ },
+};
+
#if IS_ENABLED(CONFIG_REGULATOR)
static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
{
@@ -2738,54 +2781,11 @@ static int pmbus_regulator_disable(struct regulator_dev *rdev)
return _pmbus_regulator_on_off(rdev, 0);
}
-/* A PMBus status flag and the corresponding REGULATOR_ERROR_* flag */
-struct pmbus_regulator_status_assoc {
- int pflag, rflag;
-};
-
-/* PMBus->regulator bit mappings for a PMBus status register */
-struct pmbus_regulator_status_category {
- int func;
- int reg;
- const struct pmbus_regulator_status_assoc *bits; /* zero-terminated */
-};
-
-static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = {
- {
- .func = PMBUS_HAVE_STATUS_VOUT,
- .reg = PMBUS_STATUS_VOUT,
- .bits = (const struct pmbus_regulator_status_assoc[]) {
- { PB_VOLTAGE_UV_WARNING, REGULATOR_ERROR_UNDER_VOLTAGE_WARN },
- { PB_VOLTAGE_UV_FAULT, REGULATOR_ERROR_UNDER_VOLTAGE },
- { PB_VOLTAGE_OV_WARNING, REGULATOR_ERROR_OVER_VOLTAGE_WARN },
- { PB_VOLTAGE_OV_FAULT, REGULATOR_ERROR_REGULATION_OUT },
- { },
- },
- }, {
- .func = PMBUS_HAVE_STATUS_IOUT,
- .reg = PMBUS_STATUS_IOUT,
- .bits = (const struct pmbus_regulator_status_assoc[]) {
- { PB_IOUT_OC_WARNING, REGULATOR_ERROR_OVER_CURRENT_WARN },
- { PB_IOUT_OC_FAULT, REGULATOR_ERROR_OVER_CURRENT },
- { PB_IOUT_OC_LV_FAULT, REGULATOR_ERROR_OVER_CURRENT },
- { },
- },
- }, {
- .func = PMBUS_HAVE_STATUS_TEMP,
- .reg = PMBUS_STATUS_TEMPERATURE,
- .bits = (const struct pmbus_regulator_status_assoc[]) {
- { PB_TEMP_OT_WARNING, REGULATOR_ERROR_OVER_TEMP_WARN },
- { PB_TEMP_OT_FAULT, REGULATOR_ERROR_OVER_TEMP },
- { },
- },
- },
-};
-
static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags)
{
int i, status;
- const struct pmbus_regulator_status_category *cat;
- const struct pmbus_regulator_status_assoc *bit;
+ const struct pmbus_status_category *cat;
+ const struct pmbus_status_assoc *bit;
struct device *dev = rdev_get_dev(rdev);
struct i2c_client *client = to_i2c_client(dev->parent);
struct pmbus_data *data = i2c_get_clientdata(client);
@@ -2796,8 +2796,8 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
mutex_lock(&data->update_lock);
- for (i = 0; i < ARRAY_SIZE(pmbus_regulator_flag_map); i++) {
- cat = &pmbus_regulator_flag_map[i];
+ for (i = 0; i < ARRAY_SIZE(pmbus_status_flag_map); i++) {
+ cat = &pmbus_status_flag_map[i];
if (!(func & cat->func))
continue;
base-commit: 5720a18baa4686d56d0a235e6ecbcc55f8d716d7
--
2.39.1
Add function pmbus get status that can be used to get both pmbus
specific status & regulator status
Signed-off-by: Naresh Solanki <[email protected]>
...
Changes in V3:
- Add pmbus_is_enabled function
Changes in V2:
- Add __maybe attribute for pmbus_get_status function
- Remove unrelated changes
---
drivers/hwmon/pmbus/pmbus_core.c | 98 ++++++++++++++++++++------------
1 file changed, 62 insertions(+), 36 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 1b70cf3be313..f8ac9016ea0e 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2735,18 +2735,12 @@ static const struct pmbus_status_category __maybe_unused pmbus_status_flag_map[]
},
};
-#if IS_ENABLED(CONFIG_REGULATOR)
-static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
+static int _pmbus_is_enabled(struct device *dev, u8 page)
{
- struct device *dev = rdev_get_dev(rdev);
struct i2c_client *client = to_i2c_client(dev->parent);
- struct pmbus_data *data = i2c_get_clientdata(client);
- u8 page = rdev_get_id(rdev);
int ret;
- mutex_lock(&data->update_lock);
ret = _pmbus_read_byte_data(client, page, PMBUS_OPERATION);
- mutex_unlock(&data->update_lock);
if (ret < 0)
return ret;
@@ -2754,58 +2748,38 @@ static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
return !!(ret & PB_OPERATION_CONTROL_ON);
}
-static int _pmbus_regulator_on_off(struct regulator_dev *rdev, bool enable)
+static int __maybe_unused pmbus_is_enabled(struct device *dev, u8 page)
{
- struct device *dev = rdev_get_dev(rdev);
struct i2c_client *client = to_i2c_client(dev->parent);
struct pmbus_data *data = i2c_get_clientdata(client);
- u8 page = rdev_get_id(rdev);
int ret;
mutex_lock(&data->update_lock);
- ret = pmbus_update_byte_data(client, page, PMBUS_OPERATION,
- PB_OPERATION_CONTROL_ON,
- enable ? PB_OPERATION_CONTROL_ON : 0);
+ ret = _pmbus_is_enabled(dev, page);
mutex_unlock(&data->update_lock);
- return ret;
-}
-
-static int pmbus_regulator_enable(struct regulator_dev *rdev)
-{
- return _pmbus_regulator_on_off(rdev, 1);
-}
-
-static int pmbus_regulator_disable(struct regulator_dev *rdev)
-{
- return _pmbus_regulator_on_off(rdev, 0);
+ return !!(ret & PB_OPERATION_CONTROL_ON);
}
-static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags)
+static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags)
{
int i, status;
const struct pmbus_status_category *cat;
const struct pmbus_status_assoc *bit;
- struct device *dev = rdev_get_dev(rdev);
- struct i2c_client *client = to_i2c_client(dev->parent);
- struct pmbus_data *data = i2c_get_clientdata(client);
- u8 page = rdev_get_id(rdev);
+ struct device *dev = data->dev;
+ struct i2c_client *client = to_i2c_client(dev);
int func = data->info->func[page];
*flags = 0;
- mutex_lock(&data->update_lock);
-
for (i = 0; i < ARRAY_SIZE(pmbus_status_flag_map); i++) {
cat = &pmbus_status_flag_map[i];
if (!(func & cat->func))
continue;
status = _pmbus_read_byte_data(client, page, cat->reg);
- if (status < 0) {
- mutex_unlock(&data->update_lock);
+ if (status < 0)
return status;
- }
for (bit = cat->bits; bit->pflag; bit++) {
if (status & bit->pflag)
@@ -2823,11 +2797,10 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
* REGULATOR_ERROR_<foo>_WARN.
*/
status = pmbus_get_status(client, page, PMBUS_STATUS_WORD);
- mutex_unlock(&data->update_lock);
if (status < 0)
return status;
- if (pmbus_regulator_is_enabled(rdev)) {
+ if (_pmbus_is_enabled(dev, page)) {
if (status & PB_STATUS_OFF)
*flags |= REGULATOR_ERROR_FAIL;
@@ -2855,6 +2828,59 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
return 0;
}
+static int __maybe_unused pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags)
+{
+ int ret;
+
+ mutex_lock(&data->update_lock);
+ ret = _pmbus_get_flags(data, page, flags);
+ mutex_unlock(&data->update_lock);
+
+ return ret;
+}
+
+#if IS_ENABLED(CONFIG_REGULATOR)
+static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
+{
+ return pmbus_is_enabled(rdev_get_dev(rdev), rdev_get_id(rdev));
+}
+
+static int _pmbus_regulator_on_off(struct regulator_dev *rdev, bool enable)
+{
+ struct device *dev = rdev_get_dev(rdev);
+ struct i2c_client *client = to_i2c_client(dev->parent);
+ struct pmbus_data *data = i2c_get_clientdata(client);
+ u8 page = rdev_get_id(rdev);
+ int ret;
+
+ mutex_lock(&data->update_lock);
+ ret = pmbus_update_byte_data(client, page, PMBUS_OPERATION,
+ PB_OPERATION_CONTROL_ON,
+ enable ? PB_OPERATION_CONTROL_ON : 0);
+ mutex_unlock(&data->update_lock);
+
+ return ret;
+}
+
+static int pmbus_regulator_enable(struct regulator_dev *rdev)
+{
+ return _pmbus_regulator_on_off(rdev, 1);
+}
+
+static int pmbus_regulator_disable(struct regulator_dev *rdev)
+{
+ return _pmbus_regulator_on_off(rdev, 0);
+}
+
+static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags)
+{
+ struct device *dev = rdev_get_dev(rdev);
+ struct i2c_client *client = to_i2c_client(dev->parent);
+ struct pmbus_data *data = i2c_get_clientdata(client);
+
+ return pmbus_get_flags(data, rdev_get_id(rdev), flags);
+}
+
static int pmbus_regulator_get_status(struct regulator_dev *rdev)
{
struct device *dev = rdev_get_dev(rdev);
--
2.39.1
From: Patrick Rudolph <[email protected]>
Implement PMBUS irq handler.
Signed-off-by: Patrick Rudolph <[email protected]>
Signed-off-by: Naresh Solanki <[email protected]>
...
Changes in V3:
- Remove pmbus word check for SMBALERT writes
- Remove variable ret & use err instead.
- Use dev_dbg_once instead of error.
- Remove error print when writing to misc_status register.
- Move client irq check to pmbus_irq_setup.
---
drivers/hwmon/pmbus/pmbus.h | 2 +-
drivers/hwmon/pmbus/pmbus_core.c | 78 ++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 713ea7915425..11e84e141126 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 f8ac9016ea0e..d0415d5ac7d9 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -3105,6 +3105,80 @@ static int pmbus_regulator_register(struct pmbus_data *data)
}
#endif
+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, status;
+
+ mutex_lock(&data->update_lock);
+ for (i = 0; i < data->info->pages; i++) {
+ status = pmbus_read_status_word(client, i);
+ 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, i);
+ }
+ mutex_unlock(&data->update_lock);
+
+ return IRQ_HANDLED;
+}
+
+static int pmbus_irq_setup(struct i2c_client *client, struct pmbus_data *data)
+{
+ struct device *dev = &client->dev;
+ const struct pmbus_status_category *cat;
+ const struct pmbus_status_assoc *bit;
+ int i, j, err, func;
+ u8 mask;
+
+ static const u8 misc_status[] = {PMBUS_STATUS_CML, PMBUS_STATUS_OTHER,
+ PMBUS_STATUS_MFR_SPECIFIC, PMBUS_STATUS_FAN_12,
+ PMBUS_STATUS_FAN_34};
+
+ if (!client->irq)
+ return 0;
+
+ for (i = 0; i < data->info->pages; i++) {
+ func = data->info->func[i];
+
+ for (j = 0; j < ARRAY_SIZE(pmbus_status_flag_map); j++) {
+ cat = &pmbus_status_flag_map[j];
+ 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_dbg_once(dev, "Failed to set smbalert for reg 0x%02x\n",
+ cat->reg);
+ }
+
+ for (j = 0; j < ARRAY_SIZE(misc_status); j++)
+ pmbus_write_smbalert_mask(client, i, misc_status[j], 0xff);
+ }
+
+ /* Register notifiers */
+ err = devm_request_threaded_irq(dev, client->irq, NULL, pmbus_fault_handler, 0,
+ "pmbus-irq", data);
+ if (err) {
+ dev_err(dev, "failed to request an irq %d\n", err);
+ return err;
+ }
+
+ return 0;
+}
+
static struct dentry *pmbus_debugfs_dir; /* pmbus debugfs directory */
#if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -3467,6 +3541,10 @@ int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info)
if (ret)
return ret;
+ 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");
--
2.39.1
From: Patrick Rudolph <[email protected]>
Notify hwmon events using the pmbus irq handler.
Signed-off-by: Patrick Rudolph <[email protected]>
Signed-off-by: Naresh Solanki <[email protected]>
...
Changes in V3:
- Remove empty line
- locked version of pmbus_get_flags
Changes in V2
- Remove __maybe_unsed attribute as its not needed.
---
drivers/hwmon/pmbus/pmbus_core.c | 48 ++++++++++++++++++++++++++++----
1 file changed, 43 insertions(+), 5 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index d0415d5ac7d9..2d10a354fe7a 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2761,7 +2761,35 @@ static int __maybe_unused pmbus_is_enabled(struct device *dev, u8 page)
return !!(ret & PB_OPERATION_CONTROL_ON);
}
-static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags)
+#define to_dev_attr(_dev_attr) \
+ container_of(_dev_attr, struct device_attribute, attr)
+
+static void pmbus_notify(struct pmbus_data *data, int page, int reg, int flags)
+{
+ int i;
+
+ for (i = 0; i < data->num_attributes; i++) {
+ struct device_attribute *da = to_dev_attr(data->group.attrs[i]);
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ int index = attr->index;
+ u16 smask = pb_index_to_mask(index);
+ u8 spage = pb_index_to_page(index);
+ u16 sreg = pb_index_to_reg(index);
+
+ if (reg == sreg && page == spage && (smask & flags)) {
+ dev_dbg(data->dev, "sysfs notify: %s", da->attr.name);
+ sysfs_notify(&data->dev->kobj, NULL, da->attr.name);
+ kobject_uevent(&data->dev->kobj, KOBJ_CHANGE);
+ flags &= ~smask;
+ }
+
+ if (!flags)
+ break;
+ }
+}
+
+static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags,
+ bool notify)
{
int i, status;
const struct pmbus_status_category *cat;
@@ -2785,6 +2813,10 @@ static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flag
if (status & bit->pflag)
*flags |= bit->rflag;
}
+
+ if (notify && status)
+ pmbus_notify(data, page, cat->reg, status);
+
}
/*
@@ -2828,12 +2860,13 @@ static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flag
return 0;
}
-static int __maybe_unused pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags)
+static int __maybe_unused pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags,
+ bool notify)
{
int ret;
mutex_lock(&data->update_lock);
- ret = _pmbus_get_flags(data, page, flags);
+ ret = _pmbus_get_flags(data, page, flags, notify);
mutex_unlock(&data->update_lock);
return ret;
@@ -2878,7 +2911,7 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
struct i2c_client *client = to_i2c_client(dev->parent);
struct pmbus_data *data = i2c_get_clientdata(client);
- return pmbus_get_flags(data, rdev_get_id(rdev), flags);
+ return pmbus_get_flags(data, rdev_get_id(rdev), flags, false);
}
static int pmbus_regulator_get_status(struct regulator_dev *rdev)
@@ -3114,10 +3147,14 @@ 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, status;
+ int i, status, ret;
mutex_lock(&data->update_lock);
for (i = 0; i < data->info->pages; i++) {
+ ret = _pmbus_get_flags(data, i, &status, true);
+ if (ret)
+ return ret;
+
status = pmbus_read_status_word(client, i);
if (status < 0) {
mutex_unlock(&data->update_lock);
@@ -3126,6 +3163,7 @@ static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
if (status & ~(PB_STATUS_OFF | PB_STATUS_BUSY | PB_STATUS_POWER_GOOD_N))
pmbus_clear_fault_page(client, i);
+
}
mutex_unlock(&data->update_lock);
--
2.39.1
On Fri, Feb 17, 2023 at 09:36:30AM +0100, Naresh Solanki wrote:
> From: Patrick Rudolph <[email protected]>
>
> Implement PMBUS irq handler.
>
> Signed-off-by: Patrick Rudolph <[email protected]>
> Signed-off-by: Naresh Solanki <[email protected]>
> ...
> Changes in V3:
> - Remove pmbus word check for SMBALERT writes
> - Remove variable ret & use err instead.
> - Use dev_dbg_once instead of error.
> - Remove error print when writing to misc_status register.
> - Move client irq check to pmbus_irq_setup.
> ---
> drivers/hwmon/pmbus/pmbus.h | 2 +-
> drivers/hwmon/pmbus/pmbus_core.c | 78 ++++++++++++++++++++++++++++++++
> 2 files changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index 713ea7915425..11e84e141126 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 f8ac9016ea0e..d0415d5ac7d9 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -3105,6 +3105,80 @@ static int pmbus_regulator_register(struct pmbus_data *data)
> }
> #endif
>
> +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, status;
> +
> + mutex_lock(&data->update_lock);
> + for (i = 0; i < data->info->pages; i++) {
> + status = pmbus_read_status_word(client, i);
> + if (status < 0) {
> + mutex_unlock(&data->update_lock);
> + return status;
Unfortunately this is not a valid return value for interrupt handlers.
I think the best approach here would be to just continue with the loop.
> + }
> +
> + if (status & ~(PB_STATUS_OFF | PB_STATUS_BUSY | PB_STATUS_POWER_GOOD_N))
> + pmbus_clear_fault_page(client, i);
Should those fault flags even be checked ? If another bit is set, they will
be cleared anyway, and "clear flags only if some other flag is set" seems
a bit inconsistent.
Overall it seems to me that we should maybe just call pmbus_clear_faults()
at the end of the function. Sure, that clears faults on pages where there
is no fault, but the above code reads the status on all pages anyway, so
it is more costly overall then just clearing the faults on all pages.
> + }
> + mutex_unlock(&data->update_lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int pmbus_irq_setup(struct i2c_client *client, struct pmbus_data *data)
> +{
> + struct device *dev = &client->dev;
> + const struct pmbus_status_category *cat;
> + const struct pmbus_status_assoc *bit;
> + int i, j, err, func;
> + u8 mask;
> +
> + static const u8 misc_status[] = {PMBUS_STATUS_CML, PMBUS_STATUS_OTHER,
> + PMBUS_STATUS_MFR_SPECIFIC, PMBUS_STATUS_FAN_12,
> + PMBUS_STATUS_FAN_34};
> +
> + if (!client->irq)
> + return 0;
> +
> + for (i = 0; i < data->info->pages; i++) {
> + func = data->info->func[i];
> +
> + for (j = 0; j < ARRAY_SIZE(pmbus_status_flag_map); j++) {
> + cat = &pmbus_status_flag_map[j];
> + 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_dbg_once(dev, "Failed to set smbalert for reg 0x%02x\n",
> + cat->reg);
> + }
> +
> + for (j = 0; j < ARRAY_SIZE(misc_status); j++)
> + pmbus_write_smbalert_mask(client, i, misc_status[j], 0xff);
> + }
> +
> + /* Register notifiers */
> + err = devm_request_threaded_irq(dev, client->irq, NULL, pmbus_fault_handler, 0,
> + "pmbus-irq", data);
> + if (err) {
> + dev_err(dev, "failed to request an irq %d\n", err);
> + return err;
> + }
> +
> + return 0;
> +}
> +
> static struct dentry *pmbus_debugfs_dir; /* pmbus debugfs directory */
>
> #if IS_ENABLED(CONFIG_DEBUG_FS)
> @@ -3467,6 +3541,10 @@ int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info)
> if (ret)
> return ret;
>
> + 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");
On 2/17/23 00:36, Naresh Solanki wrote:
> From: Patrick Rudolph <[email protected]>
>
> Notify hwmon events using the pmbus irq handler.
>
> Signed-off-by: Patrick Rudolph <[email protected]>
> Signed-off-by: Naresh Solanki <[email protected]>
> ...
> Changes in V3:
> - Remove empty line
> - locked version of pmbus_get_flags
> Changes in V2
> - Remove __maybe_unsed attribute as its not needed.
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 48 ++++++++++++++++++++++++++++----
> 1 file changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index d0415d5ac7d9..2d10a354fe7a 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -2761,7 +2761,35 @@ static int __maybe_unused pmbus_is_enabled(struct device *dev, u8 page)
> return !!(ret & PB_OPERATION_CONTROL_ON);
> }
>
> -static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags)
> +#define to_dev_attr(_dev_attr) \
> + container_of(_dev_attr, struct device_attribute, attr)
> +
> +static void pmbus_notify(struct pmbus_data *data, int page, int reg, int flags)
> +{
> + int i;
> +
> + for (i = 0; i < data->num_attributes; i++) {
> + struct device_attribute *da = to_dev_attr(data->group.attrs[i]);
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + int index = attr->index;
> + u16 smask = pb_index_to_mask(index);
> + u8 spage = pb_index_to_page(index);
> + u16 sreg = pb_index_to_reg(index);
> +
> + if (reg == sreg && page == spage && (smask & flags)) {
> + dev_dbg(data->dev, "sysfs notify: %s", da->attr.name);
> + sysfs_notify(&data->dev->kobj, NULL, da->attr.name);
> + kobject_uevent(&data->dev->kobj, KOBJ_CHANGE);
> + flags &= ~smask;
> + }
> +
> + if (!flags)
> + break;
> + }
> +}
> +
> +static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags,
> + bool notify)
> {
> int i, status;
> const struct pmbus_status_category *cat;
> @@ -2785,6 +2813,10 @@ static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flag
> if (status & bit->pflag)
> *flags |= bit->rflag;
> }
> +
> + if (notify && status)
> + pmbus_notify(data, page, cat->reg, status);
> +
> }
>
> /*
> @@ -2828,12 +2860,13 @@ static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flag
> return 0;
> }
>
> -static int __maybe_unused pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags)
> +static int __maybe_unused pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags,
> + bool notify)
> {
> int ret;
>
> mutex_lock(&data->update_lock);
> - ret = _pmbus_get_flags(data, page, flags);
> + ret = _pmbus_get_flags(data, page, flags, notify);
> mutex_unlock(&data->update_lock);
>
> return ret;
> @@ -2878,7 +2911,7 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
> struct i2c_client *client = to_i2c_client(dev->parent);
> struct pmbus_data *data = i2c_get_clientdata(client);
>
> - return pmbus_get_flags(data, rdev_get_id(rdev), flags);
> + return pmbus_get_flags(data, rdev_get_id(rdev), flags, false);
> }
>
> static int pmbus_regulator_get_status(struct regulator_dev *rdev)
> @@ -3114,10 +3147,14 @@ 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, status;
> + int i, status, ret;
>
> mutex_lock(&data->update_lock);
> for (i = 0; i < data->info->pages; i++) {
> + ret = _pmbus_get_flags(data, i, &status, true);
> + if (ret)
> + return ret;
> +
Same as with patch 3 - this is not a valid return value for an
interrupt handler.
> status = pmbus_read_status_word(client, i);
> if (status < 0) {
> mutex_unlock(&data->update_lock);
> @@ -3126,6 +3163,7 @@ static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
>
> if (status & ~(PB_STATUS_OFF | PB_STATUS_BUSY | PB_STATUS_POWER_GOOD_N))
> pmbus_clear_fault_page(client, i);
> +
Stray whitespace change.
> }
> mutex_unlock(&data->update_lock);
>
On Fri, Feb 17, 2023 at 09:36:28AM +0100, Naresh Solanki wrote:
> The PMBus status flag map(pmbus_regulator_status_flag_map) is moved
> outside of the regulator #if block and the associated variable/struct
> name updated to reflect as generic PMBus status.
>
> This will make the PMBus status flag map more versatile and easier to
> incorporate into different contexts and functions.
>
> Signed-off-by: Naresh Solanki <[email protected]>
For my reference:
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 94 ++++++++++++++++----------------
> 1 file changed, 47 insertions(+), 47 deletions(-)
>
>
> base-commit: 5720a18baa4686d56d0a235e6ecbcc55f8d716d7
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 95e95783972a..1b70cf3be313 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -2692,6 +2692,49 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,
> return 0;
> }
>
> +/* A PMBus status flag and the corresponding REGULATOR_ERROR_* flag */
> +struct pmbus_status_assoc {
> + int pflag, rflag;
> +};
> +
> +/* PMBus->regulator bit mappings for a PMBus status register */
> +struct pmbus_status_category {
> + int func;
> + int reg;
> + const struct pmbus_status_assoc *bits; /* zero-terminated */
> +};
> +
> +static const struct pmbus_status_category __maybe_unused pmbus_status_flag_map[] = {
> + {
> + .func = PMBUS_HAVE_STATUS_VOUT,
> + .reg = PMBUS_STATUS_VOUT,
> + .bits = (const struct pmbus_status_assoc[]) {
> + { PB_VOLTAGE_UV_WARNING, REGULATOR_ERROR_UNDER_VOLTAGE_WARN },
> + { PB_VOLTAGE_UV_FAULT, REGULATOR_ERROR_UNDER_VOLTAGE },
> + { PB_VOLTAGE_OV_WARNING, REGULATOR_ERROR_OVER_VOLTAGE_WARN },
> + { PB_VOLTAGE_OV_FAULT, REGULATOR_ERROR_REGULATION_OUT },
> + { },
> + },
> + }, {
> + .func = PMBUS_HAVE_STATUS_IOUT,
> + .reg = PMBUS_STATUS_IOUT,
> + .bits = (const struct pmbus_status_assoc[]) {
> + { PB_IOUT_OC_WARNING, REGULATOR_ERROR_OVER_CURRENT_WARN },
> + { PB_IOUT_OC_FAULT, REGULATOR_ERROR_OVER_CURRENT },
> + { PB_IOUT_OC_LV_FAULT, REGULATOR_ERROR_OVER_CURRENT },
> + { },
> + },
> + }, {
> + .func = PMBUS_HAVE_STATUS_TEMP,
> + .reg = PMBUS_STATUS_TEMPERATURE,
> + .bits = (const struct pmbus_status_assoc[]) {
> + { PB_TEMP_OT_WARNING, REGULATOR_ERROR_OVER_TEMP_WARN },
> + { PB_TEMP_OT_FAULT, REGULATOR_ERROR_OVER_TEMP },
> + { },
> + },
> + },
> +};
> +
> #if IS_ENABLED(CONFIG_REGULATOR)
> static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
> {
> @@ -2738,54 +2781,11 @@ static int pmbus_regulator_disable(struct regulator_dev *rdev)
> return _pmbus_regulator_on_off(rdev, 0);
> }
>
> -/* A PMBus status flag and the corresponding REGULATOR_ERROR_* flag */
> -struct pmbus_regulator_status_assoc {
> - int pflag, rflag;
> -};
> -
> -/* PMBus->regulator bit mappings for a PMBus status register */
> -struct pmbus_regulator_status_category {
> - int func;
> - int reg;
> - const struct pmbus_regulator_status_assoc *bits; /* zero-terminated */
> -};
> -
> -static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = {
> - {
> - .func = PMBUS_HAVE_STATUS_VOUT,
> - .reg = PMBUS_STATUS_VOUT,
> - .bits = (const struct pmbus_regulator_status_assoc[]) {
> - { PB_VOLTAGE_UV_WARNING, REGULATOR_ERROR_UNDER_VOLTAGE_WARN },
> - { PB_VOLTAGE_UV_FAULT, REGULATOR_ERROR_UNDER_VOLTAGE },
> - { PB_VOLTAGE_OV_WARNING, REGULATOR_ERROR_OVER_VOLTAGE_WARN },
> - { PB_VOLTAGE_OV_FAULT, REGULATOR_ERROR_REGULATION_OUT },
> - { },
> - },
> - }, {
> - .func = PMBUS_HAVE_STATUS_IOUT,
> - .reg = PMBUS_STATUS_IOUT,
> - .bits = (const struct pmbus_regulator_status_assoc[]) {
> - { PB_IOUT_OC_WARNING, REGULATOR_ERROR_OVER_CURRENT_WARN },
> - { PB_IOUT_OC_FAULT, REGULATOR_ERROR_OVER_CURRENT },
> - { PB_IOUT_OC_LV_FAULT, REGULATOR_ERROR_OVER_CURRENT },
> - { },
> - },
> - }, {
> - .func = PMBUS_HAVE_STATUS_TEMP,
> - .reg = PMBUS_STATUS_TEMPERATURE,
> - .bits = (const struct pmbus_regulator_status_assoc[]) {
> - { PB_TEMP_OT_WARNING, REGULATOR_ERROR_OVER_TEMP_WARN },
> - { PB_TEMP_OT_FAULT, REGULATOR_ERROR_OVER_TEMP },
> - { },
> - },
> - },
> -};
> -
> static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags)
> {
> int i, status;
> - const struct pmbus_regulator_status_category *cat;
> - const struct pmbus_regulator_status_assoc *bit;
> + const struct pmbus_status_category *cat;
> + const struct pmbus_status_assoc *bit;
> struct device *dev = rdev_get_dev(rdev);
> struct i2c_client *client = to_i2c_client(dev->parent);
> struct pmbus_data *data = i2c_get_clientdata(client);
> @@ -2796,8 +2796,8 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
>
> mutex_lock(&data->update_lock);
>
> - for (i = 0; i < ARRAY_SIZE(pmbus_regulator_flag_map); i++) {
> - cat = &pmbus_regulator_flag_map[i];
> + for (i = 0; i < ARRAY_SIZE(pmbus_status_flag_map); i++) {
> + cat = &pmbus_status_flag_map[i];
> if (!(func & cat->func))
> continue;
>
On Fri, Feb 17, 2023 at 09:36:29AM +0100, Naresh Solanki wrote:
> Add function pmbus get status that can be used to get both pmbus
> specific status & regulator status
>
> Signed-off-by: Naresh Solanki <[email protected]>
For my reference:
Reviewed-by: Guenter Roeck <[email protected]>
> ...
> Changes in V3:
> - Add pmbus_is_enabled function
> Changes in V2:
> - Add __maybe attribute for pmbus_get_status function
> - Remove unrelated changes
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 98 ++++++++++++++++++++------------
> 1 file changed, 62 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 1b70cf3be313..f8ac9016ea0e 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -2735,18 +2735,12 @@ static const struct pmbus_status_category __maybe_unused pmbus_status_flag_map[]
> },
> };
>
> -#if IS_ENABLED(CONFIG_REGULATOR)
> -static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
> +static int _pmbus_is_enabled(struct device *dev, u8 page)
> {
> - struct device *dev = rdev_get_dev(rdev);
> struct i2c_client *client = to_i2c_client(dev->parent);
> - struct pmbus_data *data = i2c_get_clientdata(client);
> - u8 page = rdev_get_id(rdev);
> int ret;
>
> - mutex_lock(&data->update_lock);
> ret = _pmbus_read_byte_data(client, page, PMBUS_OPERATION);
> - mutex_unlock(&data->update_lock);
>
> if (ret < 0)
> return ret;
> @@ -2754,58 +2748,38 @@ static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
> return !!(ret & PB_OPERATION_CONTROL_ON);
> }
>
> -static int _pmbus_regulator_on_off(struct regulator_dev *rdev, bool enable)
> +static int __maybe_unused pmbus_is_enabled(struct device *dev, u8 page)
> {
> - struct device *dev = rdev_get_dev(rdev);
> struct i2c_client *client = to_i2c_client(dev->parent);
> struct pmbus_data *data = i2c_get_clientdata(client);
> - u8 page = rdev_get_id(rdev);
> int ret;
>
> mutex_lock(&data->update_lock);
> - ret = pmbus_update_byte_data(client, page, PMBUS_OPERATION,
> - PB_OPERATION_CONTROL_ON,
> - enable ? PB_OPERATION_CONTROL_ON : 0);
> + ret = _pmbus_is_enabled(dev, page);
> mutex_unlock(&data->update_lock);
>
> - return ret;
> -}
> -
> -static int pmbus_regulator_enable(struct regulator_dev *rdev)
> -{
> - return _pmbus_regulator_on_off(rdev, 1);
> -}
> -
> -static int pmbus_regulator_disable(struct regulator_dev *rdev)
> -{
> - return _pmbus_regulator_on_off(rdev, 0);
> + return !!(ret & PB_OPERATION_CONTROL_ON);
> }
>
> -static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags)
> +static int _pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags)
> {
> int i, status;
> const struct pmbus_status_category *cat;
> const struct pmbus_status_assoc *bit;
> - struct device *dev = rdev_get_dev(rdev);
> - struct i2c_client *client = to_i2c_client(dev->parent);
> - struct pmbus_data *data = i2c_get_clientdata(client);
> - u8 page = rdev_get_id(rdev);
> + struct device *dev = data->dev;
> + struct i2c_client *client = to_i2c_client(dev);
> int func = data->info->func[page];
>
> *flags = 0;
>
> - mutex_lock(&data->update_lock);
> -
> for (i = 0; i < ARRAY_SIZE(pmbus_status_flag_map); i++) {
> cat = &pmbus_status_flag_map[i];
> if (!(func & cat->func))
> continue;
>
> status = _pmbus_read_byte_data(client, page, cat->reg);
> - if (status < 0) {
> - mutex_unlock(&data->update_lock);
> + if (status < 0)
> return status;
> - }
>
> for (bit = cat->bits; bit->pflag; bit++) {
> if (status & bit->pflag)
> @@ -2823,11 +2797,10 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
> * REGULATOR_ERROR_<foo>_WARN.
> */
> status = pmbus_get_status(client, page, PMBUS_STATUS_WORD);
> - mutex_unlock(&data->update_lock);
> if (status < 0)
> return status;
>
> - if (pmbus_regulator_is_enabled(rdev)) {
> + if (_pmbus_is_enabled(dev, page)) {
> if (status & PB_STATUS_OFF)
> *flags |= REGULATOR_ERROR_FAIL;
>
> @@ -2855,6 +2828,59 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
> return 0;
> }
>
> +static int __maybe_unused pmbus_get_flags(struct pmbus_data *data, u8 page, unsigned int *flags)
> +{
> + int ret;
> +
> + mutex_lock(&data->update_lock);
> + ret = _pmbus_get_flags(data, page, flags);
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +#if IS_ENABLED(CONFIG_REGULATOR)
> +static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
> +{
> + return pmbus_is_enabled(rdev_get_dev(rdev), rdev_get_id(rdev));
> +}
> +
> +static int _pmbus_regulator_on_off(struct regulator_dev *rdev, bool enable)
> +{
> + struct device *dev = rdev_get_dev(rdev);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + struct pmbus_data *data = i2c_get_clientdata(client);
> + u8 page = rdev_get_id(rdev);
> + int ret;
> +
> + mutex_lock(&data->update_lock);
> + ret = pmbus_update_byte_data(client, page, PMBUS_OPERATION,
> + PB_OPERATION_CONTROL_ON,
> + enable ? PB_OPERATION_CONTROL_ON : 0);
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +static int pmbus_regulator_enable(struct regulator_dev *rdev)
> +{
> + return _pmbus_regulator_on_off(rdev, 1);
> +}
> +
> +static int pmbus_regulator_disable(struct regulator_dev *rdev)
> +{
> + return _pmbus_regulator_on_off(rdev, 0);
> +}
> +
> +static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned int *flags)
> +{
> + struct device *dev = rdev_get_dev(rdev);
> + struct i2c_client *client = to_i2c_client(dev->parent);
> + struct pmbus_data *data = i2c_get_clientdata(client);
> +
> + return pmbus_get_flags(data, rdev_get_id(rdev), flags);
> +}
> +
> static int pmbus_regulator_get_status(struct regulator_dev *rdev)
> {
> struct device *dev = rdev_get_dev(rdev);