Re: [PATCH] Platform real time clock driver for Dallas 1511 chip.

[Atsushi Nemoto <anemo@mba.ocn.ne.jp>]

On Tue, 4 Dec 2007 16:46:51 -0800, Andrew Sharp <andy.sharp@onstor.com> wrote:
> +struct rtc_plat_data {
> +	struct rtc_device *rtc;
> +	void __iomem *ioaddr;		/* virtual base address */
> +	unsigned long baseaddr;		/* physical base address */
> +	int size;				/* amount of memory mapped */
> +	int irq;
> +	unsigned int irqen;
> +	int alrm_sec;
> +	int alrm_min;
> +	int alrm_hour;
> +	int alrm_mday;
> +};
> +
> +static DEFINE_SPINLOCK(ds1511_lock);
> +
> +static __iomem char *ds1511_base;

If you used ds1511_base, ioaddr in rtc_plat_data is not needed.

> + static noinline void
> +rtc_write(uint8_t val, uint32_t reg)
> +{
> +	writeb(val, ds1511_base + (reg * reg_spacing));
> +}

Still "noinline" here and there.

> +static struct bin_attribute ds1511_nvram_attr = {
> +	.attr = {
> +		.name = "nvram",
> +		.mode = S_IRUGO | S_IWUGO,
> +		.owner = THIS_MODULE,
> +	},
> +	.size = DS1511_RAM_MAX,
> +	.read = ds1511_nvram_read,
> +	.write = ds1511_nvram_write,
> +};

It would be better to use S_IWUSR instead of S_IWUGO.  Please check
rtc-ds1553 changes in current git tree.

> +static struct platform_driver ds1511_rtc_driver = {
> +	.probe		= ds1511_rtc_probe,
> +	.remove		= __devexit_p(ds1511_rtc_remove),
> +	.driver		= {
> +		.name	= "ds1511",
> +		.owner	= THIS_MODULE,
> +	},
> +};

Use "rtc-ds1511" for driver name for hotplugging.  Please check
rtc-ds1553 changes in current git tree.

> + static void __exit
> +ds1511_rtc_exit(void)
> +{
> +	return platform_driver_unregister(&ds1511_rtc_driver);
> +}

Do not return void value.  Please check rtc-ds1553 changes in current
git tree.

---
Atsushi Nemoto