| 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
| ||
|
Date: Thu, 14 May 2015 09:06:23 +0300
From: Pantelis Antoniou <panto@...oniou-consulting.com>
To: Maxime Ripard <maxime.ripard@...e-electrons.com>
Cc: Wolfram Sang <wsa@...-dreams.de>,
Matt Porter <mporter@...sulko.com>,
Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
linux-i2c@...r.kernel.org, Koen Kooi <koen@...inion.thruhere.net>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] i2c: eeprom: at24: Provide an EEPROM framework interface
> On May 13, 2015, at 23:36 , Maxime Ripard <maxime.ripard@...e-electrons.com> wrote:
>
> Hi Pantelis,
>
> On Wed, May 13, 2015 at 10:38:17AM +0300, Pantelis Antoniou wrote:
>> For DT and in-kernel users there is no interface to the
>> at24 EEPROMs so provide an EEPROM framework interface.
>>
>> This allows us to use AT24 based EEPROMs and reference them
>> from within the DT tree.
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@...sulko.com>
>
> You should probably mention that the EEPROM framework hasn't been
> merged yet.
>
Well, that’s true. But it’s on version 4 and it does do what I need it
to do, namely provide a DT interface for in-kernel users.
>> ---
>> drivers/misc/eeprom/at24.c | 222 +++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 174 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
>> index 2d3db81..afa719a 100644
>> --- a/drivers/misc/eeprom/at24.c
>> +++ b/drivers/misc/eeprom/at24.c
>> @@ -3,6 +3,7 @@
>> *
>> * Copyright (C) 2005-2007 David Brownell
>> * Copyright (C) 2008 Wolfram Sang, Pengutronix
>> + * Copyright (C) 2015 Pantelis Antoniou, Konsulko Group
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License as published by
>> @@ -23,6 +24,9 @@
>> #include <linux/of.h>
>> #include <linux/i2c.h>
>> #include <linux/platform_data/at24.h>
>> +#include <linux/regmap.h>
>> +#include <linux/eeprom-provider.h>
>> +#include <linux/io.h>
>>
>> /*
>> * I2C EEPROMs from most vendors are inexpensive and mostly interchangeable.
>> @@ -63,12 +67,16 @@ struct at24_data {
>> * but not from changes by other I2C masters.
>> */
>> struct mutex lock;
>> - struct bin_attribute bin;
>>
>> u8 *writebuf;
>> unsigned write_max;
>> unsigned num_addresses;
>>
>> + struct regmap_config *regmap_config;
>> + struct regmap *regmap;
>> + struct eeprom_config *eeprom_config;
>> + struct eeprom_device *eeprom_dev;
>> +
>> /*
>> * Some chips tie up multiple I2C addresses; dummy devices reserve
>> * them for us, and we'll use them with SMBus calls.
>> @@ -131,6 +139,8 @@ static const struct i2c_device_id at24_ids[] = {
>> };
>> MODULE_DEVICE_TABLE(i2c, at24_ids);
>>
>> +static DEFINE_IDA(at24_ida);
>> +
>> /*-------------------------------------------------------------------------*/
>>
>> /*
>> @@ -301,17 +311,6 @@ static ssize_t at24_read(struct at24_data *at24,
>> return retval;
>> }
>>
>> -static ssize_t at24_bin_read(struct file *filp, struct kobject *kobj,
>> - struct bin_attribute *attr,
>> - char *buf, loff_t off, size_t count)
>> -{
>> - struct at24_data *at24;
>> -
>> - at24 = dev_get_drvdata(container_of(kobj, struct device, kobj));
>> - return at24_read(at24, buf, off, count);
>> -}
>> -
>> -
>> /*
>> * Note that if the hardware write-protect pin is pulled high, the whole
>> * chip is normally write protected. But there are plenty of product
>> @@ -432,21 +431,10 @@ static ssize_t at24_write(struct at24_data *at24, const char *buf, loff_t off,
>> return retval;
>> }
>>
>> -static ssize_t at24_bin_write(struct file *filp, struct kobject *kobj,
>> - struct bin_attribute *attr,
>> - char *buf, loff_t off, size_t count)
>> -{
>> - struct at24_data *at24;
>> -
>> - if (unlikely(off >= attr->size))
>> - return -EFBIG;
>> -
>> - at24 = dev_get_drvdata(container_of(kobj, struct device, kobj));
>> - return at24_write(at24, buf, off, count);
>> -}
>> -
>> /*-------------------------------------------------------------------------*/
>>
>> +/* panto: using the EEPROM framework macc is superfluous */
>> +
>
> Is it? it was kind of one of the point to be able to remove the memory
> accessors stuff (which is only used by that driver btw).
>
Yes it completely redundant now. But it’s not my call to remove it, that would
be Wolfram.
>> /*
>> * This lets other kernel code access the eeprom data. For example, it
>> * might hold a board's Ethernet address, or board-specific calibration
>> @@ -492,6 +480,91 @@ static void at24_get_ofdata(struct i2c_client *client,
>> { }
>> #endif /* CONFIG_OF */
>>
>> +static int regmap_at24_read(void *context,
>> + const void *reg, size_t reg_size,
>> + void *val, size_t val_size)
>> +{
>> + struct i2c_client *client = context;
>> + struct at24_data *at24;
>> + unsigned int offset;
>> + int ret;
>> +
>> + /* reg bits is hardcoded to 32 bits */
>> + BUG_ON(reg_size != 4);
>> + offset = __raw_readl(reg);
>> +
>> + at24 = i2c_get_clientdata(client);
>> + if (at24 == NULL)
>> + return -ENODEV;
>> +
>> + /* val bytes is always 1 */
>> + BUG_ON(at24->regmap_config->val_bits != 8);
>> +
>> + ret = at24_read(at24, val, offset, val_size);
>> + if (ret < 0)
>> + return ret;
>> + if (ret != val_size)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> +static int regmap_at24_gather_write(void *context,
>> + const void *reg, size_t reg_size,
>> + const void *val, size_t val_size)
>> +{
>> + struct i2c_client *client = context;
>> + struct at24_data *at24;
>> + unsigned int offset;
>> + int ret;
>> +
>> + at24 = i2c_get_clientdata(client);
>> + if (at24 == NULL)
>> + return -ENODEV;
>> +
>> + BUG_ON(reg_size != 4);
>> + offset = __raw_readl(reg);
>> +
>> + /* val bytes is always 1 */
>> + BUG_ON(at24->regmap_config->val_bits != 8);
>> +
>> + ret = at24_write(at24, val, offset, val_size);
>> + if (ret < 0)
>> + return ret;
>> + if (ret != val_size)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> +static int regmap_at24_write(void *context, const void *data, size_t count)
>> +{
>> + struct i2c_client *client = context;
>> + struct at24_data *at24;
>> + unsigned int reg_bytes, offset;
>> +
>> + at24 = i2c_get_clientdata(client);
>> + if (at24 == NULL)
>> + return -ENODEV;
>> +
>> + reg_bytes = at24->regmap_config->reg_bits / 8;
>> + offset = reg_bytes;
>> +
>> + BUG_ON(reg_bytes != 4);
>> + BUG_ON(count <= offset);
>> +
>> + return regmap_at24_gather_write(context, data, reg_bytes,
>> + data + offset, count - offset);
>> +}
>> +
>> +static struct regmap_bus regmap_at24_bus = {
>> + .read = regmap_at24_read,
>> + .write = regmap_at24_write,
>> + .gather_write = regmap_at24_gather_write,
>> + .reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
>> + .val_format_endian_default = REGMAP_ENDIAN_NATIVE,
>
> I wouldn't expect the content of the EEPROM to change of endianness
> when we change the kernel’s
It doesn’t matter really; these EEPROMs are byte accessible, and the register
values are completely internal to the driver.
The native format saves a few cycles converting in case you run on a wrong endian
architecture.
> .
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
Regards
— Pantelis
--
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