All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Valo, Kalle" <kvalo@qca.qualcomm.com>
To: Joe Perches <joe@perches.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] ath10k: use dma_zalloc_coherent()
Date: Tue, 24 Jan 2017 12:13:32 +0000	[thread overview]
Message-ID: <87r33s8xli.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <1485235529.12563.39.camel@perches.com> (Joe Perches's message of "Mon, 23 Jan 2017 21:25:29 -0800")

Sm9lIFBlcmNoZXMgPGpvZUBwZXJjaGVzLmNvbT4gd3JpdGVzOg0KDQo+IE9uIFR1ZSwgMjAxNy0w
MS0yNCBhdCAwNToxOCArMDAwMCwgVmFsbywgS2FsbGUgd3JvdGU6DQo+PiBKb2UgUGVyY2hlcyA8
am9lQHBlcmNoZXMuY29tPiB3cml0ZXM6DQo+PiANCj4+ID4gT24gTW9uLCAyMDE3LTAxLTIzIGF0
IDE1OjA0ICswMDAwLCBTcmluaXZhcyBLYW5kYWdhdGxhIHdyb3RlOg0KPj4gPiA+IHVzZSBkbWFf
emFsbG9jX2NvaGVyZW50KCkgaW5zdGVhZCBvZiBkbWFfYWxsb2NfY29oZXJlbnQgYW5kIG1lbXNl
dCgpLg0KPj4gPiANCj4+ID4gW10NCj4+ID4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2ly
ZWxlc3MvYXRoL2F0aDEway9wY2kuYw0KPj4gPiA+IGIvZHJpdmVycy9uZXQvd2lyZWxlc3MvYXRo
L2F0aDEway9wY2kuYw0KPj4gPiANCj4+ID4gW10NCj4+ID4gPiBAQCAtODk2LDcgKzg5Niw3IEBA
IHN0YXRpYyBpbnQgYXRoMTBrX3BjaV9kaWFnX3JlYWRfbWVtKHN0cnVjdCBhdGgxMGsgKmFyLCB1
MzIgYWRkcmVzcywgdm9pZCAqZGF0YSwNCj4+ID4gPiAgCSAqLw0KPj4gPiA+ICAJYWxsb2NfbmJ5
dGVzID0gbWluX3QodW5zaWduZWQgaW50LCBuYnl0ZXMsIERJQUdfVFJBTlNGRVJfTElNSVQpOw0K
Pj4gPiA+ICANCj4+ID4gPiAtCWRhdGFfYnVmID0gKHVuc2lnbmVkIGNoYXIgKilkbWFfYWxsb2Nf
Y29oZXJlbnQoYXItPmRldiwNCj4+ID4gPiArCWRhdGFfYnVmID0gKHVuc2lnbmVkIGNoYXIgKilk
bWFfemFsbG9jX2NvaGVyZW50KGFyLT5kZXYsDQo+PiA+ID4gIAkJCQkJCSAgICAgICBhbGxvY19u
Ynl0ZXMsDQo+PiA+ID4gIAkJCQkJCSAgICAgICAmY2VfZGF0YV9iYXNlLA0KPj4gPiA+ICAJCQkJ
CQkgICAgICAgR0ZQX0FUT01JQyk7DQo+PiA+IA0KPj4gPiB0cml2aWE6DQo+PiA+IA0KPj4gPiBO
aWNlciB0byByZWFsaWduIGFyZ3VtZW50cyBhbmQgcmVtb3ZlIHRoZSB1bm5lY2Vzc2FyeSBjYXN0
Lg0KPj4gPiANCj4+ID4gUGVyaGFwczoNCj4+ID4gDQo+PiA+IAlkYXRhX2J1ZiA9IGRtYV96YWxs
b2NfY29oZXJlbnQoYXItPmRldizCoGFsbG9jX25ieXRlcywgJmNlX2RhdGFfYmFzZSwNCj4+ID4g
CQkJCSAgICAgIMKgR0ZQX0FUT01JQyk7DQo+PiANCj4+IFN1cmUsIGJ1dCB0aGF0IHNob3VsZCBi
ZSBpbiBhIHNlcGFyYXRlIHBhdGNoLg0KPg0KPiBJIGRvbid0IHRoaW5rIHNvLCB0cml2aWFsIHBh
dGNoZXMgY2FuIGJlIGNvbWJpbmVkLg0KPg0KPiBJdCdzIGFsc28gbmljZXIgdG8gcmVhbGlnbiBh
bGwgbW9kaWZpZWQgbXVsdGlsaW5lDQo+IGFyZ3VtZW50cyB3aGVuIHBlcmZvcm1pbmcgdGhlc2Ug
Y2hhbmdlcy4NCj4NCj4gQ29jY2luZWxsZSBnZW5lcmFsbHkgZG9lcyBpdCBhdXRvbWF0aWNhbGx5
Lg0KDQpBIG1hdHRlciBvZiBwcmVmZXJlbmNlIHJlYWxseS4gSSBwcmVmZXIga2VlcGluZyBzdHls
ZSBhbmQgZnVuY3Rpb25hbA0KY2hhbmdlcyBpbiBzZXBhcmF0ZSBwYXRjaGVzLCBrZWVwcyB0aGUg
cmV2aWV3IHNpbXBsZS4gQW5kIHN0eWxlIGNoYW5nZXMNCmNhbiBoaWRlIGJ1Z3MuDQoNCi0tIA0K
S2FsbGUgVmFsbw==

WARNING: multiple messages have this Message-ID (diff)
From: "Valo, Kalle" <kvalo@qca.qualcomm.com>
To: Joe Perches <joe@perches.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] ath10k: use dma_zalloc_coherent()
Date: Tue, 24 Jan 2017 12:13:32 +0000	[thread overview]
Message-ID: <87r33s8xli.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <1485235529.12563.39.camel@perches.com> (Joe Perches's message of "Mon, 23 Jan 2017 21:25:29 -0800")

Joe Perches <joe@perches.com> writes:

> On Tue, 2017-01-24 at 05:18 +0000, Valo, Kalle wrote:
>> Joe Perches <joe@perches.com> writes:
>> 
>> > On Mon, 2017-01-23 at 15:04 +0000, Srinivas Kandagatla wrote:
>> > > use dma_zalloc_coherent() instead of dma_alloc_coherent and memset().
>> > 
>> > []
>> > > diff --git a/drivers/net/wireless/ath/ath10k/pci.c
>> > > b/drivers/net/wireless/ath/ath10k/pci.c
>> > 
>> > []
>> > > @@ -896,7 +896,7 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32 address, void *data,
>> > >  	 */
>> > >  	alloc_nbytes = min_t(unsigned int, nbytes, DIAG_TRANSFER_LIMIT);
>> > >  
>> > > -	data_buf = (unsigned char *)dma_alloc_coherent(ar->dev,
>> > > +	data_buf = (unsigned char *)dma_zalloc_coherent(ar->dev,
>> > >  						       alloc_nbytes,
>> > >  						       &ce_data_base,
>> > >  						       GFP_ATOMIC);
>> > 
>> > trivia:
>> > 
>> > Nicer to realign arguments and remove the unnecessary cast.
>> > 
>> > Perhaps:
>> > 
>> > 	data_buf = dma_zalloc_coherent(ar->dev, alloc_nbytes, &ce_data_base,
>> > 				       GFP_ATOMIC);
>> 
>> Sure, but that should be in a separate patch.
>
> I don't think so, trivial patches can be combined.
>
> It's also nicer to realign all modified multiline
> arguments when performing these changes.
>
> Coccinelle generally does it automatically.

A matter of preference really. I prefer keeping style and functional
changes in separate patches, keeps the review simple. And style changes
can hide bugs.

-- 
Kalle Valo

WARNING: multiple messages have this Message-ID (diff)
From: "Valo, Kalle" <kvalo@qca.qualcomm.com>
To: Joe Perches <joe@perches.com>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH 2/3] ath10k: use dma_zalloc_coherent()
Date: Tue, 24 Jan 2017 12:13:32 +0000	[thread overview]
Message-ID: <87r33s8xli.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <1485235529.12563.39.camel@perches.com> (Joe Perches's message of "Mon, 23 Jan 2017 21:25:29 -0800")

Joe Perches <joe@perches.com> writes:

> On Tue, 2017-01-24 at 05:18 +0000, Valo, Kalle wrote:
>> Joe Perches <joe@perches.com> writes:
>> 
>> > On Mon, 2017-01-23 at 15:04 +0000, Srinivas Kandagatla wrote:
>> > > use dma_zalloc_coherent() instead of dma_alloc_coherent and memset().
>> > 
>> > []
>> > > diff --git a/drivers/net/wireless/ath/ath10k/pci.c
>> > > b/drivers/net/wireless/ath/ath10k/pci.c
>> > 
>> > []
>> > > @@ -896,7 +896,7 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32 address, void *data,
>> > >  	 */
>> > >  	alloc_nbytes = min_t(unsigned int, nbytes, DIAG_TRANSFER_LIMIT);
>> > >  
>> > > -	data_buf = (unsigned char *)dma_alloc_coherent(ar->dev,
>> > > +	data_buf = (unsigned char *)dma_zalloc_coherent(ar->dev,
>> > >  						       alloc_nbytes,
>> > >  						       &ce_data_base,
>> > >  						       GFP_ATOMIC);
>> > 
>> > trivia:
>> > 
>> > Nicer to realign arguments and remove the unnecessary cast.
>> > 
>> > Perhaps:
>> > 
>> > 	data_buf = dma_zalloc_coherent(ar->dev, alloc_nbytes, &ce_data_base,
>> > 				       GFP_ATOMIC);
>> 
>> Sure, but that should be in a separate patch.
>
> I don't think so, trivial patches can be combined.
>
> It's also nicer to realign all modified multiline
> arguments when performing these changes.
>
> Coccinelle generally does it automatically.

A matter of preference really. I prefer keeping style and functional
changes in separate patches, keeps the review simple. And style changes
can hide bugs.

-- 
Kalle Valo
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

  reply	other threads:[~2017-01-24 12:13 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-23 15:04 [PATCH 1/3] ath10k: remove multiple defines of DIAG_TRANSFER_LIMIT Srinivas Kandagatla
2017-01-23 15:04 ` Srinivas Kandagatla
2017-01-23 15:04 ` [PATCH 2/3] ath10k: use dma_zalloc_coherent() Srinivas Kandagatla
2017-01-23 15:04   ` Srinivas Kandagatla
2017-01-23 23:19   ` Joe Perches
2017-01-23 23:19     ` Joe Perches
2017-01-24  5:18     ` Valo, Kalle
2017-01-24  5:18       ` Valo, Kalle
2017-01-24  5:18       ` Valo, Kalle
2017-01-24  5:25       ` Joe Perches
2017-01-24  5:25         ` Joe Perches
2017-01-24 12:13         ` Valo, Kalle [this message]
2017-01-24 12:13           ` Valo, Kalle
2017-01-24 12:13           ` Valo, Kalle
2017-01-23 15:04 ` [PATCH 3/3] ath10k: fix typo in addr calculation Srinivas Kandagatla
2017-01-23 15:04   ` Srinivas Kandagatla
2017-01-27 18:04 ` [1/3] ath10k: remove multiple defines of DIAG_TRANSFER_LIMIT Kalle Valo
2017-01-27 18:04   ` Kalle Valo
2017-01-27 18:04   ` Kalle Valo

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=87r33s8xli.fsf@kamboji.qca.qualcomm.com \
    --to=kvalo@qca.qualcomm.com \
    --cc=ath10k@lists.infradead.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=srinivas.kandagatla@linaro.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.