[PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Walter Mack]

The MRF24j40 device supports the use of external receivers/transmitters.
Microchip literature refers to them as LNA and PA respectively.
However, if external receivers/transmitters are present, register
"TESTMODE" has to be programmed with value 0f.
This patch adds a boolean module parameter ext_rx_tx. Setting it to true
configures the driver to adjust the mrf24j40 operation to work
with external receivers/transmitters.

Microchip's MRF24J40MB module is one instance that has external PA/LNA.
Without this new module parameter, transmission between two of these
modules was limited to less than 5 feet. With the module parameter set,
the range increased to well above 100 feet.

Signed-off-by: Walter Mack <wmack@componentsw.com>
---
 drivers/net/ieee802154/mrf24j40.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 9e6a124..e9cb5c6 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;
+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,12 @@ 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)
-- 
1.9.1

Re: [PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Alexander Aring]

Alan,

ping?

This looks for me like what Simon Vincent has send about a month ago.

I like more to check on chip revision instead setting module parameter,
but you have the last word.

Please, handle this in any way which you like. I see now there comming
more patches for the same issue, I don't like that. We should add
support for this.

- Alex

Re: [PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Walter Mack]

Alex,

this is not a question of chip revisions. Chips with the same revision 
can be used in designs with or without the external 
transmitter/receiver. Its just that the chip has to be set up to be 
compatible with the design.

On 09/16/2014 12:01 AM, Alexander Aring wrote:
> Alan,
>
> ping?
>
> This looks for me like what Simon Vincent has send about a month ago.
>
> I like more to check on chip revision instead setting module parameter,
> but you have the last word.
>
> Please, handle this in any way which you like. I see now there comming
> more patches for the same issue, I don't like that. We should add
> support for this.
>
> - Alex
>

Re: [PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Alexander Aring]

Hi Walter,

On Tue, Sep 16, 2014 at 01:13:12PM -0700, Walter Mack wrote:
> Alex,
> 
> this is not a question of chip revisions. Chips with the same revision can
> be used in designs with or without the external transmitter/receiver. Its
> just that the chip has to be set up to be compatible with the design.
> 

ah, ok. Dunno, I will wait on alan's opinion. If I don't heard anything in one
week I will apply it.

Deal?

Maybe alan wants also add a device tree support for this driver and then we could
make a dt property/platform data for that. But for now, I am also fine with module
parameter solution... because we don't have a dt support right now.

- Alex

Re: [PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Alan Ott]

On 09/16/2014 04:20 PM, Alexander Aring wrote:
> Hi Walter,
>
> On Tue, Sep 16, 2014 at 01:13:12PM -0700, Walter Mack wrote:
>> Alex,
>>
>> this is not a question of chip revisions. Chips with the same revision can
>> be used in designs with or without the external transmitter/receiver. Its
>> just that the chip has to be set up to be compatible with the design.
>>
> ah, ok. Dunno, I will wait on alan's opinion. If I don't heard anything in one
> week I will apply it.

Did Simon's patch get applied? I couldn't see it in any of the obvious 
places when I looked the other day.

>
> Maybe alan wants also add a device tree support for this driver and then we could
> make a dt property/platform data for that. But for now, I am also fine with module
> parameter solution... because we don't have a dt support right now.
>

I don't like this as a module parameter. That's not what module params 
are really designed for. It's better as DT so that it can be turned on 
and off for the individual devices, not system-wide.

mrf24j40 loads up fine under DT. There just isn't anything to configure 
(so far) other than the SPI.

Are there any other known boards which use the external amplifier 
besides the MB and MC (and MD and ME, which are the same as the MB and MC)?

Alex, I'm not sure how this fits with Simon's patch, as I haven't 
investigated it fully. I'd expect both implementations to be the same, 
but they're not.

Alan.

Re: [PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Alexander Aring]

Hi,

