Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1300270 > unrolled thread

[PATCH 0/8] rtc-ab-b5ze-s3: Fine-tuning for some function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/8] rtc-ab-b5ze-s3: Fine-tuning for some function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 09:50 +0100
    [PATCH 3/8] rtc-ab-b5ze-s3: Delete an unnecessary variable  initialisation in _abb5zes3_rtc_set_timer() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 10:00 +0100
    [PATCH 5/8] rtc-ab-b5ze-s3: Replace a variable initialisation by an  assignment in _abb5zes3_rtc_read_alarm() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 10:00 +0100
    [PATCH 2/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  abb5zes3_rtc_set_alarm() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 10:00 +0100
    [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_interrupt() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 10:00 +0100
      Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_interrupt() Julia Lawall <julia.lawall@lip6.fr> - 2016-01-03 13:50 +0100
        Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_interrupt() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 18:10 +0100
      Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_interrupt() Julia Lawall <julia.lawall@lip6.fr> - 2016-01-03 13:50 +0100
        Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_interrupt() Julia Lawall <julia.lawall@lip6.fr> - 2016-01-03 18:00 +0100
        Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_interrupt() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 18:00 +0100
    [PATCH 4/8] rtc-ab-b5ze-s3: Replace a variable initialisation by an  assignment in _abb5zes3_rtc_set_alarm() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 10:00 +0100
    [PATCH 1/8] rtc-ab-b5ze-s3: Better exception handling in  abb5zes3_probe() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 10:00 +0100
    [PATCH 6/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_read_timer() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 10:00 +0100
    [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_set_timer() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 10:00 +0100
      Re: [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_set_timer() Julia Lawall <julia.lawall@lip6.fr> - 2016-01-03 13:50 +0100
        Re: [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_set_timer() Julia Lawall <julia.lawall@lip6.fr> - 2016-01-03 18:30 +0100
        Re: [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in  _abb5zes3_rtc_set_timer() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-03 18:30 +0100

#1300270 — [PATCH 0/8] rtc-ab-b5ze-s3: Fine-tuning for some function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 09:37:34 +0100

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (8):
  Better exception handling in abb5zes3_probe()
  Delete an unnecessary variable in abb5zes3_rtc_set_alarm()
  Delete an unnecessary variable initialisation in _abb5zes3_rtc_set_timer()
  Replace a variable initialisation by an assignment in _abb5zes3_rtc_set_alarm()
  Replace a variable initialisation by an assignment in _abb5zes3_rtc_read_alarm()
  Delete an unnecessary variable in _abb5zes3_rtc_read_timer()
  Delete an unnecessary variable in _abb5zes3_rtc_interrupt()
  Delete an unnecessary variable in _abb5zes3_rtc_set_timer()

 drivers/rtc/rtc-ab-b5ze-s3.c | 59 ++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 32 deletions(-)

-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1300271 — [PATCH 3/8] rtc-ab-b5ze-s3: Delete an unnecessary variable initialisation in _abb5zes3_rtc_set_timer()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 07:42:18 +0100

Omit explicit initialisation at the beginning for one local variable
that is redefined before its first use.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/rtc/rtc-ab-b5ze-s3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
index ed9b873..33a7cf1 100644
--- a/drivers/rtc/rtc-ab-b5ze-s3.c
+++ b/drivers/rtc/rtc-ab-b5ze-s3.c
@@ -560,7 +560,7 @@ static int _abb5zes3_rtc_set_timer(struct device *dev, struct rtc_wkalrm *alarm,
 	struct abb5zes3_rtc_data *data = dev_get_drvdata(dev);
 	u8 regs[ABB5ZES3_TIMA_SEC_LEN];
 	u8 mask = ABB5ZES3_REG_TIM_CLK_TAC0 | ABB5ZES3_REG_TIM_CLK_TAC1;
-	int ret = 0;
+	int ret;
 
 	/* Program given number of seconds to Timer A registers */
 	sec_to_timer_a(secs, &regs[0], &regs[1]);
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300272 — [PATCH 5/8] rtc-ab-b5ze-s3: Replace a variable initialisation by an assignment in _abb5zes3_rtc_read_alarm()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 08:00:29 +0100

Replace an explicit initialisation for one local variable at the beginning
by an assignment.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/rtc/rtc-ab-b5ze-s3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
index 0986715..02e3443 100644
--- a/drivers/rtc/rtc-ab-b5ze-s3.c
+++ b/drivers/rtc/rtc-ab-b5ze-s3.c
@@ -382,7 +382,7 @@ static int _abb5zes3_rtc_read_alarm(struct device *dev,
 				    struct rtc_wkalrm *alarm)
 {
 	struct abb5zes3_rtc_data *data = dev_get_drvdata(dev);
-	struct rtc_time rtc_tm, *alarm_tm = &alarm->time;
+	struct rtc_time rtc_tm, *alarm_tm;
 	unsigned long rtc_secs, alarm_secs;
 	u8 regs[ABB5ZES3_ALRM_SEC_LEN];
 	unsigned int reg;
@@ -396,6 +396,7 @@ static int _abb5zes3_rtc_read_alarm(struct device *dev,
 		goto err;
 	}
 
+	alarm_tm = &alarm->time;
 	alarm_tm->tm_sec  = 0;
 	alarm_tm->tm_min  = bcd2bin(regs[0] & 0x7f);
 	alarm_tm->tm_hour = bcd2bin(regs[1] & 0x3f);
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300273 — [PATCH 2/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in abb5zes3_rtc_set_alarm()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 07:35:09 +0100

Pass the address of the data structure element "time" directly in a call
of the function "rtc_tm_to_time" instead of an extra initialisation
for one local variable at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/rtc/rtc-ab-b5ze-s3.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
index 1291206..ed9b873 100644
--- a/drivers/rtc/rtc-ab-b5ze-s3.c
+++ b/drivers/rtc/rtc-ab-b5ze-s3.c
@@ -596,7 +596,6 @@ err:
 static int abb5zes3_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
 	struct abb5zes3_rtc_data *data = dev_get_drvdata(dev);
-	struct rtc_time *alarm_tm = &alarm->time;
 	unsigned long rtc_secs, alarm_secs;
 	struct rtc_time rtc_tm;
 	int ret;
@@ -610,7 +609,7 @@ static int abb5zes3_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	if (ret)
 		goto err;
 
-	ret = rtc_tm_to_time(alarm_tm, &alarm_secs);
+	ret = rtc_tm_to_time(&alarm->time, &alarm_secs);
 	if (ret)
 		goto err;
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300274 — [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_interrupt()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 09:00:30 +0100

Pass the address of the data structure element "time" directly in calls
of the function "rtc_update_irq" instead of an extra initialisation
for one local variable at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/rtc/rtc-ab-b5ze-s3.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
index e3a015a..88f1d0b 100644
--- a/drivers/rtc/rtc-ab-b5ze-s3.c
+++ b/drivers/rtc/rtc-ab-b5ze-s3.c
@@ -816,7 +816,6 @@ static irqreturn_t _abb5zes3_rtc_interrupt(int irq, void *data)
 	struct i2c_client *client = data;
 	struct device *dev = &client->dev;
 	struct abb5zes3_rtc_data *rtc_data = dev_get_drvdata(dev);
-	struct rtc_device *rtc = rtc_data->rtc;
 	u8 regs[ABB5ZES3_CTRL_SEC_LEN];
 	int ret, handled = IRQ_NONE;
 
@@ -844,8 +843,7 @@ static irqreturn_t _abb5zes3_rtc_interrupt(int irq, void *data)
 	/* Check alarm flag */
 	if (regs[ABB5ZES3_REG_CTRL2] & ABB5ZES3_REG_CTRL2_AF) {
 		dev_dbg(dev, "RTC alarm!\n");
-
-		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+		rtc_update_irq(rtc_data->rtc, 1, RTC_IRQF | RTC_AF);
 
 		/* Acknowledge and disable the alarm */
 		_abb5zes3_rtc_clear_alarm(dev);
@@ -857,8 +855,7 @@ static irqreturn_t _abb5zes3_rtc_interrupt(int irq, void *data)
 	/* Check watchdog Timer A flag */
 	if (regs[ABB5ZES3_REG_CTRL2] & ABB5ZES3_REG_CTRL2_WTAF) {
 		dev_dbg(dev, "RTC timer!\n");
-
-		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+		rtc_update_irq(rtc_data->rtc, 1, RTC_IRQF | RTC_AF);
 
 		/*
 		 * Acknowledge and disable the alarm. Note: WTAF
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300294 — Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_interrupt()

On Sun, 3 Jan 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 3 Jan 2016 09:00:30 +0100
> 
> Pass the address of the data structure element "time" directly in calls
> of the function "rtc_update_irq" instead of an extra initialisation
> for one local variable at the beginning.

Also, I don't see anything related to time in this patch.

julia

> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/rtc/rtc-ab-b5ze-s3.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
> index e3a015a..88f1d0b 100644
> --- a/drivers/rtc/rtc-ab-b5ze-s3.c
> +++ b/drivers/rtc/rtc-ab-b5ze-s3.c
> @@ -816,7 +816,6 @@ static irqreturn_t _abb5zes3_rtc_interrupt(int irq, void *data)
>  	struct i2c_client *client = data;
>  	struct device *dev = &client->dev;
>  	struct abb5zes3_rtc_data *rtc_data = dev_get_drvdata(dev);
> -	struct rtc_device *rtc = rtc_data->rtc;
>  	u8 regs[ABB5ZES3_CTRL_SEC_LEN];
>  	int ret, handled = IRQ_NONE;
>  
> @@ -844,8 +843,7 @@ static irqreturn_t _abb5zes3_rtc_interrupt(int irq, void *data)
>  	/* Check alarm flag */
>  	if (regs[ABB5ZES3_REG_CTRL2] & ABB5ZES3_REG_CTRL2_AF) {
>  		dev_dbg(dev, "RTC alarm!\n");
> -
> -		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
> +		rtc_update_irq(rtc_data->rtc, 1, RTC_IRQF | RTC_AF);
>  
>  		/* Acknowledge and disable the alarm */
>  		_abb5zes3_rtc_clear_alarm(dev);
> @@ -857,8 +855,7 @@ static irqreturn_t _abb5zes3_rtc_interrupt(int irq, void *data)
>  	/* Check watchdog Timer A flag */
>  	if (regs[ABB5ZES3_REG_CTRL2] & ABB5ZES3_REG_CTRL2_WTAF) {
>  		dev_dbg(dev, "RTC timer!\n");
> -
> -		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
> +		rtc_update_irq(rtc_data->rtc, 1, RTC_IRQF | RTC_AF);
>  
>  		/*
>  		 * Acknowledge and disable the alarm. Note: WTAF
> -- 
> 2.6.3
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300338 — Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_interrupt()

>> Pass the address of the data structure element "time" directly in calls
>> of the function "rtc_update_irq" instead of an extra initialisation
>> for one local variable at the beginning.
> 
> Also, I don't see anything related to time in this patch.

I should have referred to the data structure element "rtc" here.
Should I resend this patch series with a corrected commit message now?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300296 — Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_interrupt()

On Sun, 3 Jan 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 3 Jan 2016 09:00:30 +0100
> 
> Pass the address of the data structure element "time" directly in calls
> of the function "rtc_update_irq" instead of an extra initialisation
> for one local variable at the beginning.

Why is it better?

julia
 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/rtc/rtc-ab-b5ze-s3.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
> index e3a015a..88f1d0b 100644
> --- a/drivers/rtc/rtc-ab-b5ze-s3.c
> +++ b/drivers/rtc/rtc-ab-b5ze-s3.c
> @@ -816,7 +816,6 @@ static irqreturn_t _abb5zes3_rtc_interrupt(int irq, void *data)
>  	struct i2c_client *client = data;
>  	struct device *dev = &client->dev;
>  	struct abb5zes3_rtc_data *rtc_data = dev_get_drvdata(dev);
> -	struct rtc_device *rtc = rtc_data->rtc;
>  	u8 regs[ABB5ZES3_CTRL_SEC_LEN];
>  	int ret, handled = IRQ_NONE;
>  
> @@ -844,8 +843,7 @@ static irqreturn_t _abb5zes3_rtc_interrupt(int irq, void *data)
>  	/* Check alarm flag */
>  	if (regs[ABB5ZES3_REG_CTRL2] & ABB5ZES3_REG_CTRL2_AF) {
>  		dev_dbg(dev, "RTC alarm!\n");
> -
> -		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
> +		rtc_update_irq(rtc_data->rtc, 1, RTC_IRQF | RTC_AF);
>  
>  		/* Acknowledge and disable the alarm */
>  		_abb5zes3_rtc_clear_alarm(dev);
> @@ -857,8 +855,7 @@ static irqreturn_t _abb5zes3_rtc_interrupt(int irq, void *data)
>  	/* Check watchdog Timer A flag */
>  	if (regs[ABB5ZES3_REG_CTRL2] & ABB5ZES3_REG_CTRL2_WTAF) {
>  		dev_dbg(dev, "RTC timer!\n");
> -
> -		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
> +		rtc_update_irq(rtc_data->rtc, 1, RTC_IRQF | RTC_AF);
>  
>  		/*
>  		 * Acknowledge and disable the alarm. Note: WTAF
> -- 
> 2.6.3
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300335 — Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_interrupt()

On Sun, 3 Jan 2016, SF Markus Elfring wrote:

> >> Pass the address of the data structure element "time" directly in calls
> >> of the function "rtc_update_irq" instead of an extra initialisation
> >> for one local variable at the beginning.
> > 
> > Why is it better?
> 
> I suggest to refer to the data item "rtc_data->rtc" directly because
> the variable "rtc" was read only in two if branches.
> Does it make sense then to reduce the variable allocation?

No.  That is the job of the compiler.  For a local variable whose address 
is never taken, the compiler can easily detect its live region, and place 
the initialization in an optimal way.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300337 — Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_interrupt()

>> Pass the address of the data structure element "time" directly in calls
>> of the function "rtc_update_irq" instead of an extra initialisation
>> for one local variable at the beginning.
> 
> Why is it better?

I suggest to refer to the data item "rtc_data->rtc" directly because
the variable "rtc" was read only in two if branches.
Does it make sense then to reduce the variable allocation?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300275 — [PATCH 4/8] rtc-ab-b5ze-s3: Replace a variable initialisation by an assignment in _abb5zes3_rtc_set_alarm()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 07:51:49 +0100

Replace an explicit initialisation for one local variable at the beginning
by an assignment.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/rtc/rtc-ab-b5ze-s3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
index 33a7cf1..0986715 100644
--- a/drivers/rtc/rtc-ab-b5ze-s3.c
+++ b/drivers/rtc/rtc-ab-b5ze-s3.c
@@ -476,7 +476,7 @@ static int abb5zes3_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 static int _abb5zes3_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
 	struct abb5zes3_rtc_data *data = dev_get_drvdata(dev);
-	struct rtc_time *alarm_tm = &alarm->time;
+	struct rtc_time *alarm_tm;
 	unsigned long rtc_secs, alarm_secs;
 	u8 regs[ABB5ZES3_ALRM_SEC_LEN];
 	struct rtc_time rtc_tm;
@@ -490,6 +490,7 @@ static int _abb5zes3_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	if (ret)
 		goto err;
 
+	alarm_tm = &alarm->time;
 	ret = rtc_tm_to_time(alarm_tm, &alarm_secs);
 	if (ret)
 		goto err;
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300276 — [PATCH 1/8] rtc-ab-b5ze-s3: Better exception handling in abb5zes3_probe()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 07:07:49 +0100

This issue was detected by using the Coccinelle software.

* Return directly before the data structure element "irq" was assigned.

* Drop the explicit initialisation for the variable "data"
  at the beginning then.

* Adjust jump targets according to the Linux coding style convention.

* Simplify a condition check at the end.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/rtc/rtc-ab-b5ze-s3.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
index a319bf1..1291206 100644
--- a/drivers/rtc/rtc-ab-b5ze-s3.c
+++ b/drivers/rtc/rtc-ab-b5ze-s3.c
@@ -889,35 +889,31 @@ static const struct regmap_config abb5zes3_rtc_regmap_config = {
 static int abb5zes3_probe(struct i2c_client *client,
 			  const struct i2c_device_id *id)
 {
-	struct abb5zes3_rtc_data *data = NULL;
+	struct abb5zes3_rtc_data *data;
 	struct device *dev = &client->dev;
 	struct regmap *regmap;
 	int ret;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
 				     I2C_FUNC_SMBUS_BYTE_DATA |
-				     I2C_FUNC_SMBUS_I2C_BLOCK)) {
-		ret = -ENODEV;
-		goto err;
-	}
+				     I2C_FUNC_SMBUS_I2C_BLOCK))
+		return -ENODEV;
 
 	regmap = devm_regmap_init_i2c(client, &abb5zes3_rtc_regmap_config);
 	if (IS_ERR(regmap)) {
 		ret = PTR_ERR(regmap);
 		dev_err(dev, "%s: regmap allocation failed: %d\n",
 			__func__, ret);
-		goto err;
+		return ret;
 	}
 
 	ret = abb5zes3_i2c_validate_chip(regmap);
 	if (ret)
-		goto err;
+		return ret;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
-	if (!data) {
-		ret = -ENOMEM;
-		goto err;
-	}
+	if (!data)
+		return -ENOMEM;
 
 	mutex_init(&data->lock);
 	data->regmap = regmap;
@@ -925,7 +921,7 @@ static int abb5zes3_probe(struct i2c_client *client,
 
 	ret = abb5zes3_rtc_check_setup(dev);
 	if (ret)
-		goto err;
+		return ret;
 
 	if (client->irq > 0) {
 		ret = devm_request_threaded_irq(dev, client->irq, NULL,
@@ -940,7 +936,7 @@ static int abb5zes3_probe(struct i2c_client *client,
 		} else {
 			dev_err(dev, "%s: irq %d unavailable (%d)\n",
 				__func__, client->irq, ret);
-			goto err;
+			return ret;
 		}
 	}
 
@@ -950,7 +946,7 @@ static int abb5zes3_probe(struct i2c_client *client,
 	if (ret) {
 		dev_err(dev, "%s: unable to register RTC device (%d)\n",
 			__func__, ret);
-		goto err;
+		goto check_irq;
 	}
 
 	/* Enable battery low detection interrupt if battery not already low */
@@ -959,12 +955,12 @@ static int abb5zes3_probe(struct i2c_client *client,
 		if (ret) {
 			dev_err(dev, "%s: enabling battery low interrupt "
 				"generation failed (%d)\n", __func__, ret);
-			goto err;
+			goto check_irq;
 		}
 	}
-
-err:
-	if (ret && data && data->irq)
+	return 0;
+check_irq:
+	if (data->irq)
 		device_init_wakeup(dev, false);
 	return ret;
 }
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300277 — [PATCH 6/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_read_timer()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 08:46:50 +0100

Pass the address of the data structure element "time" directly in a call
of the function "rtc_time_to_tm" instead of an extra initialisation
for one local variable at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/rtc/rtc-ab-b5ze-s3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
index 02e3443..e3a015a 100644
--- a/drivers/rtc/rtc-ab-b5ze-s3.c
+++ b/drivers/rtc/rtc-ab-b5ze-s3.c
@@ -326,7 +326,7 @@ static int _abb5zes3_rtc_read_timer(struct device *dev,
 				    struct rtc_wkalrm *alarm)
 {
 	struct abb5zes3_rtc_data *data = dev_get_drvdata(dev);
-	struct rtc_time rtc_tm, *alarm_tm = &alarm->time;
+	struct rtc_time rtc_tm;
 	u8 regs[ABB5ZES3_TIMA_SEC_LEN + 1];
 	unsigned long rtc_secs;
 	unsigned int reg;
@@ -362,7 +362,7 @@ static int _abb5zes3_rtc_read_timer(struct device *dev,
 		goto err;
 
 	/* ... and convert back. */
-	rtc_time_to_tm(rtc_secs + timer_secs, alarm_tm);
+	rtc_time_to_tm(rtc_secs + timer_secs, &alarm->time);
 
 	ret = regmap_read(data->regmap, ABB5ZES3_REG_CTRL2, &reg);
 	if (ret) {
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300278 — [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_set_timer()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 3 Jan 2016 09:19:32 +0100

Pass a value directly in a call of the function "regmap_update_bits"
instead of an extra initialisation for one local variable at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/rtc/rtc-ab-b5ze-s3.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
index 88f1d0b..fed52d0 100644
--- a/drivers/rtc/rtc-ab-b5ze-s3.c
+++ b/drivers/rtc/rtc-ab-b5ze-s3.c
@@ -561,7 +561,6 @@ static int _abb5zes3_rtc_set_timer(struct device *dev, struct rtc_wkalrm *alarm,
 {
 	struct abb5zes3_rtc_data *data = dev_get_drvdata(dev);
 	u8 regs[ABB5ZES3_TIMA_SEC_LEN];
-	u8 mask = ABB5ZES3_REG_TIM_CLK_TAC0 | ABB5ZES3_REG_TIM_CLK_TAC1;
 	int ret;
 
 	/* Program given number of seconds to Timer A registers */
@@ -575,7 +574,9 @@ static int _abb5zes3_rtc_set_timer(struct device *dev, struct rtc_wkalrm *alarm,
 
 	/* Configure Timer A as a watchdog timer */
 	ret = regmap_update_bits(data->regmap, ABB5ZES3_REG_TIM_CLK,
-				 mask, ABB5ZES3_REG_TIM_CLK_TAC1);
+				 ABB5ZES3_REG_TIM_CLK_TAC0
+				 | ABB5ZES3_REG_TIM_CLK_TAC1,
+				 ABB5ZES3_REG_TIM_CLK_TAC1);
 	if (ret)
 		dev_err(dev, "%s: failed to update timer\n", __func__);
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300295 — Re: [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_set_timer()

On Sun, 3 Jan 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 3 Jan 2016 09:19:32 +0100
> 
> Pass a value directly in a call of the function "regmap_update_bits"
> instead of an extra initialisation for one local variable at the beginning.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/rtc/rtc-ab-b5ze-s3.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
> index 88f1d0b..fed52d0 100644
> --- a/drivers/rtc/rtc-ab-b5ze-s3.c
> +++ b/drivers/rtc/rtc-ab-b5ze-s3.c
> @@ -561,7 +561,6 @@ static int _abb5zes3_rtc_set_timer(struct device *dev, struct rtc_wkalrm *alarm,
>  {
>  	struct abb5zes3_rtc_data *data = dev_get_drvdata(dev);
>  	u8 regs[ABB5ZES3_TIMA_SEC_LEN];
> -	u8 mask = ABB5ZES3_REG_TIM_CLK_TAC0 | ABB5ZES3_REG_TIM_CLK_TAC1;
>  	int ret;
>  
>  	/* Program given number of seconds to Timer A registers */
> @@ -575,7 +574,9 @@ static int _abb5zes3_rtc_set_timer(struct device *dev, struct rtc_wkalrm *alarm,
>  
>  	/* Configure Timer A as a watchdog timer */
>  	ret = regmap_update_bits(data->regmap, ABB5ZES3_REG_TIM_CLK,
> -				 mask, ABB5ZES3_REG_TIM_CLK_TAC1);
> +				 ABB5ZES3_REG_TIM_CLK_TAC0
> +				 | ABB5ZES3_REG_TIM_CLK_TAC1,
> +				 ABB5ZES3_REG_TIM_CLK_TAC1);

This doesn't seem like an improvement.  The concept (mask) has 
disappeared, the binary operation is strangely broken, and the function 
call has one more line of arguments, which all look sort of the same and 
thus are hard to understand.

Don't underestimate the value of naming things.

julia

>  	if (ret)
>  		dev_err(dev, "%s: failed to update timer\n", __func__);
>  
> -- 
> 2.6.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1300341 — Re: [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_set_timer()

[Multipart message — attachments visible in raw view] — view raw

On Sun, 3 Jan 2016, SF Markus Elfring wrote:

> >>  	ret = regmap_update_bits(data->regmap, ABB5ZES3_REG_TIM_CLK,
> >> -				 mask, ABB5ZES3_REG_TIM_CLK_TAC1);
> >> +				 ABB5ZES3_REG_TIM_CLK_TAC0
> >> +				 | ABB5ZES3_REG_TIM_CLK_TAC1,
> >> +				 ABB5ZES3_REG_TIM_CLK_TAC1);
> > 
> > This doesn't seem like an improvement.
> 
> Interesting …
> 
> 
> > The concept (mask) has disappeared,
> 
> I suggest to drop another local variable.
> Can the operator "Bitwise OR" be sufficient to indicate the concept "mask"?
> 
> 
> > the binary operation is strangely broken,
> 
> Do you prefer an other source code formatting within the usual line length range?
> 
> 
> > and the function call has one more line of arguments,
> 
> How should several long preprocessor symbols be combined together with indentation
> so that they will fit into the limit of 80 characters?
> 
> 
> > which all look sort of the same and thus are hard to understand.
> 
> Is this an usual consequence from an ordinary name pattern?

The original code was better.  No 80 character problem, easy to 
distinguish one argument from another, moderately meaningful variable 
name, etc.

julia

[toc] | [prev] | [next] | [standalone]

#1300345 — Re: [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_set_timer()

>>  	ret = regmap_update_bits(data->regmap, ABB5ZES3_REG_TIM_CLK,
>> -				 mask, ABB5ZES3_REG_TIM_CLK_TAC1);
>> +				 ABB5ZES3_REG_TIM_CLK_TAC0
>> +				 | ABB5ZES3_REG_TIM_CLK_TAC1,
>> +				 ABB5ZES3_REG_TIM_CLK_TAC1);
> 
> This doesn't seem like an improvement.

Interesting …


> The concept (mask) has disappeared,

I suggest to drop another local variable.
Can the operator "Bitwise OR" be sufficient to indicate the concept "mask"?


> the binary operation is strangely broken,

Do you prefer an other source code formatting within the usual line length range?


> and the function call has one more line of arguments,

How should several long preprocessor symbols be combined together with indentation
so that they will fit into the limit of 80 characters?


> which all look sort of the same and thus are hard to understand.

Is this an usual consequence from an ordinary name pattern?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web