Re: [PATCH] i2c: viperboard: remove superfluous assignment

2014-01-08 Thread Lars Poeschel 
On Wednesday 08 January 2014 22:21:33, Wolfram Sang wrote:
 cppcheck rightfully says:
 
 drivers/i2c/busses/i2c-viperboard.c:169: style: Variable 'bytes_xfer' is
 assigned a value that is never used.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de
 ---
  drivers/i2c/busses/i2c-viperboard.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-viperboard.c
 b/drivers/i2c/busses/i2c-viperboard.c index 6976e1c..7533fa3 100644
 --- a/drivers/i2c/busses/i2c-viperboard.c
 +++ b/drivers/i2c/busses/i2c-viperboard.c
 @@ -118,8 +118,7 @@ static int vprbrd_i2c_addr(struct usb_device *usb_dev,
  static int vprbrd_i2c_read(struct vprbrd *vb, struct i2c_msg *msg)
  {
   int ret;
 - u16 remain_len, bytes_xfer, len1, len2,
 - start = 0x;
 + u16 remain_len, len1, len2, start = 0x;
   struct vprbrd_i2c_read_msg *rmsg =
   (struct vprbrd_i2c_read_msg *)vb-buf;
 
 @@ -166,7 +165,6 @@ static int vprbrd_i2c_read(struct vprbrd *vb, struct
 i2c_msg *msg) rmsg-header.len3 = remain_len - 512;
   rmsg-header.len4 = 0x00;
   rmsg-header.len5 = 0x00;
 - bytes_xfer = remain_len;
   remain_len = 0;
   } else if (remain_len = 1022) {
   len1 = 512;

Acked-by: Lars Poeschel poesc...@lemonage.de
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/13] i2c: viperboard: Use devm_kzalloc() functions

2013-12-19 Thread Lars Poeschel 
Am Dienstag, 17. Dezember 2013, 16:01:41 schrieb Jingoo Han:
 Use devm_kzalloc() functions to make cleanup paths simpler.
 
 Signed-off-by: Jingoo Han jg1@samsung.com
 ---
  drivers/i2c/busses/i2c-viperboard.c |   12 +++-
  1 file changed, 3 insertions(+), 9 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-viperboard.c
 b/drivers/i2c/busses/i2c-viperboard.c index c68450c..6976e1c 100644
 --- a/drivers/i2c/busses/i2c-viperboard.c
 +++ b/drivers/i2c/busses/i2c-viperboard.c
 @@ -367,7 +367,7 @@ static int vprbrd_i2c_probe(struct platform_device
 *pdev) int ret;
   int pipe;
 
 - vb_i2c = kzalloc(sizeof(*vb_i2c), GFP_KERNEL);
 + vb_i2c = devm_kzalloc(pdev-dev, sizeof(*vb_i2c), GFP_KERNEL);
   if (vb_i2c == NULL)
   return -ENOMEM;
 
 @@ -394,14 +394,12 @@ static int vprbrd_i2c_probe(struct platform_device
 *pdev) if (ret != 1) {
   dev_err(pdev-dev,
   failure setting i2c_bus_freq to %d\n, i2c_bus_freq);
 - ret = -EIO;
 - goto error;
 + return -EIO;
   }
   } else {
   dev_err(pdev-dev,
   invalid i2c_bus_freq setting:%d\n, i2c_bus_freq);
 - ret = -EIO;
 - goto error;
 + return -EIO;
   }
 
   vb_i2c-i2c.dev.parent = pdev-dev;
 @@ -412,10 +410,6 @@ static int vprbrd_i2c_probe(struct platform_device
 *pdev) platform_set_drvdata(pdev, vb_i2c);
 
   return 0;
 -
 -error:
 - kfree(vb_i2c);
 - return ret;
  }
 
  static int vprbrd_i2c_remove(struct platform_device *pdev)

Acked-by: Lars Poeschel poesc...@lemonage.de

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 1/1] gpio: mcp23s08: convert driver to DT

