[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6cc2948c-04eb-d62b-dfa0-5a94544ea6c9@gmail.com>
Date: Sat, 29 Oct 2022 09:45:53 +0300
From: Péter Ujfalusi <peter.ujfalusi@...il.com>
To: Nishanth Menon <nm@...com>
Cc: ssantosh@...nel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, khilman@...libre.com,
nfrayer@...libre.com
Subject: Re: [PATCH] soc: ti: k3-ringacc: Allow the driver to be built as
module
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@...com>");
>> --
>> 2.38.1
>>
>
--
Péter
Powered by blists - more mailing lists