Skip to content

Commit

Permalink
gpio-cdev: move kmod-leds-uleds dependency to MX100
Browse files Browse the repository at this point in the history 
The inclusion of the kmod-leds-uleds into the userspace
nu801 package causes a circular dependency inside the
buildsystem... which causes it to be picked regardless
of other DEPENDS values.

In case of the mx100, this could be solved by moving the
kmod-leds-uled dependency to the kmod-meraki-mx100.

Bonus: drop @!LINUX_5_4 from kmod-meraki-mx100
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
  • Loading branch information
@chunkeey
chunkeey committed Mar 27, 2022
1 parent 7368345 commit eeb8fd4
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion package/system/gpio-cdev/nu801/Makefile
View file
Expand Up @@ -20,7 +20,7 @@ define Package/nu801
SECTION:=utils
CATEGORY:=Utilities
SUBMENU:=Userspace GPIO Drivers
DEPENDS:=@TARGET_x86 +kmod-leds-uleds
DEPENDS:=@TARGET_x86
KCONFIG:=CONFIG_GPIO_CDEV=y
TITLE:=NU801 LED Driver
endef
Expand Down
4 changes: 2 additions & 2 deletions target/linux/x86/modules.mk
View file
Expand Up @@ -87,8 +87,8 @@ $(eval $(call KernelPackage,pcengines-apuv2))
define KernelPackage/meraki-mx100
SUBMENU:=$(OTHER_MENU)
TITLE:=Cisco Meraki MX100 Platform Driver
DEPENDS:=@TARGET_x86 @!LINUX_5_4 +kmod-tg3 +kmod-gpio-button-hotplug +kmod-leds-gpio \
+kmod-usb-ledtrig-usbport +nu801 +kmod-itco-wdt
DEPENDS:=@TARGET_x86 +kmod-tg3 +kmod-gpio-button-hotplug +kmod-leds-gpio \
+kmod-usb-ledtrig-usbport +nu801 +kmod-itco-wdt +kmod-leds-uleds
KCONFIG:=CONFIG_MERAKI_MX100
FILES:=$(LINUX_DIR)/drivers/platform/x86/meraki-mx100.ko
AUTOLOAD:=$(call AutoLoad,60,meraki-mx100,1)
Expand Down

6 comments on commit eeb8fd4

@Grommish
Copy link
Contributor

@Grommish Grommish commented on eeb8fd4 Mar 30, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

