On Wed 12 Apr 14:15 PDT 2017, Jordan Crouse wrote:
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
[..]
> +static int _pil_tz_load_image(struct platform_device *pdev)
> +{

You should be able to replace this with a call into
drivers/soc/qcom/mdt_loader.c (if not please let me know so we can make
that work for you).

> +     char str[64] = { 0 };
> +     const char *fwname;
> +     const struct elf32_hdr *ehdr;
> +     const struct elf32_phdr *phdrs;
> +     const struct firmware *mdt;
> +     phys_addr_t fw_min_addr, fw_max_addr;
> +     dma_addr_t fw_phys;
> +     size_t fw_size;
> +     u32 pas_id;
> +     void *ptr;
> +     int i, ret;
> +
> +     if (pdev == NULL)
> +             return -ENODEV;
> +
> +     if (!qcom_scm_is_available()) {
> +             dev_err(&pdev->dev, "SCM is not available\n");
> +             return -EINVAL;
> +     }
> +
> +     ret = of_reserved_mem_device_init(&pdev->dev);

You're supposed to do this once per device, so although I believe you
only call this function as an extension of the init function it would be
better for maintainability if you move this call to where you create the
device.

> +
> +     if (ret) {
> +             dev_err(&pdev->dev, "Unable to set up the reserved memory\n");
> +             return ret;
> +     }
> +
> +     /* Get the firmware and PAS id from the device node */
> +     if (of_property_read_string(pdev->dev.of_node, "qcom,firmware",
> +             &fwname)) {
> +             dev_err(&pdev->dev, "Could not read a firmware name\n");
> +             return -EINVAL;
> +     }
> +
> +     if (of_property_read_u32(pdev->dev.of_node, "qcom,pas-id", &pas_id)) {
> +             dev_err(&pdev->dev, "Could not read the pas ID\n");
> +             return -EINVAL;
> +     }
> +

As noted in the later patches, there's no point in introducing this code
(that's incompatible with the DT binding) just to remove it again.
Squash in the other code-patches (DT binding should be separate) and
this would be cleaner.

> +     snprintf(str, sizeof(str) - 1, "%s.mdt", fwname);

N.B. snprintf() takes the size of the buffer as second argument and will
make sure there's a NUL at the end, so no -1.

Regards,
Bjorn
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Reply via email to