From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53884) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eoAju-0006IX-2w for qemu-devel@nongnu.org; Tue, 20 Feb 2018 11:25:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eoAjq-0002K3-QF for qemu-devel@nongnu.org; Tue, 20 Feb 2018 11:25:58 -0500 Date: Tue, 20 Feb 2018 17:25:40 +0100 From: Cornelia Huck Message-ID: <20180220172540.0c1e6584.cohuck@redhat.com> In-Reply-To: <054d7875-140c-c9c0-f5c3-db52e79feeac@redhat.com> References: <20180220150713.6056-1-pasic@linux.vnet.ibm.com> <20180220165323.02898d8a.cohuck@redhat.com> <437eecac-036e-bbdd-a275-31dc0c812f32@de.ibm.com> <20180220170740.766b2234.cohuck@redhat.com> <054d7875-140c-c9c0-f5c3-db52e79feeac@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] 390x/cpumodel: document S390FeatDef.bit not applicable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Hildenbrand Cc: Christian Borntraeger , Halil Pasic , qemu-devel@nongnu.org, Alexander Graf , qemu-s390x@nongnu.org, Richard Henderson On Tue, 20 Feb 2018 17:08:52 +0100 David Hildenbrand wrote: > On 20.02.2018 17:07, Cornelia Huck wrote: > > On Tue, 20 Feb 2018 17:04:19 +0100 > > Christian Borntraeger wrote: > > > >> On 02/20/2018 04:55 PM, David Hildenbrand wrote: > >>> On 20.02.2018 16:53, Cornelia Huck wrote: > >>>> On Tue, 20 Feb 2018 16:07:13 +0100 > >>>> Halil Pasic wrote: > >>>> > >>>>> The 'bit' field of the 'S390FeatDef' structure is not applicable to all > >>>>> it's instances. Currently a this field is not applicable, and remains > >>>> > >>>> s/it's/its/ > >>>> > >>>> s/a this/this/ > >>>> > >>>>> unused, iff the feature is of type S390_FEAT_TYPE_MISC. Having the value 0 > >>>>> specified for multiple such feature definition was a little confusing, > >>>>> as it's a perfectly legit bit value, and as usually the value of the bit > >>>>> field is ought to be unique for each feature. > >>>>> > >>>>> Let's document this, and hopefully reduce the potential for confusion. > >>>>> > >>>>> Signed-off-by: Halil Pasic > >>>>> --- > >>>>> > >>>>> Hi! > >>>>> > >>>>> This may be an overkill. A comment where the misc features > >>>>> are defined would do to, but I think this is nicer. So > >>>>> I decided to try it with this approach first. > >>>> > >>>> Is there likely to be anything else than FEAT_MISC _not_ using .bit? If > >>>> not, would it be better to at a comment to the FEAT_MISC definition? > >>> > >>> Doubt it right now. I would sign the "overkill" part :) > >> > >> I can cconfirm that this code caused some questions and it took me some > >> minutes to remember why 0 and 0 was ok. So I certainly want to have a comment > >> of some form. > >> > > > > I'd prefer a comment about FEAT_MISC usage rather than a magic value. > > > > We can also add FEAT_INIT_MISC. And add a comment in the initializer. > That's what I like best.