Hi!
On 11/12/2013 09:31 PM, [email protected] wrote:
Hi!
Obsolete code updated to the actual kernel ACPI API.
Changed test-cases handling from ioctl to sysfs.
User-space program rewritten.
User-space program loads/unloads kernel module.
Added new test-cases: traverse ACPI devices and sysfs support
for device description.
Signed-off-by: Alexey Kodanev <[email protected]>
---
testcases/kernel/device-drivers/acpi/.gitignore | 8 +
testcases/kernel/device-drivers/acpi/LtpAcpi.h | 120 +--
testcases/kernel/device-drivers/acpi/LtpAcpiCmds.c | 922 +++++++++-----------
Can you get rid of the MixedCase filenames as well?
Ideally in a follow up patch (and use git mv).
OK
+static int acpi_failure(acpi_status status, int *err, const char *name)
{
- printk(KERN_ALERT "ltpdev_open \n");
+ if (ACPI_FAILURE(status)) {
+ ACPI_EXCEPTION((AE_INFO, status, name));
+ *err = 1;
+ return 1;
+ }
return 0;
}
Hmm, I don't like that the status is both returned and set to the err
variable. It forces you to add dummy variables into the code of the
caller, acpi_traverse_from_root() for example.
What is wrong with:
err |= acpi_error(status, "err msg");
if (err)
return err;
or with:
return acpi_error(status, "err msg");
Or use acpi_error() only to print errors and use status directly...
OK, I'll use the one which returns err.
-static int ltpdev_release(struct gendisk *disk, fmode_t mode)
+static int acpi_get_info(acpi_handle h, int *type, char name[])
{
+ int err = 0;
+ struct acpi_device_info *dev_info;
+ acpi_status status;
+ status = acpi_get_object_info(h, &dev_info);
- printk(KERN_ALERT "ltpdev_release \n");
- return 0;
-}
+ if (acpi_failure(status, &err, "acpi_object_info failed"))
+ return err;
-static u32 ltp_test_power_button_ev_handler(void *context)
-{
- printk(KERN_ALERT "ltp_test_power_button_ev_handler \n");
- return 1;
-}
+ *type = dev_info->type;
-static u32 ltp_test_sleep_button_ev_handler(void *context)
-{
- printk(KERN_ALERT "ltp_test_sleep_button_ev_handler \n");
- return 1;
+ sprintf(name, "%4.4s", (char *)&dev_info->name);
It would be easier just to return the dev_info and free it later in the
caller, that would save us the need to copy the data. Given that both
the functions that use this one have just one return on the codepath
after this function is called all that would be needed is just one free
before the return.
dev_info = acpi_get_info(handle);
if (!dev_info)
return error;
kfree(dev_info);
return success;
Yes, it makes sense, I will rewrite.
+ kfree(dev_info);
+ return err;
}
-static int ltpdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
- unsigned long arg)
+static int level;
+static char ind[64];
+
+static void set_indent(void)
{
- acpi_status status;
-// acpi_handle sys_bus_handle;
- acpi_handle start_handle = 0;
- acpi_handle parent_handle;
- acpi_handle child_handle;
- acpi_handle next_child_handle;
- acpi_status level;
- struct acpi_ec *ec;
- struct acpi_device *device;
- struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ memset(ind, 0x20, sizeof(ind));
+ ind[level * 4] = '\0';
+}
Are you sure that the maximal level in the ACPI structure is <= 15?
What about sanity check for a level value in the acpi_traverse() before
you call it recursively?
Hmm and you don't have to do memset on each set_indent(), if you create
a static ind array full of spaces with terminating null and use pointer
to a string to point inside of the array.
I see. I will check the value and use a pointer to the static array of
spaces.
-
- status = acpi_get_handle(0, ACPI_NS_SYSTEM_BUS, &parent_handle);
+ buf = kmalloc(str_obj->buffer.length / 2, GFP_KERNEL);
Check for kmalloc() failure?
OK
+
+
+ char *cmd;
+ SAFE_ASPRINTF(cleanup, &cmd,
+ "find /sys/devices -name description "
+ "| xargs grep '%s' > /dev/null 2>&1", buf);
+
+ int res = system(cmd);
+ free(cmd);
Hmm, if I understand this right we have _STR attribute from first device
that has this attribute and we are trying to assert that it exists in
the sysfs tree.
What about asserting that all _STR attributes are there?
I.e. change the module to write next _STR attribute each time the sysfs
file si read (we would need to emulate the recursion stack in an array).
And build a table of all _STR attributes in the userspace and check that
each attribute read from the module is in the table?
Yes, it's possible. Fortunately, I can get sysfs paths of acpi devices
from its structures, so I don't need the strings table. It seems some
devices don't have sysfs path but have _STR attribute, so the test will
check it also.
------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
