On 01/14/2017 01:36 PM, Loic PALLARDY wrote: > > >> -----Original Message----- >> From: Suman Anna [mailto:s-anna@xxxxxx] >> Sent: Saturday, January 14, 2017 3:41 AM >> To: Loic PALLARDY <loic.pallardy@xxxxxx>; bjorn.andersson@xxxxxxxxxx; >> ohad@xxxxxxxxxx; lee.jones@xxxxxxxxxx; Patrice CHOTARD >> <patrice.chotard@xxxxxx> >> Cc: linux-remoteproc@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxx >> Subject: Re: [PATCH v1 6/6] rpmsg: virtio_rpmsg: set buffer configuration to >> virtio >> >> On 12/07/2016 02:35 PM, Loic Pallardy wrote: >>> Rpmsg is allocating buffer one dedicated communication link with some >>> specificity like number of buffers, size of one buffer... >>> These characteristics should be shared with remote coprocessor to >>> guarantee communication link coherency. >>> >>> Proposal is to update rpmsg configuration fields in coprocessor >>> firmware resource table if it exists. >>> >>> This is possible thanks to virtio set interface which allows to update >>> cfg fields of struct fw_rsc_vdev. >>> >>> Signed-off-by: Loic Pallardy <loic.pallardy@xxxxxx> >>> --- >>> drivers/rpmsg/virtio_rpmsg_bus.c | 25 +++++++++++++++++++++++++ >>> 1 file changed, 25 insertions(+) >>> >>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c >>> b/drivers/rpmsg/virtio_rpmsg_bus.c >>> index b12fe2a..5527b65 100644 >>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c >>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c >>> @@ -924,6 +924,23 @@ static int virtio_rpmsg_get_config(struct >>> virtio_device *vdev) >>> >>> } >>> >>> +static void virtio_rpmsg_set_config(struct virtio_device *vdev) { >>> + struct virtio_rpmsg_cfg virtio_cfg; >>> + struct virtproc_info *vrp = vdev->priv; >>> + >>> + /* fill virtio_cfg struct */ >>> + memset(&virtio_cfg, 0, sizeof(virtio_cfg)); >>> + virtio_cfg.id = VIRTIO_ID_RPMSG; >>> + virtio_cfg.da = vrp->bufs_dma; >> >> Hmm, I would expect this to fail if the device was behind an IOMMU. > I think so. I think I did the same vring. I need to check. > Perhaps better to set virtio_cfg.da to ANY_ADDRESS (-1) and add a comment about adding IOMMU support. > What do you think? Yeah, that is a better interim change. regards Suman > > Regards, > Loic >> >> regards >> Suman >> >>> + virtio_cfg.pa = vrp->bufs_dma; >>> + virtio_cfg.len = vrp->num_bufs * vrp->buf_size; >>> + virtio_cfg.buf_size = vrp->buf_size; >>> + >>> + vdev->config->set(vdev, RPMSG_CONFIG_OFFSET, &virtio_cfg, >>> + sizeof(virtio_cfg)); >>> +} >>> + >>> static int rpmsg_probe(struct virtio_device *vdev) { >>> vq_callback_t *vq_cbs[] = { rpmsg_recv_done, rpmsg_xmit_done }; >> @@ >>> -934,6 +951,7 @@ static int rpmsg_probe(struct virtio_device *vdev) >>> int err = 0, i; >>> size_t total_buf_space; >>> bool notify; >>> + bool has_cfg = false; >>> >>> vrp = kzalloc(sizeof(*vrp), GFP_KERNEL); >>> if (!vrp) >>> @@ -973,6 +991,9 @@ static int rpmsg_probe(struct virtio_device *vdev) >>> if (err < 0) >>> goto free_vrp; >>> >>> + if (err) >>> + has_cfg = true; >>> + >>> /* Allocate buffer if none provided by low level platform driver */ >>> if (!vrp->ext_buffer) { >>> total_buf_space = vrp->num_bufs * vrp->buf_size; @@ - >> 993,6 +1014,10 >>> @@ static int rpmsg_probe(struct virtio_device *vdev) >>> >>> /* and half is dedicated for TX */ >>> vrp->sbufs = bufs_va + total_buf_space / 2; >>> + >>> + /* Notify configuration to coprocessor */ >>> + if (has_cfg) >>> + virtio_rpmsg_set_config(vdev); >>> } >>> >>> /* set up the receive buffers */ >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
