Hello!
On 23.04.2020 10:58, Christophe JAILLET wrote:
> Axe a memory allocation failure log message. This message is useless and
> incorrect (vmalloc is not used here for the memory allocation)
>
> This has been like that since the very beginning of this driver in
> commit 43f66a6ce8da ("Add ipw2200 wireless driver.")
>
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> drivers/net/wireless/intel/ipw2x00/ipw2200.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
> index 60b5e08dd6df..30c4f041f565 100644
> --- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
> +++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
> @@ -3770,10 +3770,9 @@ static int ipw_queue_tx_init(struct ipw_priv *priv,
> struct pci_dev *dev = priv->pci_dev;
>
> q->txb = kmalloc_array(count, sizeof(q->txb[0]), GFP_KERNEL);
> - if (!q->txb) {
> - IPW_ERROR("vmalloc for auxiliary BD structures failed\n");
> + if (!q->txb)
> return -ENOMEM;
> - }
> +
No need for this extra empty line.
>
> q->bd =
> pci_alloc_consistent(dev, sizeof(q->bd[0]) * count, &q->q.dma_addr);
MBR, Sergei
Le 23/04/2020 à 11:46, Sergei Shtylyov a écrit :
> Hello!
>
> On 23.04.2020 10:58, Christophe JAILLET wrote:
>
>> Axe a memory allocation failure log message. This message is useless and
>> incorrect (vmalloc is not used here for the memory allocation)
>>
>> This has been like that since the very beginning of this driver in
>> commit 43f66a6ce8da ("Add ipw2200 wireless driver.")
>>
>> Signed-off-by: Christophe JAILLET <[email protected]>
>> ---
>> drivers/net/wireless/intel/ipw2x00/ipw2200.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>> b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>> index 60b5e08dd6df..30c4f041f565 100644
>> --- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>> +++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>> @@ -3770,10 +3770,9 @@ static int ipw_queue_tx_init(struct ipw_priv
>> *priv,
>> struct pci_dev *dev = priv->pci_dev;
>> q->txb = kmalloc_array(count, sizeof(q->txb[0]), GFP_KERNEL);
>> - if (!q->txb) {
>> - IPW_ERROR("vmalloc for auxiliary BD structures failed\n");
>> + if (!q->txb)
>> return -ENOMEM;
>> - }
>> +
>
> No need for this extra empty line.
That's right, sorry about that.
Can it be fixed when/if the patch is applied, or should I send a V2?
If a V2 is required, should kcalloc be used, as pointed out by Joe Perches?
(personally, If the code works fine as-is, I don't think it is required,
but it can't hurt)
CJ
>
>> q->bd =
>> pci_alloc_consistent(dev, sizeof(q->bd[0]) * count,
>> &q->q.dma_addr);
>
> MBR, Sergei
>
>
Christophe JAILLET <[email protected]> writes:
> Le 23/04/2020 à 11:46, Sergei Shtylyov a écrit :
>> Hello!
>>
>> On 23.04.2020 10:58, Christophe JAILLET wrote:
>>
>>> Axe a memory allocation failure log message. This message is useless and
>>> incorrect (vmalloc is not used here for the memory allocation)
>>>
>>> This has been like that since the very beginning of this driver in
>>> commit 43f66a6ce8da ("Add ipw2200 wireless driver.")
>>>
>>> Signed-off-by: Christophe JAILLET <[email protected]>
>>> ---
>>> drivers/net/wireless/intel/ipw2x00/ipw2200.c | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> index 60b5e08dd6df..30c4f041f565 100644
>>> --- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> +++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> @@ -3770,10 +3770,9 @@ static int ipw_queue_tx_init(struct ipw_priv
>>> *priv,
>>> struct pci_dev *dev = priv->pci_dev;
>>> q->txb = kmalloc_array(count, sizeof(q->txb[0]), GFP_KERNEL);
>>> - if (!q->txb) {
>>> - IPW_ERROR("vmalloc for auxiliary BD structures failed\n");
>>> + if (!q->txb)
>>> return -ENOMEM;
>>> - }
>>> +
>>
>> No need for this extra empty line.
>
>
> That's right, sorry about that.
>
> Can it be fixed when/if the patch is applied, or should I send a V2?
Please send v2.
> If a V2 is required, should kcalloc be used, as pointed out by Joe Perches?
> (personally, If the code works fine as-is, I don't think it is
> required, but it can't hurt)
There's always the risk of regressions, which happens even with cleanup
patches so hurting is always possible :)
I can take a patch changing the allocation but please do it in a
separate patch. Though personally I wouldn't bother, ipw2x00 is an old
driver and not being actively developed anymore.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches