[Freedreno] [PATCH 1/6] drm/msm: Add a quick and dirty PIL loader

[Bjorn Andersson]

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