| 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: <20070412070854.GA32440@2ka.mipt.ru>
Date: Thu, 12 Apr 2007 11:08:54 +0400
From: Evgeniy Polyakov <johnpol@....mipt.ru>
To: Anton Vorontsov <cbou@...l.ru>
Cc: linux-kernel@...r.kernel.org, kernel-discuss@...dhelds.org
Subject: Re: [PATCH 5/7] [RFC] ds2760 W1 slave
On Thu, Apr 12, 2007 at 03:25:17AM +0400, Anton Vorontsov (cbou@...l.ru) wrote:
> This is W1 slave for ds2760 chip, found inside almost every HP iPaq and
> HTC PDAs/phones.
Hi Anton, I have some cleanup-related comments, patch itself looks good.
> diff --git a/drivers/w1/slaves/w1_ds2760.c b/drivers/w1/slaves/w1_ds2760.c
> new file mode 100644
> index 0000000..21b0ef6
> --- /dev/null
> +++ b/drivers/w1/slaves/w1_ds2760.c
> @@ -0,0 +1,162 @@
> +/*
> + * 1-Wire implementation for the ds2760 chip
> + *
> + * Copyright (c) 2004-2005, Szabolcs Gyurko <szabolcs.gyurko@....hu>
> + *
> + * Use consistent with the GNU GPL is permitted,
> + * provided that this copyright notice is
> + * preserved in its entirety in all copies and derived works.
> + *
> + */
> +
> +#include <asm/types.h>
Is it really needed?
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/device.h>
> +#include <linux/types.h>
> +#include <linux/jiffies.h>
> +
> +#include "../w1.h"
> +#include "../w1_int.h"
> +#include "../w1_family.h"
> +#include "w1_ds2760.h"
> +
> +static int w1_ds2760_io(struct device *dev, char *buf, int addr, size_t count,
> + int io)
> +{
> + struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
> +
> + if (!dev)
> + return 0;
> +
> + mutex_lock(&sl->master->mutex);
> +
> + if (addr > DS2760_DATA_SIZE || addr < 0) {
> + count = 0;
> + goto out;
> + }
> + if (addr + count > DS2760_DATA_SIZE)
> + count = DS2760_DATA_SIZE - addr;
> +
> + if (!w1_reset_select_slave(sl)) {
> + if (!io) {
> + w1_write_8(sl->master, W1_DS2760_READ_DATA);
> + w1_write_8(sl->master, addr);
> + count = w1_read_block(sl->master, buf, count);
> + } else {
> + w1_write_8(sl->master, W1_DS2760_WRITE_DATA);
> + w1_write_8(sl->master, addr);
> + w1_write_block(sl->master, buf, count);
> + /* XXX w1_write_block returns void, not n_written */
> + }
> + }
> +
> +out:
> + mutex_unlock(&sl->master->mutex);
> +
> + return count;
> +}
> +
> +int w1_ds2760_read(struct device *dev, char *buf, int addr, size_t count)
> +{
> + return w1_ds2760_io(dev, buf, addr, count, 0);
> +}
> +
> +int w1_ds2760_write(struct device *dev, char *buf, int addr, size_t count)
> +{
> + return w1_ds2760_io(dev, buf, addr, count, 1);
> +}
Both are exported, who use them?
> +/* io = 0 means copy from EEPROM to SRAM, 1 means from SRAM to EEPROM */
> +static int w1_ds2760_eeprom(struct device *dev, int addr, int io)
> +{
> + struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
> + int ret = 0;
> +
> + mutex_lock(&sl->master->mutex);
> +
> + if (!w1_reset_select_slave(sl)) {
> + if (!io)
> + w1_write_8(sl->master, W1_DS2760_RECALL_DATA);
> + else
> + w1_write_8(sl->master, W1_DS2760_COPY_DATA);
> + w1_write_8(sl->master, addr);
> + }
> +
> + mutex_unlock(&sl->master->mutex);
> +
> + return ret;
> +}
> +
> +int w1_ds2760_recall(struct device *dev, int addr)
> +{
> + return w1_ds2760_eeprom(dev, addr, 0);
> +}
> +
> +int w1_ds2760_copy(struct device *dev, int addr)
> +{
> + return w1_ds2760_eeprom(dev, addr, 1);
> +}
The same.
> +static ssize_t w1_ds2760_read_bin(struct kobject *kobj, char *buf, loff_t off,
> + size_t count)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + return w1_ds2760_read(dev, buf, off, count);
> +}
> +
> +static struct bin_attribute w1_ds2760_bin_attr = {
> + .attr = {
> + .name = "w1_slave",
> + .mode = S_IRUGO,
> + .owner = THIS_MODULE,
> + },
> + .size = DS2760_DATA_SIZE,
> + .read = w1_ds2760_read_bin,
> +};
> +
> +static int w1_ds2760_add_slave(struct w1_slave *sl)
> +{
> + return sysfs_create_bin_file(&sl->dev.kobj, &w1_ds2760_bin_attr);
> +}
> +
> +static void w1_ds2760_remove_slave(struct w1_slave *sl)
> +{
> + sysfs_remove_bin_file(&sl->dev.kobj, &w1_ds2760_bin_attr);
> +}
> +
> +static struct w1_family_ops w1_ds2760_fops = {
> + .add_slave = w1_ds2760_add_slave,
> + .remove_slave = w1_ds2760_remove_slave,
> +};
> +
> +static struct w1_family w1_ds2760_family = {
> + .fid = W1_FAMILY_DS2760,
> + .fops = &w1_ds2760_fops,
> +};
> +
> +static int __init w1_ds2760_init(void)
> +{
> + printk("1-Wire driver for the DS2760 battery monitor chip "
> + " - (c) 2004-2005, Szabolcs Gyurko\n");
> + return w1_register_family(&w1_ds2760_family);
> +}
Add something like KERN_INFO into printk.
--
Evgeniy Polyakov
-
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