[PATCHv4 bluetooth-next 0/2] at86rf230: add support for xtal trim register

[PATCHv4 bluetooth-next 0/2] at86rf230: add support for xtal trim register

[Alexander Aring]

Hi,

I want to clarify how xtal_trim value is calculated now.

First of all the xtal_mode set to 0x5 will require a 16 Mhz clock signal at
pin 26. I don't have such at86rf2xx board which can do that, so I don't want
to support it right now.

IMPORTANT:
The xtal_mode for external frequency differs per at86rf2xx transceiver. For
at86rf230/at86rf231 it's 0x4. For at86rf233 it's 0x5. (Thanks Werner
Almesberger who pointed this out). That doesn't matter right now, because we
don't change the "normal" mode.


The xtal_trim is only necesarry for external crystal, when xtal_mode == 0xF.
The xtal_trim value is calculated by:

CL = capacitor of used crystal
CX = connected capacitors at xtal pins
CPAR = in all at86rf2xx datasheets this is a constant value 3 pF,
       but this is different on each board setup. You need to fine
       tuning this value via CTRIM.
CTRIM = variable capacitor setting. Resolution is 0.3 pF range is
        0 pF upto 4.5 pF.

CL = 0.5 * (CX + CTRIM + CPAR)


Examples:

On atben [0]:

CL = 8 pF
CX = 12 pF
CPAR = 3 pF (We assume the magic constant from datasheet)
CTRIM = 0.9 pF

(12+0.9+3)/2 = 7.95 which is nearly at 8 pF

openlabs [1]:

CL = 16 pF
CX = 22 pF
CPAR = 3 pF (We assume the magic constant from datasheet)
CTRIM = 4.5 pF

(22+4.5+3)/2 = 14.75 which is the nearest value to 16 pF

For more information it's the section "Integrated Oscillator Setup" inside
the at86rf2xx datasheets. (In my case 8111C–MCU Wireless–09/09).

For a better calculation of the CPAR value, Werner Almesberger developed some
diagnostic tool [2], which I don't tried out yet.

- Alex

[0] http://projects.qi-hardware.com/index.php/p/ben-wpan/source/tree/master/atben
    (atben.sch, requires kicad tool)
[1] http://openlabs.co/OSHW/Raspberry-Pi-802.15.4-radio-files/rpi802154-r1.pdf
[2] http://projects.qi-hardware.com/index.php/p/ben-wpan/source/tree/master/tools/atrf-xtal

changes since v4:
 - remove platform data from at86rf230_local
 - add note in cover-letter because at86rf2xx have different xtal_mode values

changes since v3:
 - remove setting of xtal_mode. Instead we setting xtal_trim only.

changes since v2:
 - copy platform data to driver allocated space

Cc: Werner Almesberger <werner@almesberger.net>
Cc: Thomas Stilwell <stilwellt@openlabs.co>

Alexander Aring (2):
  at86rf230: copy pdata to driver allocated space
  at86rf230: add support for external xtal trim

 .../bindings/net/ieee802154/at86rf230.txt          |  3 +
 drivers/net/ieee802154/at86rf230.c                 | 66 +++++++++++++---------
 include/linux/spi/at86rf230.h                      |  1 +
 3 files changed, 42 insertions(+), 28 deletions(-)

-- 
2.3.0

[PATCHv4 bluetooth-next 1/2] at86rf230: copy pdata to driver allocated space

[Alexander Aring]

This patch copies the platform data in driver allocated space at first.
With this change we ensure that we access the allocated platform data as
readonly space.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
Reported-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ieee802154/at86rf230.c | 51 +++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index cbfc8c5..9888b7f 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1377,24 +1377,24 @@ static int at86rf230_hw_init(struct at86rf230_local *lp)
 	return at86rf230_write_subreg(lp, SR_SLOTTED_OPERATION, 0);
 }
 
