Hello Péter,
On 14/12/23 21:11, Péter Ujfalusi wrote:
>
>
> On 12/12/2023 13:10, Siddharth Vadapalli wrote:
>> The existing function k3_udma_glue_request_tx_chn() supports requesting
>> a TX DMA channel by its name. Add support to request TX channel by ID in
>> the form of a new function k3_udma_glue_request_tx_chn_by_id() and export
>> it for use by drivers which are probed by alternate methods (non
>> device-tree) but still wish to make use of the existing DMA APIs. Such
>> drivers could be informed about the TX channel to use by RPMsg for example.
>>
>> Since the implementation of k3_udma_glue_request_tx_chn_by_id() reuses
>> most of the code in k3_udma_glue_request_tx_chn(), create a new function
>> for the common code named as k3_udma_glue_request_tx_chn_common().
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@xxxxxx>
>> ---
>> Changes since v1:
>> - Updated commit message indicating the use-case for which the patch is
>> being added.
>>
>> drivers/dma/ti/k3-udma-glue.c | 101 +++++++++++++++++++++++--------
>> include/linux/dma/k3-udma-glue.h | 4 ++
>> 2 files changed, 79 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/dma/ti/k3-udma-glue.c b/drivers/dma/ti/k3-udma-glue.c
>> index eff1ae3d3efe..ea5119a5e8eb 100644
>> --- a/drivers/dma/ti/k3-udma-glue.c
>> +++ b/drivers/dma/ti/k3-udma-glue.c
>> @@ -274,29 +274,13 @@ static int k3_udma_glue_cfg_tx_chn(struct k3_udma_glue_tx_channel *tx_chn)
>> return tisci_rm->tisci_udmap_ops->tx_ch_cfg(tisci_rm->tisci, &req);
>> }
>>
...
>>
>> err:
>> @@ -395,6 +410,40 @@ struct k3_udma_glue_tx_channel *k3_udma_glue_request_tx_chn(struct device *dev,
>> }
>> EXPORT_SYMBOL_GPL(k3_udma_glue_request_tx_chn);
>>
>> +struct k3_udma_glue_tx_channel *
>> +k3_udma_glue_request_tx_chn_by_id(struct device *dev, struct k3_udma_glue_tx_channel_cfg *cfg,
>> + struct device_node *udmax_np, u32 thread_id)
>
> udmax_np is not dev->of_node ?
I am not sure I fully understand the question. If you meant to ask if the driver
which uses this API will not have its device's of_node set to udmax_np, then yes
that's correct.
The driver shall be probed over RPMsg-bus, due to which its device's of_node
will not be udmax_np. Additionally, the udmax_np is the device-tree node of one
of the DMA Controllers described in the device-tree. The driver shall obtain the
reference to the udmax_np node using the API:
of_find_compatible_node()
with the compatible to be passed to the above API being a part of the driver's
data. Thus, it is possible to specify which DMA Controller to use by specifying
the compatible in the driver's data. I hope that I have answered your question.
Please let me know otherwise.
>
>> +{
>> + struct k3_udma_glue_tx_channel *tx_chn;
>> + int ret;
>> +
>> + tx_chn = devm_kzalloc(dev, sizeof(*tx_chn), GFP_KERNEL);
>> + if (!tx_chn)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + tx_chn->common.dev = dev;
>> + tx_chn->common.swdata_size = cfg->swdata_size;
>> + tx_chn->tx_pause_on_err = cfg->tx_pause_on_err;
>> + tx_chn->tx_filt_einfo = cfg->tx_filt_einfo;
>> + tx_chn->tx_filt_pswords = cfg->tx_filt_pswords;
>> + tx_chn->tx_supr_tdpkt = cfg->tx_supr_tdpkt;
>> +
>> + ret = of_k3_udma_glue_parse_chn_by_id(udmax_np, &tx_chn->common, true, thread_id);
>> + if (ret)
>> + goto err;
>> +
>> + ret = k3_udma_glue_request_tx_chn_common(dev, tx_chn, cfg);
>> + if (ret)
>> + goto err;
>> +
>> + return tx_chn;
>> +
>> +err:
>> + k3_udma_glue_release_tx_chn(tx_chn);
>> + return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(k3_udma_glue_request_tx_chn_by_id);
>> +
>> void k3_udma_glue_release_tx_chn(struct k3_udma_glue_tx_channel *tx_chn)
>> {
>> if (tx_chn->psil_paired) {
>> diff --git a/include/linux/dma/k3-udma-glue.h b/include/linux/dma/k3-udma-glue.h
>> index e443be4d3b4b..6205d84430ca 100644
>> --- a/include/linux/dma/k3-udma-glue.h
>> +++ b/include/linux/dma/k3-udma-glue.h
>> @@ -26,6 +26,10 @@ struct k3_udma_glue_tx_channel;
>> struct k3_udma_glue_tx_channel *k3_udma_glue_request_tx_chn(struct device *dev,
>> const char *name, struct k3_udma_glue_tx_channel_cfg *cfg);
>>
>> +struct k3_udma_glue_tx_channel *
>> +k3_udma_glue_request_tx_chn_by_id(struct device *dev, struct k3_udma_glue_tx_channel_cfg *cfg,
>
> I know it is going to be longer, but can the function be named more
> precisely?
> k3_udma_glue_request_tx_chn_by_thread_id
>
> For TX/RX: a channel is always for one thread, right?
> Probably:
> k3_udma_glue_request_tx_chn_for_thread_id()
>
> ?
Sure, I will rename it to:
k3_udma_glue_request_tx_chn_for_thread_id()
as suggested by you.
>
> Other then this the series looks OK.
Thank you for reviewing the series. I will rename the API as mentioned above and
if the question you had above regarding the of_node has been addressed, I will
post the v3 series. Kindly let me know.
--
Regards,
Siddharth.
