* [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
* [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
* [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 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 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 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
* 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
* 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
* 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).