On Mon, Sep 22, 2014 at 11:51:43PM -0400, Alan Ott wrote:
> On 09/16/2014 04:20 PM, Alexander Aring wrote:
> >Hi Walter,
> >
> >On Tue, Sep 16, 2014 at 01:13:12PM -0700, Walter Mack wrote:
> >>Alex,
> >>
> >>this is not a question of chip revisions. Chips with the same revision can
> >>be used in designs with or without the external transmitter/receiver. Its
> >>just that the chip has to be set up to be compatible with the design.
> >>
> >ah, ok. Dunno, I will wait on alan's opinion. If I don't heard anything in one
> >week I will apply it.
> 
> Did Simon's patch get applied? I couldn't see it in any of the obvious
> places when I looked the other day.
> 

no, I finally wait for your Acked-by: .... otherwise I will not apply.
That's indicate for me that this patch is okay for you and came into
mainline. There are also pending patches from Varka, if you don't reply
on it, I will not apply it. Should I always simple apply the patches
if you don't response? It's better for me that I see a "Acked-by" from you.

Simon's patch state:
You had some reviewing notes at Simon's patch. I think Simon doesn't
response to this yet. I didn't see any new versions of this.



For DT example you can take a look into at86rf230 driver or cc2520. I
mean examples to add some DT properties. Also if you want to make this
driver dt enabled then you need a documentation in:

Documentation/devicetree/bindings/net/ieee802154/$DRIVERNAME.txt


- Alex

Re: [PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Simon Vincent]

I was under the impression the patch had been signed off. Maybe I missed 
a mail...
I can confirm the mrf24j40 works just fine using a DT.

Simon

On 23/09/14 08:19, Alexander Aring wrote:
> Hi,
>
> On Mon, Sep 22, 2014 at 11:51:43PM -0400, Alan Ott wrote:
>> On 09/16/2014 04:20 PM, Alexander Aring wrote:
>>> Hi Walter,
>>>
>>> On Tue, Sep 16, 2014 at 01:13:12PM -0700, Walter Mack wrote:
>>>> Alex,
>>>>
>>>> this is not a question of chip revisions. Chips with the same revision can
>>>> be used in designs with or without the external transmitter/receiver. Its
>>>> just that the chip has to be set up to be compatible with the design.
>>>>
>>> ah, ok. Dunno, I will wait on alan's opinion. If I don't heard anything in one
>>> week I will apply it.
>> Did Simon's patch get applied? I couldn't see it in any of the obvious
>> places when I looked the other day.
>>
> no, I finally wait for your Acked-by: .... otherwise I will not apply.
> That's indicate for me that this patch is okay for you and came into
> mainline. There are also pending patches from Varka, if you don't reply
> on it, I will not apply it. Should I always simple apply the patches
> if you don't response? It's better for me that I see a "Acked-by" from you.
>
> Simon's patch state:
> You had some reviewing notes at Simon's patch. I think Simon doesn't
> response to this yet. I didn't see any new versions of this.
>
>
>
> For DT example you can take a look into at86rf230 driver or cc2520. I
> mean examples to add some DT properties. Also if you want to make this
> driver dt enabled then you need a documentation in:
>
> Documentation/devicetree/bindings/net/ieee802154/$DRIVERNAME.txt
>
>
> - Alex

Re: [PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Alexander Aring]

On Tue, Sep 23, 2014 at 08:58:06AM +0100, Simon Vincent wrote:
> I was under the impression the patch had been signed off. Maybe I missed a
> mail...

mhh, I didn't see any signed-off or acked-by from alan. Maybe just cc
linux-wpan or me there.

> I can confirm the mrf24j40 works just fine using a DT.
> 

Yes basically this works, because dt have some out of the box feature to
load devices at boottime by modulename.

Normally you have some compatible names for your device like:

static const struct of_device_id at86rf230_of_match[] = {
        { .compatible = "atmel,at86rf230", },
        { .compatible = "atmel,at86rf231", },
        { .compatible = "atmel,at86rf233", },
        { .compatible = "atmel,at86rf212", },
        { },
};
MODULE_DEVICE_TABLE(of, at86rf230_of_match);

the mrf24j40 works for dt and has no platform data or something else.
But it's different than make the driver dt-enabled. It's only working
because DT allows to load modules by modulenames.

- Alex

Re: [PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Walter Mack]

