soc: ti: k3-ringacc: Allow the driver to be built as module
Commit Message
The ring accelerator driver can be built as module since all depending
functions are exported.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
---
drivers/soc/ti/Kconfig | 2 +-
drivers/soc/ti/k3-ringacc.c | 29 ++++++++++++++++++++++++++---
2 files changed, 27 insertions(+), 4 deletions(-)
Comments
Le ven. 21 oct. 2022 à 09:53, Peter Ujfalusi
<peter.ujfalusi@gmail.com> a écrit :
>
> The ring accelerator driver can be built as module since all depending
> functions are exported.
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
Reviewed-by: Nicolas Frayer <nfrayer@baylibre.com>
Tested-by: Nicolas Frayer <nfrayer@baylibre.com>
> ---
> drivers/soc/ti/Kconfig | 2 +-
> drivers/soc/ti/k3-ringacc.c | 29 ++++++++++++++++++++++++++---
> 2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> index 7e2fb1c16af1..e009d9589af4 100644
> --- a/drivers/soc/ti/Kconfig
> +++ b/drivers/soc/ti/Kconfig
> @@ -63,7 +63,7 @@ config TI_SCI_PM_DOMAINS
> rootfs may be available.
>
> config TI_K3_RINGACC
> - bool "K3 Ring accelerator Sub System"
> + tristate "K3 Ring accelerator Sub System"
> depends on ARCH_K3 || COMPILE_TEST
> depends on TI_SCI_INTA_IRQCHIP
> help
> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
> index f7bf18b8229a..e011412f2975 100644
> --- a/drivers/soc/ti/k3-ringacc.c
> +++ b/drivers/soc/ti/k3-ringacc.c
> @@ -7,7 +7,7 @@
>
> #include <linux/dma-mapping.h>
> #include <linux/io.h>
> -#include <linux/init.h>
> +#include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> @@ -336,6 +336,11 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
>
> mutex_lock(&ringacc->req_lock);
>
> + if (!try_module_get(ringacc->dev->driver->owner)) {
> + mutex_unlock(&ringacc->req_lock);
> + return NULL;
> + }
> +
> if (id == K3_RINGACC_RING_ID_ANY) {
> /* Request for any general purpose ring */
> struct ti_sci_resource_desc *gp_rings =
> @@ -380,6 +385,7 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
> return &ringacc->rings[id];
>
> error:
> + module_put(ringacc->dev->driver->owner);
> mutex_unlock(&ringacc->req_lock);
> return NULL;
> }
> @@ -616,6 +622,8 @@ int k3_ringacc_ring_free(struct k3_ring *ring)
> no_init:
> clear_bit(ring->ring_id, ringacc->rings_inuse);
>
> + module_put(ringacc->dev->driver->owner);
> +
> out:
> mutex_unlock(&ringacc->req_lock);
> return 0;
> @@ -1450,6 +1458,7 @@ static const struct of_device_id k3_ringacc_of_match[] = {
> { .compatible = "ti,am654-navss-ringacc", .data = &k3_ringacc_data, },
> {},
> };
> +MODULE_DEVICE_TABLE(of, k3_ringacc_of_match);
>
> struct k3_ringacc *k3_ringacc_dmarings_init(struct platform_device *pdev,
> struct k3_ringacc_init_data *data)
> @@ -1544,12 +1553,26 @@ static int k3_ringacc_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static int k3_ringacc_remove(struct platform_device *pdev)
> +{
> + struct k3_ringacc *ringacc = dev_get_drvdata(&pdev->dev);
> +
> + mutex_lock(&k3_ringacc_list_lock);
> + list_del(&ringacc->list);
> + mutex_unlock(&k3_ringacc_list_lock);
> + return 0;
> +}
> +
> static struct platform_driver k3_ringacc_driver = {
> .probe = k3_ringacc_probe,
> + .remove = k3_ringacc_remove,
> .driver = {
> .name = "k3-ringacc",
> .of_match_table = k3_ringacc_of_match,
> - .suppress_bind_attrs = true,
> },
> };
> -builtin_platform_driver(k3_ringacc_driver);
> +module_platform_driver(k3_ringacc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("TI Ringacc driver for K3 SOCs");
> +MODULE_AUTHOR("Grygorii Strashko <grygorii.strashko@ti.com>");
> --
> 2.38.1
>
On 11:02-20221021, Peter Ujfalusi wrote:
> The ring accelerator driver can be built as module since all depending
> functions are exported.
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
> ---
> drivers/soc/ti/Kconfig | 2 +-
> drivers/soc/ti/k3-ringacc.c | 29 ++++++++++++++++++++++++++---
> 2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> index 7e2fb1c16af1..e009d9589af4 100644
> --- a/drivers/soc/ti/Kconfig
> +++ b/drivers/soc/ti/Kconfig
> @@ -63,7 +63,7 @@ config TI_SCI_PM_DOMAINS
> rootfs may be available.
>
> config TI_K3_RINGACC
> - bool "K3 Ring accelerator Sub System"
> + tristate "K3 Ring accelerator Sub System"
> depends on ARCH_K3 || COMPILE_TEST
> depends on TI_SCI_INTA_IRQCHIP
> help
> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
> index f7bf18b8229a..e011412f2975 100644
> --- a/drivers/soc/ti/k3-ringacc.c
> +++ b/drivers/soc/ti/k3-ringacc.c
> @@ -7,7 +7,7 @@
>
> #include <linux/dma-mapping.h>
> #include <linux/io.h>
> -#include <linux/init.h>
> +#include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> @@ -336,6 +336,11 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
>
> mutex_lock(&ringacc->req_lock);
>
> + if (!try_module_get(ringacc->dev->driver->owner)) {
Does a cascaded error handling make sense instead?
goto error_mod_fail?
> + mutex_unlock(&ringacc->req_lock);
> + return NULL;
> + }
> +
> if (id == K3_RINGACC_RING_ID_ANY) {
> /* Request for any general purpose ring */
> struct ti_sci_resource_desc *gp_rings =
> @@ -380,6 +385,7 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
> return &ringacc->rings[id];
>
> error:
> + module_put(ringacc->dev->driver->owner);
error_mod_fail:
> mutex_unlock(&ringacc->req_lock);
> return NULL;
> }
> @@ -616,6 +622,8 @@ int k3_ringacc_ring_free(struct k3_ring *ring)
> no_init:
> clear_bit(ring->ring_id, ringacc->rings_inuse);
>
> + module_put(ringacc->dev->driver->owner);
> +
> out:
> mutex_unlock(&ringacc->req_lock);
> return 0;
> @@ -1450,6 +1458,7 @@ static const struct of_device_id k3_ringacc_of_match[] = {
> { .compatible = "ti,am654-navss-ringacc", .data = &k3_ringacc_data, },
> {},
> };
> +MODULE_DEVICE_TABLE(of, k3_ringacc_of_match);
>
> struct k3_ringacc *k3_ringacc_dmarings_init(struct platform_device *pdev,
> struct k3_ringacc_init_data *data)
> @@ -1544,12 +1553,26 @@ static int k3_ringacc_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static int k3_ringacc_remove(struct platform_device *pdev)
> +{
> + struct k3_ringacc *ringacc = dev_get_drvdata(&pdev->dev);
> +
> + mutex_lock(&k3_ringacc_list_lock);
> + list_del(&ringacc->list);
> + mutex_unlock(&k3_ringacc_list_lock);
> + return 0;
> +}
> +
> static struct platform_driver k3_ringacc_driver = {
> .probe = k3_ringacc_probe,
> + .remove = k3_ringacc_remove,
> .driver = {
> .name = "k3-ringacc",
> .of_match_table = k3_ringacc_of_match,
> - .suppress_bind_attrs = true,
Might be good to note sysfs behavior change in commit log?
> },
> };
> -builtin_platform_driver(k3_ringacc_driver);
> +module_platform_driver(k3_ringacc_driver);
> +
> +MODULE_LICENSE("GPL v2");
GPL ? checkpatch should have complained?
> +MODULE_DESCRIPTION("TI Ringacc driver for K3 SOCs");
> +MODULE_AUTHOR("Grygorii Strashko <grygorii.strashko@ti.com>");
> --
> 2.38.1
>
On 10/28/22 03:55, Nishanth Menon wrote:
>> @@ -336,6 +336,11 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
>>
>> mutex_lock(&ringacc->req_lock);
>>
>> + if (!try_module_get(ringacc->dev->driver->owner)) {
>
> Does a cascaded error handling make sense instead?
>
> goto error_mod_fail?
I think it does not make the code nicer, but I can do that way
>
>> + mutex_unlock(&ringacc->req_lock);
>> + return NULL;
>> + }
>> +
>> if (id == K3_RINGACC_RING_ID_ANY) {
>> /* Request for any general purpose ring */
>> struct ti_sci_resource_desc *gp_rings =
>> @@ -380,6 +385,7 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
>> return &ringacc->rings[id];
>>
>> error:
>> + module_put(ringacc->dev->driver->owner);
>
> error_mod_fail:
>
>> mutex_unlock(&ringacc->req_lock);
>> return NULL;
>> }
>> @@ -616,6 +622,8 @@ int k3_ringacc_ring_free(struct k3_ring *ring)
>> no_init:
>> clear_bit(ring->ring_id, ringacc->rings_inuse);
>>
>> + module_put(ringacc->dev->driver->owner);
>> +
>> out:
>> mutex_unlock(&ringacc->req_lock);
>> return 0;
>> @@ -1450,6 +1458,7 @@ static const struct of_device_id k3_ringacc_of_match[] = {
>> { .compatible = "ti,am654-navss-ringacc", .data = &k3_ringacc_data, },
>> {},
>> };
>> +MODULE_DEVICE_TABLE(of, k3_ringacc_of_match);
>>
>> struct k3_ringacc *k3_ringacc_dmarings_init(struct platform_device *pdev,
>> struct k3_ringacc_init_data *data)
>> @@ -1544,12 +1553,26 @@ static int k3_ringacc_probe(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static int k3_ringacc_remove(struct platform_device *pdev)
>> +{
>> + struct k3_ringacc *ringacc = dev_get_drvdata(&pdev->dev);
>> +
>> + mutex_lock(&k3_ringacc_list_lock);
>> + list_del(&ringacc->list);
>> + mutex_unlock(&k3_ringacc_list_lock);
>> + return 0;
>> +}
>> +
>> static struct platform_driver k3_ringacc_driver = {
>> .probe = k3_ringacc_probe,
>> + .remove = k3_ringacc_remove,
>> .driver = {
>> .name = "k3-ringacc",
>> .of_match_table = k3_ringacc_of_match,
>> - .suppress_bind_attrs = true,
>
> Might be good to note sysfs behavior change in commit log?
Since the udma driver kept this I will keep it here as well.
>
>> },
>> };
>> -builtin_platform_driver(k3_ringacc_driver);
>> +module_platform_driver(k3_ringacc_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>
> GPL ? checkpatch should have complained?
Yes, it complains if I run it ;)
>
>> +MODULE_DESCRIPTION("TI Ringacc driver for K3 SOCs");
>> +MODULE_AUTHOR("Grygorii Strashko <grygorii.strashko@ti.com>");
>> --
>> 2.38.1
>>
>
@@ -63,7 +63,7 @@ config TI_SCI_PM_DOMAINS
rootfs may be available.
config TI_K3_RINGACC
- bool "K3 Ring accelerator Sub System"
+ tristate "K3 Ring accelerator Sub System"
depends on ARCH_K3 || COMPILE_TEST
depends on TI_SCI_INTA_IRQCHIP
help
@@ -7,7 +7,7 @@
#include <linux/dma-mapping.h>
#include <linux/io.h>
-#include <linux/init.h>
+#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
@@ -336,6 +336,11 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
mutex_lock(&ringacc->req_lock);
+ if (!try_module_get(ringacc->dev->driver->owner)) {
+ mutex_unlock(&ringacc->req_lock);
+ return NULL;
+ }
+
if (id == K3_RINGACC_RING_ID_ANY) {
/* Request for any general purpose ring */
struct ti_sci_resource_desc *gp_rings =
@@ -380,6 +385,7 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
return &ringacc->rings[id];
error:
+ module_put(ringacc->dev->driver->owner);
mutex_unlock(&ringacc->req_lock);
return NULL;
}
@@ -616,6 +622,8 @@ int k3_ringacc_ring_free(struct k3_ring *ring)
no_init:
clear_bit(ring->ring_id, ringacc->rings_inuse);
+ module_put(ringacc->dev->driver->owner);
+
out:
mutex_unlock(&ringacc->req_lock);
return 0;
@@ -1450,6 +1458,7 @@ static const struct of_device_id k3_ringacc_of_match[] = {
{ .compatible = "ti,am654-navss-ringacc", .data = &k3_ringacc_data, },
{},
};
+MODULE_DEVICE_TABLE(of, k3_ringacc_of_match);
struct k3_ringacc *k3_ringacc_dmarings_init(struct platform_device *pdev,
struct k3_ringacc_init_data *data)
@@ -1544,12 +1553,26 @@ static int k3_ringacc_probe(struct platform_device *pdev)
return 0;
}
+static int k3_ringacc_remove(struct platform_device *pdev)
+{
+ struct k3_ringacc *ringacc = dev_get_drvdata(&pdev->dev);
+
+ mutex_lock(&k3_ringacc_list_lock);
+ list_del(&ringacc->list);
+ mutex_unlock(&k3_ringacc_list_lock);
+ return 0;
+}
+
static struct platform_driver k3_ringacc_driver = {
.probe = k3_ringacc_probe,
+ .remove = k3_ringacc_remove,
.driver = {
.name = "k3-ringacc",
.of_match_table = k3_ringacc_of_match,
- .suppress_bind_attrs = true,
},
};
-builtin_platform_driver(k3_ringacc_driver);
+module_platform_driver(k3_ringacc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("TI Ringacc driver for K3 SOCs");
+MODULE_AUTHOR("Grygorii Strashko <grygorii.strashko@ti.com>");