| Message ID | 20230825205345.632792-2-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Enhancements for tmp51x driver | expand |
On Fri, Aug 25, 2023 at 09:53:43PM +0100, Biju Das wrote: > The tmp512 chip has 3 channels whereas tmp513 has 4 channels. Avoid > using tmp51x_ids for this HW difference by replacing OF/ID table > data with maximum channels supported by the device. > > Add max_channels variable to struct tmp51x_data and fix the logic for > invalid channel in tmp51x_is_visible(). This patch does not fix anything. The existing logic in tmp51x_is_visible() is perfectly fine. Guenter > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v3: > * New patch split from patch #3 > * Avoided Yoda style logic. > * Replaced OF/ID data from tmp51x_ids->max_channels > --- > drivers/hwmon/tmp513.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/drivers/hwmon/tmp513.c b/drivers/hwmon/tmp513.c > index 9a180b1030c9..99f66f9d5f19 100644 > --- a/drivers/hwmon/tmp513.c > +++ b/drivers/hwmon/tmp513.c > @@ -113,6 +113,9 @@ > > #define MAX_TEMP_HYST 127500 > > +#define TMP512_MAX_CHANNELS 3 > +#define TMP513_MAX_CHANNELS 4 > + > static const u8 TMP51X_TEMP_INPUT[4] = { > TMP51X_LOCAL_TEMP_RESULT, > TMP51X_REMOTE_TEMP_RESULT_1, > @@ -170,6 +173,7 @@ struct tmp51x_data { > u32 pwr_lsb_uw; > > enum tmp51x_ids id; > + u8 max_channels; > struct regmap *regmap; > }; > > @@ -434,7 +438,7 @@ static umode_t tmp51x_is_visible(const void *_data, > > switch (type) { > case hwmon_temp: > - if (data->id == tmp512 && channel == 3) > + if (channel >= data->max_channels) > return 0; > switch (attr) { > case hwmon_temp_input: > @@ -601,21 +605,15 @@ static int tmp51x_init(struct tmp51x_data *data) > } > > static const struct i2c_device_id tmp51x_id[] = { > - { "tmp512", tmp512 }, > - { "tmp513", tmp513 }, > + { "tmp512", TMP512_MAX_CHANNELS }, > + { "tmp513", TMP513_MAX_CHANNELS }, > { } > }; > MODULE_DEVICE_TABLE(i2c, tmp51x_id); > > static const struct of_device_id tmp51x_of_match[] = { > - { > - .compatible = "ti,tmp512", > - .data = (void *)tmp512 > - }, > - { > - .compatible = "ti,tmp513", > - .data = (void *)tmp513 > - }, > + { .compatible = "ti,tmp512", .data = (void *)TMP512_MAX_CHANNELS }, > + { .compatible = "ti,tmp513", .data = (void *)TMP513_MAX_CHANNELS }, > { }, > }; > MODULE_DEVICE_TABLE(of, tmp51x_of_match); > @@ -720,7 +718,11 @@ static int tmp51x_probe(struct i2c_client *client) > if (!data) > return -ENOMEM; > > - data->id = (uintptr_t)i2c_get_match_data(client); > + data->max_channels = (uintptr_t)i2c_get_match_data(client); > + if (data->max_channels == TMP513_MAX_CHANNELS) > + data->id = tmp513; > + else > + data->id = tmp512; > > ret = tmp51x_configure(dev, data); > if (ret < 0) { > -- > 2.25.1 >
Hi Guenter Roeck, Thanks for the feedback. > Subject: Re: [PATCH v3 1/3] hwmon: tmp513: Add max_channels variable to > struct tmp51x_data > > On Fri, Aug 25, 2023 at 09:53:43PM +0100, Biju Das wrote: > > The tmp512 chip has 3 channels whereas tmp513 has 4 channels. Avoid > > using tmp51x_ids for this HW difference by replacing OF/ID table data > > with maximum channels supported by the device. > > > > Add max_channels variable to struct tmp51x_data and fix the logic for > > invalid channel in tmp51x_is_visible(). > > This patch does not fix anything. The existing logic in tmp51x_is_visible() > is perfectly fine. I agree. It is not a fix, but it is an improvement. Earlier logic checks invalid channels for only tmp512, whereas the new logic checks invalid channels for both tmp512 and tmp513. Cheers, Biju > > Guenter > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v3: > > * New patch split from patch #3 > > * Avoided Yoda style logic. > > * Replaced OF/ID data from tmp51x_ids->max_channels > > --- > > drivers/hwmon/tmp513.c | 26 ++++++++++++++------------ > > 1 file changed, 14 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/hwmon/tmp513.c b/drivers/hwmon/tmp513.c index > > 9a180b1030c9..99f66f9d5f19 100644 > > --- a/drivers/hwmon/tmp513.c > > +++ b/drivers/hwmon/tmp513.c > > @@ -113,6 +113,9 @@ > > > > #define MAX_TEMP_HYST 127500 > > > > +#define TMP512_MAX_CHANNELS 3 > > +#define TMP513_MAX_CHANNELS 4 > > + > > static const u8 TMP51X_TEMP_INPUT[4] = { > > TMP51X_LOCAL_TEMP_RESULT, > > TMP51X_REMOTE_TEMP_RESULT_1, > > @@ -170,6 +173,7 @@ struct tmp51x_data { > > u32 pwr_lsb_uw; > > > > enum tmp51x_ids id; > > + u8 max_channels; > > struct regmap *regmap; > > }; > > > > @@ -434,7 +438,7 @@ static umode_t tmp51x_is_visible(const void > > *_data, > > > > switch (type) { > > case hwmon_temp: > > - if (data->id == tmp512 && channel == 3) > > + if (channel >= data->max_channels) > > return 0; > > switch (attr) { > > case hwmon_temp_input: > > @@ -601,21 +605,15 @@ static int tmp51x_init(struct tmp51x_data *data) > > } > > > > static const struct i2c_device_id tmp51x_id[] = { > > - { "tmp512", tmp512 }, > > - { "tmp513", tmp513 }, > > + { "tmp512", TMP512_MAX_CHANNELS }, > > + { "tmp513", TMP513_MAX_CHANNELS }, > > { } > > }; > > MODULE_DEVICE_TABLE(i2c, tmp51x_id); > > > > static const struct of_device_id tmp51x_of_match[] = { > > - { > > - .compatible = "ti,tmp512", > > - .data = (void *)tmp512 > > - }, > > - { > > - .compatible = "ti,tmp513", > > - .data = (void *)tmp513 > > - }, > > + { .compatible = "ti,tmp512", .data = (void *)TMP512_MAX_CHANNELS }, > > + { .compatible = "ti,tmp513", .data = (void *)TMP513_MAX_CHANNELS }, > > { }, > > }; > > MODULE_DEVICE_TABLE(of, tmp51x_of_match); @@ -720,7 +718,11 @@ static > > int tmp51x_probe(struct i2c_client *client) > > if (!data) > > return -ENOMEM; > > > > - data->id = (uintptr_t)i2c_get_match_data(client); > > + data->max_channels = (uintptr_t)i2c_get_match_data(client); > > + if (data->max_channels == TMP513_MAX_CHANNELS) > > + data->id = tmp513; > > + else > > + data->id = tmp512; > > > > ret = tmp51x_configure(dev, data); > > if (ret < 0) { > > -- > > 2.25.1 > >
diff --git a/drivers/hwmon/tmp513.c b/drivers/hwmon/tmp513.c index 9a180b1030c9..99f66f9d5f19 100644 --- a/drivers/hwmon/tmp513.c +++ b/drivers/hwmon/tmp513.c @@ -113,6 +113,9 @@ #define MAX_TEMP_HYST 127500 +#define TMP512_MAX_CHANNELS 3 +#define TMP513_MAX_CHANNELS 4 + static const u8 TMP51X_TEMP_INPUT[4] = { TMP51X_LOCAL_TEMP_RESULT, TMP51X_REMOTE_TEMP_RESULT_1, @@ -170,6 +173,7 @@ struct tmp51x_data { u32 pwr_lsb_uw; enum tmp51x_ids id; + u8 max_channels; struct regmap *regmap; }; @@ -434,7 +438,7 @@ static umode_t tmp51x_is_visible(const void *_data, switch (type) { case hwmon_temp: - if (data->id == tmp512 && channel == 3) + if (channel >= data->max_channels) return 0; switch (attr) { case hwmon_temp_input: @@ -601,21 +605,15 @@ static int tmp51x_init(struct tmp51x_data *data) } static const struct i2c_device_id tmp51x_id[] = { - { "tmp512", tmp512 }, - { "tmp513", tmp513 }, + { "tmp512", TMP512_MAX_CHANNELS }, + { "tmp513", TMP513_MAX_CHANNELS }, { } }; MODULE_DEVICE_TABLE(i2c, tmp51x_id); static const struct of_device_id tmp51x_of_match[] = { - { - .compatible = "ti,tmp512", - .data = (void *)tmp512 - }, - { - .compatible = "ti,tmp513", - .data = (void *)tmp513 - }, + { .compatible = "ti,tmp512", .data = (void *)TMP512_MAX_CHANNELS }, + { .compatible = "ti,tmp513", .data = (void *)TMP513_MAX_CHANNELS }, { }, }; MODULE_DEVICE_TABLE(of, tmp51x_of_match); @@ -720,7 +718,11 @@ static int tmp51x_probe(struct i2c_client *client) if (!data) return -ENOMEM; - data->id = (uintptr_t)i2c_get_match_data(client); + data->max_channels = (uintptr_t)i2c_get_match_data(client); + if (data->max_channels == TMP513_MAX_CHANNELS) + data->id = tmp513; + else + data->id = tmp512; ret = tmp51x_configure(dev, data); if (ret < 0) {
The tmp512 chip has 3 channels whereas tmp513 has 4 channels. Avoid using tmp51x_ids for this HW difference by replacing OF/ID table data with maximum channels supported by the device. Add max_channels variable to struct tmp51x_data and fix the logic for invalid channel in tmp51x_is_visible(). Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v3: * New patch split from patch #3 * Avoided Yoda style logic. * Replaced OF/ID data from tmp51x_ids->max_channels --- drivers/hwmon/tmp513.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)