From: "Péter Ujfalusi" <peter.ujfalusi@gmail.com>
To: Vignesh Raghavendra <vigneshr@ti.com>, Vinod Koul <vkoul@kernel.org>
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
Linux ARM Mailing List <linux-arm-kernel@lists.infradead.org>,
Nishanth Menon <nm@ti.com>
Subject: Re: [PATCH] dma: ti: k3-udma: Fix smatch warnings
Date: Fri, 10 Dec 2021 22:10:38 +0200 [thread overview]
Message-ID: <b1966631-d0ea-eccd-a07d-a4d573b942f6@gmail.com> (raw)
In-Reply-To: <8ae3b70e-3697-2d3f-9a62-378f1a3748d7@gmail.com>
On 10/12/2021 22:10, Péter Ujfalusi wrote:
>
>
> On 09/12/2021 20:09, Vignesh Raghavendra wrote:
>> Smatch reports below warnings [1] wrt dereferencing rm_res when it can
>> potentially be ERR_PTR(). This is possible when entire range is
>> allocated to Linux
>> Fix this case by making sure, there is no deference of rm_res when its
>> ERR_PTR().
>
> Valid, early sysfs did not had rm ranges, thus we assumed all channels
s/sysfs/sysfw
> are for Linux, then we got support for one range per channel type and
> then the sets got introduced for supporting the differnt throughput
> levels of channles in the types.
> This surely got overlooked.
>
> Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
>
>>
>> [1]:
>> drivers/dma/ti/k3-udma.c:4524 udma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4537 udma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4681 bcdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4696 bcdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4711 bcdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4848 pktdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>> drivers/dma/ti/k3-udma.c:4861 pktdma_setup_resources() error: 'rm_res' dereferencing possible ERR_PTR()
>>
>> Reported-by: Nishanth Menon <nm@ti.com>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> ---
>> drivers/dma/ti/k3-udma.c | 157 ++++++++++++++++++++++++++-------------
>> 1 file changed, 107 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>> index 041d8e32d630..6e56d1cef5ee 100644
>> --- a/drivers/dma/ti/k3-udma.c
>> +++ b/drivers/dma/ti/k3-udma.c
>> @@ -4534,45 +4534,60 @@ static int udma_setup_resources(struct udma_dev *ud)
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->tchan_map, ud->tchan_cnt);
>> + irq_res.sets = 1;
>> } else {
>> bitmap_fill(ud->tchan_map, ud->tchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->tchan_map,
>> &rm_res->desc[i], "tchan");
>> + irq_res.sets = rm_res->sets;
>> }
>> - irq_res.sets = rm_res->sets;
>>
>> /* rchan and matching default flow ranges */
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->rchan_map, ud->rchan_cnt);
>> + irq_res.sets++;
>> } else {
>> bitmap_fill(ud->rchan_map, ud->rchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->rchan_map,
>> &rm_res->desc[i], "rchan");
>> + irq_res.sets += rm_res->sets;
>> }
>>
>> - irq_res.sets += rm_res->sets;
>> irq_res.desc = kcalloc(irq_res.sets, sizeof(*irq_res.desc), GFP_KERNEL);
>> + if (!irq_res.desc)
>> + return -ENOMEM;
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TCHAN];
>> - for (i = 0; i < rm_res->sets; i++) {
>> - irq_res.desc[i].start = rm_res->desc[i].start;
>> - irq_res.desc[i].num = rm_res->desc[i].num;
>> - irq_res.desc[i].start_sec = rm_res->desc[i].start_sec;
>> - irq_res.desc[i].num_sec = rm_res->desc[i].num_sec;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[0].start = 0;
>> + irq_res.desc[0].num = ud->tchan_cnt;
>> + i = 1;
>> + } else {
>> + for (i = 0; i < rm_res->sets; i++) {
>> + irq_res.desc[i].start = rm_res->desc[i].start;
>> + irq_res.desc[i].num = rm_res->desc[i].num;
>> + irq_res.desc[i].start_sec = rm_res->desc[i].start_sec;
>> + irq_res.desc[i].num_sec = rm_res->desc[i].num_sec;
>> + }
>> }
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RCHAN];
>> - for (j = 0; j < rm_res->sets; j++, i++) {
>> - if (rm_res->desc[j].num) {
>> - irq_res.desc[i].start = rm_res->desc[j].start +
>> - ud->soc_data->oes.udma_rchan;
>> - irq_res.desc[i].num = rm_res->desc[j].num;
>> - }
>> - if (rm_res->desc[j].num_sec) {
>> - irq_res.desc[i].start_sec = rm_res->desc[j].start_sec +
>> - ud->soc_data->oes.udma_rchan;
>> - irq_res.desc[i].num_sec = rm_res->desc[j].num_sec;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[i].start = 0;
>> + irq_res.desc[i].num = ud->rchan_cnt;
>> + } else {
>> + for (j = 0; j < rm_res->sets; j++, i++) {
>> + if (rm_res->desc[j].num) {
>> + irq_res.desc[i].start = rm_res->desc[j].start +
>> + ud->soc_data->oes.udma_rchan;
>> + irq_res.desc[i].num = rm_res->desc[j].num;
>> + }
>> + if (rm_res->desc[j].num_sec) {
>> + irq_res.desc[i].start_sec = rm_res->desc[j].start_sec +
>> + ud->soc_data->oes.udma_rchan;
>> + irq_res.desc[i].num_sec = rm_res->desc[j].num_sec;
>> + }
>> }
>> }
>> ret = ti_sci_inta_msi_domain_alloc_irqs(ud->dev, &irq_res);
>> @@ -4690,14 +4705,15 @@ static int bcdma_setup_resources(struct udma_dev *ud)
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_BCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->bchan_map, ud->bchan_cnt);
>> + irq_res.sets++;
>> } else {
>> bitmap_fill(ud->bchan_map, ud->bchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->bchan_map,
>> &rm_res->desc[i],
>> "bchan");
>> + irq_res.sets += rm_res->sets;
>> }
>> - irq_res.sets += rm_res->sets;
>> }
>>
>> /* tchan ranges */
>> @@ -4705,14 +4721,15 @@ static int bcdma_setup_resources(struct udma_dev *ud)
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->tchan_map, ud->tchan_cnt);
>> + irq_res.sets += 2;
>> } else {
>> bitmap_fill(ud->tchan_map, ud->tchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->tchan_map,
>> &rm_res->desc[i],
>> "tchan");
>> + irq_res.sets += rm_res->sets * 2;
>> }
>> - irq_res.sets += rm_res->sets * 2;
>> }
>>
>> /* rchan ranges */
>> @@ -4720,47 +4737,72 @@ static int bcdma_setup_resources(struct udma_dev *ud)
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RCHAN];
>> if (IS_ERR(rm_res)) {
>> bitmap_zero(ud->rchan_map, ud->rchan_cnt);
>> + irq_res.sets += 2;
>> } else {
>> bitmap_fill(ud->rchan_map, ud->rchan_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->rchan_map,
>> &rm_res->desc[i],
>> "rchan");
>> + irq_res.sets += rm_res->sets * 2;
>> }
>> - irq_res.sets += rm_res->sets * 2;
>> }
>>
>> irq_res.desc = kcalloc(irq_res.sets, sizeof(*irq_res.desc), GFP_KERNEL);
>> + if (!irq_res.desc)
>> + return -ENOMEM;
>> if (ud->bchan_cnt) {
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_BCHAN];
>> - for (i = 0; i < rm_res->sets; i++) {
>> - irq_res.desc[i].start = rm_res->desc[i].start +
>> - oes->bcdma_bchan_ring;
>> - irq_res.desc[i].num = rm_res->desc[i].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[0].start = oes->bcdma_bchan_ring;
>> + irq_res.desc[0].num = ud->bchan_cnt;
>> + i = 1;
>> + } else {
>> + for (i = 0; i < rm_res->sets; i++) {
>> + irq_res.desc[i].start = rm_res->desc[i].start +
>> + oes->bcdma_bchan_ring;
>> + irq_res.desc[i].num = rm_res->desc[i].num;
>> + }
>> }
>> }
>> if (ud->tchan_cnt) {
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TCHAN];
>> - for (j = 0; j < rm_res->sets; j++, i += 2) {
>> - irq_res.desc[i].start = rm_res->desc[j].start +
>> - oes->bcdma_tchan_data;
>> - irq_res.desc[i].num = rm_res->desc[j].num;
>> -
>> - irq_res.desc[i + 1].start = rm_res->desc[j].start +
>> - oes->bcdma_tchan_ring;
>> - irq_res.desc[i + 1].num = rm_res->desc[j].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[i].start = oes->bcdma_tchan_data;
>> + irq_res.desc[i].num = ud->tchan_cnt;
>> + irq_res.desc[i + 1].start = oes->bcdma_tchan_ring;
>> + irq_res.desc[i + 1].num = ud->tchan_cnt;
>> + i += 2;
>> + } else {
>> + for (j = 0; j < rm_res->sets; j++, i += 2) {
>> + irq_res.desc[i].start = rm_res->desc[j].start +
>> + oes->bcdma_tchan_data;
>> + irq_res.desc[i].num = rm_res->desc[j].num;
>> +
>> + irq_res.desc[i + 1].start = rm_res->desc[j].start +
>> + oes->bcdma_tchan_ring;
>> + irq_res.desc[i + 1].num = rm_res->desc[j].num;
>> + }
>> }
>> }
>> if (ud->rchan_cnt) {
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RCHAN];
>> - for (j = 0; j < rm_res->sets; j++, i += 2) {
>> - irq_res.desc[i].start = rm_res->desc[j].start +
>> - oes->bcdma_rchan_data;
>> - irq_res.desc[i].num = rm_res->desc[j].num;
>> -
>> - irq_res.desc[i + 1].start = rm_res->desc[j].start +
>> - oes->bcdma_rchan_ring;
>> - irq_res.desc[i + 1].num = rm_res->desc[j].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[i].start = oes->bcdma_rchan_data;
>> + irq_res.desc[i].num = ud->rchan_cnt;
>> + irq_res.desc[i + 1].start = oes->bcdma_rchan_ring;
>> + irq_res.desc[i + 1].num = ud->rchan_cnt;
>> + i += 2;
>> + } else {
>> + for (j = 0; j < rm_res->sets; j++, i += 2) {
>> + irq_res.desc[i].start = rm_res->desc[j].start +
>> + oes->bcdma_rchan_data;
>> + irq_res.desc[i].num = rm_res->desc[j].num;
>> +
>> + irq_res.desc[i + 1].start = rm_res->desc[j].start +
>> + oes->bcdma_rchan_ring;
>> + irq_res.desc[i + 1].num = rm_res->desc[j].num;
>> + }
>> }
>> }
>>
>> @@ -4858,39 +4900,54 @@ static int pktdma_setup_resources(struct udma_dev *ud)
>> if (IS_ERR(rm_res)) {
>> /* all rflows are assigned exclusively to Linux */
>> bitmap_zero(ud->rflow_in_use, ud->rflow_cnt);
>> + irq_res.sets = 1;
>> } else {
>> bitmap_fill(ud->rflow_in_use, ud->rflow_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->rflow_in_use,
>> &rm_res->desc[i], "rflow");
>> + irq_res.sets = rm_res->sets;
>> }
>> - irq_res.sets = rm_res->sets;
>>
>> /* tflow ranges */
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TFLOW];
>> if (IS_ERR(rm_res)) {
>> /* all tflows are assigned exclusively to Linux */
>> bitmap_zero(ud->tflow_map, ud->tflow_cnt);
>> + irq_res.sets++;
>> } else {
>> bitmap_fill(ud->tflow_map, ud->tflow_cnt);
>> for (i = 0; i < rm_res->sets; i++)
>> udma_mark_resource_ranges(ud, ud->tflow_map,
>> &rm_res->desc[i], "tflow");
>> + irq_res.sets += rm_res->sets;
>> }
>> - irq_res.sets += rm_res->sets;
>>
>> irq_res.desc = kcalloc(irq_res.sets, sizeof(*irq_res.desc), GFP_KERNEL);
>> + if (!irq_res.desc)
>> + return -ENOMEM;
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_TFLOW];
>> - for (i = 0; i < rm_res->sets; i++) {
>> - irq_res.desc[i].start = rm_res->desc[i].start +
>> - oes->pktdma_tchan_flow;
>> - irq_res.desc[i].num = rm_res->desc[i].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[0].start = oes->pktdma_tchan_flow;
>> + irq_res.desc[0].num = ud->tflow_cnt;
>> + i = 1;
>> + } else {
>> + for (i = 0; i < rm_res->sets; i++) {
>> + irq_res.desc[i].start = rm_res->desc[i].start +
>> + oes->pktdma_tchan_flow;
>> + irq_res.desc[i].num = rm_res->desc[i].num;
>> + }
>> }
>> rm_res = tisci_rm->rm_ranges[RM_RANGE_RFLOW];
>> - for (j = 0; j < rm_res->sets; j++, i++) {
>> - irq_res.desc[i].start = rm_res->desc[j].start +
>> - oes->pktdma_rchan_flow;
>> - irq_res.desc[i].num = rm_res->desc[j].num;
>> + if (IS_ERR(rm_res)) {
>> + irq_res.desc[i].start = oes->pktdma_rchan_flow;
>> + irq_res.desc[i].num = ud->rflow_cnt;
>> + } else {
>> + for (j = 0; j < rm_res->sets; j++, i++) {
>> + irq_res.desc[i].start = rm_res->desc[j].start +
>> + oes->pktdma_rchan_flow;
>> + irq_res.desc[i].num = rm_res->desc[j].num;
>> + }
>> }
>> ret = ti_sci_inta_msi_domain_alloc_irqs(ud->dev, &irq_res);
>> kfree(irq_res.desc);
>>
>
--
Péter
next prev parent reply other threads:[~2021-12-10 20:10 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-09 18:09 [PATCH] dma: ti: k3-udma: Fix smatch warnings Vignesh Raghavendra
2021-12-10 20:10 ` Péter Ujfalusi
2021-12-10 20:10 ` Péter Ujfalusi [this message]
2021-12-13 5:32 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b1966631-d0ea-eccd-a07d-a4d573b942f6@gmail.com \
--to=peter.ujfalusi@gmail.com \
--cc=dmaengine@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nm@ti.com \
--cc=vigneshr@ti.com \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).