[rtc-linux] Re: [PATCH 2/2] RTC: s35390a: implement ioctls

[Alexandre Belloni <alexandre.belloni@free-electrons.com>]

On 12/01/2017 at 11:43:38 +0100, Fabien Lahoudere wrote :
> Implements RTC_VL_READ and RTC_VL_CLR ioctls.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> ---
>  drivers/rtc/rtc-s35390a.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> index ef4ada9..2bd8301 100644
> --- a/drivers/rtc/rtc-s35390a.c
> +++ b/drivers/rtc/rtc-s35390a.c
> @@ -121,7 +122,9 @@ static int s35390a_reset(struct s35390a *s35390a, char *status1)
>  		 * detection circuit is in operation.
>  		 */
>  		msleep(500);
> -	else if (!(*status1 & S35390A_FLAG_BLD))
> +	else if (*status1 & S35390A_FLAG_BLD)
> +		s35390a->lowvoltage = 1;
> +	else
>  		/*
>  		 * If both POC and BLD are unset everything is fine.
>  		 */
> @@ -393,12 +396,44 @@ static int s35390a_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	return s35390a_set_datetime(to_i2c_client(dev), tm);
>  }
>  
> +static int s35390a_rtc_ioctl(struct device *dev, unsigned int cmd,
> +			     unsigned long arg)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct s35390a *s35390a = i2c_get_clientdata(client);
> +	char sts;
> +	int err;
> +
> +	switch (cmd) {
> +	case RTC_VL_READ:

It seems wasteful to call s35390a_reset() every time. If
s35390a->lowvoltage is already set to 1 then you already have the answer
you need (and you already resetted the RTC)

> +		/* s35390a_reset set lowvoltage flag and init RTC if needed */
> +		err = s35390a_reset(s35390a, &sts);
> +		if (err < 0)
> +			return err;
> +		if (copy_to_user((void __user *)arg, &s35390a->lowvoltage,
> +				 sizeof(int)))
> +			return -EFAULT;
> +
> +	case RTC_VL_CLR:
> +		/* update flag and clear register */
> +		err = s35390a_reset(s35390a, &sts);
> +		if ((err == 1) || (err == 0))

Whatever the error, once the first s35390a_get_reg(), the POC and BLD
flags are lost. I would reset lowvoltage in every cases.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.