Re: [PATCH v5 5/5] Driver: VMBus: Add device tree support

[Rob Herring]

On Thu, Feb 9, 2023 at 6:15 AM Saurabh Sengar
<ssengar@xxxxxxxxxxxxxxxxxxx> wrote:
>
> Update the driver to support device tree boot as well along with ACPI.

Devicetree

> At present the device tree parsing only provides the mmio region info

Devicetree

And anywhere else.

> and is not the exact copy of ACPI parsing. This is sufficient to cater
> all the current device tree usecases for VMBus.
>
> Currently device tree is supported only for x86 systems.
>
> Signed-off-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
> ---
> - Removed #else for device tree parsing code. This should help better
>   test coverage.
> - Fix macro '__maybe_unused' warning
> - Added below options in Kconfig to enable device tree options for HYPERV
>         select OF if !ACPI
>         select OF_EARLY_FLATTREE if !ACPI
>
>  drivers/hv/Kconfig     |  6 +++--
>  drivers/hv/vmbus_drv.c | 60 ++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 62 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index 0747a8f1fcee..1a55bf32d195 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -4,11 +4,13 @@ menu "Microsoft Hyper-V guest support"
>
>  config HYPERV
>         tristate "Microsoft Hyper-V client drivers"
> -       depends on ACPI && ((X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
> -               || (ARM64 && !CPU_BIG_ENDIAN))
> +       depends on (X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
> +               || (ACPI && ARM64 && !CPU_BIG_ENDIAN)
>         select PARAVIRT
>         select X86_HV_CALLBACK_VECTOR if X86
>         select VMAP_PFN
> +       select OF if !ACPI
> +       select OF_EARLY_FLATTREE if !ACPI
>         help
>           Select this option to run Linux as a Hyper-V client operating
>           system.
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 73497157a23a..02f6bab61c37 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -20,6 +20,7 @@
>  #include <linux/completion.h>
>  #include <linux/hyperv.h>
>  #include <linux/kernel_stat.h>
> +#include <linux/of_address.h>
>  #include <linux/clockchips.h>
>  #include <linux/cpu.h>
>  #include <linux/sched/isolation.h>
> @@ -2152,7 +2153,7 @@ void vmbus_device_unregister(struct hv_device *device_obj)
>         device_unregister(&device_obj->device);
>  }
>
> -
> +#ifdef CONFIG_ACPI
>  /*
>   * VMBUS is an acpi enumerated device. Get the information we
>   * need from DSDT.
> @@ -2262,6 +2263,7 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
>
>         return AE_OK;
>  }
> +#endif
>
>  static void vmbus_mmio_remove(void)
>  {
> @@ -2282,7 +2284,7 @@ static void vmbus_mmio_remove(void)
>         }
>  }
>
> -static void vmbus_reserve_fb(void)
> +static void __maybe_unused vmbus_reserve_fb(void)
>  {
>         resource_size_t start = 0, size;
>         struct pci_dev *pdev;
> @@ -2442,6 +2444,7 @@ void vmbus_free_mmio(resource_size_t start, resource_size_t size)
>  }
>  EXPORT_SYMBOL_GPL(vmbus_free_mmio);
>
> +#ifdef CONFIG_ACPI
>  static int vmbus_acpi_add(struct platform_device *pdev)
>  {
>         acpi_status result;
> @@ -2494,10 +2497,50 @@ static int vmbus_acpi_add(struct platform_device *pdev)
>                 vmbus_mmio_remove();
>         return ret_val;
>  }
> +#endif
> +
> +static int vmbus_device_add(struct platform_device *pdev)
> +{
> +       struct resource **cur_res = &hyperv_mmio;
> +       struct of_range range;
> +       struct of_range_parser parser;
> +       struct device_node *np;
> +       int ret = 0;

No need to initialize.

> +
> +       hv_dev = &pdev->dev;
> +       np = pdev->dev.of_node;

Set this on the declaration.

> +
> +       ret = of_range_parser_init(&parser, np);
> +       if (ret) {
> +               dev_err(hv_dev, "Failed to parse resources.\n");

If a print is needed, put it in of_range_parser_init().

> +               return ret;
> +       }
> +
> +       for_each_of_range(&parser, &range) {
> +               struct resource *res;
> +
> +               res = kzalloc(sizeof(*res), GFP_ATOMIC);
> +               if (!res)
> +                       return -ENOMEM;
> +
> +               res->name = "hyperv mmio";
> +               res->flags = IORESOURCE_MEM | IORESOURCE_MEM_64;
> +               res->start = range.pci_addr;

This is not PCI. It's a union, so use 'bus_addr' instead.

But wait, resources and IORESOURCE_MEM are *CPU* addresses. You need
cpu_addr here. Your DT happens to do 1:1 addresses so it happens to
work either way.

Rob