| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20081105014100.419e22fd@i1501.lan.towertech.it>
Date: Wed, 5 Nov 2008 01:41:00 +0100
From: Alessandro Zummo <alessandro.zummo@...ertech.it>
To: Mark Jackson <mpfj@...c.co.uk>
Cc: dbrownell@...rs.sourceforge.net,
lkml <linux-kernel@...r.kernel.org>, rtc-linux@...glegroups.com,
spi-devel-general@...ts.sourceforge.net
Subject: Re: [PATCH v5] Add Dallas DS1390/93/94 RTC chips
On Tue, 04 Nov 2008 16:35:39 +0000
Mark Jackson <mpfj@...c.co.uk> wrote:
> v5 of this patch with even more code tidying as per previous comments.
> Also only a single tx/rx buffer is used.
> Now uses spi_write_then_read()
> Comments changed to include extra chips in the family
>
> This patch adds support for the Dallas DS1390/93/94 SPI RTC chip.
Hi Mark, almost ok, it fails checkpatch.pl on a couple of points .
please find some notes below:
> Signed-off-by: Mark Jackson <mpfj@...c.co.uk>
> ---
> drivers/rtc/Kconfig | 11 +++
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-ds1390.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 229 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 814f49f..4e10716 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -292,6 +292,17 @@ config RTC_DRV_DS1305
> This driver can also be built as a module. If so, the module
> will be called rtc-ds1305.
>
> +config RTC_DRV_DS1390
> + tristate "Dallas/Maxim DS1390/93/94"
> + help
> + If you say yes here you get support for the DS1390/93/94 chips.
> +
> + This driver only supports the RTC feature, and not other chip
> + features such as alarms and trickle charging.
> +
> + This driver can also be built as a module. If so, the module
> + will be called rtc-ds1390.
> +
> config RTC_DRV_MAX6902
> tristate "Maxim MAX6902"
> help
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index d6a9ac7..316c3ce 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_RTC_DRV_DS1302) += rtc-ds1302.o
> obj-$(CONFIG_RTC_DRV_DS1305) += rtc-ds1305.o
> obj-$(CONFIG_RTC_DRV_DS1307) += rtc-ds1307.o
> obj-$(CONFIG_RTC_DRV_DS1374) += rtc-ds1374.o
> +obj-$(CONFIG_RTC_DRV_DS1390) += rtc-ds1390.o
> obj-$(CONFIG_RTC_DRV_DS1511) += rtc-ds1511.o
> obj-$(CONFIG_RTC_DRV_DS1553) += rtc-ds1553.o
> obj-$(CONFIG_RTC_DRV_DS1672) += rtc-ds1672.o
> diff --git a/drivers/rtc/rtc-ds1390.c b/drivers/rtc/rtc-ds1390.c
> new file mode 100644
> index 0000000..4ba9b02
> --- /dev/null
> +++ b/drivers/rtc/rtc-ds1390.c
> @@ -0,0 +1,217 @@
> +/*
> + * rtc-ds1390.c -- driver for DS1390/93/94
> + *
> + * Copyright (C) 2008 Mercury IMC Ltd
> + * Written by Mark Jackson <mpfj@...c.co.uk>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * NOTE : Currently this driver only supports the bare minimum for read
> + * and write the RTC. The extra features provided by the chip family
> + * (alarms, trickle charger, different control registers) are unavailable.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/rtc.h>
> +#include <linux/spi/spi.h>
> +#include <linux/bcd.h>
I think you need at least linux/module.h for the MODULE_XXX macros
and linux/init.h for __devinit . they are probably included along
the way, but it's more correct to declare includes for the things
you are going to use.
> +#define DS1390_REG_100THS 0x00
> +#define DS1390_REG_SECONDS 0x01
> +#define DS1390_REG_MINUTES 0x02
> +#define DS1390_REG_HOURS 0x03
> +#define DS1390_REG_DAY 0x04
> +#define DS1390_REG_DATE 0x05
> +#define DS1390_REG_MONTH_CENT 0x06
> +#define DS1390_REG_YEAR 0x07
> +
> +#define DS1390_REG_ALARM_100THS 0x08
> +#define DS1390_REG_ALARM_SECONDS 0x09
> +#define DS1390_REG_ALARM_MINUTES 0x0A
> +#define DS1390_REG_ALARM_HOURS 0x0B
> +#define DS1390_REG_ALARM_DAY_DATE 0x0C
> +
> +#define DS1390_REG_CONTROL 0x0D
> +#define DS1390_REG_STATUS 0x0E
> +#define DS1390_REG_TRICKLE 0x0F
> +
> +struct ds1390 {
> + struct rtc_device *rtc;
> + u8 txrx_buf[9]; /* cmd + 8 registers */
> +};
> +
> +static void ds1390_set_reg(struct device *dev, unsigned char address,
> + unsigned char data)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> + struct ds1390 *chip = dev_get_drvdata(dev);
> +
> + /* Set MSB to indicate write */
> + chip->txrx_buf[0] = address | 0x80;
> + chip->txrx_buf[1] = data;
> +
> + /* do the i/o */
> + spi_write_then_read(spi, chip->txrx_buf, 2, NULL, 0);
> +}
> +
> +static int ds1390_get_reg(struct device *dev, unsigned char address,
> + unsigned char *data)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> + struct ds1390 *chip = dev_get_drvdata(dev);
> + int status;
> +
> + /* Clear MSB to indicate read */
> + chip->txrx_buf[0] = address & 0x7f;
> + /* do the i/o */
> + status = spi_write_then_read(spi, chip->txrx_buf, 1, chip->txrx_buf, 1);
> + if (status != 0)
> + return status;
> +
> + *data = chip->txrx_buf[1];
> +
> + return 0;
> +}
> +
> +static int ds1390_get_datetime(struct device *dev, struct rtc_time *dt)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> + struct ds1390 *chip = dev_get_drvdata(dev);
> + int status;
> +
> + /* build the message */
> + chip->txrx_buf[0] = DS1390_REG_SECONDS;
> +
> + /* do the i/o */
> + status = spi_write_then_read(spi, chip->txrx_buf, 1, chip->txrx_buf, 8);
> + if (status != 0)
> + return status;
> +
> + /* The chip sends data in this order:
> + * Seconds, Minutes, Hours, Day, Date, Month / Century, Year */
> + dt->tm_sec = bcd2bin(chip->txrx_buf[0]);
> + dt->tm_min = bcd2bin(chip->txrx_buf[1]);
> + dt->tm_hour = bcd2bin(chip->txrx_buf[2]);
> + dt->tm_wday = bcd2bin(chip->txrx_buf[3]);
> + dt->tm_mday = bcd2bin(chip->txrx_buf[4]);
> + /* mask off century bit */
> + dt->tm_mon = bcd2bin(chip->txrx_buf[5] & 0x7f) - 1;
> + /* adjust for century bit */
> + dt->tm_year = bcd2bin(chip->txrx_buf[6]) + ((chip->txrx_buf[5] & 0x80) ? 100 : 0);
> +
> + return rtc_valid_tm(dt);
> +}
> +
> +static int ds1390_set_datetime(struct device *dev, struct rtc_time *dt)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> + struct ds1390 *chip = dev_get_drvdata(dev);
> +
> + /* build the message */
> + chip->txrx_buf[0] = DS1390_REG_SECONDS | 0x80;
> + chip->txrx_buf[1] = bin2bcd(dt->tm_sec);
> + chip->txrx_buf[2] = bin2bcd(dt->tm_min);
> + chip->txrx_buf[3] = bin2bcd(dt->tm_hour);
> + chip->txrx_buf[4] = bin2bcd(dt->tm_wday);
> + chip->txrx_buf[5] = bin2bcd(dt->tm_mday);
> + chip->txrx_buf[6] = bin2bcd(dt->tm_mon + 1) |
> + ((dt->tm_year > 99) ? 0x80 : 0x00);
> + chip->txrx_buf[7] = bin2bcd(dt->tm_year % 100);
> +
> + /* do the i/o */
> + return spi_write_then_read(spi, chip->txrx_buf, 8, NULL, 0);
> +}
> +
> +static int ds1390_read_time(struct device *dev, struct rtc_time *tm)
> +{
> + return ds1390_get_datetime(dev, tm);
> +}
> +
> +static int ds1390_set_time(struct device *dev, struct rtc_time *tm)
> +{
> + return ds1390_set_datetime(dev, tm);
> +}
> +
> +static const struct rtc_class_ops ds1390_rtc_ops = {
> + .read_time = ds1390_read_time,
> + .set_time = ds1390_set_time,
> +};
> +
> +static int __devinit ds1390_probe(struct spi_device *spi)
> +{
> + struct rtc_device *rtc;
> + unsigned char tmp;
> + struct ds1390 *chip;
> + int res;
> +
> + printk(KERN_INFO "DS1390 SPI RTC driver\n");
> +
> + rtc = rtc_device_register("ds1390",
> + &spi->dev, &ds1390_rtc_ops, THIS_MODULE);
> + if (IS_ERR(rtc)) {
> + printk(KERN_ALERT "DS1390: unable to register device\n");
please use dev_err instead of printk.
> + return PTR_ERR(rtc);
> + }
> +
> + spi->mode = SPI_MODE_3;
> + spi->bits_per_word = 8;
> + spi_setup(spi);
> +
> + chip = kzalloc(sizeof *chip, GFP_KERNEL);
> + if (!chip) {
> + printk(KERN_ALERT "DS1390: unable to allocate device memory\n");
> + rtc_device_unregister(rtc);
> + return -ENOMEM;
> + }
> + chip->rtc = rtc;
> + dev_set_drvdata(&spi->dev, chip);
> + res = ds1390_get_reg(&spi->dev, DS1390_REG_SECONDS, &tmp);
> + if (res) {
> + printk(KERN_ALERT "DS1390: unable to read device\n");
dev_err
> + rtc_device_unregister(rtc);
> + return res;
you need to kfree(chip) here.
you might want to allocate chip at the beginning of this function,
initialize your chip structure, check that you are able to read and only
then you should register the rtc.
> + }
> +
> + return 0;
> +}
> +
> +static int __devexit ds1390_remove(struct spi_device *spi)
> +{
> + struct ds1390 *chip = platform_get_drvdata(spi);
> + struct rtc_device *rtc = chip->rtc;
> +
> + if (rtc)
no need to check rtc here
> + rtc_device_unregister(rtc);
> +
> + kfree(chip);
> +
> + return 0;
> +}
> +
> +static struct spi_driver ds1390_driver = {
> + .driver = {
> + .name = "rtc-ds1390",
> + .owner = THIS_MODULE,
> + },
> + .probe = ds1390_probe,
> + .remove = __devexit_p(ds1390_remove),
> +};
> +
> +static __init int ds1390_init(void)
> +{
> + return spi_register_driver(&ds1390_driver);
> +}
> +module_init(ds1390_init);
> +
> +static __exit void ds1390_exit(void)
> +{
> + spi_unregister_driver(&ds1390_driver);
> +}
> +module_exit(ds1390_exit);
> +
> +MODULE_DESCRIPTION("DS1390/93/94 SPI RTC driver");
> +MODULE_AUTHOR("Mark Jackson <mpfj@...c.co.uk>");
> +MODULE_LICENSE("GPL");
--
Best regards,
Alessandro Zummo,
Tower Technologies - Torino, Italy
http://www.towertech.it
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists