Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752830AbcKSEEF (ORCPT ); Fri, 18 Nov 2016 23:04:05 -0500 Received: from regular1.263xmail.com ([211.150.99.131]:44023 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752234AbcKSEED (ORCPT ); Fri, 18 Nov 2016 23:04:03 -0500 X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-RL-SENDER: wxt@rock-chips.com X-FST-TO: wxt@rock-chips.com X-SENDER-IP: 103.29.142.67 X-LOGIN-NAME: wxt@rock-chips.com X-UNIQUE-TAG: X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Subject: Re: [PATCH 3/3] thermal: rockchip: don't pass table structs by value To: Brian Norris , Zhang Rui , Eduardo Valentin References: <1479513177-81504-1-git-send-email-briannorris@chromium.org> <1479513177-81504-3-git-send-email-briannorris@chromium.org> Cc: Heiko Stuebner , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Stephen Barber , linux-rockchip@lists.infradead.org, Caesar Wang From: Caesar Wang Message-ID: <0a05cc27-53d2-3c81-7ccc-31fd22ba584a@rock-chips.com> Date: Sat, 19 Nov 2016 12:03:32 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1479513177-81504-3-git-send-email-briannorris@chromium.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8787 Lines: 239 在 2016年11月19日 07:52, Brian Norris 写道: > This driver passes struct chip_tsadc_table by value throughout; this is > inefficient, and AFAICT, there is no reason for it. Let's pass pointers > instead. > > Signed-off-by: Brian Norris Reviewed-by: Caesar Wang Tested-by: Caesar Wang Yup, that make sense to improve efficiency. Thanks the fixes. And tested on rk3399 evb board. > --- > drivers/thermal/rockchip_thermal.c | 80 +++++++++++++++++++------------------- > 1 file changed, 40 insertions(+), 40 deletions(-) > > diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c > index 35554d146b9d..30fb95a0dff0 100644 > --- a/drivers/thermal/rockchip_thermal.c > +++ b/drivers/thermal/rockchip_thermal.c > @@ -118,11 +118,11 @@ struct rockchip_tsadc_chip { > void (*control)(void __iomem *reg, bool on); > > /* Per-sensor methods */ > - int (*get_temp)(struct chip_tsadc_table table, > + int (*get_temp)(const struct chip_tsadc_table *table, > int chn, void __iomem *reg, int *temp); > - void (*set_alarm_temp)(struct chip_tsadc_table table, > + void (*set_alarm_temp)(const struct chip_tsadc_table *table, > int chn, void __iomem *reg, int temp); > - void (*set_tshut_temp)(struct chip_tsadc_table table, > + void (*set_tshut_temp)(const struct chip_tsadc_table *table, > int chn, void __iomem *reg, int temp); > void (*set_tshut_mode)(int chn, void __iomem *reg, enum tshut_mode m); > > @@ -397,26 +397,26 @@ static const struct tsadc_table rk3399_code_table[] = { > {TSADCV3_DATA_MASK, 125000}, > }; > > -static u32 rk_tsadcv2_temp_to_code(struct chip_tsadc_table table, > +static u32 rk_tsadcv2_temp_to_code(const struct chip_tsadc_table *table, > int temp) > { > int high, low, mid; > u32 error = 0; > > low = 0; > - high = table.length - 1; > + high = table->length - 1; > mid = (high + low) / 2; > > /* Return mask code data when the temp is over table range */ > - if (temp < table.id[low].temp || temp > table.id[high].temp) { > - error = table.data_mask; > + if (temp < table->id[low].temp || temp > table->id[high].temp) { > + error = table->data_mask; > goto exit; > } > > while (low <= high) { > - if (temp == table.id[mid].temp) > - return table.id[mid].code; > - else if (temp < table.id[mid].temp) > + if (temp == table->id[mid].temp) > + return table->id[mid].code; > + else if (temp < table->id[mid].temp) > high = mid - 1; > else > low = mid + 1; > @@ -429,28 +429,28 @@ static u32 rk_tsadcv2_temp_to_code(struct chip_tsadc_table table, > return error; > } > > -static int rk_tsadcv2_code_to_temp(struct chip_tsadc_table table, u32 code, > - int *temp) > +static int rk_tsadcv2_code_to_temp(const struct chip_tsadc_table *table, > + u32 code, int *temp) > { > unsigned int low = 1; > - unsigned int high = table.length - 1; > + unsigned int high = table->length - 1; > unsigned int mid = (low + high) / 2; > unsigned int num; > unsigned long denom; > > - WARN_ON(table.length < 2); > + WARN_ON(table->length < 2); > > - switch (table.mode) { > + switch (table->mode) { > case ADC_DECREMENT: > - code &= table.data_mask; > - if (code < table.id[high].code) > + code &= table->data_mask; > + if (code < table->id[high].code) > return -EAGAIN; /* Incorrect reading */ > > while (low <= high) { > - if (code >= table.id[mid].code && > - code < table.id[mid - 1].code) > + if (code >= table->id[mid].code && > + code < table->id[mid - 1].code) > break; > - else if (code < table.id[mid].code) > + else if (code < table->id[mid].code) > low = mid + 1; > else > high = mid - 1; > @@ -459,15 +459,15 @@ static int rk_tsadcv2_code_to_temp(struct chip_tsadc_table table, u32 code, > } > break; > case ADC_INCREMENT: > - code &= table.data_mask; > - if (code < table.id[low].code) > + code &= table->data_mask; > + if (code < table->id[low].code) > return -EAGAIN; /* Incorrect reading */ > > while (low <= high) { > - if (code <= table.id[mid].code && > - code > table.id[mid - 1].code) > + if (code <= table->id[mid].code && > + code > table->id[mid - 1].code) > break; > - else if (code > table.id[mid].code) > + else if (code > table->id[mid].code) > low = mid + 1; > else > high = mid - 1; > @@ -477,7 +477,7 @@ static int rk_tsadcv2_code_to_temp(struct chip_tsadc_table table, u32 code, > break; > default: > pr_err("%s: invalid conversion table, mode=%d\n", > - __func__, table.mode); > + __func__, table->mode); > return -EINVAL; > } > > @@ -487,10 +487,10 @@ static int rk_tsadcv2_code_to_temp(struct chip_tsadc_table table, u32 code, > * temperature between 2 table entries is linear and interpolate > * to produce less granular result. > */ > - num = table.id[mid].temp - table.id[mid - 1].temp; > - num *= abs(table.id[mid - 1].code - code); > - denom = abs(table.id[mid - 1].code - table.id[mid].code); > - *temp = table.id[mid - 1].temp + (num / denom); > + num = table->id[mid].temp - table->id[mid - 1].temp; > + num *= abs(table->id[mid - 1].code - code); > + denom = abs(table->id[mid - 1].code - table->id[mid].code); > + *temp = table->id[mid - 1].temp + (num / denom); > > return 0; > } > @@ -646,7 +646,7 @@ static void rk_tsadcv3_control(void __iomem *regs, bool enable) > writel_relaxed(val, regs + TSADCV2_AUTO_CON); > } > > -static int rk_tsadcv2_get_temp(struct chip_tsadc_table table, > +static int rk_tsadcv2_get_temp(const struct chip_tsadc_table *table, > int chn, void __iomem *regs, int *temp) > { > u32 val; > @@ -656,17 +656,17 @@ static int rk_tsadcv2_get_temp(struct chip_tsadc_table table, > return rk_tsadcv2_code_to_temp(table, val, temp); > } > > -static void rk_tsadcv2_alarm_temp(struct chip_tsadc_table table, > +static void rk_tsadcv2_alarm_temp(const struct chip_tsadc_table *table, > int chn, void __iomem *regs, int temp) > { > u32 alarm_value, int_en; > > /* Make sure the value is valid */ > alarm_value = rk_tsadcv2_temp_to_code(table, temp); > - if (alarm_value == table.data_mask) > + if (alarm_value == table->data_mask) > return; > > - writel_relaxed(alarm_value & table.data_mask, > + writel_relaxed(alarm_value & table->data_mask, > regs + TSADCV2_COMP_INT(chn)); > > int_en = readl_relaxed(regs + TSADCV2_INT_EN); > @@ -674,14 +674,14 @@ static void rk_tsadcv2_alarm_temp(struct chip_tsadc_table table, > writel_relaxed(int_en, regs + TSADCV2_INT_EN); > } > > -static void rk_tsadcv2_tshut_temp(struct chip_tsadc_table table, > +static void rk_tsadcv2_tshut_temp(const struct chip_tsadc_table *table, > int chn, void __iomem *regs, int temp) > { > u32 tshut_value, val; > > /* Make sure the value is valid */ > tshut_value = rk_tsadcv2_temp_to_code(table, temp); > - if (tshut_value == table.data_mask) > + if (tshut_value == table->data_mask) > return; > > writel_relaxed(tshut_value, regs + TSADCV2_COMP_SHUT(chn)); > @@ -891,7 +891,7 @@ static int rockchip_thermal_set_trips(void *_sensor, int low, int high) > dev_dbg(&thermal->pdev->dev, "%s: sensor %d: low: %d, high %d\n", > __func__, sensor->id, low, high); > > - tsadc->set_alarm_temp(tsadc->table, > + tsadc->set_alarm_temp(&tsadc->table, > sensor->id, thermal->regs, high); > > return 0; > @@ -904,7 +904,7 @@ static int rockchip_thermal_get_temp(void *_sensor, int *out_temp) > const struct rockchip_tsadc_chip *tsadc = sensor->thermal->chip; > int retval; > > - retval = tsadc->get_temp(tsadc->table, > + retval = tsadc->get_temp(&tsadc->table, > sensor->id, thermal->regs, out_temp); > dev_dbg(&thermal->pdev->dev, "sensor %d - temp: %d, retval: %d\n", > sensor->id, *out_temp, retval); > @@ -988,7 +988,7 @@ rockchip_thermal_register_sensor(struct platform_device *pdev, > int error; > > tsadc->set_tshut_mode(id, thermal->regs, thermal->tshut_mode); > - tsadc->set_tshut_temp(tsadc->table, id, thermal->regs, > + tsadc->set_tshut_temp(&tsadc->table, id, thermal->regs, > thermal->tshut_temp); > > sensor->thermal = thermal; > @@ -1202,7 +1202,7 @@ static int __maybe_unused rockchip_thermal_resume(struct device *dev) > > thermal->chip->set_tshut_mode(id, thermal->regs, > thermal->tshut_mode); > - thermal->chip->set_tshut_temp(thermal->chip->table, > + thermal->chip->set_tshut_temp(&thermal->chip->table, > id, thermal->regs, > thermal->tshut_temp); > }