2013-02-06 Thread Lars Poeschel 
On Tuesday 05 February 2013 at 15:29:09, Grant Likely wrote:
 On Thu, 31 Jan 2013 21:51:36 +0100, Linus Walleij 
linus.wall...@linaro.org wrote:
  On Thu, Jan 31, 2013 at 4:58 PM, Lars Poeschel la...@wh2.tu-dresden.de 
wrote:
   --- /dev/null
   +++ b/Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt
   @@ -0,0 +1,27 @@
   +Microchip MCP2308/MCP23S08/MCP23017/MCP23S17 driver for
   +8-/16-bit I/O expander with serial interface (I2C/SPI)
   +
   +Required properties:
   +- compatible : Should be mcp,mcp23s08-gpio, mcp,mcp23s17-gpio,
   +   mcp,mcp23008-gpio or mcp,mcp23017-gpio
   +- base : The first gpio number that should be assigned by this chip.
  
  No. We do not tie the global GPIO numbers into the device tree.
  
  In the DT GPIOs are referenced by ampersand gpio0 1 2
  notation referring to the instance, so as you realize DT itself
  has no need for that number.
  
  Further it is not OS-neutral.
  
  You have to find another way to handle this in the driver code.
  In worst case: use AUXDATA.
 
 Hi Lars,
 
 The trick is to declare the io expander to be a gpio-controller and
 use the #gpio-cells property to declare how many cells (32-bit numbers)
 are need to specify a single gpio line. Most gpio controllers use
 gpio-cells=2; The first cell is the *controller local* gpio
 number, and the second cell is used for flags. That way your gpio
 controller can be referenced by other nodes in the tree with a gpios
 property.
 
 You can find lots of examples of this in the tree.

Linus, Grant, thanks for the explanations. I think I have catched where it 
should go.
The thing that confused me was, that the platform_data for the chip has a 
mandatory base member, that sets the linux global gpio number at which the 
chip should appear. A value of -1 for automatic assigning gpio number is not 
allowed, the chip will not probe.
I have to change the driver to allow at least this -1 as an additional value.
As Linus pointed out, it is not desirable to set the global gpio base number 
from device tree, right ? If I have 3 instances of this chips then, how can 
userspace sw distinguish then to which one it is talking ?

This is an example for a DT entry (for i2c version) of the chip as I am 
targetting it now:

gpiom1: gpio@20 {
reg = 0x20;
compatible = mcp,mcp23017-gpio;
gpio-controller;
#gpio-cells = 2;
};

I am working on this but I have some strange issues with the driver 
probing/not probing and kernel debug output. I hope I will solve this today. 
I will send a new patch then.

Lars
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] Convert mcp23s08 to DT usage

2013-02-06 Thread Lars Poeschel 
From: Lars Poeschel poesc...@lemonage.de

I wanted to use mcp23s08 driver with a device that boots using device tree.
I modified the driver to allow the DT usage and tested with a mcp23017
which is a i2c device. I could not test the spi path, because I have no
such device.

Regards,
Lars

Lars Poeschel (2):
  gpio: mcp23s08: Allow -1 as a legal value for global gpio base
  gpio: mcp23s08: convert driver to DT

 .../devicetree/bindings/gpio/gpio-mcp23s08.txt |   36 
 drivers/gpio/gpio-mcp23s08.c   |   95 ++--
 include/linux/spi/mcp23s08.h   |1 +
 3 files changed, 126 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] gpio: mcp23s08: Allow -1 as a legal value for global gpio base

2013-02-06 Thread Lars Poeschel 
From: Lars Poeschel poesc...@lemonage.de

Explicitly allow -1 as a legal value for the
mcp23s08_platform_data-base. This is the special value lets the
kernel choose a valid global gpio base number.

Signed-off-by: Lars Poeschel poesc...@lemonage.de
---
 drivers/gpio/gpio-mcp23s08.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c