-static struct at86rf230_platform_data *
-at86rf230_get_pdata(struct spi_device *spi)
+static int
+at86rf230_get_pdata(struct spi_device *spi, int *rstn, int *slp_tr)
 {
-	struct at86rf230_platform_data *pdata;
+	struct at86rf230_platform_data *pdata = spi->dev.platform_data;
 
-	if (!IS_ENABLED(CONFIG_OF) || !spi->dev.of_node)
-		return spi->dev.platform_data;
+	if (!IS_ENABLED(CONFIG_OF) || !spi->dev.of_node) {
+		if (!pdata)
+			return -ENOENT;
 
-	pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		goto done;
+		*rstn = pdata->rstn;
+		*slp_tr = pdata->slp_tr;
+		return 0;
+	}
 
-	pdata->rstn = of_get_named_gpio(spi->dev.of_node, "reset-gpio", 0);
-	pdata->slp_tr = of_get_named_gpio(spi->dev.of_node, "sleep-gpio", 0);
+	*rstn = of_get_named_gpio(spi->dev.of_node, "reset-gpio", 0);
+	*slp_tr = of_get_named_gpio(spi->dev.of_node, "sleep-gpio", 0);
 
-	spi->dev.platform_data = pdata;
-done:
-	return pdata;
+	return 0;
 }
 
 static int
