Re: [PATCH v13 05/11] remoteproc: mediatek: Introduce cluster on single-core SCP

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 07/06/23 10:00, TingHan Shen (沈廷翰) ha scritto:
> Hi Angelo,
> 
> On Wed, 2023-06-07 at 09:43 +0200, AngeloGioacchino Del Regno wrote:
>> Il 07/06/23 09:22, Tinghan Shen ha scritto:
>>> This is the preliminary step for probing multi-core SCP.
>>> The initialization procedure for remoteproc is similar for both
>>> single-core and multi-core architectures and is reusing to avoid
>>> redundant code.
>>>
>>> Rewrite the probing flow of single-core SCP to adapt with the 'cluster'
>>> concept needed by probing the multi-core SCP. The main differences
>>> are,
>>> - the SCP core object(s) is maintained at the cluster list instead of at
>>>     the platofmr device driver data property.
>>
>> s/platofmr/platform/g
>>
>>> - save the cluster information at the platofmr device driver data property.
>>> - In order to keep the compatibility of exported SCP APIs which getting
>>>     the SCP core object by SCP node phandle, move the SCP core object
>>>     pointers to the platform device platform data property.
>>>
>>> The registers of config and l1tcm are shared for multi-core
>>> SCP. Reuse the mapped addresses for all cores.
>>>
>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>> ---
>>>    drivers/remoteproc/mtk_common.h |   2 +
>>>    drivers/remoteproc/mtk_scp.c    | 151 +++++++++++++++++++++++---------
>>>    2 files changed, 112 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
>>> index c0905aec3b4b..56395e8664cb 100644
>>> --- a/drivers/remoteproc/mtk_common.h
>>> +++ b/drivers/remoteproc/mtk_common.h
>>> @@ -128,6 +128,8 @@ struct mtk_scp {
>>>    	size_t dram_size;
>>>    
>>>    	struct rproc_subdev *rpmsg_subdev;
>>> +
>>> +	struct list_head elem;
>>>    };
>>>    
>>>    /**
>>> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
>>> index d66822dad943..c8fc6b46f82b 100644
>>> --- a/drivers/remoteproc/mtk_scp.c
>>> +++ b/drivers/remoteproc/mtk_scp.c
>>> @@ -23,6 +23,14 @@
>>>    #define MAX_CODE_SIZE 0x500000
>>>    #define SECTION_NAME_IPI_BUFFER ".ipi_buffer"
>>>    
>>> +struct mtk_scp_of_cluster {
>>> +	void __iomem *reg_base;
>>> +	void __iomem *l1tcm_base;
>>> +	size_t l1tcm_size;
>>> +	phys_addr_t l1tcm_phys;
>>> +	struct list_head mtk_scp_list;
>>> +};
>>> +
>>>    /**
>>>     * scp_get() - get a reference to SCP.
>>>     *
>>> @@ -51,7 +59,7 @@ struct mtk_scp *scp_get(struct platform_device *pdev)
>>>    		return NULL;
>>>    	}
>>>    
>>> -	return platform_get_drvdata(scp_pdev);
>>> +	return *(struct mtk_scp **)dev_get_platdata(&scp_pdev->dev);
>>>    }
>>>    EXPORT_SYMBOL_GPL(scp_get);
>>>    
>>> @@ -810,14 +818,14 @@ static void scp_unmap_memory_region(struct mtk_scp *scp)
>>>    static int scp_register_ipi(struct platform_device *pdev, u32 id,
>>>    			    ipi_handler_t handler, void *priv)
>>>    {
>>> -	struct mtk_scp *scp = platform_get_drvdata(pdev);
>>> +	struct mtk_scp *scp = *(struct mtk_scp **)dev_get_platdata(&pdev->dev);
>>>    
>>>    	return scp_ipi_register(scp, id, handler, priv);
>>>    }
>>>    
>>>    static void scp_unregister_ipi(struct platform_device *pdev, u32 id)
>>>    {
>>> -	struct mtk_scp *scp = platform_get_drvdata(pdev);
>>> +	struct mtk_scp *scp = *(struct mtk_scp **)dev_get_platdata(&pdev->dev);
>>>    
>>>    	scp_ipi_unregister(scp, id);
>>>    }
>>> @@ -825,7 +833,7 @@ static void scp_unregister_ipi(struct platform_device *pdev, u32 id)
>>>    static int scp_send_ipi(struct platform_device *pdev, u32 id, void *buf,
>>>    			unsigned int len, unsigned int wait)
>>>    {
>>> -	struct mtk_scp *scp = platform_get_drvdata(pdev);
>>> +	struct mtk_scp *scp = *(struct mtk_scp **)dev_get_platdata(&pdev->dev);
>>>    
>>>    	return scp_ipi_send(scp, id, buf, len, wait);
>>>    }
>>> @@ -855,7 +863,8 @@ static void scp_remove_rpmsg_subdev(struct mtk_scp *scp)
>>>    	}
>>>    }
>>>    
>>> -static int scp_probe(struct platform_device *pdev)
>>> +static struct mtk_scp *scp_rproc_init(struct platform_device *pdev,
>>> +				      struct mtk_scp_of_cluster *scp_cluster)
>>>    {
>>>    	struct device *dev = &pdev->dev;
>>>    	struct device_node *np = dev->of_node;
>>> @@ -867,52 +876,42 @@ static int scp_probe(struct platform_device *pdev)
>>>    
>>>    	ret = rproc_of_parse_firmware(dev, 0, &fw_name);
>>>    	if (ret < 0 && ret != -EINVAL)
>>> -		return ret;
>>> +		return ERR_PTR(ret);
>>>    
>>>    	rproc = devm_rproc_alloc(dev, np->name, &scp_ops, fw_name, sizeof(*scp));
>>> -	if (!rproc)
>>> -		return dev_err_probe(dev, -ENOMEM, "unable to allocate remoteproc\n");
>>> +	if (!rproc) {
>>> +		dev_err(dev, "unable to allocate remoteproc\n");
>>> +		return ERR_PTR(-ENOMEM);
>>
>> Why are you converting those dev_err_probe to dev_err->return?!
>>
>> Regards,
>> Angelo
> 
> It's because the return type of this function changed from "int" to "struct mtk_scp *".
> Maybe I should change them to "ERR_PTR(dev_err_probe(...))" ?
> 

Oh, sorry - you're right. No, it's fine as you did it.

Cheers