[RFC bluetooth-next 00/21] mrf24j40: async rx/tx handling and new features

[RFC bluetooth-next 00/21] mrf24j40: async rx/tx handling and new features

[Alexander Aring]

Hi,

this patch series contains some cleanups, devicetree support, rx/tx async
handling, threaded irq to hardirq, csma params settings, cca mode settings,
cca energy detection levels, tx power settings and promiscuous mode settings.

Also I add regmap support, the lowlevel spi calls are only used in hotpaths
for handling receive/transmit and irq handling. There are also some many
magic numbers, maybe we can introduce some register bits defines.

First I will send this as RFC to get some respone if the patches looks fine
and acceptable for mainline.

- Alex

Alexander Aring (21):
  mrf24j40: cleanup define identation
  mrf24j40: use ieee802154_alloc_hw for private data
  mrf24j40: calling ieee802154_register_hw at last
  mrf24j40: remove spi settings overwrite
  mrf24j40: add device-tree support
  mrf24j40: add default channel setting
  mrf24j40: add random extended addr generation
  mrf24j40: add more register defines
  mrf24j40: add regmap support
  mrf24j40: use regmap for register access
  mrf24j40: change to frame delivery with crc
  ieee802154: add helpers for frame control checks
  mrf24j40: rework tx handling to async tx handling
  mrf24j40: rework rx handling to async rx handling
  mrf24j40: async interrupt handling
  mrf24j40: add csma params support
  mrf24j40: add cca mode support
  mrf24j40: add cca ed level support
  mrf24j40: add tx power support
  mrf24j40: add promiscuous mode support
  mrf24j40: change irq trigger type behaviour

 .../bindings/net/ieee802154/mrf24j40.txt           |   20 +
 MAINTAINERS                                        |    1 +
 drivers/net/ieee802154/Kconfig                     |    1 +
 drivers/net/ieee802154/mrf24j40.c                  | 1342 +++++++++++++-------
 include/linux/ieee802154.h                         |   29 +
 5 files changed, 969 insertions(+), 424 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt

-- 
2.5.0

[RFC bluetooth-next 01/21] mrf24j40: cleanup define identation

[Alexander Aring]

This patch replaces the spaces after define by a tab.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 76 +++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 997724b..2b7fc00 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -23,46 +23,46 @@
 #include <net/mac802154.h>
 
 /* MRF24J40 Short Address Registers */
-#define REG_RXMCR    0x00  /* Receive MAC control */
-#define REG_PANIDL   0x01  /* PAN ID (low) */
-#define REG_PANIDH   0x02  /* PAN ID (high) */
-#define REG_SADRL    0x03  /* Short address (low) */
-#define REG_SADRH    0x04  /* Short address (high) */
-#define REG_EADR0    0x05  /* Long address (low) (high is EADR7) */
-#define REG_TXMCR    0x11  /* Transmit MAC control */
-#define REG_PACON0   0x16  /* Power Amplifier Control */
-#define REG_PACON1   0x17  /* Power Amplifier Control */
-#define REG_PACON2   0x18  /* Power Amplifier Control */
-#define REG_TXNCON   0x1B  /* Transmit Normal FIFO Control */
-#define REG_TXSTAT   0x24  /* TX MAC Status Register */
-#define REG_SOFTRST  0x2A  /* Soft Reset */
-#define REG_TXSTBL   0x2E  /* TX Stabilization */
-#define REG_INTSTAT  0x31  /* Interrupt Status */
-#define REG_INTCON   0x32  /* Interrupt Control */
-#define REG_GPIO     0x33  /* GPIO */
-#define REG_TRISGPIO 0x34  /* GPIO direction */
-#define REG_RFCTL    0x36  /* RF Control Mode Register */
-#define REG_BBREG1   0x39  /* Baseband Registers */
-#define REG_BBREG2   0x3A  /* */
-#define REG_BBREG6   0x3E  /* */
-#define REG_CCAEDTH  0x3F  /* Energy Detection Threshold */
+#define REG_RXMCR	0x00  /* Receive MAC control */
+#define REG_PANIDL	0x01  /* PAN ID (low) */
+#define REG_PANIDH	0x02  /* PAN ID (high) */
+#define REG_SADRL	0x03  /* Short address (low) */
+#define REG_SADRH	0x04  /* Short address (high) */
+#define REG_EADR0	0x05  /* Long address (low) (high is EADR7) */
+#define REG_TXMCR	0x11  /* Transmit MAC control */
+#define REG_PACON0	0x16  /* Power Amplifier Control */
+#define REG_PACON1	0x17  /* Power Amplifier Control */
+#define REG_PACON2	0x18  /* Power Amplifier Control */
+#define REG_TXNCON	0x1B  /* Transmit Normal FIFO Control */
+#define REG_TXSTAT	0x24  /* TX MAC Status Register */
+#define REG_SOFTRST	0x2A  /* Soft Reset */
+#define REG_TXSTBL	0x2E  /* TX Stabilization */
+#define REG_INTSTAT	0x31  /* Interrupt Status */
+#define REG_INTCON	0x32  /* Interrupt Control */
+#define REG_GPIO	0x33  /* GPIO */
+#define REG_TRISGPIO	0x34  /* GPIO direction */
+#define REG_RFCTL	0x36  /* RF Control Mode Register */
+#define REG_BBREG1	0x39  /* Baseband Registers */
+#define REG_BBREG2	0x3A  /* */
+#define REG_BBREG6	0x3E  /* */
+#define REG_CCAEDTH	0x3F  /* Energy Detection Threshold */
 
 /* MRF24J40 Long Address Registers */
-#define REG_RFCON0     0x200  /* RF Control Registers */
-#define REG_RFCON1     0x201
-#define REG_RFCON2     0x202
-#define REG_RFCON3     0x203
-#define REG_RFCON5     0x205
-#define REG_RFCON6     0x206
-#define REG_RFCON7     0x207
-#define REG_RFCON8     0x208
-#define REG_RSSI       0x210
-#define REG_SLPCON0    0x211  /* Sleep Clock Control Registers */
-#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 */
-#define REG_RX_FIFO    0x300  /* Receive FIFO */
+#define REG_RFCON0	0x200  /* RF Control Registers */
+#define REG_RFCON1	0x201
+#define REG_RFCON2	0x202
+#define REG_RFCON3	0x203
+#define REG_RFCON5	0x205
+#define REG_RFCON6	0x206
+#define REG_RFCON7	0x207
+#define REG_RFCON8	0x208
+#define REG_RSSI	0x210
+#define REG_SLPCON0	0x211  /* Sleep Clock Control Registers */
+#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 */
+#define REG_RX_FIFO	0x300  /* Receive FIFO */
 
 /* Device configuration: Only channels 11-26 on page 0 are supported. */
 #define MRF24J40_CHAN_MIN 11
-- 
2.5.0

Re: [RFC bluetooth-next 01/21] mrf24j40: cleanup define identation

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch replaces the spaces after define by a tab.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 76 +++++++++++++++++++--------------------
>   1 file changed, 38 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 997724b..2b7fc00 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -23,46 +23,46 @@
>   #include <net/mac802154.h>
>   
>   /* MRF24J40 Short Address Registers */
> -#define REG_RXMCR    0x00  /* Receive MAC control */
> -#define REG_PANIDL   0x01  /* PAN ID (low) */
> -#define REG_PANIDH   0x02  /* PAN ID (high) */
> -#define REG_SADRL    0x03  /* Short address (low) */
> -#define REG_SADRH    0x04  /* Short address (high) */
> -#define REG_EADR0    0x05  /* Long address (low) (high is EADR7) */
> -#define REG_TXMCR    0x11  /* Transmit MAC control */
> -#define REG_PACON0   0x16  /* Power Amplifier Control */
> -#define REG_PACON1   0x17  /* Power Amplifier Control */
> -#define REG_PACON2   0x18  /* Power Amplifier Control */
> -#define REG_TXNCON   0x1B  /* Transmit Normal FIFO Control */
> -#define REG_TXSTAT   0x24  /* TX MAC Status Register */
> -#define REG_SOFTRST  0x2A  /* Soft Reset */
> -#define REG_TXSTBL   0x2E  /* TX Stabilization */
> -#define REG_INTSTAT  0x31  /* Interrupt Status */
> -#define REG_INTCON   0x32  /* Interrupt Control */
> -#define REG_GPIO     0x33  /* GPIO */
> -#define REG_TRISGPIO 0x34  /* GPIO direction */
> -#define REG_RFCTL    0x36  /* RF Control Mode Register */
> -#define REG_BBREG1   0x39  /* Baseband Registers */
> -#define REG_BBREG2   0x3A  /* */
> -#define REG_BBREG6   0x3E  /* */
> -#define REG_CCAEDTH  0x3F  /* Energy Detection Threshold */
> +#define REG_RXMCR	0x00  /* Receive MAC control */
> +#define REG_PANIDL	0x01  /* PAN ID (low) */
> +#define REG_PANIDH	0x02  /* PAN ID (high) */
> +#define REG_SADRL	0x03  /* Short address (low) */
> +#define REG_SADRH	0x04  /* Short address (high) */
> +#define REG_EADR0	0x05  /* Long address (low) (high is EADR7) */
> +#define REG_TXMCR	0x11  /* Transmit MAC control */
> +#define REG_PACON0	0x16  /* Power Amplifier Control */
> +#define REG_PACON1	0x17  /* Power Amplifier Control */
> +#define REG_PACON2	0x18  /* Power Amplifier Control */
> +#define REG_TXNCON	0x1B  /* Transmit Normal FIFO Control */
> +#define REG_TXSTAT	0x24  /* TX MAC Status Register */
> +#define REG_SOFTRST	0x2A  /* Soft Reset */
> +#define REG_TXSTBL	0x2E  /* TX Stabilization */
> +#define REG_INTSTAT	0x31  /* Interrupt Status */
> +#define REG_INTCON	0x32  /* Interrupt Control */
> +#define REG_GPIO	0x33  /* GPIO */
> +#define REG_TRISGPIO	0x34  /* GPIO direction */
> +#define REG_RFCTL	0x36  /* RF Control Mode Register */
> +#define REG_BBREG1	0x39  /* Baseband Registers */
> +#define REG_BBREG2	0x3A  /* */
> +#define REG_BBREG6	0x3E  /* */
> +#define REG_CCAEDTH	0x3F  /* Energy Detection Threshold */
>   
>   /* MRF24J40 Long Address Registers */
> -#define REG_RFCON0     0x200  /* RF Control Registers */
> -#define REG_RFCON1     0x201
> -#define REG_RFCON2     0x202
> -#define REG_RFCON3     0x203
> -#define REG_RFCON5     0x205
> -#define REG_RFCON6     0x206
> -#define REG_RFCON7     0x207
> -#define REG_RFCON8     0x208
> -#define REG_RSSI       0x210
> -#define REG_SLPCON0    0x211  /* Sleep Clock Control Registers */
> -#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 */
> -#define REG_RX_FIFO    0x300  /* Receive FIFO */
> +#define REG_RFCON0	0x200  /* RF Control Registers */
> +#define REG_RFCON1	0x201
> +#define REG_RFCON2	0x202
> +#define REG_RFCON3	0x203
> +#define REG_RFCON5	0x205
> +#define REG_RFCON6	0x206
> +#define REG_RFCON7	0x207
> +#define REG_RFCON8	0x208
> +#define REG_RSSI	0x210
> +#define REG_SLPCON0	0x211  /* Sleep Clock Control Registers */
> +#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 */
> +#define REG_RX_FIFO	0x300  /* Receive FIFO */
>   
>   /* Device configuration: Only channels 11-26 on page 0 are supported. */
>   #define MRF24J40_CHAN_MIN 11

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 02/21] mrf24j40: use ieee802154_alloc_hw for private data

[Alexander Aring]

