Patchwork i2c-tools: Add i2c-dev.h to staging directory for userspace i2c drivers.

login
register
mail settings
Submitter Jouko Nikula
Date Sept. 24, 2013, 10:18 a.m.
Message ID <1380017924-20072-1-git-send-email-jouko.nikula@espotel.com>
Download mbox | patch
Permalink /patch/277423/
State Accepted
Headers show

Comments

Jouko Nikula - Sept. 24, 2013, 10:18 a.m.
Signed-off-by: Jouko Nikula <jouko.nikula@espotel.com>
---
 package/i2c-tools/i2c-tools.mk |    5 +++++
 1 file changed, 5 insertions(+)
Thomas Petazzoni - Sept. 24, 2013, 3:20 p.m.
Dear Jouko Nikula,

On Tue, 24 Sep 2013 13:18:43 +0300, Jouko Nikula wrote:

> +define I2C_TOOLS_INSTALL_STAGING_CMDS
> +	$(INSTALL) -D -m644 $(@D)/include/linux/i2c-dev.h $(STAGING_DIR)/usr/include/linux/
> +endef

This header is already part of the kernel headers, so it should already
be part of the toolchain. Do you have a specific case where this header
is missing?

Best regards,

Thomas
Baruch Siach - Sept. 24, 2013, 3:55 p.m.
Hi Thomas,

On Tue, Sep 24, 2013 at 05:20:41PM +0200, Thomas Petazzoni wrote:
> On Tue, 24 Sep 2013 13:18:43 +0300, Jouko Nikula wrote:
> > +define I2C_TOOLS_INSTALL_STAGING_CMDS
> > +	$(INSTALL) -D -m644 $(@D)/include/linux/i2c-dev.h $(STAGING_DIR)/usr/include/linux/
> > +endef
> 
> This header is already part of the kernel headers, so it should already
> be part of the toolchain. Do you have a specific case where this header
> is missing?

This is a different header than the kernel one, although confusingly named 
identically. On my Debian system the libi2c-dev package provides i2c-dev.h, 
and diverts the kernel header to i2c-dev.h.kernel.

$ dpkg -S /usr/include/linux/i2c-dev.h
diversion by libi2c-dev from: /usr/include/linux/i2c-dev.h
diversion by libi2c-dev to: /usr/include/linux/i2c-dev.h.kernel
linux-libc-dev:amd64, libi2c-dev: /usr/include/linux/i2c-dev.h

baruch
Will Wagner - Sept. 24, 2013, 4:46 p.m.
On 24/09/2013 16:20, Thomas Petazzoni wrote:
> Dear Jouko Nikula,
>
> On Tue, 24 Sep 2013 13:18:43 +0300, Jouko Nikula wrote:
>
>> +define I2C_TOOLS_INSTALL_STAGING_CMDS
>> +	$(INSTALL) -D -m644 $(@D)/include/linux/i2c-dev.h $(STAGING_DIR)/usr/include/linux/
>> +endef
>
> This header is already part of the kernel headers, so it should already
> be part of the toolchain. Do you have a specific case where this header
> is missing?
>

The contents of the header are different, the one from i2c-tools 
contains more.

We also carry a patch similar to this. Our patch has an addition which 
is a dependency on linux (assuming it is selected in the config). This 
ensures that the header from the kernel gets over ridden by the one from 
i2c-tools and not the other way around.

Regards
Will
Jouko Nikula - Sept. 25, 2013, 5:14 a.m.
From: buildroot-bounces@busybox.net [mailto:buildroot-bounces@busybox.net] On Behalf Of Will Wagner
Sent: 24. syyskuuta 2013 19:46

>> This header is already part of the kernel headers, so it should 
>> already be part of the toolchain. Do you have a specific case where 
>> this header is missing?

>The contents of the header are different, the one from i2c-tools contains more.

Yes. In the kernel documentation (Documentation/i2c/dev-interface) it is explained as follows:

"So let's say you want to access an i2c adapter from a C program. The
first thing to do is "#include <linux/i2c-dev.h>". Please note that
there are two files named "i2c-dev.h" out there, one is distributed
with the Linux kernel and is meant to be included from kernel
driver code, the other one is distributed with i2c-tools and is
meant to be included from user-space programs. You obviously want
the second one here."

