[PATCH] acpi: device: Fix check for sequence number
Currently the function acpi_check_seq() checks whether dev->req_seq is unequal to "-1", but it should actually check dev->seq. Change it to check dev->seq.
For req_seq the value "-1" would be a valid (meaning 'any'), while for seq the value "-1" means 'none' and is not valid.
Quoting the description of udevice in device.h: * @req_seq: Requested sequence number for this device (-1 = any) * @seq: Allocated sequence number for this device (-1 = none). * This is set up when the device is probed and will be unique * within the device's uclass.
Signed-off-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
Fixes: commit fefac0b0643b ("dm: acpi: Enhance acpi_get_name()")
---
lib/acpi/acpi_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c index 3c75b6d962..76194ea1b4 100644 --- a/lib/acpi/acpi_device.c +++ b/lib/acpi/acpi_device.c @@ -743,12 +743,12 @@ static const char *acpi_name_from_id(enum uclass_id id)
static int acpi_check_seq(const struct udevice *dev) { - if (dev->req_seq == -1) { + if (dev->seq == -1) { log_warning("Device '%s' has no seq\n", dev->name); return log_msg_ret("no seq", -ENXIO); }
- return dev->req_seq; + return dev->seq; }
/* If you change this function, add test cases to dm_test_acpi_get_name() */
Hi Wolfgang,
On Thu, 30 Jul 2020 at 06:47, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Currently the function acpi_check_seq() checks whether dev->req_seq is unequal to "-1", but it should actually check dev->seq. Change it to check dev->seq.
For req_seq the value "-1" would be a valid (meaning 'any'), while for seq the value "-1" means 'none' and is not valid.
Quoting the description of udevice in device.h:
- @req_seq: Requested sequence number for this device (-1 = any)
- @seq: Allocated sequence number for this device (-1 = none).
This is set up when the device is probed and will be uniquewithin the device's uclass.Signed-off-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
Fixes: commit fefac0b0643b ("dm: acpi: Enhance acpi_get_name()")
lib/acpi/acpi_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
What problem are you seeing without this patch?
At present the ACPI device may not always be probed, and probing is when the sequence number is currently set up.
I have been thinking about dropping req_seq and doing everything when the device is bound, but haven't dug into it in detail yet.
Regards, SImon
Hi Simon,
-----"Simon Glass" sjg@chromium.org schrieb: -----
Betreff: Re: [PATCH] acpi: device: Fix check for sequence number
Hi Wolfgang,
On Thu, 30 Jul 2020 at 06:47, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Currently the function acpi_check_seq() checks whether dev->req_seq is unequal to "-1", but it should actually check dev->seq. Change it to check dev->seq.
For req_seq the value "-1" would be a valid (meaning 'any'), while for seq the value "-1" means 'none' and is not valid.
Quoting the description of udevice in device.h:
- @req_seq: Requested sequence number for this device (-1 = any)
- @seq: Allocated sequence number for this device (-1 = none).
This is set up when the device is probed and will be uniquewithin the device's uclass.Signed-off-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
Fixes: commit fefac0b0643b ("dm: acpi: Enhance acpi_get_name()")
lib/acpi/acpi_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
What problem are you seeing without this patch?
I see the following warning: "Device 'serial@18,2' has no seq".
In my case req_seq for the UART is -1 ("any"), while seq is 0. Why would we check for req_seq and not for seq in this function?
At present the ACPI device may not always be probed, and probing is when the sequence number is currently set up.
In my case the UART is already probed before the ACPI tables are generated.
I have been thinking about dropping req_seq and doing everything when the device is bound, but haven't dug into it in detail yet.
regards, Wolfgang
Hi Wolfgang,
On Thu, 13 Aug 2020 at 01:23, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Simon,
-----"Simon Glass" sjg@chromium.org schrieb: -----
Betreff: Re: [PATCH] acpi: device: Fix check for sequence number
Hi Wolfgang,
On Thu, 30 Jul 2020 at 06:47, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Currently the function acpi_check_seq() checks whether dev->req_seq is unequal to "-1", but it should actually check dev->seq. Change it to check dev->seq.
For req_seq the value "-1" would be a valid (meaning 'any'), while for seq the value "-1" means 'none' and is not valid.
Quoting the description of udevice in device.h:
- @req_seq: Requested sequence number for this device (-1 = any)
- @seq: Allocated sequence number for this device (-1 = none).
This is set up when the device is probed and will be uniquewithin the device's uclass.Signed-off-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
Fixes: commit fefac0b0643b ("dm: acpi: Enhance acpi_get_name()")
lib/acpi/acpi_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
What problem are you seeing without this patch?
I see the following warning: "Device 'serial@18,2' has no seq".
In my case req_seq for the UART is -1 ("any"), while seq is 0. Why would we check for req_seq and not for seq in this function?
At present the ACPI device may not always be probed, and probing is when the sequence number is currently set up.
In my case the UART is already probed before the ACPI tables are generated.
I would expect req_seq to be set to the UART number, i.e. the value of the alias (uart0, uart1) that points to the node.
I wonder why that doesn't work in your case?
Are you sure that all UARTs are probed before ACPI tables are created? Normally U-Boot would only probe the one being used for the console.
I have been thinking about dropping req_seq and doing everything when the device is bound, but haven't dug into it in detail yet.
regards, Wolfgang
Regards, Simon
Hi Simon,
-----"Simon Glass" sjg@chromium.org schrieb: -----
Betreff: Re: [PATCH] acpi: device: Fix check for sequence number
Hi Wolfgang,
On Thu, 13 Aug 2020 at 01:23, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Simon,
-----"Simon Glass" sjg@chromium.org schrieb: -----
Betreff: Re: [PATCH] acpi: device: Fix check for sequence number
Hi Wolfgang,
On Thu, 30 Jul 2020 at 06:47, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Currently the function acpi_check_seq() checks whether dev->req_seq is unequal to "-1", but it should actually check dev->seq. Change it to check dev->seq.
For req_seq the value "-1" would be a valid (meaning 'any'), while for seq the value "-1" means 'none' and is not valid.
Quoting the description of udevice in device.h:
- @req_seq: Requested sequence number for this device (-1 = any)
- @seq: Allocated sequence number for this device (-1 = none).
This is set up when the device is probed and will be uniquewithin the device's uclass.Signed-off-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
Fixes: commit fefac0b0643b ("dm: acpi: Enhance acpi_get_name()")
lib/acpi/acpi_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
What problem are you seeing without this patch?
I see the following warning: "Device 'serial@18,2' has no seq".
In my case req_seq for the UART is -1 ("any"), while seq is 0. Why would we check for req_seq and not for seq in this function?
At present the ACPI device may not always be probed, and probing is when the sequence number is currently set up.
In my case the UART is already probed before the ACPI tables are generated.
I would expect req_seq to be set to the UART number, i.e. the value of the alias (uart0, uart1) that points to the node.
I wonder why that doesn't work in your case?
I did not have an alias for my serial. I have added one and now it works as expected.
I misunderstood how that code is expected to work. Thanks for the explanation, now it makes sense.
This also means that my patch is wrong and should be dropped. @Bin: please drop this patch.
Are you sure that all UARTs are probed before ACPI tables are created? Normally U-Boot would only probe the one being used for the console.
I have been thinking about dropping req_seq and doing everything when the device is bound, but haven't dug into it in detail yet.
regards, Wolfgang
Hi Wolfgang,
On Thu, Sep 10, 2020 at 3:01 PM Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Simon,
-----"Simon Glass" sjg@chromium.org schrieb: -----
Betreff: Re: [PATCH] acpi: device: Fix check for sequence number
Hi Wolfgang,
On Thu, 13 Aug 2020 at 01:23, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Simon,
-----"Simon Glass" sjg@chromium.org schrieb: -----
Betreff: Re: [PATCH] acpi: device: Fix check for sequence number
Hi Wolfgang,
On Thu, 30 Jul 2020 at 06:47, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Currently the function acpi_check_seq() checks whether dev->req_seq is unequal to "-1", but it should actually check dev->seq. Change it to check dev->seq.
For req_seq the value "-1" would be a valid (meaning 'any'), while for seq the value "-1" means 'none' and is not valid.
Quoting the description of udevice in device.h:
- @req_seq: Requested sequence number for this device (-1 = any)
- @seq: Allocated sequence number for this device (-1 = none).
This is set up when the device is probed and will be uniquewithin the device's uclass.Signed-off-by: Wolfgang Wallner wolfgang.wallner@br-automation.com
Fixes: commit fefac0b0643b ("dm: acpi: Enhance acpi_get_name()")
lib/acpi/acpi_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
What problem are you seeing without this patch?
I see the following warning: "Device 'serial@18,2' has no seq".
In my case req_seq for the UART is -1 ("any"), while seq is 0. Why would we check for req_seq and not for seq in this function?
At present the ACPI device may not always be probed, and probing is when the sequence number is currently set up.
In my case the UART is already probed before the ACPI tables are generated.
I would expect req_seq to be set to the UART number, i.e. the value of the alias (uart0, uart1) that points to the node.
I wonder why that doesn't work in your case?
I did not have an alias for my serial. I have added one and now it works as expected.
I misunderstood how that code is expected to work. Thanks for the explanation, now it makes sense.
This also means that my patch is wrong and should be dropped. @Bin: please drop this patch.
Thanks. Have updated the status of this patch in patchwork.
Are you sure that all UARTs are probed before ACPI tables are created? Normally U-Boot would only probe the one being used for the console.
I have been thinking about dropping req_seq and doing everything when the device is bound, but haven't dug into it in detail yet.
Regards, Bin
participants (3)
-
Bin Meng -
Simon Glass -
Wolfgang Wallner