* [PATCH bluetooth-next 0/3] ieee802154: cleanup for mrf24j40
@ 2014-09-16 4:48 Varka Bhadram
2014-09-16 4:48 ` [PATCH bluetooth-next 1/3] ieee802154: mrf24j40: fix Missing a blank line after declarations Varka Bhadram
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Varka Bhadram @ 2014-09-16 4:48 UTC (permalink / raw)
To: linux-wpan, linux-bluetooth; +Cc: alan, Varka Bhadram
Varka Bhadram (3):
ieee802154: mrf24j40: fix Missing a blank line after declarations
ieee802154: mrf24j40: remove return statement
ieee802154: mrf24j40: use pr_* / dev_*
drivers/net/ieee802154/mrf24j40.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH bluetooth-next 1/3] ieee802154: mrf24j40: fix Missing a blank line after declarations 2014-09-16 4:48 [PATCH bluetooth-next 0/3] ieee802154: cleanup for mrf24j40 Varka Bhadram @ 2014-09-16 4:48 ` Varka Bhadram 2014-09-18 22:54 ` Alan Ott 2014-09-16 4:48 ` [PATCH bluetooth-next 2/3] ieee802154: mrf24j40: remove return statement Varka Bhadram 2014-09-16 4:48 ` [PATCH bluetooth-next 3/3] ieee802154: mrf24j40: use pr_* / dev_* Varka Bhadram 2 siblings, 1 reply; 10+ messages in thread From: Varka Bhadram @ 2014-09-16 4:48 UTC (permalink / raw) To: linux-wpan, linux-bluetooth; +Cc: alan, Varka Bhadram Signed-off-by: Varka Bhadram <varkab@cdac.in> --- drivers/net/ieee802154/mrf24j40.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c index 9e6a124..466da57 100644 --- a/drivers/net/ieee802154/mrf24j40.c +++ b/drivers/net/ieee802154/mrf24j40.c @@ -412,6 +412,7 @@ static void mrf24j40_stop(struct ieee802154_dev *dev) struct mrf24j40 *devrec = dev->priv; u8 val; int ret; + dev_dbg(printdev(devrec), "stop\n"); ret = read_short_reg(devrec, REG_INTCON, &val); @@ -465,6 +466,7 @@ static int mrf24j40_filter(struct ieee802154_dev *dev, if (changed & IEEE802515_AFILT_SADDR_CHANGED) { /* Short Addr */ u8 addrh, addrl; + addrh = le16_to_cpu(filt->short_addr) >> 8 & 0xff; addrl = le16_to_cpu(filt->short_addr) & 0xff; @@ -493,6 +495,7 @@ static int mrf24j40_filter(struct ieee802154_dev *dev, if (changed & IEEE802515_AFILT_PANID_CHANGED) { /* PAN ID */ u8 panidl, panidh; + panidh = le16_to_cpu(filt->pan_id) >> 8 & 0xff; panidl = le16_to_cpu(filt->pan_id) & 0xff; write_short_reg(devrec, REG_PANIDH, panidh); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bluetooth-next 1/3] ieee802154: mrf24j40: fix Missing a blank line after declarations 2014-09-16 4:48 ` [PATCH bluetooth-next 1/3] ieee802154: mrf24j40: fix Missing a blank line after declarations Varka Bhadram @ 2014-09-18 22:54 ` Alan Ott 2014-09-19 3:22 ` Varka Bhadram 0 siblings, 1 reply; 10+ messages in thread From: Alan Ott @ 2014-09-18 22:54 UTC (permalink / raw) To: Varka Bhadram, linux-wpan, linux-bluetooth; +Cc: Varka Bhadram Make the first line of the commit message: mrf24j40: fix Missing a blank line after declarations On 09/16/2014 12:48 AM, Varka Bhadram wrote: > Signed-off-by: Varka Bhadram <varkab@cdac.in> > --- > drivers/net/ieee802154/mrf24j40.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c > index 9e6a124..466da57 100644 > --- a/drivers/net/ieee802154/mrf24j40.c > +++ b/drivers/net/ieee802154/mrf24j40.c > @@ -412,6 +412,7 @@ static void mrf24j40_stop(struct ieee802154_dev *dev) > struct mrf24j40 *devrec = dev->priv; > u8 val; > int ret; > + > dev_dbg(printdev(devrec), "stop\n"); > > ret = read_short_reg(devrec, REG_INTCON, &val); > @@ -465,6 +466,7 @@ static int mrf24j40_filter(struct ieee802154_dev *dev, > if (changed & IEEE802515_AFILT_SADDR_CHANGED) { > /* Short Addr */ > u8 addrh, addrl; > + > addrh = le16_to_cpu(filt->short_addr) >> 8 & 0xff; > addrl = le16_to_cpu(filt->short_addr) & 0xff; > > @@ -493,6 +495,7 @@ static int mrf24j40_filter(struct ieee802154_dev *dev, > if (changed & IEEE802515_AFILT_PANID_CHANGED) { > /* PAN ID */ > u8 panidl, panidh; > + > panidh = le16_to_cpu(filt->pan_id) >> 8 & 0xff; > panidl = le16_to_cpu(filt->pan_id) & 0xff; > write_short_reg(devrec, REG_PANIDH, panidh); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bluetooth-next 1/3] ieee802154: mrf24j40: fix Missing a blank line after declarations 2014-09-18 22:54 ` Alan Ott @ 2014-09-19 3:22 ` Varka Bhadram 0 siblings, 0 replies; 10+ messages in thread From: Varka Bhadram @ 2014-09-19 3:22 UTC (permalink / raw) To: Alan Ott, linux-wpan, linux-bluetooth; +Cc: Varka Bhadram On 09/19/2014 04:24 AM, Alan Ott wrote: > Make the first line of the commit message: > mrf24j40: fix Missing a blank line after declarations > Ok...Thanks -- Regards, Varka Bhadram. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bluetooth-next 2/3] ieee802154: mrf24j40: remove return statement 2014-09-16 4:48 [PATCH bluetooth-next 0/3] ieee802154: cleanup for mrf24j40 Varka Bhadram 2014-09-16 4:48 ` [PATCH bluetooth-next 1/3] ieee802154: mrf24j40: fix Missing a blank line after declarations Varka Bhadram @ 2014-09-16 4:48 ` Varka Bhadram 2014-09-18 22:55 ` Alan Ott 2014-09-16 4:48 ` [PATCH bluetooth-next 3/3] ieee802154: mrf24j40: use pr_* / dev_* Varka Bhadram 2 siblings, 1 reply; 10+ messages in thread From: Varka Bhadram @ 2014-09-16 4:48 UTC (permalink / raw) To: linux-wpan, linux-bluetooth; +Cc: alan, Varka Bhadram This patch remove the return statement for void function. void function return statements are not generally useful. Signed-off-by: Varka Bhadram <varkab@cdac.in> --- drivers/net/ieee802154/mrf24j40.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c index 466da57..2c617e3 100644 --- a/drivers/net/ieee802154/mrf24j40.c +++ b/drivers/net/ieee802154/mrf24j40.c @@ -420,8 +420,6 @@ static void mrf24j40_stop(struct ieee802154_dev *dev) return; val |= 0x1|0x8; /* Set TXNIE and RXIE. Disable Interrupts */ write_short_reg(devrec, REG_INTCON, val); - - return; } static int mrf24j40_set_channel(struct ieee802154_dev *dev, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bluetooth-next 2/3] ieee802154: mrf24j40: remove return statement 2014-09-16 4:48 ` [PATCH bluetooth-next 2/3] ieee802154: mrf24j40: remove return statement Varka Bhadram @ 2014-09-18 22:55 ` Alan Ott 2014-09-19 3:21 ` Varka Bhadram 0 siblings, 1 reply; 10+ messages in thread From: Alan Ott @ 2014-09-18 22:55 UTC (permalink / raw) To: Varka Bhadram, linux-wpan, linux-bluetooth; +Cc: Varka Bhadram Make the first line of the commit message: mrf24j40: remove unnecessary return statement On 09/16/2014 12:48 AM, Varka Bhadram wrote: > This patch remove the return statement for void function. > void function return statements are not generally useful. > Take out "this patch" and the second line. Make the comment: Remove the return statement in the void function. > Signed-off-by: Varka Bhadram <varkab@cdac.in> > --- > drivers/net/ieee802154/mrf24j40.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c > index 466da57..2c617e3 100644 > --- a/drivers/net/ieee802154/mrf24j40.c > +++ b/drivers/net/ieee802154/mrf24j40.c > @@ -420,8 +420,6 @@ static void mrf24j40_stop(struct ieee802154_dev *dev) > return; > val |= 0x1|0x8; /* Set TXNIE and RXIE. Disable Interrupts */ > write_short_reg(devrec, REG_INTCON, val); > - > - return; > } > > static int mrf24j40_set_channel(struct ieee802154_dev *dev, > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bluetooth-next 2/3] ieee802154: mrf24j40: remove return statement 2014-09-18 22:55 ` Alan Ott @ 2014-09-19 3:21 ` Varka Bhadram 0 siblings, 0 replies; 10+ messages in thread From: Varka Bhadram @ 2014-09-19 3:21 UTC (permalink / raw) To: Alan Ott, linux-wpan, linux-bluetooth; +Cc: Varka Bhadram On 09/19/2014 04:25 AM, Alan Ott wrote: > Make the first line of the commit message: > mrf24j40: remove unnecessary return statement > > On 09/16/2014 12:48 AM, Varka Bhadram wrote: >> This patch remove the return statement for void function. >> void function return statements are not generally useful. >> > > Take out "this patch" and the second line. Make the comment: > > Remove the return statement in the void function. > > Ok. Thanks Alan. -- Regards, Varka Bhadram. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bluetooth-next 3/3] ieee802154: mrf24j40: use pr_* / dev_* 2014-09-16 4:48 [PATCH bluetooth-next 0/3] ieee802154: cleanup for mrf24j40 Varka Bhadram 2014-09-16 4:48 ` [PATCH bluetooth-next 1/3] ieee802154: mrf24j40: fix Missing a blank line after declarations Varka Bhadram 2014-09-16 4:48 ` [PATCH bluetooth-next 2/3] ieee802154: mrf24j40: remove return statement Varka Bhadram @ 2014-09-16 4:48 ` Varka Bhadram 2014-09-18 23:01 ` Alan Ott 2 siblings, 1 reply; 10+ messages in thread From: Varka Bhadram @ 2014-09-16 4:48 UTC (permalink / raw) To: linux-wpan, linux-bluetooth; +Cc: alan, Varka Bhadram This patch replace printk() with dev_* if dev is available or replace with pr_* Signed-off-by: Varka Bhadram <varkab@cdac.in> --- drivers/net/ieee802154/mrf24j40.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c index 2c617e3..80b6adf 100644 --- a/drivers/net/ieee802154/mrf24j40.c +++ b/drivers/net/ieee802154/mrf24j40.c @@ -323,8 +323,8 @@ static int mrf24j40_read_rx_buf(struct mrf24j40 *devrec, #ifdef DEBUG print_hex_dump(KERN_DEBUG, "mrf24j40 rx: ", DUMP_PREFIX_OFFSET, 16, 1, data, *len, 0); - printk(KERN_DEBUG "mrf24j40 rx: lqi: %02hhx rssi: %02hhx\n", - lqi_rssi[0], lqi_rssi[1]); + pr_debug("mrf24j40 rx: lqi: %02hhx rssi: %02hhx\n", + lqi_rssi[0], lqi_rssi[1]); #endif out: @@ -385,7 +385,7 @@ err: static int mrf24j40_ed(struct ieee802154_dev *dev, u8 *level) { /* TODO: */ - printk(KERN_WARNING "mrf24j40: ed not implemented\n"); + pr_warn("mrf24j40: ed not implemented\n"); *level = 0; return 0; } @@ -482,12 +482,10 @@ static int mrf24j40_filter(struct ieee802154_dev *dev, for (i = 0; i < 8; i++) write_short_reg(devrec, REG_EADR0 + i, addr[i]); -#ifdef DEBUG - printk(KERN_DEBUG "Set long addr to: "); + pr_debug("Set long addr to: "); for (i = 0; i < 8; i++) - printk("%02hhx ", addr[7 - i]); - printk(KERN_DEBUG "\n"); -#endif + pr_debug("%02hhx ", addr[7 - i]); + pr_debug("\n"); } if (changed & IEEE802515_AFILT_PANID_CHANGED) { @@ -702,7 +700,7 @@ static int mrf24j40_probe(struct spi_device *spi) int ret = -ENOMEM; struct mrf24j40 *devrec; - printk(KERN_INFO "mrf24j40: probe(). IRQ: %d\n", spi->irq); + dev_info(&spi->dev, "probe(). IRQ: %d\n", spi->irq); devrec = devm_kzalloc(&spi->dev, sizeof(struct mrf24j40), GFP_KERNEL); if (!devrec) -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bluetooth-next 3/3] ieee802154: mrf24j40: use pr_* / dev_* 2014-09-16 4:48 ` [PATCH bluetooth-next 3/3] ieee802154: mrf24j40: use pr_* / dev_* Varka Bhadram @ 2014-09-18 23:01 ` Alan Ott [not found] ` <541BA2AB.2030503@gmail.com> 0 siblings, 1 reply; 10+ messages in thread From: Alan Ott @ 2014-09-18 23:01 UTC (permalink / raw) To: Varka Bhadram, linux-wpan, linux-bluetooth; +Cc: Varka Bhadram Take out the "ieee802154." Make the first line of the commit message: mrf24j40: use pr_* / dev_* instead of printk() On 09/16/2014 12:48 AM, Varka Bhadram wrote: > This patch replace printk() with dev_* if dev is available > or replace with pr_* Take out "this patch." Make it: Replace printk() with dev_*() pr_*() > > Signed-off-by: Varka Bhadram <varkab@cdac.in> > --- > drivers/net/ieee802154/mrf24j40.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c > index 2c617e3..80b6adf 100644 > --- a/drivers/net/ieee802154/mrf24j40.c > +++ b/drivers/net/ieee802154/mrf24j40.c > @@ -323,8 +323,8 @@ static int mrf24j40_read_rx_buf(struct mrf24j40 *devrec, > #ifdef DEBUG > print_hex_dump(KERN_DEBUG, "mrf24j40 rx: ", > DUMP_PREFIX_OFFSET, 16, 1, data, *len, 0); > - printk(KERN_DEBUG "mrf24j40 rx: lqi: %02hhx rssi: %02hhx\n", > - lqi_rssi[0], lqi_rssi[1]); > + pr_debug("mrf24j40 rx: lqi: %02hhx rssi: %02hhx\n", > + lqi_rssi[0], lqi_rssi[1]); > #endif > > out: > @@ -385,7 +385,7 @@ err: > static int mrf24j40_ed(struct ieee802154_dev *dev, u8 *level) > { > /* TODO: */ > - printk(KERN_WARNING "mrf24j40: ed not implemented\n"); > + pr_warn("mrf24j40: ed not implemented\n"); > *level = 0; > return 0; > } > @@ -482,12 +482,10 @@ static int mrf24j40_filter(struct ieee802154_dev *dev, > for (i = 0; i < 8; i++) > write_short_reg(devrec, REG_EADR0 + i, addr[i]); > > -#ifdef DEBUG > - printk(KERN_DEBUG "Set long addr to: "); > + pr_debug("Set long addr to: "); > for (i = 0; i < 8; i++) > - printk("%02hhx ", addr[7 - i]); > - printk(KERN_DEBUG "\n"); > -#endif > + pr_debug("%02hhx ", addr[7 - i]); > + pr_debug("\n"); Hmm... You took out the #ifdef DEBUG, but there's still a loop in there that will execute (optimizer aside). The pr_debug is ok, but leave it all inside the #ifdef DEBUG. > } > > if (changed & IEEE802515_AFILT_PANID_CHANGED) { > @@ -702,7 +700,7 @@ static int mrf24j40_probe(struct spi_device *spi) > int ret = -ENOMEM; > struct mrf24j40 *devrec; > > - printk(KERN_INFO "mrf24j40: probe(). IRQ: %d\n", spi->irq); > + dev_info(&spi->dev, "probe(). IRQ: %d\n", spi->irq); > > devrec = devm_kzalloc(&spi->dev, sizeof(struct mrf24j40), GFP_KERNEL); > if (!devrec) The rest looks ok. Thanks Varka. Alan. ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <541BA2AB.2030503@gmail.com>]
* Re: [PATCH bluetooth-next 3/3] ieee802154: mrf24j40: use pr_* / dev_* [not found] ` <541BA2AB.2030503@gmail.com> @ 2014-09-19 4:25 ` Alan Ott 0 siblings, 0 replies; 10+ messages in thread From: Alan Ott @ 2014-09-19 4:25 UTC (permalink / raw) To: Varka Bhadram, linux-wpan, linux-bluetooth; +Cc: Varka Bhadram On 09/18/2014 11:27 PM, Varka Bhadram wrote: >>> @@ -482,12 +482,10 @@ static int mrf24j40_filter(struct >>> ieee802154_dev *dev, >>> for (i = 0; i < 8; i++) >>> write_short_reg(devrec, REG_EADR0 + i, addr[i]); >>> -#ifdef DEBUG >>> - printk(KERN_DEBUG "Set long addr to: "); >>> + pr_debug("Set long addr to: "); >>> for (i = 0; i < 8; i++) >>> - printk("%02hhx ", addr[7 - i]); >>> - printk(KERN_DEBUG "\n"); >>> -#endif >>> + pr_debug("%02hhx ", addr[7 - i]); >>> + pr_debug("\n"); >> >> Hmm... You took out the #ifdef DEBUG, but there's still a loop in >> there that will execute (optimizer aside). The pr_debug is ok, but >> leave it all inside the #ifdef DEBUG. >> > I think if we use the pr_debug(), no need to need put*#ifdef DEBUG*...? > > I didn't get your point here... > What I mean is, even though pr_debug() is taken out by the preprocessor when DEBUG is not defined, the for-loop is not. Most likely, the empty loop gets optimized out by the compiler, but either way it's better to have the code be representative of what's going to execute. Therefore, I think we should keep the #ifdef DEBUG. Alan. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-09-19 4:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-16 4:48 [PATCH bluetooth-next 0/3] ieee802154: cleanup for mrf24j40 Varka Bhadram
2014-09-16 4:48 ` [PATCH bluetooth-next 1/3] ieee802154: mrf24j40: fix Missing a blank line after declarations Varka Bhadram
2014-09-18 22:54 ` Alan Ott
2014-09-19 3:22 ` Varka Bhadram
2014-09-16 4:48 ` [PATCH bluetooth-next 2/3] ieee802154: mrf24j40: remove return statement Varka Bhadram
2014-09-18 22:55 ` Alan Ott
2014-09-19 3:21 ` Varka Bhadram
2014-09-16 4:48 ` [PATCH bluetooth-next 3/3] ieee802154: mrf24j40: use pr_* / dev_* Varka Bhadram
2014-09-18 23:01 ` Alan Ott
[not found] ` <541BA2AB.2030503@gmail.com>
2014-09-19 4:25 ` Alan Ott
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).