>We also carry a patch similar to this. Our patch has an addition which is a dependency on linux (assuming it is selected in the config). This ensures that the >header from the kernel gets over ridden by the one from i2c-tools and not the other way around.

Makes sense. Could this patch be mainlined, instead of what I sent?

Regards
	- Jouko
Thomas Petazzoni - March 3, 2014, 9:10 p.m.
Dear Jouko Nikula,

On Tue, 24 Sep 2013 13:18:43 +0300, Jouko Nikula wrote:
> Signed-off-by: Jouko Nikula <jouko.nikula@espotel.com>
> ---
>  package/i2c-tools/i2c-tools.mk |    5 +++++
>  1 file changed, 5 insertions(+)

Applied, thanks.

Thomas
Thomas Petazzoni - March 4, 2014, 6:14 p.m.
Hello,

On Mon, 3 Mar 2014 22:10:28 +0100, Thomas Petazzoni wrote:
> Dear Jouko Nikula,
> 
> On Tue, 24 Sep 2013 13:18:43 +0300, Jouko Nikula wrote:
> > Signed-off-by: Jouko Nikula <jouko.nikula@espotel.com>
> > ---
> >  package/i2c-tools/i2c-tools.mk |    5 +++++
> >  1 file changed, 5 insertions(+)
> 
> Applied, thanks.

In fact, I will revert it: the modified i2c-dev.h by the i2c-tools
has some definitions that conflicts with the i2c.h normally provided by
the kernel headers. Therefore, if a userspace program or library
includes both, it fails to build. This is the case with libsoc, see
http://autobuild.buildroot.org/results/1f4/1f451a338487a2a3c8a8f9b18540d41b90ee5aac/build-end.log:

In file included from ../lib/include/libsoc_i2c.h:2:0,
                 from i2c.c:11:
/home/test/test/1/output/host/usr/sh4-buildroot-linux-uclibc/sysroot/usr/include/linux/i2c-dev.h:38:8: error: redefinition of 'struct i2c_msg'
In file included from ../lib/include/libsoc_i2c.h:1:0,
                 from i2c.c:11:
/home/test/test/1/output/host/usr/sh4-buildroot-linux-uclibc/sysroot/usr/include/linux/i2c.h:68:8: note: originally defined here
In file included from ../lib/include/libsoc_i2c.h:2:0,
                 from i2c.c:11:
/home/test/test/1/output/host/usr/sh4-buildroot-linux-uclibc/sysroot/usr/include/linux/i2c-dev.h:90:7: error: redefinition of 'union i2c_smbus_data'
In file included from ../lib/include/libsoc_i2c.h:1:0,
                 from i2c.c:11:
/home/test/test/1/output/host/usr/sh4-buildroot-linux-uclibc/sysroot/usr/include/linux/i2c.h:128:7: note: originally defined here
make[2]: *** [libsoc_la-i2c.lo] Error 1

Apparently, it is well-known to the i2c-tools developers that their
i2c-dev.h is broken, and not really usable in practice.

Best regards,

Thomas

Patch

diff --git a/package/i2c-tools/i2c-tools.mk b/package/i2c-tools/i2c-tools.mk
index 1589d04..3281dbe 100644
--- a/package/i2c-tools/i2c-tools.mk
+++ b/package/i2c-tools/i2c-tools.mk
@@ -7,6 +7,7 @@ 
 I2C_TOOLS_VERSION = 3.1.0
 I2C_TOOLS_SOURCE = i2c-tools-$(I2C_TOOLS_VERSION).tar.bz2
 I2C_TOOLS_SITE = http://dl.lm-sensors.org/i2c-tools/releases
+I2C_TOOLS_INSTALL_STAGING = YES
 
 define I2C_TOOLS_BUILD_CMDS
  $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D)
@@ -19,4 +20,8 @@  define I2C_TOOLS_INSTALL_TARGET_CMDS
 	done
 endef
 
+define I2C_TOOLS_INSTALL_STAGING_CMDS
+	$(INSTALL) -D -m644 $(@D)/include/linux/i2c-dev.h $(STAGING_DIR)/usr/include/linux/
+endef
+
 $(eval $(generic-package))