FAILED: CMakeFiles/nu801.dir/nu801.c.o
/home/grommish/openwrt/staging_dir/toolchain-mips64_octeonplus_64_gcc-11.2.0_musl/bin/mips64-openwrt-linux-musl-gcc -D_GNU_SOURCE  -Os -pipe -mno-branch-likely -march=octeon+ -mabi=64 -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -msoft-float -fmacro-prefix-map=/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c=nu801-d9942c0c -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -DNDEBUG -MD -MT CMakeFiles/nu801.dir/nu801.c.o -MF CMakeFiles/nu801.dir/nu801.c.o.d -o CMakeFiles/nu801.dir/nu801.c.o -c /home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c
In file included from /home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:35:
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/gpio-utils.h:30:35: warning: 'struct gpio_v2_line_config' declared inside parameter list will not be visible outside of this definition or declaration
   30 |                            struct gpio_v2_line_config *config,
      |                                   ^~~~~~~~~~~~~~~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/gpio-utils.h:32:47: warning: 'struct gpio_v2_line_values' declared inside parameter list will not be visible outside of this definition or declaration
   32 | int gpiotools_set_values(const int fd, struct gpio_v2_line_values *values);
      |                                               ^~~~~~~~~~~~~~~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/gpio-utils.h:33:47: warning: 'struct gpio_v2_line_values' declared inside parameter list will not be visible outside of this definition or declaration
   33 | int gpiotools_get_values(const int fd, struct gpio_v2_line_values *values);
      |                                               ^~~~~~~~~~~~~~~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/gpio-utils.h: In function 'gpiotools_set_bit':
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/gpio-utils.h:47:15: warning: implicit declaration of function '_BITULL' [-Wimplicit-function-declaration]
   47 |         *b |= _BITULL(n);
      |               ^~~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c: At top level:
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:125:15: error: variable 'values' has initializer but incomplete type
  125 | static struct gpio_v2_line_values values = { 0 };
      |               ^~~~~~~~~~~~~~~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:125:46: warning: excess elements in struct initializer
  125 | static struct gpio_v2_line_values values = { 0 };
      |                                              ^
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:125:46: note: (near initialization for 'values')
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c: In function 'register_gpio':
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:177:16: error: variable 'config' has initializer but incomplete type
  177 |         struct gpio_v2_line_config config = { 0 };
      |                ^~~~~~~~~~~~~~~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:177:47: warning: excess elements in struct initializer
  177 |         struct gpio_v2_line_config config = { 0 };
      |                                               ^
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:177:47: note: (near initialization for 'config')
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:177:36: error: storage size of 'config' isn't known
  177 |         struct gpio_v2_line_config config = { 0 };
      |                                    ^~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:187:24: error: 'GPIO_V2_LINE_FLAG_OUTPUT' undeclared (first use in this function); did you mean 'GPIOLINE_FLAG_IS_OUT'?
  187 |         config.flags = GPIO_V2_LINE_FLAG_OUTPUT;
      |                        ^~~~~~~~~~~~~~~~~~~~~~~~
      |                        GPIOLINE_FLAG_IS_OUT
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:187:24: note: each undeclared identifier is reported only once for each function it appears in
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:212:42: error: invalid use of undefined type 'struct gpio_v2_line_values'
  212 |                 gpiotools_set_bit(&values.mask, i);
      |                                          ^
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:215:46: warning: passing argument 2 of 'gpiotools_get_values' from incompatible pointer type [-Wincompatible-pointer-types]
  215 |         ret = gpiotools_get_values(_gpio_fd, &values);
      |                                              ^~~~~~~
      |                                              |
      |                                              struct gpio_v2_line_values *
In file included from /home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:35:
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/gpio-utils.h:33:68: note: expected 'struct gpio_v2_line_values *' but argument is of type 'struct gpio_v2_line_values *'
   33 | int gpiotools_get_values(const int fd, struct gpio_v2_line_values *values);
      |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:222:23: error: invalid use of undefined type 'struct gpio_v2_line_values'
  222 |                 values.bits, values.mask);
      |                       ^
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:133:56: note: in definition of macro 'DPRINTF'
  133 | #define DPRINTF(fmt, ...) { if (debug) printf((fmt), ##__VA_ARGS__); }
      |                                                        ^~~~~~~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:222:36: error: invalid use of undefined type 'struct gpio_v2_line_values'
  222 |                 values.bits, values.mask);
      |                                    ^
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:133:56: note: in definition of macro 'DPRINTF'
  133 | #define DPRINTF(fmt, ...) { if (debug) printf((fmt), ##__VA_ARGS__); }
      |                                                        ^~~~~~~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c: In function 'gpio_set':
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:229:37: error: invalid use of undefined type 'struct gpio_v2_line_values'
  229 |         gpiotools_assign_bit(&values.bits, gpio, state);
      |                                     ^
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c: In function 'gpio_commit':
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:234:39: warning: passing argument 2 of 'gpiotools_set_values' from incompatible pointer type [-Wincompatible-pointer-types]
  234 |         gpiotools_set_values(gpio_fd, &values);
      |                                       ^~~~~~~
      |                                       |
      |                                       struct gpio_v2_line_values *
In file included from /home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:35:
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/gpio-utils.h:32:68: note: expected 'struct gpio_v2_line_values *' but argument is of type 'struct gpio_v2_line_values *'
   32 | int gpiotools_set_values(const int fd, struct gpio_v2_line_values *values);
      |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c: At top level:
