From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1Kl798-00083P-A2
	for qemu-devel@nongnu.org; Wed, 01 Oct 2008 15:14:02 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1Kl796-00082X-Qn
	for qemu-devel@nongnu.org; Wed, 01 Oct 2008 15:14:01 -0400
Received: from [199.232.76.173] (port=44425 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Kl796-00082U-Kq
	for qemu-devel@nongnu.org; Wed, 01 Oct 2008 15:14:00 -0400
Received: from mx2.redhat.com ([66.187.237.31]:36201)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <kraxel@redhat.com>) id 1Kl796-00034U-31
	for qemu-devel@nongnu.org; Wed, 01 Oct 2008 15:14:00 -0400
Message-ID: <48E3CAF9.5040207@redhat.com>
Date: Wed, 01 Oct 2008 21:09:45 +0200
From: Gerd Hoffmann <kraxel@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 4/4] pci: use pci_config_header in pci.c
References: <1222870375-13489-1-git-send-email-kraxel@redhat.com>
	<1222870375-13489-4-git-send-email-kraxel@redhat.com>
	<48E3A5BD.8010000@codemonkey.ws>
In-Reply-To: <48E3A5BD.8010000@codemonkey.ws>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org, Paul Brook <paul@codesourcery.com>

Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> This makes use of the new pci_config_header struct in pci.c,
>> squashing a bunch of casts and hard-coded magic numbers.
>>   
> 
> I thought the last bit of feedback that you received from Paul was that
> it seemed like a waste to go through the trouble of introducing this PCI
> config structure but then forcing the users to do explicit endian
> casting.  I agree with this and was expecting the updated patches to do
> the endian conversion automatically.
> 
> Any reasons for not doing it this way?

I'm not sure what you are expecting here.  The idea to introduce
gcc-checkable byteordered types (via struct trick) was rejected.  So I
have le32 & friends just typedef'ed to the corrosponding int*_t types
and use the existing cpu_*() byteorder macros.  The new types are
basically documentation now.

Endian swapping must still be done by the users.  Lots of casts can be
dropped though, which IMHO improves the readability alot (random chunk
picked from patch):

-    cmd = le16_to_cpu(*(uint16_t *)(d->config + PCI_COMMAND));
+    cmd = le16_to_cpu(conf->command);

I'm not aware of any way to do the byteswapping here automatically.

Except maybe introducing accessor functions/macros for each and every
struct element and hide the byteswapping macro call therein.  Which in
turn makes providing the struct to pci drivers sort of pointless ...

cheers,
  Gerd