| Message ID | b2aa14df2092e86191986cd558b48ee6536aa47d.1632984254.git.krzysztof.adamski@nokia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add per channel properies support in tmp421 | expand |
On Thu, Sep 30, 2021 at 08:59:36AM +0200, Krzysztof Adamski wrote: > tmp42x is a multichannel temperature sensor with several external > channels. Since those channels can be used to connect diodes placed > anywhere in the system, their meaning will vary depending on the > project. For this case, the hwmon framework has an idea of labels which > allows us to assign the meaning to each channel. > > The similar concept is already implemented in ina3221 - the label for > each channel can be defined via device tree. See commit a9e9dd9c6de5 > ("hwmon: (ina3221) Read channel input source info from DT") > > This patch adds support for similar feature to tmp421. > > Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com> > --- > drivers/hwmon/tmp421.c | 61 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c > index f3d59ea347e3..7feef60a592c 100644 > --- a/drivers/hwmon/tmp421.c > +++ b/drivers/hwmon/tmp421.c > @@ -88,6 +88,7 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = { > MODULE_DEVICE_TABLE(of, tmp421_of_match); > > struct tmp421_channel { > + const char *label; > s16 temp; > }; > > @@ -187,6 +188,16 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type, > > } > > +static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, const char **str) > +{ > + struct tmp421_data *data = dev_get_drvdata(dev); > + > + *str = data->channel[channel].label; > + > + return 0; > +} > + > static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type, > u32 attr, int channel) > { > @@ -194,6 +205,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type, > case hwmon_temp_fault: > case hwmon_temp_input: > return 0444; > + case hwmon_temp_label: > + return 0444; > default: > return 0; > } > @@ -286,9 +299,53 @@ static int tmp421_detect(struct i2c_client *client, > return 0; > } > > +static int tmp421_probe_child_from_dt(struct i2c_client *client, > + struct device_node *child, > + struct tmp421_data *data) > + > +{ > + struct device *dev = &client->dev; > + u32 i; > + int err; > + > + err = of_property_read_u32(child, "reg", &i); > + if (err) { > + dev_err(dev, "missing reg property of %pOFn\n", child); > + return err; > + } > + > + if (i > MAX_CHANNELS) { I think this needs to be >=. MAX_CHANNELS is 4, the array size is 4, and i == 4 will write after the end of the array. > + dev_err(dev, "invalid reg %d of %pOFn\n", i, child); > + return -EINVAL; > + } > + > + of_property_read_string(child, "label", &data->channel[i].label); > + if (data->channel[i].label) > + data->temp_config[i] |= HWMON_T_LABEL; > + > + return 0; > +} > + > +static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *data) > +{ > + struct device *dev = &client->dev; > + const struct device_node *np = dev->of_node; > + struct device_node *child; > + int err; > + > + for_each_child_of_node(np, child) { > + err = tmp421_probe_child_from_dt(client, child, data); > + if (err) > + return err; > + } > + > + return 0; > +} > + > static const struct hwmon_ops tmp421_ops = { > .is_visible = tmp421_is_visible, > .read = tmp421_read, > + .read_string = tmp421_read_string, > }; > > static int tmp421_probe(struct i2c_client *client) > @@ -317,6 +374,10 @@ static int tmp421_probe(struct i2c_client *client) > for (i = 0; i < data->channels; i++) > data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT; > > + err = tmp421_probe_from_dt(client, data); > + if (err) > + return err; > + > data->chip.ops = &tmp421_ops; > data->chip.info = data->info; > > -- > 2.31.1 >
Dnia Sat, Oct 02, 2021 at 07:08:39AM -0700, Guenter Roeck napisał(a): >> @@ -286,9 +299,53 @@ static int tmp421_detect(struct i2c_client *client, >> return 0; >> } >> >> +static int tmp421_probe_child_from_dt(struct i2c_client *client, >> + struct device_node *child, >> + struct tmp421_data *data) >> + >> +{ >> + struct device *dev = &client->dev; >> + u32 i; >> + int err; >> + >> + err = of_property_read_u32(child, "reg", &i); >> + if (err) { >> + dev_err(dev, "missing reg property of %pOFn\n", child); >> + return err; >> + } >> + >> + if (i > MAX_CHANNELS) { > >I think this needs to be >=. MAX_CHANNELS is 4, the array size >is 4, and i == 4 will write after the end of the array. You are absoluetly right here. Off by one error. I will fix this in v4. Krzysztof
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c index f3d59ea347e3..7feef60a592c 100644 --- a/drivers/hwmon/tmp421.c +++ b/drivers/hwmon/tmp421.c @@ -88,6 +88,7 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = { MODULE_DEVICE_TABLE(of, tmp421_of_match); struct tmp421_channel { + const char *label; s16 temp; }; @@ -187,6 +188,16 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type, } +static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, const char **str) +{ + struct tmp421_data *data = dev_get_drvdata(dev); + + *str = data->channel[channel].label; + + return 0; +} + static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr, int channel) { @@ -194,6 +205,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type, case hwmon_temp_fault: case hwmon_temp_input: return 0444; + case hwmon_temp_label: + return 0444; default: return 0; } @@ -286,9 +299,53 @@ static int tmp421_detect(struct i2c_client *client, return 0; } +static int tmp421_probe_child_from_dt(struct i2c_client *client, + struct device_node *child, + struct tmp421_data *data) + +{ + struct device *dev = &client->dev; + u32 i; + int err; + + err = of_property_read_u32(child, "reg", &i); + if (err) { + dev_err(dev, "missing reg property of %pOFn\n", child); + return err; + } + + if (i > MAX_CHANNELS) { + dev_err(dev, "invalid reg %d of %pOFn\n", i, child); + return -EINVAL; + } + + of_property_read_string(child, "label", &data->channel[i].label); + if (data->channel[i].label) + data->temp_config[i] |= HWMON_T_LABEL; + + return 0; +} + +static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *data) +{ + struct device *dev = &client->dev; + const struct device_node *np = dev->of_node; + struct device_node *child; + int err; + + for_each_child_of_node(np, child) { + err = tmp421_probe_child_from_dt(client, child, data); + if (err) + return err; + } + + return 0; +} + static const struct hwmon_ops tmp421_ops = { .is_visible = tmp421_is_visible, .read = tmp421_read, + .read_string = tmp421_read_string, }; static int tmp421_probe(struct i2c_client *client) @@ -317,6 +374,10 @@ static int tmp421_probe(struct i2c_client *client) for (i = 0; i < data->channels; i++) data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT; + err = tmp421_probe_from_dt(client, data); + if (err) + return err; + data->chip.ops = &tmp421_ops; data->chip.info = data->info;
tmp42x is a multichannel temperature sensor with several external channels. Since those channels can be used to connect diodes placed anywhere in the system, their meaning will vary depending on the project. For this case, the hwmon framework has an idea of labels which allows us to assign the meaning to each channel. The similar concept is already implemented in ina3221 - the label for each channel can be defined via device tree. See commit a9e9dd9c6de5 ("hwmon: (ina3221) Read channel input source info from DT") This patch adds support for similar feature to tmp421. Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com> --- drivers/hwmon/tmp421.c | 61 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)