RE: [PATCH 3/3] ACPI: Evaluate _CRS while creating device nodeobjects

[Moore, Robert]

Rafael,

I sounds like with a few changes, we can enhance this mechanism to be more useful to you and others. Some comments below. I need to look at the code in question a bit more, but I see no insurmountable issues.

Bob


> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@xxxxxxx]
> Sent: Tuesday, November 13, 2012 2:57 PM
> To: Moore, Robert
> Cc: Mika Westerberg; mathias.nyman@xxxxxxxxxxxxxxx; linux-
> acpi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; lenb@xxxxxxxxxx;
> Wysocki, Rafael J; broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx;
> grant.likely@xxxxxxxxxxxx; linus.walleij@xxxxxxxxxx; khali@xxxxxxxxxxxx;
> Bjorn Helgaas; Zheng, Lv
> Subject: Re: [PATCH 3/3] ACPI: Evaluate _CRS while creating device node
> objects
> 
> On Tuesday, November 13, 2012 10:06:03 PM Moore, Robert wrote:
> > I may not quite understand what you are asking for, but I will try.
> > It seems like we already have much of what you want/need, so maybe I'm
> > missing something.
> 
> I think all of the necessary pieces are there.
> 
> > > So what I would like to have, in general terms, is something like
> > > acpi_walk_resources() split into three parts:
> > >
> > >  (1) One that processes the _CRS output and creates a list of
> > >      struct acpi_resource objects for us to play with.  I suppose
> > >      it's OK if that's just a buffer filled with resource objects,
> > >      but a linked list might be more convenient.
> > >
> >
> > This sounds like AcpiGetCurrentResources. It executes _CRS and formats
> > the data into acpi_resource objects.
> 
> Yes, it does.  However, it is not completely clear to me if/how the caller
> is supposed to prepare the buffer object pointed to by the second arg.
> 
> If the buffer is initialized by AcpiGetCurrentResources, then that's what
> I need for (1).


It looks to me that at least AcpiGetCurrentResources does not actually ever allocate a buffer for the resource template, it expects the caller to eventually provide one of at least the size of the returned resource template.

This is really quite a bit out-of-date as far as the memory allocation model. It should also support the option to just allocate the buffer of the appropriate size before returning it to the caller.




> 
> > >  (2) One that allows us to access (read/write) resources in the
> > >      list returned by (1).  We don't need to open code walking
> > >      the list and I probably wouldn't event want to do that.  What
> > >      we need is to be able to walk the same list for a number of
> > >      times and possibly to modify values in the resource objects
> > >      if there are conflicts.
> >
> > This sounds like AcpiWalkResources. I suppose a possible issue is that
> > currently, AcpiWalkResources actually invokes the _CRS, _PRS, or _AEI
> > method on behalf of the caller.
> 
> Yes, that exactly is the problem.
> 
> > It might make more sense to allow the caller to pass in the resource
> > buffer returned from a call to _CRS, etc.
> 
> Yes! :-)


I'll take a closer look at this tomorrow.




> 
> > >
> > >  (3) One allowing us to free the list returned by (1) if not needed
> > >      any more.
> > >
> >
> > AcpiGetCurrentResources: Currently, everything is returned in a single
> > buffer to minimize the number of allocations. A buffer you can free
> > when you are done with it.
> 
> I suppose I should use ACPI_FREE(buffer.pointer) for that, but isn't it
> for the ACPICA's internal use only?
> 
> Besides, I would prefer to be able to pass just "buffer" for freeing,
> without having to touch its internals.  No big deal, but it would be
> nicer. :-)


The ACPI_BUFFER type is in fact a public type that is meant to return both the buffer and the (actual) length. You will find many instances of ACPI_FREE(buffer.pointer) within existing linux code, since it also used for objects returned by control method execution/object evaluation.


> 
> > I think I saw where you mentioned that you cannot copy this buffer
> > because of internal pointers to other areas of the buffer. Yes.
> > However, we can build linked lists all day if you really want them :-)
> 
> I really won't care if I can pass a resource buffer to a "walker" routine.
> :-)
> 
> > > And it would be great if we could take the list returned by (1),
> > > modify the resources in it and feed it back to _SRS (after
> > > conversion back to the format that _SRS understands).
> > >
> >
> > AcpiSetCurrentResources.
> >
> > The AML debugger already has a command that illustrates the use of the
> > various resource interfaces, see dbcmds.c
> 
> I will.
> 
> Thanks,
> Rafael
> 
> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ0¶ìh®å’i