Re: [PATCHv5,RESEND] rtc: Add support for Intersil ISL12057 I2C RTC chip

[Mark Brown <broonie@xxxxxxxxxx>]

On Tue, Dec 17, 2013 at 08:07:28PM +0100, Arnaud Ebalard wrote:
> 
> Intersil ISL12057 is an I2C RTC chip also supporting two alarms. This
> patch only adds support for basic RTC functionalities (i.e. getting and
> setting time). Tests have been performed on NETGEAR ReadyNAS 102 w/
> startup/shutdown scripts, hwclock, ntpdate and openntpd.

Reviewed-by: Mark Brown <broonie@xxxxxxxxxx>

but

> +static int isl12057_rtc_proc(struct device *dev, struct seq_file *seq)
> +{
> +	struct isl12057_rtc_data *data = dev_get_drvdata(dev);
> +	int reg, ret;
> +
> +	mutex_lock(&data->lock);
> +
> +	/* Status register */
> +	ret = regmap_read(data->regmap, ISL12057_REG_SR, &reg);
> +	if (ret) {
> +		dev_err(dev, "%s: reading SR failed\n", __func__);
> +		goto out;
> +	}
> +
> +	seq_printf(seq, "status_reg\t:%s%s%s (0x%.2x)\n",
> +		   (reg & ISL12057_REG_SR_OSF) ? " OSF" : "",
> +		   (reg & ISL12057_REG_SR_A1F) ? " A1F" : "",
> +		   (reg & ISL12057_REG_SR_A2F) ? " A2F" : "", reg);
> +
> +	/* Control register */
> +	ret = regmap_read(data->regmap, ISL12057_REG_INT, &reg);
> +	if (ret) {
> +		dev_err(dev, "%s: reading INT failed\n", __func__);
> +		goto out;
> +	}
> +
> +	seq_printf(seq, "control_reg\t:%s%s%s%s%s%s (0x%.2x)\n",
> +		   (reg & ISL12057_REG_INT_A1IE) ?  " A1IE"  : "",
> +		   (reg & ISL12057_REG_INT_A2IE) ?  " A2IE"  : "",
> +		   (reg & ISL12057_REG_INT_INTCN) ? " INTCN" : "",
> +		   (reg & ISL12057_REG_INT_RS1) ?   " RS1"   : "",
> +		   (reg & ISL12057_REG_INT_RS2) ?   " RS2"   : "",
> +		   (reg & ISL12057_REG_INT_EOSC) ?  " EOSC"  : "", reg);
> +
> + out:
> +	mutex_unlock(&data->lock);

I'm not sure the lock is achieving anything any more - the only time it
actualy protects more than one operation is the above but since the
status register is volatile anyway it might change between it being read
and the control register being read.

Attachment: signature.asc
Description: Digital signature