Message ID | 20220906083356.21067-2-farbere@amazon.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Variety of fixes and new features for mr75203 driver | expand |
On Tue, Sep 06, 2022 at 08:33:36AM +0000, Eliav Farber wrote: > Fix: "ERROR: space required before the open parenthesis '('" > > Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller") Coding style "fixes" do not fix the code. I consider using the Fixes: tag for those to close to an abuse of that tag (and it would be great if that was spelled out somewhere). As it is, I can not with good conscience apply this patch to the mainline kernel (especially not for -rc5), meaning the entire series will have to wait for the next release window unless there are no conflicts. Guenter > Signed-off-by: Eliav Farber <farbere@amazon.com> > --- > drivers/hwmon/mr75203.c | 40 ++++++++++++++++++++-------------------- > 1 file changed, 20 insertions(+), 20 deletions(-) > > diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c > index 046523d47c29..8adfbb15453f 100644 > --- a/drivers/hwmon/mr75203.c > +++ b/drivers/hwmon/mr75203.c > @@ -155,7 +155,7 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val) > return ret; > > ret = regmap_read(t_map, SDIF_DATA(channel), &nbs); > - if(ret < 0) > + if (ret < 0) > return ret; > > nbs &= SAMPLE_DATA_MSK; > @@ -197,7 +197,7 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val) > return ret; > > ret = regmap_read(v_map, VM_SDIF_DATA(vm_idx), &n); > - if(ret < 0) > + if (ret < 0) > return ret; > > n &= SAMPLE_DATA_MSK; > @@ -291,19 +291,19 @@ static int pvt_init(struct pvt_device *pvt) > > if (t_num) { > ret = regmap_write(t_map, SDIF_SMPL_CTRL, 0x0); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_write(t_map, SDIF_HALT, 0x0); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_write(t_map, CLK_SYNTH, clk_synth); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_write(t_map, SDIF_DISABLE, 0x0); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_read_poll_timeout(t_map, SDIF_STAT, > @@ -316,7 +316,7 @@ static int pvt_init(struct pvt_device *pvt) > val = CFG0_MODE_2 | CFG0_PARALLEL_OUT | CFG0_12_BIT | > IP_CFG << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG; > ret = regmap_write(t_map, SDIF_W, val); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_read_poll_timeout(t_map, SDIF_STAT, > @@ -329,7 +329,7 @@ static int pvt_init(struct pvt_device *pvt) > val = POWER_DELAY_CYCLE_256 | IP_TMR << SDIF_ADDR_SFT | > SDIF_WRN_W | SDIF_PROG; > ret = regmap_write(t_map, SDIF_W, val); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_read_poll_timeout(t_map, SDIF_STAT, > @@ -343,39 +343,39 @@ static int pvt_init(struct pvt_device *pvt) > IP_CTRL << SDIF_ADDR_SFT | > SDIF_WRN_W | SDIF_PROG; > ret = regmap_write(t_map, SDIF_W, val); > - if(ret < 0) > + if (ret < 0) > return ret; > } > > if (p_num) { > ret = regmap_write(p_map, SDIF_HALT, 0x0); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_write(p_map, SDIF_DISABLE, BIT(p_num) - 1); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_write(p_map, CLK_SYNTH, clk_synth); > - if(ret < 0) > + if (ret < 0) > return ret; > } > > if (v_num) { > ret = regmap_write(v_map, SDIF_SMPL_CTRL, 0x0); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_write(v_map, SDIF_HALT, 0x0); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_write(v_map, CLK_SYNTH, clk_synth); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_write(v_map, SDIF_DISABLE, 0x0); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_read_poll_timeout(v_map, SDIF_STAT, > @@ -389,7 +389,7 @@ static int pvt_init(struct pvt_device *pvt) > CFG1_14_BIT | IP_CFG << SDIF_ADDR_SFT | > SDIF_WRN_W | SDIF_PROG; > ret = regmap_write(v_map, SDIF_W, val); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_read_poll_timeout(v_map, SDIF_STAT, > @@ -402,7 +402,7 @@ static int pvt_init(struct pvt_device *pvt) > val = POWER_DELAY_CYCLE_64 | IP_TMR << SDIF_ADDR_SFT | > SDIF_WRN_W | SDIF_PROG; > ret = regmap_write(v_map, SDIF_W, val); > - if(ret < 0) > + if (ret < 0) > return ret; > > ret = regmap_read_poll_timeout(v_map, SDIF_STAT, > @@ -416,7 +416,7 @@ static int pvt_init(struct pvt_device *pvt) > IP_CTRL << SDIF_ADDR_SFT | > SDIF_WRN_W | SDIF_PROG; > ret = regmap_write(v_map, SDIF_W, val); > - if(ret < 0) > + if (ret < 0) > return ret; > } > > @@ -535,7 +535,7 @@ static int mr75203_probe(struct platform_device *pdev) > return dev_err_probe(dev, ret, "cannot deassert reset control\n"); > > ret = regmap_read(pvt->c_map, PVT_IP_CONFIG, &val); > - if(ret < 0) > + if (ret < 0) > return ret; > > ts_num = (val & TS_NUM_MSK) >> TS_NUM_SFT;
On 9/6/2022 7:52 PM, Guenter Roeck wrote: > On Tue, Sep 06, 2022 at 08:33:36AM +0000, Eliav Farber wrote: >> Fix: "ERROR: space required before the open parenthesis '('" >> >> Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for >> Moortec MR75203 PVT controller") > > Coding style "fixes" do not fix the code. I consider using the Fixes: tag > for those to close to an abuse of that tag (and it would be great if that > was spelled out somewhere). As it is, I can not with good conscience > apply > this patch to the mainline kernel (especially not for -rc5), meaning the > entire series will have to wait for the next release window unless there > are no conflicts. Because as you mentioned it is not a functional fix in the code I’ll remove the Fixes tag in v5. I checked older kernel versions and patch applies without conflicts. That’s also why I moved it to be first in the series, so it will be before any of my other changes that night cause merge problems for other branches. -- Thanks, Eliav
On Wed, Sep 07, 2022 at 09:50:00AM +0300, Farber, Eliav wrote: > On 9/6/2022 7:52 PM, Guenter Roeck wrote: > > On Tue, Sep 06, 2022 at 08:33:36AM +0000, Eliav Farber wrote: > > > Fix: "ERROR: space required before the open parenthesis '('" > > > > > > Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for > > > Moortec MR75203 PVT controller") > > > > Coding style "fixes" do not fix the code. I consider using the Fixes: tag > > for those to close to an abuse of that tag (and it would be great if that > > was spelled out somewhere). As it is, I can not with good conscience > > apply > > this patch to the mainline kernel (especially not for -rc5), meaning the > > entire series will have to wait for the next release window unless there > > are no conflicts. > Because as you mentioned it is not a functional fix in the code I’ll > remove the Fixes tag in v5. > I checked older kernel versions and patch applies without conflicts. > That’s also why I moved it to be first in the series, so it will be > before any of my other changes that night cause merge problems for > other branches. If it's not a fix, it should go _after_ real fixes. And by nature this kind of patch is usually at the end of the series.
On 9/7/22 07:42, Andy Shevchenko wrote: > On Wed, Sep 07, 2022 at 09:50:00AM +0300, Farber, Eliav wrote: >> On 9/6/2022 7:52 PM, Guenter Roeck wrote: >>> On Tue, Sep 06, 2022 at 08:33:36AM +0000, Eliav Farber wrote: >>>> Fix: "ERROR: space required before the open parenthesis '('" >>>> >>>> Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for >>>> Moortec MR75203 PVT controller") >>> >>> Coding style "fixes" do not fix the code. I consider using the Fixes: tag >>> for those to close to an abuse of that tag (and it would be great if that >>> was spelled out somewhere). As it is, I can not with good conscience >>> apply >>> this patch to the mainline kernel (especially not for -rc5), meaning the >>> entire series will have to wait for the next release window unless there >>> are no conflicts. >> Because as you mentioned it is not a functional fix in the code I’ll >> remove the Fixes tag in v5. >> I checked older kernel versions and patch applies without conflicts. >> That’s also why I moved it to be first in the series, so it will be >> before any of my other changes that night cause merge problems for >> other branches. > > If it's not a fix, it should go _after_ real fixes. > And by nature this kind of patch is usually at the end of the series. > I don't care where it is, really, as long as it is after the real fixes. And obviously I would not want to see a patch fixing something introduced in a previous patch of the same series. Thanks, Guenter
diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c index 046523d47c29..8adfbb15453f 100644 --- a/drivers/hwmon/mr75203.c +++ b/drivers/hwmon/mr75203.c @@ -155,7 +155,7 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val) return ret; ret = regmap_read(t_map, SDIF_DATA(channel), &nbs); - if(ret < 0) + if (ret < 0) return ret; nbs &= SAMPLE_DATA_MSK; @@ -197,7 +197,7 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val) return ret; ret = regmap_read(v_map, VM_SDIF_DATA(vm_idx), &n); - if(ret < 0) + if (ret < 0) return ret; n &= SAMPLE_DATA_MSK; @@ -291,19 +291,19 @@ static int pvt_init(struct pvt_device *pvt) if (t_num) { ret = regmap_write(t_map, SDIF_SMPL_CTRL, 0x0); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_write(t_map, SDIF_HALT, 0x0); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_write(t_map, CLK_SYNTH, clk_synth); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_write(t_map, SDIF_DISABLE, 0x0); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_read_poll_timeout(t_map, SDIF_STAT, @@ -316,7 +316,7 @@ static int pvt_init(struct pvt_device *pvt) val = CFG0_MODE_2 | CFG0_PARALLEL_OUT | CFG0_12_BIT | IP_CFG << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG; ret = regmap_write(t_map, SDIF_W, val); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_read_poll_timeout(t_map, SDIF_STAT, @@ -329,7 +329,7 @@ static int pvt_init(struct pvt_device *pvt) val = POWER_DELAY_CYCLE_256 | IP_TMR << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG; ret = regmap_write(t_map, SDIF_W, val); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_read_poll_timeout(t_map, SDIF_STAT, @@ -343,39 +343,39 @@ static int pvt_init(struct pvt_device *pvt) IP_CTRL << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG; ret = regmap_write(t_map, SDIF_W, val); - if(ret < 0) + if (ret < 0) return ret; } if (p_num) { ret = regmap_write(p_map, SDIF_HALT, 0x0); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_write(p_map, SDIF_DISABLE, BIT(p_num) - 1); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_write(p_map, CLK_SYNTH, clk_synth); - if(ret < 0) + if (ret < 0) return ret; } if (v_num) { ret = regmap_write(v_map, SDIF_SMPL_CTRL, 0x0); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_write(v_map, SDIF_HALT, 0x0); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_write(v_map, CLK_SYNTH, clk_synth); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_write(v_map, SDIF_DISABLE, 0x0); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_read_poll_timeout(v_map, SDIF_STAT, @@ -389,7 +389,7 @@ static int pvt_init(struct pvt_device *pvt) CFG1_14_BIT | IP_CFG << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG; ret = regmap_write(v_map, SDIF_W, val); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_read_poll_timeout(v_map, SDIF_STAT, @@ -402,7 +402,7 @@ static int pvt_init(struct pvt_device *pvt) val = POWER_DELAY_CYCLE_64 | IP_TMR << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG; ret = regmap_write(v_map, SDIF_W, val); - if(ret < 0) + if (ret < 0) return ret; ret = regmap_read_poll_timeout(v_map, SDIF_STAT, @@ -416,7 +416,7 @@ static int pvt_init(struct pvt_device *pvt) IP_CTRL << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG; ret = regmap_write(v_map, SDIF_W, val); - if(ret < 0) + if (ret < 0) return ret; } @@ -535,7 +535,7 @@ static int mr75203_probe(struct platform_device *pdev) return dev_err_probe(dev, ret, "cannot deassert reset control\n"); ret = regmap_read(pvt->c_map, PVT_IP_CONFIG, &val); - if(ret < 0) + if (ret < 0) return ret; ts_num = (val & TS_NUM_MSK) >> TS_NUM_SFT;
Fix: "ERROR: space required before the open parenthesis '('" Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller") Signed-off-by: Eliav Farber <farbere@amazon.com> --- drivers/hwmon/mr75203.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-)