| Message ID | 20220830192212.28570-12-farbere@amazon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Variety of fixes and new features for mr75203 driver | expand |
On Tue, Aug 30, 2022 at 07:22:04PM +0000, Eliav Farber wrote: > Add mr76006 pre-scaler support to normalize the voltage output result for > channels that use pre-scaler units to get the measurement to be within > the range that the sensor supports. > > For channels that are listed in the device-tree to have a pre-scaler, the > voltage result is multiplied by a factor of 2, to represent to the user > the actual voltage source which is measured. ... > +static int pvt_get_pre_scaler(struct device *dev, struct pvt_device *pvt) > +{ > + const struct device_node *np = dev->of_node; > + u32 total_channels = pvt->vm_channels.total; > + u32 channel; > + u8 *pre_scaler_ch_list; > + int i, ret, num_ch; > + > + /* Set default pre-scaler value to be 1. */ > + for (i = 0; i < total_channels; i++) > + pvt->vd[i].pre_scaler = PRE_SCALER_X1; > + > + /* Get number of channels configured in "moortec,vm-pre-scaler". */ > + num_ch = of_property_count_u8_elems(np, "moortec,vm-pre-scaler"); of_ ?! > + if (num_ch <= 0) > + return 0; > + > + pre_scaler_ch_list = kcalloc(total_channels, > + sizeof(*pre_scaler_ch_list), GFP_KERNEL); > + if (!pre_scaler_ch_list) > + return -ENOMEM; > + > + /* Get list of all channels that have pre-scaler of 2. */ > + ret = device_property_read_u8_array(dev, "moortec,vm-pre-scaler", > + pre_scaler_ch_list, num_ch); > + if (ret) > + goto out; > + > + for (i = 0; i < num_ch; i++) { > + channel = pre_scaler_ch_list[i]; > + Unnecessary blank line. > + if (channel >= total_channels) { > + dev_err(dev, > + "invalid channel (%u) in pre-scaler list\n", > + channel); > + ret = -EINVAL; > + goto out; Wouldn't break; suffice? (I understand the point, up to you) > + } > + > + pvt->vd[channel].pre_scaler = PRE_SCALER_X2; > + } > + > +out: out_free: > + kfree(pre_scaler_ch_list); > + > + return ret; > +}
On 8/31/2022 3:02 PM, Andy Shevchenko wrote: > On Tue, Aug 30, 2022 at 07:22:04PM +0000, Eliav Farber wrote: >> +static int pvt_get_pre_scaler(struct device *dev, struct pvt_device >> *pvt) >> +{ >> + const struct device_node *np = dev->of_node; >> + u32 total_channels = pvt->vm_channels.total; >> + u32 channel; >> + u8 *pre_scaler_ch_list; >> + int i, ret, num_ch; >> + >> + /* Set default pre-scaler value to be 1. */ >> + for (i = 0; i < total_channels; i++) >> + pvt->vd[i].pre_scaler = PRE_SCALER_X1; >> + >> + /* Get number of channels configured in >> "moortec,vm-pre-scaler". */ >> + num_ch = of_property_count_u8_elems(np, "moortec,vm-pre-scaler"); > > of_ ?! > Replaced of_property_count_u8_elems() with device_property_count_u8(). >> + if (num_ch <= 0) >> + return 0; >> + >> + pre_scaler_ch_list = kcalloc(total_channels, >> + sizeof(*pre_scaler_ch_list), >> GFP_KERNEL); >> + if (!pre_scaler_ch_list) >> + return -ENOMEM; >> + >> + /* Get list of all channels that have pre-scaler of 2. */ >> + ret = device_property_read_u8_array(dev, "moortec,vm-pre-scaler", >> + pre_scaler_ch_list, num_ch); >> + if (ret) >> + goto out; >> + >> + for (i = 0; i < num_ch; i++) { >> + channel = pre_scaler_ch_list[i]; > >> + > > Unnecessary blank line. Blank line removed. >> + if (channel >= total_channels) { >> + dev_err(dev, >> + "invalid channel (%u) in pre-scaler >> list\n", >> + channel); >> + ret = -EINVAL; > >> + goto out; > > Wouldn't > > break; > > suffice? (I understand the point, up to you) I prefer to exit the moment I detect a problem. For now I can use a break but in the future someone else can add new code in between that will set ret to 0 and instead of failing driver flow will continue with incomplete pre-scaler value. So I prefer keeping it as it. -- Best regards, Eliav
diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c index 6925e8490587..1cd5ff6eacce 100644 --- a/drivers/hwmon/mr75203.c +++ b/drivers/hwmon/mr75203.c @@ -13,10 +13,12 @@ #include <linux/module.h> #include <linux/mod_devicetable.h> #include <linux/mutex.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/property.h> #include <linux/regmap.h> #include <linux/reset.h> +#include <linux/slab.h> /* PVT Common register */ #define PVT_IP_CONFIG 0x04 @@ -108,9 +110,13 @@ #define PVT_N_CONST 90 #define PVT_R_CONST 245805 +#define PRE_SCALER_X1 1 +#define PRE_SCALER_X2 2 + struct voltage_device { u32 vm_map; /* Map channel number to VM index */ u32 ch_map; /* Map channel number to channel index */ + u32 pre_scaler; }; struct voltage_channels { @@ -192,7 +198,7 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val) { struct pvt_device *pvt = dev_get_drvdata(dev); struct regmap *v_map = pvt->v_map; - u32 n, stat; + u32 n, stat, pre_scaler; u8 vm_idx, ch_idx; int ret; @@ -217,7 +223,9 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val) n &= SAMPLE_DATA_MSK; /* Convert the N bitstream count into voltage */ - *val = (PVT_N_CONST * n - PVT_R_CONST) >> PVT_CONV_BITS; + pre_scaler = pvt->vd[channel].pre_scaler; + *val = pre_scaler * (PVT_N_CONST * n - PVT_R_CONST) >> + PVT_CONV_BITS; return 0; default: @@ -582,6 +590,54 @@ static int pvt_get_active_channel(struct device *dev, struct pvt_device *pvt, return 0; } +static int pvt_get_pre_scaler(struct device *dev, struct pvt_device *pvt) +{ + const struct device_node *np = dev->of_node; + u32 total_channels = pvt->vm_channels.total; + u32 channel; + u8 *pre_scaler_ch_list; + int i, ret, num_ch; + + /* Set default pre-scaler value to be 1. */ + for (i = 0; i < total_channels; i++) + pvt->vd[i].pre_scaler = PRE_SCALER_X1; + + /* Get number of channels configured in "moortec,vm-pre-scaler". */ + num_ch = of_property_count_u8_elems(np, "moortec,vm-pre-scaler"); + if (num_ch <= 0) + return 0; + + pre_scaler_ch_list = kcalloc(total_channels, + sizeof(*pre_scaler_ch_list), GFP_KERNEL); + if (!pre_scaler_ch_list) + return -ENOMEM; + + /* Get list of all channels that have pre-scaler of 2. */ + ret = device_property_read_u8_array(dev, "moortec,vm-pre-scaler", + pre_scaler_ch_list, num_ch); + if (ret) + goto out; + + for (i = 0; i < num_ch; i++) { + channel = pre_scaler_ch_list[i]; + + if (channel >= total_channels) { + dev_err(dev, + "invalid channel (%u) in pre-scaler list\n", + channel); + ret = -EINVAL; + goto out; + } + + pvt->vd[channel].pre_scaler = PRE_SCALER_X2; + } + +out: + kfree(pre_scaler_ch_list); + + return ret; +} + static int mr75203_probe(struct platform_device *pdev) { const struct hwmon_channel_info **pvt_info; @@ -697,6 +753,10 @@ static int mr75203_probe(struct platform_device *pdev) if (ret) return ret; + ret = pvt_get_pre_scaler(dev, pvt); + if (ret) + return ret; + in_config = devm_kcalloc(dev, pvt->vm_channels.total + 1, sizeof(*in_config), GFP_KERNEL); if (!in_config)
Add mr76006 pre-scaler support to normalize the voltage output result for channels that use pre-scaler units to get the measurement to be within the range that the sensor supports. For channels that are listed in the device-tree to have a pre-scaler, the voltage result is multiplied by a factor of 2, to represent to the user the actual voltage source which is measured. Signed-off-by: Eliav Farber <farbere@amazon.com> --- V3 -> V2: - Modify code according to new property format. drivers/hwmon/mr75203.c | 64 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 2 deletions(-)