/home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/nu801.c:125:35: error: storage size of 'values' isn't known
  125 | static struct gpio_v2_line_values values = { 0 };
      |                                   ^~~~~~
ninja: build stopped: subcommand failed.
make[3]: *** [Makefile:42: /home/grommish/openwrt/build_dir/target-mips64_octeonplus_64_musl/nu801-d9942c0c/.built] Error 1
make[3]: Leaving directory '/home/grommish/openwrt/package/system/gpio-cdev/nu801'
time: package/system/gpio-cdev/nu801/compile#0.27#0.06#0.28
    ERROR: package/system/gpio-cdev/nu801 failed to build.
make[2]: *** [package/Makefile:116: package/system/gpio-cdev/nu801/compile] Error 1
make[2]: Leaving directory '/home/grommish/openwrt'
make[1]: *** [package/Makefile:110: /home/grommish/openwrt/staging_dir/target-mips64_octeonplus_64_musl/stamp/.package_compile] Error 2
make[1]: Leaving directory '/home/grommish/openwrt'
make: *** [/home/grommish/openwrt/include/toplevel.mk:230: world] Error 2

Seeing this error, rebased to 8822a8d

This package should not be attempting to build, as the target is MIPS64 and not x86

Edit: I have had to revert to the 5.4 kernel due to the Octeon memleak issue. I suspect @!LINUX_5_4 being removed exposed this, however, @TARGET_x86 should still keep it from building.

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on eeb8fd4 Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chunkeey
hi
this change would not change anything, it still cause the compile of nu801
I think we should drop depends on nu801 in target/linux/x86/modules.mk

diff --git a/target/linux/x86/modules.mk b/target/linux/x86/modules.mk
index f3226e3a60..33cbbf5f06 100644
--- a/target/linux/x86/modules.mk
+++ b/target/linux/x86/modules.mk
@@ -88,7 +88,7 @@ define KernelPackage/meraki-mx100
   SUBMENU:=$(OTHER_MENU)
   TITLE:=Cisco Meraki MX100 Platform Driver
   DEPENDS:=@TARGET_x86 +kmod-tg3 +kmod-gpio-button-hotplug +kmod-leds-gpio \
-    +kmod-usb-ledtrig-usbport +nu801 +kmod-itco-wdt +kmod-leds-uleds
+    +kmod-usb-ledtrig-usbport +kmod-itco-wdt +kmod-leds-uleds
   KCONFIG:=CONFIG_MERAKI_MX100
   FILES:=$(LINUX_DIR)/drivers/platform/x86/meraki-mx100.ko
   AUTOLOAD:=$(call AutoLoad,60,meraki-mx100,1)

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on eeb8fd4 Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not reasonable that kmod depends on app, invert is good

@chunkeey
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ptpt52 I think I know where the gpio-cdev is coming from. It's pulled in as a compile dependency for the kernel. With
this patch, it should no longer built on anything other than x86.

https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=9a137477811611f6363fb049b8356457798169a6

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on eeb8fd4 May 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chunkeey
but I still do not understand why the kmod-meraki-mx100 need to depends on the nu801
kmod-meraki-mx100 build would failed if no nu801 ? or what?

@chunkeey
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reason being that x86 has no per-device target device definitions for the supported devices. There are just the
generic squashfs/ext4- images that are supposed to work on every x86 device. So, the kmod-meraki-mx100 is
used as a stand-in-target for the MX100. I.e. once you select it (in the chef/imagebuilder), it pulls in the necessary
ethernet drivers (tg3) and status-led (nu801) for the mx100 to work.

As for "it is not reasonable that kmod depends on app, invert is good". Yes, this setup is an odd one.
Though the invert way also comes with its own problems:

#4846 (spidev-test pulls in kmod-spi-dev and this breaks SDK builts for spidev-test).

(nu801 also works for the ath79-ized Meraki MR18, though this is out-of-tree because
adding it as-is would cause other, already supported ath79 nand to break.)

Please sign in to comment.