* [PATCH bluetooth-next] at86rf230: add support for setting external xtal
@ 2015-02-23 15:05 Alexander Aring
2015-02-23 15:10 ` Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Aring @ 2015-02-23 15:05 UTC (permalink / raw)
To: linux-wpan; +Cc: kernel, Alexander Aring
This patch adds support for setting external xtal. This is recommended
by the at86rf230 transceivers to reduce power consuming and for a better
clock source.
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
.../bindings/net/ieee802154/at86rf230.txt | 6 ++++++
drivers/net/ieee802154/at86rf230.c | 25 ++++++++++++++++++++--
include/linux/spi/at86rf230.h | 2 ++
3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt b/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
index d3bbdded..4d47c2e 100644
--- a/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
+++ b/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
@@ -11,6 +11,10 @@ Required properties:
Optional properties:
- reset-gpio: GPIO spec for the rstn pin
- sleep-gpio: GPIO spec for the slp_tr pin
+ - external-xtal: boolean to use external xtal if present
+ - xtal-trim: u8 value to fine tuning the internal capacitance
+ arrays of xtal pins. This value should not above 0x0F
+ and only present when external-xtal is enabled
Example:
@@ -20,4 +24,6 @@ Example:
reg = <0>;
interrupts = <19 1>;
interrupt-parent = <&gpio3>;
+ external-xtal;
+ xtal-trim = /bits/ 8 <0x06>;
};
diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index cbfc8c5..1a95520 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1315,7 +1315,8 @@ 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,
+ const struct at86rf230_platform_data *pdata)
{
int rc, irq_type, irq_pol = IRQ_ACTIVE_HIGH;
unsigned int dvdd;
@@ -1362,6 +1363,16 @@ static int at86rf230_hw_init(struct at86rf230_local *lp)
usleep_range(lp->data->t_sleep_cycle,
lp->data->t_sleep_cycle + 100);
+ if (pdata->xtal) {
+ rc = at86rf230_write_subreg(lp, SR_XTAL_TRIM, pdata->xtal_trim);
+ if (rc)
+ return rc;
+
+ rc = at86rf230_write_subreg(lp, SR_XTAL_MODE, 0x5);
+ if (rc)
+ return rc;
+ }
+
rc = at86rf230_read_subreg(lp, SR_DVDD_OK, &dvdd);
if (rc)
return rc;
@@ -1392,6 +1403,16 @@ at86rf230_get_pdata(struct spi_device *spi)
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);
+ pdata->xtal = of_property_read_bool(spi->dev.of_node, "external-xtal");
+ if (pdata->xtal) {
+ int ret;
+
+ ret = of_property_read_u8(spi->dev.of_node, "xtal-trim",
+ &pdata->xtal_trim);
+ if (ret < 0 || pdata->xtal_trim > 0xF)
+ return NULL;
+ }
+
spi->dev.platform_data = pdata;
done:
return pdata;
@@ -1571,7 +1592,7 @@ static int at86rf230_probe(struct spi_device *spi)
spi_set_drvdata(spi, lp);
- rc = at86rf230_hw_init(lp);
+ rc = at86rf230_hw_init(lp, pdata);
if (rc)
goto free_dev;
diff --git a/include/linux/spi/at86rf230.h b/include/linux/spi/at86rf230.h
index cd519a1..632ad90 100644
--- a/include/linux/spi/at86rf230.h
+++ b/include/linux/spi/at86rf230.h
@@ -22,6 +22,8 @@ struct at86rf230_platform_data {
int rstn;
int slp_tr;
int dig2;
+ bool xtal;
+ u8 xtal_trim;
};
#endif
--
2.3.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bluetooth-next] at86rf230: add support for setting external xtal
2015-02-23 15:05 [PATCH bluetooth-next] at86rf230: add support for setting external xtal Alexander Aring
@ 2015-02-23 15:10 ` Marc Kleine-Budde
2015-02-23 15:13 ` Alexander Aring
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2015-02-23 15:10 UTC (permalink / raw)
To: Alexander Aring, linux-wpan; +Cc: kernel
[-- Attachment #1: Type: text/plain, Size: 3312 bytes --]
On 02/23/2015 04:05 PM, Alexander Aring wrote:
> This patch adds support for setting external xtal. This is recommended
> by the at86rf230 transceivers to reduce power consuming and for a better
> clock source.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
> .../bindings/net/ieee802154/at86rf230.txt | 6 ++++++
> drivers/net/ieee802154/at86rf230.c | 25 ++++++++++++++++++++--
> include/linux/spi/at86rf230.h | 2 ++
> 3 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt b/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
> index d3bbdded..4d47c2e 100644
> --- a/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
> +++ b/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
> @@ -11,6 +11,10 @@ Required properties:
> Optional properties:
> - reset-gpio: GPIO spec for the rstn pin
> - sleep-gpio: GPIO spec for the slp_tr pin
> + - external-xtal: boolean to use external xtal if present
> + - xtal-trim: u8 value to fine tuning the internal capacitance
> + arrays of xtal pins. This value should not above 0x0F
> + and only present when external-xtal is enabled
>
> Example:
>
> @@ -20,4 +24,6 @@ Example:
> reg = <0>;
> interrupts = <19 1>;
> interrupt-parent = <&gpio3>;
> + external-xtal;
> + xtal-trim = /bits/ 8 <0x06>;
> };
> diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> index cbfc8c5..1a95520 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -1315,7 +1315,8 @@ 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,
> + const struct at86rf230_platform_data *pdata)
> {
> int rc, irq_type, irq_pol = IRQ_ACTIVE_HIGH;
> unsigned int dvdd;
> @@ -1362,6 +1363,16 @@ static int at86rf230_hw_init(struct at86rf230_local *lp)
> usleep_range(lp->data->t_sleep_cycle,
> lp->data->t_sleep_cycle + 100);
>
> + if (pdata->xtal) {
> + rc = at86rf230_write_subreg(lp, SR_XTAL_TRIM, pdata->xtal_trim);
> + if (rc)
> + return rc;
> +
> + rc = at86rf230_write_subreg(lp, SR_XTAL_MODE, 0x5);
> + if (rc)
> + return rc;
> + }
> +
> rc = at86rf230_read_subreg(lp, SR_DVDD_OK, &dvdd);
> if (rc)
> return rc;
> @@ -1392,6 +1403,16 @@ at86rf230_get_pdata(struct spi_device *spi)
> 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);
>
> + pdata->xtal = of_property_read_bool(spi->dev.of_node, "external-xtal");
The platform data should be considered read only by the driver. Better
put the information into your per-driver private data struct.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bluetooth-next] at86rf230: add support for setting external xtal
2015-02-23 15:10 ` Marc Kleine-Budde
@ 2015-02-23 15:13 ` Alexander Aring
2015-02-23 15:41 ` Alexander Aring
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Aring @ 2015-02-23 15:13 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-wpan, kernel
On Mon, Feb 23, 2015 at 04:10:58PM +0100, Marc Kleine-Budde wrote:
> On 02/23/2015 04:05 PM, Alexander Aring wrote:
> > This patch adds support for setting external xtal. This is recommended
> > by the at86rf230 transceivers to reduce power consuming and for a better
> > clock source.
> >
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > ---
> > .../bindings/net/ieee802154/at86rf230.txt | 6 ++++++
> > drivers/net/ieee802154/at86rf230.c | 25 ++++++++++++++++++++--
> > include/linux/spi/at86rf230.h | 2 ++
> > 3 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt b/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
> > index d3bbdded..4d47c2e 100644
> > --- a/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
> > +++ b/Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
> > @@ -11,6 +11,10 @@ Required properties:
> > Optional properties:
> > - reset-gpio: GPIO spec for the rstn pin
> > - sleep-gpio: GPIO spec for the slp_tr pin
> > + - external-xtal: boolean to use external xtal if present
> > + - xtal-trim: u8 value to fine tuning the internal capacitance
> > + arrays of xtal pins. This value should not above 0x0F
> > + and only present when external-xtal is enabled
> >
> > Example:
> >
> > @@ -20,4 +24,6 @@ Example:
> > reg = <0>;
> > interrupts = <19 1>;
> > interrupt-parent = <&gpio3>;
> > + external-xtal;
> > + xtal-trim = /bits/ 8 <0x06>;
> > };
> > diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> > index cbfc8c5..1a95520 100644
> > --- a/drivers/net/ieee802154/at86rf230.c
> > +++ b/drivers/net/ieee802154/at86rf230.c
> > @@ -1315,7 +1315,8 @@ 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,
> > + const struct at86rf230_platform_data *pdata)
> > {
> > int rc, irq_type, irq_pol = IRQ_ACTIVE_HIGH;
> > unsigned int dvdd;
> > @@ -1362,6 +1363,16 @@ static int at86rf230_hw_init(struct at86rf230_local *lp)
> > usleep_range(lp->data->t_sleep_cycle,
> > lp->data->t_sleep_cycle + 100);
> >
> > + if (pdata->xtal) {
> > + rc = at86rf230_write_subreg(lp, SR_XTAL_TRIM, pdata->xtal_trim);
> > + if (rc)
> > + return rc;
> > +
> > + rc = at86rf230_write_subreg(lp, SR_XTAL_MODE, 0x5);
> > + if (rc)
> > + return rc;
> > + }
> > +
> > rc = at86rf230_read_subreg(lp, SR_DVDD_OK, &dvdd);
> > if (rc)
> > return rc;
> > @@ -1392,6 +1403,16 @@ at86rf230_get_pdata(struct spi_device *spi)
> > 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);
> >
> > + pdata->xtal = of_property_read_bool(spi->dev.of_node, "external-xtal");
>
> The platform data should be considered read only by the driver. Better
> put the information into your per-driver private data struct.
>
ah, yes you are completely right here. Sorry :)
- Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bluetooth-next] at86rf230: add support for setting external xtal
2015-02-23 15:13 ` Alexander Aring
@ 2015-02-23 15:41 ` Alexander Aring
2015-02-23 15:54 ` Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Aring @ 2015-02-23 15:41 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-wpan, kernel
Marc,
On Mon, Feb 23, 2015 at 04:13:29PM +0100, Alexander Aring wrote:
> On Mon, Feb 23, 2015 at 04:10:58PM +0100, Marc Kleine-Budde wrote:
> > On 02/23/2015 04:05 PM, Alexander Aring wrote:
> > > This patch adds support for setting external xtal. This is recommended
...
> > > rc = at86rf230_read_subreg(lp, SR_DVDD_OK, &dvdd);
> > > if (rc)
> > > return rc;
> > > @@ -1392,6 +1403,16 @@ at86rf230_get_pdata(struct spi_device *spi)
> > > 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);
> > >
> > > + pdata->xtal = of_property_read_bool(spi->dev.of_node, "external-xtal");
> >
> > The platform data should be considered read only by the driver. Better
> > put the information into your per-driver private data struct.
> >
>
> ah, yes you are completely right here. Sorry :)
>
I am not sure if this is okay here in that case. For non devicetree this
code will never touched and I grab this code from some i2c driver like
[0].
They also use the platform-data for setting the devicetree properties.
The only thing which I can see now is the check if (xtal > 0xF) which is
not available on non devicetree settings... but I can also remove this
check, somebody should make something complete wrong if this value is
above 0xF and regmap should mask this value.
- Alex
[0] http://lxr.free-electrons.com/source/drivers/media/i2c/adv7343.c#L404
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bluetooth-next] at86rf230: add support for setting external xtal
2015-02-23 15:41 ` Alexander Aring
@ 2015-02-23 15:54 ` Marc Kleine-Budde
2015-02-23 16:06 ` Alexander Aring
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2015-02-23 15:54 UTC (permalink / raw)
To: Alexander Aring; +Cc: linux-wpan, kernel
[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]
On 02/23/2015 04:41 PM, Alexander Aring wrote:
> Marc,
>
> On Mon, Feb 23, 2015 at 04:13:29PM +0100, Alexander Aring wrote:
>> On Mon, Feb 23, 2015 at 04:10:58PM +0100, Marc Kleine-Budde wrote:
>>> On 02/23/2015 04:05 PM, Alexander Aring wrote:
>>>> This patch adds support for setting external xtal. This is recommended
> ...
>>>> rc = at86rf230_read_subreg(lp, SR_DVDD_OK, &dvdd);
>>>> if (rc)
>>>> return rc;
>>>> @@ -1392,6 +1403,16 @@ at86rf230_get_pdata(struct spi_device *spi)
>>>> 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);
>>>>
>>>> + pdata->xtal = of_property_read_bool(spi->dev.of_node, "external-xtal");
>>>
>>> The platform data should be considered read only by the driver. Better
>>> put the information into your per-driver private data struct.
>>>
>>
>> ah, yes you are completely right here. Sorry :)
>>
>
> I am not sure if this is okay here in that case. For non devicetree this
> code will never touched and I grab this code from some i2c driver like
> [0].
>
> They also use the platform-data for setting the devicetree properties.
That doesn't make it better.
> The only thing which I can see now is the check if (xtal > 0xF) which is
> not available on non devicetree settings... but I can also remove this
> check, somebody should make something complete wrong if this value is
> above 0xF and regmap should mask this value.
IMHO this driver has been poorly converted to DT. The usual way would be
to add data to at86rf230_local.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bluetooth-next] at86rf230: add support for setting external xtal
2015-02-23 15:54 ` Marc Kleine-Budde
@ 2015-02-23 16:06 ` Alexander Aring
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Aring @ 2015-02-23 16:06 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-wpan, kernel
On Mon, Feb 23, 2015 at 04:54:15PM +0100, Marc Kleine-Budde wrote:
> On 02/23/2015 04:41 PM, Alexander Aring wrote:
> > Marc,
> >
> > On Mon, Feb 23, 2015 at 04:13:29PM +0100, Alexander Aring wrote:
> >> On Mon, Feb 23, 2015 at 04:10:58PM +0100, Marc Kleine-Budde wrote:
> >>> On 02/23/2015 04:05 PM, Alexander Aring wrote:
> >>>> This patch adds support for setting external xtal. This is recommended
> > ...
> >>>> rc = at86rf230_read_subreg(lp, SR_DVDD_OK, &dvdd);
> >>>> if (rc)
> >>>> return rc;
> >>>> @@ -1392,6 +1403,16 @@ at86rf230_get_pdata(struct spi_device *spi)
> >>>> 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);
> >>>>
> >>>> + pdata->xtal = of_property_read_bool(spi->dev.of_node, "external-xtal");
> >>>
> >>> The platform data should be considered read only by the driver. Better
> >>> put the information into your per-driver private data struct.
> >>>
> >>
> >> ah, yes you are completely right here. Sorry :)
> >>
> >
> > I am not sure if this is okay here in that case. For non devicetree this
> > code will never touched and I grab this code from some i2c driver like
> > [0].
> >
> > They also use the platform-data for setting the devicetree properties.
>
> That doesn't make it better.
>
> > The only thing which I can see now is the check if (xtal > 0xF) which is
> > not available on non devicetree settings... but I can also remove this
> > check, somebody should make something complete wrong if this value is
> > above 0xF and regmap should mask this value.
>
> IMHO this driver has been poorly converted to DT. The usual way would be
> to add data to at86rf230_local.
Okay, then I will do that and fix the other things and never use another
code for an example. (except the can branch) ;-)
- Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-02-23 16:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-23 15:05 [PATCH bluetooth-next] at86rf230: add support for setting external xtal Alexander Aring
2015-02-23 15:10 ` Marc Kleine-Budde
2015-02-23 15:13 ` Alexander Aring
2015-02-23 15:41 ` Alexander Aring
2015-02-23 15:54 ` Marc Kleine-Budde
2015-02-23 16:06 ` Alexander Aring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.