index 3cea0ea..2afb828 100644
--- a/drivers/gpio/gpio-mcp23s08.c
+++ b/drivers/gpio/gpio-mcp23s08.c
@@ -483,7 +483,7 @@ static int mcp230xx_probe(struct i2c_client *client,
int status;
 
pdata = client-dev.platform_data;
-   if (!pdata || !gpio_is_valid(pdata-base)) {
+   if ((!pdata || !gpio_is_valid(pdata-base))  pdata-base != -1) {
dev_dbg(client-dev, invalid or missing platform data\n);
return -EINVAL;
}
@@ -570,7 +570,7 @@ static int mcp23s08_probe(struct spi_device *spi)
type = spi_get_device_id(spi)-driver_data;
 
pdata = spi-dev.platform_data;
-   if (!pdata || !gpio_is_valid(pdata-base)) {
+   if ((!pdata || !gpio_is_valid(pdata-base))  pdata-base != -1) {
dev_dbg(spi-dev, invalid or missing platform data\n);
return -EINVAL;
}
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] gpio: mcp23s08: convert driver to DT

2013-02-06 Thread Lars Poeschel 
From: Lars Poeschel poesc...@lemonage.de

This converts the mcp23s08 driver to be able to be used with device
tree.
There is a special mcp,chips property, that correspond to the chips
member of the struct mcp23s08_platform_data. It can be used for
multiple mcp23s08/mc23s17 on the same spi chipselect.

Signed-off-by: Lars Poeschel poesc...@lemonage.de
---
 .../devicetree/bindings/gpio/gpio-mcp23s08.txt |   36 
 drivers/gpio/gpio-mcp23s08.c   |   91 +++-
 include/linux/spi/mcp23s08.h   |1 +
 3 files changed, 124 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt 
b/Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt
new file mode 100644
index 000..17eb669
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt
@@ -0,0 +1,36 @@
+Microchip MCP2308/MCP23S08/MCP23017/MCP23S17 driver for
+8-/16-bit I/O expander with serial interface (I2C/SPI)
+
+Required properties:
+- compatible : Should be
+- mcp,mcp23s08 for  8 GPIO SPI version
+- mcp,mcp23s17 for 16 GPIO SPI version
+- mcp,mcp23008 for  8 GPIO I2C version or
+- mcp,mcp23017 for 16 GPIO I2C version of the chip
+- #gpio-cells : Should be two.
+  - first cell is the pin number
+  - second cell is used to specify optional parameters (currently unused)
+- gpio-controller : Marks the device node as a GPIO controller.
+- reg : For an address on its bus
+
+Optional device specific properties:
+- mcp,chips : This is a table with 2 columns and up to 8 entries. The first 
column
+   is a is_present flag, that makes only sense for SPI chips. Multiple
+   chips can share the same chipselect. This flag can be 0 or 1 depending
+   if there is a chip at this address or not.
+   The second column is written to the GPPU register, selecting a 100k
+   pullup resistor or not. Setting a 1 is activating the pullup.
+   For I2C chips only the first line in this table is used. Further chips
+   are registered at different addresses at the I2C bus.
+
+Example:
+gpiom1: gpio@20 {
+compatible = mcp,mcp23017;
+gpio-controller;
+#gpio-cells = 2;
+reg = 0x20;
+   chips = 
+   /* is_present  pullups */
+   1   0x07/* chip address 0 */
+   ;
+};
diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c
index 2afb828..9c9bc1a 100644
--- a/drivers/gpio/gpio-mcp23s08.c
+++ b/drivers/gpio/gpio-mcp23s08.c
@@ -12,6 +12,8 @@
 #include linux/spi/mcp23s08.h
 #include linux/slab.h
 #include asm/byteorder.h
+#include linux/of.h
+#include linux/of_device.h
 
 /**
  * MCP types supported by driver
@@ -473,16 +475,87 @@ fail:
 
 /*--*/
 
+#ifdef CONFIG_OF
+static struct of_device_id mcp23s08_of_match[] = {
+#ifdef CONFIG_SPI_MASTER
+   {
+   .compatible = mcp,mcp23s08,
+   },
+   {
+   .compatible = mcp,mcp23s17,
+   },
+#endif
+#if IS_ENABLED(CONFIG_I2C)
+   {
+   .compatible = mcp,mcp23008,
+   },
+   {
+   .compatible = mcp,mcp23017,
+   },
+#endif
+   { },
+};
+MODULE_DEVICE_TABLE(of, mcp23s08_of_match);
+
+static struct mcp23s08_platform_data *
+   of_get_mcp23s08_pdata(struct device *dev)
+{
+   struct mcp23s08_platform_data *pdata;
+   struct device_node *np = dev-of_node;
+   u32 chips[sizeof(pdata-chip)];
+   int ret, i, j;
+
+   pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+
+   pdata-base = -1;
+
+   for (i = ARRAY_SIZE(pdata-chip) * MCP23S08_CHIP_INFO_MEMBERS;
+   i  0; i -= MCP23S08_CHIP_INFO_MEMBERS) {
+   ret = of_property_read_u32_array(np, mcp,chips, chips, i);
+   if (ret == -EOVERFLOW)
+   continue;
+   break;
+   }
+   if (!ret) {
+   for (j = 0; j  i / MCP23S08_CHIP_INFO_MEMBERS ; j++) {
+   pdata-chip[j].is_present =
+   chips[j * MCP23S08_CHIP_INFO_MEMBERS];
+   pdata-chip[j].pullups =
+   chips[j * MCP23S08_CHIP_INFO_MEMBERS + 1];
+   }
+   }
+
+   return pdata;
+}
+#else
+static struct mcp23s08_platform_data *
+   of_get_mcp23s08_pdata(struct i2c_client *client, int *chip_id)
+{
+   return NULL;
+}
+#endif /* CONFIG_OF */
+
+
 #if IS_ENABLED(CONFIG_I2C)
 
 static int mcp230xx_probe(struct i2c_client *client,
const struct i2c_device_id *id)
 {
-   struct mcp23s08_platform_data *pdata;
+   struct mcp23s08_platform_data *pdata = NULL;
struct mcp23s08 *mcp;
int status;
+   const struct

Re: [PATCH RFC] misc/at24: distinguish between eeprom and fram chips

2013-02-06 Thread Lars Poeschel 
On Wednesday 05 December 2012 at 17:41:53, Wolfram Sang wrote:

   The method of accessing EEPROMs is used by way more chips than FRAMs.
   So, I'd prefer to have the text updated more generic like EEPROMs and
   similar devices like RAMs, ROMs, etc Describing setting .flags in
   Kconfig is overkill.
  
  A patch updating Kconfig is below.
 
 Looks good from a glimpse, will apply it later.

Hello Wolfram,

what happend to this one ? It was a patch updating Kconfig help for at24.

https://patchwork.kernel.org/patch/1840591/

Regards,
Lars
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 0/1] Convert mcp23s08 to DT usage

2013-01-31 Thread Lars Poeschel 
From: Lars Poeschel poesc...@lemonage.de

I wanted to use mcp23s08 driver with a device that boots using device tree.
Therefore I had to modify the driver to be able to take it's platform_data
from the device tree.
I am using a mcp23017 (I2C) device successfully with the patch booted over
device tree. I am not sure if I did this right and I am not able to test the
SPI part of the driver, so I am requesting for comments.

Thank you,
Lars

Lars Poeschel (1):
  gpio: mcp23s08: convert driver to DT

 .../devicetree/bindings/gpio/gpio-mcp23s08.txt |   27 ++
 drivers/gpio/gpio-mcp23s08.c   |   93 +++-
 include/linux/spi/mcp23s08.h   |1 +
 3 files changed, 117 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 1/1] gpio: mcp23s08: convert driver to DT

2013-01-31 Thread Lars Poeschel 
From: Lars Poeschel poesc...@lemonage.de

This converts the mcp23s08 driver to be able to be used with device
tree.
There are two properties taken, that correspond to the members of
the struct mcp23s08_platform_data, that is the base member and the
chip array member.

Signed-off-by: Lars Poeschel poesc...@lemonage.de
---
 .../devicetree/bindings/gpio/gpio-mcp23s08.txt |   27 ++
 drivers/gpio/gpio-mcp23s08.c   |   93 +++-
 include/linux/spi/mcp23s08.h   |1 +
 3 files changed, 117 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt 
b/Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt
new file mode 100644
index 000..572bc87
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mcp23s08.txt
@@ -0,0 +1,27 @@
+Microchip MCP2308/MCP23S08/MCP23017/MCP23S17 driver for
+8-/16-bit I/O expander with serial interface (I2C/SPI)
+
+Required properties:
+- compatible : Should be mcp,mcp23s08-gpio, mcp,mcp23s17-gpio,
+   mcp,mcp23008-gpio or mcp,mcp23017-gpio
+- base : The first gpio number that should be assigned by this chip.
+
+Optional properties:
+- chips : This is a table with 2 columns and up to 8 entries. The first column
+   is a is_present flag, that makes only sense for SPI chips. Multiple
+   chips can share the same chipselect. This flag can be 0 or 1 depending
+   if there is a chip at this address or not.
+   The second column is written to the GPPU register, selecting a 100k
+   pullup resistor or not. Setting a 1 is activating the pullup.
+   For I2C chips only the first line in this table is used. Further chips
+   are registered at different addresses at the I2C bus.
+
+Example:
+mcp0 {
+   compatible = mcp,mcp23017;
+   base = 128;
+   chips = 
+   /* is_present  pullups */
+   1   0x07/* chip addr 0 */
+   ;
+};
diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c
index 3cea0ea..7f90d11 100644
--- a/drivers/gpio/gpio-mcp23s08.c
+++ b/drivers/gpio/gpio-mcp23s08.c
@@ -12,6 +12,8 @@
 #include linux/spi/mcp23s08.h
 #include linux/slab.h
 #include asm/byteorder.h
+#include linux/of.h
+#include linux/of_device.h
 
 /**
  * MCP types supported by driver
@@ -473,16 +475,89 @@ fail:
 
 /*--*/
 
+#ifdef CONFIG_OF
+static struct of_device_id mcp23s08_of_match[] = {
+#ifdef CONFIG_SPI_MASTER
+   {
+   .compatible = mcp,mcp23s08-gpio,
+   },
+   {
+   .compatible = mcp,mcp23s17-gpio,
+   },
+#endif
+#if IS_ENABLED(CONFIG_I2C)
+   {
+   .compatible = mcp,mcp23008-gpio,
+   },
+   {
+   .compatible = mcp,mcp23017-gpio,
+   },
+#endif
+   { },
+};
+MODULE_DEVICE_TABLE(of, mcp23s08_of_match);
+
+static struct mcp23s08_platform_data *
+   of_get_mcp23s08_pdata(struct device *dev)
+{
+   struct mcp23s08_platform_data *pdata;
+   struct device_node *np = dev-of_node;
+   u32 gpio_base;
+   u32 chips[sizeof(pdata-chip)];
+   int ret, i, j;
+
+   pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+
+   ret = of_property_read_u32(np, base, gpio_base);
+   if (!ret)
+   pdata-base = gpio_base;
+
+   for (i = ARRAY_SIZE(pdata-chip) * MCP23S08_CHIP_INFO_MEMBERS;
+   i  0; i -= MCP23S08_CHIP_INFO_MEMBERS) {
+   ret = of_property_read_u32_array(np, chips, chips, i);
+   if (ret == -EOVERFLOW)
+   continue;
+   break;
+   }
+   if (!ret) {
+   for (j = 0; j  i / MCP23S08_CHIP_INFO_MEMBERS ; j++) {
+   pdata-chip[j].is_present =
+   chips[j * MCP23S08_CHIP_INFO_MEMBERS];
+   pdata-chip[j].pullups =
+   chips[j * MCP23S08_CHIP_INFO_MEMBERS + 1];
+   }
+   }
+
+   return pdata;
+}
+#else
+static struct mcp23s08_platform_data *
+   of_get_mcp23s08_pdata(struct i2c_client *client, int *chip_id)
+{
+   return NULL;
+}
+#endif /* CONFIG_OF */
+
 #if IS_ENABLED(CONFIG_I2C)
 
 static int mcp230xx_probe(struct i2c_client *client,
const struct i2c_device_id *id)
 {
-   struct mcp23s08_platform_data *pdata;
+   struct mcp23s08_platform_data *pdata = NULL;
struct mcp23s08 *mcp;
int status;
+   const struct of_device_id *match;
+
+   match = of_match_device(of_match_ptr(mcp23s08_of_match), client-dev);
+   if (match)
+   pdata = of_get_mcp23s08_pdata(client-dev);
+
+   /* if there was no pdata in DT, take it the legacy way */
+   if (!pdata

Re: [PATCH RFC] misc/at24: distinguish between eeprom and fram chips

2013-01-28 Thread Lars Poeschel 
On Thursday 24 January 2013 at 08:27:01, Wolfram Sang wrote:
  I wanted to use a fm24c04 i2c fram chip with linux. I grepped
  the source and found nothing. I later found that my chip can be
  handled by at24 eeprom driver. It creates a sysfs file called
  eeprom to read from and write to the chip. Userspace has no
  chance to distinguish if it is writing an eeprom or a fram
  chip.
 
 Why should it?

Because writes are much faster and it doesn't have to take care on
erase cycles. It could use other write strategies on such devices
and update informations that have to survive power downs more
often.
   
   I agree. I think that a seperate attribute named e.g. 'page_size'
   would be more helpful than renaming the binary file to fram?
  
  Yes, this is a much better solution! Adding a seperate sysfs file
  page_size and a file for the type of device which would read eeprom,
  fram, etc then. If you also think this is the way to go, I would spent
  one of my next free timeslots to this.
 
 Oops, this mail seems to have dropped off :(

Luckily I did not have a free timeslot to invest yet. ;)

 I am all for the 'page_size' attribute, but still not convinced what
 gain the 'type' attribute would allow. For FRAM, the page size will be
 large. Isn't this enough information?

Yes, this would be enough information and I think this is the way we should 
go.
I set this on my todo list. Although the change will be quite simple, I think 
I will not find the time to hit the upcoming merge window.

Lars
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] misc/at24: distinguish between eeprom and fram chips

2012-12-07 Thread Lars Poeschel 
I wanted to use a fm24c04 i2c fram chip with linux. I grepped the
source and found nothing. I later found that my chip can be handled
by at24 eeprom driver. It creates a sysfs file called eeprom to
read from and write to the chip. Userspace has no chance to
distinguish if it is writing an eeprom or a fram chip.
   
   Why should it?
  
  Because writes are much faster and it doesn't have to take care on erase
  cycles. It could use other write strategies on such devices and update
  informations that have to survive power downs more often.
 
 I agree. I think that a seperate attribute named e.g. 'page_size' would
 be more helpful than renaming the binary file to fram?

Yes, this is a much better solution! Adding a seperate sysfs file page_size 
and a file for the type of device which would read eeprom, fram, etc then.
If you also think this is the way to go, I would spent one of my next free 
timeslots to this.

   The method of accessing EEPROMs is used by way more chips than FRAMs.
   So, I'd prefer to have the text updated more generic like EEPROMs and
   similar devices like RAMs, ROMs, etc Describing setting .flags in
   Kconfig is overkill.
  
  A patch updating Kconfig is below.
 
 Looks good from a glimpse, will apply it later.

Thank you!

Lars
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] misc/at24: distinguish between eeprom and fram chips

2012-12-05 Thread Lars Poeschel 
I see there where to much nos to get anything in, but thank you for
your comments and explanations.

  I wanted to use a fm24c04 i2c fram chip with linux. I grepped the source
  and found nothing. I later found that my chip can be handled by at24
  eeprom driver. It creates a sysfs file called eeprom to read from and
  write to the chip. Userspace has no chance to distinguish if it is
  writing an eeprom or a fram chip.
 
 Why should it?

Because writes are much faster and it doesn't have to take care on erase
cycles. It could use other write strategies on such devices and update
informations that have to survive power downs more often.
 
  diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
  index c9e695e..55948a5 100644
  --- a/drivers/misc/eeprom/Kconfig
  +++ b/drivers/misc/eeprom/Kconfig
  @@ -12,6 +12,12 @@ config EEPROM_AT24
  
   24c00, 24c01, 24c02, spd (readonly 24c02), 24c04, 24c08,
   24c16, 24c32, 24c64, 24c128, 24c256, 24c512, 24c1024
  
  + This driver also supports I2C FRAM chips that are feature
  + compatible to the 24cxx ones. In your at24_platform_data set
  + .flags = AT24_FLAG_FRAM. These generic names are supported:
  +
  +fm24c04
  +
 
 The method of accessing EEPROMs is used by way more chips than FRAMs.
 So, I'd prefer to have the text updated more generic like EEPROMs and
 similar devices like RAMs, ROMs, etc Describing setting .flags in
 Kconfig is overkill.

A patch updating Kconfig is below.
 
  -   chip.page_size = 1;
  +   if (chip.flags  AT24_FLAG_FRAM)
  +   chip.page_size = 128;
  +   else
  +   chip.page_size = 1;
 
 I'd think most FRAMs can have the chip_size as the page_size since they
 probably don't do buffering. But do you know for all the chips out
 there? So, let's still play safe. If you want performance, you need to
 setup the driver properly.

No one knows all chips out there.
For the fm24c04 I use page_size != chip_size. It does not buffer but it has 
two pages, 256 bytes each.

-- 8 --
From 82f77ade238755b7a1196b24f41a03a0e7344d89 Mon Sep 17 00:00:00 2001
From: Lars Poeschel poesc...@lemonage.de
Date: Wed, 5 Dec 2012 10:08:07 +0100
Subject: [PATCH] misc/at24: Update Kconfig to mention FRAMs, SRAMs and ROMs

As the at24 driver is able handle a bunch of serial storage chips other than
EEPROMs this is now mentioned in Kconfig.

Signed-off-by: Lars Poeschel poesc...@lemonage.de

diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
index c9e695e..04f2e1f 100644
--- a/drivers/misc/eeprom/Kconfig
+++ b/drivers/misc/eeprom/Kconfig
@@ -1,13 +1,14 @@
 menu EEPROM support
 
 config EEPROM_AT24
-   tristate I2C EEPROMs from most vendors
+   tristate I2C EEPROMs / RAMs / ROMs from most vendors
depends on I2C  SYSFS
help
- Enable this driver to get read/write support to most I2C EEPROMs,
- after you configure the driver to know about each EEPROM on
- your target board.  Use these generic chip names, instead of
- vendor-specific ones like at24c64 or 24lc02:
+ Enable this driver to get read/write support to most I2C EEPROMs
+ and compatible devices like FRAMs, SRAMs, ROMs etc. After you
+ configure the driver to know about each chip on your target
+ board.  Use these generic chip names, instead of vendor-specific
+ ones like at24c64, 24lc02 or fm24c04:
 
 24c00, 24c01, 24c02, spd (readonly 24c02), 24c04, 24c08,
 24c16, 24c32, 24c64, 24c128, 24c256, 24c512, 24c1024
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC] misc/at24: distinguish between eeprom and fram chips

2012-12-04 Thread Lars Poeschel 
Hello!

I wanted to use a fm24c04 i2c fram chip with linux. I grepped the source
and found nothing. I later found that my chip can be handled by at24
eeprom driver. It creates a sysfs file called eeprom to read from and
write to the chip. Userspace has no chance to distinguish if it is
writing an eeprom or a fram chip.

I present this patch for 3 reasons:
1. For other people grepping finding a little more reference.
2. For userspace being able to distinguish eeprom and fram.
3. Raising the bytes per write for fram chips.

What do you kernel developers think ?

Cheers,
Lars
-- 8 --
From 4fab49fae62390995868e3b6dee7e0693fce5be9 Mon Sep 17 00:00:00 2001
From: Lars Poeschel poesc...@lemonage.de
Date: Tue, 4 Dec 2012 15:41:40 +0100
Subject: [PATCH] misc/at24: distinguish between eeprom and fram chips

Add a AT24_FLAGS_FRAM state to the flags to make userspace able to
distinguish if it is using eeprom or fram. The sysfs entry gets the
name fram instead of eeprom.
For frams the bytes/write can be at least 128 bytes, since these
chips have no need to internally buffer writes.

Signed-off-by: Lars Poeschel poesc...@lemonage.de

diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
index c9e695e..55948a5 100644
--- a/drivers/misc/eeprom/Kconfig
+++ b/drivers/misc/eeprom/Kconfig
@@ -12,6 +12,12 @@ config EEPROM_AT24
 24c00, 24c01, 24c02, spd (readonly 24c02), 24c04, 24c08,
 24c16, 24c32, 24c64, 24c128, 24c256, 24c512, 24c1024
 
+ This driver also supports I2C FRAM chips that are feature
+ compatible to the 24cxx ones. In your at24_platform_data set
+ .flags = AT24_FLAG_FRAM. These generic names are supported:
+
+fm24c04
+
  Unless you like data loss puzzles, always be sure that any chip
  you configure as a 24c32 (32 kbit) or larger is NOT really a
  24c16 (16 kbit) or smaller, and vice versa. Marking the chip
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index ab1ad41..02a03a1 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -125,6 +125,7 @@ static const struct i2c_device_id at24_ids[] = {
{ 24c256, AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16) },
{ 24c512, AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16) },
{ 24c1024, AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16) },
+   { fm24c04, AT24_DEVICE_MAGIC(4096 / 8, AT24_FLAG_FRAM) },
{ at24, 0 },
{ /* END OF LIST */ }
 };
@@ -504,8 +505,13 @@ static int at24_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
 * This is slow, but we can't know all eeproms, so we better
 * play safe. Specifying custom eeprom-types via platform_data
 * is recommended anyhow.
+* For fram chips, we can allow minmum 128 bytes, as there is
+* no page size and 128 is the smallest so far seen chip.
 */
-   chip.page_size = 1;
+   if (chip.flags  AT24_FLAG_FRAM)
+   chip.page_size = 128;
+   else
+   chip.page_size = 1;
 
