* [PATCH] cleanup ti-soc-thermal @ 2015-01-03 11:19 Pavel Machek 2015-01-03 17:48 ` Nishanth Menon 2015-01-18 20:17 ` [PATCHv2] " Pavel Machek 0 siblings, 2 replies; 7+ messages in thread From: Pavel Machek @ 2015-01-03 11:19 UTC (permalink / raw) To: edubezval, rui.zhang, linux-kernel, linux-pm Simplify code by removing goto's where they point to simple return. Avoid confusing |= on error values. Correct whitespace. Signed-off-by: Pavel Machek <pavel@ucw.cz> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c index 634b6ce..3b4e72f 100644 --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c @@ -263,18 +264,13 @@ static int ti_bandgap_adc_to_mcelsius(struct ti_bandgap *bgp, int adc_val, int *t) { const struct ti_bandgap_data *conf = bgp->conf; - int ret = 0; /* look up for temperature in the table and return the temperature */ - if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val) { - ret = -ERANGE; - goto exit; - } + if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val) + return -ERANGE; *t = bgp->conf->conv_table[adc_val - conf->adc_start_val]; - -exit: - return ret; + return 0; } /** @@ -295,16 +291,14 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc) { const struct ti_bandgap_data *conf = bgp->conf; const int *conv_table = bgp->conf->conv_table; - int high, low, mid, ret = 0; + int high, low, mid; low = 0; high = conf->adc_end_val - conf->adc_start_val; mid = (high + low) / 2; - if (temp < conv_table[low] || temp > conv_table[high]) { - ret = -ERANGE; - goto exit; - } + if (temp < conv_table[low] || temp > conv_table[high]) + return -ERANGE; while (low < high) { if (temp < conv_table[mid]) @@ -315,9 +309,7 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc) } *adc = conf->adc_start_val + low; - -exit: - return ret; + return 0; } /** @@ -343,13 +335,11 @@ int ti_bandgap_add_hyst(struct ti_bandgap *bgp, int adc_val, int hyst_val, */ ret = ti_bandgap_adc_to_mcelsius(bgp, adc_val, &temp); if (ret < 0) - goto exit; + return ret; temp += hyst_val; ret = ti_bandgap_mcelsius_to_adc(bgp, temp, sum); - -exit: return ret; } @@ -468,22 +458,18 @@ exit: */ static inline int ti_bandgap_validate(struct ti_bandgap *bgp, int id) { - int ret = 0; - if (!bgp || IS_ERR(bgp)) { pr_err("%s: invalid bandgap pointer\n", __func__); - ret = -EINVAL; - goto exit; + return -EINVAL; } if ((id < 0) || (id >= bgp->conf->sensor_count)) { dev_err(bgp->dev, "%s: sensor id out of range (%d)\n", __func__, id); - ret = -ERANGE; + return -ERANGE; } -exit: - return ret; + return 0; } /** @@ -511,12 +497,10 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val, ret = ti_bandgap_validate(bgp, id); if (ret) - goto exit; + return ret; - if (!TI_BANDGAP_HAS(bgp, TALERT)) { - ret = -ENOTSUPP; - goto exit; - } + if (!TI_BANDGAP_HAS(bgp, TALERT)) + return -ENOTSUPP; ts_data = bgp->conf->sensors[id].ts_data; tsr = bgp->conf->sensors[id].registers; @@ -529,17 +513,15 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val, } if (ret) - goto exit; + return ret; ret = ti_bandgap_mcelsius_to_adc(bgp, val, &adc_val); if (ret < 0) - goto exit; + return ret; spin_lock(&bgp->lock); ret = ti_bandgap_update_alert_threshold(bgp, id, adc_val, hot); spin_unlock(&bgp->lock); - -exit: return ret; } @@ -582,7 +564,7 @@ static int _ti_bandgap_read_threshold(struct ti_bandgap *bgp, int id, temp = ti_bandgap_readl(bgp, tsr->bgap_threshold); temp = (temp & mask) >> __ffs(mask); - ret |= ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); + ret = ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); if (ret) { dev_err(bgp->dev, "failed to read thot\n"); ret = -EIO; @@ -1220,11 +1222,10 @@ int ti_bandgap_probe(struct platform_device *pdev) goto free_irqs; } - bgp->div_clk = clk_get(NULL, bgp->conf->div_ck_name); + bgp->div_clk = clk_get(NULL, bgp->conf->div_ck_name); ret = IS_ERR(bgp->div_clk); if (ret) { - dev_err(&pdev->dev, - "failed to request div_ts_ck clock ref\n"); + dev_err(&pdev->dev, "failed to request div_ts_ck clock ref\n"); ret = PTR_ERR(bgp->div_clk); goto free_irqs; } diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c index 5fd0386..c62a6de 100644 --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c @@ -76,7 +76,7 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c) } /* thermal zone ops */ -/* Get temperature callback function for thermal zone*/ +/* Get temperature callback function for thermal zone */ static inline int __ti_thermal_get_temp(void *devdata, long *temp) { struct thermal_zone_device *pcb_tz = NULL; -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] cleanup ti-soc-thermal 2015-01-03 11:19 [PATCH] cleanup ti-soc-thermal Pavel Machek @ 2015-01-03 17:48 ` Nishanth Menon 2015-01-03 18:51 ` Pavel Machek 2015-01-03 18:53 ` Pavel Machek 2015-01-18 20:17 ` [PATCHv2] " Pavel Machek 1 sibling, 2 replies; 7+ messages in thread From: Nishanth Menon @ 2015-01-03 17:48 UTC (permalink / raw) To: Pavel Machek; +Cc: edubezval, rui.zhang, linux-kernel, linux-pm On 12:19-20150103, Pavel Machek wrote: > Simplify code by removing goto's where they point to simple return. > > Avoid confusing |= on error values. Please split these up into chunks of related changes. there is white space and quiet a few other changes all mixed in. Also please cc linux-omap mailing list as well to get adequate audience. > > Correct whitespace. > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > index 634b6ce..3b4e72f 100644 > --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c > +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > @@ -263,18 +264,13 @@ static > int ti_bandgap_adc_to_mcelsius(struct ti_bandgap *bgp, int adc_val, int *t) > { > const struct ti_bandgap_data *conf = bgp->conf; > - int ret = 0; > > /* look up for temperature in the table and return the temperature */ > - if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val) { > - ret = -ERANGE; > - goto exit; > - } > + if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val) > + return -ERANGE; > > *t = bgp->conf->conv_table[adc_val - conf->adc_start_val]; > - > -exit: > - return ret; > + return 0; > } > > /** > @@ -295,16 +291,14 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc) > { > const struct ti_bandgap_data *conf = bgp->conf; > const int *conv_table = bgp->conf->conv_table; > - int high, low, mid, ret = 0; > + int high, low, mid; > > low = 0; > high = conf->adc_end_val - conf->adc_start_val; > mid = (high + low) / 2; > > - if (temp < conv_table[low] || temp > conv_table[high]) { > - ret = -ERANGE; > - goto exit; > - } > + if (temp < conv_table[low] || temp > conv_table[high]) > + return -ERANGE; > > while (low < high) { > if (temp < conv_table[mid]) > @@ -315,9 +309,7 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc) > } > > *adc = conf->adc_start_val + low; > - > -exit: > - return ret; > + return 0; > } > > /** > @@ -343,13 +335,11 @@ int ti_bandgap_add_hyst(struct ti_bandgap *bgp, int adc_val, int hyst_val, > */ > ret = ti_bandgap_adc_to_mcelsius(bgp, adc_val, &temp); > if (ret < 0) > - goto exit; > + return ret; > > temp += hyst_val; > > ret = ti_bandgap_mcelsius_to_adc(bgp, temp, sum); > - > -exit: > return ret; > } > > @@ -468,22 +458,18 @@ exit: > */ > static inline int ti_bandgap_validate(struct ti_bandgap *bgp, int id) > { > - int ret = 0; > - > if (!bgp || IS_ERR(bgp)) { > pr_err("%s: invalid bandgap pointer\n", __func__); > - ret = -EINVAL; > - goto exit; > + return -EINVAL; > } > > if ((id < 0) || (id >= bgp->conf->sensor_count)) { > dev_err(bgp->dev, "%s: sensor id out of range (%d)\n", > __func__, id); > - ret = -ERANGE; > + return -ERANGE; > } > > -exit: > - return ret; > + return 0; > } > > /** > @@ -511,12 +497,10 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val, > > ret = ti_bandgap_validate(bgp, id); > if (ret) > - goto exit; > + return ret; > > - if (!TI_BANDGAP_HAS(bgp, TALERT)) { > - ret = -ENOTSUPP; > - goto exit; > - } > + if (!TI_BANDGAP_HAS(bgp, TALERT)) > + return -ENOTSUPP; > > ts_data = bgp->conf->sensors[id].ts_data; > tsr = bgp->conf->sensors[id].registers; > @@ -529,17 +513,15 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val, > } > > if (ret) > - goto exit; > + return ret; > > ret = ti_bandgap_mcelsius_to_adc(bgp, val, &adc_val); > if (ret < 0) > - goto exit; > + return ret; > > spin_lock(&bgp->lock); > ret = ti_bandgap_update_alert_threshold(bgp, id, adc_val, hot); > spin_unlock(&bgp->lock); > - > -exit: > return ret; > } > > @@ -582,7 +564,7 @@ static int _ti_bandgap_read_threshold(struct ti_bandgap *bgp, int id, > > temp = ti_bandgap_readl(bgp, tsr->bgap_threshold); > temp = (temp & mask) >> __ffs(mask); > - ret |= ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); > + ret = ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); > if (ret) { > dev_err(bgp->dev, "failed to read thot\n"); > ret = -EIO; > @@ -1220,11 +1222,10 @@ int ti_bandgap_probe(struct platform_device *pdev) > goto free_irqs; > } > > - bgp->div_clk = clk_get(NULL, bgp->conf->div_ck_name); > + bgp->div_clk = clk_get(NULL, bgp->conf->div_ck_name); > ret = IS_ERR(bgp->div_clk); > if (ret) { > - dev_err(&pdev->dev, > - "failed to request div_ts_ck clock ref\n"); > + dev_err(&pdev->dev, "failed to request div_ts_ck clock ref\n"); > ret = PTR_ERR(bgp->div_clk); > goto free_irqs; > } > diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > index 5fd0386..c62a6de 100644 > --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > @@ -76,7 +76,7 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c) > } > > /* thermal zone ops */ > -/* Get temperature callback function for thermal zone*/ > +/* Get temperature callback function for thermal zone */ > static inline int __ti_thermal_get_temp(void *devdata, long *temp) > { > struct thermal_zone_device *pcb_tz = NULL; > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cleanup ti-soc-thermal 2015-01-03 17:48 ` Nishanth Menon @ 2015-01-03 18:51 ` Pavel Machek 2015-01-03 18:57 ` Nishanth Menon 2015-01-03 18:53 ` Pavel Machek 1 sibling, 1 reply; 7+ messages in thread From: Pavel Machek @ 2015-01-03 18:51 UTC (permalink / raw) To: Nishanth Menon, linux-omap; +Cc: edubezval, rui.zhang, linux-kernel, linux-pm On Sat 2015-01-03 11:48:04, Nishanth Menon wrote: > On 12:19-20150103, Pavel Machek wrote: > > Simplify code by removing goto's where they point to simple return. > > > > Avoid confusing |= on error values. > > Please split these up into chunks of related changes. there is white > space and quiet a few other changes all mixed in. Do you believe that the patch is too big to be understood? I don't think it is. It is four screens, and it is pretty obvious it does not change any code. If Eduardo asks me, I may do it, but it will waste my time, time of everyone else on the list, and will not really improve anything. [Patch left below, so others can see we are talking 4 screens of code.] > Also please cc linux-omap mailing list as well to get adequate > audience. Can do. Pavel > > > > Correct whitespace. > > > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > > > diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > > index 634b6ce..3b4e72f 100644 > > --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c > > +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c > > @@ -263,18 +264,13 @@ static > > int ti_bandgap_adc_to_mcelsius(struct ti_bandgap *bgp, int adc_val, int *t) > > { > > const struct ti_bandgap_data *conf = bgp->conf; > > - int ret = 0; > > > > /* look up for temperature in the table and return the temperature */ > > - if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val) { > > - ret = -ERANGE; > > - goto exit; > > - } > > + if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val) > > + return -ERANGE; > > > > *t = bgp->conf->conv_table[adc_val - conf->adc_start_val]; > > - > > -exit: > > - return ret; > > + return 0; > > } > > > > /** > > @@ -295,16 +291,14 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc) > > { > > const struct ti_bandgap_data *conf = bgp->conf; > > const int *conv_table = bgp->conf->conv_table; > > - int high, low, mid, ret = 0; > > + int high, low, mid; > > > > low = 0; > > high = conf->adc_end_val - conf->adc_start_val; > > mid = (high + low) / 2; > > > > - if (temp < conv_table[low] || temp > conv_table[high]) { > > - ret = -ERANGE; > > - goto exit; > > - } > > + if (temp < conv_table[low] || temp > conv_table[high]) > > + return -ERANGE; > > > > while (low < high) { > > if (temp < conv_table[mid]) > > @@ -315,9 +309,7 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc) > > } > > > > *adc = conf->adc_start_val + low; > > - > > -exit: > > - return ret; > > + return 0; > > } > > > > /** > > @@ -343,13 +335,11 @@ int ti_bandgap_add_hyst(struct ti_bandgap *bgp, int adc_val, int hyst_val, > > */ > > ret = ti_bandgap_adc_to_mcelsius(bgp, adc_val, &temp); > > if (ret < 0) > > - goto exit; > > + return ret; > > > > temp += hyst_val; > > > > ret = ti_bandgap_mcelsius_to_adc(bgp, temp, sum); > > - > > -exit: > > return ret; > > } > > > > @@ -468,22 +458,18 @@ exit: > > */ > > static inline int ti_bandgap_validate(struct ti_bandgap *bgp, int id) > > { > > - int ret = 0; > > - > > if (!bgp || IS_ERR(bgp)) { > > pr_err("%s: invalid bandgap pointer\n", __func__); > > - ret = -EINVAL; > > - goto exit; > > + return -EINVAL; > > } > > > > if ((id < 0) || (id >= bgp->conf->sensor_count)) { > > dev_err(bgp->dev, "%s: sensor id out of range (%d)\n", > > __func__, id); > > - ret = -ERANGE; > > + return -ERANGE; > > } > > > > -exit: > > - return ret; > > + return 0; > > } > > > > /** > > @@ -511,12 +497,10 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val, > > > > ret = ti_bandgap_validate(bgp, id); > > if (ret) > > - goto exit; > > + return ret; > > > > - if (!TI_BANDGAP_HAS(bgp, TALERT)) { > > - ret = -ENOTSUPP; > > - goto exit; > > - } > > + if (!TI_BANDGAP_HAS(bgp, TALERT)) > > + return -ENOTSUPP; > > > > ts_data = bgp->conf->sensors[id].ts_data; > > tsr = bgp->conf->sensors[id].registers; > > @@ -529,17 +513,15 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val, > > } > > > > if (ret) > > - goto exit; > > + return ret; > > > > ret = ti_bandgap_mcelsius_to_adc(bgp, val, &adc_val); > > if (ret < 0) > > - goto exit; > > + return ret; > > > > spin_lock(&bgp->lock); > > ret = ti_bandgap_update_alert_threshold(bgp, id, adc_val, hot); > > spin_unlock(&bgp->lock); > > - > > -exit: > > return ret; > > } > > > > @@ -582,7 +564,7 @@ static int _ti_bandgap_read_threshold(struct ti_bandgap *bgp, int id, > > > > temp = ti_bandgap_readl(bgp, tsr->bgap_threshold); > > temp = (temp & mask) >> __ffs(mask); > > - ret |= ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); > > + ret = ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); > > if (ret) { > > dev_err(bgp->dev, "failed to read thot\n"); > > ret = -EIO; > > @@ -1220,11 +1222,10 @@ int ti_bandgap_probe(struct platform_device *pdev) > > goto free_irqs; > > } > > > > - bgp->div_clk = clk_get(NULL, bgp->conf->div_ck_name); > > + bgp->div_clk = clk_get(NULL, bgp->conf->div_ck_name); > > ret = IS_ERR(bgp->div_clk); > > if (ret) { > > - dev_err(&pdev->dev, > > - "failed to request div_ts_ck clock ref\n"); > > + dev_err(&pdev->dev, "failed to request div_ts_ck clock ref\n"); > > ret = PTR_ERR(bgp->div_clk); > > goto free_irqs; > > } > > diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > index 5fd0386..c62a6de 100644 > > --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c > > @@ -76,7 +76,7 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c) > > } > > > > /* thermal zone ops */ > > -/* Get temperature callback function for thermal zone*/ > > +/* Get temperature callback function for thermal zone */ > > static inline int __ti_thermal_get_temp(void *devdata, long *temp) > > { > > struct thermal_zone_device *pcb_tz = NULL; -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cleanup ti-soc-thermal 2015-01-03 18:51 ` Pavel Machek @ 2015-01-03 18:57 ` Nishanth Menon 2015-01-03 19:02 ` Pavel Machek 0 siblings, 1 reply; 7+ messages in thread From: Nishanth Menon @ 2015-01-03 18:57 UTC (permalink / raw) To: Pavel Machek, linux-omap; +Cc: edubezval, rui.zhang, linux-kernel, linux-pm On 01/03/2015 12:51 PM, Pavel Machek wrote: > On Sat 2015-01-03 11:48:04, Nishanth Menon wrote: >> On 12:19-20150103, Pavel Machek wrote: >>> Simplify code by removing goto's where they point to simple return. >>> >>> Avoid confusing |= on error values. >> >> Please split these up into chunks of related changes. there is white >> space and quiet a few other changes all mixed in. > > Do you believe that the patch is too big to be understood? I don't > think it is. > > It is four screens, and it is pretty obvious it does not change any > code. It basic patch generation stuff - where you are being nice to the reviewer. When you are doing functional changes, at least drop out or at the very least mention that there are white space changes in your patch submission. I dont care too much to review stuff where I have to look at multiple changes in a single patch - trivial or not. Sorry.. just a waste of my time. -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cleanup ti-soc-thermal 2015-01-03 18:57 ` Nishanth Menon @ 2015-01-03 19:02 ` Pavel Machek 0 siblings, 0 replies; 7+ messages in thread From: Pavel Machek @ 2015-01-03 19:02 UTC (permalink / raw) To: Nishanth Menon; +Cc: linux-omap, edubezval, rui.zhang, linux-kernel, linux-pm On Sat 2015-01-03 12:57:04, Nishanth Menon wrote: > On 01/03/2015 12:51 PM, Pavel Machek wrote: > >On Sat 2015-01-03 11:48:04, Nishanth Menon wrote: > >>On 12:19-20150103, Pavel Machek wrote: > >>>Simplify code by removing goto's where they point to simple return. > >>> > >>>Avoid confusing |= on error values. > >> > >>Please split these up into chunks of related changes. there is white > >>space and quiet a few other changes all mixed in. > > > >Do you believe that the patch is too big to be understood? I don't > >think it is. > > > >It is four screens, and it is pretty obvious it does not change any > >code. > > It basic patch generation stuff - where you are being nice to the reviewer. > When you are doing functional changes, at least drop out or at the very > least mention that there are white space changes in your patch > submission. Original submission said: # Correct whitespace. Some maintainers actually hate separate whitespace changes; talk to Al Viro some day. > I dont care too much to review stuff where I have to look at multiple > changes in a single patch - trivial or not. Sorry.. just a waste of my time. If 4 screens of trivial changes are too much for you, don't review trivial patches, then. It is not your job. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cleanup ti-soc-thermal 2015-01-03 17:48 ` Nishanth Menon 2015-01-03 18:51 ` Pavel Machek @ 2015-01-03 18:53 ` Pavel Machek 1 sibling, 0 replies; 7+ messages in thread From: Pavel Machek @ 2015-01-03 18:53 UTC (permalink / raw) To: Nishanth Menon; +Cc: edubezval, rui.zhang, linux-kernel, linux-pm On Sat 2015-01-03 11:48:04, Nishanth Menon wrote: > On 12:19-20150103, Pavel Machek wrote: > > Simplify code by removing goto's where they point to simple return. > > > > Avoid confusing |= on error values. > > Please split these up into chunks of related changes. there is white > space and quiet a few other changes all mixed in. > > Also please cc linux-omap mailing list as well to get adequate > audience. Actually, if you want linux-omap list to be consistently cc-ed, you may want to update MAINTAINERS entry. Right now it says: TI BANDGAP AND THERMAL DRIVER M: Eduardo Valentin <edubezval@gmail.com> L: linux-pm@vger.kernel.org S: Supported F: drivers/thermal/ti-soc-thermal/ Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCHv2] cleanup ti-soc-thermal 2015-01-03 11:19 [PATCH] cleanup ti-soc-thermal Pavel Machek 2015-01-03 17:48 ` Nishanth Menon @ 2015-01-18 20:17 ` Pavel Machek 1 sibling, 0 replies; 7+ messages in thread From: Pavel Machek @ 2015-01-18 20:17 UTC (permalink / raw) To: edubezval, rui.zhang, linux-kernel, linux-pm, linux-omap Simplify code by removing goto's where they point to simple return. Avoid confusing |= on error values. Correct whitespace. Signed-off-by: Pavel Machek <pavel@ucw.cz> --- Please apply, Pavel diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c index 634b6ce..2fde78c 100644 --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c @@ -103,19 +105,15 @@ do { \ */ static int ti_bandgap_power(struct ti_bandgap *bgp, bool on) { - int i, ret = 0; + int i; - if (!TI_BANDGAP_HAS(bgp, POWER_SWITCH)) { - ret = -ENOTSUPP; - goto exit; - } + if (!TI_BANDGAP_HAS(bgp, POWER_SWITCH)) + return -ENOTSUPP; for (i = 0; i < bgp->conf->sensor_count; i++) /* active on 0 */ RMW_BITS(bgp, i, temp_sensor_ctrl, bgap_tempsoff_mask, !on); - -exit: - return ret; + return 0; } /** @@ -263,18 +261,13 @@ static int ti_bandgap_adc_to_mcelsius(struct ti_bandgap *bgp, int adc_val, int *t) { const struct ti_bandgap_data *conf = bgp->conf; - int ret = 0; /* look up for temperature in the table and return the temperature */ - if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val) { - ret = -ERANGE; - goto exit; - } + if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val) + return -ERANGE; *t = bgp->conf->conv_table[adc_val - conf->adc_start_val]; - -exit: - return ret; + return 0; } /** @@ -295,16 +288,14 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc) { const struct ti_bandgap_data *conf = bgp->conf; const int *conv_table = bgp->conf->conv_table; - int high, low, mid, ret = 0; + int high, low, mid; low = 0; high = conf->adc_end_val - conf->adc_start_val; mid = (high + low) / 2; - if (temp < conv_table[low] || temp > conv_table[high]) { - ret = -ERANGE; - goto exit; - } + if (temp < conv_table[low] || temp > conv_table[high]) + return -ERANGE; while (low < high) { if (temp < conv_table[mid]) @@ -315,9 +306,7 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc) } *adc = conf->adc_start_val + low; - -exit: - return ret; + return 0; } /** @@ -343,13 +332,11 @@ int ti_bandgap_add_hyst(struct ti_bandgap *bgp, int adc_val, int hyst_val, */ ret = ti_bandgap_adc_to_mcelsius(bgp, adc_val, &temp); if (ret < 0) - goto exit; + return ret; temp += hyst_val; ret = ti_bandgap_mcelsius_to_adc(bgp, temp, sum); - -exit: return ret; } @@ -468,22 +455,18 @@ exit: */ static inline int ti_bandgap_validate(struct ti_bandgap *bgp, int id) { - int ret = 0; - if (!bgp || IS_ERR(bgp)) { pr_err("%s: invalid bandgap pointer\n", __func__); - ret = -EINVAL; - goto exit; + return -EINVAL; } if ((id < 0) || (id >= bgp->conf->sensor_count)) { dev_err(bgp->dev, "%s: sensor id out of range (%d)\n", __func__, id); - ret = -ERANGE; + return -ERANGE; } -exit: - return ret; + return 0; } /** @@ -511,12 +494,10 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val, ret = ti_bandgap_validate(bgp, id); if (ret) - goto exit; + return ret; - if (!TI_BANDGAP_HAS(bgp, TALERT)) { - ret = -ENOTSUPP; - goto exit; - } + if (!TI_BANDGAP_HAS(bgp, TALERT)) + return -ENOTSUPP; ts_data = bgp->conf->sensors[id].ts_data; tsr = bgp->conf->sensors[id].registers; @@ -529,17 +510,15 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val, } if (ret) - goto exit; + return ret; ret = ti_bandgap_mcelsius_to_adc(bgp, val, &adc_val); if (ret < 0) - goto exit; + return ret; spin_lock(&bgp->lock); ret = ti_bandgap_update_alert_threshold(bgp, id, adc_val, hot); spin_unlock(&bgp->lock); - -exit: return ret; } @@ -582,7 +561,7 @@ static int _ti_bandgap_read_threshold(struct ti_bandgap *bgp, int id, temp = ti_bandgap_readl(bgp, tsr->bgap_threshold); temp = (temp & mask) >> __ffs(mask); - ret |= ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); + ret = ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); if (ret) { dev_err(bgp->dev, "failed to read thot\n"); ret = -EIO; @@ -852,6 +831,6 @@ int ti_bandgap_read_temperature(struct ti_bandgap *bgp, int id, spin_unlock(&bgp->lock); - ret |= ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); + ret = ti_bandgap_adc_to_mcelsius(bgp, temp, &temp); if (ret) return -EIO; @@ -1220,11 +1222,10 @@ int ti_bandgap_probe(struct platform_device *pdev) goto free_irqs; } - bgp->div_clk = clk_get(NULL, bgp->conf->div_ck_name); + bgp->div_clk = clk_get(NULL, bgp->conf->div_ck_name); ret = IS_ERR(bgp->div_clk); if (ret) { - dev_err(&pdev->dev, - "failed to request div_ts_ck clock ref\n"); + dev_err(&pdev->dev, "failed to request div_ts_ck clock ref\n"); ret = PTR_ERR(bgp->div_clk); goto free_irqs; } index 5fd0386..33b2c01 100644 --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c @@ -76,7 +76,7 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c) } /* thermal zone ops */ -/* Get temperature callback function for thermal zone*/ +/* Get temperature callback function for thermal zone */ static inline int __ti_thermal_get_temp(void *devdata, long *temp) { struct thermal_zone_device *pcb_tz = NULL; -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-18 20:17 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-03 11:19 [PATCH] cleanup ti-soc-thermal Pavel Machek 2015-01-03 17:48 ` Nishanth Menon 2015-01-03 18:51 ` Pavel Machek 2015-01-03 18:57 ` Nishanth Menon 2015-01-03 19:02 ` Pavel Machek 2015-01-03 18:53 ` Pavel Machek 2015-01-18 20:17 ` [PATCHv2] " Pavel Machek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).