PATCH IEEE 802.15.4: Add module parameter to mrf24j40 to allow use of external transmitters/receivers

PATCH IEEE 802.15.4: Add module parameter to mrf24j40 to allow use of external transmitters/receivers

[Walter Mack]

Add module parameter ext_rx_tx to support designs that use an external 
transmitter/receiver

Signed-off-by: Walter J. Mack<wmack@componentsw.com>


---
diff --git a/drivers/net/ieee802154/mrf24j40.c 
b/drivers/net/ieee802154/mrf24j40.c
index 9e6a124..6f87889 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -26,6 +26,10 @@
  #include <net/mac802154.h>
  #include <net/ieee802154.h>

+static bool ext_rx_tx = false ;
+module_param(ext_rx_tx, bool, 0444);
+MODULE_PARM_DESC(ext_rx_tx, "turn on statemachine to manage external 
tx/rx");
+
  /* MRF24J40 Short Address Registers */
  #define REG_RXMCR    0x00  /* Receive MAC control */
  #define REG_PANIDL   0x01  /* PAN ID (low) */
@@ -63,6 +67,8 @@
  #define REG_SLPCON1    0x220
  #define REG_WAKETIMEL  0x222  /* Wake-up Time Match Value Low */
  #define REG_WAKETIMEH  0x223  /* Wake-up Time Match Value High */
+#define REG_TESTMODE   0x22f  /* test mode and state machine control 
register */
+
  #define REG_RX_FIFO    0x300  /* Receive FIFO */

  /* Device configuration: Only channels 11-26 on page 0 are supported. */
@@ -679,6 +685,13 @@ static int mrf24j40_hw_init(struct mrf24j40 *devrec)

      udelay(192);

+    if (ext_rx_tx){
+      ret = write_long_reg(devrec, REG_TESTMODE, 0x0f);
+      if (ret)
+        goto err_ret;
+    }
+
+
      /* Set RX Mode. RXMCR<1:0>: 0x0 normal, 0x1 promisc, 0x2 error */
      ret = read_short_reg(devrec, REG_RXMCR, &val);
      if (ret)

Re: PATCH IEEE 802.15.4: Add module parameter to mrf24j40 to allow use of external transmitters/receivers

[Marcel Holtmann]

Hi Walter,

> Add module parameter ext_rx_tx to support designs that use an external transmitter/receiver

this commit message is now missing all the details. That is not what I asked for. Start by reading Documentation/SubmittingPatches on what is expected.

> Signed-off-by: Walter J. Mack<wmack@componentsw.com>

you really need to address the review comments that I made on your previous submission. Just sending the same patch that has the same coding style issues and has not addressed all comments is a waste of everyone's time.

Regards

Marcel

Re: PATCH IEEE 802.15.4: Add module parameter to mrf24j40 to allow use of external transmitters/receivers

[Marcel Holtmann]

Hi Walter,

I would really prefer if you keep the CC list in tact and start using the reply-all button. Also my preference is that people do not top-post on mailing lists.

> I did read through the submittingpatches document, and through the
> coding style document. I must have not understood them.
> 
> For my benefit, could you forward me an email that shows how a good
> patch submission looks like?
> 
> Regarding the style document, I cleaned up what I could see as being not
> conformant (spaces, curly braces), and I don't know what other
> violations I made.

My advise is to use git-format-patch and git-send-email. That makes sure that a lot of details are just done right. And of course git-log has plenty of examples on how other people are writing commit messages.

And the scripts/checkpatch.pl --strict is really what you want. I have not run the script, but I could spot obvious coding style mistakes from a mile away. Either you did it wrong or you mail client is messing up your email. Either way you need to get that fixed.

' might be better as '"Walter J. Mack" <wmack@componentsw.com>
#45: 
Signed-off-by: Walter J. Mack<wmack@componentsw.com>

ERROR: patch seems to be corrupt (line wrapped?)
#57: FILE: drivers/net/ieee802154/mrf24j40.c:27:


ERROR: DOS line endings
#58: FILE: drivers/net/ieee802154/mrf24j40.c:28:
+static bool ext_rx_tx = false ;^M$

ERROR: do not initialise statics to 0 or NULL
#58: FILE: drivers/net/ieee802154/mrf24j40.c:28:
+static bool ext_rx_tx = false ;

WARNING: space prohibited before semicolon
#58: FILE: drivers/net/ieee802154/mrf24j40.c:28:
+static bool ext_rx_tx = false ;

ERROR: DOS line endings
#59: FILE: drivers/net/ieee802154/mrf24j40.c:29:
+module_param(ext_rx_tx, bool, 0444);^M$

ERROR: trailing whitespace
#60: FILE: drivers/net/ieee802154/mrf24j40.c:30:
+MODULE_PARM_DESC(ext_rx_tx, "turn on statemachine to manage external ^M$

ERROR: trailing whitespace
#62: FILE: drivers/net/ieee802154/mrf24j40.c:31:
+^M$

ERROR: trailing whitespace
#70: FILE: drivers/net/ieee802154/mrf24j40.c:70:
+#define REG_TESTMODE   0x22f  /* test mode and state machine control ^M$

ERROR: trailing whitespace
#72: FILE: drivers/net/ieee802154/mrf24j40.c:71:
+^M$

ERROR: DOS line endings
#80: FILE: drivers/net/ieee802154/mrf24j40.c:686:
+    if (ext_rx_tx){^M$

WARNING: please, no spaces at the start of a line
#80: FILE: drivers/net/ieee802154/mrf24j40.c:686:
+    if (ext_rx_tx){^M$

WARNING: suspect code indent for conditional statements (4, 6)
#80: FILE: drivers/net/ieee802154/mrf24j40.c:686:
+    if (ext_rx_tx){
+      ret = write_long_reg(devrec, REG_TESTMODE, 0x0f);

ERROR: space required before the open brace '{'
#80: FILE: drivers/net/ieee802154/mrf24j40.c:686:
+    if (ext_rx_tx){

ERROR: DOS line endings
#81: FILE: drivers/net/ieee802154/mrf24j40.c:687:
+      ret = write_long_reg(devrec, REG_TESTMODE, 0x0f);^M$

WARNING: please, no spaces at the start of a line
#81: FILE: drivers/net/ieee802154/mrf24j40.c:687:
+      ret = write_long_reg(devrec, REG_TESTMODE, 0x0f);^M$

ERROR: DOS line endings
#82: FILE: drivers/net/ieee802154/mrf24j40.c:688:
+      if (ret)^M$

WARNING: please, no spaces at the start of a line
#82: FILE: drivers/net/ieee802154/mrf24j40.c:688:
+      if (ret)^M$

ERROR: DOS line endings
#83: FILE: drivers/net/ieee802154/mrf24j40.c:689:
+        goto err_ret;^M$

ERROR: code indent should use tabs where possible
#83: FILE: drivers/net/ieee802154/mrf24j40.c:689:
+        goto err_ret;^M$

WARNING: please, no spaces at the start of a line
#83: FILE: drivers/net/ieee802154/mrf24j40.c:689:
+        goto err_ret;^M$

ERROR: DOS line endings
#84: FILE: drivers/net/ieee802154/mrf24j40.c:690:
+    }^M$

WARNING: please, no spaces at the start of a line
#84: FILE: drivers/net/ieee802154/mrf24j40.c:690:
+    }^M$

ERROR: DOS line endings
#85: FILE: drivers/net/ieee802154/mrf24j40.c:691:
+^M$

ERROR: DOS line endings
#86: FILE: drivers/net/ieee802154/mrf24j40.c:692:
+^M$

CHECK: Please don't use multiple blank lines
#86: FILE: drivers/net/ieee802154/mrf24j40.c:692:
+
+

total: 17 errors, 8 warnings, 1 checks, 39 lines checked

NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
      scripts/cleanfile

Regards

Marcel

Re: PATCH IEEE 802.15.4: Add module parameter to mrf24j40 to allow use of external transmitters/receivers

[Alexander Aring]

Hi Marcel,

thanks for caring about this.

On Sun, Sep 14, 2014 at 01:30:02PM -0700, Marcel Holtmann wrote:
> Hi Walter,
> 
> > Add module parameter ext_rx_tx to support designs that use an external transmitter/receiver
> 
> this commit message is now missing all the details. That is not what I asked for. Start by reading Documentation/SubmittingPatches on what is expected.
> 
> > Signed-off-by: Walter J. Mack<wmack@componentsw.com>
> 
> you really need to address the review comments that I made on your previous submission. Just sending the same patch that has the same coding style issues and has not addressed all comments is a waste of everyone's time.
> 

Please also tag and base the patch for wpan-next.

Additional add the mrf24j40 maintainer alan@signal11.us to cc. After alan
ack alan it, I will apply this patch.

- Alex

Re: PATCH IEEE 802.15.4: Add module parameter to mrf24j40 to allow use of external transmitters/receivers

[Alexander Aring]

On Mon, Sep 15, 2014 at 01:13:50AM +0200, Alexander Aring wrote:
...
> 
> Please also tag and base the patch for wpan-next.
> 

oh, read the other commit msg now. So it was broken before? Then wpan please.

Then we can also think about for sending it to stable.

- Alex