Re: [Qemu-devel] [PATCH v6 05/22] hw/acpi/aml-build: Add aml_interrupt() term

[Shannon Zhao]

On 2015/5/15 20:14, Igor Mammedov wrote:
> On Thu, 7 May 2015 16:51:31 +0100
> Peter Maydell <address@hidden> wrote:
> 
>> > On 7 May 2015 at 10:29, Shannon Zhao <address@hidden> wrote:
>>> > > From: Shannon Zhao <address@hidden>
>>> > >
>>> > > Add aml_interrupt() for describing device interrupt in resource 
>>> > > template.
>>> > > These can be used to generating DSDT table for ACPI on ARM.
>> > 
>>> > > +    /* Interrupt Number */
>>> > > +    build_append_byte(var->buf, extract32(irq, 0, 8)); /* bits[7:0] */
>>> > > +    build_append_byte(var->buf, extract32(irq, 8, 8)); /* bits[15:8] */
>>> > > +    build_append_byte(var->buf, extract32(irq, 16, 8)); /* bits[23:16] 
>>> > > */
>>> > > +    build_append_byte(var->buf, extract32(irq, 24, 8)); /* bits[31:24] 
>>> > > */
>> > 
>> > You used this twice in the previous patch and again here. Can
>> > we factor it out so we can
>> >   build_append_uint32(var->buf, irq);
> I'd prefer to leave it as it is now, yes it's a bit of code duplication
> but when you compare to spec it's 1:1 match and easy to compare
> 

I think we could use build_append_uint32 because this really makes the
code cleaner and it also matches the spec. When comparing to the spec,
uint32 means 4 bytes, so it implies that it includes this byte and next
3 bytes and it also says the 4 bytes are integral.

-- 
Shannon