This patch removes the own private dataroom allocation by calling
devm_kzalloc for devrec and assign this pointer to "devrec->hw->priv".
Instead we using like all other drivers ieee802154_alloc_hw and give the
size for the private driver dataroom at the first argument.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 2b7fc00..1023cd2 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -723,16 +723,28 @@ err_ret:
 static int mrf24j40_probe(struct spi_device *spi)
 {
 	int ret = -ENOMEM;
+	struct ieee802154_hw *hw;
 	struct mrf24j40 *devrec;
 
 	dev_info(&spi->dev, "probe(). IRQ: %d\n", spi->irq);
 
-	devrec = devm_kzalloc(&spi->dev, sizeof(struct mrf24j40), GFP_KERNEL);
-	if (!devrec)
+	/* Register with the 802154 subsystem */
+
+	hw = ieee802154_alloc_hw(sizeof(*devrec), &mrf24j40_ops);
+	if (!hw)
 		goto err_ret;
+
+	devrec = hw->priv;
+	devrec->spi = spi;
+	spi_set_drvdata(spi, devrec);
+	devrec->hw = hw;
+	devrec->hw->parent = &spi->dev;
+	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
+	devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AFILT;
+
 	devrec->buf = devm_kzalloc(&spi->dev, 3, GFP_KERNEL);
 	if (!devrec->buf)
-		goto err_ret;
+		goto err_register_device;
 
 	spi->mode = SPI_MODE_0; /* TODO: Is this appropriate for right here? */
 	if (spi->max_speed_hz > MAX_SPI_SPEED_HZ)
@@ -740,19 +752,6 @@ static int mrf24j40_probe(struct spi_device *spi)
 
 	mutex_init(&devrec->buffer_mutex);
 	init_completion(&devrec->tx_complete);
-	devrec->spi = spi;
-	spi_set_drvdata(spi, devrec);
-
-	/* Register with the 802154 subsystem */
-
-	devrec->hw = ieee802154_alloc_hw(0, &mrf24j40_ops);
-	if (!devrec->hw)
-		goto err_ret;
-
-	devrec->hw->priv = devrec;
-	devrec->hw->parent = &devrec->spi->dev;
-	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
-	devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AFILT;
 
 	dev_dbg(printdev(devrec), "registered mrf24j40\n");
 	ret = ieee802154_register_hw(devrec->hw);
-- 
2.5.0

Re: [RFC bluetooth-next 02/21] mrf24j40: use ieee802154_alloc_hw for private data

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch removes the own private dataroom allocation by calling
> devm_kzalloc for devrec and assign this pointer to "devrec->hw->priv".
> Instead we using like all other drivers ieee802154_alloc_hw and give the
> size for the private driver dataroom at the first argument.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 31 +++++++++++++++----------------
>   1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 2b7fc00..1023cd2 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -723,16 +723,28 @@ err_ret:
>   static int mrf24j40_probe(struct spi_device *spi)
>   {
>   	int ret = -ENOMEM;
> +	struct ieee802154_hw *hw;
>   	struct mrf24j40 *devrec;
>   
>   	dev_info(&spi->dev, "probe(). IRQ: %d\n", spi->irq);
>   
> -	devrec = devm_kzalloc(&spi->dev, sizeof(struct mrf24j40), GFP_KERNEL);
> -	if (!devrec)
> +	/* Register with the 802154 subsystem */
> +
> +	hw = ieee802154_alloc_hw(sizeof(*devrec), &mrf24j40_ops);
> +	if (!hw)
>   		goto err_ret;
> +
> +	devrec = hw->priv;
> +	devrec->spi = spi;
> +	spi_set_drvdata(spi, devrec);
> +	devrec->hw = hw;
> +	devrec->hw->parent = &spi->dev;
> +	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
> +	devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AFILT;
> +
>   	devrec->buf = devm_kzalloc(&spi->dev, 3, GFP_KERNEL);
>   	if (!devrec->buf)
> -		goto err_ret;
> +		goto err_register_device;
>   
>   	spi->mode = SPI_MODE_0; /* TODO: Is this appropriate for right here? */
>   	if (spi->max_speed_hz > MAX_SPI_SPEED_HZ)
> @@ -740,19 +752,6 @@ static int mrf24j40_probe(struct spi_device *spi)
>   
>   	mutex_init(&devrec->buffer_mutex);
>   	init_completion(&devrec->tx_complete);
> -	devrec->spi = spi;
> -	spi_set_drvdata(spi, devrec);
> -
> -	/* Register with the 802154 subsystem */
> -
> -	devrec->hw = ieee802154_alloc_hw(0, &mrf24j40_ops);
> -	if (!devrec->hw)
> -		goto err_ret;
> -
> -	devrec->hw->priv = devrec;
> -	devrec->hw->parent = &devrec->spi->dev;
> -	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
> -	devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AFILT;
>   
>   	dev_dbg(printdev(devrec), "registered mrf24j40\n");
>   	ret = ieee802154_register_hw(devrec->hw);

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 03/21] mrf24j40: calling ieee802154_register_hw at last

[Alexander Aring]

The function ieee802154_register_hw should always called at last.
Currently we do hardware init and such things after register hardware
into the subsystem. It could be that the subsystem already call driver
operations while running hardware init.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 1023cd2..de63cba 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -753,14 +753,9 @@ static int mrf24j40_probe(struct spi_device *spi)
 	mutex_init(&devrec->buffer_mutex);
 	init_completion(&devrec->tx_complete);
 
-	dev_dbg(printdev(devrec), "registered mrf24j40\n");
-	ret = ieee802154_register_hw(devrec->hw);
-	if (ret)
-		goto err_register_device;
-
 	ret = mrf24j40_hw_init(devrec);
 	if (ret)
-		goto err_hw_init;
+		goto err_register_device;
 
 	ret = devm_request_threaded_irq(&spi->dev,
 					spi->irq,
@@ -772,14 +767,16 @@ static int mrf24j40_probe(struct spi_device *spi)
 
 	if (ret) {
 		dev_err(printdev(devrec), "Unable to get IRQ");
-		goto err_irq;
+		goto err_register_device;
 	}
 
+	dev_dbg(printdev(devrec), "registered mrf24j40\n");
+	ret = ieee802154_register_hw(devrec->hw);
+	if (ret)
+		goto err_register_device;
+
 	return 0;
 
-err_irq:
-err_hw_init:
-	ieee802154_unregister_hw(devrec->hw);
 err_register_device:
 	ieee802154_free_hw(devrec->hw);
 err_ret:
-- 
2.5.0

Re: [RFC bluetooth-next 03/21] mrf24j40: calling ieee802154_register_hw at last

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> The function ieee802154_register_hw should always called at last.
> Currently we do hardware init and such things after register hardware
> into the subsystem. It could be that the subsystem already call driver
> operations while running hardware init.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 17 +++++++----------
>   1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 1023cd2..de63cba 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -753,14 +753,9 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	mutex_init(&devrec->buffer_mutex);
>   	init_completion(&devrec->tx_complete);
>   
> -	dev_dbg(printdev(devrec), "registered mrf24j40\n");
> -	ret = ieee802154_register_hw(devrec->hw);
> -	if (ret)
> -		goto err_register_device;
> -
>   	ret = mrf24j40_hw_init(devrec);
>   	if (ret)
> -		goto err_hw_init;
> +		goto err_register_device;
>   
>   	ret = devm_request_threaded_irq(&spi->dev,
>   					spi->irq,
> @@ -772,14 +767,16 @@ static int mrf24j40_probe(struct spi_device *spi)
>   
>   	if (ret) {
>   		dev_err(printdev(devrec), "Unable to get IRQ");
> -		goto err_irq;
> +		goto err_register_device;
>   	}
>   
> +	dev_dbg(printdev(devrec), "registered mrf24j40\n");
> +	ret = ieee802154_register_hw(devrec->hw);
> +	if (ret)
> +		goto err_register_device;
> +
>   	return 0;
>   
> -err_irq:
> -err_hw_init:
> -	ieee802154_unregister_hw(devrec->hw);
>   err_register_device:
>   	ieee802154_free_hw(devrec->hw);
>   err_ret:

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 04/21] mrf24j40: remove spi settings overwrite

[Alexander Aring]

This patch removes spi settings while mrf24j40 probing. These settings
cannot be overwrite while device probing where spi controller should be
already configured. These settings need to be setup by device tree or
platform data.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index de63cba..d16bef3 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -746,10 +746,6 @@ static int mrf24j40_probe(struct spi_device *spi)
 	if (!devrec->buf)
 		goto err_register_device;
 
-	spi->mode = SPI_MODE_0; /* TODO: Is this appropriate for right here? */
-	if (spi->max_speed_hz > MAX_SPI_SPEED_HZ)
-		spi->max_speed_hz = MAX_SPI_SPEED_HZ;
-
 	mutex_init(&devrec->buffer_mutex);
 	init_completion(&devrec->tx_complete);
 
-- 
2.5.0

Re: [RFC bluetooth-next 04/21] mrf24j40: remove spi settings overwrite

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch removes spi settings while mrf24j40 probing. These settings
> cannot be overwrite while device probing where spi controller should be
> already configured. These settings need to be setup by device tree or
> platform data.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index de63cba..d16bef3 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -746,10 +746,6 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	if (!devrec->buf)
>   		goto err_register_device;
>   
> -	spi->mode = SPI_MODE_0; /* TODO: Is this appropriate for right here? */
> -	if (spi->max_speed_hz > MAX_SPI_SPEED_HZ)
> -		spi->max_speed_hz = MAX_SPI_SPEED_HZ;
> -
>   	mutex_init(&devrec->buffer_mutex);
>   	init_completion(&devrec->tx_complete);
>   

So far I only have been setting the SPI speed but never the mode via 
devicetree. Digging for it I found that we can set the various modes 
easily via devicetree.
Documentation/devicetree/bindings/spi/spi-bus.txt

- spi-cpol        - (optional) Empty property indicating device requires
     		inverse clock polarity (CPOL) mode
- spi-cpha        - (optional) Empty property indicating device requires
     		shifted clock phase (CPHA) mode
- spi-cs-high     - (optional) Empty property indicating device requires
     		chip select active high
- spi-3wire       - (optional) Empty property indicating device requires
     		    3-wire mode.
- spi-lsb-first   - (optional) Empty property indicating device requires
		LSB first mode.


Platform data could always do that anyway so we are good here.

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

Re: [RFC bluetooth-next 04/21] mrf24j40: remove spi settings overwrite

[Alexander Aring]

Hi,

On Thu, Aug 27, 2015 at 03:14:31PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> On 13/08/15 14:22, Alexander Aring wrote:
> >This patch removes spi settings while mrf24j40 probing. These settings
> >cannot be overwrite while device probing where spi controller should be
> >already configured. These settings need to be setup by device tree or
> >platform data.
> >
> >Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> >---
> >  drivers/net/ieee802154/mrf24j40.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> >diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> >index de63cba..d16bef3 100644
> >--- a/drivers/net/ieee802154/mrf24j40.c
> >+++ b/drivers/net/ieee802154/mrf24j40.c
> >@@ -746,10 +746,6 @@ static int mrf24j40_probe(struct spi_device *spi)
> >  	if (!devrec->buf)
> >  		goto err_register_device;
> >-	spi->mode = SPI_MODE_0; /* TODO: Is this appropriate for right here? */
> >-	if (spi->max_speed_hz > MAX_SPI_SPEED_HZ)
> >-		spi->max_speed_hz = MAX_SPI_SPEED_HZ;
> >-
> >  	mutex_init(&devrec->buffer_mutex);
> >  	init_completion(&devrec->tx_complete);
> 
> So far I only have been setting the SPI speed but never the mode via
> devicetree. Digging for it I found that we can set the various modes easily
> via devicetree.
> Documentation/devicetree/bindings/spi/spi-bus.txt
> 
> - spi-cpol        - (optional) Empty property indicating device requires
>     		inverse clock polarity (CPOL) mode
> - spi-cpha        - (optional) Empty property indicating device requires
>     		shifted clock phase (CPHA) mode
> - spi-cs-high     - (optional) Empty property indicating device requires
>     		chip select active high
> - spi-3wire       - (optional) Empty property indicating device requires
>     		    3-wire mode.
> - spi-lsb-first   - (optional) Empty property indicating device requires
> 		LSB first mode.
> 
> 
> Platform data could always do that anyway so we are good here.
> 

I think, that changing the attribute only, will not change the spi bus
controller. This setup was before any spi device will be probed.

One reason is here because we already access the spi device inside
probing of mrf24j40 -> spi need to be setup correctly. It can't be
changed again in any device probing, or we need to call some other
function that we can tell the spi controller these spi attributes
was changed, if possible.

For the SPI_MODE think, it's mostly SPI_MODE_0 which sets non of these
flags. They call it "original MicroWire".

#define SPI_MODE_0      (0|0)                   /* (original MicroWire) */

What we could do is to make some
"WARN_ON(spi->max_speed_hz > MAX_SPI_SPEED_HZ, "foobar\n");

if the SPI clock is above maximum.

- Alex

Re: [RFC bluetooth-next 04/21] mrf24j40: remove spi settings overwrite

[Stefan Schmidt]

Hello.

On 28/08/15 09:50, Alexander Aring wrote:
> Hi,
>
> On Thu, Aug 27, 2015 at 03:14:31PM +0200, Stefan Schmidt wrote:
>> Hello.
>>
>> On 13/08/15 14:22, Alexander Aring wrote:
>>> This patch removes spi settings while mrf24j40 probing. These settings
>>> cannot be overwrite while device probing where spi controller should be
>>> already configured. These settings need to be setup by device tree or
>>> platform data.
>>>
>>> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
>>> ---
>>>   drivers/net/ieee802154/mrf24j40.c | 4 ----
>>>   1 file changed, 4 deletions(-)
>>>
>>> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
>>> index de63cba..d16bef3 100644
>>> --- a/drivers/net/ieee802154/mrf24j40.c
>>> +++ b/drivers/net/ieee802154/mrf24j40.c
>>> @@ -746,10 +746,6 @@ static int mrf24j40_probe(struct spi_device *spi)
>>>   	if (!devrec->buf)
>>>   		goto err_register_device;
>>> -	spi->mode = SPI_MODE_0; /* TODO: Is this appropriate for right here? */
>>> -	if (spi->max_speed_hz > MAX_SPI_SPEED_HZ)
>>> -		spi->max_speed_hz = MAX_SPI_SPEED_HZ;
>>> -
>>>   	mutex_init(&devrec->buffer_mutex);
>>>   	init_completion(&devrec->tx_complete);
>> So far I only have been setting the SPI speed but never the mode via
>> devicetree. Digging for it I found that we can set the various modes easily
>> via devicetree.
>> Documentation/devicetree/bindings/spi/spi-bus.txt
>>
>> - spi-cpol        - (optional) Empty property indicating device requires
>>      		inverse clock polarity (CPOL) mode
>> - spi-cpha        - (optional) Empty property indicating device requires
>>      		shifted clock phase (CPHA) mode
>> - spi-cs-high     - (optional) Empty property indicating device requires
>>      		chip select active high
>> - spi-3wire       - (optional) Empty property indicating device requires
>>      		    3-wire mode.
>> - spi-lsb-first   - (optional) Empty property indicating device requires
>> 		LSB first mode.
>>
>>
>> Platform data could always do that anyway so we are good here.
>>
> I think, that changing the attribute only, will not change the spi bus
> controller. This setup was before any spi device will be probed.
>
> One reason is here because we already access the spi device inside
> probing of mrf24j40 -> spi need to be setup correctly. It can't be
> changed again in any device probing, or we need to call some other
> function that we can tell the spi controller these spi attributes
> was changed, if possible.
>
> For the SPI_MODE think, it's mostly SPI_MODE_0 which sets non of these
> flags. They call it "original MicroWire".
>
> #define SPI_MODE_0      (0|0)                   /* (original MicroWire) */

Yeah, I can think we can safely remove the SPI_MODE part here.
> What we could do is to make some
> "WARN_ON(spi->max_speed_hz > MAX_SPI_SPEED_HZ, "foobar\n");
>
> if the SPI clock is above maximum.

That sounds like a good idea to me. While it is mentioned in the 
devicetree binding we are not enforcing it anymore. A warning would be good.

regards
Stefan Schmidt
> - Alex
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC bluetooth-next 05/21] mrf24j40: add device-tree support

[Alexander Aring]

This patch adds devicetree support to mrf24j40 with proper devicetree
compatible strings.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 .../devicetree/bindings/net/ieee802154/mrf24j40.txt  | 20 ++++++++++++++++++++
 MAINTAINERS                                          |  1 +
 drivers/net/ieee802154/mrf24j40.c                    |  9 +++++++++
 3 files changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt

diff --git a/Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt b/Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
new file mode 100644
index 0000000..a4ed2ef
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
@@ -0,0 +1,20 @@
+* MRF24J40 IEEE 802.15.4 *
+
+Required properties:
+  - compatible:		should be "microchip,mrf24j40", "microchip,mrf24j40ma",
+			or "microchip,mrf24j40mc" depends on your transceiver
+			board
+  - spi-max-frequency:	maximal bus speed, should be set something under or equal
+			10000000
+  - reg:		the chipselect index
+  - interrupts:		the interrupt generated by the device.
+
+Example:
+
+	mrf24j40ma@0 {
+		compatible = "microchip,mrf24j40ma";
+		spi-max-frequency = <8500000>;
+		reg = <0>;
+		interrupts = <19 8>;
+		interrupt-parent = <&gpio3>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 5baa91c..0af4165 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6816,6 +6816,7 @@ M:	Alan Ott <alan@signal11.us>
 L:	linux-wpan@vger.kernel.org
 S:	Maintained
 F:	drivers/net/ieee802154/mrf24j40.c
+F:	Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
 
 MSI LAPTOP SUPPORT
 M:	"Lee, Chun-Yi" <jlee@suse.com>
diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index d16bef3..7df80d8 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -793,6 +793,14 @@ static int mrf24j40_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id mrf24j40_of_match[] = {
+	{ .compatible = "microchip,mrf24j40", .data = (void *)MRF24J40 },
+	{ .compatible = "microchip,mrf24j40ma", .data = (void *)MRF24J40MA },
+	{ .compatible = "microchip,mrf24j40mc", .data = (void *)MRF24J40MC },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, mrf24j40_of_match);
+
 static const struct spi_device_id mrf24j40_ids[] = {
 	{ "mrf24j40", MRF24J40 },
 	{ "mrf24j40ma", MRF24J40MA },
@@ -803,6 +811,7 @@ MODULE_DEVICE_TABLE(spi, mrf24j40_ids);
 
 static struct spi_driver mrf24j40_driver = {
 	.driver = {
+		.of_match_table = of_match_ptr(mrf24j40_of_match),
 		.name = "mrf24j40",
 		.owner = THIS_MODULE,
 	},
-- 
2.5.0

Re: [RFC bluetooth-next 05/21] mrf24j40: add device-tree support

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch adds devicetree support to mrf24j40 with proper devicetree
> compatible strings.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   .../devicetree/bindings/net/ieee802154/mrf24j40.txt  | 20 ++++++++++++++++++++
>   MAINTAINERS                                          |  1 +
>   drivers/net/ieee802154/mrf24j40.c                    |  9 +++++++++
>   3 files changed, 30 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
>
> diff --git a/Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt b/Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
> new file mode 100644
> index 0000000..a4ed2ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
> @@ -0,0 +1,20 @@
> +* MRF24J40 IEEE 802.15.4 *
> +
> +Required properties:
> +  - compatible:		should be "microchip,mrf24j40", "microchip,mrf24j40ma",
> +			or "microchip,mrf24j40mc" depends on your transceiver
> +			board
> +  - spi-max-frequency:	maximal bus speed, should be set something under or equal
> +			10000000
> +  - reg:		the chipselect index
> +  - interrupts:		the interrupt generated by the device.
> +
> +Example:
> +
> +	mrf24j40ma@0 {
> +		compatible = "microchip,mrf24j40ma";
> +		spi-max-frequency = <8500000>;
> +		reg = <0>;
> +		interrupts = <19 8>;
> +		interrupt-parent = <&gpio3>;
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5baa91c..0af4165 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6816,6 +6816,7 @@ M:	Alan Ott <alan@signal11.us>
>   L:	linux-wpan@vger.kernel.org
>   S:	Maintained
>   F:	drivers/net/ieee802154/mrf24j40.c
> +F:	Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
>   
>   MSI LAPTOP SUPPORT
>   M:	"Lee, Chun-Yi" <jlee@suse.com>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index d16bef3..7df80d8 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -793,6 +793,14 @@ static int mrf24j40_remove(struct spi_device *spi)
>   	return 0;
>   }
>   
> +static const struct of_device_id mrf24j40_of_match[] = {
> +	{ .compatible = "microchip,mrf24j40", .data = (void *)MRF24J40 },
> +	{ .compatible = "microchip,mrf24j40ma", .data = (void *)MRF24J40MA },
> +	{ .compatible = "microchip,mrf24j40mc", .data = (void *)MRF24J40MC },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, mrf24j40_of_match);
> +
>   static const struct spi_device_id mrf24j40_ids[] = {
>   	{ "mrf24j40", MRF24J40 },
>   	{ "mrf24j40ma", MRF24J40MA },
> @@ -803,6 +811,7 @@ MODULE_DEVICE_TABLE(spi, mrf24j40_ids);
>   
>   static struct spi_driver mrf24j40_driver = {
>   	.driver = {
> +		.of_match_table = of_match_ptr(mrf24j40_of_match),
>   		.name = "mrf24j40",
>   		.owner = THIS_MODULE,
>   	},

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 06/21] mrf24j40: add default channel setting

[Alexander Aring]

Per default mrf24j40 has the channel 11 after reset. This patch adds the
right phy default value for the channel setting.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 7df80d8..4051310 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -720,6 +720,11 @@ err_ret:
 	return ret;
 }
 
+static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
+{
+	devrec->hw->phy->current_channel = 11;
+}
+
 static int mrf24j40_probe(struct spi_device *spi)
 {
 	int ret = -ENOMEM;
@@ -753,6 +758,8 @@ static int mrf24j40_probe(struct spi_device *spi)
 	if (ret)
 		goto err_register_device;
 
+	mrf24j40_phy_setup(devrec);
+
 	ret = devm_request_threaded_irq(&spi->dev,
 					spi->irq,
 					NULL,
-- 
2.5.0

Re: [RFC bluetooth-next 06/21] mrf24j40: add default channel setting

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> Per default mrf24j40 has the channel 11 after reset. This patch adds the
> right phy default value for the channel setting.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 7df80d8..4051310 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -720,6 +720,11 @@ err_ret:
>   	return ret;
>   }
>   
> +static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
> +{
> +	devrec->hw->phy->current_channel = 11;
> +}
> +

I was about to write here that a new function just for this might be a 
bit to much but looking down further the patchset I can see that you are 
adding more and more phy related bits here so its fine to introduce it here.
>   static int mrf24j40_probe(struct spi_device *spi)
>   {
>   	int ret = -ENOMEM;
> @@ -753,6 +758,8 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	if (ret)
>   		goto err_register_device;
>   
> +	mrf24j40_phy_setup(devrec);
> +
>   	ret = devm_request_threaded_irq(&spi->dev,
>   					spi->irq,
>   					NULL,

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 07/21] mrf24j40: add random extended addr generation

[Alexander Aring]

The mrf24j40 has no source to get a permanent extended address. This
patch will add a random generated permanent extended address source.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 4051310..e92b29a 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -722,6 +722,7 @@ err_ret:
 
 static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
 {
+	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
 	devrec->hw->phy->current_channel = 11;
 }
 
-- 
2.5.0

Re: [RFC bluetooth-next 07/21] mrf24j40: add random extended addr generation

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> The mrf24j40 has no source to get a permanent extended address. This
> patch will add a random generated permanent extended address source.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 4051310..e92b29a 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -722,6 +722,7 @@ err_ret:
>   
>   static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>   {
> +	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
>   	devrec->hw->phy->current_channel = 11;
>   }
>   

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 08/21] mrf24j40: add more register defines

[Alexander Aring]

For supporting regmap, this patch will add more register defines to
prepare a full register dump by regmap debugfs.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 68 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index e92b29a..883f2c9 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -29,21 +29,56 @@
 #define REG_SADRL	0x03  /* Short address (low) */
 #define REG_SADRH	0x04  /* Short address (high) */
 #define REG_EADR0	0x05  /* Long address (low) (high is EADR7) */
+#define REG_EADR1	0x06
+#define REG_EADR2	0x07
+#define REG_EADR3	0x08
+#define REG_EADR4	0x09
+#define REG_EADR5	0x0A
+#define REG_EADR6	0x0B
+#define REG_EADR7	0x0C
+#define REG_RXFLUSH	0x0D
+#define REG_ORDER	0x10
 #define REG_TXMCR	0x11  /* Transmit MAC control */
+#define REG_ACKTMOUT	0x12
+#define REG_ESLOTG1	0x13
+#define REG_SYMTICKL	0x14
+#define REG_SYMTICKH	0x15
 #define REG_PACON0	0x16  /* Power Amplifier Control */
 #define REG_PACON1	0x17  /* Power Amplifier Control */
 #define REG_PACON2	0x18  /* Power Amplifier Control */
+#define REG_TXBCON0	0x1A
 #define REG_TXNCON	0x1B  /* Transmit Normal FIFO Control */
+#define REG_TXG1CON	0x1C
+#define REG_TXG2CON	0x1D
+#define REG_ESLOTG23	0x1E
+#define REG_ESLOTG45	0x1F
+#define REG_ESLOTG67	0x20
+#define REG_TXPEND	0x21
+#define REG_WAKECON	0x22
+#define REG_FROMOFFSET	0x23
 #define REG_TXSTAT	0x24  /* TX MAC Status Register */
+#define REG_TXBCON1	0x25
+#define REG_GATECLK	0x26
+#define REG_TXTIME	0x27
+#define REG_HSYMTMRL	0x28
+#define REG_HSYMTMRH	0x29
 #define REG_SOFTRST	0x2A  /* Soft Reset */
+#define REG_SECCON0	0x2C
+#define REG_SECCON1	0x2D
 #define REG_TXSTBL	0x2E  /* TX Stabilization */
+#define REG_RXSR	0x30
 #define REG_INTSTAT	0x31  /* Interrupt Status */
 #define REG_INTCON	0x32  /* Interrupt Control */
 #define REG_GPIO	0x33  /* GPIO */
 #define REG_TRISGPIO	0x34  /* GPIO direction */
+#define REG_SLPACK	0x35
 #define REG_RFCTL	0x36  /* RF Control Mode Register */
+#define REG_SECCR2	0x37
+#define REG_BBREG0	0x38
 #define REG_BBREG1	0x39  /* Baseband Registers */
 #define REG_BBREG2	0x3A  /* */
+#define REG_BBREG3	0x3B
+#define REG_BBREG4	0x3C
 #define REG_BBREG6	0x3E  /* */
 #define REG_CCAEDTH	0x3F  /* Energy Detection Threshold */
 
@@ -56,12 +91,45 @@
 #define REG_RFCON6	0x206
 #define REG_RFCON7	0x207
 #define REG_RFCON8	0x208
+#define REG_SLPCAL0	0x209
+#define REG_SLPCAL1	0x20A
+#define REG_SLPCAL2	0x20B
+#define REG_RFSTATE	0x20F
 #define REG_RSSI	0x210
 #define REG_SLPCON0	0x211  /* Sleep Clock Control Registers */
 #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_REMCNTL	0x224
+#define REG_REMCNTH	0x225
+#define REG_MAINCNT0	0x226
+#define REG_MAINCNT1	0x227
+#define REG_MAINCNT2	0x228
+#define REG_MAINCNT3	0x229
 #define REG_TESTMODE	0x22F  /* Test mode */
+#define REG_ASSOEAR0	0x230
+#define REG_ASSOEAR1	0x231
+#define REG_ASSOEAR2	0x232
+#define REG_ASSOEAR3	0x233
+#define REG_ASSOEAR4	0x234
+#define REG_ASSOEAR5	0x235
+#define REG_ASSOEAR6	0x236
+#define REG_ASSOEAR7	0x237
+#define REG_ASSOSAR0	0x238
+#define REG_ASSOSAR1	0x239
+#define REG_UNONCE0	0x240
+#define REG_UNONCE1	0x241
+#define REG_UNONCE2	0x242
+#define REG_UNONCE3	0x243
+#define REG_UNONCE4	0x244
+#define REG_UNONCE5	0x245
+#define REG_UNONCE6	0x246
+#define REG_UNONCE7	0x247
+#define REG_UNONCE8	0x248
+#define REG_UNONCE9	0x249
+#define REG_UNONCE10	0x24A
+#define REG_UNONCE11	0x24B
+#define REG_UNONCE12	0x24C
 #define REG_RX_FIFO	0x300  /* Receive FIFO */
 
 /* Device configuration: Only channels 11-26 on page 0 are supported. */
-- 
2.5.0

Re: [RFC bluetooth-next 08/21] mrf24j40: add more register defines

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> For supporting regmap, this patch will add more register defines to
> prepare a full register dump by regmap debugfs.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 68 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 68 insertions(+)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index e92b29a..883f2c9 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -29,21 +29,56 @@
>   #define REG_SADRL	0x03  /* Short address (low) */
>   #define REG_SADRH	0x04  /* Short address (high) */
>   #define REG_EADR0	0x05  /* Long address (low) (high is EADR7) */
> +#define REG_EADR1	0x06
> +#define REG_EADR2	0x07
> +#define REG_EADR3	0x08
> +#define REG_EADR4	0x09
> +#define REG_EADR5	0x0A
> +#define REG_EADR6	0x0B
> +#define REG_EADR7	0x0C
> +#define REG_RXFLUSH	0x0D
> +#define REG_ORDER	0x10
>   #define REG_TXMCR	0x11  /* Transmit MAC control */
> +#define REG_ACKTMOUT	0x12
> +#define REG_ESLOTG1	0x13
> +#define REG_SYMTICKL	0x14
> +#define REG_SYMTICKH	0x15
>   #define REG_PACON0	0x16  /* Power Amplifier Control */
>   #define REG_PACON1	0x17  /* Power Amplifier Control */
>   #define REG_PACON2	0x18  /* Power Amplifier Control */
> +#define REG_TXBCON0	0x1A
>   #define REG_TXNCON	0x1B  /* Transmit Normal FIFO Control */
> +#define REG_TXG1CON	0x1C
> +#define REG_TXG2CON	0x1D
> +#define REG_ESLOTG23	0x1E
> +#define REG_ESLOTG45	0x1F
> +#define REG_ESLOTG67	0x20
> +#define REG_TXPEND	0x21
> +#define REG_WAKECON	0x22
> +#define REG_FROMOFFSET	0x23
>   #define REG_TXSTAT	0x24  /* TX MAC Status Register */
> +#define REG_TXBCON1	0x25
> +#define REG_GATECLK	0x26
> +#define REG_TXTIME	0x27
> +#define REG_HSYMTMRL	0x28
> +#define REG_HSYMTMRH	0x29
>   #define REG_SOFTRST	0x2A  /* Soft Reset */
> +#define REG_SECCON0	0x2C
> +#define REG_SECCON1	0x2D
>   #define REG_TXSTBL	0x2E  /* TX Stabilization */
> +#define REG_RXSR	0x30
>   #define REG_INTSTAT	0x31  /* Interrupt Status */
>   #define REG_INTCON	0x32  /* Interrupt Control */
>   #define REG_GPIO	0x33  /* GPIO */
>   #define REG_TRISGPIO	0x34  /* GPIO direction */
> +#define REG_SLPACK	0x35
>   #define REG_RFCTL	0x36  /* RF Control Mode Register */
> +#define REG_SECCR2	0x37
> +#define REG_BBREG0	0x38
>   #define REG_BBREG1	0x39  /* Baseband Registers */
>   #define REG_BBREG2	0x3A  /* */
> +#define REG_BBREG3	0x3B
> +#define REG_BBREG4	0x3C
>   #define REG_BBREG6	0x3E  /* */
>   #define REG_CCAEDTH	0x3F  /* Energy Detection Threshold */
>   
> @@ -56,12 +91,45 @@
>   #define REG_RFCON6	0x206
>   #define REG_RFCON7	0x207
>   #define REG_RFCON8	0x208
> +#define REG_SLPCAL0	0x209
> +#define REG_SLPCAL1	0x20A
> +#define REG_SLPCAL2	0x20B
> +#define REG_RFSTATE	0x20F
>   #define REG_RSSI	0x210
>   #define REG_SLPCON0	0x211  /* Sleep Clock Control Registers */
>   #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_REMCNTL	0x224
> +#define REG_REMCNTH	0x225
> +#define REG_MAINCNT0	0x226
> +#define REG_MAINCNT1	0x227
> +#define REG_MAINCNT2	0x228
> +#define REG_MAINCNT3	0x229
>   #define REG_TESTMODE	0x22F  /* Test mode */
> +#define REG_ASSOEAR0	0x230
> +#define REG_ASSOEAR1	0x231
> +#define REG_ASSOEAR2	0x232
> +#define REG_ASSOEAR3	0x233
> +#define REG_ASSOEAR4	0x234
> +#define REG_ASSOEAR5	0x235
> +#define REG_ASSOEAR6	0x236
> +#define REG_ASSOEAR7	0x237
> +#define REG_ASSOSAR0	0x238
> +#define REG_ASSOSAR1	0x239
> +#define REG_UNONCE0	0x240
> +#define REG_UNONCE1	0x241
> +#define REG_UNONCE2	0x242
> +#define REG_UNONCE3	0x243
> +#define REG_UNONCE4	0x244
> +#define REG_UNONCE5	0x245
> +#define REG_UNONCE6	0x246
> +#define REG_UNONCE7	0x247
> +#define REG_UNONCE8	0x248
> +#define REG_UNONCE9	0x249
> +#define REG_UNONCE10	0x24A
> +#define REG_UNONCE11	0x24B
> +#define REG_UNONCE12	0x24C
>   #define REG_RX_FIFO	0x300  /* Receive FIFO */
>   
>   /* Device configuration: Only channels 11-26 on page 0 are supported. */

This together with the regmap support further down was a big help when 
trying to debug what might be the difference between a working setup 
from Alex and mine. Dumping the regs, checking for difference and 
looking up in the docs what they mean was helpful.

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 09/21] mrf24j40: add regmap support

[Alexander Aring]

This patch introduce regmap support for short and long address space of
mrf24j40. It's only possible to use regmap_read/write/update_bits for
long address range. This is because I added lowlevel bus operation
because the write operation need to set the 12th bit to mark a register
write, but regmap only supports to set bits for register write access in
the first byte. We use other regmap register functions than
read/write/update_bits, so this should be fine.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/Kconfig    |   1 +
 drivers/net/ieee802154/mrf24j40.c | 312 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 313 insertions(+)

diff --git a/drivers/net/ieee802154/Kconfig b/drivers/net/ieee802154/Kconfig
index 1dd5ab8..6a465b2 100644
--- a/drivers/net/ieee802154/Kconfig
+++ b/drivers/net/ieee802154/Kconfig
@@ -36,6 +36,7 @@ config IEEE802154_MRF24J40
 	tristate "Microchip MRF24J40 transceiver driver"
 	depends on IEEE802154_DRIVERS && MAC802154
 	depends on SPI
+	select REGMAP_SPI
 	---help---
 	  Say Y here to enable the MRF24J20 SPI 802.15.4 wireless
 	  controller.
diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 883f2c9..6578f68 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -18,6 +18,7 @@
 #include <linux/spi/spi.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/ieee802154.h>
 #include <net/cfg802154.h>
 #include <net/mac802154.h>
@@ -149,11 +150,22 @@ struct mrf24j40 {
 	struct spi_device *spi;
 	struct ieee802154_hw *hw;
 
+	struct regmap *regmap_short;
+	struct regmap *regmap_long;
 	struct mutex buffer_mutex; /* only used to protect buf */
 	struct completion tx_complete;
 	u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */
 };
 
+/* regmap information for short address register access */
+#define MRF24J40_SHORT_WRITE	0x01
+#define MRF24J40_SHORT_READ	0x00
+#define MRF24J40_SHORT_NUMREGS	0x3F
+
+/* regmap information for long address register access */
+#define MRF24J40_LONG_ACCESS	0x80
+#define MRF24J40_LONG_NUMREGS	0x38F
+
 /* Read/Write SPI Commands for Short and Long Address registers. */
 #define MRF24J40_READSHORT(reg) ((reg) << 1)
 #define MRF24J40_WRITESHORT(reg) ((reg) << 1 | 1)
@@ -165,6 +177,287 @@ struct mrf24j40 {
 
 #define printdev(X) (&X->spi->dev)
 
+static bool
+mrf24j40_short_reg_writeable(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case REG_RXMCR:
+	case REG_PANIDL:
+	case REG_PANIDH:
+	case REG_SADRL:
+	case REG_SADRH:
+	case REG_EADR0:
+	case REG_EADR1:
+	case REG_EADR2:
+	case REG_EADR3:
+	case REG_EADR4:
+	case REG_EADR5:
+	case REG_EADR6:
+	case REG_EADR7:
+	case REG_RXFLUSH:
+	case REG_ORDER:
+	case REG_TXMCR:
+	case REG_ACKTMOUT:
+	case REG_ESLOTG1:
+	case REG_SYMTICKL:
+	case REG_SYMTICKH:
+	case REG_PACON0:
+	case REG_PACON1:
+	case REG_PACON2:
+	case REG_TXBCON0:
+	case REG_TXNCON:
+	case REG_TXG1CON:
+	case REG_TXG2CON:
+	case REG_ESLOTG23:
+	case REG_ESLOTG45:
+	case REG_ESLOTG67:
+	case REG_TXPEND:
+	case REG_WAKECON:
+	case REG_FROMOFFSET:
+	case REG_TXBCON1:
+	case REG_GATECLK:
+	case REG_TXTIME:
+	case REG_HSYMTMRL:
+	case REG_HSYMTMRH:
+	case REG_SOFTRST:
+	case REG_SECCON0:
+	case REG_SECCON1:
+	case REG_TXSTBL:
+	case REG_RXSR:
+	case REG_INTCON:
+	case REG_TRISGPIO:
+	case REG_GPIO:
+	case REG_RFCTL:
+	case REG_SLPACK:
+	case REG_BBREG0:
+	case REG_BBREG1:
+	case REG_BBREG2:
+	case REG_BBREG3:
+	case REG_BBREG4:
+	case REG_BBREG6:
+	case REG_CCAEDTH:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool
+mrf24j40_short_reg_readable(struct device *dev, unsigned int reg)
+{
+	bool rc;
+
+	/* all writeable are also readable */
+	rc = mrf24j40_short_reg_writeable(dev, reg);
+	if (rc)
+		return rc;
+
+	/* readonly regs */
+	switch (reg) {
+	case REG_TXSTAT:
+	case REG_INTSTAT:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool
+mrf24j40_short_reg_volatile(struct device *dev, unsigned int reg)
+{
+	/* can be changed during runtime */
+	switch (reg) {
+	case REG_TXSTAT:
+	case REG_INTSTAT:
+	case REG_RXFLUSH:
+	case REG_TXNCON:
+	case REG_SOFTRST:
+	case REG_RFCTL:
+	case REG_TXBCON0:
+	case REG_TXG1CON:
+	case REG_TXG2CON:
+	case REG_TXBCON1:
+	case REG_SECCON0:
+	case REG_RXSR:
+	case REG_SLPACK:
+	case REG_SECCR2:
+	case REG_BBREG6:
+	/* use them in spi_async and regmap so it's volatile */
+	case REG_BBREG1:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool
+mrf24j40_short_reg_precious(struct device *dev, unsigned int reg)
+{
+	/* don't clear irq line on read */
+	switch (reg) {
+	case REG_INTSTAT:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config mrf24j40_short_regmap = {
+	.name = "mrf24j40_short",
+	.reg_bits = 7,
+	.val_bits = 8,
+	.pad_bits = 1,
+	.write_flag_mask = MRF24J40_SHORT_WRITE,
+	.read_flag_mask = MRF24J40_SHORT_READ,
+	.cache_type = REGCACHE_RBTREE,
+	.max_register = MRF24J40_SHORT_NUMREGS,
+	.writeable_reg = mrf24j40_short_reg_writeable,
+	.readable_reg = mrf24j40_short_reg_readable,
+	.volatile_reg = mrf24j40_short_reg_volatile,
+	.precious_reg = mrf24j40_short_reg_precious,
+};
+
+static bool
+mrf24j40_long_reg_writeable(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case REG_RFCON0:
+	case REG_RFCON1:
+	case REG_RFCON2:
+	case REG_RFCON3:
+	case REG_RFCON5:
+	case REG_RFCON6:
+	case REG_RFCON7:
+	case REG_RFCON8:
+	case REG_SLPCAL2:
+	case REG_SLPCON0:
+	case REG_SLPCON1:
+	case REG_WAKETIMEL:
+	case REG_WAKETIMEH:
+	case REG_REMCNTL:
+	case REG_REMCNTH:
+	case REG_MAINCNT0:
+	case REG_MAINCNT1:
+	case REG_MAINCNT2:
+	case REG_MAINCNT3:
+	case REG_TESTMODE:
+	case REG_ASSOEAR0:
+	case REG_ASSOEAR1:
+	case REG_ASSOEAR2:
+	case REG_ASSOEAR3:
+	case REG_ASSOEAR4:
+	case REG_ASSOEAR5:
+	case REG_ASSOEAR6:
+	case REG_ASSOEAR7:
+	case REG_ASSOSAR0:
+	case REG_ASSOSAR1:
+	case REG_UNONCE0:
+	case REG_UNONCE1:
+	case REG_UNONCE2:
+	case REG_UNONCE3:
+	case REG_UNONCE4:
+	case REG_UNONCE5:
+	case REG_UNONCE6:
+	case REG_UNONCE7:
+	case REG_UNONCE8:
+	case REG_UNONCE9:
+	case REG_UNONCE10:
+	case REG_UNONCE11:
+	case REG_UNONCE12:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool
+mrf24j40_long_reg_readable(struct device *dev, unsigned int reg)
+{
+	bool rc;
+
+	/* all writeable are also readable */
+	rc = mrf24j40_long_reg_writeable(dev, reg);
+	if (rc)
+		return rc;
+
+	/* readonly regs */
+	switch (reg) {
+	case REG_SLPCAL0:
+	case REG_SLPCAL1:
+	case REG_RFSTATE:
+	case REG_RSSI:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool
+mrf24j40_long_reg_volatile(struct device *dev, unsigned int reg)
+{
+	/* can be changed during runtime */
+	switch (reg) {
+	case REG_SLPCAL0:
+	case REG_SLPCAL1:
+	case REG_SLPCAL2:
+	case REG_RFSTATE:
+	case REG_RSSI:
+	case REG_MAINCNT3:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config mrf24j40_long_regmap = {
+	.name = "mrf24j40_long",
+	.reg_bits = 11,
+	.val_bits = 8,
+	.pad_bits = 5,
+	.write_flag_mask = MRF24J40_LONG_ACCESS,
+	.read_flag_mask = MRF24J40_LONG_ACCESS,
+	.cache_type = REGCACHE_RBTREE,
+	.max_register = MRF24J40_LONG_NUMREGS,
+	.writeable_reg = mrf24j40_long_reg_writeable,
+	.readable_reg = mrf24j40_long_reg_readable,
+	.volatile_reg = mrf24j40_long_reg_volatile,
+};
+
+static int mrf24j40_long_regmap_write(void *context, const void *data,
+				      size_t count)
+{
+	struct spi_device *spi = context;
+	u8 buf[3];
+
+	if (count > 3)
+		return -EINVAL;
+
+	/* regmap supports read/write mask only in frist byte
+	 * long write access need to set the 12th bit, so we
+	 * make special handling for write.
+	 */
+	memcpy(buf, data, count);
+	buf[1] |= (1 << 4);
+
+	return spi_write(spi, buf, count);
+}
+
+static int
+mrf24j40_long_regmap_read(void *context, const void *reg, size_t reg_size,
+			  void *val, size_t val_size)
+{
+	struct spi_device *spi = context;
+
+	return spi_write_then_read(spi, reg, reg_size, val, val_size);
+}
+
+static const struct regmap_bus mrf24j40_long_regmap_bus = {
+	.write = mrf24j40_long_regmap_write,
+	.read = mrf24j40_long_regmap_read,
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
+};
+
 static int write_short_reg(struct mrf24j40 *devrec, u8 reg, u8 value)
 {
 	int ret;
@@ -816,6 +1109,25 @@ static int mrf24j40_probe(struct spi_device *spi)
 	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
 	devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AFILT;
 
+	devrec->regmap_short = devm_regmap_init_spi(spi,
+						    &mrf24j40_short_regmap);
+	if (IS_ERR(devrec->regmap_short)) {
+		ret = PTR_ERR(devrec->regmap_short);
+		dev_err(&spi->dev, "Failed to allocate short register map: %d\n",
+			ret);
+		goto err_register_device;
+	}
+
+	devrec->regmap_long = devm_regmap_init(&spi->dev,
+					       &mrf24j40_long_regmap_bus,
+					       spi, &mrf24j40_long_regmap);
+	if (IS_ERR(devrec->regmap_long)) {
+		ret = PTR_ERR(devrec->regmap_long);
+		dev_err(&spi->dev, "Failed to allocate long register map: %d\n",
+			ret);
+		goto err_register_device;
+	}
+
 	devrec->buf = devm_kzalloc(&spi->dev, 3, GFP_KERNEL);
 	if (!devrec->buf)
 		goto err_register_device;
-- 
2.5.0

Re: [RFC bluetooth-next 09/21] mrf24j40: add regmap support

[Alexander Aring]

On Thu, Aug 13, 2015 at 02:22:36PM +0200, Alexander Aring wrote:
> This patch introduce regmap support for short and long address space of
> mrf24j40. It's only possible to use regmap_read/write/update_bits for
> long address range. This is because I added lowlevel bus operation
> because the write operation need to set the 12th bit to mark a register
> write, but regmap only supports to set bits for register write access in
> the first byte. We use other regmap register functions than

should be "We don't use other regmap functions than ...."

> read/write/update_bits, so this should be fine.
> 

- Alex

Re: [RFC bluetooth-next 09/21] mrf24j40: add regmap support

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch introduce regmap support for short and long address space of
> mrf24j40. It's only possible to use regmap_read/write/update_bits for
> long address range. This is because I added lowlevel bus operation
> because the write operation need to set the 12th bit to mark a register
> write, but regmap only supports to set bits for register write access in
> the first byte. We use other regmap register functions than
> read/write/update_bits, so this should be fine.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/Kconfig    |   1 +
>   drivers/net/ieee802154/mrf24j40.c | 312 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 313 insertions(+)
>
> diff --git a/drivers/net/ieee802154/Kconfig b/drivers/net/ieee802154/Kconfig
> index 1dd5ab8..6a465b2 100644
> --- a/drivers/net/ieee802154/Kconfig
> +++ b/drivers/net/ieee802154/Kconfig
> @@ -36,6 +36,7 @@ config IEEE802154_MRF24J40
>   	tristate "Microchip MRF24J40 transceiver driver"
>   	depends on IEEE802154_DRIVERS && MAC802154
>   	depends on SPI
> +	select REGMAP_SPI
>   	---help---
>   	  Say Y here to enable the MRF24J20 SPI 802.15.4 wireless
>   	  controller.
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 883f2c9..6578f68 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -18,6 +18,7 @@
>   #include <linux/spi/spi.h>
>   #include <linux/interrupt.h>
>   #include <linux/module.h>
> +#include <linux/regmap.h>
>   #include <linux/ieee802154.h>
>   #include <net/cfg802154.h>
>   #include <net/mac802154.h>
> @@ -149,11 +150,22 @@ struct mrf24j40 {
>   	struct spi_device *spi;
>   	struct ieee802154_hw *hw;
>   
> +	struct regmap *regmap_short;
> +	struct regmap *regmap_long;
>   	struct mutex buffer_mutex; /* only used to protect buf */
>   	struct completion tx_complete;
>   	u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */
>   };
>   
> +/* regmap information for short address register access */
> +#define MRF24J40_SHORT_WRITE	0x01
> +#define MRF24J40_SHORT_READ	0x00
> +#define MRF24J40_SHORT_NUMREGS	0x3F
> +
> +/* regmap information for long address register access */
> +#define MRF24J40_LONG_ACCESS	0x80
> +#define MRF24J40_LONG_NUMREGS	0x38F
> +
>   /* Read/Write SPI Commands for Short and Long Address registers. */
>   #define MRF24J40_READSHORT(reg) ((reg) << 1)
>   #define MRF24J40_WRITESHORT(reg) ((reg) << 1 | 1)
> @@ -165,6 +177,287 @@ struct mrf24j40 {
>   
>   #define printdev(X) (&X->spi->dev)
>   
> +static bool
> +mrf24j40_short_reg_writeable(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case REG_RXMCR:
> +	case REG_PANIDL:
> +	case REG_PANIDH:
> +	case REG_SADRL:
> +	case REG_SADRH:
> +	case REG_EADR0:
> +	case REG_EADR1:
> +	case REG_EADR2:
> +	case REG_EADR3:
> +	case REG_EADR4:
> +	case REG_EADR5:
> +	case REG_EADR6:
> +	case REG_EADR7:
> +	case REG_RXFLUSH:
> +	case REG_ORDER:
> +	case REG_TXMCR:
> +	case REG_ACKTMOUT:
> +	case REG_ESLOTG1:
> +	case REG_SYMTICKL:
> +	case REG_SYMTICKH:
> +	case REG_PACON0:
> +	case REG_PACON1:
> +	case REG_PACON2:
> +	case REG_TXBCON0:
> +	case REG_TXNCON:
> +	case REG_TXG1CON:
> +	case REG_TXG2CON:
> +	case REG_ESLOTG23:
> +	case REG_ESLOTG45:
> +	case REG_ESLOTG67:
> +	case REG_TXPEND:
> +	case REG_WAKECON:
> +	case REG_FROMOFFSET:
> +	case REG_TXBCON1:
> +	case REG_GATECLK:
> +	case REG_TXTIME:
> +	case REG_HSYMTMRL:
> +	case REG_HSYMTMRH:
> +	case REG_SOFTRST:
> +	case REG_SECCON0:
> +	case REG_SECCON1:
> +	case REG_TXSTBL:
> +	case REG_RXSR:
> +	case REG_INTCON:
> +	case REG_TRISGPIO:
> +	case REG_GPIO:
> +	case REG_RFCTL:
> +	case REG_SLPACK:
> +	case REG_BBREG0:
> +	case REG_BBREG1:
> +	case REG_BBREG2:
> +	case REG_BBREG3:
> +	case REG_BBREG4:
> +	case REG_BBREG6:
> +	case REG_CCAEDTH:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool
> +mrf24j40_short_reg_readable(struct device *dev, unsigned int reg)
> +{
> +	bool rc;
> +
> +	/* all writeable are also readable */
> +	rc = mrf24j40_short_reg_writeable(dev, reg);
> +	if (rc)
> +		return rc;
> +
> +	/* readonly regs */
> +	switch (reg) {
> +	case REG_TXSTAT:
> +	case REG_INTSTAT:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool
> +mrf24j40_short_reg_volatile(struct device *dev, unsigned int reg)
> +{
> +	/* can be changed during runtime */
> +	switch (reg) {
> +	case REG_TXSTAT:
> +	case REG_INTSTAT:
> +	case REG_RXFLUSH:
> +	case REG_TXNCON:
> +	case REG_SOFTRST:
> +	case REG_RFCTL:
> +	case REG_TXBCON0:
> +	case REG_TXG1CON:
> +	case REG_TXG2CON:
> +	case REG_TXBCON1:
> +	case REG_SECCON0:
> +	case REG_RXSR:
> +	case REG_SLPACK:
> +	case REG_SECCR2:
> +	case REG_BBREG6:
> +	/* use them in spi_async and regmap so it's volatile */
> +	case REG_BBREG1:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool
> +mrf24j40_short_reg_precious(struct device *dev, unsigned int reg)
> +{
> +	/* don't clear irq line on read */
> +	switch (reg) {
> +	case REG_INTSTAT:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct regmap_config mrf24j40_short_regmap = {
> +	.name = "mrf24j40_short",
> +	.reg_bits = 7,
> +	.val_bits = 8,
> +	.pad_bits = 1,
> +	.write_flag_mask = MRF24J40_SHORT_WRITE,
> +	.read_flag_mask = MRF24J40_SHORT_READ,
> +	.cache_type = REGCACHE_RBTREE,
> +	.max_register = MRF24J40_SHORT_NUMREGS,
> +	.writeable_reg = mrf24j40_short_reg_writeable,
> +	.readable_reg = mrf24j40_short_reg_readable,
> +	.volatile_reg = mrf24j40_short_reg_volatile,
> +	.precious_reg = mrf24j40_short_reg_precious,
> +};
> +
> +static bool
> +mrf24j40_long_reg_writeable(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case REG_RFCON0:
> +	case REG_RFCON1:
> +	case REG_RFCON2:
> +	case REG_RFCON3:
> +	case REG_RFCON5:
> +	case REG_RFCON6:
> +	case REG_RFCON7:
> +	case REG_RFCON8:
> +	case REG_SLPCAL2:
> +	case REG_SLPCON0:
> +	case REG_SLPCON1:
> +	case REG_WAKETIMEL:
> +	case REG_WAKETIMEH:
> +	case REG_REMCNTL:
> +	case REG_REMCNTH:
> +	case REG_MAINCNT0:
> +	case REG_MAINCNT1:
> +	case REG_MAINCNT2:
> +	case REG_MAINCNT3:
> +	case REG_TESTMODE:
> +	case REG_ASSOEAR0:
> +	case REG_ASSOEAR1:
> +	case REG_ASSOEAR2:
> +	case REG_ASSOEAR3:
> +	case REG_ASSOEAR4:
> +	case REG_ASSOEAR5:
> +	case REG_ASSOEAR6:
> +	case REG_ASSOEAR7:
> +	case REG_ASSOSAR0:
> +	case REG_ASSOSAR1:
> +	case REG_UNONCE0:
> +	case REG_UNONCE1:
> +	case REG_UNONCE2:
> +	case REG_UNONCE3:
> +	case REG_UNONCE4:
> +	case REG_UNONCE5:
> +	case REG_UNONCE6:
> +	case REG_UNONCE7:
> +	case REG_UNONCE8:
> +	case REG_UNONCE9:
> +	case REG_UNONCE10:
> +	case REG_UNONCE11:
> +	case REG_UNONCE12:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool
> +mrf24j40_long_reg_readable(struct device *dev, unsigned int reg)
> +{
> +	bool rc;
> +
> +	/* all writeable are also readable */
> +	rc = mrf24j40_long_reg_writeable(dev, reg);
> +	if (rc)
> +		return rc;
> +
> +	/* readonly regs */
> +	switch (reg) {
> +	case REG_SLPCAL0:
> +	case REG_SLPCAL1:
> +	case REG_RFSTATE:
> +	case REG_RSSI:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool
> +mrf24j40_long_reg_volatile(struct device *dev, unsigned int reg)
> +{
> +	/* can be changed during runtime */
> +	switch (reg) {
> +	case REG_SLPCAL0:
> +	case REG_SLPCAL1:
> +	case REG_SLPCAL2:
> +	case REG_RFSTATE:
> +	case REG_RSSI:
> +	case REG_MAINCNT3:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct regmap_config mrf24j40_long_regmap = {
> +	.name = "mrf24j40_long",
> +	.reg_bits = 11,
> +	.val_bits = 8,
> +	.pad_bits = 5,
> +	.write_flag_mask = MRF24J40_LONG_ACCESS,
> +	.read_flag_mask = MRF24J40_LONG_ACCESS,
> +	.cache_type = REGCACHE_RBTREE,
> +	.max_register = MRF24J40_LONG_NUMREGS,
> +	.writeable_reg = mrf24j40_long_reg_writeable,
> +	.readable_reg = mrf24j40_long_reg_readable,
> +	.volatile_reg = mrf24j40_long_reg_volatile,
> +};
> +
> +static int mrf24j40_long_regmap_write(void *context, const void *data,
> +				      size_t count)
> +{
> +	struct spi_device *spi = context;
> +	u8 buf[3];
> +
> +	if (count > 3)
> +		return -EINVAL;
> +
> +	/* regmap supports read/write mask only in frist byte
> +	 * long write access need to set the 12th bit, so we
> +	 * make special handling for write.
> +	 */
> +	memcpy(buf, data, count);
> +	buf[1] |= (1 << 4);
> +
> +	return spi_write(spi, buf, count);
> +}
> +
> +static int
> +mrf24j40_long_regmap_read(void *context, const void *reg, size_t reg_size,
> +			  void *val, size_t val_size)
> +{
> +	struct spi_device *spi = context;
> +
> +	return spi_write_then_read(spi, reg, reg_size, val, val_size);
> +}
> +
> +static const struct regmap_bus mrf24j40_long_regmap_bus = {
> +	.write = mrf24j40_long_regmap_write,
> +	.read = mrf24j40_long_regmap_read,
> +	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
> +	.val_format_endian_default = REGMAP_ENDIAN_BIG,
> +};
> +
>   static int write_short_reg(struct mrf24j40 *devrec, u8 reg, u8 value)
>   {
>   	int ret;
> @@ -816,6 +1109,25 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
>   	devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AFILT;
>   
> +	devrec->regmap_short = devm_regmap_init_spi(spi,
> +						    &mrf24j40_short_regmap);
> +	if (IS_ERR(devrec->regmap_short)) {
> +		ret = PTR_ERR(devrec->regmap_short);
> +		dev_err(&spi->dev, "Failed to allocate short register map: %d\n",
> +			ret);
> +		goto err_register_device;
> +	}
> +
> +	devrec->regmap_long = devm_regmap_init(&spi->dev,
> +					       &mrf24j40_long_regmap_bus,
> +					       spi, &mrf24j40_long_regmap);
> +	if (IS_ERR(devrec->regmap_long)) {
> +		ret = PTR_ERR(devrec->regmap_long);
> +		dev_err(&spi->dev, "Failed to allocate long register map: %d\n",
> +			ret);
> +		goto err_register_device;
> +	}
> +
>   	devrec->buf = devm_kzalloc(&spi->dev, 3, GFP_KERNEL);
>   	if (!devrec->buf)
>   		goto err_register_device;


I never converted a driver to use regmap myself so my review might not 
be to deep here. I have then this code working fine for me though.

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 10/21] mrf24j40: use regmap for register access

[Alexander Aring]

This patch uses the regmap functions for transceiver register settings
where it's possible. This means everything except the hotpaths like
receive/transmit handling.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 147 +++++++++++++-------------------------
 1 file changed, 50 insertions(+), 97 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 6578f68..32de5d6 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -543,35 +543,6 @@ static int read_long_reg(struct mrf24j40 *devrec, u16 reg, u8 *value)
 	return ret;
 }
 
-static int write_long_reg(struct mrf24j40 *devrec, u16 reg, u8 val)
-{
-	int ret;
-	u16 cmd;
-	struct spi_message msg;
-	struct spi_transfer xfer = {
-		.len = 3,
-		.tx_buf = devrec->buf,
-		.rx_buf = devrec->buf,
-	};
-
-	spi_message_init(&msg);
-	spi_message_add_tail(&xfer, &msg);
-
-	cmd = MRF24J40_WRITELONG(reg);
-	mutex_lock(&devrec->buffer_mutex);
-	devrec->buf[0] = cmd >> 8 & 0xff;
-	devrec->buf[1] = cmd & 0xff;
-	devrec->buf[2] = val;
-
-	ret = spi_sync(devrec->spi, &msg);
-	if (ret)
-		dev_err(printdev(devrec),
-			"SPI write Failed for long register 0x%hx\n", reg);
-
-	mutex_unlock(&devrec->buffer_mutex);
-	return ret;
-}
-
 /* This function relies on an undocumented write method. Once a write command
    and address is set, as many bytes of data as desired can be clocked into
    the device. The datasheet only shows setting one byte at a time. */
@@ -755,33 +726,23 @@ static int mrf24j40_ed(struct ieee802154_hw *hw, u8 *level)
 static int mrf24j40_start(struct ieee802154_hw *hw)
 {
 	struct mrf24j40 *devrec = hw->priv;
-	u8 val;
-	int ret;
 
 	dev_dbg(printdev(devrec), "start\n");
 
-	ret = read_short_reg(devrec, REG_INTCON, &val);
-	if (ret)
-		return ret;
-	val &= ~(0x1|0x8); /* Clear TXNIE and RXIE. Enable interrupts */
-	write_short_reg(devrec, REG_INTCON, val);
-
-	return 0;
+	/* Clear TXNIE and RXIE. Enable interrupts */
+	return regmap_update_bits(devrec->regmap_short, REG_INTCON,
+				  0x01 | 0x08, 0x00);
 }
 
 static void mrf24j40_stop(struct ieee802154_hw *hw)
 {
 	struct mrf24j40 *devrec = hw->priv;
-	u8 val;
-	int ret;
 
 	dev_dbg(printdev(devrec), "stop\n");
 
-	ret = read_short_reg(devrec, REG_INTCON, &val);
-	if (ret)
-		return;
-	val |= 0x1|0x8; /* Set TXNIE and RXIE. Disable Interrupts */
-	write_short_reg(devrec, REG_INTCON, val);
+	/* Set TXNIE and RXIE. Disable Interrupts */
+	regmap_update_bits(devrec->regmap_short, REG_INTCON, 0x01 | 0x08,
+			   0x01 | 0x08);
 }
 
 static int mrf24j40_set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
@@ -798,20 +759,20 @@ static int mrf24j40_set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 
 	/* Set Channel TODO */
 	val = (channel-11) << 4 | 0x03;
-	write_long_reg(devrec, REG_RFCON0, val);
+	ret = regmap_update_bits(devrec->regmap_long, REG_RFCON0, 0xf0, val);
+	if (ret)
+		return ret;
 
 	/* RF Reset */
-	ret = read_short_reg(devrec, REG_RFCTL, &val);
+	ret = regmap_update_bits(devrec->regmap_short, REG_RFCTL, 0x04, 0x04);
 	if (ret)
 		return ret;
-	val |= 0x04;
-	write_short_reg(devrec, REG_RFCTL, val);
-	val &= ~0x04;
-	write_short_reg(devrec, REG_RFCTL, val);
 
-	udelay(SET_CHANNEL_DELAY_US); /* per datasheet */
+	ret = regmap_update_bits(devrec->regmap_short, REG_RFCTL, 0x04, 0x00);
+	if (!ret)
+		udelay(SET_CHANNEL_DELAY_US); /* per datasheet */
 
-	return 0;
+	return ret;
 }
 
 static int mrf24j40_filter(struct ieee802154_hw *hw,
@@ -829,8 +790,8 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
 		addrh = le16_to_cpu(filt->short_addr) >> 8 & 0xff;
 		addrl = le16_to_cpu(filt->short_addr) & 0xff;
 
-		write_short_reg(devrec, REG_SADRH, addrh);
-		write_short_reg(devrec, REG_SADRL, addrl);
+		regmap_write(devrec->regmap_short, REG_SADRH, addrh);
+		regmap_write(devrec->regmap_short, REG_SADRL, addrl);
 		dev_dbg(printdev(devrec),
 			"Set short addr to %04hx\n", filt->short_addr);
 	}
@@ -841,7 +802,8 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
 
 		memcpy(addr, &filt->ieee_addr, 8);
 		for (i = 0; i < 8; i++)
-			write_short_reg(devrec, REG_EADR0 + i, addr[i]);
+			regmap_write(devrec->regmap_short, REG_EADR0 + i,
+				     addr[i]);
 
 #ifdef DEBUG
 		pr_debug("Set long addr to: ");
@@ -857,8 +819,8 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
 
 		panidh = le16_to_cpu(filt->pan_id) >> 8 & 0xff;
 		panidl = le16_to_cpu(filt->pan_id) & 0xff;
-		write_short_reg(devrec, REG_PANIDH, panidh);
-		write_short_reg(devrec, REG_PANIDL, panidl);
+		regmap_write(devrec->regmap_short, REG_PANIDH, panidh);
+		regmap_write(devrec->regmap_short, REG_PANIDL, panidl);
 
 		dev_dbg(printdev(devrec), "Set PANID to %04hx\n", filt->pan_id);
 	}
@@ -868,14 +830,14 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
 		u8 val;
 		int ret;
 
-		ret = read_short_reg(devrec, REG_RXMCR, &val);
-		if (ret)
-			return ret;
 		if (filt->pan_coord)
-			val |= 0x8;
+			val = 0x8;
 		else
-			val &= ~0x8;
-		write_short_reg(devrec, REG_RXMCR, val);
+			val = 0x0;
+		ret = regmap_update_bits(devrec->regmap_short, REG_RXMCR, 0x8,
+					 val);
+		if (ret)
+			return ret;
 
 		/* REG_SLOTTED is maintained as default (unslotted/CSMA-CA).
 		 * REG_ORDER is maintained as default (no beacon/superframe).
@@ -976,80 +938,73 @@ out:
 static int mrf24j40_hw_init(struct mrf24j40 *devrec)
 {
 	int ret;
-	u8 val;
 
 	/* Initialize the device.
 		From datasheet section 3.2: Initialization. */
-	ret = write_short_reg(devrec, REG_SOFTRST, 0x07);
+	ret = regmap_write(devrec->regmap_short, REG_SOFTRST, 0x07);
 	if (ret)
 		goto err_ret;
 
-	ret = write_short_reg(devrec, REG_PACON2, 0x98);
+	ret = regmap_write(devrec->regmap_short, REG_PACON2, 0x98);
 	if (ret)
 		goto err_ret;
 
-	ret = write_short_reg(devrec, REG_TXSTBL, 0x95);
+	ret = regmap_write(devrec->regmap_short, REG_TXSTBL, 0x95);
 	if (ret)
 		goto err_ret;
 
-	ret = write_long_reg(devrec, REG_RFCON0, 0x03);
+	ret = regmap_write(devrec->regmap_long, REG_RFCON0, 0x03);
 	if (ret)
 		goto err_ret;
 
-	ret = write_long_reg(devrec, REG_RFCON1, 0x01);
+	ret = regmap_write(devrec->regmap_long, REG_RFCON1, 0x01);
 	if (ret)
 		goto err_ret;
 
-	ret = write_long_reg(devrec, REG_RFCON2, 0x80);
+	ret = regmap_write(devrec->regmap_long, REG_RFCON2, 0x80);
 	if (ret)
 		goto err_ret;
 
-	ret = write_long_reg(devrec, REG_RFCON6, 0x90);
+	ret = regmap_write(devrec->regmap_long, REG_RFCON6, 0x90);
 	if (ret)
 		goto err_ret;
 
-	ret = write_long_reg(devrec, REG_RFCON7, 0x80);
+	ret = regmap_write(devrec->regmap_long, REG_RFCON7, 0x80);
 	if (ret)
 		goto err_ret;
 
-	ret = write_long_reg(devrec, REG_RFCON8, 0x10);
+	ret = regmap_write(devrec->regmap_long, REG_RFCON8, 0x10);
 	if (ret)
 		goto err_ret;
 
-	ret = write_long_reg(devrec, REG_SLPCON1, 0x21);
+	ret = regmap_write(devrec->regmap_long, REG_SLPCON1, 0x21);
 	if (ret)
 		goto err_ret;
 
-	ret = write_short_reg(devrec, REG_BBREG2, 0x80);
+	ret = regmap_write(devrec->regmap_short, REG_BBREG2, 0x80);
 	if (ret)
 		goto err_ret;
 
-	ret = write_short_reg(devrec, REG_CCAEDTH, 0x60);
+	ret = regmap_write(devrec->regmap_short, REG_CCAEDTH, 0x60);
 	if (ret)
 		goto err_ret;
 
-	ret = write_short_reg(devrec, REG_BBREG6, 0x40);
+	ret = regmap_write(devrec->regmap_short, REG_BBREG6, 0x40);
 	if (ret)
 		goto err_ret;
 
-	ret = write_short_reg(devrec, REG_RFCTL, 0x04);
+	ret = regmap_write(devrec->regmap_short, REG_RFCTL, 0x04);
 	if (ret)
 		goto err_ret;
 
-	ret = write_short_reg(devrec, REG_RFCTL, 0x0);
+	ret = regmap_write(devrec->regmap_short, REG_RFCTL, 0x0);
 	if (ret)
 		goto err_ret;
 
 	udelay(192);
 
 	/* Set RX Mode. RXMCR<1:0>: 0x0 normal, 0x1 promisc, 0x2 error */
-	ret = read_short_reg(devrec, REG_RXMCR, &val);
-	if (ret)
-		goto err_ret;
-
-	val &= ~0x3; /* Clear RX mode (normal) */
-
-	ret = write_short_reg(devrec, REG_RXMCR, val);
+	ret = regmap_update_bits(devrec->regmap_short, REG_RXMCR, 0x03, 0x00);
 	if (ret)
 		goto err_ret;
 
@@ -1057,22 +1012,20 @@ static int mrf24j40_hw_init(struct mrf24j40 *devrec)
 		/* Enable external amplifier.
 		 * From MRF24J40MC datasheet section 1.3: Operation.
 		 */
-		read_long_reg(devrec, REG_TESTMODE, &val);
-		val |= 0x7; /* Configure GPIO 0-2 to control amplifier */
-		write_long_reg(devrec, REG_TESTMODE, val);
+		regmap_update_bits(devrec->regmap_long, REG_TESTMODE, 0x07,
+				   0x07);
 
-		read_short_reg(devrec, REG_TRISGPIO, &val);
-		val |= 0x8; /* Set GPIO3 as output. */
-		write_short_reg(devrec, REG_TRISGPIO, val);
+		/* Set GPIO3 as output. */
+		regmap_update_bits(devrec->regmap_short, REG_TRISGPIO, 0x08,
+				   0x08);
 
-		read_short_reg(devrec, REG_GPIO, &val);
-		val |= 0x8; /* Set GPIO3 HIGH to enable U5 voltage regulator */
-		write_short_reg(devrec, REG_GPIO, val);
+		/* Set GPIO3 HIGH to enable U5 voltage regulator */
+		regmap_update_bits(devrec->regmap_short, REG_GPIO, 0x08, 0x08);
 
 		/* Reduce TX pwr to meet FCC requirements.
 		 * From MRF24J40MC datasheet section 3.1.1
 		 */
-		write_long_reg(devrec, REG_RFCON3, 0x28);
+		regmap_write(devrec->regmap_long, REG_RFCON3, 0x28);
 	}
 
 	return 0;
-- 
2.5.0

Re: [RFC bluetooth-next 10/21] mrf24j40: use regmap for register access

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch uses the regmap functions for transceiver register settings
> where it's possible. This means everything except the hotpaths like
> receive/transmit handling.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 147 +++++++++++++-------------------------
>   1 file changed, 50 insertions(+), 97 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 6578f68..32de5d6 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -543,35 +543,6 @@ static int read_long_reg(struct mrf24j40 *devrec, u16 reg, u8 *value)
>   	return ret;
>   }
>   
> -static int write_long_reg(struct mrf24j40 *devrec, u16 reg, u8 val)
> -{
> -	int ret;
> -	u16 cmd;
> -	struct spi_message msg;
> -	struct spi_transfer xfer = {
> -		.len = 3,
> -		.tx_buf = devrec->buf,
> -		.rx_buf = devrec->buf,
> -	};
> -
> -	spi_message_init(&msg);
> -	spi_message_add_tail(&xfer, &msg);
> -
> -	cmd = MRF24J40_WRITELONG(reg);
> -	mutex_lock(&devrec->buffer_mutex);
> -	devrec->buf[0] = cmd >> 8 & 0xff;
> -	devrec->buf[1] = cmd & 0xff;
> -	devrec->buf[2] = val;
> -
> -	ret = spi_sync(devrec->spi, &msg);
> -	if (ret)
> -		dev_err(printdev(devrec),
> -			"SPI write Failed for long register 0x%hx\n", reg);
> -
> -	mutex_unlock(&devrec->buffer_mutex);
> -	return ret;
> -}
> -
>   /* This function relies on an undocumented write method. Once a write command
>      and address is set, as many bytes of data as desired can be clocked into
>      the device. The datasheet only shows setting one byte at a time. */
> @@ -755,33 +726,23 @@ static int mrf24j40_ed(struct ieee802154_hw *hw, u8 *level)
>   static int mrf24j40_start(struct ieee802154_hw *hw)
>   {
>   	struct mrf24j40 *devrec = hw->priv;
> -	u8 val;
> -	int ret;
>   
>   	dev_dbg(printdev(devrec), "start\n");
>   
> -	ret = read_short_reg(devrec, REG_INTCON, &val);
> -	if (ret)
> -		return ret;
> -	val &= ~(0x1|0x8); /* Clear TXNIE and RXIE. Enable interrupts */
> -	write_short_reg(devrec, REG_INTCON, val);
> -
> -	return 0;
> +	/* Clear TXNIE and RXIE. Enable interrupts */
> +	return regmap_update_bits(devrec->regmap_short, REG_INTCON,
> +				  0x01 | 0x08, 0x00);
>   }
>   
>   static void mrf24j40_stop(struct ieee802154_hw *hw)
>   {
>   	struct mrf24j40 *devrec = hw->priv;
> -	u8 val;
> -	int ret;
>   
>   	dev_dbg(printdev(devrec), "stop\n");
>   
> -	ret = read_short_reg(devrec, REG_INTCON, &val);
> -	if (ret)
> -		return;
> -	val |= 0x1|0x8; /* Set TXNIE and RXIE. Disable Interrupts */
> -	write_short_reg(devrec, REG_INTCON, val);
> +	/* Set TXNIE and RXIE. Disable Interrupts */
> +	regmap_update_bits(devrec->regmap_short, REG_INTCON, 0x01 | 0x08,
> +			   0x01 | 0x08);
>   }
>   
>   static int mrf24j40_set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
> @@ -798,20 +759,20 @@ static int mrf24j40_set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
>   
>   	/* Set Channel TODO */
>   	val = (channel-11) << 4 | 0x03;
> -	write_long_reg(devrec, REG_RFCON0, val);
> +	ret = regmap_update_bits(devrec->regmap_long, REG_RFCON0, 0xf0, val);
> +	if (ret)
> +		return ret;
>   
>   	/* RF Reset */
> -	ret = read_short_reg(devrec, REG_RFCTL, &val);
> +	ret = regmap_update_bits(devrec->regmap_short, REG_RFCTL, 0x04, 0x04);
>   	if (ret)
>   		return ret;
> -	val |= 0x04;
> -	write_short_reg(devrec, REG_RFCTL, val);
> -	val &= ~0x04;
> -	write_short_reg(devrec, REG_RFCTL, val);
>   
> -	udelay(SET_CHANNEL_DELAY_US); /* per datasheet */
> +	ret = regmap_update_bits(devrec->regmap_short, REG_RFCTL, 0x04, 0x00);
> +	if (!ret)
> +		udelay(SET_CHANNEL_DELAY_US); /* per datasheet */
>   
> -	return 0;
> +	return ret;
>   }
>   
>   static int mrf24j40_filter(struct ieee802154_hw *hw,
> @@ -829,8 +790,8 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
>   		addrh = le16_to_cpu(filt->short_addr) >> 8 & 0xff;
>   		addrl = le16_to_cpu(filt->short_addr) & 0xff;
>   
> -		write_short_reg(devrec, REG_SADRH, addrh);
> -		write_short_reg(devrec, REG_SADRL, addrl);
> +		regmap_write(devrec->regmap_short, REG_SADRH, addrh);
> +		regmap_write(devrec->regmap_short, REG_SADRL, addrl);
>   		dev_dbg(printdev(devrec),
>   			"Set short addr to %04hx\n", filt->short_addr);
>   	}
> @@ -841,7 +802,8 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
>   
>   		memcpy(addr, &filt->ieee_addr, 8);
>   		for (i = 0; i < 8; i++)
> -			write_short_reg(devrec, REG_EADR0 + i, addr[i]);
> +			regmap_write(devrec->regmap_short, REG_EADR0 + i,
> +				     addr[i]);
>   
>   #ifdef DEBUG
>   		pr_debug("Set long addr to: ");
> @@ -857,8 +819,8 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
>   
>   		panidh = le16_to_cpu(filt->pan_id) >> 8 & 0xff;
>   		panidl = le16_to_cpu(filt->pan_id) & 0xff;
> -		write_short_reg(devrec, REG_PANIDH, panidh);
> -		write_short_reg(devrec, REG_PANIDL, panidl);
> +		regmap_write(devrec->regmap_short, REG_PANIDH, panidh);
> +		regmap_write(devrec->regmap_short, REG_PANIDL, panidl);
>   
>   		dev_dbg(printdev(devrec), "Set PANID to %04hx\n", filt->pan_id);
>   	}
> @@ -868,14 +830,14 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
>   		u8 val;
>   		int ret;
>   
> -		ret = read_short_reg(devrec, REG_RXMCR, &val);
> -		if (ret)
> -			return ret;
>   		if (filt->pan_coord)
> -			val |= 0x8;
> +			val = 0x8;
>   		else
> -			val &= ~0x8;
> -		write_short_reg(devrec, REG_RXMCR, val);
> +			val = 0x0;
> +		ret = regmap_update_bits(devrec->regmap_short, REG_RXMCR, 0x8,
> +					 val);
> +		if (ret)
> +			return ret;
>   
>   		/* REG_SLOTTED is maintained as default (unslotted/CSMA-CA).
>   		 * REG_ORDER is maintained as default (no beacon/superframe).
> @@ -976,80 +938,73 @@ out:
>   static int mrf24j40_hw_init(struct mrf24j40 *devrec)
>   {
>   	int ret;
> -	u8 val;
>   
>   	/* Initialize the device.
>   		From datasheet section 3.2: Initialization. */
> -	ret = write_short_reg(devrec, REG_SOFTRST, 0x07);
> +	ret = regmap_write(devrec->regmap_short, REG_SOFTRST, 0x07);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_short_reg(devrec, REG_PACON2, 0x98);
> +	ret = regmap_write(devrec->regmap_short, REG_PACON2, 0x98);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_short_reg(devrec, REG_TXSTBL, 0x95);
> +	ret = regmap_write(devrec->regmap_short, REG_TXSTBL, 0x95);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_long_reg(devrec, REG_RFCON0, 0x03);
> +	ret = regmap_write(devrec->regmap_long, REG_RFCON0, 0x03);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_long_reg(devrec, REG_RFCON1, 0x01);
> +	ret = regmap_write(devrec->regmap_long, REG_RFCON1, 0x01);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_long_reg(devrec, REG_RFCON2, 0x80);
> +	ret = regmap_write(devrec->regmap_long, REG_RFCON2, 0x80);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_long_reg(devrec, REG_RFCON6, 0x90);
> +	ret = regmap_write(devrec->regmap_long, REG_RFCON6, 0x90);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_long_reg(devrec, REG_RFCON7, 0x80);
> +	ret = regmap_write(devrec->regmap_long, REG_RFCON7, 0x80);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_long_reg(devrec, REG_RFCON8, 0x10);
> +	ret = regmap_write(devrec->regmap_long, REG_RFCON8, 0x10);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_long_reg(devrec, REG_SLPCON1, 0x21);
> +	ret = regmap_write(devrec->regmap_long, REG_SLPCON1, 0x21);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_short_reg(devrec, REG_BBREG2, 0x80);
> +	ret = regmap_write(devrec->regmap_short, REG_BBREG2, 0x80);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_short_reg(devrec, REG_CCAEDTH, 0x60);
> +	ret = regmap_write(devrec->regmap_short, REG_CCAEDTH, 0x60);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_short_reg(devrec, REG_BBREG6, 0x40);
> +	ret = regmap_write(devrec->regmap_short, REG_BBREG6, 0x40);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_short_reg(devrec, REG_RFCTL, 0x04);
> +	ret = regmap_write(devrec->regmap_short, REG_RFCTL, 0x04);
>   	if (ret)
>   		goto err_ret;
>   
> -	ret = write_short_reg(devrec, REG_RFCTL, 0x0);
> +	ret = regmap_write(devrec->regmap_short, REG_RFCTL, 0x0);
>   	if (ret)
>   		goto err_ret;
>   
>   	udelay(192);
>   
>   	/* Set RX Mode. RXMCR<1:0>: 0x0 normal, 0x1 promisc, 0x2 error */
> -	ret = read_short_reg(devrec, REG_RXMCR, &val);
> -	if (ret)
> -		goto err_ret;
> -
> -	val &= ~0x3; /* Clear RX mode (normal) */
> -
> -	ret = write_short_reg(devrec, REG_RXMCR, val);
> +	ret = regmap_update_bits(devrec->regmap_short, REG_RXMCR, 0x03, 0x00);
>   	if (ret)
>   		goto err_ret;
>   
> @@ -1057,22 +1012,20 @@ static int mrf24j40_hw_init(struct mrf24j40 *devrec)
>   		/* Enable external amplifier.
>   		 * From MRF24J40MC datasheet section 1.3: Operation.
>   		 */
> -		read_long_reg(devrec, REG_TESTMODE, &val);
> -		val |= 0x7; /* Configure GPIO 0-2 to control amplifier */
> -		write_long_reg(devrec, REG_TESTMODE, val);
> +		regmap_update_bits(devrec->regmap_long, REG_TESTMODE, 0x07,
> +				   0x07);
>   
> -		read_short_reg(devrec, REG_TRISGPIO, &val);
> -		val |= 0x8; /* Set GPIO3 as output. */
> -		write_short_reg(devrec, REG_TRISGPIO, val);
> +		/* Set GPIO3 as output. */
> +		regmap_update_bits(devrec->regmap_short, REG_TRISGPIO, 0x08,
> +				   0x08);
>   
> -		read_short_reg(devrec, REG_GPIO, &val);
> -		val |= 0x8; /* Set GPIO3 HIGH to enable U5 voltage regulator */
> -		write_short_reg(devrec, REG_GPIO, val);
> +		/* Set GPIO3 HIGH to enable U5 voltage regulator */
> +		regmap_update_bits(devrec->regmap_short, REG_GPIO, 0x08, 0x08);
>   
>   		/* Reduce TX pwr to meet FCC requirements.
>   		 * From MRF24J40MC datasheet section 3.1.1
>   		 */
> -		write_long_reg(devrec, REG_RFCON3, 0x28);
> +		regmap_write(devrec->regmap_long, REG_RFCON3, 0x28);
>   	}
>   
>   	return 0;

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 11/21] mrf24j40: change to frame delivery with crc

[Alexander Aring]

This patch changes the frame delivery to mac802154 with crc. This is
useful for monitor interface types which deliver the crc to userspace.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 32de5d6..7e6a038 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -881,9 +881,6 @@ static int mrf24j40_handle_rx(struct mrf24j40 *devrec)
 		goto out;
 	}
 
-	/* Cut off the checksum */
-	skb_trim(skb, len-2);
-
 	/* TODO: Other drivers call ieee20154_rx_irqsafe() here (eg: cc2040,
 	 * also from a workqueue).  I think irqsafe is not necessary here.
 	 * Can someone confirm? */
@@ -1060,7 +1057,7 @@ static int mrf24j40_probe(struct spi_device *spi)
 	devrec->hw = hw;
 	devrec->hw->parent = &spi->dev;
 	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
-	devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AFILT;
+	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
 
 	devrec->regmap_short = devm_regmap_init_spi(spi,
 						    &mrf24j40_short_regmap);
-- 
2.5.0

Re: [RFC bluetooth-next 11/21] mrf24j40: change to frame delivery with crc

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch changes the frame delivery to mac802154 with crc. This is
> useful for monitor interface types which deliver the crc to userspace.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 32de5d6..7e6a038 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -881,9 +881,6 @@ static int mrf24j40_handle_rx(struct mrf24j40 *devrec)
>   		goto out;
>   	}
>   
> -	/* Cut off the checksum */
> -	skb_trim(skb, len-2);
> -
>   	/* TODO: Other drivers call ieee20154_rx_irqsafe() here (eg: cc2040,
>   	 * also from a workqueue).  I think irqsafe is not necessary here.
>   	 * Can someone confirm? */
> @@ -1060,7 +1057,7 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	devrec->hw = hw;
>   	devrec->hw->parent = &spi->dev;
>   	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
> -	devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AFILT;
> +	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
>   
>   	devrec->regmap_short = devm_regmap_init_spi(spi,
>   						    &mrf24j40_short_regmap);

Good change. Let the stack handle this part and decide on what the user 
wants (monitor mode or normal)

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 12/21] ieee802154: add helpers for frame control checks

[Alexander Aring]

This patch introduce two static inline functions. The first to get the
frame control field from an sk_buff. The second is for checking on the
acknowledgment request bit on the frame control field. Later we can
introduce more functions to check on the frame control fields.

These will deprecate the current behaviour which requires a
host-byteorder conversion and manually bit handling.

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/linux/ieee802154.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index 1dc1f4e..e1d5f1f 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -25,6 +25,8 @@
 
 #include <linux/types.h>
 #include <linux/random.h>
+#include <linux/skbuff.h>
+#include <linux/unaligned/memmove.h>
 #include <asm/byteorder.h>
 
 #define IEEE802154_MTU			127
@@ -205,6 +207,33 @@ enum {
 	IEEE802154_SCAN_IN_PROGRESS = 0xfc,
 };
 
+/* frame control handling */
+#define IEEE802154_FCTL_ACKREQ	0x0020
+
+/**
+ * ieee802154_is_ackreq - check if acknowledgment request bit is set
+ * @fc: frame control bytes in little-endian byteorder
+ */
+static inline bool ieee802154_is_ackreq(__le16 fc)
+{
+	return fc & cpu_to_le16(IEEE802154_FCTL_ACKREQ);
+}
+
+/**
+ * ieee802154_get_fc_from_skb - get the frame control field from a skb
+ * @skb: skb which contains the frame control field at skb_mac_header
+ */
+static inline __le16 ieee802154_get_fc_from_skb(const struct sk_buff *skb)
+{
+	/* return on invalid fc */
+	if (unlikely(skb->mac_len < 2)) {
+		WARN_ON(1);
+		return cpu_to_le16(0);
+	}
+
+	return (__force __le16)__get_unaligned_memmove16(skb_mac_header(skb));
+}
+
 /**
  * ieee802154_is_valid_psdu_len - check if psdu len is valid
  * available lengths:
-- 
2.5.0

[RFC bluetooth-next 13/21] mrf24j40: rework tx handling to async tx handling

[Alexander Aring]

This patch reworks the current transmit API to spi_async handling. We
removed the error handling check, because mac802154 has no change to
report it. Also the transmit timeout handling can't be handled by xmit
async handling, for this usecase we need to implement the netdev
watchdog. These are all unlikely cases which we drop now and should be
provided by netdev watchdog.

We also drop the bit setting for TXNACKREQ at register TXNCON, this is
not necessary. The TXNCON register should set only once for each frame,
previous settings doesn't matter.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 161 ++++++++++++++++++--------------------
 1 file changed, 78 insertions(+), 83 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 7e6a038..8851475 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -152,8 +152,22 @@ struct mrf24j40 {
 
 	struct regmap *regmap_short;
 	struct regmap *regmap_long;
+
+	/* for writing txfifo */
+	struct spi_message tx_msg;
+	u8 tx_hdr_buf[2];
+	struct spi_transfer tx_hdr_trx;
+	u8 tx_len_buf[2];
+	struct spi_transfer tx_len_trx;
+	struct spi_transfer tx_buf_trx;
+	struct sk_buff *tx_skb;
+
+	/* post transmit message to send frame out  */
+	struct spi_message tx_post_msg;
+	u8 tx_post_buf[2];
+	struct spi_transfer tx_post_trx;
+
 	struct mutex buffer_mutex; /* only used to protect buf */
-	struct completion tx_complete;
 	u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */
 };
 
@@ -543,28 +557,33 @@ static int read_long_reg(struct mrf24j40 *devrec, u16 reg, u8 *value)
 	return ret;
 }
 
+static void write_tx_buf_complete(void *context)
+{
+	struct mrf24j40 *devrec = context;
+	__le16 fc = ieee802154_get_fc_from_skb(devrec->tx_skb);
+	u8 val = 0x01;
+	int ret;
+
+	if (ieee802154_is_ackreq(fc))
+		val |= 0x04;
+
+	devrec->tx_post_msg.complete = NULL;
+	devrec->tx_post_buf[0] = MRF24J40_WRITESHORT(REG_TXNCON);
+	devrec->tx_post_buf[1] = val;
+
+	ret = spi_async(devrec->spi, &devrec->tx_post_msg);
+	if (ret)
+		dev_err(printdev(devrec), "SPI write Failed for transmit buf\n");
+}
+
 /* This function relies on an undocumented write method. Once a write command
    and address is set, as many bytes of data as desired can be clocked into
    the device. The datasheet only shows setting one byte at a time. */
 static int write_tx_buf(struct mrf24j40 *devrec, u16 reg,
 			const u8 *data, size_t length)
 {
-	int ret;
 	u16 cmd;
-	u8 lengths[2];
-	struct spi_message msg;
-	struct spi_transfer addr_xfer = {
-		.len = 2,
-		.tx_buf = devrec->buf,
-	};
-	struct spi_transfer lengths_xfer = {
-		.len = 2,
-		.tx_buf = &lengths, /* TODO: Is DMA really required for SPI? */
-	};
-	struct spi_transfer data_xfer = {
-		.len = length,
-		.tx_buf = data,
-	};
+	int ret;
 
 	/* Range check the length. 2 bytes are used for the length fields.*/
 	if (length > TX_FIFO_SIZE-2) {
@@ -572,26 +591,31 @@ static int write_tx_buf(struct mrf24j40 *devrec, u16 reg,
 		length = TX_FIFO_SIZE-2;
 	}
 
-	spi_message_init(&msg);
-	spi_message_add_tail(&addr_xfer, &msg);
-	spi_message_add_tail(&lengths_xfer, &msg);
-	spi_message_add_tail(&data_xfer, &msg);
-
 	cmd = MRF24J40_WRITELONG(reg);
-	mutex_lock(&devrec->buffer_mutex);
-	devrec->buf[0] = cmd >> 8 & 0xff;
-	devrec->buf[1] = cmd & 0xff;
-	lengths[0] = 0x0; /* Header Length. Set to 0 for now. TODO */
-	lengths[1] = length; /* Total length */
-
-	ret = spi_sync(devrec->spi, &msg);
+	devrec->tx_hdr_buf[0] = cmd >> 8 & 0xff;
+	devrec->tx_hdr_buf[1] = cmd & 0xff;
+	devrec->tx_len_buf[0] = 0x0; /* Header Length. Set to 0 for now. TODO */
+	devrec->tx_len_buf[1] = length; /* Total length */
+	devrec->tx_buf_trx.tx_buf = data;
+	devrec->tx_buf_trx.len = length;
+
+	ret = spi_async(devrec->spi, &devrec->tx_msg);
 	if (ret)
 		dev_err(printdev(devrec), "SPI write Failed for TX buf\n");
 
-	mutex_unlock(&devrec->buffer_mutex);
 	return ret;
 }
 
+static int mrf24j40_tx(struct ieee802154_hw *hw, struct sk_buff *skb)
+{
+	struct mrf24j40 *devrec = hw->priv;
+
+	dev_dbg(printdev(devrec), "tx packet of %d bytes\n", skb->len);
+	devrec->tx_skb = skb;
+
+	return write_tx_buf(devrec, 0x000, skb->data, skb->len);
+}
+
 static int mrf24j40_read_rx_buf(struct mrf24j40 *devrec,
 				u8 *data, u8 *len, u8 *lqi)
 {
@@ -664,57 +688,6 @@ out:
 	return ret;
 }
 
-static int mrf24j40_tx(struct ieee802154_hw *hw, struct sk_buff *skb)
-{
-	struct mrf24j40 *devrec = hw->priv;
-	u8 val;
-	int ret = 0;
-
-	dev_dbg(printdev(devrec), "tx packet of %d bytes\n", skb->len);
-
-	ret = write_tx_buf(devrec, 0x000, skb->data, skb->len);
-	if (ret)
-		goto err;
-
-	reinit_completion(&devrec->tx_complete);
-
-	/* Set TXNTRIG bit of TXNCON to send packet */
-	ret = read_short_reg(devrec, REG_TXNCON, &val);
-	if (ret)
-		goto err;
-	val |= 0x1;
-	/* Set TXNACKREQ if the ACK bit is set in the packet. */
-	if (skb->data[0] & IEEE802154_FC_ACK_REQ)
-		val |= 0x4;
-	write_short_reg(devrec, REG_TXNCON, val);
-
-	/* Wait for the device to send the TX complete interrupt. */
-	ret = wait_for_completion_interruptible_timeout(
-						&devrec->tx_complete,
-						5 * HZ);
-	if (ret == -ERESTARTSYS)
-		goto err;
-	if (ret == 0) {
-		dev_warn(printdev(devrec), "Timeout waiting for TX interrupt\n");
-		ret = -ETIMEDOUT;
-		goto err;
-	}
-
-	/* Check for send error from the device. */
-	ret = read_short_reg(devrec, REG_TXSTAT, &val);
-	if (ret)
-		goto err;
-	if (val & 0x1) {
-		dev_dbg(printdev(devrec), "Error Sending. Retry count exceeded\n");
-		ret = -ECOMM; /* TODO: Better error code ? */
-	} else
-		dev_dbg(printdev(devrec), "Packet Sent\n");
-
-err:
-
-	return ret;
-}
-
 static int mrf24j40_ed(struct ieee802154_hw *hw, u8 *level)
 {
 	/* TODO: */
@@ -901,7 +874,7 @@ out:
 
 static const struct ieee802154_ops mrf24j40_ops = {
 	.owner = THIS_MODULE,
-	.xmit_sync = mrf24j40_tx,
+	.xmit_async = mrf24j40_tx,
 	.ed = mrf24j40_ed,
 	.start = mrf24j40_start,
 	.stop = mrf24j40_stop,
@@ -922,7 +895,7 @@ static irqreturn_t mrf24j40_isr(int irq, void *data)
 
 	/* Check for TX complete */
 	if (intstat & 0x1)
-		complete(&devrec->tx_complete);
+		ieee802154_xmit_complete(devrec->hw, devrec->tx_skb, false);
 
 	/* Check for Rx */
 	if (intstat & 0x8)
@@ -1031,6 +1004,27 @@ err_ret:
 	return ret;
 }
 
+static void
+mrf24j40_setup_tx_spi_messages(struct mrf24j40 *devrec)
+{
+	spi_message_init(&devrec->tx_msg);
+	devrec->tx_msg.context = devrec;
+	devrec->tx_msg.complete = write_tx_buf_complete;
+	devrec->tx_hdr_trx.len = 2;
+	devrec->tx_hdr_trx.tx_buf = devrec->tx_hdr_buf;
+	spi_message_add_tail(&devrec->tx_hdr_trx, &devrec->tx_msg);
+	devrec->tx_len_trx.len = 2;
+	devrec->tx_len_trx.tx_buf = devrec->tx_len_buf;
+	spi_message_add_tail(&devrec->tx_len_trx, &devrec->tx_msg);
+	spi_message_add_tail(&devrec->tx_buf_trx, &devrec->tx_msg);
+
+	spi_message_init(&devrec->tx_post_msg);
+	devrec->tx_post_msg.context = devrec;
+	devrec->tx_post_trx.len = 2;
+	devrec->tx_post_trx.tx_buf = devrec->tx_post_buf;
+	spi_message_add_tail(&devrec->tx_post_trx, &devrec->tx_post_msg);
+}
+
 static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
 {
 	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
@@ -1059,6 +1053,8 @@ static int mrf24j40_probe(struct spi_device *spi)
 	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
 	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
 
+	mrf24j40_setup_tx_spi_messages(devrec);
+
 	devrec->regmap_short = devm_regmap_init_spi(spi,
 						    &mrf24j40_short_regmap);
 	if (IS_ERR(devrec->regmap_short)) {
@@ -1083,7 +1079,6 @@ static int mrf24j40_probe(struct spi_device *spi)
 		goto err_register_device;
 
 	mutex_init(&devrec->buffer_mutex);
-	init_completion(&devrec->tx_complete);
 
 	ret = mrf24j40_hw_init(devrec);
 	if (ret)
-- 
2.5.0

Re: [RFC bluetooth-next 13/21] mrf24j40: rework tx handling to async tx handling

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch reworks the current transmit API to spi_async handling. We
> removed the error handling check, because mac802154 has no change to

s/change/chance/
> report it. Also the transmit timeout handling can't be handled by xmit
> async handling, for this usecase we need to implement the netdev
> watchdog. These are all unlikely cases which we drop now and should be
> provided by netdev watchdog.
>
> We also drop the bit setting for TXNACKREQ at register TXNCON, this is
> not necessary. The TXNCON register should set only once for each frame,
> previous settings doesn't matter.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 161 ++++++++++++++++++--------------------
>   1 file changed, 78 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 7e6a038..8851475 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -152,8 +152,22 @@ struct mrf24j40 {
>   
>   	struct regmap *regmap_short;
>   	struct regmap *regmap_long;
> +
> +	/* for writing txfifo */
> +	struct spi_message tx_msg;
> +	u8 tx_hdr_buf[2];
> +	struct spi_transfer tx_hdr_trx;
> +	u8 tx_len_buf[2];
> +	struct spi_transfer tx_len_trx;
> +	struct spi_transfer tx_buf_trx;
> +	struct sk_buff *tx_skb;
> +
> +	/* post transmit message to send frame out  */
> +	struct spi_message tx_post_msg;
> +	u8 tx_post_buf[2];
> +	struct spi_transfer tx_post_trx;
> +
>   	struct mutex buffer_mutex; /* only used to protect buf */
> -	struct completion tx_complete;
>   	u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */
>   };
>   
> @@ -543,28 +557,33 @@ static int read_long_reg(struct mrf24j40 *devrec, u16 reg, u8 *value)
>   	return ret;
>   }
>   
> +static void write_tx_buf_complete(void *context)
> +{
> +	struct mrf24j40 *devrec = context;
> +	__le16 fc = ieee802154_get_fc_from_skb(devrec->tx_skb);
> +	u8 val = 0x01;
> +	int ret;
> +
> +	if (ieee802154_is_ackreq(fc))
> +		val |= 0x04;

Magic number.
> +
> +	devrec->tx_post_msg.complete = NULL;
> +	devrec->tx_post_buf[0] = MRF24J40_WRITESHORT(REG_TXNCON);
> +	devrec->tx_post_buf[1] = val;
> +
> +	ret = spi_async(devrec->spi, &devrec->tx_post_msg);
> +	if (ret)
> +		dev_err(printdev(devrec), "SPI write Failed for transmit buf\n");
> +}
> +
>   /* This function relies on an undocumented write method. Once a write command
>      and address is set, as many bytes of data as desired can be clocked into
>      the device. The datasheet only shows setting one byte at a time. */
>   static int write_tx_buf(struct mrf24j40 *devrec, u16 reg,
>   			const u8 *data, size_t length)
>   {
> -	int ret;
>   	u16 cmd;
> -	u8 lengths[2];
> -	struct spi_message msg;
> -	struct spi_transfer addr_xfer = {
> -		.len = 2,
> -		.tx_buf = devrec->buf,
> -	};
> -	struct spi_transfer lengths_xfer = {
> -		.len = 2,
> -		.tx_buf = &lengths, /* TODO: Is DMA really required for SPI? */
> -	};
> -	struct spi_transfer data_xfer = {
> -		.len = length,
> -		.tx_buf = data,
> -	};
> +	int ret;
>   
>   	/* Range check the length. 2 bytes are used for the length fields.*/
>   	if (length > TX_FIFO_SIZE-2) {
> @@ -572,26 +591,31 @@ static int write_tx_buf(struct mrf24j40 *devrec, u16 reg,
>   		length = TX_FIFO_SIZE-2;
>   	}
>   
> -	spi_message_init(&msg);
> -	spi_message_add_tail(&addr_xfer, &msg);
> -	spi_message_add_tail(&lengths_xfer, &msg);
> -	spi_message_add_tail(&data_xfer, &msg);
> -
>   	cmd = MRF24J40_WRITELONG(reg);
> -	mutex_lock(&devrec->buffer_mutex);
> -	devrec->buf[0] = cmd >> 8 & 0xff;
> -	devrec->buf[1] = cmd & 0xff;
> -	lengths[0] = 0x0; /* Header Length. Set to 0 for now. TODO */
> -	lengths[1] = length; /* Total length */
> -
> -	ret = spi_sync(devrec->spi, &msg);
> +	devrec->tx_hdr_buf[0] = cmd >> 8 & 0xff;
> +	devrec->tx_hdr_buf[1] = cmd & 0xff;
> +	devrec->tx_len_buf[0] = 0x0; /* Header Length. Set to 0 for now. TODO */
> +	devrec->tx_len_buf[1] = length; /* Total length */
> +	devrec->tx_buf_trx.tx_buf = data;
> +	devrec->tx_buf_trx.len = length;
> +
> +	ret = spi_async(devrec->spi, &devrec->tx_msg);
>   	if (ret)
>   		dev_err(printdev(devrec), "SPI write Failed for TX buf\n");
>   
> -	mutex_unlock(&devrec->buffer_mutex);
>   	return ret;
>   }
>   
> +static int mrf24j40_tx(struct ieee802154_hw *hw, struct sk_buff *skb)
> +{
> +	struct mrf24j40 *devrec = hw->priv;
> +
> +	dev_dbg(printdev(devrec), "tx packet of %d bytes\n", skb->len);
> +	devrec->tx_skb = skb;
> +
> +	return write_tx_buf(devrec, 0x000, skb->data, skb->len);
> +}
> +
>   static int mrf24j40_read_rx_buf(struct mrf24j40 *devrec,
>   				u8 *data, u8 *len, u8 *lqi)
>   {
> @@ -664,57 +688,6 @@ out:
>   	return ret;
>   }
>   
> -static int mrf24j40_tx(struct ieee802154_hw *hw, struct sk_buff *skb)
> -{
> -	struct mrf24j40 *devrec = hw->priv;
> -	u8 val;
> -	int ret = 0;
> -
> -	dev_dbg(printdev(devrec), "tx packet of %d bytes\n", skb->len);
> -
> -	ret = write_tx_buf(devrec, 0x000, skb->data, skb->len);
> -	if (ret)
> -		goto err;
> -
> -	reinit_completion(&devrec->tx_complete);
> -
> -	/* Set TXNTRIG bit of TXNCON to send packet */
> -	ret = read_short_reg(devrec, REG_TXNCON, &val);
> -	if (ret)
> -		goto err;
> -	val |= 0x1;
> -	/* Set TXNACKREQ if the ACK bit is set in the packet. */
> -	if (skb->data[0] & IEEE802154_FC_ACK_REQ)
> -		val |= 0x4;
> -	write_short_reg(devrec, REG_TXNCON, val);
> -
> -	/* Wait for the device to send the TX complete interrupt. */
> -	ret = wait_for_completion_interruptible_timeout(
> -						&devrec->tx_complete,
> -						5 * HZ);
> -	if (ret == -ERESTARTSYS)
> -		goto err;
> -	if (ret == 0) {
> -		dev_warn(printdev(devrec), "Timeout waiting for TX interrupt\n");
> -		ret = -ETIMEDOUT;
> -		goto err;
> -	}
> -
> -	/* Check for send error from the device. */
> -	ret = read_short_reg(devrec, REG_TXSTAT, &val);
> -	if (ret)
> -		goto err;
> -	if (val & 0x1) {
> -		dev_dbg(printdev(devrec), "Error Sending. Retry count exceeded\n");
> -		ret = -ECOMM; /* TODO: Better error code ? */
> -	} else
> -		dev_dbg(printdev(devrec), "Packet Sent\n");
> -
> -err:
> -
> -	return ret;
> -}
> -
>   static int mrf24j40_ed(struct ieee802154_hw *hw, u8 *level)
>   {
>   	/* TODO: */
> @@ -901,7 +874,7 @@ out:
>   
>   static const struct ieee802154_ops mrf24j40_ops = {
>   	.owner = THIS_MODULE,
> -	.xmit_sync = mrf24j40_tx,
> +	.xmit_async = mrf24j40_tx,
>   	.ed = mrf24j40_ed,
>   	.start = mrf24j40_start,
>   	.stop = mrf24j40_stop,
> @@ -922,7 +895,7 @@ static irqreturn_t mrf24j40_isr(int irq, void *data)
>   
>   	/* Check for TX complete */
>   	if (intstat & 0x1)
> -		complete(&devrec->tx_complete);
> +		ieee802154_xmit_complete(devrec->hw, devrec->tx_skb, false);
>   
>   	/* Check for Rx */
>   	if (intstat & 0x8)
> @@ -1031,6 +1004,27 @@ err_ret:
>   	return ret;
>   }
>   
> +static void
> +mrf24j40_setup_tx_spi_messages(struct mrf24j40 *devrec)
> +{
> +	spi_message_init(&devrec->tx_msg);
> +	devrec->tx_msg.context = devrec;
> +	devrec->tx_msg.complete = write_tx_buf_complete;
> +	devrec->tx_hdr_trx.len = 2;
> +	devrec->tx_hdr_trx.tx_buf = devrec->tx_hdr_buf;
> +	spi_message_add_tail(&devrec->tx_hdr_trx, &devrec->tx_msg);
> +	devrec->tx_len_trx.len = 2;
> +	devrec->tx_len_trx.tx_buf = devrec->tx_len_buf;
> +	spi_message_add_tail(&devrec->tx_len_trx, &devrec->tx_msg);
> +	spi_message_add_tail(&devrec->tx_buf_trx, &devrec->tx_msg);
> +
> +	spi_message_init(&devrec->tx_post_msg);
> +	devrec->tx_post_msg.context = devrec;
> +	devrec->tx_post_trx.len = 2;
> +	devrec->tx_post_trx.tx_buf = devrec->tx_post_buf;
> +	spi_message_add_tail(&devrec->tx_post_trx, &devrec->tx_post_msg);
> +}
> +
>   static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>   {
>   	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
> @@ -1059,6 +1053,8 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
>   	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
>   
> +	mrf24j40_setup_tx_spi_messages(devrec);
> +
>   	devrec->regmap_short = devm_regmap_init_spi(spi,
>   						    &mrf24j40_short_regmap);
>   	if (IS_ERR(devrec->regmap_short)) {
> @@ -1083,7 +1079,6 @@ static int mrf24j40_probe(struct spi_device *spi)
>   		goto err_register_device;
>   
>   	mutex_init(&devrec->buffer_mutex);
> -	init_completion(&devrec->tx_complete);
>   
>   	ret = mrf24j40_hw_init(devrec);
>   	if (ret)

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 14/21] mrf24j40: rework rx handling to async rx handling

[Alexander Aring]

This patch prepares that we can do the receive handling inside interrupt
context by using spi_async. This is necessary for introduce a
non-threaded irq handling.

Also we drop the bit setting for "RXDECINV" at register "BBREG1", we do
a driectly full write of register "BBREG1", because it contains the bit
RXDECINV only.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 275 +++++++++++++++-----------------------
 1 file changed, 111 insertions(+), 164 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 8851475..044d6c6 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -167,6 +167,20 @@ struct mrf24j40 {
 	u8 tx_post_buf[2];
 	struct spi_transfer tx_post_trx;
 
+	/* for protect/unprotect/read length rxfifo */
+	struct spi_message rx_msg;
+	u8 rx_buf[3];
+	struct spi_transfer rx_trx;
+
+	/* receive handling */
+	struct spi_message rx_buf_msg;
+	u8 rx_addr_buf[2];
+	struct spi_transfer rx_addr_trx;
+	u8 rx_lqi_buf[2];
+	struct spi_transfer rx_lqi_trx;
+	u8 rx_fifo_buf[RX_FIFO_SIZE];
+	struct spi_transfer rx_fifo_buf_trx;
+
 	struct mutex buffer_mutex; /* only used to protect buf */
 	u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */
 };
@@ -472,32 +486,6 @@ static const struct regmap_bus mrf24j40_long_regmap_bus = {
 	.val_format_endian_default = REGMAP_ENDIAN_BIG,
 };
 
-static int write_short_reg(struct mrf24j40 *devrec, u8 reg, u8 value)
-{
-	int ret;
-	struct spi_message msg;
-	struct spi_transfer xfer = {
-		.len = 2,
-		.tx_buf = devrec->buf,
-		.rx_buf = devrec->buf,
-	};
-
-	spi_message_init(&msg);
-	spi_message_add_tail(&xfer, &msg);
-
-	mutex_lock(&devrec->buffer_mutex);
-	devrec->buf[0] = MRF24J40_WRITESHORT(reg);
-	devrec->buf[1] = value;
-
-	ret = spi_sync(devrec->spi, &msg);
-	if (ret)
-		dev_err(printdev(devrec),
-			"SPI write Failed for short register 0x%hhx\n", reg);
-
-	mutex_unlock(&devrec->buffer_mutex);
-	return ret;
-}
-
 static int read_short_reg(struct mrf24j40 *devrec, u8 reg, u8 *val)
 {
 	int ret = -1;
@@ -526,37 +514,6 @@ static int read_short_reg(struct mrf24j40 *devrec, u8 reg, u8 *val)
 	return ret;
 }
 
-static int read_long_reg(struct mrf24j40 *devrec, u16 reg, u8 *value)
-{
-	int ret;
-	u16 cmd;
-	struct spi_message msg;
-	struct spi_transfer xfer = {
-		.len = 3,
-		.tx_buf = devrec->buf,
-		.rx_buf = devrec->buf,
-	};
-
-	spi_message_init(&msg);
-	spi_message_add_tail(&xfer, &msg);
-
-	cmd = MRF24J40_READLONG(reg);
-	mutex_lock(&devrec->buffer_mutex);
-	devrec->buf[0] = cmd >> 8 & 0xff;
-	devrec->buf[1] = cmd & 0xff;
-	devrec->buf[2] = 0;
-
-	ret = spi_sync(devrec->spi, &msg);
-	if (ret)
-		dev_err(printdev(devrec),
-			"SPI read Failed for long register 0x%hx\n", reg);
-	else
-		*value = devrec->buf[2];
-
-	mutex_unlock(&devrec->buffer_mutex);
-	return ret;
-}
-
 static void write_tx_buf_complete(void *context)
 {
 	struct mrf24j40 *devrec = context;
@@ -616,78 +573,6 @@ static int mrf24j40_tx(struct ieee802154_hw *hw, struct sk_buff *skb)
 	return write_tx_buf(devrec, 0x000, skb->data, skb->len);
 }
 
-static int mrf24j40_read_rx_buf(struct mrf24j40 *devrec,
-				u8 *data, u8 *len, u8 *lqi)
-{
-	u8 rx_len;
-	u8 addr[2];
-	u8 lqi_rssi[2];
-	u16 cmd;
-	int ret;
-	struct spi_message msg;
-	struct spi_transfer addr_xfer = {
-		.len = 2,
-		.tx_buf = &addr,
-	};
-	struct spi_transfer data_xfer = {
-		.len = 0x0, /* set below */
-		.rx_buf = data,
-	};
-	struct spi_transfer status_xfer = {
-		.len = 2,
-		.rx_buf = &lqi_rssi,
-	};
-
-	/* Get the length of the data in the RX FIFO. The length in this
-	 * register exclues the 1-byte length field at the beginning. */
-	ret = read_long_reg(devrec, REG_RX_FIFO, &rx_len);
-	if (ret)
-		goto out;
-
-	/* Range check the RX FIFO length, accounting for the one-byte
-	 * length field at the beginning. */
-	if (rx_len > RX_FIFO_SIZE-1) {
-		dev_err(printdev(devrec), "Invalid length read from device. Performing short read.\n");
-		rx_len = RX_FIFO_SIZE-1;
-	}
-
-	if (rx_len > *len) {
-		/* Passed in buffer wasn't big enough. Should never happen. */
-		dev_err(printdev(devrec), "Buffer not big enough. Performing short read\n");
-		rx_len = *len;
-	}
-
-	/* Set up the commands to read the data. */
-	cmd = MRF24J40_READLONG(REG_RX_FIFO+1);
-	addr[0] = cmd >> 8 & 0xff;
-	addr[1] = cmd & 0xff;
-	data_xfer.len = rx_len;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(&addr_xfer, &msg);
-	spi_message_add_tail(&data_xfer, &msg);
-	spi_message_add_tail(&status_xfer, &msg);
-
-	ret = spi_sync(devrec->spi, &msg);
-	if (ret) {
-		dev_err(printdev(devrec), "SPI RX Buffer Read Failed.\n");
-		goto out;
-	}
-
-	*lqi = lqi_rssi[0];
-	*len = rx_len;
-
-#ifdef DEBUG
-	print_hex_dump(KERN_DEBUG, "mrf24j40 rx: ",
-		       DUMP_PREFIX_OFFSET, 16, 1, data, *len, 0);
-	pr_debug("mrf24j40 rx: lqi: %02hhx rssi: %02hhx\n",
-		 lqi_rssi[0], lqi_rssi[1]);
-#endif
-
-out:
-	return ret;
-}
-
 static int mrf24j40_ed(struct ieee802154_hw *hw, u8 *level)
 {
 	/* TODO: */
@@ -823,53 +708,91 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
 	return 0;
 }
 
-static int mrf24j40_handle_rx(struct mrf24j40 *devrec)
+static void mrf24j40_handle_rx_read_buf_unlock(struct mrf24j40 *devrec)
 {
-	u8 len = RX_FIFO_SIZE;
-	u8 lqi = 0;
-	u8 val;
-	int ret = 0;
-	int ret2;
-	struct sk_buff *skb;
+	int ret;
 
-	/* Turn off reception of packets off the air. This prevents the
-	 * device from overwriting the buffer while we're reading it. */
-	ret = read_short_reg(devrec, REG_BBREG1, &val);
+	/* Turn back on reception of packets off the air. */
+	devrec->rx_msg.complete = NULL;
+	devrec->rx_buf[0] = MRF24J40_WRITESHORT(REG_BBREG1);
+	devrec->rx_buf[1] = 0x00; /* CLR RXDECINV */
+	ret = spi_async(devrec->spi, &devrec->rx_msg);
 	if (ret)
-		goto out;
-	val |= 4; /* SET RXDECINV */
-	write_short_reg(devrec, REG_BBREG1, val);
+		dev_err(printdev(devrec), "failed to unlock rx buffer\n");
+}
 
-	skb = dev_alloc_skb(len);
+static void mrf24j40_handle_rx_read_buf_complete(void *context)
+{
+	struct mrf24j40 *devrec = context;
+	u8 len = devrec->rx_buf[2];
+	u8 rx_local_buf[RX_FIFO_SIZE];
+	struct sk_buff *skb;
+
+	memcpy(rx_local_buf, devrec->rx_fifo_buf, len);
+	mrf24j40_handle_rx_read_buf_unlock(devrec);
+
+	skb = dev_alloc_skb(IEEE802154_MTU);
 	if (!skb) {
-		ret = -ENOMEM;
-		goto out;
+		dev_err(printdev(devrec), "failed to allocate skb\n");
+		return;
 	}
 
-	ret = mrf24j40_read_rx_buf(devrec, skb_put(skb, len), &len, &lqi);
-	if (ret < 0) {
-		dev_err(printdev(devrec), "Failure reading RX FIFO\n");
-		kfree_skb(skb);
-		ret = -EINVAL;
-		goto out;
+	memcpy(skb_put(skb, len), rx_local_buf, len);
+	ieee802154_rx_irqsafe(devrec->hw, skb, 0);
+}
+
+static void mrf24j40_handle_rx_read_buf(void *context)
+{
+	struct mrf24j40 *devrec = context;
+	u16 cmd;
+	int ret;
+
+	/* if length is invalid read the full MTU */
+	if (!ieee802154_is_valid_psdu_len(devrec->rx_buf[2]))
+		devrec->rx_buf[2] = IEEE802154_MTU;
+
+	cmd = MRF24J40_READLONG(REG_RX_FIFO + 1);
+	devrec->rx_addr_buf[0] = cmd >> 8 & 0xff;
+	devrec->rx_addr_buf[1] = cmd & 0xff;
+	devrec->rx_fifo_buf_trx.len = devrec->rx_buf[2];
+	ret = spi_async(devrec->spi, &devrec->rx_buf_msg);
+	if (ret) {
+		dev_err(printdev(devrec), "failed to read rx buffer\n");
+		mrf24j40_handle_rx_read_buf_unlock(devrec);
 	}
+}
+
+static void mrf24j40_handle_rx_read_len(void *context)
+{
+	struct mrf24j40 *devrec = context;
+	u16 cmd;
+	int ret;
 
-	/* TODO: Other drivers call ieee20154_rx_irqsafe() here (eg: cc2040,
-	 * also from a workqueue).  I think irqsafe is not necessary here.
-	 * Can someone confirm? */
-	ieee802154_rx_irqsafe(devrec->hw, skb, lqi);
+	/* read the length of received frame */
+	devrec->rx_msg.complete = mrf24j40_handle_rx_read_buf;
+	devrec->rx_trx.len = 3;
+	cmd = MRF24J40_READLONG(REG_RX_FIFO);
+	devrec->rx_buf[0] = cmd >> 8 & 0xff;
+	devrec->rx_buf[1] = cmd & 0xff;
 
-	dev_dbg(printdev(devrec), "RX Handled\n");
+	ret = spi_async(devrec->spi, &devrec->rx_msg);
+	if (ret) {
+		dev_err(printdev(devrec), "failed to read rx buffer length\n");
+		mrf24j40_handle_rx_read_buf_unlock(devrec);
+	}
+}
 
-out:
-	/* Turn back on reception of packets off the air. */
-	ret2 = read_short_reg(devrec, REG_BBREG1, &val);
-	if (ret2)
-		return ret2;
-	val &= ~0x4; /* Clear RXDECINV */
-	write_short_reg(devrec, REG_BBREG1, val);
+static int mrf24j40_handle_rx(struct mrf24j40 *devrec)
+{
+	/* Turn off reception of packets off the air. This prevents the
+	 * device from overwriting the buffer while we're reading it.
+	 */
+	devrec->rx_msg.complete = mrf24j40_handle_rx_read_len;
+	devrec->rx_trx.len = 2;
+	devrec->rx_buf[0] = MRF24J40_WRITESHORT(REG_BBREG1);
+	devrec->rx_buf[1] = 0x04; /* SET RXDECINV */
 
-	return ret;
+	return spi_async(devrec->spi, &devrec->rx_msg);
 }
 
 static const struct ieee802154_ops mrf24j40_ops = {
@@ -1025,6 +948,29 @@ mrf24j40_setup_tx_spi_messages(struct mrf24j40 *devrec)
 	spi_message_add_tail(&devrec->tx_post_trx, &devrec->tx_post_msg);
 }
 
+static void
+mrf24j40_setup_rx_spi_messages(struct mrf24j40 *devrec)
+{
+	spi_message_init(&devrec->rx_msg);
+	devrec->rx_msg.context = devrec;
+	devrec->rx_trx.len = 2;
+	devrec->rx_trx.tx_buf = devrec->rx_buf;
+	devrec->rx_trx.rx_buf = devrec->rx_buf;
+	spi_message_add_tail(&devrec->rx_trx, &devrec->rx_msg);
+
+	spi_message_init(&devrec->rx_buf_msg);
+	devrec->rx_buf_msg.context = devrec;
+	devrec->rx_buf_msg.complete = mrf24j40_handle_rx_read_buf_complete;
+	devrec->rx_addr_trx.len = 2;
+	devrec->rx_addr_trx.tx_buf = devrec->rx_addr_buf;
+	spi_message_add_tail(&devrec->rx_addr_trx, &devrec->rx_buf_msg);
+	devrec->rx_fifo_buf_trx.rx_buf = devrec->rx_fifo_buf;
+	spi_message_add_tail(&devrec->rx_fifo_buf_trx, &devrec->rx_buf_msg);
+	devrec->rx_lqi_trx.len = 2;
+	devrec->rx_lqi_trx.rx_buf = devrec->rx_lqi_buf;
+	spi_message_add_tail(&devrec->rx_lqi_trx, &devrec->rx_buf_msg);
+}
+
 static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
 {
 	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
@@ -1054,6 +1000,7 @@ static int mrf24j40_probe(struct spi_device *spi)
 	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
 
 	mrf24j40_setup_tx_spi_messages(devrec);
+	mrf24j40_setup_rx_spi_messages(devrec);
 
 	devrec->regmap_short = devm_regmap_init_spi(spi,
 						    &mrf24j40_short_regmap);
-- 
2.5.0

Re: [RFC bluetooth-next 14/21] mrf24j40: rework rx handling to async rx handling

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch prepares that we can do the receive handling inside interrupt
> context by using spi_async. This is necessary for introduce a
> non-threaded irq handling.
>
> Also we drop the bit setting for "RXDECINV" at register "BBREG1", we do
> a driectly full write of register "BBREG1", because it contains the bit
> RXDECINV only.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 275 +++++++++++++++-----------------------
>   1 file changed, 111 insertions(+), 164 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 8851475..044d6c6 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -167,6 +167,20 @@ struct mrf24j40 {
>   	u8 tx_post_buf[2];
>   	struct spi_transfer tx_post_trx;
>   
> +	/* for protect/unprotect/read length rxfifo */
> +	struct spi_message rx_msg;
> +	u8 rx_buf[3];
> +	struct spi_transfer rx_trx;
> +
> +	/* receive handling */
> +	struct spi_message rx_buf_msg;
> +	u8 rx_addr_buf[2];
> +	struct spi_transfer rx_addr_trx;
> +	u8 rx_lqi_buf[2];
> +	struct spi_transfer rx_lqi_trx;
> +	u8 rx_fifo_buf[RX_FIFO_SIZE];
> +	struct spi_transfer rx_fifo_buf_trx;
> +
>   	struct mutex buffer_mutex; /* only used to protect buf */
>   	u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */
>   };
> @@ -472,32 +486,6 @@ static const struct regmap_bus mrf24j40_long_regmap_bus = {
>   	.val_format_endian_default = REGMAP_ENDIAN_BIG,
>   };
>   
> -static int write_short_reg(struct mrf24j40 *devrec, u8 reg, u8 value)
> -{
> -	int ret;
> -	struct spi_message msg;
> -	struct spi_transfer xfer = {
> -		.len = 2,
> -		.tx_buf = devrec->buf,
> -		.rx_buf = devrec->buf,
> -	};
> -
> -	spi_message_init(&msg);
> -	spi_message_add_tail(&xfer, &msg);
> -
> -	mutex_lock(&devrec->buffer_mutex);
> -	devrec->buf[0] = MRF24J40_WRITESHORT(reg);
> -	devrec->buf[1] = value;
> -
> -	ret = spi_sync(devrec->spi, &msg);
> -	if (ret)
> -		dev_err(printdev(devrec),
> -			"SPI write Failed for short register 0x%hhx\n", reg);
> -
> -	mutex_unlock(&devrec->buffer_mutex);
> -	return ret;
> -}
> -
>   static int read_short_reg(struct mrf24j40 *devrec, u8 reg, u8 *val)
>   {
>   	int ret = -1;
> @@ -526,37 +514,6 @@ static int read_short_reg(struct mrf24j40 *devrec, u8 reg, u8 *val)
>   	return ret;
>   }
>   
> -static int read_long_reg(struct mrf24j40 *devrec, u16 reg, u8 *value)
> -{
> -	int ret;
> -	u16 cmd;
> -	struct spi_message msg;
> -	struct spi_transfer xfer = {
> -		.len = 3,
> -		.tx_buf = devrec->buf,
> -		.rx_buf = devrec->buf,
> -	};
> -
> -	spi_message_init(&msg);
> -	spi_message_add_tail(&xfer, &msg);
> -
> -	cmd = MRF24J40_READLONG(reg);
> -	mutex_lock(&devrec->buffer_mutex);
> -	devrec->buf[0] = cmd >> 8 & 0xff;
> -	devrec->buf[1] = cmd & 0xff;
> -	devrec->buf[2] = 0;
> -
> -	ret = spi_sync(devrec->spi, &msg);
> -	if (ret)
> -		dev_err(printdev(devrec),
> -			"SPI read Failed for long register 0x%hx\n", reg);
> -	else
> -		*value = devrec->buf[2];
> -
> -	mutex_unlock(&devrec->buffer_mutex);
> -	return ret;
> -}
> -
>   static void write_tx_buf_complete(void *context)
>   {
>   	struct mrf24j40 *devrec = context;
> @@ -616,78 +573,6 @@ static int mrf24j40_tx(struct ieee802154_hw *hw, struct sk_buff *skb)
>   	return write_tx_buf(devrec, 0x000, skb->data, skb->len);
>   }
>   
> -static int mrf24j40_read_rx_buf(struct mrf24j40 *devrec,
> -				u8 *data, u8 *len, u8 *lqi)
> -{
> -	u8 rx_len;
> -	u8 addr[2];
> -	u8 lqi_rssi[2];
> -	u16 cmd;
> -	int ret;
> -	struct spi_message msg;
> -	struct spi_transfer addr_xfer = {
> -		.len = 2,
> -		.tx_buf = &addr,
> -	};
> -	struct spi_transfer data_xfer = {
> -		.len = 0x0, /* set below */
> -		.rx_buf = data,
> -	};
> -	struct spi_transfer status_xfer = {
> -		.len = 2,
> -		.rx_buf = &lqi_rssi,
> -	};
> -
> -	/* Get the length of the data in the RX FIFO. The length in this
> -	 * register exclues the 1-byte length field at the beginning. */
> -	ret = read_long_reg(devrec, REG_RX_FIFO, &rx_len);
> -	if (ret)
> -		goto out;
> -
> -	/* Range check the RX FIFO length, accounting for the one-byte
> -	 * length field at the beginning. */
> -	if (rx_len > RX_FIFO_SIZE-1) {
> -		dev_err(printdev(devrec), "Invalid length read from device. Performing short read.\n");
> -		rx_len = RX_FIFO_SIZE-1;
> -	}
> -
> -	if (rx_len > *len) {
> -		/* Passed in buffer wasn't big enough. Should never happen. */
> -		dev_err(printdev(devrec), "Buffer not big enough. Performing short read\n");
> -		rx_len = *len;
> -	}
> -
> -	/* Set up the commands to read the data. */
> -	cmd = MRF24J40_READLONG(REG_RX_FIFO+1);
> -	addr[0] = cmd >> 8 & 0xff;
> -	addr[1] = cmd & 0xff;
> -	data_xfer.len = rx_len;
> -
> -	spi_message_init(&msg);
> -	spi_message_add_tail(&addr_xfer, &msg);
> -	spi_message_add_tail(&data_xfer, &msg);
> -	spi_message_add_tail(&status_xfer, &msg);
> -
> -	ret = spi_sync(devrec->spi, &msg);
> -	if (ret) {
> -		dev_err(printdev(devrec), "SPI RX Buffer Read Failed.\n");
> -		goto out;
> -	}
> -
> -	*lqi = lqi_rssi[0];
> -	*len = rx_len;
> -
> -#ifdef DEBUG
> -	print_hex_dump(KERN_DEBUG, "mrf24j40 rx: ",
> -		       DUMP_PREFIX_OFFSET, 16, 1, data, *len, 0);
> -	pr_debug("mrf24j40 rx: lqi: %02hhx rssi: %02hhx\n",
> -		 lqi_rssi[0], lqi_rssi[1]);
> -#endif
> -

Not adding back this DEBUG block here was on purpose?

> -out:
> -	return ret;
> -}
> -
>   static int mrf24j40_ed(struct ieee802154_hw *hw, u8 *level)
>   {
>   	/* TODO: */
> @@ -823,53 +708,91 @@ static int mrf24j40_filter(struct ieee802154_hw *hw,
>   	return 0;
>   }
>   
> -static int mrf24j40_handle_rx(struct mrf24j40 *devrec)
> +static void mrf24j40_handle_rx_read_buf_unlock(struct mrf24j40 *devrec)
>   {
> -	u8 len = RX_FIFO_SIZE;
> -	u8 lqi = 0;
> -	u8 val;
> -	int ret = 0;
> -	int ret2;
> -	struct sk_buff *skb;
> +	int ret;
>   
> -	/* Turn off reception of packets off the air. This prevents the
> -	 * device from overwriting the buffer while we're reading it. */
> -	ret = read_short_reg(devrec, REG_BBREG1, &val);
> +	/* Turn back on reception of packets off the air. */
> +	devrec->rx_msg.complete = NULL;
> +	devrec->rx_buf[0] = MRF24J40_WRITESHORT(REG_BBREG1);
> +	devrec->rx_buf[1] = 0x00; /* CLR RXDECINV */
> +	ret = spi_async(devrec->spi, &devrec->rx_msg);
>   	if (ret)
> -		goto out;
> -	val |= 4; /* SET RXDECINV */
> -	write_short_reg(devrec, REG_BBREG1, val);
> +		dev_err(printdev(devrec), "failed to unlock rx buffer\n");
> +}
>   
> -	skb = dev_alloc_skb(len);
> +static void mrf24j40_handle_rx_read_buf_complete(void *context)
> +{
> +	struct mrf24j40 *devrec = context;
> +	u8 len = devrec->rx_buf[2];
> +	u8 rx_local_buf[RX_FIFO_SIZE];
> +	struct sk_buff *skb;
> +
> +	memcpy(rx_local_buf, devrec->rx_fifo_buf, len);
> +	mrf24j40_handle_rx_read_buf_unlock(devrec);
> +
> +	skb = dev_alloc_skb(IEEE802154_MTU);
>   	if (!skb) {
> -		ret = -ENOMEM;
> -		goto out;
> +		dev_err(printdev(devrec), "failed to allocate skb\n");
> +		return;
>   	}
>   
> -	ret = mrf24j40_read_rx_buf(devrec, skb_put(skb, len), &len, &lqi);
> -	if (ret < 0) {
> -		dev_err(printdev(devrec), "Failure reading RX FIFO\n");
> -		kfree_skb(skb);
> -		ret = -EINVAL;
> -		goto out;
> +	memcpy(skb_put(skb, len), rx_local_buf, len);
> +	ieee802154_rx_irqsafe(devrec->hw, skb, 0);
> +}
> +
> +static void mrf24j40_handle_rx_read_buf(void *context)
> +{
> +	struct mrf24j40 *devrec = context;
> +	u16 cmd;
> +	int ret;
> +
> +	/* if length is invalid read the full MTU */
> +	if (!ieee802154_is_valid_psdu_len(devrec->rx_buf[2]))
> +		devrec->rx_buf[2] = IEEE802154_MTU;
> +
> +	cmd = MRF24J40_READLONG(REG_RX_FIFO + 1);
> +	devrec->rx_addr_buf[0] = cmd >> 8 & 0xff;
> +	devrec->rx_addr_buf[1] = cmd & 0xff;
> +	devrec->rx_fifo_buf_trx.len = devrec->rx_buf[2];
> +	ret = spi_async(devrec->spi, &devrec->rx_buf_msg);
> +	if (ret) {
> +		dev_err(printdev(devrec), "failed to read rx buffer\n");
> +		mrf24j40_handle_rx_read_buf_unlock(devrec);
>   	}
> +}
> +
> +static void mrf24j40_handle_rx_read_len(void *context)
> +{
> +	struct mrf24j40 *devrec = context;
> +	u16 cmd;
> +	int ret;
>   
> -	/* TODO: Other drivers call ieee20154_rx_irqsafe() here (eg: cc2040,
> -	 * also from a workqueue).  I think irqsafe is not necessary here.
> -	 * Can someone confirm? */
> -	ieee802154_rx_irqsafe(devrec->hw, skb, lqi);
> +	/* read the length of received frame */
> +	devrec->rx_msg.complete = mrf24j40_handle_rx_read_buf;
> +	devrec->rx_trx.len = 3;
> +	cmd = MRF24J40_READLONG(REG_RX_FIFO);
> +	devrec->rx_buf[0] = cmd >> 8 & 0xff;
> +	devrec->rx_buf[1] = cmd & 0xff;
>   
> -	dev_dbg(printdev(devrec), "RX Handled\n");
> +	ret = spi_async(devrec->spi, &devrec->rx_msg);
> +	if (ret) {
> +		dev_err(printdev(devrec), "failed to read rx buffer length\n");
> +		mrf24j40_handle_rx_read_buf_unlock(devrec);
> +	}
> +}
>   
> -out:
> -	/* Turn back on reception of packets off the air. */
> -	ret2 = read_short_reg(devrec, REG_BBREG1, &val);
> -	if (ret2)
> -		return ret2;
> -	val &= ~0x4; /* Clear RXDECINV */
> -	write_short_reg(devrec, REG_BBREG1, val);
> +static int mrf24j40_handle_rx(struct mrf24j40 *devrec)
> +{
> +	/* Turn off reception of packets off the air. This prevents the
> +	 * device from overwriting the buffer while we're reading it.
> +	 */
> +	devrec->rx_msg.complete = mrf24j40_handle_rx_read_len;
> +	devrec->rx_trx.len = 2;
> +	devrec->rx_buf[0] = MRF24J40_WRITESHORT(REG_BBREG1);
> +	devrec->rx_buf[1] = 0x04; /* SET RXDECINV */
>   
> -	return ret;
> +	return spi_async(devrec->spi, &devrec->rx_msg);
>   }
>   
>   static const struct ieee802154_ops mrf24j40_ops = {
> @@ -1025,6 +948,29 @@ mrf24j40_setup_tx_spi_messages(struct mrf24j40 *devrec)
>   	spi_message_add_tail(&devrec->tx_post_trx, &devrec->tx_post_msg);
>   }
>   
> +static void
> +mrf24j40_setup_rx_spi_messages(struct mrf24j40 *devrec)
> +{
> +	spi_message_init(&devrec->rx_msg);
> +	devrec->rx_msg.context = devrec;
> +	devrec->rx_trx.len = 2;
> +	devrec->rx_trx.tx_buf = devrec->rx_buf;
> +	devrec->rx_trx.rx_buf = devrec->rx_buf;
> +	spi_message_add_tail(&devrec->rx_trx, &devrec->rx_msg);
> +
> +	spi_message_init(&devrec->rx_buf_msg);
> +	devrec->rx_buf_msg.context = devrec;
> +	devrec->rx_buf_msg.complete = mrf24j40_handle_rx_read_buf_complete;
> +	devrec->rx_addr_trx.len = 2;
> +	devrec->rx_addr_trx.tx_buf = devrec->rx_addr_buf;
> +	spi_message_add_tail(&devrec->rx_addr_trx, &devrec->rx_buf_msg);
> +	devrec->rx_fifo_buf_trx.rx_buf = devrec->rx_fifo_buf;
> +	spi_message_add_tail(&devrec->rx_fifo_buf_trx, &devrec->rx_buf_msg);
> +	devrec->rx_lqi_trx.len = 2;
> +	devrec->rx_lqi_trx.rx_buf = devrec->rx_lqi_buf;
> +	spi_message_add_tail(&devrec->rx_lqi_trx, &devrec->rx_buf_msg);
> +}
> +
>   static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>   {
>   	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
> @@ -1054,6 +1000,7 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
>   
>   	mrf24j40_setup_tx_spi_messages(devrec);
> +	mrf24j40_setup_rx_spi_messages(devrec);
>   
>   	devrec->regmap_short = devm_regmap_init_spi(spi,
>   						    &mrf24j40_short_regmap);

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 15/21] mrf24j40: async interrupt handling

[Alexander Aring]

This patch removes the threaded irq handling and do a hardirq instead.
We need to switch to spi_async for this step for getting the irq status
register.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 93 +++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 53 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 044d6c6..e992bff 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -181,8 +181,10 @@ struct mrf24j40 {
 	u8 rx_fifo_buf[RX_FIFO_SIZE];
 	struct spi_transfer rx_fifo_buf_trx;
 
-	struct mutex buffer_mutex; /* only used to protect buf */
-	u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */
+	/* isr handling for reading intstat */
+	struct spi_message irq_msg;
+	u8 irq_buf[2];
+	struct spi_transfer irq_trx;
 };
 
 /* regmap information for short address register access */
@@ -486,34 +488,6 @@ static const struct regmap_bus mrf24j40_long_regmap_bus = {
 	.val_format_endian_default = REGMAP_ENDIAN_BIG,
 };
 
-static int read_short_reg(struct mrf24j40 *devrec, u8 reg, u8 *val)
-{
-	int ret = -1;
-	struct spi_message msg;
-	struct spi_transfer xfer = {
-		.len = 2,
-		.tx_buf = devrec->buf,
-		.rx_buf = devrec->buf,
-	};
-
-	spi_message_init(&msg);
-	spi_message_add_tail(&xfer, &msg);
-
-	mutex_lock(&devrec->buffer_mutex);
-	devrec->buf[0] = MRF24J40_READSHORT(reg);
-	devrec->buf[1] = 0;
-
-	ret = spi_sync(devrec->spi, &msg);
-	if (ret)
-		dev_err(printdev(devrec),
-			"SPI read Failed for short register 0x%hhx\n", reg);
-	else
-		*val = devrec->buf[1];
-
-	mutex_unlock(&devrec->buffer_mutex);
-	return ret;
-}
-
 static void write_tx_buf_complete(void *context)
 {
 	struct mrf24j40 *devrec = context;
@@ -805,16 +779,12 @@ static const struct ieee802154_ops mrf24j40_ops = {
 	.set_hw_addr_filt = mrf24j40_filter,
 };
 
-static irqreturn_t mrf24j40_isr(int irq, void *data)
+static void mrf24j40_intstat_complete(void *context)
 {
-	struct mrf24j40 *devrec = data;
-	u8 intstat;
-	int ret;
+	struct mrf24j40 *devrec = context;
+	u8 intstat = devrec->irq_buf[1];
 
-	/* Read the interrupt status */
-	ret = read_short_reg(devrec, REG_INTSTAT, &intstat);
-	if (ret)
-		goto out;
+	enable_irq(devrec->spi->irq);
 
 	/* Check for TX complete */
 	if (intstat & 0x1)
@@ -823,8 +793,23 @@ static irqreturn_t mrf24j40_isr(int irq, void *data)
 	/* Check for Rx */
 	if (intstat & 0x8)
 		mrf24j40_handle_rx(devrec);
+}
+
+static irqreturn_t mrf24j40_isr(int irq, void *data)
+{
+	struct mrf24j40 *devrec = data;
+	int ret;
+
+	disable_irq_nosync(irq);
+
+	devrec->irq_buf[0] = MRF24J40_READSHORT(REG_INTSTAT);
+	/* Read the interrupt status */
+	ret = spi_async(devrec->spi, &devrec->irq_msg);
+	if (ret) {
+		enable_irq(irq);
+		return IRQ_NONE;
+	}
 
-out:
 	return IRQ_HANDLED;
 }
 
@@ -971,6 +956,18 @@ mrf24j40_setup_rx_spi_messages(struct mrf24j40 *devrec)
 	spi_message_add_tail(&devrec->rx_lqi_trx, &devrec->rx_buf_msg);
 }
 
+static void
+mrf24j40_setup_irq_spi_messages(struct mrf24j40 *devrec)
+{
+	spi_message_init(&devrec->irq_msg);
+	devrec->irq_msg.context = devrec;
+	devrec->irq_msg.complete = mrf24j40_intstat_complete;
+	devrec->irq_trx.len = 2;
+	devrec->irq_trx.tx_buf = devrec->irq_buf;
+	devrec->irq_trx.rx_buf = devrec->irq_buf;
+	spi_message_add_tail(&devrec->irq_trx, &devrec->irq_msg);
+}
+
 static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
 {
 	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
@@ -1001,6 +998,7 @@ static int mrf24j40_probe(struct spi_device *spi)
 
 	mrf24j40_setup_tx_spi_messages(devrec);
 	mrf24j40_setup_rx_spi_messages(devrec);
+	mrf24j40_setup_irq_spi_messages(devrec);
 
 	devrec->regmap_short = devm_regmap_init_spi(spi,
 						    &mrf24j40_short_regmap);
@@ -1021,26 +1019,15 @@ static int mrf24j40_probe(struct spi_device *spi)
 		goto err_register_device;
 	}
 
-	devrec->buf = devm_kzalloc(&spi->dev, 3, GFP_KERNEL);
-	if (!devrec->buf)
-		goto err_register_device;
-
-	mutex_init(&devrec->buffer_mutex);
-
 	ret = mrf24j40_hw_init(devrec);
 	if (ret)
 		goto err_register_device;
 
 	mrf24j40_phy_setup(devrec);
 
-	ret = devm_request_threaded_irq(&spi->dev,
-					spi->irq,
-					NULL,
-					mrf24j40_isr,
-					IRQF_TRIGGER_LOW|IRQF_ONESHOT,
-					dev_name(&spi->dev),
-					devrec);
-
+	ret = devm_request_irq(&spi->dev, spi->irq, mrf24j40_isr,
+			       IRQF_TRIGGER_LOW, dev_name(&spi->dev),
+			       devrec);
 	if (ret) {
 		dev_err(printdev(devrec), "Unable to get IRQ");
 		goto err_register_device;
-- 
2.5.0

Re: [RFC bluetooth-next 15/21] mrf24j40: async interrupt handling

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch removes the threaded irq handling and do a hardirq instead.
> We need to switch to spi_async for this step for getting the irq status
> register.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 93 +++++++++++++++++----------------------
>   1 file changed, 40 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 044d6c6..e992bff 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -181,8 +181,10 @@ struct mrf24j40 {
>   	u8 rx_fifo_buf[RX_FIFO_SIZE];
>   	struct spi_transfer rx_fifo_buf_trx;
>   
> -	struct mutex buffer_mutex; /* only used to protect buf */
> -	u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */
> +	/* isr handling for reading intstat */
> +	struct spi_message irq_msg;
> +	u8 irq_buf[2];
> +	struct spi_transfer irq_trx;
>   };
>   
>   /* regmap information for short address register access */
> @@ -486,34 +488,6 @@ static const struct regmap_bus mrf24j40_long_regmap_bus = {
>   	.val_format_endian_default = REGMAP_ENDIAN_BIG,
>   };
>   
> -static int read_short_reg(struct mrf24j40 *devrec, u8 reg, u8 *val)
> -{
> -	int ret = -1;
> -	struct spi_message msg;
> -	struct spi_transfer xfer = {
> -		.len = 2,
> -		.tx_buf = devrec->buf,
> -		.rx_buf = devrec->buf,
> -	};
> -
> -	spi_message_init(&msg);
> -	spi_message_add_tail(&xfer, &msg);
> -
> -	mutex_lock(&devrec->buffer_mutex);
> -	devrec->buf[0] = MRF24J40_READSHORT(reg);
> -	devrec->buf[1] = 0;
> -
> -	ret = spi_sync(devrec->spi, &msg);
> -	if (ret)
> -		dev_err(printdev(devrec),
> -			"SPI read Failed for short register 0x%hhx\n", reg);
> -	else
> -		*val = devrec->buf[1];
> -
> -	mutex_unlock(&devrec->buffer_mutex);
> -	return ret;
> -}
> -
>   static void write_tx_buf_complete(void *context)
>   {
>   	struct mrf24j40 *devrec = context;
> @@ -805,16 +779,12 @@ static const struct ieee802154_ops mrf24j40_ops = {
>   	.set_hw_addr_filt = mrf24j40_filter,
>   };
>   
> -static irqreturn_t mrf24j40_isr(int irq, void *data)
> +static void mrf24j40_intstat_complete(void *context)
>   {
> -	struct mrf24j40 *devrec = data;
> -	u8 intstat;
> -	int ret;
> +	struct mrf24j40 *devrec = context;
> +	u8 intstat = devrec->irq_buf[1];
>   
> -	/* Read the interrupt status */
> -	ret = read_short_reg(devrec, REG_INTSTAT, &intstat);
> -	if (ret)
> -		goto out;
> +	enable_irq(devrec->spi->irq);
>   
>   	/* Check for TX complete */
>   	if (intstat & 0x1)
> @@ -823,8 +793,23 @@ static irqreturn_t mrf24j40_isr(int irq, void *data)
>   	/* Check for Rx */
>   	if (intstat & 0x8)
>   		mrf24j40_handle_rx(devrec);
> +}
> +
> +static irqreturn_t mrf24j40_isr(int irq, void *data)
> +{
> +	struct mrf24j40 *devrec = data;
> +	int ret;
> +
> +	disable_irq_nosync(irq);
> +
> +	devrec->irq_buf[0] = MRF24J40_READSHORT(REG_INTSTAT);
> +	/* Read the interrupt status */
> +	ret = spi_async(devrec->spi, &devrec->irq_msg);
> +	if (ret) {
> +		enable_irq(irq);
> +		return IRQ_NONE;
> +	}
>   
> -out:
>   	return IRQ_HANDLED;
>   }
>   
> @@ -971,6 +956,18 @@ mrf24j40_setup_rx_spi_messages(struct mrf24j40 *devrec)
>   	spi_message_add_tail(&devrec->rx_lqi_trx, &devrec->rx_buf_msg);
>   }
>   
> +static void
> +mrf24j40_setup_irq_spi_messages(struct mrf24j40 *devrec)
> +{
> +	spi_message_init(&devrec->irq_msg);
> +	devrec->irq_msg.context = devrec;
> +	devrec->irq_msg.complete = mrf24j40_intstat_complete;
> +	devrec->irq_trx.len = 2;
> +	devrec->irq_trx.tx_buf = devrec->irq_buf;
> +	devrec->irq_trx.rx_buf = devrec->irq_buf;
> +	spi_message_add_tail(&devrec->irq_trx, &devrec->irq_msg);
> +}
> +
>   static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>   {
>   	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
> @@ -1001,6 +998,7 @@ static int mrf24j40_probe(struct spi_device *spi)
>   
>   	mrf24j40_setup_tx_spi_messages(devrec);
>   	mrf24j40_setup_rx_spi_messages(devrec);
> +	mrf24j40_setup_irq_spi_messages(devrec);
>   
>   	devrec->regmap_short = devm_regmap_init_spi(spi,
>   						    &mrf24j40_short_regmap);
> @@ -1021,26 +1019,15 @@ static int mrf24j40_probe(struct spi_device *spi)
>   		goto err_register_device;
>   	}
>   
> -	devrec->buf = devm_kzalloc(&spi->dev, 3, GFP_KERNEL);
> -	if (!devrec->buf)
> -		goto err_register_device;
> -
> -	mutex_init(&devrec->buffer_mutex);
> -
>   	ret = mrf24j40_hw_init(devrec);
>   	if (ret)
>   		goto err_register_device;
>   
>   	mrf24j40_phy_setup(devrec);
>   
> -	ret = devm_request_threaded_irq(&spi->dev,
> -					spi->irq,
> -					NULL,
> -					mrf24j40_isr,
> -					IRQF_TRIGGER_LOW|IRQF_ONESHOT,
> -					dev_name(&spi->dev),
> -					devrec);
> -
> +	ret = devm_request_irq(&spi->dev, spi->irq, mrf24j40_isr,
> +			       IRQF_TRIGGER_LOW, dev_name(&spi->dev),
> +			       devrec);
>   	if (ret) {
>   		dev_err(printdev(devrec), "Unable to get IRQ");
>   		goto err_register_device;

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 16/21] mrf24j40: add csma params support

[Alexander Aring]

This patch adds supports to change the CSMA parameters. The datasheet
doesn't say anything about max_be value. Seems not configurable and we
assume the 802.15.4 default. But this value must exists because there is
a min_be value.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index e992bff..fdb0b84 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -769,6 +769,26 @@ static int mrf24j40_handle_rx(struct mrf24j40 *devrec)
 	return spi_async(devrec->spi, &devrec->rx_msg);
 }
 
+static int
+mrf24j40_csma_params(struct ieee802154_hw *hw, u8 min_be, u8 max_be,
+		     u8 retries)
+{
+	struct mrf24j40 *devrec = hw->priv;
+	u8 val;
+
+	/* datasheet doesn't say anything about max_be, but we have min_be
+	 * So we assume the max_be default.
+	 */
+	WARN_ON(max_be != 5);
+
+	/* min_be */
+	val = min_be << 3;
+	/* csma backoffs */
+	val |= retries;
+
+	return regmap_update_bits(devrec->regmap_short, REG_TXMCR, 0x1f, val);
+}
+
 static const struct ieee802154_ops mrf24j40_ops = {
 	.owner = THIS_MODULE,
 	.xmit_async = mrf24j40_tx,
@@ -777,6 +797,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
 	.stop = mrf24j40_stop,
 	.set_channel = mrf24j40_set_channel,
 	.set_hw_addr_filt = mrf24j40_filter,
+	.set_csma_params = mrf24j40_csma_params,
 };
 
 static void mrf24j40_intstat_complete(void *context)
@@ -972,6 +993,12 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
 {
 	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
 	devrec->hw->phy->current_channel = 11;
+
+	/* mrf24j40 supports max_minbe 0 - 3 */
+	devrec->hw->phy->supported.max_minbe = 3;
+	/* We assume max_be is 802.15.4 default */
+	devrec->hw->phy->supported.min_maxbe = 5;
+	devrec->hw->phy->supported.max_maxbe = 5;
 }
 
 static int mrf24j40_probe(struct spi_device *spi)
@@ -994,7 +1021,8 @@ static int mrf24j40_probe(struct spi_device *spi)
 	devrec->hw = hw;
 	devrec->hw->parent = &spi->dev;
 	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
-	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
+	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
+			    IEEE802154_HW_CSMA_PARAMS;
 
 	mrf24j40_setup_tx_spi_messages(devrec);
 	mrf24j40_setup_rx_spi_messages(devrec);
-- 
2.5.0

Re: [RFC bluetooth-next 16/21] mrf24j40: add csma params support

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch adds supports to change the CSMA parameters. The datasheet
> doesn't say anything about max_be value. Seems not configurable and we
> assume the 802.15.4 default. But this value must exists because there is
> a min_be value.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 30 +++++++++++++++++++++++++++++-
>   1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index e992bff..fdb0b84 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -769,6 +769,26 @@ static int mrf24j40_handle_rx(struct mrf24j40 *devrec)
>   	return spi_async(devrec->spi, &devrec->rx_msg);
>   }
>   
> +static int
> +mrf24j40_csma_params(struct ieee802154_hw *hw, u8 min_be, u8 max_be,
> +		     u8 retries)
> +{
> +	struct mrf24j40 *devrec = hw->priv;
> +	u8 val;
> +
> +	/* datasheet doesn't say anything about max_be, but we have min_be
> +	 * So we assume the max_be default.
> +	 */
> +	WARN_ON(max_be != 5);
Hmm, the WARN_ON here should never trigger as long as no change is made 
to the driver, right? You advertise min_maxbe and max_maxbe both to 5 
below and this is the only value that could come from userspace. Maybe 
remove the WARN_ON here and move the comment below?

> +
> +	/* min_be */
> +	val = min_be << 3;
> +	/* csma backoffs */
> +	val |= retries;
> +
> +	return regmap_update_bits(devrec->regmap_short, REG_TXMCR, 0x1f, val);
> +}
> +
>   static const struct ieee802154_ops mrf24j40_ops = {
>   	.owner = THIS_MODULE,
>   	.xmit_async = mrf24j40_tx,
> @@ -777,6 +797,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
>   	.stop = mrf24j40_stop,
>   	.set_channel = mrf24j40_set_channel,
>   	.set_hw_addr_filt = mrf24j40_filter,
> +	.set_csma_params = mrf24j40_csma_params,
>   };
>   
>   static void mrf24j40_intstat_complete(void *context)
> @@ -972,6 +993,12 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>   {
>   	ieee802154_random_extended_addr(&devrec->hw->phy->perm_extended_addr);
>   	devrec->hw->phy->current_channel = 11;
> +
> +	/* mrf24j40 supports max_minbe 0 - 3 */
> +	devrec->hw->phy->supported.max_minbe = 3;
> +	/* We assume max_be is 802.15.4 default */
> +	devrec->hw->phy->supported.min_maxbe = 5;
> +	devrec->hw->phy->supported.max_maxbe = 5;
>   }
>   
>   static int mrf24j40_probe(struct spi_device *spi)
> @@ -994,7 +1021,8 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	devrec->hw = hw;
>   	devrec->hw->parent = &spi->dev;
>   	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
> -	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
> +	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
> +			    IEEE802154_HW_CSMA_PARAMS;
>   
>   	mrf24j40_setup_tx_spi_messages(devrec);
>   	mrf24j40_setup_rx_spi_messages(devrec);

regards
Stefan Schmidt

Re: [RFC bluetooth-next 16/21] mrf24j40: add csma params support

[Alexander Aring]

On Thu, Aug 27, 2015 at 03:46:59PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> On 13/08/15 14:22, Alexander Aring wrote:
> >This patch adds supports to change the CSMA parameters. The datasheet
> >doesn't say anything about max_be value. Seems not configurable and we
> >assume the 802.15.4 default. But this value must exists because there is
> >a min_be value.
> >
> >Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> >---
> >  drivers/net/ieee802154/mrf24j40.c | 30 +++++++++++++++++++++++++++++-
> >  1 file changed, 29 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> >index e992bff..fdb0b84 100644
> >--- a/drivers/net/ieee802154/mrf24j40.c
> >+++ b/drivers/net/ieee802154/mrf24j40.c
> >@@ -769,6 +769,26 @@ static int mrf24j40_handle_rx(struct mrf24j40 *devrec)
> >  	return spi_async(devrec->spi, &devrec->rx_msg);
> >  }
> >+static int
> >+mrf24j40_csma_params(struct ieee802154_hw *hw, u8 min_be, u8 max_be,
> >+		     u8 retries)
> >+{
> >+	struct mrf24j40 *devrec = hw->priv;
> >+	u8 val;
> >+
> >+	/* datasheet doesn't say anything about max_be, but we have min_be
> >+	 * So we assume the max_be default.
> >+	 */
> >+	WARN_ON(max_be != 5);
> Hmm, the WARN_ON here should never trigger as long as no change is made to
> the driver, right? You advertise min_maxbe and max_maxbe both to 5 below and
> this is the only value that could come from userspace. Maybe remove the
> WARN_ON here and move the comment below?
> 

ok. I will remove it, it should never happen yes. Of course also move
the comment to the range limits.

- Alex

[RFC bluetooth-next 17/21] mrf24j40: add cca mode support

[Alexander Aring]

This patch supports cca mode handling for mrf24j40.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 40 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index fdb0b84..0fc7628 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -789,6 +789,37 @@ mrf24j40_csma_params(struct ieee802154_hw *hw, u8 min_be, u8 max_be,
 	return regmap_update_bits(devrec->regmap_short, REG_TXMCR, 0x1f, val);
 }
 
+static int mrf24j40_set_cca_mode(struct ieee802154_hw *hw,
+				 const struct wpan_phy_cca *cca)
+{
+	struct mrf24j40 *devrec = hw->priv;
+	u8 val;
+
+	/* mapping 802.15.4 to driver spec */
+	switch (cca->mode) {
+	case NL802154_CCA_ENERGY:
+		val = 2;
+		break;
+	case NL802154_CCA_CARRIER:
+		val = 1;
+		break;
+	case NL802154_CCA_ENERGY_CARRIER:
+		switch (cca->opt) {
+		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
+			val = 3;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(devrec->regmap_short, REG_BBREG2, 0xc0,
+				  val << 6);
+}
+
 static const struct ieee802154_ops mrf24j40_ops = {
 	.owner = THIS_MODULE,
 	.xmit_async = mrf24j40_tx,
@@ -798,6 +829,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
 	.set_channel = mrf24j40_set_channel,
 	.set_hw_addr_filt = mrf24j40_filter,
 	.set_csma_params = mrf24j40_csma_params,
+	.set_cca_mode = mrf24j40_set_cca_mode,
 };
 
 static void mrf24j40_intstat_complete(void *context)
@@ -999,6 +1031,12 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
 	/* We assume max_be is 802.15.4 default */
 	devrec->hw->phy->supported.min_maxbe = 5;
 	devrec->hw->phy->supported.max_maxbe = 5;
+
+	devrec->hw->phy->cca.mode = NL802154_CCA_CARRIER;;
+	devrec->hw->phy->supported.cca_modes = BIT(NL802154_CCA_ENERGY) |
+					       BIT(NL802154_CCA_CARRIER) |
+					       BIT(NL802154_CCA_ENERGY_CARRIER);
+	devrec->hw->phy->supported.cca_opts = BIT(NL802154_CCA_OPT_ENERGY_CARRIER_AND);
 }
 
 static int mrf24j40_probe(struct spi_device *spi)
@@ -1024,6 +1062,8 @@ static int mrf24j40_probe(struct spi_device *spi)
 	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
 			    IEEE802154_HW_CSMA_PARAMS;
 
+	devrec->hw->phy->flags = WPAN_PHY_FLAG_CCA_MODE;
+
 	mrf24j40_setup_tx_spi_messages(devrec);
 	mrf24j40_setup_rx_spi_messages(devrec);
 	mrf24j40_setup_irq_spi_messages(devrec);
-- 
2.5.0

Re: [RFC bluetooth-next 17/21] mrf24j40: add cca mode support

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch supports cca mode handling for mrf24j40.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 40 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index fdb0b84..0fc7628 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -789,6 +789,37 @@ mrf24j40_csma_params(struct ieee802154_hw *hw, u8 min_be, u8 max_be,
>   	return regmap_update_bits(devrec->regmap_short, REG_TXMCR, 0x1f, val);
>   }
>   
> +static int mrf24j40_set_cca_mode(struct ieee802154_hw *hw,
> +				 const struct wpan_phy_cca *cca)
> +{
> +	struct mrf24j40 *devrec = hw->priv;
> +	u8 val;
> +
> +	/* mapping 802.15.4 to driver spec */
> +	switch (cca->mode) {
> +	case NL802154_CCA_ENERGY:
> +		val = 2;
> +		break;
> +	case NL802154_CCA_CARRIER:
> +		val = 1;
> +		break;
> +	case NL802154_CCA_ENERGY_CARRIER:
> +		switch (cca->opt) {
> +		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> +			val = 3;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return regmap_update_bits(devrec->regmap_short, REG_BBREG2, 0xc0,
> +				  val << 6);

Something I spotted here but also in other patches. These magic values 
here are a bit annoying. Maybe we could get some more meaningful defines 
for them

> +}
> +
>   static const struct ieee802154_ops mrf24j40_ops = {
>   	.owner = THIS_MODULE,
>   	.xmit_async = mrf24j40_tx,
> @@ -798,6 +829,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
>   	.set_channel = mrf24j40_set_channel,
>   	.set_hw_addr_filt = mrf24j40_filter,
>   	.set_csma_params = mrf24j40_csma_params,
> +	.set_cca_mode = mrf24j40_set_cca_mode,
>   };
>   
>   static void mrf24j40_intstat_complete(void *context)
> @@ -999,6 +1031,12 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>   	/* We assume max_be is 802.15.4 default */
>   	devrec->hw->phy->supported.min_maxbe = 5;
>   	devrec->hw->phy->supported.max_maxbe = 5;
> +
> +	devrec->hw->phy->cca.mode = NL802154_CCA_CARRIER;;
> +	devrec->hw->phy->supported.cca_modes = BIT(NL802154_CCA_ENERGY) |
> +					       BIT(NL802154_CCA_CARRIER) |
> +					       BIT(NL802154_CCA_ENERGY_CARRIER);
> +	devrec->hw->phy->supported.cca_opts = BIT(NL802154_CCA_OPT_ENERGY_CARRIER_AND);
>   }
>   
>   static int mrf24j40_probe(struct spi_device *spi)
> @@ -1024,6 +1062,8 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
>   			    IEEE802154_HW_CSMA_PARAMS;
>   
> +	devrec->hw->phy->flags = WPAN_PHY_FLAG_CCA_MODE;
> +
>   	mrf24j40_setup_tx_spi_messages(devrec);
>   	mrf24j40_setup_rx_spi_messages(devrec);
>   	mrf24j40_setup_irq_spi_messages(devrec);

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

Re: [RFC bluetooth-next 17/21] mrf24j40: add cca mode support

[Alexander Aring]

On Thu, Aug 13, 2015 at 02:22:44PM +0200, Alexander Aring wrote:
> This patch supports cca mode handling for mrf24j40.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  drivers/net/ieee802154/mrf24j40.c | 40 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index fdb0b84..0fc7628 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -789,6 +789,37 @@ mrf24j40_csma_params(struct ieee802154_hw *hw, u8 min_be, u8 max_be,
>  	return regmap_update_bits(devrec->regmap_short, REG_TXMCR, 0x1f, val);
>  }
>  
> +static int mrf24j40_set_cca_mode(struct ieee802154_hw *hw,
> +				 const struct wpan_phy_cca *cca)
> +{
> +	struct mrf24j40 *devrec = hw->priv;
> +	u8 val;
> +
> +	/* mapping 802.15.4 to driver spec */
> +	switch (cca->mode) {
> +	case NL802154_CCA_ENERGY:
> +		val = 2;
> +		break;
> +	case NL802154_CCA_CARRIER:
> +		val = 1;
> +		break;
> +	case NL802154_CCA_ENERGY_CARRIER:
> +		switch (cca->opt) {
> +		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:

changed NL802154_CCA_OPT_ENERGY_CARRIER_OR to
NL802154_CCA_OPT_ENERGY_CARRIER_AND.

Reason see below.

> +			val = 3;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return regmap_update_bits(devrec->regmap_short, REG_BBREG2, 0xc0,
> +				  val << 6);
> +}
> +
>  static const struct ieee802154_ops mrf24j40_ops = {
>  	.owner = THIS_MODULE,
>  	.xmit_async = mrf24j40_tx,
> @@ -798,6 +829,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
>  	.set_channel = mrf24j40_set_channel,
>  	.set_hw_addr_filt = mrf24j40_filter,
>  	.set_csma_params = mrf24j40_csma_params,
> +	.set_cca_mode = mrf24j40_set_cca_mode,
>  };
>  
>  static void mrf24j40_intstat_complete(void *context)
> @@ -999,6 +1031,12 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>  	/* We assume max_be is 802.15.4 default */
>  	devrec->hw->phy->supported.min_maxbe = 5;
>  	devrec->hw->phy->supported.max_maxbe = 5;
> +
> +	devrec->hw->phy->cca.mode = NL802154_CCA_CARRIER;;
> +	devrec->hw->phy->supported.cca_modes = BIT(NL802154_CCA_ENERGY) |
> +					       BIT(NL802154_CCA_CARRIER) |
> +					       BIT(NL802154_CCA_ENERGY_CARRIER);
> +	devrec->hw->phy->supported.cca_opts = BIT(NL802154_CCA_OPT_ENERGY_CARRIER_AND);

We set "NL802154_CCA_OPT_ENERGY_CARRIER_AND" here, I was not sure about
that and had "NL802154_CCA_OPT_ENERGY_CARRIER_OR" before.

I think it's "and" because there is a "and" in the explanation and no
"or".

"CCA Mode 3: Carrier sense with energy above threshold. CCA shall report
a busy medium only upon the detection of a signal with the modulation and
spreading characteristics of IEEE 802.15.4 with energy above the energy
Detection (ED) threshold."

I mean "...modulation and spreading...", otherwise I don't know if it's
or XOR and, but it must one of them.

- Alex

[RFC bluetooth-next 18/21] mrf24j40: add cca ed level support

[Alexander Aring]

This patch supports handling to set the cca energy detection level for
the mrf24j40 transceiver.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 51 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 0fc7628..00fd97e 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -820,6 +820,49 @@ static int mrf24j40_set_cca_mode(struct ieee802154_hw *hw,
 				  val << 6);
 }
 
+/* array for representing ed levels */
+static const s32 mrf24j40_ed_levels[] = {
+	-9000, -8900, -8800, -8700, -8600, -8500, -8400, -8300, -8200, -8100,
+	-8000, -7900, -7800, -7700, -7600, -7500, -7400, -7300, -7200, -7100,
+	-7000, -6900, -6800, -6700, -6600, -6500, -6400, -6300, -6200, -6100,
+	-6000, -5900, -5800, -5700, -5600, -5500, -5400, -5300, -5200, -5100,
+	-5000, -4900, -4800, -4700, -4600, -4500, -4400, -4300, -4200, -4100,
+	-4000, -3900, -3800, -3700, -3600, -3500
+};
+
+/* map ed levels to register value */
+static const s32 mrf24j40_ed_levels_map[][2] = {
+	{ -9000, 0 }, { -8900, 1 }, { -8800, 2 }, { -8700, 5 }, { -8600, 9 },
+	{ -8500, 13 }, { -8400, 18 }, { -8300, 23 }, { -8200, 27 },
+	{ -8100, 32 }, { -8000, 37 }, { -7900, 43 }, { -7800, 48 },
+	{ -7700, 53 }, { -7600, 58 }, { -7500, 63 }, { -7400, 68 },
+	{ -7300, 73 }, { -7200, 78 }, { -7100, 83 }, { -7000, 89 },
+	{ -6900, 95 }, { -6800, 100 }, { -6700, 107 }, { -6600, 111 },
+	{ -6500, 117 }, { -6400, 121 }, { -6300, 125 }, { -6200, 129 },
+	{ -6100, 133 },	{ -6000, 138 }, { -5900, 143 }, { -5800, 148 },
+	{ -5700, 153 }, { -5600, 159 },	{ -5500, 165 }, { -5400, 170 },
+	{ -5300, 176 }, { -5200, 183 }, { -5100, 188 }, { -5000, 193 },
+	{ -4900, 198 }, { -4800, 203 }, { -4700, 207 }, { -4600, 212 },
+	{ -4500, 216 }, { -4400, 221 }, { -4300, 225 }, { -4200, 228 },
+	{ -4100, 233 }, { -4000, 239 }, { -3900, 245 }, { -3800, 250 },
+	{ -3700, 253 }, { -3600, 254 }, { -3500, 255 },
+};
+
+static int mrf24j40_set_cca_ed_level(struct ieee802154_hw *hw, s32 mbm)
+{
+	struct mrf24j40 *devrec = hw->priv;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mrf24j40_ed_levels_map); i++) {
+		if (mrf24j40_ed_levels_map[i][0] == mbm)
+			return regmap_update_bits(devrec->regmap_short,
+						  REG_CCAEDTH, 0xff,
+						  mrf24j40_ed_levels_map[i][1]);
+	}
+
+	return -EINVAL;
+}
+
 static const struct ieee802154_ops mrf24j40_ops = {
 	.owner = THIS_MODULE,
 	.xmit_async = mrf24j40_tx,
@@ -830,6 +873,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
 	.set_hw_addr_filt = mrf24j40_filter,
 	.set_csma_params = mrf24j40_csma_params,
 	.set_cca_mode = mrf24j40_set_cca_mode,
+	.set_cca_ed_level = mrf24j40_set_cca_ed_level,
 };
 
 static void mrf24j40_intstat_complete(void *context)
@@ -1037,6 +1081,10 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
 					       BIT(NL802154_CCA_CARRIER) |
 					       BIT(NL802154_CCA_ENERGY_CARRIER);
 	devrec->hw->phy->supported.cca_opts = BIT(NL802154_CCA_OPT_ENERGY_CARRIER_AND);
+
+	devrec->hw->phy->cca_ed_level = -6900;
+	devrec->hw->phy->supported.cca_ed_levels = mrf24j40_ed_levels;
+	devrec->hw->phy->supported.cca_ed_levels_size = ARRAY_SIZE(mrf24j40_ed_levels);
 }
 
 static int mrf24j40_probe(struct spi_device *spi)
@@ -1062,7 +1110,8 @@ static int mrf24j40_probe(struct spi_device *spi)
 	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
 			    IEEE802154_HW_CSMA_PARAMS;
 
-	devrec->hw->phy->flags = WPAN_PHY_FLAG_CCA_MODE;
+	devrec->hw->phy->flags = WPAN_PHY_FLAG_CCA_MODE |
+				 WPAN_PHY_FLAG_CCA_ED_LEVEL;
 
 	mrf24j40_setup_tx_spi_messages(devrec);
 	mrf24j40_setup_rx_spi_messages(devrec);
-- 
2.5.0

Re: [RFC bluetooth-next 18/21] mrf24j40: add cca ed level support

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch supports handling to set the cca energy detection level for
> the mrf24j40 transceiver.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 51 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 0fc7628..00fd97e 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -820,6 +820,49 @@ static int mrf24j40_set_cca_mode(struct ieee802154_hw *hw,
>   				  val << 6);
>   }
>   
> +/* array for representing ed levels */
> +static const s32 mrf24j40_ed_levels[] = {
> +	-9000, -8900, -8800, -8700, -8600, -8500, -8400, -8300, -8200, -8100,
> +	-8000, -7900, -7800, -7700, -7600, -7500, -7400, -7300, -7200, -7100,
> +	-7000, -6900, -6800, -6700, -6600, -6500, -6400, -6300, -6200, -6100,
> +	-6000, -5900, -5800, -5700, -5600, -5500, -5400, -5300, -5200, -5100,
> +	-5000, -4900, -4800, -4700, -4600, -4500, -4400, -4300, -4200, -4100,
> +	-4000, -3900, -3800, -3700, -3600, -3500
> +};
> +
> +/* map ed levels to register value */
> +static const s32 mrf24j40_ed_levels_map[][2] = {
> +	{ -9000, 0 }, { -8900, 1 }, { -8800, 2 }, { -8700, 5 }, { -8600, 9 },
> +	{ -8500, 13 }, { -8400, 18 }, { -8300, 23 }, { -8200, 27 },
> +	{ -8100, 32 }, { -8000, 37 }, { -7900, 43 }, { -7800, 48 },
> +	{ -7700, 53 }, { -7600, 58 }, { -7500, 63 }, { -7400, 68 },
> +	{ -7300, 73 }, { -7200, 78 }, { -7100, 83 }, { -7000, 89 },
> +	{ -6900, 95 }, { -6800, 100 }, { -6700, 107 }, { -6600, 111 },
> +	{ -6500, 117 }, { -6400, 121 }, { -6300, 125 }, { -6200, 129 },
> +	{ -6100, 133 },	{ -6000, 138 }, { -5900, 143 }, { -5800, 148 },
> +	{ -5700, 153 }, { -5600, 159 },	{ -5500, 165 }, { -5400, 170 },
> +	{ -5300, 176 }, { -5200, 183 }, { -5100, 188 }, { -5000, 193 },
> +	{ -4900, 198 }, { -4800, 203 }, { -4700, 207 }, { -4600, 212 },
> +	{ -4500, 216 }, { -4400, 221 }, { -4300, 225 }, { -4200, 228 },
> +	{ -4100, 233 }, { -4000, 239 }, { -3900, 245 }, { -3800, 250 },
> +	{ -3700, 253 }, { -3600, 254 }, { -3500, 255 },
> +};
> +
> +static int mrf24j40_set_cca_ed_level(struct ieee802154_hw *hw, s32 mbm)
> +{
> +	struct mrf24j40 *devrec = hw->priv;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(mrf24j40_ed_levels_map); i++) {
> +		if (mrf24j40_ed_levels_map[i][0] == mbm)
> +			return regmap_update_bits(devrec->regmap_short,
> +						  REG_CCAEDTH, 0xff,
> +						  mrf24j40_ed_levels_map[i][1]);
> +	}
> +
> +	return -EINVAL;
> +}
> +
>   static const struct ieee802154_ops mrf24j40_ops = {
>   	.owner = THIS_MODULE,
>   	.xmit_async = mrf24j40_tx,
> @@ -830,6 +873,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
>   	.set_hw_addr_filt = mrf24j40_filter,
>   	.set_csma_params = mrf24j40_csma_params,
>   	.set_cca_mode = mrf24j40_set_cca_mode,
> +	.set_cca_ed_level = mrf24j40_set_cca_ed_level,
>   };
>   
>   static void mrf24j40_intstat_complete(void *context)
> @@ -1037,6 +1081,10 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>   					       BIT(NL802154_CCA_CARRIER) |
>   					       BIT(NL802154_CCA_ENERGY_CARRIER);
>   	devrec->hw->phy->supported.cca_opts = BIT(NL802154_CCA_OPT_ENERGY_CARRIER_AND);
> +
> +	devrec->hw->phy->cca_ed_level = -6900;

-6900 is the default on chip reset?

> +	devrec->hw->phy->supported.cca_ed_levels = mrf24j40_ed_levels;
> +	devrec->hw->phy->supported.cca_ed_levels_size = ARRAY_SIZE(mrf24j40_ed_levels);
>   }
>   
>   static int mrf24j40_probe(struct spi_device *spi)
> @@ -1062,7 +1110,8 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
>   			    IEEE802154_HW_CSMA_PARAMS;
>   
> -	devrec->hw->phy->flags = WPAN_PHY_FLAG_CCA_MODE;
> +	devrec->hw->phy->flags = WPAN_PHY_FLAG_CCA_MODE |
> +				 WPAN_PHY_FLAG_CCA_ED_LEVEL;
>   
>   	mrf24j40_setup_tx_spi_messages(devrec);
>   	mrf24j40_setup_rx_spi_messages(devrec);

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

Re: [RFC bluetooth-next 18/21] mrf24j40: add cca ed level support

[Alexander Aring]

On Thu, Aug 13, 2015 at 02:22:45PM +0200, Alexander Aring wrote:
> This patch supports handling to set the cca energy detection level for
> the mrf24j40 transceiver.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  drivers/net/ieee802154/mrf24j40.c | 51 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 0fc7628..00fd97e 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -820,6 +820,49 @@ static int mrf24j40_set_cca_mode(struct ieee802154_hw *hw,
>  				  val << 6);
>  }
>  
> +/* array for representing ed levels */
> +static const s32 mrf24j40_ed_levels[] = {
> +	-9000, -8900, -8800, -8700, -8600, -8500, -8400, -8300, -8200, -8100,
> +	-8000, -7900, -7800, -7700, -7600, -7500, -7400, -7300, -7200, -7100,
> +	-7000, -6900, -6800, -6700, -6600, -6500, -6400, -6300, -6200, -6100,
> +	-6000, -5900, -5800, -5700, -5600, -5500, -5400, -5300, -5200, -5100,
> +	-5000, -4900, -4800, -4700, -4600, -4500, -4400, -4300, -4200, -4100,
> +	-4000, -3900, -3800, -3700, -3600, -3500
> +};
> +
> +/* map ed levels to register value */
> +static const s32 mrf24j40_ed_levels_map[][2] = {
> +	{ -9000, 0 }, { -8900, 1 }, { -8800, 2 }, { -8700, 5 }, { -8600, 9 },
> +	{ -8500, 13 }, { -8400, 18 }, { -8300, 23 }, { -8200, 27 },
> +	{ -8100, 32 }, { -8000, 37 }, { -7900, 43 }, { -7800, 48 },
> +	{ -7700, 53 }, { -7600, 58 }, { -7500, 63 }, { -7400, 68 },
> +	{ -7300, 73 }, { -7200, 78 }, { -7100, 83 }, { -7000, 89 },
> +	{ -6900, 95 }, { -6800, 100 }, { -6700, 107 }, { -6600, 111 },
> +	{ -6500, 117 }, { -6400, 121 }, { -6300, 125 }, { -6200, 129 },
> +	{ -6100, 133 },	{ -6000, 138 }, { -5900, 143 }, { -5800, 148 },
> +	{ -5700, 153 }, { -5600, 159 },	{ -5500, 165 }, { -5400, 170 },
> +	{ -5300, 176 }, { -5200, 183 }, { -5100, 188 }, { -5000, 193 },
> +	{ -4900, 198 }, { -4800, 203 }, { -4700, 207 }, { -4600, 212 },
> +	{ -4500, 216 }, { -4400, 221 }, { -4300, 225 }, { -4200, 228 },
> +	{ -4100, 233 }, { -4000, 239 }, { -3900, 245 }, { -3800, 250 },
> +	{ -3700, 253 }, { -3600, 254 }, { -3500, 255 },
> +};
> +
> +static int mrf24j40_set_cca_ed_level(struct ieee802154_hw *hw, s32 mbm)
> +{
> +	struct mrf24j40 *devrec = hw->priv;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(mrf24j40_ed_levels_map); i++) {
> +		if (mrf24j40_ed_levels_map[i][0] == mbm)
> +			return regmap_update_bits(devrec->regmap_short,
> +						  REG_CCAEDTH, 0xff,
> +						  mrf24j40_ed_levels_map[i][1]);

I changed this function from regmap_update_bits to regmap_write, the
mask is "0xff" here and then regmap_write is enough.

- Alex

[RFC bluetooth-next 19/21] mrf24j40: add tx power support

[Alexander Aring]

This patch supports setting of transmit power for the mrf24j40ma
transceiver only. The mrf24j40mc has some amplifier to change the
transmit power, I am currently not sure how the mapping for this
amplifier looks like.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 70 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 00fd97e..e9a03fd 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -863,6 +863,65 @@ static int mrf24j40_set_cca_ed_level(struct ieee802154_hw *hw, s32 mbm)
 	return -EINVAL;
 }
 
+static const s32 mrf24j40ma_powers[] = {
+	0, -50, -120, -190, -280, -370, -490, -630, -1000, -1050, -1120, -1190,
+	-1280, -1370, -1490, -1630, -2000, -2050, -2120, -2190, -2280, -2370,
+	-2490, -2630, -3000, -3050, -3120, -3190, -3280, -3370, -3490, -3630,
+};
+
+static int mrf24j40_set_txpower(struct ieee802154_hw *hw, s32 mbm)
+{
+	struct mrf24j40 *devrec = hw->priv;
+	s32 small_scale;
+	u8 val;
+
+	if (0 >= mbm && mbm > -1000) {
+		val = 0;
+		small_scale = mbm;
+	} else if (-1000 >= mbm && mbm > -2000) {
+		val = 0x40;
+		small_scale = mbm + 1000;
+	} else if (-2000 >= mbm && mbm > -3000) {
+		val = 0x80;
+		small_scale = mbm + 2000;
+	} else if (-3000 >= mbm && mbm > -4000) {
+		val = 0xc0;
+		small_scale = mbm + 3000;
+	} else {
+		return -EINVAL;
+	}
+
+	switch (small_scale) {
+	case 0:
+		break;
+	case -50:
+		val |= 0x08;
+		break;
+	case -120:
+		val |= 0x10;
+		break;
+	case -190:
+		val |= 0x18;
+		break;
+	case -280:
+		val |= 0x20;
+		break;
+	case -370:
+		val |= 0x28;
+		break;
+	case -490:
+		val |= 0x30;
+		break;
+	case -630:
+		val |= 0x38;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(devrec->regmap_long, REG_RFCON3, 0xf8, val);
+}
+
 static const struct ieee802154_ops mrf24j40_ops = {
 	.owner = THIS_MODULE,
 	.xmit_async = mrf24j40_tx,
@@ -874,6 +933,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
 	.set_csma_params = mrf24j40_csma_params,
 	.set_cca_mode = mrf24j40_set_cca_mode,
 	.set_cca_ed_level = mrf24j40_set_cca_ed_level,
+	.set_txpower = mrf24j40_set_txpower,
 };
 
 static void mrf24j40_intstat_complete(void *context)
@@ -1085,6 +1145,16 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
 	devrec->hw->phy->cca_ed_level = -6900;
 	devrec->hw->phy->supported.cca_ed_levels = mrf24j40_ed_levels;
 	devrec->hw->phy->supported.cca_ed_levels_size = ARRAY_SIZE(mrf24j40_ed_levels);
+
+	switch (spi_get_device_id(devrec->spi)->driver_data) {
+	case MRF24J40MA:
+		devrec->hw->phy->supported.tx_powers = mrf24j40ma_powers;
+		devrec->hw->phy->supported.tx_powers_size = ARRAY_SIZE(mrf24j40ma_powers);
+		devrec->hw->phy->flags |= WPAN_PHY_FLAG_TXPOWER;
+		break;
+	default:
+		break;
+	}
 }
 
 static int mrf24j40_probe(struct spi_device *spi)
-- 
2.5.0

Re: [RFC bluetooth-next 19/21] mrf24j40: add tx power support

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch supports setting of transmit power for the mrf24j40ma
> transceiver only. The mrf24j40mc has some amplifier to change the
> transmit power, I am currently not sure how the mapping for this
> amplifier looks like.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 70 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 70 insertions(+)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 00fd97e..e9a03fd 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -863,6 +863,65 @@ static int mrf24j40_set_cca_ed_level(struct ieee802154_hw *hw, s32 mbm)
>   	return -EINVAL;
>   }
>   
> +static const s32 mrf24j40ma_powers[] = {
> +	0, -50, -120, -190, -280, -370, -490, -630, -1000, -1050, -1120, -1190,
> +	-1280, -1370, -1490, -1630, -2000, -2050, -2120, -2190, -2280, -2370,
> +	-2490, -2630, -3000, -3050, -3120, -3190, -3280, -3370, -3490, -3630,
> +};
> +
> +static int mrf24j40_set_txpower(struct ieee802154_hw *hw, s32 mbm)
> +{
> +	struct mrf24j40 *devrec = hw->priv;
> +	s32 small_scale;
> +	u8 val;
> +
> +	if (0 >= mbm && mbm > -1000) {
> +		val = 0;
> +		small_scale = mbm;
> +	} else if (-1000 >= mbm && mbm > -2000) {
> +		val = 0x40;
> +		small_scale = mbm + 1000;
> +	} else if (-2000 >= mbm && mbm > -3000) {
> +		val = 0x80;
> +		small_scale = mbm + 2000;
> +	} else if (-3000 >= mbm && mbm > -4000) {
> +		val = 0xc0;
> +		small_scale = mbm + 3000;
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	switch (small_scale) {
> +	case 0:
> +		break;
> +	case -50:
> +		val |= 0x08;
> +		break;
> +	case -120:
> +		val |= 0x10;
> +		break;
> +	case -190:
> +		val |= 0x18;
> +		break;
> +	case -280:
> +		val |= 0x20;
> +		break;
> +	case -370:
> +		val |= 0x28;
> +		break;
> +	case -490:
> +		val |= 0x30;
> +		break;
> +	case -630:
> +		val |= 0x38;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return regmap_update_bits(devrec->regmap_long, REG_RFCON3, 0xf8, val);
> +}
> +
>   static const struct ieee802154_ops mrf24j40_ops = {
>   	.owner = THIS_MODULE,
>   	.xmit_async = mrf24j40_tx,
> @@ -874,6 +933,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
>   	.set_csma_params = mrf24j40_csma_params,
>   	.set_cca_mode = mrf24j40_set_cca_mode,
>   	.set_cca_ed_level = mrf24j40_set_cca_ed_level,
> +	.set_txpower = mrf24j40_set_txpower,
>   };
>   
>   static void mrf24j40_intstat_complete(void *context)
> @@ -1085,6 +1145,16 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>   	devrec->hw->phy->cca_ed_level = -6900;
>   	devrec->hw->phy->supported.cca_ed_levels = mrf24j40_ed_levels;
>   	devrec->hw->phy->supported.cca_ed_levels_size = ARRAY_SIZE(mrf24j40_ed_levels);
> +
> +	switch (spi_get_device_id(devrec->spi)->driver_data) {
> +	case MRF24J40MA:

Hmm, would these not also be valid for MRF24J40? To me it seems on the 
the MRF24J40C case would be different here due to the amplifier.

> +		devrec->hw->phy->supported.tx_powers = mrf24j40ma_powers;
> +		devrec->hw->phy->supported.tx_powers_size = ARRAY_SIZE(mrf24j40ma_powers);
> +		devrec->hw->phy->flags |= WPAN_PHY_FLAG_TXPOWER;
> +		break;
> +	default:
> +		break;
> +	}
>   }
>   
>   static int mrf24j40_probe(struct spi_device *spi)

regards
Stefan Schmidt

[RFC bluetooth-next 20/21] mrf24j40: add promiscuous mode support

[Alexander Aring]

This patch adds support for promiscuous mode by setting promiscuous (no
frame filtering), disable automatic ack handling and not filtering
frames where the crc is invalid.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index e9a03fd..560db88 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -922,6 +922,24 @@ static int mrf24j40_set_txpower(struct ieee802154_hw *hw, s32 mbm)
 	return regmap_update_bits(devrec->regmap_long, REG_RFCON3, 0xf8, val);
 }
 
+static int mrf24j40_set_promiscuous_mode(struct ieee802154_hw *hw, bool on)
+{
+	struct mrf24j40 *devrec = hw->priv;
+	int ret;
+
+	if (on) {
+		/* set PROMI, ERRPKT and NOACKRSP */
+		ret = regmap_update_bits(devrec->regmap_short, REG_RXMCR, 0x23,
+					 0x23);
+	} else {
+		/* clear PROMI, ERRPKT and NOACKRSP */
+		ret = regmap_update_bits(devrec->regmap_short, REG_RXMCR, 0x23,
+					 0x00);
+	}
+
+	return ret;
+}
+
 static const struct ieee802154_ops mrf24j40_ops = {
 	.owner = THIS_MODULE,
 	.xmit_async = mrf24j40_tx,
@@ -934,6 +952,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
 	.set_cca_mode = mrf24j40_set_cca_mode,
 	.set_cca_ed_level = mrf24j40_set_cca_ed_level,
 	.set_txpower = mrf24j40_set_txpower,
+	.set_promiscuous_mode = mrf24j40_set_promiscuous_mode,
 };
 
 static void mrf24j40_intstat_complete(void *context)
@@ -1178,7 +1197,8 @@ static int mrf24j40_probe(struct spi_device *spi)
 	devrec->hw->parent = &spi->dev;
 	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
 	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
-			    IEEE802154_HW_CSMA_PARAMS;
+			    IEEE802154_HW_CSMA_PARAMS |
+			    IEEE802154_HW_PROMISCUOUS;
 
 	devrec->hw->phy->flags = WPAN_PHY_FLAG_CCA_MODE |
 				 WPAN_PHY_FLAG_CCA_ED_LEVEL;
-- 
2.5.0

Re: [RFC bluetooth-next 20/21] mrf24j40: add promiscuous mode support

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch adds support for promiscuous mode by setting promiscuous (no
> frame filtering), disable automatic ack handling and not filtering
> frames where the crc is invalid.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 22 +++++++++++++++++++++-
>   1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index e9a03fd..560db88 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -922,6 +922,24 @@ static int mrf24j40_set_txpower(struct ieee802154_hw *hw, s32 mbm)
>   	return regmap_update_bits(devrec->regmap_long, REG_RFCON3, 0xf8, val);
>   }
>   
> +static int mrf24j40_set_promiscuous_mode(struct ieee802154_hw *hw, bool on)
> +{
> +	struct mrf24j40 *devrec = hw->priv;
> +	int ret;
> +
> +	if (on) {
> +		/* set PROMI, ERRPKT and NOACKRSP */
> +		ret = regmap_update_bits(devrec->regmap_short, REG_RXMCR, 0x23,
> +					 0x23);
> +	} else {
> +		/* clear PROMI, ERRPKT and NOACKRSP */
> +		ret = regmap_update_bits(devrec->regmap_short, REG_RXMCR, 0x23,
> +					 0x00);

More magic numbers.

> +	}
> +
> +	return ret;
> +}
> +
>   static const struct ieee802154_ops mrf24j40_ops = {
>   	.owner = THIS_MODULE,
>   	.xmit_async = mrf24j40_tx,
> @@ -934,6 +952,7 @@ static const struct ieee802154_ops mrf24j40_ops = {
>   	.set_cca_mode = mrf24j40_set_cca_mode,
>   	.set_cca_ed_level = mrf24j40_set_cca_ed_level,
>   	.set_txpower = mrf24j40_set_txpower,
> +	.set_promiscuous_mode = mrf24j40_set_promiscuous_mode,
>   };
>   
>   static void mrf24j40_intstat_complete(void *context)
> @@ -1178,7 +1197,8 @@ static int mrf24j40_probe(struct spi_device *spi)
>   	devrec->hw->parent = &spi->dev;
>   	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
>   	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
> -			    IEEE802154_HW_CSMA_PARAMS;
> +			    IEEE802154_HW_CSMA_PARAMS |
> +			    IEEE802154_HW_PROMISCUOUS;
>   
>   	devrec->hw->phy->flags = WPAN_PHY_FLAG_CCA_MODE |
>   				 WPAN_PHY_FLAG_CCA_ED_LEVEL;

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[RFC bluetooth-next 21/21] mrf24j40: change irq trigger type behaviour

[Alexander Aring]

This patch changes the irq trigger type value while calling
devm_request_irq by using IRQF_TRIGGER_LOW when no irq type was given.
Additional we add support for change the irq polarity while hw init if
high level or rising edge triggered irq type are given.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/mrf24j40.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 560db88..7bd540a 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/ieee802154.h>
+#include <linux/irq.h>
 #include <net/cfg802154.h>
 #include <net/mac802154.h>
 
@@ -1082,6 +1083,20 @@ static int mrf24j40_hw_init(struct mrf24j40 *devrec)
 		regmap_write(devrec->regmap_long, REG_RFCON3, 0x28);
 	}
 
+	switch (irq_get_trigger_type(devrec->spi->irq)) {
+	case IRQ_TYPE_EDGE_RISING:
+	case IRQ_TYPE_LEVEL_HIGH:
+		/* set interrupt polarity to rising */
+		ret = regmap_update_bits(devrec->regmap_long, REG_SLPCON0,
+					 0x02, 0x02);
+		if (ret)
+			goto err_ret;
+		break;
+	default:
+		/* default is falling edge */
+		break;
+	}
+
 	return 0;
 
 err_ret:
@@ -1178,7 +1193,7 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
 
 static int mrf24j40_probe(struct spi_device *spi)
 {
-	int ret = -ENOMEM;
+	int ret = -ENOMEM, irq_type;
 	struct ieee802154_hw *hw;
 	struct mrf24j40 *devrec;
 
@@ -1232,9 +1247,13 @@ static int mrf24j40_probe(struct spi_device *spi)
 
 	mrf24j40_phy_setup(devrec);
 
+	/* request IRQF_TRIGGER_LOW as fallback default */
+	irq_type = irq_get_trigger_type(spi->irq);
+	if (!irq_type)
+		irq_type = IRQF_TRIGGER_LOW;
+
 	ret = devm_request_irq(&spi->dev, spi->irq, mrf24j40_isr,
-			       IRQF_TRIGGER_LOW, dev_name(&spi->dev),
-			       devrec);
+			       irq_type, dev_name(&spi->dev), devrec);
 	if (ret) {
 		dev_err(printdev(devrec), "Unable to get IRQ");
 		goto err_register_device;
-- 
2.5.0

Re: [RFC bluetooth-next 21/21] mrf24j40: change irq trigger type behaviour

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> This patch changes the irq trigger type value while calling
> devm_request_irq by using IRQF_TRIGGER_LOW when no irq type was given.
> Additional we add support for change the irq polarity while hw init if
> high level or rising edge triggered irq type are given.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 25 ++++++++++++++++++++++---
>   1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 560db88..7bd540a 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -20,6 +20,7 @@
>   #include <linux/module.h>
>   #include <linux/regmap.h>
>   #include <linux/ieee802154.h>
> +#include <linux/irq.h>
>   #include <net/cfg802154.h>
>   #include <net/mac802154.h>
>   
> @@ -1082,6 +1083,20 @@ static int mrf24j40_hw_init(struct mrf24j40 *devrec)
>   		regmap_write(devrec->regmap_long, REG_RFCON3, 0x28);
>   	}
>   
> +	switch (irq_get_trigger_type(devrec->spi->irq)) {
> +	case IRQ_TYPE_EDGE_RISING:
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		/* set interrupt polarity to rising */
> +		ret = regmap_update_bits(devrec->regmap_long, REG_SLPCON0,
> +					 0x02, 0x02);
> +		if (ret)
> +			goto err_ret;
> +		break;
> +	default:
> +		/* default is falling edge */
> +		break;
> +	}
> +
>   	return 0;
>   
>   err_ret:
> @@ -1178,7 +1193,7 @@ static void  mrf24j40_phy_setup(struct mrf24j40 *devrec)
>   
>   static int mrf24j40_probe(struct spi_device *spi)
>   {
> -	int ret = -ENOMEM;
> +	int ret = -ENOMEM, irq_type;
>   	struct ieee802154_hw *hw;
>   	struct mrf24j40 *devrec;
>   
> @@ -1232,9 +1247,13 @@ static int mrf24j40_probe(struct spi_device *spi)
>   
>   	mrf24j40_phy_setup(devrec);
>   
> +	/* request IRQF_TRIGGER_LOW as fallback default */
> +	irq_type = irq_get_trigger_type(spi->irq);
> +	if (!irq_type)
> +		irq_type = IRQF_TRIGGER_LOW;
> +
>   	ret = devm_request_irq(&spi->dev, spi->irq, mrf24j40_isr,
> -			       IRQF_TRIGGER_LOW, dev_name(&spi->dev),
> -			       devrec);
> +			       irq_type, dev_name(&spi->dev), devrec);
>   	if (ret) {
>   		dev_err(printdev(devrec), "Unable to get IRQ");
>   		goto err_register_device;

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

Re: [RFC bluetooth-next 00/21] mrf24j40: async rx/tx handling and new features

[Alan Ott]

Hi Alexander, there's definitely a lot of work represented here. I'd like to make some comments and I have some questions, but I'm on travel so it might be a few days before I get some emails out about it.

Thanks!

Alan.

On August 13, 2015 5:22:27 AM MST, Alexander Aring <alex.aring@gmail.com> wrote:
>Hi,
>
>this patch series contains some cleanups, devicetree support, rx/tx
>async
>handling, threaded irq to hardirq, csma params settings, cca mode
>settings,
>cca energy detection levels, tx power settings and promiscuous mode
>settings.
>
>Also I add regmap support, the lowlevel spi calls are only used in
>hotpaths
>for handling receive/transmit and irq handling. There are also some
>many
>magic numbers, maybe we can introduce some register bits defines.
>
>First I will send this as RFC to get some respone if the patches looks
>fine
>and acceptable for mainline.
>
>- Alex
>
>Alexander Aring (21):
>  mrf24j40: cleanup define identation
>  mrf24j40: use ieee802154_alloc_hw for private data
>  mrf24j40: calling ieee802154_register_hw at last
>  mrf24j40: remove spi settings overwrite
>  mrf24j40: add device-tree support
>  mrf24j40: add default channel setting
>  mrf24j40: add random extended addr generation
>  mrf24j40: add more register defines
>  mrf24j40: add regmap support
>  mrf24j40: use regmap for register access
>  mrf24j40: change to frame delivery with crc
>  ieee802154: add helpers for frame control checks
>  mrf24j40: rework tx handling to async tx handling
>  mrf24j40: rework rx handling to async rx handling
>  mrf24j40: async interrupt handling
>  mrf24j40: add csma params support
>  mrf24j40: add cca mode support
>  mrf24j40: add cca ed level support
>  mrf24j40: add tx power support
>  mrf24j40: add promiscuous mode support
>  mrf24j40: change irq trigger type behaviour
>
> .../bindings/net/ieee802154/mrf24j40.txt           |   20 +
> MAINTAINERS                                        |    1 +
> drivers/net/ieee802154/Kconfig                     |    1 +
>drivers/net/ieee802154/mrf24j40.c                  | 1342
>+++++++++++++-------
> include/linux/ieee802154.h                         |   29 +
> 5 files changed, 969 insertions(+), 424 deletions(-)
>create mode 100644
>Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
>
>-- 
>2.5.0

Re: [RFC bluetooth-next 00/21] mrf24j40: async rx/tx handling and new features

[Stefan Schmidt]

Hello.

On 13/08/15 14:22, Alexander Aring wrote:
> Hi,
>
> this patch series contains some cleanups, devicetree support, rx/tx async
> handling, threaded irq to hardirq, csma params settings, cca mode settings,
> cca energy detection levels, tx power settings and promiscuous mode settings.
>
> Also I add regmap support, the lowlevel spi calls are only used in hotpaths
> for handling receive/transmit and irq handling. There are also some many
> magic numbers, maybe we can introduce some register bits defines.
>
> First I will send this as RFC to get some respone if the patches looks fine
> and acceptable for mainline.

I waited for Alan to give some comments first here but he seems rather 
busy these days.

I struggled a while to hook up my mrf24j40ma module to a Pi B+ but it 
works now and I have been testing this patchset with it. Nothing 
problematic showed up and especially the updates for newer stack 
features are good to see for in-tree drivers.

You can have my

Tested-by: Stefan Schmidt <stefan@osg.samsung.com>

for the whole patchset. Review tags will individually while I go through it.

regards
Stefan Schmidt

Re: [RFC bluetooth-next 00/21] mrf24j40: async rx/tx handling and new features

[Alan Ott]

On August 27, 2015 8:29:08 AM EDT, Stefan Schmidt <stefan@osg.samsung.com> wrote:
>Hello.
>
>On 13/08/15 14:22, Alexander Aring wrote:
>> Hi,
>>
>> this patch series contains some cleanups, devicetree support, rx/tx
>async
>> handling, threaded irq to hardirq, csma params settings, cca mode
>settings,
>> cca energy detection levels, tx power settings and promiscuous mode
>settings.
>>
>> Also I add regmap support, the lowlevel spi calls are only used in
>hotpaths
>> for handling receive/transmit and irq handling. There are also some
>many
>> magic numbers, maybe we can introduce some register bits defines.
>>
>> First I will send this as RFC to get some respone if the patches
>looks fine
>> and acceptable for mainline.
>
>I waited for Alan to give some comments first here but he seems rather 
>busy these days.
>
>I struggled a while to hook up my mrf24j40ma module to a Pi B+ but it 
>works now and I have been testing this patchset with it. Nothing 
>problematic showed up and especially the updates for newer stack 
>features are good to see for in-tree drivers.
>
>You can have my
>
>Tested-by: Stefan Schmidt <stefan@osg.samsung.com>
>
>for the whole patchset. Review tags will individually while I go
>through it.
>
>regards
>Stefan Schmidt

Thanks Stefan. I've been on the road. Hopefully I can get some time this weekend to get a proper response out.

Alan.