[PATCH bluetooth-next 0/4] Atusb driver improvements

[PATCH bluetooth-next 0/4] Atusb driver improvements

[Stefan Schmidt]

Hello.

Some patches to let the atusb driver catchup a bit with newer stack features.
Namely we now support tx power and promiscuous mode setting. The first patch
adds some infrastructure for this by adding a function to write parts of a
register.

The last patch adds me as the maintainer for the driver. Alex suggested that
I should do this and as I have the device, working on the firmware as well
and are motivated to enhance it I think its a good idea.

Stefan Schmidt (4):
  ieee802154/atusb: Add function for partial register writes
  ieee802154/atusb: Add .set_txpower operation to the driver
  ieee802154/atusb: Add .set_promiscuous_mode driver operation
  MAINTAINERS: Add myself as maintainer for the atusb driver

 MAINTAINERS                    |   8 ++++
 drivers/net/ieee802154/atusb.c | 100 +++++++++++++++++++++++++++++++++--------
 2 files changed, 89 insertions(+), 19 deletions(-)

regards
Stefan Schmidt

-- 
2.1.0

[PATCH bluetooth-next 1/4] ieee802154/atusb: Add function for partial register writes

[Stefan Schmidt]

From: Stefan Schmidt <s.schmidt@samsung.com>

With this function we can set individual bits in the registers if needed.
With the old SR_VALUE macro we could only set one bit in the register
which was ok for some scenarios but not for all. With this subreg write
function we can now set more bits defined by the mask while not touching
the rest.

We start using it for the current SR_VALUE use case and will use it more
in upcoming patches.

Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 drivers/net/ieee802154/atusb.c | 44 +++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 5b6bb9a..93431a5 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -58,17 +58,6 @@ struct atusb {
 	uint8_t tx_ack_seq;		/* current TX ACK sequence number */
 };
 
