Re: [Qemu-devel] [PATCH 1/2] hw/arm/sysbus-fdt: rename xgmac platform fdt node creation function

[Eric Auger <eric.auger@linaro.org>]

Hi Vikram, Shanker,
On 01/30/2016 12:00 AM, Shanker Donthineni wrote:
> From: Vikram Sethi <vikrams@codeaurora.org>
> 
> The function that creates a device tree node for the Calxeda XGMAC
> platform device is fairly generic and can be reused for other
> platform devices that require just reg, interrupts and dma-coherent
> properties. Rename add_calxeda_midway_xgmac_fdt_node to
> add_generic_platform_fdt_node to allow reuse.
> 
> Signed-off-by: Vikram Sethi <vikrams@codeaurora.org>
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> ---
>  hw/arm/sysbus-fdt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index 9d28797..6ee7af2 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -64,7 +64,7 @@ typedef struct NodeCreationPair {
>   * Generates a simple node with following properties:
>   * compatible string, regs, interrupts, dma-coherent
>   */
> -static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque)
> +static int add_generic_platform_fdt_node(SysBusDevice *sbdev, void *opaque)
>  {
>      PlatformBusFDTData *data = opaque;
>      PlatformBusDevice *pbus = data->pbus;
> @@ -125,7 +125,7 @@ fail_reg:
>  
>  /* list of supported dynamic sysbus devices */
>  static const NodeCreationPair add_fdt_node_functions[] = {
> -    {TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
> +    {TYPE_VFIO_CALXEDA_XGMAC, add_generic_platform_fdt_node},
Introducing such a "generic" node creation function was discussed in the
past and Alex Graf (added in CC) warned against that. What is arguable
is that interrupts are defined here as level sensitive and you have the
dma-coherent property set. So where does genericity starts and where
does it end? But Maybe Alex changed his mind ;-)

The type of IRQ can be retrieved generically by looking at intp->flags &
VFIO_IRQ_INFO_AUTOMASKED as I did in add_amd_xgbe_fdt_node ([PATCH v6
7/8] hw/arm/sysbus-fdt: enable amd-xgbe dynamic instantiation)

The dma-coherent property can now be retrieved from host device-tree
using the helpers introduced in the above series. However if you use
ACPI on host there is no such helper yet.

Maybe another approach to factorize the code is to introduce a helper
routine that adds the reg and interrupts properties to the node, using
the above detection of IRQ type and you keep a differentiated function
for HiDma, adding the dma-coherent capability.

Hope it helps

Best Regards

Eric

>      {"", NULL}, /* last element */
>  };
>  
>