Damien Zammit, le sam. 03 avril 2021 23:16:33 +1100, a ecrit:
> +acpi_status
> +acpi_os_create_lock(acpi_spinlock *lockp)
> +{
> + acpi_semaphore sema = NULL;
> + if (acpi_os_create_semaphore(1, 0, &sema) == AE_OK) {
> + *lockp = sema;
No, directly pass lockp to acpi_os_create_semaphore. You cannot assume
that copying the content of a lock can produce a working lock. It
happens to be ok for the sem_t implementation of GNU/Hurd, but that's
only a coincidence.
> +acpi_status
> +acpi_os_wait_semaphore(acpi_semaphore handle, u32 units, u16 timeout)
> +{
> + int i;
> +
> + if (!timeout)
> + timeout = 1;
> +
> + for (i = 0; i < timeout; i++) {
> + if (!sem_trywait(handle)) {
> + return AE_OK;
Is timeout really expected to be the number of *times* to try to take
the semaphore? Isn't it rather a time value that you can rather pass to
sem_timedwait?
> + if ((fd_mem = open("/dev/mem", O_RDWR)) < 0) {
> + acpi_os_printf("Can't open /dev/mem\n");
> + return (void *)-1;
> + }
> +
> + /* MAP_FIXED so we can access particular physical regions */
??? MAP_FIXED has nothing to do with the offset within the fd_mem. It is
about the address you pass as first argument, here NULL, so MAP_FIXED is
meaningless here.
> +acpi_status
> +acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
> + acpi_string *new_val)
> +{
> + if (!init_val || !new_val)
> + return AE_BAD_PARAMETER;
> +
> + *new_val = init_val->val;
> +
> + return AE_OK;
> +}
What is the rationale for this function? It looks odd that it'd to be
implemented so trivially.
Samuel
