[linux,dev-4.10,2/2] drivers: hwmon: occ: Add sysfs_notify to throttling attributes

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
Related show

Commit Message

Eddie James Oct. 6, 2017, 8:43 p.m.
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(-)

Comments

Andrew Jeffery Oct. 13, 2017, 6:11 a.m. | #1
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
Eddie James Oct. 13, 2017, 3:15 p.m. | #2
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
Andrew Jeffery Oct. 16, 2017, 5:31 a.m. | #3
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

Patch

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: