Re: [PATCH 33/50] staging: omap-thermal: refactor APIs handling threshold values
On Sat, Mar 16, 2013 at 08:49:20AM -0400, Eduardo Valentin wrote: > On 16-03-2013 04:39, Dan Carpenter wrote: > >On Fri, Mar 15, 2013 at 09:00:21AM -0400, Eduardo Valentin wrote: > >>if (ret) { > >>dev_err(bg_ptr->dev, "failed to read thot\n"); > >>- return -EIO; > >>+ ret = -EIO; > >>+ goto exit; > >>} > >> > >>- *thot = temp; > >>+ *val = temp; > >> > >>+exit: > >>return 0; > >> } > > > > > >The bunny hop has introduced a bug and this always returns success. > > What was the bug introduced? > > I will send a patch to fix the return value. Returning the wrong value *is* the bug. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 33/50] staging: omap-thermal: refactor APIs handling threshold values
On 16-03-2013 04:39, Dan Carpenter wrote: On Fri, Mar 15, 2013 at 09:00:21AM -0400, Eduardo Valentin wrote: if (ret) { dev_err(bg_ptr->dev, "failed to read thot\n"); - return -EIO; + ret = -EIO; + goto exit; } - *thot = temp; + *val = temp; +exit: return 0; } The bunny hop has introduced a bug and this always returns success. What was the bug introduced? I will send a patch to fix the return value. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 33/50] staging: omap-thermal: refactor APIs handling threshold values
On Fri, Mar 15, 2013 at 09:00:21AM -0400, Eduardo Valentin wrote: > if (ret) { > dev_err(bg_ptr->dev, "failed to read thot\n"); > - return -EIO; > + ret = -EIO; > + goto exit; > } > > - *thot = temp; > + *val = temp; > > +exit: > return 0; > } The bunny hop has introduced a bug and this always returns success. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 33/50] staging: omap-thermal: refactor APIs handling threshold values
On Fri, Mar 15, 2013 at 09:00:21AM -0400, Eduardo Valentin wrote: if (ret) { dev_err(bg_ptr-dev, failed to read thot\n); - return -EIO; + ret = -EIO; + goto exit; } - *thot = temp; + *val = temp; +exit: return 0; } The bunny hop has introduced a bug and this always returns success. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 33/50] staging: omap-thermal: refactor APIs handling threshold values
On 16-03-2013 04:39, Dan Carpenter wrote: On Fri, Mar 15, 2013 at 09:00:21AM -0400, Eduardo Valentin wrote: if (ret) { dev_err(bg_ptr-dev, failed to read thot\n); - return -EIO; + ret = -EIO; + goto exit; } - *thot = temp; + *val = temp; +exit: return 0; } The bunny hop has introduced a bug and this always returns success. What was the bug introduced? I will send a patch to fix the return value. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 33/50] staging: omap-thermal: refactor APIs handling threshold values
On Sat, Mar 16, 2013 at 08:49:20AM -0400, Eduardo Valentin wrote: On 16-03-2013 04:39, Dan Carpenter wrote: On Fri, Mar 15, 2013 at 09:00:21AM -0400, Eduardo Valentin wrote: if (ret) { dev_err(bg_ptr-dev, failed to read thot\n); - return -EIO; + ret = -EIO; + goto exit; } - *thot = temp; + *val = temp; +exit: return 0; } The bunny hop has introduced a bug and this always returns success. What was the bug introduced? I will send a patch to fix the return value. Returning the wrong value *is* the bug. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 33/50] staging: omap-thermal: refactor APIs handling threshold values
This patch improves the code that handles threshold values by creating single functions that are usable for tcold and thot. This way we won't have duplicated functionality just because it is handling different bitfields. Now the added functions are reused in several places where it is needed to update any threshold. This patch also removes macros that are used only inside the _validate helper function. In this patch there is also an addition of an extra function section for Exposed APIs, used outside the omap-bandgap.c, but inside the omap-thermal driver. Signed-off-by: Eduardo Valentin --- drivers/staging/omap-thermal/omap-bandgap.c | 278 +++ 1 files changed, 117 insertions(+), 161 deletions(-) diff --git a/drivers/staging/omap-thermal/omap-bandgap.c b/drivers/staging/omap-thermal/omap-bandgap.c index d001323..2a13bf0 100644 --- a/drivers/staging/omap-thermal/omap-bandgap.c +++ b/drivers/staging/omap-thermal/omap-bandgap.c @@ -370,145 +370,172 @@ static void omap_bandgap_unmask_interrupts(struct omap_bandgap *bg_ptr, int id, omap_bandgap_writel(bg_ptr, reg_val, tsr->bgap_mask_ctrl); } -/* Talert Thot threshold. Call it only if HAS(TALERT) is set */ static -int temp_sensor_configure_thot(struct omap_bandgap *bg_ptr, int id, int t_hot) +int omap_bandgap_update_alert_threshold(struct omap_bandgap *bg_ptr, int id, + int val, bool hot) { struct temp_sensor_data *ts_data = bg_ptr->conf->sensors[id].ts_data; struct temp_sensor_registers *tsr; - u32 thresh_val, reg_val; - int cold, err = 0; + u32 thresh_val, reg_val, t_hot, t_cold; + int err = 0; tsr = bg_ptr->conf->sensors[id].registers; - /* obtain the T cold value */ + /* obtain the current value */ thresh_val = omap_bandgap_readl(bg_ptr, tsr->bgap_threshold); - cold = (thresh_val & tsr->threshold_tcold_mask) >> - __ffs(tsr->threshold_tcold_mask); - if (t_hot <= cold) { - /* change the t_cold to t_hot - 5000 millidegrees */ - err |= omap_bandgap_add_hyst(bg_ptr, t_hot, --ts_data->hyst_val, ); - /* write the new t_cold value */ - reg_val = thresh_val & (~tsr->threshold_tcold_mask); - reg_val |= cold << __ffs(tsr->threshold_tcold_mask); - omap_bandgap_writel(bg_ptr, reg_val, tsr->bgap_threshold); - thresh_val = reg_val; + t_cold = (thresh_val & tsr->threshold_tcold_mask) >> + __ffs(tsr->threshold_tcold_mask); + t_hot = (thresh_val & tsr->threshold_thot_mask) >> + __ffs(tsr->threshold_thot_mask); + if (hot) + t_hot = val; + else + t_cold = val; + + if (t_cold < t_hot) { + if (hot) + err = omap_bandgap_add_hyst(bg_ptr, t_hot, + -ts_data->hyst_val, + _cold); + else + err = omap_bandgap_add_hyst(bg_ptr, t_cold, + ts_data->hyst_val, + _hot); } - /* write the new t_hot value */ + /* write the new threshold values */ reg_val = thresh_val & ~tsr->threshold_thot_mask; reg_val |= (t_hot << __ffs(tsr->threshold_thot_mask)); + reg_val |= thresh_val & ~tsr->threshold_tcold_mask; + reg_val |= (t_cold << __ffs(tsr->threshold_tcold_mask)); omap_bandgap_writel(bg_ptr, reg_val, tsr->bgap_threshold); + if (err) { dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n"); - return -EIO; + err = -EIO; + goto exit; } - omap_bandgap_unmask_interrupts(bg_ptr, id, t_hot, cold); - - return 0; + omap_bandgap_unmask_interrupts(bg_ptr, id, t_hot, t_cold); +exit: + return err; } -/* Talert Tcold threshold. Call it only if HAS(TALERT) is set */ -static -int temp_sensor_configure_tcold(struct omap_bandgap *bg_ptr, int id, - int t_cold) +static inline int omap_bandgap_validate(struct omap_bandgap *bg_ptr, int id) { - struct temp_sensor_data *ts_data = bg_ptr->conf->sensors[id].ts_data; - struct temp_sensor_registers *tsr; - u32 thresh_val, reg_val; - int hot, err = 0; + int ret = 0; - tsr = bg_ptr->conf->sensors[id].registers; - /* obtain the T cold value */ - thresh_val = omap_bandgap_readl(bg_ptr, tsr->bgap_threshold); - hot = (thresh_val & tsr->threshold_thot_mask) >> - __ffs(tsr->threshold_thot_mask); - - if (t_cold >= hot) { - /* change the t_hot to t_cold + 5000 millidegrees */ - err |= omap_bandgap_add_hyst(bg_ptr, t_cold, -
[PATCH 33/50] staging: omap-thermal: refactor APIs handling threshold values
This patch improves the code that handles threshold values by creating single functions that are usable for tcold and thot. This way we won't have duplicated functionality just because it is handling different bitfields. Now the added functions are reused in several places where it is needed to update any threshold. This patch also removes macros that are used only inside the _validate helper function. In this patch there is also an addition of an extra function section for Exposed APIs, used outside the omap-bandgap.c, but inside the omap-thermal driver. Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com --- drivers/staging/omap-thermal/omap-bandgap.c | 278 +++ 1 files changed, 117 insertions(+), 161 deletions(-) diff --git a/drivers/staging/omap-thermal/omap-bandgap.c b/drivers/staging/omap-thermal/omap-bandgap.c index d001323..2a13bf0 100644 --- a/drivers/staging/omap-thermal/omap-bandgap.c +++ b/drivers/staging/omap-thermal/omap-bandgap.c @@ -370,145 +370,172 @@ static void omap_bandgap_unmask_interrupts(struct omap_bandgap *bg_ptr, int id, omap_bandgap_writel(bg_ptr, reg_val, tsr-bgap_mask_ctrl); } -/* Talert Thot threshold. Call it only if HAS(TALERT) is set */ static -int temp_sensor_configure_thot(struct omap_bandgap *bg_ptr, int id, int t_hot) +int omap_bandgap_update_alert_threshold(struct omap_bandgap *bg_ptr, int id, + int val, bool hot) { struct temp_sensor_data *ts_data = bg_ptr-conf-sensors[id].ts_data; struct temp_sensor_registers *tsr; - u32 thresh_val, reg_val; - int cold, err = 0; + u32 thresh_val, reg_val, t_hot, t_cold; + int err = 0; tsr = bg_ptr-conf-sensors[id].registers; - /* obtain the T cold value */ + /* obtain the current value */ thresh_val = omap_bandgap_readl(bg_ptr, tsr-bgap_threshold); - cold = (thresh_val tsr-threshold_tcold_mask) - __ffs(tsr-threshold_tcold_mask); - if (t_hot = cold) { - /* change the t_cold to t_hot - 5000 millidegrees */ - err |= omap_bandgap_add_hyst(bg_ptr, t_hot, --ts_data-hyst_val, cold); - /* write the new t_cold value */ - reg_val = thresh_val (~tsr-threshold_tcold_mask); - reg_val |= cold __ffs(tsr-threshold_tcold_mask); - omap_bandgap_writel(bg_ptr, reg_val, tsr-bgap_threshold); - thresh_val = reg_val; + t_cold = (thresh_val tsr-threshold_tcold_mask) + __ffs(tsr-threshold_tcold_mask); + t_hot = (thresh_val tsr-threshold_thot_mask) + __ffs(tsr-threshold_thot_mask); + if (hot) + t_hot = val; + else + t_cold = val; + + if (t_cold t_hot) { + if (hot) + err = omap_bandgap_add_hyst(bg_ptr, t_hot, + -ts_data-hyst_val, + t_cold); + else + err = omap_bandgap_add_hyst(bg_ptr, t_cold, + ts_data-hyst_val, + t_hot); } - /* write the new t_hot value */ + /* write the new threshold values */ reg_val = thresh_val ~tsr-threshold_thot_mask; reg_val |= (t_hot __ffs(tsr-threshold_thot_mask)); + reg_val |= thresh_val ~tsr-threshold_tcold_mask; + reg_val |= (t_cold __ffs(tsr-threshold_tcold_mask)); omap_bandgap_writel(bg_ptr, reg_val, tsr-bgap_threshold); + if (err) { dev_err(bg_ptr-dev, failed to reprogram thot threshold\n); - return -EIO; + err = -EIO; + goto exit; } - omap_bandgap_unmask_interrupts(bg_ptr, id, t_hot, cold); - - return 0; + omap_bandgap_unmask_interrupts(bg_ptr, id, t_hot, t_cold); +exit: + return err; } -/* Talert Tcold threshold. Call it only if HAS(TALERT) is set */ -static -int temp_sensor_configure_tcold(struct omap_bandgap *bg_ptr, int id, - int t_cold) +static inline int omap_bandgap_validate(struct omap_bandgap *bg_ptr, int id) { - struct temp_sensor_data *ts_data = bg_ptr-conf-sensors[id].ts_data; - struct temp_sensor_registers *tsr; - u32 thresh_val, reg_val; - int hot, err = 0; + int ret = 0; - tsr = bg_ptr-conf-sensors[id].registers; - /* obtain the T cold value */ - thresh_val = omap_bandgap_readl(bg_ptr, tsr-bgap_threshold); - hot = (thresh_val tsr-threshold_thot_mask) - __ffs(tsr-threshold_thot_mask); - - if (t_cold = hot) { - /* change the t_hot to t_cold + 5000 millidegrees */ - err |= omap_bandgap_add_hyst(bg_ptr, t_cold, -