Re: [PATCH v4 3/3] selftest/gpio: add gpio test case

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Shuah

On 2016/11/17 7:42, Shuah Khan wrote:
On 10/21/2016 05:54 AM, Linus Walleij wrote:
On Fri, Oct 14, 2016 at 4:48 AM, Bamvor Jian Zhang
<bamvor.zhangjian@xxxxxxxxxx> wrote:

From: Bamvor Jian Zhang <bamvor.zhangjian@xxxxxxxxxx>

This test script try to do whitebox testing for gpio subsystem(
based on gpiolib). It manipulate gpio device through chardev or
sysfs and check the result from debugfs. This script test
gpio-mockup through chardev by default.

In details, it test the following things:
1.  Add single, multi gpiochip to do overlap check.
2.  Test direction and output value for valid pin.
3.  Test dynamic allocation of gpio base.

Run "tools/testing/selftests/gpio/gpio-mockup.sh -h" to know how to
use it.

Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@xxxxxxxxxx>

I like the overall idea with this, but some comments:

+CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/

Is this really what people are doing?

Isn't -I/usr/include the right way to express this?

+LDLIBS += -lmount -I/usr/include/libmount
+
+$(BINARIES): ../../../gpio/gpio-utils.o ../../../../usr/include/linux/gpio.h
+
+../../../gpio/gpio-utils.o:
+       make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio

Ah, that is clever. I hope I can get an ACK from Shuah if this is
how we're supposed to handle cross dependencies of common
helper code in the kernel.

It looks a bit spaghetti, unfortunately.

Yeah it does. Let's get this in for now and we can try to improve
it.

Sorry for the delay, here is the ack



+../../../../usr/include/linux/gpio.h:
+       make -C ../../../.. headers_install

Don't do this please, you would have to be root and it's very
fragile. How does tests in general resolve dependencies on
kernel headers? Please look around a bit and check what
they do. I think they just expect them to be installed.

Yeah - it is a good idea to have the user install the headers.
It shouldn't be done in the script without use consent.
I am tring to follow you. Do you want a better solution or just
install to usr/include(and update all the similar issue in
tools or selftests in another patch)?

../../../../usr/include/linux/gpio.h:
        make -C ../../../.. headers_install INSTALL_HDR_PATH=`pwd`/../../../../usr/


I like the tests overall, but I'm a bit suspicious about the
sysfs tests. Maybe these should atleast print something
about the sysfs ABI being deprecated.

Has this concern been addressed. Once the above are fixed.
Sure. I will add a warning for sysfs ABI of gpio.

Regards

Bamvor

Acked-by: Shuah Khan <shuahkh@xxxxxxxxxxxxxxx>


Anyways I merged the first two patches so now we only have to
figure out this final patch!

Acked-by: Shuah Khan <shuahkh@xxxxxxxxxxxxxxx>



Yours,
Linus Walleij



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



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux