| Message ID | 20241212214927.3586509-2-ninad@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Intel CRPS PSU | expand |
On 12/12/24 13:49, Ninad Palsule wrote: > Adding label files for fan and temperature sensors in the power supply. > The openbmc application dbus-sensor(psusensor) requires those files to > consfigure those sensors. > Note that prefix for temp label is temp[A..C] used instead of temp[1..3] > as dbus-sensor(psusensor) application calculate index based on last > digit in the name so we do not want to make index double digit after > appending page index. > > Signed-off-by: Ninad Palsule <ninad@linux.ibm.com> We are not going to fix userspace problems in the kernel. Guenter > --- > drivers/hwmon/pmbus/pmbus_core.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c > index a7000314e5ad..149b3c5f3a4c 100644 > --- a/drivers/hwmon/pmbus/pmbus_core.c > +++ b/drivers/hwmon/pmbus/pmbus_core.c > @@ -2144,6 +2144,7 @@ static const struct pmbus_sensor_attr temp_attributes[] = { > { > .reg = PMBUS_READ_TEMPERATURE_1, > .class = PSC_TEMPERATURE, > + .label = "tempA", > .paged = true, > .update = true, > .compare = true, > @@ -2156,6 +2157,7 @@ static const struct pmbus_sensor_attr temp_attributes[] = { > }, { > .reg = PMBUS_READ_TEMPERATURE_2, > .class = PSC_TEMPERATURE, > + .label = "tempB", > .paged = true, > .update = true, > .compare = true, > @@ -2168,6 +2170,7 @@ static const struct pmbus_sensor_attr temp_attributes[] = { > }, { > .reg = PMBUS_READ_TEMPERATURE_3, > .class = PSC_TEMPERATURE, > + .label = "tempC", > .paged = true, > .update = true, > .compare = true, > @@ -2282,6 +2285,17 @@ static int pmbus_add_fan_attributes(struct i2c_client *client, > PSC_FAN, true, true, true) == NULL) > return -ENOMEM; > > + /* > + * Add fan label. > + * Assuming paged attributes while passing page index > + */ > + ret = pmbus_add_label(data, "fan", index, "fan", > + page + 1, 0xff); > + if (ret) { > + dev_err(data->dev, "Fan label add failed ret=%d\n", ret); > + return ret; > + } > + > /* Fan control */ > if (pmbus_check_word_register(client, page, > pmbus_fan_command_registers[f])) {
Hello Guenter, On 12/12/24 16:06, Guenter Roeck wrote: > On 12/12/24 13:49, Ninad Palsule wrote: >> Adding label files for fan and temperature sensors in the power supply. >> The openbmc application dbus-sensor(psusensor) requires those files to >> consfigure those sensors. >> Note that prefix for temp label is temp[A..C] used instead of temp[1..3] >> as dbus-sensor(psusensor) application calculate index based on last >> digit in the name so we do not want to make index double digit after >> appending page index. >> >> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com> > > We are not going to fix userspace problems in the kernel. > > Guenter > Thanks for the quick review. Sorry I am not clear on this. I feel that it is better to support labels for temperature sensors and fans like other. Are you saying we should not support these labels or I need update in the patch to support them better? Please let me know. Thanks & Regards, Ninad Palsule
On 12/13/24 08:12, Ninad Palsule wrote: > Hello Guenter, > > On 12/12/24 16:06, Guenter Roeck wrote: >> On 12/12/24 13:49, Ninad Palsule wrote: >>> Adding label files for fan and temperature sensors in the power supply. >>> The openbmc application dbus-sensor(psusensor) requires those files to >>> consfigure those sensors. >>> Note that prefix for temp label is temp[A..C] used instead of temp[1..3] >>> as dbus-sensor(psusensor) application calculate index based on last >>> digit in the name so we do not want to make index double digit after >>> appending page index. >>> >>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com> >> >> We are not going to fix userspace problems in the kernel. >> >> Guenter >> > > Thanks for the quick review. > > Sorry I am not clear on this. I feel that it is better to support labels for temperature > > sensors and fans like other. Are you saying we should not support these labels or > > I need update in the patch to support them better? > There should be no such labels. Labels are supposed to have specific meanings, such as "this is the CPU temperature sensor", not vague meanings such as "tempA". Guenter
Hi Guenter, On 12/13/24 11:08, Guenter Roeck wrote: > On 12/13/24 08:12, Ninad Palsule wrote: >> Hello Guenter, >> >> On 12/12/24 16:06, Guenter Roeck wrote: >>> On 12/12/24 13:49, Ninad Palsule wrote: >>>> Adding label files for fan and temperature sensors in the power >>>> supply. >>>> The openbmc application dbus-sensor(psusensor) requires those files to >>>> consfigure those sensors. >>>> Note that prefix for temp label is temp[A..C] used instead of >>>> temp[1..3] >>>> as dbus-sensor(psusensor) application calculate index based on last >>>> digit in the name so we do not want to make index double digit after >>>> appending page index. >>>> >>>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com> >>> >>> We are not going to fix userspace problems in the kernel. >>> >>> Guenter >>> >> >> Thanks for the quick review. >> >> Sorry I am not clear on this. I feel that it is better to support >> labels for temperature >> >> sensors and fans like other. Are you saying we should not support >> these labels or >> >> I need update in the patch to support them better? >> > > There should be no such labels. Labels are supposed to have specific > meanings, > such as "this is the CPU temperature sensor", not vague meanings such > as "tempA". > > Guenter Thanks for the quick response. I will remove these changes for now and will talk to you later about better option. Thanks & Regards, Ninad Palsule
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c index a7000314e5ad..149b3c5f3a4c 100644 --- a/drivers/hwmon/pmbus/pmbus_core.c +++ b/drivers/hwmon/pmbus/pmbus_core.c @@ -2144,6 +2144,7 @@ static const struct pmbus_sensor_attr temp_attributes[] = { { .reg = PMBUS_READ_TEMPERATURE_1, .class = PSC_TEMPERATURE, + .label = "tempA", .paged = true, .update = true, .compare = true, @@ -2156,6 +2157,7 @@ static const struct pmbus_sensor_attr temp_attributes[] = { }, { .reg = PMBUS_READ_TEMPERATURE_2, .class = PSC_TEMPERATURE, + .label = "tempB", .paged = true, .update = true, .compare = true, @@ -2168,6 +2170,7 @@ static const struct pmbus_sensor_attr temp_attributes[] = { }, { .reg = PMBUS_READ_TEMPERATURE_3, .class = PSC_TEMPERATURE, + .label = "tempC", .paged = true, .update = true, .compare = true, @@ -2282,6 +2285,17 @@ static int pmbus_add_fan_attributes(struct i2c_client *client, PSC_FAN, true, true, true) == NULL) return -ENOMEM; + /* + * Add fan label. + * Assuming paged attributes while passing page index + */ + ret = pmbus_add_label(data, "fan", index, "fan", + page + 1, 0xff); + if (ret) { + dev_err(data->dev, "Fan label add failed ret=%d\n", ret); + return ret; + } + /* Fan control */ if (pmbus_check_word_register(client, page, pmbus_fan_command_registers[f])) {
Adding label files for fan and temperature sensors in the power supply. The openbmc application dbus-sensor(psusensor) requires those files to consfigure those sensors. Note that prefix for temp label is temp[A..C] used instead of temp[1..3] as dbus-sensor(psusensor) application calculate index based on last digit in the name so we do not want to make index double digit after appending page index. Signed-off-by: Ninad Palsule <ninad@linux.ibm.com> --- drivers/hwmon/pmbus/pmbus_core.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)