[PATCH] acpi: device: Fix check for sequence number

[Bin Meng]

Hi Wolfgang,

On Thu, Sep 10, 2020 at 3:01 PM Wolfgang Wallner
<wolfgang.wallner at br-automation.com> wrote:
>
> Hi Simon,
>
> -----"Simon Glass" <sjg at 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 at br-automation.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > -----"Simon Glass" <sjg at 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 at 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 unique
> > > > >  *       within the device's uclass.
> > > > >
> > > > > Signed-off-by: Wolfgang Wallner <wolfgang.wallner at 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 at 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