Re: [PATCH bluetooth-next 2/3] cc2520: add set transmit power support

[Stefan Schmidt <s.schmidt@samsung.com>]

Hello.

On 20/03/15 08:22, Varka Bhadram wrote:
> Signed-off-by: Varka Bhadram <varkab@cdac.in>
> ---
>   drivers/net/ieee802154/cc2520.c |   56
> ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/cc2520.c
> b/drivers/net/ieee802154/cc2520.c
> index f833b8b..f96cc50 100644
> --- a/drivers/net/ieee802154/cc2520.c
> +++ b/drivers/net/ieee802154/cc2520.c
> @@ -53,6 +53,17 @@
>   #define	CC2520_MAXCHANNEL		26
>   #define	CC2520_CHANNEL_SPACING		5
>
> +/* Tx power values */
> +#define CC2520_TXPOWER_0		0x03 /* -18dbm */
> +#define CC2520_TXPOWER_1		0x2c /* -7dbm */
> +#define CC2520_TXPOWER_2		0x88 /* -4dbm */
> +#define CC2520_TXPOWER_3		0x81 /* -2dbm */
> +#define CC2520_TXPOWER_4		0x32 /* 0dbm */
> +#define CC2520_TXPOWER_5		0x13 /* 1dbm */
> +#define CC2520_TXPOWER_6		0xab /* 2dbm */
> +#define CC2520_TXPOWER_7		0xf2 /* 3dbm */
> +#define CC2520_TXPOWER_8		0xf7 /* 5dbm */
> +
>   /* command strobes */
>   #define	CC2520_CMD_SNOP			0x00
>   #define	CC2520_CMD_IBUFLD		0x02
> @@ -628,6 +639,48 @@ cc2520_filter(struct ieee802154_hw *hw,
>   	return 0;
>   }
>
> +static int
> +cc2520_set_txpower(struct ieee802154_hw *hw, int db)
> +{
> +	struct cc2520_private *priv = hw->priv;
> +	u8 power;
> +
> +	switch (db) {
> +	case 5:
> +		power = CC2520_TXPOWER_8;
> +		break;
> +	case 3:
> +		power = CC2520_TXPOWER_7;
> +		break;
> +	case 2:
> +		power = CC2520_TXPOWER_6;
> +		break;
> +	case 1:
> +		power = CC2520_TXPOWER_5;
> +		break;
> +	case 0:
> +		power = CC2520_TXPOWER_4;
> +		break;
> +	case -2:
> +		power = CC2520_TXPOWER_3;
> +		break;
> +	case -4:
> +		power = CC2520_TXPOWER_2;
> +		break;
> +	case -7:
> +		power = CC2520_TXPOWER_1;
> +		break;
> +	case -18:
> +		power = CC2520_TXPOWER_0;
> +		break;
> +	default:
> +		dev_err(&priv->spi->dev, "invalid tx power setting\n");
> +		return -EINVAL;
> +	}
> +
> +	return cc2520_write_register(priv, CC2520_TXPOWER, power);
> +}
> +

Thanks for working on the power setting API.

One thing I find problematic here is that the user has to know the 
values it can set for the db level beforehand and these values can 
change for different transceivers. Which makes these nl call hardware 
depended.

We should at least have a way to query what db levels are available or 
better see if we can harmonize the setting over different drivers and 
transceivers.

regards
Stefan Schmidt