From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752801AbbCWNvZ (ORCPT ); Mon, 23 Mar 2015 09:51:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45757 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752631AbbCWNvW (ORCPT ); Mon, 23 Mar 2015 09:51:22 -0400 Date: Mon, 23 Mar 2015 14:51:14 +0100 From: "Michael S. Tsirkin" To: Gerd Hoffmann Cc: virtio-dev@lists.oasis-open.org, virtualization@lists.linux-foundation.org, Rusty Russell , open list , "open list:ABI/API" Subject: Re: [PATCH 1/1] Add virtio-input driver. Message-ID: <20150323144803-mutt-send-email-mst@redhat.com> References: <1426756391-26585-1-git-send-email-kraxel@redhat.com> <1426756391-26585-2-git-send-email-kraxel@redhat.com> <20150319123940-mutt-send-email-mst@redhat.com> <1426847327.32097.60.camel@nilsson.home.kraxel.org> <20150321225356-mutt-send-email-mst@redhat.com> <1427097238.6365.27.camel@nilsson.home.kraxel.org> <1427118292.27137.39.camel@nilsson.home.kraxel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1427118292.27137.39.camel@nilsson.home.kraxel.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 23, 2015 at 02:44:52PM +0100, Gerd Hoffmann wrote: > Hi, > > > > At least, this needs a comment explaining what the function does, > > > and maybe wrap it in a helper like virtio_input_bitmap_copy or > > > virtio_bitmap_or. > > > > Can do that, sure. > > Well, the function where this is in already cares about the bitmap copy > only. Can add a comment though. OK, I think that will be enough for now. > > > You are doing leXXX everywhere, that's VERSION_1 dependency. > > > virtio_cread will do byteswaps differently without VERSION_1. > > > Just don't go there. > > > > Changed that for v2, for the config space structs. They have normal u32 > > in there now. virtio_cread() wants it this way. > > I liked the __le32 in the config space structs more though, so I've > waded through the virtio_config.h header file. > > To me it looks like we need separate virtio_cread() versions for > non-transitional drivers, which do __le32 -> u32 translation instead of > __virtio32 -> u32 translation, so I can have __le32 types in the config > space structs. > > Or I could use vdev->config->get() directly instead of virtio_cread, but > I'll loose sparse checking that way. > > Hmm. Recommendations? Better ideas? > > cheers, > Gerd So to clarify, you dislike using __virtio32 in virtio input header? -- MST