[riot-notifications] [RIOT-OS/RIOT] drivers/mrf24j40: add external PA/LNA control on MC/MD/ME devices (#11410)

Peter Kietzmann notifications at github.com
Tue May 21 09:29:17 CEST 2019


PeterKietzmann commented on this pull request.

@benpicco thanks for improving the driver further! However, it is very difficult for me to confirm your changes because (i) I don't have an other device than the MD version and (ii) it seems I didn't find the right documentation. Can you point me to the manual you used? Just by looking at the code, I found some things not  intuitive (see inline).

@Carton32 did you test this PR? Would you agree with taking this PR instead of #10625? If I see it correctly, you are still in the commit history of this PR.

> + */
+void mrf24j40_enable_pa_lna(mrf24j40_t *dev);
+
+/**
+ * @brief   Disable External Power Amplifier & Low Noise Amplifier
+ *
+ * @param[in] dev       device to disable the PA & LNA on
+ */
+void mrf24j40_disable_pa_lna(mrf24j40_t *dev);
+
+/**
+ * @brief   Enable only the External Low Noise Amplifier
+ *
+ * @param[in] dev       device enable the LNA on
+ */
+void mrf24j40_enable_lna(mrf24j40_t *dev);

`pa_lna` needs en-/disable but `lna` only needs enable?

> +#else
+    (void) dev;
+#endif
+}
+
+void mrf24j40_disable_pa_lna(mrf24j40_t *dev)
+{
+#if MRF24J40_USE_EXT_PA_LNA
+    /* Disable automatic switch on PA/LNA */
+    mrf24j40_reg_write_long(dev, MRF24J40_REG_TESTMODE, MRF24J40_TESTMODE_RSSIWAIT0);
+
+    /* Configure all GPIOs as Output */
+    mrf24j40_reg_write_short(dev, MRF24J40_REG_TRISGPIO, 0xF);
+
+    /* Disable all GPIO outputs */
+    mrf24j40_reg_write_short(dev, MRF24J40_REG_GPIO, 0);

Could it happen that other I/Os would be used for different purposes? If so, this line would disable them

> +#else
+    (void) dev;
+#endif
+}
+
+void mrf24j40_enable_lna(mrf24j40_t *dev)
+{
+#if MRF24J40_USE_EXT_PA_LNA
+    /* Disable automatic switch on PA/LNA */
+    mrf24j40_reg_write_long(dev, MRF24J40_REG_TESTMODE, MRF24J40_TESTMODE_RSSIWAIT0);
+
+    /* Configure all GPIOs as Output */
+    mrf24j40_reg_write_short(dev, MRF24J40_REG_TRISGPIO, 0xF);
+
+    /* Enable LNA */
+    mrf24j40_reg_write_short(dev, MRF24J40_REG_GPIO, 0xC);

For `lna` you need bit2 and bit3 whereas `pa_lna` only required bit3? 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11410#pullrequestreview-239864098
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190521/c0ee6073/attachment-0001.html>


More information about the notifications mailing list