| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <676aa4c6-d317-a447-ddc1-9dd9fcc92c8f@foss.st.com>
Date: Fri, 7 Jan 2022 10:30:45 +0100
From: Arnaud POULIQUEN <arnaud.pouliquen@...s.st.com>
To: Mathieu Poirier <mathieu.poirier@...aro.org>
CC: Bjorn Andersson <bjorn.andersson@...aro.org>,
<linux-remoteproc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [RFC PATCH v2 4/6] remoteproc: virtio: Create platform device for
the remoteproc_virtio
Hello Mathieu,
On 1/6/22 7:53 PM, Mathieu Poirier wrote:
> On Wed, Dec 22, 2021 at 09:23:47AM +0100, Arnaud Pouliquen wrote:
>> Define a platform driver to prepare for the management of
>> remoteproc virtio devices as platform devices.
>>
>> The platform device allows to pass rproc_vdev_data platform data to
>> specify properties that are stored in the rproc_vdev structure.
>>
>> Such approach will allow to preserve legacy remoteproc virtio device
>> creation but also to probe the device using device tree mechanism.
>>
>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@...s.st.com>
>> ---
>> Update vs previous revision:
>> - Fix commit and rename rproc_vdev_data to rproc_vdev_pdata
>> ---
>> drivers/remoteproc/remoteproc_internal.h | 6 +++
>> drivers/remoteproc/remoteproc_virtio.c | 65 ++++++++++++++++++++++++
>> include/linux/remoteproc.h | 2 +
>> 3 files changed, 73 insertions(+)
>>
>> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
>> index e9e9a551a8c2..6f511c50a15d 100644
>> --- a/drivers/remoteproc/remoteproc_internal.h
>> +++ b/drivers/remoteproc/remoteproc_internal.h
>> @@ -24,6 +24,12 @@ struct rproc_debug_trace {
>> struct rproc_mem_entry trace_mem;
>> };
>>
>> +struct rproc_vdev_pdata {
>> + u32 rsc_offset;
>> + unsigned int id;
>> + unsigned int index;
>> +};
>> +
>> /* from remoteproc_core.c */
>> void rproc_release(struct kref *kref);
>> int rproc_of_parse_firmware(struct device *dev, int index,
>> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
>> index 51d415744fc6..5f8005caeb6e 100644
>> --- a/drivers/remoteproc/remoteproc_virtio.c
>> +++ b/drivers/remoteproc/remoteproc_virtio.c
>> @@ -4,6 +4,7 @@
>> *
>> * Copyright (C) 2011 Texas Instruments, Inc.
>> * Copyright (C) 2011 Google, Inc.
>> + * Copyright (C) 2021 STMicroelectronics
>> *
>> * Ohad Ben-Cohen <ohad@...ery.com>
>> * Brian Swetland <swetland@...gle.com>
>> @@ -13,6 +14,7 @@
>> #include <linux/dma-map-ops.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/export.h>
>> +#include <linux/of_platform.h>
>> #include <linux/of_reserved_mem.h>
>> #include <linux/remoteproc.h>
>> #include <linux/virtio.h>
>> @@ -575,3 +577,66 @@ void rproc_vdev_release(struct kref *ref)
>>
>> rproc_rvdev_remove_device(rvdev);
>> }
>> +
>> +static int rproc_virtio_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct rproc_vdev_pdata *vdev_data = dev->platform_data;
>> + struct rproc_vdev *rvdev;
>> + struct rproc *rproc;
>> +
>> + if (!vdev_data)
>> + return -EINVAL;
>> +
>> + rvdev = devm_kzalloc(dev, sizeof(*rvdev), GFP_KERNEL);
>> + if (!rvdev)
>> + return -ENOMEM;
>> +
>> + rproc = container_of(dev->parent, struct rproc, dev);
>> +
>> + rvdev->rsc_offset = vdev_data->rsc_offset;
>> + rvdev->id = vdev_data->id;
>> + rvdev->index = vdev_data->index;
>> +
>> + rvdev->pdev = pdev;
>> + rvdev->rproc = rproc;
>> +
>> + platform_set_drvdata(pdev, rvdev);
>> +
>> + return rproc_rvdev_add_device(rvdev);
>> +}
>> +
>> +static int rproc_virtio_remove(struct platform_device *pdev)
>> +{
>> + struct rproc_vdev *rvdev = dev_get_drvdata(&pdev->dev);
>> + struct rproc *rproc = rvdev->rproc;
>> + struct rproc_vring *rvring;
>> + int id;
>> +
>> + for (id = 0; id < ARRAY_SIZE(rvdev->vring); id++) {
>> + rvring = &rvdev->vring[id];
>> + rproc_free_vring(rvring);
>> + }
>> +
>> + rproc_remove_subdev(rproc, &rvdev->subdev);
>> + rproc_unregister_rvdev(rvdev);
>> + dev_dbg(&pdev->dev, "virtio dev %d removed\n", rvdev->index);
>> +
>
> Function rproc_virtio_remove() doesn't do the opposite of rproc_virtio_probe(),
> making it hard for people to wrap their head around what is happening. This may
> get cleaned up as part of the error path problem we already talked about... If not
> this is something to improve one.
>
> I am done reviewing this set.
Thanks for the review. I will address all your points in next version.
Regards,
Arnaud
>
> Thanks,
> Mathieu
>
>> + return 0;
>> +}
>> +
>> +/* Platform driver */
>> +static const struct of_device_id rproc_virtio_match[] = {
>> + { .compatible = "rproc-virtio", },
>> + {},
>> +};
>> +
>> +static struct platform_driver rproc_virtio_driver = {
>> + .probe = rproc_virtio_probe,
>> + .remove = rproc_virtio_remove,
>> + .driver = {
>> + .name = "rproc-virtio",
>> + .of_match_table = rproc_virtio_match,
>> + },
>> +};
>> +builtin_platform_driver(rproc_virtio_driver);
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index e0600e1e5c17..542a3d4664f2 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -616,6 +616,7 @@ struct rproc_vring {
>> * struct rproc_vdev - remoteproc state for a supported virtio device
>> * @refcount: reference counter for the vdev and vring allocations
>> * @subdev: handle for registering the vdev as a rproc subdevice
>> + * @pdev: remoteproc virtio platform device
>> * @dev: device struct used for reference count semantics
>> * @id: virtio device id (as in virtio_ids.h)
>> * @node: list node
>> @@ -628,6 +629,7 @@ struct rproc_vdev {
>> struct kref refcount;
>>
>> struct rproc_subdev subdev;
>> + struct platform_device *pdev;
>> struct device dev;
>>
>> unsigned int id;
>> --
>> 2.17.1
>>
Powered by blists - more mailing lists