lkml.org 
[lkml]   [2023]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 3/3] remoteproc: xilinx: add mailbox channels for rpmsg
From

On 2/22/23 11:06 AM, Mathieu Poirier wrote:
> On Mon, Feb 13, 2023 at 01:18:26PM -0800, Tanmay Shah wrote:
>> [ ... ]
>> +
>> /*
>> * zynqmp_r5_set_mode()
>> *
>> @@ -617,7 +819,7 @@ static int zynqmp_r5_rproc_unprepare(struct rproc *rproc)
>> return 0;
>> }
>>
>> -static const struct rproc_ops zynqmp_r5_rproc_ops = {
>> +static struct rproc_ops zynqmp_r5_rproc_ops = {
>> .prepare = zynqmp_r5_rproc_prepare,
>> .unprepare = zynqmp_r5_rproc_unprepare,
>> .start = zynqmp_r5_rproc_start,
>> @@ -642,6 +844,7 @@ static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
>> {
>> struct zynqmp_r5_core *r5_core;
>> struct rproc *r5_rproc;
>> + struct mbox_info *ipi;
>> int ret;
>>
>> /* Set up DMA mask */
>> @@ -649,12 +852,23 @@ static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
>> if (ret)
>> return ERR_PTR(ret);
>>
>> + /*
>> + * If mailbox nodes are disabled using "status" property then setting up
>> + * mailbox channels will be failed. In that case we don't really need
>> + * kick() operation. Include .kick() only if mbox channels are acquired
>> + * successfully.
>> + */
>> + ipi = zynqmp_r5_setup_mbox(cdev);
>> + if (ipi)
>> + zynqmp_r5_rproc_ops.kick = zynqmp_r5_rproc_kick;
>> +
>> /* Allocate remoteproc instance */
>> r5_rproc = rproc_alloc(cdev, dev_name(cdev),
>> &zynqmp_r5_rproc_ops,
>> NULL, sizeof(struct zynqmp_r5_core));
>> if (!r5_rproc) {
>> dev_err(cdev, "failed to allocate memory for rproc instance\n");
>> + zynqmp_r5_free_mbox(ipi);
>> return ERR_PTR(-ENOMEM);
>> }
>>
>> @@ -665,6 +879,7 @@ static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
>> if (!r5_core->np) {
>> dev_err(cdev, "can't get device node for r5 core\n");
>> ret = -EINVAL;
>> + zynqmp_r5_free_mbox(ipi);
>> goto free_rproc;
>> }
>>
>> @@ -672,10 +887,17 @@ static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
>> ret = rproc_add(r5_rproc);
>> if (ret) {
>> dev_err(cdev, "failed to add r5 remoteproc\n");
>> + zynqmp_r5_free_mbox(ipi);
>> goto free_rproc;
>> }
>>
>> + if (ipi) {
>> + r5_core->ipi = ipi;
>> + ipi->r5_core = r5_core;
>> + }
>> +
>> r5_core->rproc = r5_rproc;
>> +
>> return r5_core;
>>
>> free_rproc:
>> @@ -918,6 +1140,7 @@ static int zynqmp_r5_cluster_init(struct zynqmp_r5_cluster *cluster)
>> while (i >= 0) {
>> put_device(child_devs[i]);
>> if (r5_cores[i]) {
>> + zynqmp_r5_free_mbox(r5_cores[i]->ipi);
> The mailboxes are initialized in zynqmp_r5_add_rproc_core() but free'd here in
> case of trouble, which introduces coupling between the two functions. I suggest
> moving zynqmp_r5_setup_mbox() in zynqmp_r5_cluster_init() and initialize both
> mailboxes in it.
>
> I am done reviewing this set.


Ack. Yes that makes sense.

Thanks,

Tanmay


>
> Thanks,
> Mathieu
>
> Thanks,
> Mathieu
>
>> of_reserved_mem_device_release(r5_cores[i]->dev);
>> rproc_del(r5_cores[i]->rproc);
>> rproc_free(r5_cores[i]->rproc);
>> @@ -942,6 +1165,7 @@ static void zynqmp_r5_cluster_exit(void *data)
>>
>> for (i = 0; i < cluster->core_count; i++) {
>> r5_core = cluster->r5_cores[i];
>> + zynqmp_r5_free_mbox(r5_core->ipi);
>> of_reserved_mem_device_release(r5_core->dev);
>> put_device(r5_core->dev);
>> rproc_del(r5_core->rproc);
>> --
>> 2.25.1
>>

\
 
 \ /
  Last update: 2023-03-27 00:32    [W:0.162 / U:0.676 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site