On Sun, Apr 13, 2014 at 08:12:57PM +0530, RAGHAVENDRA GANIGA wrote:
> +static const struct spi_device_id ds1343_id[] = {
> + { "ds1343", 0 },
> + { "ds1344", 1 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(spi, ds1343_id);
If the 0 and 1 mean anything there they should have #defines, otherwise
just omit them.
> +static int ds1343_get_reg(struct device *dev, unsigned char address,
> + unsigned char *buf)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> +
> + /* MSB of the spi address _
> + in this rtc should be zero for read operation R/W */
> + *buf = address;
> +
> + return spi_write_then_read(spi, buf, 1, buf, 1);
> +}
This and the set_reg() function look like you should be using regmap.
> +static int ds1343_ioctl(struct device *dev, unsigned int cmd, unsigned long
> arg)
> +{
> + switch (cmd) {
> +#ifdef RTC_SET_CHARGE
> + case RTC_SET_CHARGE:
> + {
> + int val;
> +
> + if (copy_from_user(&val, (int __user *)arg, sizeof(int)))
> + return -EFAULT;
> +
> + return ds1343_set_reg(dev, DS1343_TRICKLE_REG, val);
> + }
> + break;
> +#endif
> + }
What defines this? I notice that ds1302 also does this - is this device
different enouh to need a separate driver?
> +static irqreturn_t ds1343_irq(int irq, void *dev_id)
> +{
> + struct ds1343_priv *priv = dev_id;
> +
> + disable_irq_nosync(irq);
> + schedule_work(&priv->work);
> + return IRQ_HANDLED;
> +}
Use request_threaded_irq() rather than open coding it.
signature.asc
Description: Digital signature
