| Message ID | 20170605204151.1563-1-andrew.smirnov@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello, On Mon, 5 Jun 2017 13:41:51 -0700, Andrey Smirnov wrote: > As can be seen in linux/arch/powerpc/include/uapi/asm/termbits.h those > constants are defined as (note that those are octal numbers): > > \#define TABDLY 00006000 > \#define CSIZE 00001400 > \#define CRDLY 00030000 > > which gives shifts of 10, 8 and 12. Adjust socat.mk accordingly to > reflect that difference. > > Signed-off-by: Mark Hinds <zoronic@gmail.com> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> Thanks for this patch. I don't really understand why they are using an AC_TRY_RUN() test for this. I'm pretty we can call the preprocessor to get the value, and then use shell to determine the shift. This would make the test cross-compilation compliant. Demo: $ cat test.c #include <termios.h> #include <unistd.h> TABDLY CRDLY CSIZE $ ./output/host/usr/bin/powerpc64le-linux-gcc -E -o - test.c | tail -3 00006000 00030000 00001400 So, no need to be on PowerPC64 to get those values. That being said, it's probably a bit a lot more work to do. So, some comments on the implementation. > +ifeq ($(ARCH),powerpc) This will only match PowerPC and not PowerPC64 and PowerPC64le, which I believe are in the same situation, so you should use: ifeq ($(BR2_powerpc)$(BR2_powerpc64)$(BR2_powerpc64le),y) > +SOCAT_CONF_ENV = \ > + sc_cv_termios_ispeed=no \ Please move this one outside the if, since it's the same in both cases. Thanks! Thomas
On Tue, Jun 6, 2017 at 1:09 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Hello, > > On Mon, 5 Jun 2017 13:41:51 -0700, Andrey Smirnov wrote: >> As can be seen in linux/arch/powerpc/include/uapi/asm/termbits.h those >> constants are defined as (note that those are octal numbers): >> >> \#define TABDLY 00006000 >> \#define CSIZE 00001400 >> \#define CRDLY 00030000 >> >> which gives shifts of 10, 8 and 12. Adjust socat.mk accordingly to >> reflect that difference. >> >> Signed-off-by: Mark Hinds <zoronic@gmail.com> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > > Thanks for this patch. I don't really understand why they are using an > AC_TRY_RUN() test for this. I'm pretty we can call the preprocessor to > get the value, and then use shell to determine the shift. This would > make the test cross-compilation compliant. Demo: > > $ cat test.c > #include <termios.h> > #include <unistd.h> > TABDLY > CRDLY > CSIZE > $ ./output/host/usr/bin/powerpc64le-linux-gcc -E -o - test.c | tail -3 > 00006000 > 00030000 > 00001400 > > So, no need to be on PowerPC64 to get those values. > > That being said, it's probably a bit a lot more work to do. So, some > comments on the implementation. > Agreed on both accounts. >> +ifeq ($(ARCH),powerpc) > > This will only match PowerPC and not PowerPC64 and PowerPC64le, which I > believe are in the same situation, so you should use: > > ifeq ($(BR2_powerpc)$(BR2_powerpc64)$(BR2_powerpc64le),y) > >> +SOCAT_CONF_ENV = \ >> + sc_cv_termios_ispeed=no \ > > Please move this one outside the if, since it's the same in both cases. > Good points, will do in v2. Thanks, Andrey Smirnov
diff --git a/package/socat/socat.mk b/package/socat/socat.mk index ba46263..a85c22d 100644 --- a/package/socat/socat.mk +++ b/package/socat/socat.mk @@ -9,11 +9,19 @@ SOCAT_SOURCE = socat-$(SOCAT_VERSION).tar.bz2 SOCAT_SITE = http://www.dest-unreach.org/socat/download SOCAT_LICENSE = GPL-2.0 SOCAT_LICENSE_FILES = COPYING +ifeq ($(ARCH),powerpc) +SOCAT_CONF_ENV = \ + sc_cv_termios_ispeed=no \ + sc_cv_sys_crdly_shift=12 \ + sc_cv_sys_tabdly_shift=10 \ + sc_cv_sys_csize_shift=8 +else SOCAT_CONF_ENV = \ sc_cv_termios_ispeed=no \ sc_cv_sys_crdly_shift=9 \ sc_cv_sys_tabdly_shift=11 \ sc_cv_sys_csize_shift=4 +endif # We need to run autoconf to regenerate the configure script, in order # to ensure that the test checking linux/ext2_fs.h works