Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v6 06/16] remoteproc: stm32: Move resource table setup to
 rproc_ops
To:     Mathieu Poirier <mathieu.poirier@linaro.org>, <ohad@wizery.com>,
        <bjorn.andersson@linaro.org>, <arnaud.pouliquen@st.com>
CC:     <mcoquelin.stm32@gmail.com>, <alexandre.torgue@st.com>,
        <linux-remoteproc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>
References: <20210223233515.3468677-1-mathieu.poirier@linaro.org>
 <20210223233515.3468677-7-mathieu.poirier@linaro.org>
From:   Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
Message-ID: <3fe71628-d37a-94f5-3d08-132b0b0cf0f6@foss.st.com>
Date:   Fri, 26 Feb 2021 17:14:53 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20210223233515.3468677-7-mathieu.poirier@linaro.org>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2/24/21 12:35 AM, Mathieu Poirier wrote:
> Move the setting of the resource table installed by an external
> entity to rproc_ops::get_loaded_rsc_table().  This is to support
> scenarios where a remote processor has been started by the core
> but is detached at a later stage.  To re-attach the remote
> processor, the address of the resource table needs to be available
> at a later time than the platform driver's probe() function.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Reviewed-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>

Thanks,
Arnaud

> ---
>  drivers/remoteproc/stm32_rproc.c | 141 +++++++++++++++----------------
>  1 file changed, 68 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index ccb3c14a0023..f647e565014b 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -546,6 +546,73 @@ static void stm32_rproc_kick(struct rproc *rproc, int vqid)
>  	}
>  }
>  
> +static int stm32_rproc_da_to_pa(struct rproc *rproc,
> +				u64 da, phys_addr_t *pa)
> +{
> +	struct stm32_rproc *ddata = rproc->priv;
> +	struct device *dev = rproc->dev.parent;
> +	struct stm32_rproc_mem *p_mem;
> +	unsigned int i;
> +
> +	for (i = 0; i < ddata->nb_rmems; i++) {
> +		p_mem = &ddata->rmems[i];
> +
> +		if (da < p_mem->dev_addr ||
> +		    da >= p_mem->dev_addr + p_mem->size)
> +			continue;
> +
> +		*pa = da - p_mem->dev_addr + p_mem->bus_addr;
> +		dev_dbg(dev, "da %llx to pa %#x\n", da, *pa);
> +
> +		return 0;
> +	}
> +
> +	dev_err(dev, "can't translate da %llx\n", da);
> +
> +	return -EINVAL;
> +}
> +
> +static struct resource_table *
> +stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
> +{
> +	struct stm32_rproc *ddata = rproc->priv;
> +	struct device *dev = rproc->dev.parent;
> +	phys_addr_t rsc_pa;
> +	u32 rsc_da;
> +	int err;
> +
> +	/* The resource table has already been mapped, nothing to do */
> +	if (ddata->rsc_va)
> +		goto done;
> +
> +	err = regmap_read(ddata->rsctbl.map, ddata->rsctbl.reg, &rsc_da);
> +	if (err) {
> +		dev_err(dev, "failed to read rsc tbl addr\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	if (!rsc_da)
> +		/* no rsc table */
> +		return ERR_PTR(-ENOENT);
> +
> +	err = stm32_rproc_da_to_pa(rproc, rsc_da, &rsc_pa);
> +	if (err)
> +		return ERR_PTR(err);
> +
> +	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
> +	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
> +		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
> +			&rsc_pa, RSC_TBL_SIZE);
> +		ddata->rsc_va = NULL;
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +done:
> +	/* Assuming the resource table fits in 1kB is fair */
> +	*table_sz = RSC_TBL_SIZE;
> +	return (struct resource_table *)ddata->rsc_va;
> +}
> +
>  static const struct rproc_ops st_rproc_ops = {
>  	.start		= stm32_rproc_start,
>  	.stop		= stm32_rproc_stop,
> @@ -554,6 +621,7 @@ static const struct rproc_ops st_rproc_ops = {
>  	.load		= rproc_elf_load_segments,
>  	.parse_fw	= stm32_rproc_parse_fw,
>  	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> +	.get_loaded_rsc_table = stm32_rproc_get_loaded_rsc_table,
>  	.sanity_check	= rproc_elf_sanity_check,
>  	.get_boot_addr	= rproc_elf_get_boot_addr,
>  };
> @@ -695,75 +763,6 @@ static int stm32_rproc_get_m4_status(struct stm32_rproc *ddata,
>  	return regmap_read(ddata->m4_state.map, ddata->m4_state.reg, state);
>  }
>  
> -static int stm32_rproc_da_to_pa(struct platform_device *pdev,
> -				struct stm32_rproc *ddata,
> -				u64 da, phys_addr_t *pa)
> -{
> -	struct device *dev = &pdev->dev;
> -	struct stm32_rproc_mem *p_mem;
> -	unsigned int i;
> -
> -	for (i = 0; i < ddata->nb_rmems; i++) {
> -		p_mem = &ddata->rmems[i];
> -
> -		if (da < p_mem->dev_addr ||
> -		    da >= p_mem->dev_addr + p_mem->size)
> -			continue;
> -
> -		*pa = da - p_mem->dev_addr + p_mem->bus_addr;
> -		dev_dbg(dev, "da %llx to pa %#x\n", da, *pa);
> -
> -		return 0;
> -	}
> -
> -	dev_err(dev, "can't translate da %llx\n", da);
> -
> -	return -EINVAL;
> -}
> -
> -static int stm32_rproc_get_loaded_rsc_table(struct platform_device *pdev,
> -					    struct rproc *rproc,
> -					    struct stm32_rproc *ddata)
> -{
> -	struct device *dev = &pdev->dev;
> -	phys_addr_t rsc_pa;
> -	u32 rsc_da;
> -	int err;
> -
> -	err = regmap_read(ddata->rsctbl.map, ddata->rsctbl.reg, &rsc_da);
> -	if (err) {
> -		dev_err(dev, "failed to read rsc tbl addr\n");
> -		return err;
> -	}
> -
> -	if (!rsc_da)
> -		/* no rsc table */
> -		return 0;
> -
> -	err = stm32_rproc_da_to_pa(pdev, ddata, rsc_da, &rsc_pa);
> -	if (err)
> -		return err;
> -
> -	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
> -	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
> -		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
> -			&rsc_pa, RSC_TBL_SIZE);
> -		ddata->rsc_va = NULL;
> -		return -ENOMEM;
> -	}
> -
> -	/*
> -	 * The resource table is already loaded in device memory, no need
> -	 * to work with a cached table.
> -	 */
> -	rproc->cached_table = NULL;
> -	/* Assuming the resource table fits in 1kB is fair */
> -	rproc->table_sz = RSC_TBL_SIZE;
> -	rproc->table_ptr = (struct resource_table *)ddata->rsc_va;
> -
> -	return 0;
> -}
> -
>  static int stm32_rproc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -803,10 +802,6 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>  		ret = stm32_rproc_parse_memory_regions(rproc);
>  		if (ret)
>  			goto free_resources;
> -
> -		ret = stm32_rproc_get_loaded_rsc_table(pdev, rproc, ddata);
> -		if (ret)
> -			goto free_resources;
>  	}
>  
>  	rproc->has_iommu = false;
>