Re: [RFC v2 2/7] arm64: Add definitions for fwnode_handle

["Goel, Sameer" <sgoel@codeaurora.org>]

On 10/12/2017 6:45 AM, Julien Grall wrote:
> Hi,
> 
> On 21/09/17 01:37, Sameer Goel wrote:
>> This will be used as a device property to match the DMA capable devices
>> with the associated SMMU. The header file is a port from linux. The code
>> was changed to remove the types that were not needed for Xen.
> 
> I think you probably want a bit more context in the commit message about implement fwnode.h in common code.
> 
> Within this series, fwnode seems to only be used by Arm. So what would be the advantage to get that in xen/? Is it going to be used by x86 or taken advantage in common code?
> 
>>
>> Linux ChangeId:ce793486e23e: driver core / ACPI: Represent ACPI
>> companions using fwnode_handle
>>
>> Signed-off-by: Sameer Goel <sgoel@codeaurora.org>
>> ---
>>   xen/include/asm-arm/device.h |  2 ++
>>   xen/include/xen/fwnode.h     | 33 +++++++++++++++++++++++++++++++++
>>   2 files changed, 35 insertions(+)
>>   create mode 100644 xen/include/xen/fwnode.h
>>
>> diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
>> index 6734ae8..78c38fe 100644
>> --- a/xen/include/asm-arm/device.h
>> +++ b/xen/include/asm-arm/device.h
>> @@ -2,6 +2,7 @@
>>   #define __ASM_ARM_DEVICE_H
>>     #include <xen/init.h>
>> +#include <xen/fwnode.h>
>>     enum device_type
>>   {
>> @@ -19,6 +20,7 @@ struct device
>>   #ifdef CONFIG_HAS_DEVICE_TREE
>>       struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> 
> I was expecting a todo in the code after the discussion about leave of_node here.
> 
>>   #endif
>> +    struct fwnode_handle *fwnode; /*fw device node identifier */
The fwnode handle was provide a match cookie for the SMMUs and not much else. Even with this around we will need the 
dt info in the device node. I agree that this rolls up into fw spec and I can look at the code cleanup for the next patch.
> 
> Space missing before "fw".
> 
>>       struct dev_archdata archdata;
>>   };
>>   diff --git a/xen/include/xen/fwnode.h b/xen/include/xen/fwnode.h
>> new file mode 100644
>> index 0000000..0fed958
>> --- /dev/null
>> +++ b/xen/include/xen/fwnode.h
>> @@ -0,0 +1,33 @@
>> +/*
>> + * fwnode.h - Firmware device node object handle type definition.
>> + *
>> + * Copyright (C) 2015, Intel Corporation
>> + * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * Ported from Linux include/linux/fwnode.h
>> + *  => commit ce793486e23e0162a732c605189c8028e0910e86
>> + *
>> + * No functional Xen modifications.
>> + */
>> +
>> +#ifndef __XEN_FWNODE_H_
>> +#define __XEN_FWNODE_H_
>> +
>> +enum fwnode_type {
>> +    FWNODE_INVALID = 0,
>> +    FWNODE_OF,
>> +    FWNODE_ACPI,
>> +    FWNODE_ACPI_STATIC,
>> +    FWNODE_IRQCHIP
>> +};
>> +
> 
> Looking at Linux code, the fwnode_type already disappeared from Linux (see commit db3e50f3234b "device property: Get rid of struct fwnode_handle type field").
> 
> I understand the goal on using fwnode is to help porting drivers from Linux. So how much this has changed now?
This was not very useful in any case. So, I will move over to the new version.

> 
> Cheers,
> 
>> +struct fwnode_handle {
>> +    enum fwnode_type type;
>> +    struct fwnode_handle *secondary;
>> +};
>> +
>> +#endif
>>
> 

-- 
 Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

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