| Message ID | 1507322581-13947-3-git-send-email-eajames@linux.vnet.ibm.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drivers: hwmon: occ: sysfs_notify and presence detect | expand |
On Fri, 2017-10-06 at 15:43 -0500, Eddie James wrote: > From: "Edward A. James" <eajames@us.ibm.com> > > Need to alert user space when we change throttling state. > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > --- > drivers/hwmon/occ/common.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c > index e8579b6..242ba40 100644 > --- a/drivers/hwmon/occ/common.c > +++ b/drivers/hwmon/occ/common.c > @@ -182,6 +182,7 @@ int occ_poll(struct occ *occ) > u16 checksum = occ->poll_cmd_data + 1; > u8 cmd[8]; > u8 occs_present = header->occs_present; > + u8 ext_status = header->ext_status; > > cmd[0] = 0; > cmd[1] = 0; > @@ -206,10 +207,26 @@ int occ_poll(struct occ *occ) > } else > occ->last_safe = 0; > > - if (occs_present != header->occs_present && occ->hwmon && > - (header->status & OCC_STAT_MASTER)) { > - sysfs_notify(&occ->bus_dev->kobj, NULL, > - occ->status_attrs[7].dev_attr.attr.name); > + if (occ->hwmon) { > + if (occs_present != header->occs_present && > + (header->status & OCC_STAT_MASTER)) > + sysfs_notify(&occ->bus_dev->kobj, NULL, > + occ->status_attrs[7].dev_attr.attr.name); Why are we now protecting this under the occ->hwmon condition? Shouldn't we have done that in the previous patch which introduced the notify for occs_present? Andrew
On 10/13/2017 01:11 AM, Andrew Jeffery wrote: > On Fri, 2017-10-06 at 15:43 -0500, Eddie James wrote: >> From: "Edward A. James" <eajames@us.ibm.com> >> >> Need to alert user space when we change throttling state. >> >> Signed-off-by: Edward A. James <eajames@us.ibm.com> >> --- >> drivers/hwmon/occ/common.c | 25 +++++++++++++++++++++---- >> 1 file changed, 21 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c >> index e8579b6..242ba40 100644 >> --- a/drivers/hwmon/occ/common.c >> +++ b/drivers/hwmon/occ/common.c >> @@ -182,6 +182,7 @@ int occ_poll(struct occ *occ) >> u16 checksum = occ->poll_cmd_data + 1; >> u8 cmd[8]; >> u8 occs_present = header->occs_present; >> + u8 ext_status = header->ext_status; >> >> cmd[0] = 0; >> cmd[1] = 0; >> @@ -206,10 +207,26 @@ int occ_poll(struct occ *occ) >> } else >> occ->last_safe = 0; >> >> - if (occs_present != header->occs_present && occ->hwmon && >> - (header->status & OCC_STAT_MASTER)) { >> - sysfs_notify(&occ->bus_dev->kobj, NULL, >> - occ->status_attrs[7].dev_attr.attr.name); >> + if (occ->hwmon) { >> + if (occs_present != header->occs_present && >> + (header->status & OCC_STAT_MASTER)) >> + sysfs_notify(&occ->bus_dev->kobj, NULL, >> + occ->status_attrs[7].dev_attr.attr.name); > Why are we now protecting this under the occ->hwmon condition? Shouldn't we > have done that in the previous patch which introduced the notify for > occs_present? Checking for occ->hwmon was there previously. Thanks, Eddie > > Andrew
On Fri, 2017-10-13 at 10:15 -0500, Eddie James wrote: > > On 10/13/2017 01:11 AM, Andrew Jeffery wrote: > > On Fri, 2017-10-06 at 15:43 -0500, Eddie James wrote: > > > > > > From: "Edward A. James" <eajames@us.ibm.com> > > > > > > Need to alert user space when we change throttling state. > > > > > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > > > --- > > > drivers/hwmon/occ/common.c | 25 +++++++++++++++++++++---- > > > 1 file changed, 21 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c > > > index e8579b6..242ba40 100644 > > > --- a/drivers/hwmon/occ/common.c > > > +++ b/drivers/hwmon/occ/common.c > > > @@ -182,6 +182,7 @@ int occ_poll(struct occ *occ) > > > u16 checksum = occ->poll_cmd_data + 1; > > > u8 cmd[8]; > > > u8 occs_present = header->occs_present; > > > + u8 ext_status = header->ext_status; > > > > > > cmd[0] = 0; > > > cmd[1] = 0; > > > @@ -206,10 +207,26 @@ int occ_poll(struct occ *occ) > > > } else > > > occ->last_safe = 0; > > > > > > - if (occs_present != header->occs_present && occ->hwmon && > > > - (header->status & OCC_STAT_MASTER)) { > > > - sysfs_notify(&occ->bus_dev->kobj, NULL, > > > - occ->status_attrs[7].dev_attr.attr.name); > > > + if (occ->hwmon) { > > > + if (occs_present != header->occs_present && > > > + (header->status & OCC_STAT_MASTER)) > > > + sysfs_notify(&occ->bus_dev->kobj, NULL, > > > + occ->status_attrs[7].dev_attr.attr.name); > > > > Why are we now protecting this under the occ->hwmon condition? Shouldn't we > > have done that in the previous patch which introduced the notify for > > occs_present? > > Checking for occ->hwmon was there previously. Ugh, yep, glad that was the point I put the laptop away for the evening. It would be less jarring to introduce the `if (occ-hwmon)` in the first patch, but lets not let that hold us up. Cheers, Andrew
diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c index e8579b6..242ba40 100644 --- a/drivers/hwmon/occ/common.c +++ b/drivers/hwmon/occ/common.c @@ -182,6 +182,7 @@ int occ_poll(struct occ *occ) u16 checksum = occ->poll_cmd_data + 1; u8 cmd[8]; u8 occs_present = header->occs_present; + u8 ext_status = header->ext_status; cmd[0] = 0; cmd[1] = 0; @@ -206,10 +207,26 @@ int occ_poll(struct occ *occ) } else occ->last_safe = 0; - if (occs_present != header->occs_present && occ->hwmon && - (header->status & OCC_STAT_MASTER)) { - sysfs_notify(&occ->bus_dev->kobj, NULL, - occ->status_attrs[7].dev_attr.attr.name); + if (occ->hwmon) { + if (occs_present != header->occs_present && + (header->status & OCC_STAT_MASTER)) + sysfs_notify(&occ->bus_dev->kobj, NULL, + occ->status_attrs[7].dev_attr.attr.name); + + if ((header->ext_status & OCC_EXT_STAT_DVFS_OT) != + (ext_status & OCC_EXT_STAT_DVFS_OT)) + sysfs_notify(&occ->bus_dev->kobj, NULL, + occ->status_attrs[2].dev_attr.attr.name); + + if ((header->ext_status & OCC_EXT_STAT_DVFS_POWER) != + (ext_status & OCC_EXT_STAT_DVFS_POWER)) + sysfs_notify(&occ->bus_dev->kobj, NULL, + occ->status_attrs[3].dev_attr.attr.name); + + if ((header->ext_status & OCC_EXT_STAT_MEM_THROTTLE) != + (ext_status & OCC_EXT_STAT_MEM_THROTTLE)) + sysfs_notify(&occ->bus_dev->kobj, NULL, + occ->status_attrs[4].dev_attr.attr.name); } done: