Re: [PATCH 1/2] i2c: add SMBus Host Notify support
Hi Benjamin, On Tue, 23 Jun 2015 14:58:18 -0400, Benjamin Tissoires wrote: SMBus Host Notify allows a slave device to act as a master on a bus to notify the host of an interrupt. The functionality is directly implemented in the firmware so we just export a toggle function and rely on the adapter to actually support this. Why does it need to be toggled? I mean, if the controller supports it, why don't we just enable the feature all the time? Signed-off-by: Benjamin Tissoires benjamin.tissoi...@redhat.com --- Documentation/i2c/smbus-protocol | 4 drivers/i2c/i2c-core.c | 26 ++ include/linux/i2c.h | 8 include/uapi/linux/i2c.h | 1 + 4 files changed, 39 insertions(+) diff --git a/Documentation/i2c/smbus-protocol b/Documentation/i2c/smbus-protocol index 6012b12..af4e4b9 100644 --- a/Documentation/i2c/smbus-protocol +++ b/Documentation/i2c/smbus-protocol @@ -199,6 +199,10 @@ alerting device's address. [S] [HostAddr] [Wr] A [DevAddr] A [DataLow] A [DataHigh] A [P] +I2C drivers for devices which can trigger SMBus Host Notify should call +i2c_smbus_toggle_host_notify() to enable SMBUS Host Notify on the adapter +and implement the optional alert() callback. + Packet Error Checking (PEC) === diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 987c124..eaaf5b0 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -2959,6 +2959,32 @@ int i2c_slave_unregister(struct i2c_client *client) EXPORT_SYMBOL_GPL(i2c_slave_unregister); #endif +int i2c_smbus_toggle_host_notify(struct i2c_client *client, bool state) +{ + int ret; + + if (!client) + return -EINVAL; + + if (!(client-flags I2C_CLIENT_TEN)) { + /* Enforce stricter address checking */ + ret = i2c_check_addr_validity(client-addr); + if (ret) + return ret; + } + + if (!client-adapter-algo-toggle_smbus_host_notify) + return -EOPNOTSUPP; + + i2c_lock_adapter(client-adapter); + ret = client-adapter-algo-toggle_smbus_host_notify(client-adapter, + state); + i2c_unlock_adapter(client-adapter); + + return ret; +} +EXPORT_SYMBOL_GPL(i2c_smbus_toggle_host_notify); + As this is an SMBus feature, can't it go to drivers/i2c/i2c-smbus.c? MODULE_AUTHOR(Simon G. Vogl si...@tk.uni-linz.ac.at); MODULE_DESCRIPTION(I2C-Bus main module); MODULE_LICENSE(GPL); diff --git a/include/linux/i2c.h b/include/linux/i2c.h index e83a738..7ffc970 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -177,6 +177,8 @@ struct i2c_driver { * The format and meaning of the data value depends on the protocol. * For the SMBus alert protocol, there is a single bit of data passed * as the alert response's low bit (event flag). + * For the SMBus Host Notify protocol, the data corresponds to the + * 16-bits payload data reported by the external device. 16-bit (no s) */ void (*alert)(struct i2c_client *, unsigned int data); @@ -270,6 +272,9 @@ static inline int i2c_slave_event(struct i2c_client *client, } #endif + No double blank lines in source code please. +extern int i2c_smbus_toggle_host_notify(struct i2c_client *client, bool state); + /** * struct i2c_board_info - template for device creation * @type: chip type, to initialize i2c_client.name @@ -378,6 +383,8 @@ i2c_register_board_info(int busnum, struct i2c_board_info const *info, * from the I2C_FUNC_* flags. * @reg_slave: Register given client to I2C slave mode of this adapter * @unreg_slave: Unregister given client from I2C slave mode of this adapter + * @toggle_smbus_host_notify: toggle the SMBus Host Notify support of this + * adapter. * * The following structs are for those who like to implement new bus drivers: * i2c_algorithm is the interface to a class of hardware solutions which can @@ -408,6 +415,7 @@ struct i2c_algorithm { int (*reg_slave)(struct i2c_client *client); int (*unreg_slave)(struct i2c_client *client); #endif + int (*toggle_smbus_host_notify)(struct i2c_adapter *adap, bool state); Please put it after u32 (*functionality), so that the field offsets do not depend on configuration options. }; /** diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h index 0e949cb..c72708e 100644 --- a/include/uapi/linux/i2c.h +++ b/include/uapi/linux/i2c.h @@ -100,6 +100,7 @@ struct i2c_msg { #define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x0200 #define I2C_FUNC_SMBUS_READ_I2C_BLOCK0x0400 /* I2C-like block xfer */ #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK 0x0800 /* w/ 1-byte reg. addr. */ +#define I2C_FUNC_SMBUS_HOST_NOTIFY 0x1000 #define I2C_FUNC_SMBUS_BYTE
Re: [RFC PATCH 1/1] i2c: core: add of_get_i2c_adapter_by_node
Hi Wolfram, On 29.06.2015 11:17, Wolfram Sang wrote: On Mon, Jun 29, 2015 at 03:36:19PM +0900, Frkuska, Joshua wrote: This adds of_get_i2c_adapter_by_node which both finds and properly increments the adapter reference count. This is different from of_find_i2c_device_by_node which does not increment the adapter reference counter. Signed-off-by: Vladimir Zapolskiy vladimir_zapols...@mentor.com Signed-off-by: Joshua Frkuska joshua_frku...@mentor.com What is your use case? the usecase is to avoid double getting of a pointer to the wanted struct i2c_adapter with incrementing its user counter at once, now it is done in two stages, namely find adapter and get adapter: --8-- struct i2c_adapter *ddc; int id; ... - ddc = of_find_i2c_adapter_by_node(ddc_node); - if (ddc) { - id = i2c_adapter_id(dcc); - dcc = i2c_get_adapter(id); + dcc = of_get_i2c_adapter_by_node(ddc); + if (dcc) + put_device(ddc-dev); put_device(ddc-dev); - } ... i2c_put_adapter(ddc); --8-- Since i2c adapted is locked by client, it might have sense to add put_device() hidden inside of_get_i2c_adapter_by_node(). Which code will be using this function? At the moment there is an intention to add clients in drivers/gpu/drm/*, who should lock i2c bus driver to sustain DDC lines. By the way to the current version the correspondent change in include/linux/i2c.h is missing: --8-- diff --git a/include/linux/i2c.h b/include/linux/i2c.h index e83a738..555ad9c 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -638,6 +638,9 @@ extern struct i2c_client *of_find_i2c_device_by_node(struct device_node *node); /* must call put_device() when done with returned i2c_adapter device */ extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node); +/* must call put_device() when done with returned i2c_adapter device */ +extern struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node); + #else static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node) @@ -649,6 +652,11 @@ static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node { return NULL; } + +static inline struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node) +{ + return NULL; +} #endif /* CONFIG_OF */ #endif /* _LINUX_I2C_H */ --8-- -- With best wishes, Vladimir -- 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 v2] i2c-tools: Implement Module.mk for eeprog subdir
Move eeprog to separate subdir and implement Module.mk for it Signed-off-by: Matwey V. Kornilov matwey.korni...@gmail.com --- eeprog/24cXX.c | 185 eeprog/24cXX.h | 58 ++ eeprog/Module.mk | 67 eeprog/README.eeprog | 12 +++ eeprog/eeprog.8| 103 ++ eeprog/eeprog.c| 283 + eepromer/24cXX.c | 185 eepromer/24cXX.h | 58 -- eepromer/Makefile | 6 +- eepromer/README.eeprog | 12 --- eepromer/eeprog.8 | 103 -- eepromer/eeprog.c | 283 - 12 files changed, 710 insertions(+), 645 deletions(-) create mode 100644 eeprog/24cXX.c create mode 100644 eeprog/24cXX.h create mode 100644 eeprog/Module.mk create mode 100644 eeprog/README.eeprog create mode 100644 eeprog/eeprog.8 create mode 100644 eeprog/eeprog.c delete mode 100644 eepromer/24cXX.c delete mode 100644 eepromer/24cXX.h delete mode 100644 eepromer/README.eeprog delete mode 100644 eepromer/eeprog.8 delete mode 100644 eepromer/eeprog.c diff --git a/eeprog/24cXX.c b/eeprog/24cXX.c new file mode 100644 index 000..6a0b610 --- /dev/null +++ b/eeprog/24cXX.c @@ -0,0 +1,185 @@ +/*** +copyright: (C) by 2002-2003 Stefano Barbato +email: stef...@codesink.org + +$Id$ + ***/ + +/*** + * * + * 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 * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***/ +#include stdio.h +#include fcntl.h +#include unistd.h +#include stdlib.h +#include linux/fs.h +#include linux/i2c.h +#include linux/i2c-dev.h +#include sys/types.h +#include sys/ioctl.h +#include errno.h +#include assert.h +#include string.h +#include i2c/smbus.h +#include 24cXX.h + +static int i2c_write_1b(struct eeprom *e, __u8 buf) +{ + int r; + // we must simulate a plain I2C byte write with SMBus functions + r = i2c_smbus_write_byte(e-fd, buf); + if(r 0) + fprintf(stderr, Error i2c_write_1b: %s\n, strerror(errno)); + usleep(10); + return r; +} + +static int i2c_write_2b(struct eeprom *e, __u8 buf[2]) +{ + int r; + // we must simulate a plain I2C byte write with SMBus functions + r = i2c_smbus_write_byte_data(e-fd, buf[0], buf[1]); + if(r 0) + fprintf(stderr, Error i2c_write_2b: %s\n, strerror(errno)); + usleep(10); + return r; +} + +static int i2c_write_3b(struct eeprom *e, __u8 buf[3]) +{ + int r; + // we must simulate a plain I2C byte write with SMBus functions + // the __u16 data field will be byte swapped by the SMBus protocol + r = i2c_smbus_write_word_data(e-fd, buf[0], buf[2] 8 | buf[1]); + if(r 0) + fprintf(stderr, Error i2c_write_3b: %s\n, strerror(errno)); + usleep(10); + return r; +} + + +#define CHECK_I2C_FUNC( var, label ) \ + do {if(0 == (var label)) { \ + fprintf(stderr, \nError: \ + #label function is required. Program halted.\n\n); \ + exit(1); } \ + } while(0); + +int eeprom_open(char *dev_fqn, int addr, int type, struct eeprom* e) +{ + int fd, r; + unsigned long funcs; + e-fd = e-addr = 0; + e-dev = 0; + + fd = open(dev_fqn, O_RDWR); + if(fd = 0) + return -1; + + // get funcs list + if((r = ioctl(fd, I2C_FUNCS, funcs) 0)) + return r; + + + // check for req funcs + CHECK_I2C_FUNC( funcs, I2C_FUNC_SMBUS_READ_BYTE ); + CHECK_I2C_FUNC( funcs, I2C_FUNC_SMBUS_WRITE_BYTE ); + CHECK_I2C_FUNC( funcs, I2C_FUNC_SMBUS_READ_BYTE_DATA ); + CHECK_I2C_FUNC( funcs, I2C_FUNC_SMBUS_WRITE_BYTE_DATA ); + CHECK_I2C_FUNC( funcs, I2C_FUNC_SMBUS_READ_WORD_DATA ); + CHECK_I2C_FUNC( funcs, I2C_FUNC_SMBUS_WRITE_WORD_DATA ); + + // set working device + if( ( r = ioctl(fd, I2C_SLAVE, addr)) 0) + return r; + e-fd = fd; + e-addr = addr; + e-dev = dev_fqn; + e-type = type; + return 0; +} + +int eeprom_close(struct eeprom *e) +{ + close(e-fd); + e-fd = -1; + e-dev = 0; +
RE: [Patch v2] i2c: imx: add runtime pm support to improve the performance
Ping ... From: Gao Pan b54...@freescale.com Sent: Thursday, June 18, 2015 5:28 PM To: w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; Gao Pan-B54642; u.kleine-koe...@pengutronix.de Subject: [Patch v2] i2c: imx: add runtime pm support to improve the performance In our former i2c driver, i2c clk is enabled and disabled in xfer function, which contributes to power saving. However, the clk enable process brings a busy wait delay until the core is stable. As a result, the performance is sacrificed. To weigh the power consumption and i2c bus performance, runtime pm is the good solution for it. The clk is enabled when a i2c transfer starts, and disabled after a specifically defined delay. Without the patch the test case (many eeprom reads) executes with approx: real 1m7.735s user 0m0.488s sys 0m20.040s With the patch the same test case (many eeprom reads) executes with approx: real 0m54.241s user 0m0.440s sys 0m5.920s From the test result, the patch get better performance. V2: As Uwe Kleine-König's suggestion, the version do below changes: - call clk_prepare_enable in probe to avoid never enabling clock if CONFIG_PM is disabled - enable clock before request IRQ in probe - remove the pm staff in i2c_imx_isr Signed-off-by: Fugang Duan b38...@freescale.com Signed-off-by: Gao Pan b54...@freescale.com --- drivers/i2c/busses/i2c-imx.c | 76 +- -- 1 file changed, 65 insertions(+), 11 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 785aa67..37cd9f6 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -53,6 +53,7 @@ #include linux/platform_device.h #include linux/sched.h #include linux/slab.h +#include linux/pm_runtime.h /** Defines ** *** **/ @@ -118,6 +119,8 @@ #define I2CR_IEN_OPCODE_00x0 #define I2CR_IEN_OPCODE_1I2CR_IEN +#define I2C_PM_TIMEOUT 10 /* ms */ + /** Variables ** ** *** **/ @@ -520,9 +523,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx) i2c_imx_set_clk(i2c_imx); - result = clk_prepare_enable(i2c_imx-clk); - if (result) - return result; imx_i2c_write_reg(i2c_imx-ifdr, i2c_imx, IMX_I2C_IFDR); /* Enable I2C controller */ imx_i2c_write_reg(i2c_imx-hwdata-i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR); @@ -575,7 +575,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) /* Disable I2C controller */ temp = i2c_imx-hwdata-i2cr_ien_opcode ^ I2CR_IEN, imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); - clk_disable_unprepare(i2c_imx-clk); } static irqreturn_t i2c_imx_isr(int irq, void *dev_id) @@ -894,6 +893,10 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter, dev_dbg(i2c_imx-adapter.dev, %s\n, __func__); + result = pm_runtime_get_sync(i2c_imx-adapter.dev.parent); + if (IS_ERR_VALUE(result)) + goto out; + /* Start I2C transfer */ result = i2c_imx_start(i2c_imx); if (result) @@ -950,6 +953,10 @@ fail0: /* Stop I2C transfer */ i2c_imx_stop(i2c_imx); +out: + pm_runtime_mark_last_busy(i2c_imx-adapter.dev.parent); + pm_runtime_put_autosuspend(i2c_imx-adapter.dev.parent); + dev_dbg(i2c_imx-adapter.dev, %s exit with: %s: %d\n, __func__, (result 0) ? error : success msg, (result 0) ? result : num); @@ -1020,9 +1027,10 @@ static int i2c_imx_probe(struct platform_device *pdev) ret = clk_prepare_enable(i2c_imx-clk); if (ret) { - dev_err(pdev-dev, can't enable I2C clock\n); + dev_err(pdev-dev, can't enable I2C clock, ret=%d\n, ret); return ret; } + /* Request IRQ */ ret = devm_request_irq(pdev-dev, irq, i2c_imx_isr, 0, pdev-name, i2c_imx); @@ -1037,6 +1045,13 @@ static int i2c_imx_probe(struct platform_device *pdev) /* Set up adapter data */ i2c_set_adapdata(i2c_imx-adapter, i2c_imx); + /* Set up platform driver data */ + platform_set_drvdata(pdev, i2c_imx); + + pm_runtime_enable(pdev-dev); + pm_runtime_set_autosuspend_delay(pdev-dev, I2C_PM_TIMEOUT); + pm_runtime_use_autosuspend(pdev-dev); + /* Set up clock divider */ i2c_imx-bitrate = IMX_I2C_BIT_RATE; ret = of_property_read_u32(pdev-dev.of_node, @@ -1056,9 +1071,8 @@ static int i2c_imx_probe(struct platform_device *pdev) goto clk_disable; } - /* Set up platform driver data */ -
Re: [RFC PATCH 1/1] i2c: core: add of_get_i2c_adapter_by_node
On Mon, Jun 29, 2015 at 03:36:19PM +0900, Frkuska, Joshua wrote: This adds of_get_i2c_adapter_by_node which both finds and properly increments the adapter reference count. This is different from of_find_i2c_device_by_node which does not increment the adapter reference counter. Signed-off-by: Vladimir Zapolskiy vladimir_zapols...@mentor.com Signed-off-by: Joshua Frkuska joshua_frku...@mentor.com What is your use case? Which code will be using this function? signature.asc Description: Digital signature
Re: [PATCH] i2c-tools: Implement Module.mk for eepromer subdir
2015-06-29 11:41 GMT+03:00 Jean Delvare jdelv...@suse.de: Hi Matwey, On Sat, 27 Jun 2015 11:49:07 +0300, Matwey V. Kornilov wrote: eepromer/Module.mk has to be present in order to do make EXTRA=eepromer The problem with the eepromer directory is that it's a mess. There are 3 tools there which do basically the same (programming EEPROMs), but each has its own limitations: * eeprom can't deal with 16-bit addressed EEPROMs nor SMBus-only controllers. * eepromer can't deal with 8-bit addressed EEPROMs nor SMBus-only controllers. * eeprog only uses SMBus byte/word reads and writes, it lacks support for raw I2C block writes so it is slow on large EEPROMs. On top of that, eeprom and eepromer build with a lot of warnings. I really don't want to maintain 3 different tools with the same purpose. eeprog seems to be the most promising of all 3, as it is fully functional, but I don't feel comfortable dropping the other 2 until block write support is added to eeprog. Would you be interested in adding block write support to eeprog? It has been on my to-do list for a long time but I could never find the time to actually look into it. I do not use these tools myself so my personal interest is low and this is why it never makes it to the top of my to-do list. So am I. I wanted to program EEPROM on my BeagleBone and I've found that eeprog was missed in openSUSE packages. So,now I just want to simplify packaging eeprog somehow in acceptable manner. Signed-off-by: Matwey V. Kornilov matwey.korni...@gmail.com --- --- /dev/null +++ i2c-tools-3.1.1/eepromer/Module.mk @@ -0,0 +1,79 @@ +# eepromer +# +# 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 +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. + +EEPROMER_DIR := eepromer + +EEPROMER_CFLAGS := -O2 -Iinclude -Wall +ifeq ($(USE_STATIC_LIB),1) +EEPROMER_LDFLAGS := $(LIB_DIR)/$(LIB_STLIBNAME) +else +EEPROMER_LDFLAGS := -L$(LIB_DIR) -li2c +endif This will link eepromer and eeprom with libi2c even though they don't need it, right? + +EEPROMER_TARGETS := eepromer eeprom eeprog + +# +# Programs +# + +$(EEPROMER_DIR)/eepromer: $(EEPROMER_DIR)/eepromer.o + $(CC) $(LDFLAGS) -o $@ $^ $(EEPROMER_LDFLAGS) + +$(EEPROMER_DIR)/eeprom: $(EEPROMER_DIR)/eeprom.o + $(CC) $(LDFLAGS) -o $@ $^ $(EEPROMER_LDFLAGS) + +$(EEPROMER_DIR)/eeprog: $(EEPROMER_DIR)/eeprog.o $(EEPROMER_DIR)/24cXX.o + $(CC) $(LDFLAGS) -o $@ $^ $(EEPROMER_LDFLAGS) + +# +# Objects +# + +$(EEPROMER_DIR)/eepromer.o: $(EEPROMER_DIR)/eepromer.c + $(CC) $(CFLAGS) $(EEPROMER_CFLAGS) -c $ -o $@ + +$(EEPROMER_DIR)/eeprom.o: $(EEPROMER_DIR)/eeprom.c + $(CC) $(CFLAGS) $(EEPROMER_CFLAGS) -c $ -o $@ + +$(EEPROMER_DIR)/eeprog.o: $(EEPROMER_DIR)/eeprog.c $(EEPROMER_DIR)/24cXX.h + $(CC) $(CFLAGS) $(EEPROMER_CFLAGS) -c $ -o $@ + +$(EEPROMER_DIR)/24cXX.o: $(EEPROMER_DIR)/24cXX.c $(EEPROMER_DIR)/24cXX.h Also depends on $(INCLUDE_DIR)/i2c/smbus.h. + $(CC) $(CFLAGS) $(EEPROMER_CFLAGS) -c $ -o $@ + +# +# Commands +# + +all-eepromer: $(addprefix $(EEPROMER_DIR)/,$(EEPROMER_TARGETS)) + +strip-eepromer: $(addprefix $(EEPROMER_DIR)/,$(EEPROMER_TARGETS)) + strip $(addprefix $(EEPROMER_DIR)/,$(EEPROMER_TARGETS)) + +clean-eepromer: + $(RM) $(addprefix $(EEPROMER_DIR)/,*.o $(EEPROMER_TARGETS)) + +install-eepromer: $(addprefix $(EEPROMER_DIR)/,$(EEPROMER_TARGETS)) + $(INSTALL_DIR) $(DESTDIR)$(sbindir) $(DESTDIR)$(man8dir) + for program in $(EEPROMER_TARGETS) ; do \ + $(INSTALL_PROGRAM) $(EEPROMER_DIR)/$$program $(DESTDIR)$(sbindir) ; \ + $(INSTALL_DATA) $(EEPROMER_DIR)/$$program.8 $(DESTDIR)$(man8dir) ; done + +uninstall-eepromer: + for program in $(EEPROMER_TARGETS) ; do \ + $(RM) $(DESTDIR)$(sbindir)/$$program ; \ + $(RM) $(DESTDIR)$(man8dir)/$$program.8 ; done + +all: all-eepromer + +strip: strip-eepromer + +clean: clean-eepromer + +install: install-eepromer + +uninstall: uninstall-eepromer I don't think it makes sense to integrate eeprom and eepromer in the main build system if the plan is to get rid of them. I'd rather only integrate eeprog, so that distributions ship only that and everybody uses only that. Maybe eeprog should be moved to its own directory, and that new directory would be integrated into the build system while the rest of the eepromer directory would be left alone (and ultimately deleted.) It is fine for me too. -- Jean Delvare SUSE L3 Support -- With best regards, Matwey V. Kornilov http://blog.matwey.name xmpp://0x2...@jabber.ru -- 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] i2c-tools: Implement Module.mk for eepromer subdir
Hi Matwey, On Sat, 27 Jun 2015 11:49:07 +0300, Matwey V. Kornilov wrote: eepromer/Module.mk has to be present in order to do make EXTRA=eepromer The problem with the eepromer directory is that it's a mess. There are 3 tools there which do basically the same (programming EEPROMs), but each has its own limitations: * eeprom can't deal with 16-bit addressed EEPROMs nor SMBus-only controllers. * eepromer can't deal with 8-bit addressed EEPROMs nor SMBus-only controllers. * eeprog only uses SMBus byte/word reads and writes, it lacks support for raw I2C block writes so it is slow on large EEPROMs. On top of that, eeprom and eepromer build with a lot of warnings. I really don't want to maintain 3 different tools with the same purpose. eeprog seems to be the most promising of all 3, as it is fully functional, but I don't feel comfortable dropping the other 2 until block write support is added to eeprog. Would you be interested in adding block write support to eeprog? It has been on my to-do list for a long time but I could never find the time to actually look into it. I do not use these tools myself so my personal interest is low and this is why it never makes it to the top of my to-do list. Signed-off-by: Matwey V. Kornilov matwey.korni...@gmail.com --- --- /dev/null +++ i2c-tools-3.1.1/eepromer/Module.mk @@ -0,0 +1,79 @@ +# eepromer +# +# 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 +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. + +EEPROMER_DIR := eepromer + +EEPROMER_CFLAGS := -O2 -Iinclude -Wall +ifeq ($(USE_STATIC_LIB),1) +EEPROMER_LDFLAGS := $(LIB_DIR)/$(LIB_STLIBNAME) +else +EEPROMER_LDFLAGS := -L$(LIB_DIR) -li2c +endif This will link eepromer and eeprom with libi2c even though they don't need it, right? + +EEPROMER_TARGETS := eepromer eeprom eeprog + +# +# Programs +# + +$(EEPROMER_DIR)/eepromer: $(EEPROMER_DIR)/eepromer.o + $(CC) $(LDFLAGS) -o $@ $^ $(EEPROMER_LDFLAGS) + +$(EEPROMER_DIR)/eeprom: $(EEPROMER_DIR)/eeprom.o + $(CC) $(LDFLAGS) -o $@ $^ $(EEPROMER_LDFLAGS) + +$(EEPROMER_DIR)/eeprog: $(EEPROMER_DIR)/eeprog.o $(EEPROMER_DIR)/24cXX.o + $(CC) $(LDFLAGS) -o $@ $^ $(EEPROMER_LDFLAGS) + +# +# Objects +# + +$(EEPROMER_DIR)/eepromer.o: $(EEPROMER_DIR)/eepromer.c + $(CC) $(CFLAGS) $(EEPROMER_CFLAGS) -c $ -o $@ + +$(EEPROMER_DIR)/eeprom.o: $(EEPROMER_DIR)/eeprom.c + $(CC) $(CFLAGS) $(EEPROMER_CFLAGS) -c $ -o $@ + +$(EEPROMER_DIR)/eeprog.o: $(EEPROMER_DIR)/eeprog.c $(EEPROMER_DIR)/24cXX.h + $(CC) $(CFLAGS) $(EEPROMER_CFLAGS) -c $ -o $@ + +$(EEPROMER_DIR)/24cXX.o: $(EEPROMER_DIR)/24cXX.c $(EEPROMER_DIR)/24cXX.h Also depends on $(INCLUDE_DIR)/i2c/smbus.h. + $(CC) $(CFLAGS) $(EEPROMER_CFLAGS) -c $ -o $@ + +# +# Commands +# + +all-eepromer: $(addprefix $(EEPROMER_DIR)/,$(EEPROMER_TARGETS)) + +strip-eepromer: $(addprefix $(EEPROMER_DIR)/,$(EEPROMER_TARGETS)) + strip $(addprefix $(EEPROMER_DIR)/,$(EEPROMER_TARGETS)) + +clean-eepromer: + $(RM) $(addprefix $(EEPROMER_DIR)/,*.o $(EEPROMER_TARGETS)) + +install-eepromer: $(addprefix $(EEPROMER_DIR)/,$(EEPROMER_TARGETS)) + $(INSTALL_DIR) $(DESTDIR)$(sbindir) $(DESTDIR)$(man8dir) + for program in $(EEPROMER_TARGETS) ; do \ + $(INSTALL_PROGRAM) $(EEPROMER_DIR)/$$program $(DESTDIR)$(sbindir) ; \ + $(INSTALL_DATA) $(EEPROMER_DIR)/$$program.8 $(DESTDIR)$(man8dir) ; done + +uninstall-eepromer: + for program in $(EEPROMER_TARGETS) ; do \ + $(RM) $(DESTDIR)$(sbindir)/$$program ; \ + $(RM) $(DESTDIR)$(man8dir)/$$program.8 ; done + +all: all-eepromer + +strip: strip-eepromer + +clean: clean-eepromer + +install: install-eepromer + +uninstall: uninstall-eepromer I don't think it makes sense to integrate eeprom and eepromer in the main build system if the plan is to get rid of them. I'd rather only integrate eeprog, so that distributions ship only that and everybody uses only that. Maybe eeprog should be moved to its own directory, and that new directory would be integrated into the build system while the rest of the eepromer directory would be left alone (and ultimately deleted.) -- Jean Delvare SUSE L3 Support -- 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-V2 00/12] i2c: pxa: Fixes, cleanup and support for pxa910 family
On Monday 15 June 2015 09:19 PM, Vaibhav Hiremath wrote: This patch series fixes bugs/warnings, cleans up the code and adds support for PXA910 family of devices to PXA I2C bus driver. There has been one attempt made sometime back in 2012 to upstream some of the patches from below list, but did not get follow up later. I have consolidated all the patches, cleaned them up, splited into logical changes, added new patches and submitting it now. I tried to maintain authorship Signoff except where I did some significant changes to the code/logic. Link to previous post: http://permalink.gmane.org/gmane.linux.drivers.i2c/13557 Testing: - Basic testing on PMIC device on I2C-0 interface - Boot tested on platform based on PXA1928 - Probe is successfully passing - Read few registers of PMIC (RTC, ID, etc...) during boot V1 = V2: Ping !!! Thanks, Vaibhav Link to V1 - http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/347012.html - Fixed all comments from Robert Jarzmik and Wolfram Sang - Dropped Patch 05/12: using core bus reset implementation - under work. Will submit shortly. 08/12: NAKed and dropped - Seperated DT binding patch from driver changes, for easy merge Leilei Shang (1): i2c: pxa: keep i2c irq ON in suspend Shouming Wang (1): i2c: pxa: Return I2C_RETRY when timeout in pio mode Vaibhav Hiremath (8): i2c: pxa: No need to set slave addr for i2c master mode reset i2c: pxa: Reset i2c controller on timeout in interrupt and pio mode i2c: pxa: Update debug function to dump more info on error i2c:pxa: Use devm_ variants in probe function Documentation: binding: add new property 'disable_after_xfer' to i2c-pxa i2c: pxa: Add support for pxa910/988 new configuration features i2c: pxa: Add ILCR (tLow tHigh) configuration support Documentation: binding: add sclk adjustment properties to i2c-pxa Yi Zhang (1): i2c: pxa: enable/disable i2c module across msg xfer Yipeng Yao (1): i2c: pxa: Remove compile warnning in 64bit mode Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 18 ++ drivers/i2c/busses/i2c-pxa.c | 267 -- 2 files changed, 220 insertions(+), 65 deletions(-) -- 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] i2c: jz4780: Fix return value if probe fails
Current code returns 0 if fails to read clock-frequency DT property, fix it. Also add checking return value of clk_prepare_enable and propagate return value of devm_request_irq. Signed-off-by: Axel Lin axel@ingics.com --- drivers/i2c/busses/i2c-jz4780.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c index 19b2d68..f325663 100644 --- a/drivers/i2c/busses/i2c-jz4780.c +++ b/drivers/i2c/busses/i2c-jz4780.c @@ -764,12 +764,15 @@ static int jz4780_i2c_probe(struct platform_device *pdev) if (IS_ERR(i2c-clk)) return PTR_ERR(i2c-clk); - clk_prepare_enable(i2c-clk); + ret = clk_prepare_enable(i2c-clk); + if (ret) + return ret; - if (of_property_read_u32(pdev-dev.of_node, clock-frequency, -clk_freq)) { + ret = of_property_read_u32(pdev-dev.of_node, clock-frequency, + clk_freq); + if (ret) { dev_err(pdev-dev, clock-frequency not specified in DT); - return clk_freq; + goto err; } i2c-speed = clk_freq / 1000; @@ -790,10 +793,8 @@ static int jz4780_i2c_probe(struct platform_device *pdev) i2c-irq = platform_get_irq(pdev, 0); ret = devm_request_irq(pdev-dev, i2c-irq, jz4780_i2c_irq, 0, dev_name(pdev-dev), i2c); - if (ret) { - ret = -ENODEV; + if (ret) goto err; - } ret = i2c_add_adapter(i2c-adap); if (ret 0) { -- 2.1.0 -- 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] i2c: xgene-slimpro: Fix missing mbox_free_channel call in probe error path
Free requested mailbox channel before return error. Signed-off-by: Axel Lin axel@ingics.com --- drivers/i2c/busses/i2c-xgene-slimpro.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c b/drivers/i2c/busses/i2c-xgene-slimpro.c index dcca707..1c9cb65a 100644 --- a/drivers/i2c/busses/i2c-xgene-slimpro.c +++ b/drivers/i2c/busses/i2c-xgene-slimpro.c @@ -419,6 +419,7 @@ static int xgene_slimpro_i2c_probe(struct platform_device *pdev) rc = i2c_add_adapter(adapter); if (rc) { dev_err(pdev-dev, Adapter registeration failed\n); + mbox_free_channel(ctx-mbox_chan); return rc; } -- 2.1.0 -- 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