On 2015/5/15 20:14, Igor Mammedov wrote: > On Thu, 7 May 2015 16:51:31 +0100 > Peter Maydell <peter.mayd...@linaro.org> wrote: > >> > On 7 May 2015 at 10:29, Shannon Zhao <zhaoshengl...@huawei.com> wrote: >>> > > From: Shannon Zhao <shannon.z...@linaro.org> >>> > > >>> > > 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