On 28/08/2023 21:25, Bartosz Golaszewski wrote:
> This module is a platform driver that also exposes an interface for
> kernel users to allocate blocks of memory shared with the trustzone.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> ---
> drivers/firmware/Kconfig | 8 +
> drivers/firmware/Makefile | 1 +
> drivers/firmware/qcom-shm-bridge.c | 452 +++++++++++++++++++++++
> include/linux/firmware/qcom/shm-bridge.h | 32 ++
> 4 files changed, 493 insertions(+)
> create mode 100644 drivers/firmware/qcom-shm-bridge.c
> create mode 100644 include/linux/firmware/qcom/shm-bridge.h
>
...
> +/**
> + * qcom_shm_bridge_to_phys_addr - Translate address from virtual to physical.
> + *
> + * @vaddr: Virtual address to translate.
> + *
> + * Return:
> + * Physical address corresponding to 'vaddr'.
> + */
> +phys_addr_t qcom_shm_bridge_to_phys_addr(void *vaddr)
> +{
> + struct qcom_shm_bridge_chunk *chunk;
> + struct qcom_shm_bridge_pool *pool;
> +
> + guard(spinlock_irqsave)(&qcom_shm_bridge_chunks_lock);
> +
> + chunk = radix_tree_lookup(&qcom_shm_bridge_chunks,
> + (unsigned long)vaddr);
> + if (!chunk)
> + return 0;
> +
> + pool = chunk->parent;
> +
> + guard(spinlock_irqsave)(&pool->lock);
Why both locks are spinlocks? The locks are used quite a lot.
> +
> + return gen_pool_virt_to_phys(pool->genpool, (unsigned long)vaddr);
> +}
> +EXPORT_SYMBOL_GPL(qcom_shm_bridge_to_phys_addr);
> +
> +static int qcom_shm_bridge_probe(struct platform_device *pdev)
> +{
> + struct qcom_shm_bridge_pool *default_pool;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + /*
> + * We need to wait for the SCM device to be created and bound to the
> + * SCM driver.
> + */
> + if (!qcom_scm_is_available())
> + return -EPROBE_DEFER;
I think we miss here (and in all other drivers) device links to qcm.
> +
> + ret = qcom_scm_enable_shm_bridge();
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to enable the SHM bridge\n");
> +
> + default_pool = qcom_shm_bridge_pool_new_for_dev(
> + dev, qcom_shm_bridge_default_pool_size);
> + if (IS_ERR(default_pool))
> + return dev_err_probe(dev, PTR_ERR(default_pool),
> + "Failed to create the default SHM Bridge pool\n");
> +
> + WRITE_ONCE(qcom_shm_bridge_default_pool, default_pool);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id qcom_shm_bridge_of_match[] = {
> + { .compatible = "qcom,shm-bridge", },
> + { }
> +};
> +
> +static struct platform_driver qcom_shm_bridge_driver = {
> + .driver = {
> + .name = "qcom-shm-bridge",
> + .of_match_table = qcom_shm_bridge_of_match,
> + /*
> + * Once enabled, the SHM Bridge feature cannot be disabled so
> + * there's no reason to ever unbind the driver.
> + */
> + .suppress_bind_attrs = true,
> + },
> + .probe = qcom_shm_bridge_probe,
> +};
> +
> +static int __init qcom_shm_bridge_init(void)
> +{
> + return platform_driver_register(&qcom_shm_bridge_driver);
> +}
> +subsys_initcall(qcom_shm_bridge_init);
Why this is part of subsystem? Should be rather device_initcall... or
simply module (and a tristate).
Best regards,
Krzysztof
