[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4c6deafe-a6ec-40bf-873f-dc0df1a72dc4@gmail.com>
Date: Wed, 25 Jun 2025 11:14:56 +0200
From: Thomas Fourier <fourier.thomas@...il.com>
To: Simon Horman <horms@...nel.org>
Cc: Jakub Kicinski <kuba@...nel.org>, Chas Williams <3chas3@...il.com>,
"moderated list:ATM" <linux-atm-general@...ts.sourceforge.net>,
"open list:ATM" <netdev@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] atm: idt77252: Add missing `dma_map_error()`
On 24/06/2025 18:51, Simon Horman wrote:
> On Tue, Jun 24, 2025 at 08:41:47AM +0200, Thomas Fourier wrote:
>> The DMA map functions can fail and should be tested for errors.
>>
>> Signed-off-by: Thomas Fourier <fourier.thomas@...il.com>
>> ---
>> drivers/atm/idt77252.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
>> index 1206ab764ba9..f2e91b7d79f0 100644
>> --- a/drivers/atm/idt77252.c
>> +++ b/drivers/atm/idt77252.c
>> @@ -852,6 +852,8 @@ queue_skb(struct idt77252_dev *card, struct vc_map *vc,
>>
>> IDT77252_PRV_PADDR(skb) = dma_map_single(&card->pcidev->dev, skb->data,
>> skb->len, DMA_TO_DEVICE);
>> + if (dma_mapping_error(&card->pcidev->dev, IDT77252_PRV_PADDR(skb)))
>> + return -ENOMEM;
>>
>> error = -EINVAL;
>>
>> @@ -1857,6 +1859,8 @@ add_rx_skb(struct idt77252_dev *card, int queue,
>> paddr = dma_map_single(&card->pcidev->dev, skb->data,
>> skb_end_pointer(skb) - skb->data,
>> DMA_FROM_DEVICE);
>> + if (dma_mapping_error(&card->pcidev->dev, paddr))
>> + goto outpoolrm;
>> IDT77252_PRV_PADDR(skb) = paddr;
>>
>> if (push_rx_skb(card, skb, queue)) {
>> @@ -1871,6 +1875,7 @@ add_rx_skb(struct idt77252_dev *card, int queue,
>> dma_unmap_single(&card->pcidev->dev, IDT77252_PRV_PADDR(skb),
>> skb_end_pointer(skb) - skb->data, DMA_FROM_DEVICE);
>>
>> +outpoolrm:
>> handle = IDT77252_PRV_POOL(skb);
>> card->sbpool[POOL_QUEUE(handle)].skb[POOL_INDEX(handle)] = NULL;
> Hi Thomas,
>
> Can sb_pool_remove() be used here?
> It seems to be the converse of sb_pool_add().
> And safer than the code above.
> But perhaps I'm missing something.
Hi Simon,
I don't see any reason why this would be a problem,
though, I don't think it is related and the change should be in the same
patch.
Should I create another patch for that?
Powered by blists - more mailing lists