Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 14 Sep 2022 17:08:49 +0200
From:   Alexandre Belloni <alexandre.belloni@bootlin.com>
To:     Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc:     Alessandro Zummo <a.zummo@towertech.it>, linux-rtc@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/6] rtc: isl12022: simplify some expressions
Message-ID: <YyHugb47cJPNuHbs@mail.local>
References: <20220830100152.698506-1-linux@rasmusvillemoes.dk>
 <20220830100152.698506-3-linux@rasmusvillemoes.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220830100152.698506-3-linux@rasmusvillemoes.dk>
Precedence: bulk

Hi,

On 30/08/2022 12:01:48+0200, Rasmus Villemoes wrote:
> These instances of '&client->dev' might as well be spelled 'dev', since
> 'client' has been computed from 'dev' via 'client =
> to_i2c_client(dev)'.
> 
> Later patches will get rid of that local variable 'client', so remove
> these unnecessary references so those later patches become easier to
> read.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  drivers/rtc/rtc-isl12022.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
> index 2dc19061cf5f..5e6bb9153c89 100644
> --- a/drivers/rtc/rtc-isl12022.c
> +++ b/drivers/rtc/rtc-isl12022.c
> @@ -112,13 +112,13 @@ static int isl12022_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  		return ret;
>  
>  	if (buf[ISL12022_REG_SR] & (ISL12022_SR_LBAT85 | ISL12022_SR_LBAT75)) {
> -		dev_warn(&client->dev,
> +		dev_warn(dev,

While at it, I would prefer that one to also become a dev_dbg

>  			 "voltage dropped below %u%%, "
>  			 "date and time is not reliable.\n",
>  			 buf[ISL12022_REG_SR] & ISL12022_SR_LBAT85 ? 85 : 75);
>  	}
>  
> -	dev_dbg(&client->dev,
> +	dev_dbg(dev,
>  		"%s: raw data is sec=%02x, min=%02x, hr=%02x, "
>  		"mday=%02x, mon=%02x, year=%02x, wday=%02x, "
>  		"sr=%02x, int=%02x",
> @@ -141,7 +141,7 @@ static int isl12022_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	tm->tm_mon = bcd2bin(buf[ISL12022_REG_MO] & 0x1F) - 1;
>  	tm->tm_year = bcd2bin(buf[ISL12022_REG_YR]) + 100;
>  
> -	dev_dbg(&client->dev, "%s: secs=%d, mins=%d, hours=%d, "
> +	dev_dbg(dev, "%s: secs=%d, mins=%d, hours=%d, "

You probably want to convert that to %ptR

>  		"mday=%d, mon=%d, year=%d, wday=%d\n",
>  		__func__,
>  		tm->tm_sec, tm->tm_min, tm->tm_hour,
> @@ -158,7 +158,7 @@ static int isl12022_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	int ret;
>  	uint8_t buf[ISL12022_REG_DW + 1];
>  
> -	dev_dbg(&client->dev, "%s: secs=%d, mins=%d, hours=%d, "
> +	dev_dbg(dev, "%s: secs=%d, mins=%d, hours=%d, "

ditto

>  		"mday=%d, mon=%d, year=%d, wday=%d\n",
>  		__func__,
>  		tm->tm_sec, tm->tm_min, tm->tm_hour,
> @@ -173,8 +173,7 @@ static int isl12022_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  		/* Check if WRTC (write rtc enable) is set factory default is
>  		 * 0 (not set) */
>  		if (!(buf[0] & ISL12022_INT_WRTC)) {
> -			dev_info(&client->dev,
> -				 "init write enable and 24 hour format\n");
> +			dev_info(dev, "init write enable and 24 hour format\n");

I guess that one should simply be removed

>  
>  			/* Set the write enable bit. */
>  			ret = isl12022_write_reg(client,
> -- 
> 2.37.2
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com