On 09/22/2014 08:51 PM, Alan Ott wrote:
> On 09/16/2014 04:20 PM, Alexander Aring wrote:
>> Hi Walter,
>>
>> On Tue, Sep 16, 2014 at 01:13:12PM -0700, Walter Mack wrote:
>>> Alex,
>>>
>>> this is not a question of chip revisions. Chips with the same 
>>> revision can
>>> be used in designs with or without the external 
>>> transmitter/receiver. Its
>>> just that the chip has to be set up to be compatible with the design.
>>>
>> ah, ok. Dunno, I will wait on alan's opinion. If I don't heard 
>> anything in one
>> week I will apply it.
>
> Did Simon's patch get applied? I couldn't see it in any of the obvious 
> places when I looked the other day.
>
>>
>> Maybe alan wants also add a device tree support for this driver and 
>> then we could
>> make a dt property/platform data for that. But for now, I am also 
>> fine with module
>> parameter solution... because we don't have a dt support right now.
>>
>
> I don't like this as a module parameter. That's not what module params 
> are really designed for. It's better as DT so that it can be turned on 
> and off for the individual devices, not system-wide.
This is a valid point.
> mrf24j40 loads up fine under DT. There just isn't anything to 
> configure (so far) other than the SPI.
>
> Are there any other known boards which use the external amplifier 
> besides the MB and MC (and MD and ME, which are the same as the MB and 
> MC)?
Mine does. It is a custom design that hooks up to the extension port of 
the Raspberry PI. It doesn't have a product name..
>
> Alex, I'm not sure how this fits with Simon's patch, as I haven't 
> investigated it fully. I'd expect both implementations to be the same, 
> but they're not.
Is Simon's patch available for me to have a look at?
>
> Alan.
>
>

Re: [PATCH wpan-next] ieee802154: mrf24j40: support for external rx/tx

[Alexander Aring]

On Tue, Sep 23, 2014 at 11:11:01PM -0700, Walter Mack wrote:
> 
> On 09/22/2014 08:51 PM, Alan Ott wrote:
> >On 09/16/2014 04:20 PM, Alexander Aring wrote:
> >>Hi Walter,
> >>
> >>On Tue, Sep 16, 2014 at 01:13:12PM -0700, Walter Mack wrote:
> >>>Alex,
> >>>
> >>>this is not a question of chip revisions. Chips with the same revision
> >>>can
> >>>be used in designs with or without the external transmitter/receiver.
> >>>Its
> >>>just that the chip has to be set up to be compatible with the design.
> >>>
> >>ah, ok. Dunno, I will wait on alan's opinion. If I don't heard anything
> >>in one
> >>week I will apply it.
> >
> >Did Simon's patch get applied? I couldn't see it in any of the obvious
> >places when I looked the other day.
> >
> >>
> >>Maybe alan wants also add a device tree support for this driver and then
> >>we could
> >>make a dt property/platform data for that. But for now, I am also fine
> >>with module
> >>parameter solution... because we don't have a dt support right now.
> >>
> >
> >I don't like this as a module parameter. That's not what module params are
> >really designed for. It's better as DT so that it can be turned on and off
> >for the individual devices, not system-wide.
> This is a valid point.

ack.

> >mrf24j40 loads up fine under DT. There just isn't anything to configure
> >(so far) other than the SPI.
> >
> >Are there any other known boards which use the external amplifier besides
> >the MB and MC (and MD and ME, which are the same as the MB and MC)?
> Mine does. It is a custom design that hooks up to the extension port of the
> Raspberry PI. It doesn't have a product name..
> >
> >Alex, I'm not sure how this fits with Simon's patch, as I haven't
> >investigated it fully. I'd expect both implementations to be the same, but
> >they're not.
> Is Simon's patch available for me to have a look at?
> 

I upload it now to pastie.org... [0].

I want to try to add this patch in a seperate branch but this patch doesn't
apply on linux-wpan. There must be changes in the middle of them which
are not mainline. Diff reports about line 813, but the end of the file
is line 802.

Simon can you confirm this? I never tried to apply it since there was
other review notes.

The same what your patch does and Simon's is the setting of
REG_TESTMODE.

- Alex

[0] http://pastie.org/9589875