Re: [PATCH 6/7] drivers/passthrough/arm: Refactor code for arm smmu drivers

From: Sameer Goel <sameer.goel@linaro.org>
To: "Julien Grall" <julien.grall@arm.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Cc: xen-devel@lists.xenproject.org, sstabellini@kernel.org,
	shankerd@codeaurora.org, mjaggi@caviumnetworks.com
Subject: Re: [PATCH 6/7] drivers/passthrough/arm: Refactor code for arm smmu drivers
Date: Fri, 9 Feb 2018 10:54:17 -0700	[thread overview]
Message-ID: <e82cf8b8-b4d2-829f-dd34-6b0c103140fe@linaro.org> (raw)
In-Reply-To: <175537d7-cd3d-069a-cd7a-dccd68107561@arm.com>


On 2/9/2018 3:51 AM, Julien Grall wrote:
>
>>
>>> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
>>> index ad956d5b8d..4c04391e21 100644
>>> --- a/xen/drivers/passthrough/arm/smmu.c
>>> +++ b/xen/drivers/passthrough/arm/smmu.c
>>> @@ -41,6 +41,7 @@
>>>   #include <xen/irq.h>
>>>   #include <xen/lib.h>
>>>   #include <xen/list.h>
>>> +#include <xen/linux_compat.h>
The above header included for the first time.
>>>   #include <xen/mm.h>
>>>   #include <xen/vmap.h>
>>>   #include <xen/rbtree.h>
>>> @@ -51,36 +52,13 @@
>>>   #include <asm/io.h>
>>>   #include <asm/platform.h>
>>>   +#include "arm_smmu.h" /* Not a self contained header. So last in the list */
>>>   /* Xen: The below defines are redefined within the file. Undef it */
>>>   #undef SCTLR_AFE
>>>   #undef SCTLR_TRE
>>>   #undef SCTLR_M
>>>   #undef TTBCR_EAE
>>>   -/* Alias to Xen device tree helpers */
>>> -#define device_node dt_device_node
>>> -#define of_phandle_args dt_phandle_args
>>> -#define of_device_id dt_device_match
>>> -#define of_match_node dt_match_node
>>> -#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, out))
>>> -#define of_property_read_bool dt_property_read_bool
>>> -#define of_parse_phandle_with_args dt_parse_phandle_with_args
>>> -
>>> -/* Xen: Helpers to get device MMIO and IRQs */
>>> -struct resource
>>> -{
>>> -    u64 addr;
>>> -    u64 size;
>>> -    unsigned int type;
>>> -};
>>> -
>>> -#define resource_size(res) (res)->size;
>>> -
>>> -#define platform_device device
>>> -
>>> -#define IORESOURCE_MEM 0
>>> -#define IORESOURCE_IRQ 1
>>> -
>>>   static struct resource *platform_get_resource(struct platform_device *pdev,
>>>                             unsigned int type,
>>>                             unsigned int num)
>>> @@ -118,58 +96,6 @@ static struct resource *platform_get_resource(struct platform_device *pdev,
>>>     /* Xen: Helpers for IRQ functions */
>>>   #define request_irq(irq, func, flags, name, dev) request_irq(irq, flags, func, name, dev)
>>> -#define free_irq release_irq
>>> -
>>> -enum irqreturn {
>>> -    IRQ_NONE    = (0 << 0),
>>> -    IRQ_HANDLED    = (1 << 0),
>>> -};
>>> -
>>> -typedef enum irqreturn irqreturn_t;
>>
>> You remove the irqreturn enum without adding any replacement, is this
>> really unused?
>
> It is used, so looks like the SMMU driver has not been build test it. Sameer, please at least build test the changes you made in the SMMU driver.

It is build tested. The above defined now come from linux_compat.h. I introduced this with the smmu-v3 code changes as recommended by Roger on the RFC.
>
> Cheers,
>
Thanks,
Sameer

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel