From mboxrd@z Thu Jan  1 00:00:00 1970
From: Don Slutz <dslutz@verizon.com>
Subject: Re: [PATCH 1/4] hvmloader: Fixup pci_write* macros
Date: Mon, 15 Jun 2015 10:30:48 -0400
Message-ID: <557EE198.1080303@one.verizon.com>
References: <1434377752-15705-1-git-send-email-dslutz@verizon.com>
	<1434377752-15705-2-git-send-email-dslutz@verizon.com>
	<557EDEE0.9060901@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <557EDEE0.9060901@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Andrew Cooper <andrew.cooper3@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Cc: Keir Fraser <keir@xen.org>, Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Don Slutz <don.slutz@gmail.com>, Jan Beulich <jbeulich@suse.com>, Wei Liu <wei.liu2@citrix.com>
List-Id: xen-devel@lists.xenproject.org

On 06/15/15 10:19, Andrew Cooper wrote:
> On 15/06/15 15:15, Don Slutz wrote:
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> CC: Don Slutz <dslutz@verizon.com>
> 
> Fix how?  It looks like you are bracketing val.
> 

If val is an expression, the macro most likely does the wrong thing.

For example:

pci_writeb(devfn, PCI_IO_BASE, addr >> PCI_IO_SHIFT);


is pci_write(devfn, reg, 1, (uint8_t)addr >> PCI_IO_SHIFT); which is not
correct.  I would expect:

pci_write(devfn, reg, 1, (uint8_t)(addr >> PCI_IO_SHIFT));

> This is an improvement, but please always be specific as to what is
> being fixed.
> 

Does:

Improve pci_write* macros to act like a function does.

scan better?

> Furthermore, what about devfn or reg?
> 

devfn and reg do not need the bracketing since they are just passed,
but I have no issue with adding the extra brackets.

   -Don Slutz

> ~Andrew
> 
>> ---
>>  tools/firmware/hvmloader/util.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
>> index a70e4aa..8431f2d 100644
>> --- a/tools/firmware/hvmloader/util.h
>> +++ b/tools/firmware/hvmloader/util.h
>> @@ -82,9 +82,9 @@ uint32_t pci_read(uint32_t devfn, uint32_t reg, uint32_t len);
>>  #define pci_readw(devfn, reg) ((uint16_t)pci_read(devfn, reg, 2))
>>  #define pci_readl(devfn, reg) ((uint32_t)pci_read(devfn, reg, 4))
>>  void pci_write(uint32_t devfn, uint32_t reg, uint32_t len, uint32_t val);
>> -#define pci_writeb(devfn, reg, val) (pci_write(devfn, reg, 1, (uint8_t) val))
>> -#define pci_writew(devfn, reg, val) (pci_write(devfn, reg, 2, (uint16_t)val))
>> -#define pci_writel(devfn, reg, val) (pci_write(devfn, reg, 4, (uint32_t)val))
>> +#define pci_writeb(devfn, reg, val) (pci_write(devfn, reg, 1, (uint8_t) (val)))
>> +#define pci_writew(devfn, reg, val) (pci_write(devfn, reg, 2, (uint16_t)(val)))
>> +#define pci_writel(devfn, reg, val) (pci_write(devfn, reg, 4, (uint32_t)(val)))
>>  
>>  /* Get a pointer to the shared-info page */
>>  struct shared_info *get_shared_info(void) __attribute__ ((const));
>