@@ -1501,43 +1501,42 @@ at86rf230_setup_spi_messages(struct at86rf230_local *lp)
 
 static int at86rf230_probe(struct spi_device *spi)
 {
-	struct at86rf230_platform_data *pdata;
 	struct ieee802154_hw *hw;
 	struct at86rf230_local *lp;
 	unsigned int status;
-	int rc, irq_type;
+	int rc, irq_type, rstn, slp_tr;
 
 	if (!spi->irq) {
 		dev_err(&spi->dev, "no IRQ specified\n");
 		return -EINVAL;
 	}
 
-	pdata = at86rf230_get_pdata(spi);
-	if (!pdata) {
-		dev_err(&spi->dev, "no platform_data\n");
-		return -EINVAL;
+	rc = at86rf230_get_pdata(spi, &rstn, &slp_tr);
+	if (rc < 0) {
+		dev_err(&spi->dev, "failed to parse platform_data: %d\n", rc);
+		return rc;
 	}
 
-	if (gpio_is_valid(pdata->rstn)) {
-		rc = devm_gpio_request_one(&spi->dev, pdata->rstn,
+	if (gpio_is_valid(rstn)) {
+		rc = devm_gpio_request_one(&spi->dev, rstn,
 					   GPIOF_OUT_INIT_HIGH, "rstn");
 		if (rc)
 			return rc;
 	}
 
-	if (gpio_is_valid(pdata->slp_tr)) {
-		rc = devm_gpio_request_one(&spi->dev, pdata->slp_tr,
+	if (gpio_is_valid(slp_tr)) {
+		rc = devm_gpio_request_one(&spi->dev, slp_tr,
 					   GPIOF_OUT_INIT_LOW, "slp_tr");
 		if (rc)
 			return rc;
 	}
 
 	/* Reset */
-	if (gpio_is_valid(pdata->rstn)) {
+	if (gpio_is_valid(rstn)) {
 		udelay(1);
-		gpio_set_value(pdata->rstn, 0);
+		gpio_set_value(rstn, 0);
 		udelay(1);
-		gpio_set_value(pdata->rstn, 1);
+		gpio_set_value(rstn, 1);
 		usleep_range(120, 240);
 	}
 
-- 
2.3.0

[PATCHv4 bluetooth-next 2/2] at86rf230: add support for external xtal trim

[Alexander Aring]

This patch adds support for setting the xtal trim register. Some at86rf2xx
transceiver boards needs fine tuning the xtal capacitor.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 .../devicetree/bindings/net/ieee802154/at86rf230.txt  |  3 +++
 drivers/net/ieee802154/at86rf230.c                    | 19 +++++++++++++++----
 include/linux/spi/at86rf230.h                         |  1 +
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt b/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
index d3bbdded..1ae5100 100644
--- a/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
+++ b/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
@@ -11,6 +11,8 @@ Required properties:
 Optional properties:
   - reset-gpio:		GPIO spec for the rstn pin
   - sleep-gpio:		GPIO spec for the slp_tr pin
+  - xtal-trim:		u8 value for fine tuning the internal capacitance
+			arrays of xtal pins: 0 = +0 pF, 0xf = +4.5 pF
 
 Example:
 
@@ -20,4 +22,5 @@ Example:
 		reg = <0>;
 		interrupts = <19 1>;
 		interrupt-parent = <&gpio3>;
+		xtal-trim = /bits/ 8 <0x06>;
 	};
diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 9888b7f..266e67e 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1315,7 +1315,7 @@ static struct at86rf2xx_chip_data at86rf212_data = {
 	.get_desense_steps = at86rf212_get_desens_steps
 };
 
-static int at86rf230_hw_init(struct at86rf230_local *lp)
+static int at86rf230_hw_init(struct at86rf230_local *lp, u8 xtal_trim)
 {
 	int rc, irq_type, irq_pol = IRQ_ACTIVE_HIGH;
 	unsigned int dvdd;
@@ -1362,6 +1362,10 @@ static int at86rf230_hw_init(struct at86rf230_local *lp)
 	usleep_range(lp->data->t_sleep_cycle,
 		     lp->data->t_sleep_cycle + 100);
 
+	rc = at86rf230_write_subreg(lp, SR_XTAL_TRIM, xtal_trim);
+	if (rc)
+		return rc;
+
 	rc = at86rf230_read_subreg(lp, SR_DVDD_OK, &dvdd);
 	if (rc)
 		return rc;
@@ -1378,9 +1382,11 @@ static int at86rf230_hw_init(struct at86rf230_local *lp)
 }
 
 static int
-at86rf230_get_pdata(struct spi_device *spi, int *rstn, int *slp_tr)
+at86rf230_get_pdata(struct spi_device *spi, int *rstn, int *slp_tr,
+		    u8 *xtal_trim)
 {
 	struct at86rf230_platform_data *pdata = spi->dev.platform_data;
+	int ret;
 
 	if (!IS_ENABLED(CONFIG_OF) || !spi->dev.of_node) {
 		if (!pdata)
@@ -1388,11 +1394,15 @@ at86rf230_get_pdata(struct spi_device *spi, int *rstn, int *slp_tr)
 
 		*rstn = pdata->rstn;
 		*slp_tr = pdata->slp_tr;
+		*xtal_trim = pdata->xtal_trim;
 		return 0;
 	}
 
 	*rstn = of_get_named_gpio(spi->dev.of_node, "reset-gpio", 0);
 	*slp_tr = of_get_named_gpio(spi->dev.of_node, "sleep-gpio", 0);
+	ret = of_property_read_u8(spi->dev.of_node, "xtal-trim", xtal_trim);
+	if (ret < 0 && ret != -EINVAL)
+		return ret;
 
 	return 0;
 }
@@ -1505,13 +1515,14 @@ static int at86rf230_probe(struct spi_device *spi)
 	struct at86rf230_local *lp;
 	unsigned int status;
 	int rc, irq_type, rstn, slp_tr;
+	u8 xtal_trim;
 
 	if (!spi->irq) {
 		dev_err(&spi->dev, "no IRQ specified\n");
 		return -EINVAL;
 	}
 
-	rc = at86rf230_get_pdata(spi, &rstn, &slp_tr);
+	rc = at86rf230_get_pdata(spi, &rstn, &slp_tr, &xtal_trim);
 	if (rc < 0) {
 		dev_err(&spi->dev, "failed to parse platform_data: %d\n", rc);
 		return rc;
@@ -1570,7 +1581,7 @@ static int at86rf230_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, lp);
 
-	rc = at86rf230_hw_init(lp);
+	rc = at86rf230_hw_init(lp, xtal_trim);
 	if (rc)
 		goto free_dev;
 
diff --git a/include/linux/spi/at86rf230.h b/include/linux/spi/at86rf230.h
index cd519a1..b63fe6f 100644
--- a/include/linux/spi/at86rf230.h
+++ b/include/linux/spi/at86rf230.h
@@ -22,6 +22,7 @@ struct at86rf230_platform_data {
 	int rstn;
 	int slp_tr;
 	int dig2;
+	u8 xtal_trim;
 };
 
 #endif
-- 
2.3.0

Re: [PATCHv4 bluetooth-next 0/2] at86rf230: add support for xtal trim register

[Marcel Holtmann]

Hi Alex,

> I want to clarify how xtal_trim value is calculated now.
> 
> First of all the xtal_mode set to 0x5 will require a 16 Mhz clock signal at
> pin 26. I don't have such at86rf2xx board which can do that, so I don't want
> to support it right now.
> 
> IMPORTANT:
> The xtal_mode for external frequency differs per at86rf2xx transceiver. For
> at86rf230/at86rf231 it's 0x4. For at86rf233 it's 0x5. (Thanks Werner
> Almesberger who pointed this out). That doesn't matter right now, because we
> don't change the "normal" mode.
> 
> 
> The xtal_trim is only necesarry for external crystal, when xtal_mode == 0xF.
> The xtal_trim value is calculated by:
> 
> CL = capacitor of used crystal
> CX = connected capacitors at xtal pins
> CPAR = in all at86rf2xx datasheets this is a constant value 3 pF,
>       but this is different on each board setup. You need to fine
>       tuning this value via CTRIM.
> CTRIM = variable capacitor setting. Resolution is 0.3 pF range is
>        0 pF upto 4.5 pF.
> 
> CL = 0.5 * (CX + CTRIM + CPAR)
> 
> 
> Examples:
> 
> On atben [0]:
> 
> CL = 8 pF
> CX = 12 pF
> CPAR = 3 pF (We assume the magic constant from datasheet)
> CTRIM = 0.9 pF
> 
> (12+0.9+3)/2 = 7.95 which is nearly at 8 pF
> 
> openlabs [1]:
> 
> CL = 16 pF
> CX = 22 pF
> CPAR = 3 pF (We assume the magic constant from datasheet)
> CTRIM = 4.5 pF
> 
> (22+4.5+3)/2 = 14.75 which is the nearest value to 16 pF
> 
> For more information it's the section "Integrated Oscillator Setup" inside
> the at86rf2xx datasheets. (In my case 8111C–MCU Wireless–09/09).
> 
> For a better calculation of the CPAR value, Werner Almesberger developed some
> diagnostic tool [2], which I don't tried out yet.

don't you want to put this kind of information either in the commit message of the patch or actually as comment in the source code. If you keep it in the cover letter, then this will not be part of the source tree in any form or shape.

Regards

Marcel

Re: [PATCHv4 bluetooth-next 0/2] at86rf230: add support for xtal trim register

[Alexander Aring]

Hi Marcel,

On Tue, Feb 24, 2015 at 08:17:53AM -0800, Marcel Holtmann wrote:
> Hi Alex,
> 
> > I want to clarify how xtal_trim value is calculated now.
> > 
> > First of all the xtal_mode set to 0x5 will require a 16 Mhz clock signal at
> > pin 26. I don't have such at86rf2xx board which can do that, so I don't want
> > to support it right now.
> > 
> > IMPORTANT:
> > The xtal_mode for external frequency differs per at86rf2xx transceiver. For
> > at86rf230/at86rf231 it's 0x4. For at86rf233 it's 0x5. (Thanks Werner
> > Almesberger who pointed this out). That doesn't matter right now, because we
> > don't change the "normal" mode.
> > 
> > 
> > The xtal_trim is only necesarry for external crystal, when xtal_mode == 0xF.
> > The xtal_trim value is calculated by:
> > 
> > CL = capacitor of used crystal
> > CX = connected capacitors at xtal pins
> > CPAR = in all at86rf2xx datasheets this is a constant value 3 pF,
> >       but this is different on each board setup. You need to fine
> >       tuning this value via CTRIM.
> > CTRIM = variable capacitor setting. Resolution is 0.3 pF range is
> >        0 pF upto 4.5 pF.
> > 
> > CL = 0.5 * (CX + CTRIM + CPAR)
> > 
> > 
> > Examples:
> > 
> > On atben [0]:
> > 
> > CL = 8 pF
> > CX = 12 pF
> > CPAR = 3 pF (We assume the magic constant from datasheet)
> > CTRIM = 0.9 pF
> > 
> > (12+0.9+3)/2 = 7.95 which is nearly at 8 pF
> > 
> > openlabs [1]:
> > 
> > CL = 16 pF
> > CX = 22 pF
> > CPAR = 3 pF (We assume the magic constant from datasheet)
> > CTRIM = 4.5 pF
> > 
> > (22+4.5+3)/2 = 14.75 which is the nearest value to 16 pF
> > 
> > For more information it's the section "Integrated Oscillator Setup" inside
> > the at86rf2xx datasheets. (In my case 8111C–MCU Wireless–09/09).
> > 
> > For a better calculation of the CPAR value, Werner Almesberger developed some
> > diagnostic tool [2], which I don't tried out yet.
> 
> don't you want to put this kind of information either in the commit message of the patch or actually as comment in the source code. If you keep it in the cover letter, then this will not be part of the source tree in any form or shape.
> 

ok.

- Alex