-/* at86rf230.h defines values as <reg, mask, shift> tuples. We use the more
- * traditional style of having registers and or-able values. SR_REG extracts
- * the register number. SR_VALUE uses the shift to prepare a value accordingly.
- */
-
-#define __SR_REG(reg, mask, shift)	(reg)
-#define SR_REG(sr)			__SR_REG(sr)
-
-#define __SR_VALUE(reg, mask, shift, val)	((val) << (shift))
-#define SR_VALUE(sr, val)			__SR_VALUE(sr, (val))
-
 /* ----- USB commands without data ----------------------------------------- */
 
 /* To reduce the number of error checks in the code, we record the first error
@@ -130,6 +119,29 @@ static int atusb_read_reg(struct atusb *atusb, uint8_t reg)
 	return ret >= 0 ? value : ret;
 }
 
+static int atusb_write_subreg(struct atusb *atusb, uint8_t reg, uint8_t mask,
+			      uint8_t shift, uint8_t value)
+{
+	struct usb_device *usb_dev = atusb->usb_dev;
+	uint8_t orig, tmp;
+	int ret = 0;
+
+	dev_dbg(&usb_dev->dev, "atusb_write_subreg: 0x%02x <- 0x%02x\n",
+	    reg, value);
+
+	orig = atusb_read_reg(atusb, reg);
+
+	/* Write the value only into that part of the register which is allowed
+	 * by the mask. All other bits stay as before. */
+	tmp = orig & ~mask;
+	tmp |= (value << shift) & mask;
+
+	if (tmp != orig)
+		ret = atusb_write_reg(atusb, reg, tmp);
+
+	return ret;
+}
+
 static int atusb_get_and_clear_error(struct atusb *atusb)
 {
 	int err = atusb->err;
@@ -376,7 +388,6 @@ static int atusb_set_hw_addr_filt(struct ieee802154_hw *hw,
 {
 	struct atusb *atusb = hw->priv;
 	struct device *dev = &atusb->usb_dev->dev;
-	uint8_t reg;
 
 	if (changed & IEEE802154_AFILT_SADDR_CHANGED) {
 		u16 addr = le16_to_cpu(filt->short_addr);
@@ -406,12 +417,10 @@ static int atusb_set_hw_addr_filt(struct ieee802154_hw *hw,
 	if (changed & IEEE802154_AFILT_PANC_CHANGED) {
 		dev_vdbg(dev,
 			 "atusb_set_hw_addr_filt called for panc change\n");
-		reg = atusb_read_reg(atusb, SR_REG(SR_AACK_I_AM_COORD));
 		if (filt->pan_coord)
-			reg |= SR_VALUE(SR_AACK_I_AM_COORD, 1);
+			atusb_write_subreg(atusb, SR_AACK_I_AM_COORD, 1);
 		else
-			reg &= ~SR_VALUE(SR_AACK_I_AM_COORD, 1);
-		atusb_write_reg(atusb, SR_REG(SR_AACK_I_AM_COORD), reg);
+			atusb_write_subreg(atusb, SR_AACK_I_AM_COORD, 0);
 	}
 
 	return atusb_get_and_clear_error(atusb);
@@ -622,8 +631,7 @@ static int atusb_probe(struct usb_interface *interface,
 	 *     http://www.jennic.com/download_file.php?supportFile=JN-AN-1035%20Calculating%20802-15-4%20Data%20Rates-1v0.pdf
 	 */
 
-	atusb_write_reg(atusb,
-			SR_REG(SR_RX_SAFE_MODE), SR_VALUE(SR_RX_SAFE_MODE, 1));
+	atusb_write_subreg(atusb, SR_RX_SAFE_MODE, 1);
 #endif
 	atusb_write_reg(atusb, RG_IRQ_MASK, 0xff);
 
-- 
2.1.0

[PATCH bluetooth-next 2/4] ieee802154/atusb: Add .set_txpower operation to the driver

[Stefan Schmidt]

From: Stefan Schmidt <s.schmidt@samsung.com>

Atusb uses the at86rf231 transceiver so we can use the same calculation
for txpower settings for it.

Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 drivers/net/ieee802154/atusb.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 93431a5..531a1d4 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -452,6 +452,26 @@ static void atusb_stop(struct ieee802154_hw *hw)
 	atusb_get_and_clear_error(atusb);
 }
 
+#define ATUSB_MAX_TX_POWERS 0xF
+static const s32 atusb_powers[ATUSB_MAX_TX_POWERS + 1] = {
+	300, 280, 230, 180, 130, 70, 0, -100, -200, -300, -400, -500, -700,
+	-900, -1200, -1700,
+};
+
+static int
+atusb_set_txpower(struct ieee802154_hw *hw, s32 mbm)
+{
+	struct atusb *atusb = hw->priv;
+	u32 i;
+
+	for (i = 0; i < hw->phy->supported.tx_powers_size; i++) {
+		if (hw->phy->supported.tx_powers[i] == mbm)
+			return atusb_write_subreg(atusb, SR_TX_PWR_23X, i);
+	}
+
+	return -EINVAL;
+}
+
 static struct ieee802154_ops atusb_ops = {
 	.owner			= THIS_MODULE,
 	.xmit_async		= atusb_xmit,
@@ -460,6 +480,7 @@ static struct ieee802154_ops atusb_ops = {
 	.start			= atusb_start,
 	.stop			= atusb_stop,
 	.set_hw_addr_filt	= atusb_set_hw_addr_filt,
+	.set_txpower		= atusb_set_txpower,
 };
 
 /* ----- Firmware and chip version information ----------------------------- */
@@ -580,9 +601,14 @@ static int atusb_probe(struct usb_interface *interface,
 	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
 		    IEEE802154_HW_AACK;
 
+	hw->phy->flags = WPAN_PHY_FLAG_TXPOWER;
+
 	hw->phy->current_page = 0;
 	hw->phy->current_channel = 11;	/* reset default */
 	hw->phy->supported.channels[0] = 0x7FFF800;
+	hw->phy->supported.tx_powers = atusb_powers;
+	hw->phy->supported.tx_powers_size = ARRAY_SIZE(atusb_powers);
+	hw->phy->transmit_power = hw->phy->supported.tx_powers[0];
 	ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
 
 	atusb_command(atusb, ATUSB_RF_RESET, 0);
-- 
2.1.0

[PATCH bluetooth-next 3/4] ieee802154/atusb: Add .set_promiscuous_mode driver operation

[Stefan Schmidt]

From: Stefan Schmidt <s.schmidt@samsung.com>

Allow monitor mode operation with disabled AACK in hardware.

Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 drivers/net/ieee802154/atusb.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 531a1d4..52bcdbc 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -472,6 +472,33 @@ atusb_set_txpower(struct ieee802154_hw *hw, s32 mbm)
 	return -EINVAL;
 }
 
+static int
+atusb_set_promiscuous_mode(struct ieee802154_hw *hw, const bool on)
+{
+	struct atusb *atusb = hw->priv;
+	int ret;
+
+	if (on) {
+		ret = atusb_write_subreg(atusb, SR_AACK_DIS_ACK, 1);
+		if (ret < 0)
+			return ret;
+
+		ret = atusb_write_subreg(atusb, SR_AACK_PROM_MODE, 1);
+		if (ret < 0)
+			return ret;
+	} else {
+		ret = atusb_write_subreg(atusb, SR_AACK_PROM_MODE, 0);
+		if (ret < 0)
+			return ret;
+
+		ret = atusb_write_subreg(atusb, SR_AACK_DIS_ACK, 0);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static struct ieee802154_ops atusb_ops = {
 	.owner			= THIS_MODULE,
 	.xmit_async		= atusb_xmit,
@@ -481,6 +508,7 @@ static struct ieee802154_ops atusb_ops = {
 	.stop			= atusb_stop,
 	.set_hw_addr_filt	= atusb_set_hw_addr_filt,
 	.set_txpower		= atusb_set_txpower,
+	.set_promiscuous_mode	= atusb_set_promiscuous_mode,
 };
 
 /* ----- Firmware and chip version information ----------------------------- */
@@ -599,7 +627,7 @@ static int atusb_probe(struct usb_interface *interface,
 
 	hw->parent = &usb_dev->dev;
 	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
-		    IEEE802154_HW_AACK;
+		    IEEE802154_HW_AACK | IEEE802154_HW_PROMISCUOUS;
 
 	hw->phy->flags = WPAN_PHY_FLAG_TXPOWER;
 
-- 
2.1.0

[PATCH bluetooth-next 4/4] MAINTAINERS: Add myself as maintainer for the atusb driver

[Stefan Schmidt]

From: Stefan Schmidt <s.schmidt@samsung.com>

Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5d87ccb..d69a7c1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1860,6 +1860,14 @@ W:	http://www.attotech.com
 S:	Supported
 F:	drivers/scsi/esas2r
 
+ATUSB IEEE 802.15.4 RADIO DRIVER
+M:	Stefan Schmidt <stefan@osg.samsung.com>
+L:	linux-wpan@vger.kernel.org
+S:	Maintained
+F:	drivers/net/ieee802154/atusb.c
+F:	drivers/net/ieee802154/atusb.h
+F:	drivers/net/ieee802154/at86rf230.h
+
 AUDIT SUBSYSTEM
 M:	Paul Moore <paul@paul-moore.com>
 M:	Eric Paris <eparis@redhat.com>
-- 
2.1.0

Re: [PATCH bluetooth-next 1/4] ieee802154/atusb: Add function for partial register writes

[Varka Bhadram]

Hi Stefan,

I found two checkpatch issues on this patch.. See inline

On 05/29/2015 03:16 AM, Stefan Schmidt wrote:

> From: Stefan Schmidt <s.schmidt@samsung.com>
>
> With this function we can set individual bits in the registers if needed.
> With the old SR_VALUE macro we could only set one bit in the register
> which was ok for some scenarios but not for all. With this subreg write
> function we can now set more bits defined by the mask while not touching
> the rest.
>
> We start using it for the current SR_VALUE use case and will use it more
> in upcoming patches.
>
> Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
> ---
>   drivers/net/ieee802154/atusb.c | 44 +++++++++++++++++++++++++-----------------
>   1 file changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> index 5b6bb9a..93431a5 100644
> --- a/drivers/net/ieee802154/atusb.c
> +++ b/drivers/net/ieee802154/atusb.c
> @@ -58,17 +58,6 @@ struct atusb {
>   	uint8_t tx_ack_seq;		/* current TX ACK sequence number */
>   };
>   
> -/* at86rf230.h defines values as <reg, mask, shift> tuples. We use the more
> - * traditional style of having registers and or-able values. SR_REG extracts
> - * the register number. SR_VALUE uses the shift to prepare a value accordingly.
> - */
> -
> -#define __SR_REG(reg, mask, shift)	(reg)
> -#define SR_REG(sr)			__SR_REG(sr)
> -
> -#define __SR_VALUE(reg, mask, shift, val)	((val) << (shift))
> -#define SR_VALUE(sr, val)			__SR_VALUE(sr, (val))
> -
>   /* ----- USB commands without data ----------------------------------------- */
>   
>   /* To reduce the number of error checks in the code, we record the first error
> @@ -130,6 +119,29 @@ static int atusb_read_reg(struct atusb *atusb, uint8_t reg)
>   	return ret >= 0 ? value : ret;
>   }
>   
> +static int atusb_write_subreg(struct atusb *atusb, uint8_t reg, uint8_t mask,
> +			      uint8_t shift, uint8_t value)
> +{
> +	struct usb_device *usb_dev = atusb->usb_dev;
> +	uint8_t orig, tmp;
> +	int ret = 0;
> +
> +	dev_dbg(&usb_dev->dev, "atusb_write_subreg: 0x%02x <- 0x%02x\n",
> +	    reg, value);

CHECK: Alignment should match open parenthesis
#56: FILE: drivers/net/ieee802154/atusb.c:130:
+	dev_dbg(&usb_dev->dev, "atusb_write_subreg: 0x%02x <- 0x%02x\n",
+	    reg, value);

> +
> +	orig = atusb_read_reg(atusb, reg);
> +
> +	/* Write the value only into that part of the register which is allowed
> +	 * by the mask. All other bits stay as before. */

WARNING: networking block comments put the trailing */ on a separate line
#61: FILE: drivers/net/ieee802154/atusb.c:135:
+	 * by the mask. All other bits stay as before. */

regards,

-- 
Varka Bhadram

Re: [PATCH bluetooth-next 1/4] ieee802154/atusb: Add function for partial register writes

[Stefan Schmidt]

Hello.

On 29/05/15 08:59, Varka Bhadram wrote:
> Hi Stefan,
>
> I found two checkpatch issues on this patch.. See inline
>
Thanks for letting me know. Will fix and submit v2.

regards
Stefan Schmidt

> On 05/29/2015 03:16 AM, Stefan Schmidt wrote:
>
>> From: Stefan Schmidt <s.schmidt@samsung.com>
>>
>> With this function we can set individual bits in the registers if 
>> needed.
>> With the old SR_VALUE macro we could only set one bit in the register
>> which was ok for some scenarios but not for all. With this subreg write
>> function we can now set more bits defined by the mask while not touching
>> the rest.
>>
>> We start using it for the current SR_VALUE use case and will use it more
>> in upcoming patches.
>>
>> Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
>> ---
>>   drivers/net/ieee802154/atusb.c | 44 
>> +++++++++++++++++++++++++-----------------
>>   1 file changed, 26 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/net/ieee802154/atusb.c 
>> b/drivers/net/ieee802154/atusb.c
>> index 5b6bb9a..93431a5 100644
>> --- a/drivers/net/ieee802154/atusb.c
>> +++ b/drivers/net/ieee802154/atusb.c
>> @@ -58,17 +58,6 @@ struct atusb {
>>       uint8_t tx_ack_seq;        /* current TX ACK sequence number */
>>   };
>>   -/* at86rf230.h defines values as <reg, mask, shift> tuples. We use 
>> the more
>> - * traditional style of having registers and or-able values. SR_REG 
>> extracts
>> - * the register number. SR_VALUE uses the shift to prepare a value 
>> accordingly.
>> - */
>> -
>> -#define __SR_REG(reg, mask, shift)    (reg)
>> -#define SR_REG(sr)            __SR_REG(sr)
>> -
>> -#define __SR_VALUE(reg, mask, shift, val)    ((val) << (shift))
>> -#define SR_VALUE(sr, val)            __SR_VALUE(sr, (val))
>> -
>>   /* ----- USB commands without data 
>> ----------------------------------------- */
>>     /* To reduce the number of error checks in the code, we record 
>> the first error
>> @@ -130,6 +119,29 @@ static int atusb_read_reg(struct atusb *atusb, 
>> uint8_t reg)
>>       return ret >= 0 ? value : ret;
>>   }
>>   +static int atusb_write_subreg(struct atusb *atusb, uint8_t reg, 
>> uint8_t mask,
>> +                  uint8_t shift, uint8_t value)
>> +{
>> +    struct usb_device *usb_dev = atusb->usb_dev;
>> +    uint8_t orig, tmp;
>> +    int ret = 0;
>> +
>> +    dev_dbg(&usb_dev->dev, "atusb_write_subreg: 0x%02x <- 0x%02x\n",
>> +        reg, value);
>
> CHECK: Alignment should match open parenthesis
> #56: FILE: drivers/net/ieee802154/atusb.c:130:
> +    dev_dbg(&usb_dev->dev, "atusb_write_subreg: 0x%02x <- 0x%02x\n",
> +        reg, value);
>
>> +
>> +    orig = atusb_read_reg(atusb, reg);
>> +
>> +    /* Write the value only into that part of the register which is 
>> allowed
>> +     * by the mask. All other bits stay as before. */
>
> WARNING: networking block comments put the trailing */ on a separate line
> #61: FILE: drivers/net/ieee802154/atusb.c:135:
> +     * by the mask. All other bits stay as before. */
>
> regards,
>