From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35697) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSlgK-0008Km-JJ for qemu-devel@nongnu.org; Mon, 08 Feb 2016 08:16:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aSlgF-0005ws-A2 for qemu-devel@nongnu.org; Mon, 08 Feb 2016 08:16:44 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42541) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSlgF-0005wo-4b for qemu-devel@nongnu.org; Mon, 08 Feb 2016 08:16:39 -0500 Date: Mon, 8 Feb 2016 15:16:35 +0200 From: "Michael S. Tsirkin" Message-ID: <20160208151512-mutt-send-email-mst@redhat.com> References: <1453816225-139685-1-git-send-email-imammedo@redhat.com> <1453816225-139685-4-git-send-email-imammedo@redhat.com> <20160207102341-mutt-send-email-mst@redhat.com> <20160208130037.GA6420@rkaganb.sw.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160208130037.GA6420@rkaganb.sw.ru> Subject: Re: [Qemu-devel] [PATCH v7 3/3] i386: populate floppy drive information in DSDT List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Kagan , Igor Mammedov , qemu-devel@nongnu.org, jsnow@redhat.com On Mon, Feb 08, 2016 at 04:00:37PM +0300, Roman Kagan wrote: > On Sun, Feb 07, 2016 at 11:08:07AM +0200, Michael S. Tsirkin wrote: > > On Tue, Jan 26, 2016 at 02:50:25PM +0100, Igor Mammedov wrote: > > > From: Roman Kagan > > > -static Aml *build_fdc_device_aml(void) > > > +static Aml *build_fdinfo_aml(int idx, uint8_t type, uint8_t cylinders, > > > + uint8_t heads, uint8_t sectors) > > > > acpi spec says these are WORD values. Are they really uint8_t? > > Yes, see struct FDrive. > > > > + fdi = aml_package(0x10); > > > > Why 0x10 and not 16? > > I'm unaware of the difference... If you have any preference I'll > adjust. (Originally I stuck with hex because so did iasl -d and the > then hand-written ASL). Decimal as other packages please. > > > + aml_append(fdi, aml_int(idx)); /* Drive Number */ > > > + aml_append(fdi, > > > + aml_int(cmos_get_fd_drive_type(type))); /* Device Type */ > > > + aml_append(fdi, > > > + aml_int(cylinders - 1)); /* Maximum Cylinder Number */ > > > + aml_append(fdi, aml_int(sectors)); /* Maximum Sector Number */ > > > + aml_append(fdi, aml_int(heads - 1)); /* Maximum Head Number */ > > > > Doesn't above change on media change? > > I guess no, because this IMHO is supposed to describe the drive > properties, not the diskette properties. But I'll double-check. Where you take this from seems to be diskette properties though ... > I'm confused about the status of this patchset: I saw you post a pull > request with this series last week, and now you review it. What should > I do now in response to your comments: rework and resubmit it or post > incremental fixes on top of it? > > Thanks, > Roman. I dropped and re-did review because Igor reported XP crashing. You should address comments and resubmit. Thanks! -- MST