/* update chipdata if OF is present */
at24_get_ofdata(client, chip);
@@ -570,7 +576,10 @@ static int at24_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
 * By default, only root should see the data (maybe passwords etc)
 */
sysfs_bin_attr_init(at24-bin);
-   at24-bin.attr.name = eeprom;
+   if (chip.flags  AT24_FLAG_FRAM)
+   at24-bin.attr.name = fram;
+   else
+   at24-bin.attr.name = eeprom;
at24-bin.attr.mode = chip.flags  AT24_FLAG_IRUGO ? S_IRUGO : S_IRUSR;
at24-bin.read = at24_bin_read;
at24-bin.size = chip.byte_len;
diff --git a/include/linux/i2c/at24.h b/include/linux/i2c/at24.h
index 285025a..d786b71 100644
--- a/include/linux/i2c/at24.h
+++ b/include/linux/i2c/at24.h
@@ -47,6 +47,7 @@ struct at24_platform_data {
 #define AT24_FLAG_READONLY 0x40/* sysfs-entry will be read-only */
 #define AT24_FLAG_IRUGO0x20/* sysfs-entry will be 
world-readable */
 #define AT24_FLAG_TAKE8ADDR0x10/* take always 8 addresses (24c00) */
+#define AT24_FLAG_FRAM  0x08/* chip is fram not eeprom */
 
void(*setup)(struct memory_accessor *, void *context);
void*context;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html