* [PATCH 0/3] ieee802154: add support for Cascoda CA8210
@ 2016-10-24 15:04 harrymorris12
2016-10-24 15:04 ` [PATCH 2/3] ieee802154: Add device tree documentation for CA8210 harrymorris12
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: harrymorris12 @ 2016-10-24 15:04 UTC (permalink / raw)
To: linux-wpan; +Cc: alex.aring, stefan, Harry Morris
From: Harry Morris <h.morris@cascoda.com>
This patchset adds a new device driver, documentation and build support for
Cascoda's CA8210 IEEE 802.15.4 radio transceiver:
http://www.cascoda.com/products/ca-821x/
Harry Morris (3):
ieee802154: Add CA8210 IEEE 802.15.4 device driver
ieee802154: Add device tree documentation for CA8210
ieee802154: Add entry in MAINTAINTERS for CA8210 driver
.../devicetree/bindings/net/ieee802154/ca8210.txt | 28 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
MAINTAINERS | 9 +
drivers/net/ieee802154/Kconfig | 18 +
drivers/net/ieee802154/Makefile | 1 +
drivers/net/ieee802154/ca8210.c | 3533 ++++++++++++++++++++
6 files changed, 3590 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
create mode 100644 drivers/net/ieee802154/ca8210.c
--
2.9.3.windows.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] ieee802154: Add device tree documentation for CA8210
2016-10-24 15:04 [PATCH 0/3] ieee802154: add support for Cascoda CA8210 harrymorris12
@ 2016-10-24 15:04 ` harrymorris12
2016-10-25 15:54 ` Stefan Schmidt
2016-10-24 15:04 ` [PATCH 3/3] ieee802154: Add entry in MAINTAINTERS for CA8210 driver harrymorris12
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: harrymorris12 @ 2016-10-24 15:04 UTC (permalink / raw)
To: linux-wpan; +Cc: alex.aring, stefan, Harry Morris
From: Harry Morris <h.morris@cascoda.com>
Signed-off-by: Harry Morris <h.morris@cascoda.com>
---
.../devicetree/bindings/net/ieee802154/ca8210.txt | 28 ++++++++++++++++++++++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
2 files changed, 29 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
diff --git a/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt b/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
new file mode 100644
index 0000000..0297ce8
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
@@ -0,0 +1,28 @@
+* CA8210 IEEE 802.15.4 *
+
+Required properties:
+ - compatible: should be "cascoda,ca8210"
+ - reg: Controlling chip select
+ - spi-max-frequency: Maximum clock speed, should be *less than*
+ 4000000
+ - spi-cpol: Invert clock polarity
+ - reset-gpio: GPIO attached to reset
+ - irq-gpio: GPIO attached to IRQ
+Optional properties:
+ - extclock-enable: Include for the ca8210 to route its 16MHz clock
+ to an output
+ - extclock-freq: Frequency in Hz of the external clock
+ - extclock-gpio: GPIO of the ca8210 to output the clock on
+
+Example:
+ ca8210@0 {
+ compatible = "cascoda,ca8210";
+ reg = <0>;
+ spi-max-frequency = <3000000>;
+ spi-cpol;
+ reset-gpio = <&gpio1 1 GPIO_ACTIVE_HIGH>;
+ irq-gpio = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+ extclock-enable;
+ extclock-freq = 16000000;
+ extclock-gpio = 2;
+ };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d415b38..725a996 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -26,6 +26,7 @@ brcm Broadcom Corporation
buffalo Buffalo, Inc.
calxeda Calxeda
capella Capella Microsystems, Inc
+cascoda Cascoda Ltd.
cavium Cavium, Inc.
cdns Cadence Design Systems Inc.
chrp Common Hardware Reference Platform
--
2.9.3.windows.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] ieee802154: Add entry in MAINTAINTERS for CA8210 driver
2016-10-24 15:04 [PATCH 0/3] ieee802154: add support for Cascoda CA8210 harrymorris12
2016-10-24 15:04 ` [PATCH 2/3] ieee802154: Add device tree documentation for CA8210 harrymorris12
@ 2016-10-24 15:04 ` harrymorris12
2016-10-25 15:54 ` Stefan Schmidt
2016-10-25 15:16 ` [PATCH 0/3] ieee802154: add support for Cascoda CA8210 Stefan Schmidt
[not found] ` <20161024150449.11132-2-h.morris@cascoda.com>
3 siblings, 1 reply; 13+ messages in thread
From: harrymorris12 @ 2016-10-24 15:04 UTC (permalink / raw)
To: linux-wpan; +Cc: alex.aring, stefan, Harry Morris
From: Harry Morris <h.morris@cascoda.com>
Signed-off-by: Harry Morris <h.morris@cascoda.com>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 05e5450..37df5b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2130,6 +2130,15 @@ W: http://www.linux-c6x.org/wiki/index.php/Main_Page
S: Maintained
F: arch/c6x/
+CA8210 IEEE-802.15.4 RADIO DRIVER
+M: Harry Morris <h.morris@cascoda.com>
+M: linuxdev@cascoda.com
+L: linux-wpan@vger.kernel.org
+W: https://github.com/Cascoda/ca8210-linux.git
+S: Maintained
+F: drivers/net/ieee802154/ca8210.c
+F: Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
+
CACHEFILES: FS-CACHE BACKEND FOR CACHING ON MOUNTED FILESYSTEMS
M: David Howells <dhowells@redhat.com>
L: linux-cachefs@redhat.com
--
2.9.3.windows.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] ieee802154: add support for Cascoda CA8210
2016-10-24 15:04 [PATCH 0/3] ieee802154: add support for Cascoda CA8210 harrymorris12
2016-10-24 15:04 ` [PATCH 2/3] ieee802154: Add device tree documentation for CA8210 harrymorris12
2016-10-24 15:04 ` [PATCH 3/3] ieee802154: Add entry in MAINTAINTERS for CA8210 driver harrymorris12
@ 2016-10-25 15:16 ` Stefan Schmidt
[not found] ` <20161024150449.11132-2-h.morris@cascoda.com>
3 siblings, 0 replies; 13+ messages in thread
From: Stefan Schmidt @ 2016-10-25 15:16 UTC (permalink / raw)
To: harrymorris12, linux-wpan; +Cc: alex.aring, Harry Morris
Hello.
On 24/10/16 17:04, harrymorris12@gmail.com wrote:
> From: Harry Morris <h.morris@cascoda.com>
>
> This patchset adds a new device driver, documentation and build support for
> Cascoda's CA8210 IEEE 802.15.4 radio transceiver:
> http://www.cascoda.com/products/ca-821x/
First of all I'm happy to see another company stepping up to support
their 802.15.4 transceiver directly in Linux mainline. Much appreciated!
I will reviewing the driver over the next days and provide you some
feedback.
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] ieee802154: Add CA8210 IEEE 802.15.4 device driver
[not found] ` <20161024150449.11132-2-h.morris@cascoda.com>
@ 2016-10-25 15:54 ` Stefan Schmidt
2016-10-26 9:59 ` Harry Morris
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Schmidt @ 2016-10-25 15:54 UTC (permalink / raw)
To: harrymorris12, linux-wpan; +Cc: alex.aring, Harry Morris
Hello.
Against what kernel did you prepare this patches?
I tried to apply them against bluetooth-next and got the following:
Applying: ieee802154: Add CA8210 IEEE 802.15.4 device driver
error: patch failed: drivers/net/ieee802154/Kconfig:63
error: drivers/net/ieee802154/Kconfig: patch does not apply
error: patch failed: drivers/net/ieee802154/Makefile:3
error: drivers/net/ieee802154/Makefile: patch does not apply
Patch failed at 0001 ieee802154: Add CA8210 IEEE 802.15.4 device driver
The copy of the patch that failed is found in:
/home/stefan/Kernel/bluetooth-next-rpi/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
A guess in the blue from my side would be that you did this patches
against the latest kernel release (4.8) and not the bluetooth-next
kernel repo?
https://git.kernel.org/cgit/linux/kernel/git/bluetooth/bluetooth-next.git/
You would need to rebase your patches on this one so we can apply them
cleanly and test build, run checker scripts, etc.
Given the driver file itself is isolated it will likely just we some
small conflicts in the Makefile and Kconfig.
Patch 2 and 3 fail to apply as well.
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] ieee802154: Add device tree documentation for CA8210
2016-10-24 15:04 ` [PATCH 2/3] ieee802154: Add device tree documentation for CA8210 harrymorris12
@ 2016-10-25 15:54 ` Stefan Schmidt
2016-10-25 16:36 ` Harry Morris
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Schmidt @ 2016-10-25 15:54 UTC (permalink / raw)
To: harrymorris12, linux-wpan; +Cc: alex.aring, Harry Morris
Hello.
On 24/10/16 17:04, harrymorris12@gmail.com wrote:
> From: Harry Morris <h.morris@cascoda.com>
>
> Signed-off-by: Harry Morris <h.morris@cascoda.com>
> ---
> .../devicetree/bindings/net/ieee802154/ca8210.txt | 28 ++++++++++++++++++++++
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> 2 files changed, 29 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
>
> diff --git a/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt b/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
> new file mode 100644
> index 0000000..0297ce8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
> @@ -0,0 +1,28 @@
> +* CA8210 IEEE 802.15.4 *
> +
> +Required properties:
> + - compatible: should be "cascoda,ca8210"
Start should with a capital S here as you do on all other lines. Total
nitpick. :)
> + - reg: Controlling chip select
> + - spi-max-frequency: Maximum clock speed, should be *less than*
> + 4000000
> + - spi-cpol: Invert clock polarity
Is this a mandatory field for the bindings? I would expect one being the
default and used while the other one being an optional property and only
used when the hardware setup needs this. Does this make sense?
> + - reset-gpio: GPIO attached to reset
> + - irq-gpio: GPIO attached to IRQ
> +Optional properties:
> + - extclock-enable: Include for the ca8210 to route its 16MHz clock
> + to an output
> + - extclock-freq: Frequency in Hz of the external clock
> + - extclock-gpio: GPIO of the ca8210 to output the clock on
> +
> +Example:
> + ca8210@0 {
> + compatible = "cascoda,ca8210";
> + reg = <0>;
> + spi-max-frequency = <3000000>;
> + spi-cpol;
> + reset-gpio = <&gpio1 1 GPIO_ACTIVE_HIGH>;
> + irq-gpio = <&gpio1 2 GPIO_ACTIVE_HIGH>;
> + extclock-enable;
> + extclock-freq = 16000000;
> + extclock-gpio = 2;
> + };
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index d415b38..725a996 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -26,6 +26,7 @@ brcm Broadcom Corporation
> buffalo Buffalo, Inc.
> calxeda Calxeda
> capella Capella Microsystems, Inc
> +cascoda Cascoda Ltd.
> cavium Cavium, Inc.
> cdns Cadence Design Systems Inc.
> chrp Common Hardware Reference Platform
>
The rest looks good.
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] ieee802154: Add entry in MAINTAINTERS for CA8210 driver
2016-10-24 15:04 ` [PATCH 3/3] ieee802154: Add entry in MAINTAINTERS for CA8210 driver harrymorris12
@ 2016-10-25 15:54 ` Stefan Schmidt
0 siblings, 0 replies; 13+ messages in thread
From: Stefan Schmidt @ 2016-10-25 15:54 UTC (permalink / raw)
To: harrymorris12, linux-wpan; +Cc: alex.aring, Harry Morris
Hello.
On 24/10/16 17:04, harrymorris12@gmail.com wrote:
> From: Harry Morris <h.morris@cascoda.com>
>
> Signed-off-by: Harry Morris <h.morris@cascoda.com>
> ---
> MAINTAINERS | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 05e5450..37df5b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2130,6 +2130,15 @@ W: http://www.linux-c6x.org/wiki/index.php/Main_Page
> S: Maintained
> F: arch/c6x/
>
> +CA8210 IEEE-802.15.4 RADIO DRIVER
> +M: Harry Morris <h.morris@cascoda.com>
> +M: linuxdev@cascoda.com
> +L: linux-wpan@vger.kernel.org
> +W: https://github.com/Cascoda/ca8210-linux.git
> +S: Maintained
> +F: drivers/net/ieee802154/ca8210.c
> +F: Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
> +
> CACHEFILES: FS-CACHE BACKEND FOR CACHING ON MOUNTED FILESYSTEMS
> M: David Howells <dhowells@redhat.com>
> L: linux-cachefs@redhat.com
>
Looks good to me. You can add my:
Acked-by: Stefan Schmidt <stefan@osg.samsung.com>
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] ieee802154: Add device tree documentation for CA8210
2016-10-25 15:54 ` Stefan Schmidt
@ 2016-10-25 16:36 ` Harry Morris
2016-10-26 22:33 ` Stefan Schmidt
0 siblings, 1 reply; 13+ messages in thread
From: Harry Morris @ 2016-10-25 16:36 UTC (permalink / raw)
To: Stefan Schmidt, linux-wpan; +Cc: alex.aring
Hi Stefan,
On 25/10/2016 16:54, Stefan Schmidt wrote:
> Start should with a capital S here as you do on all other lines. Total
> nitpick. :)
Perfect, I'm sure there's a few more elsewhere I've missed
>> + - reg: Controlling chip select
>> + - spi-max-frequency: Maximum clock speed, should be *less than*
>> + 4000000
>> + - spi-cpol: Invert clock polarity
>
> Is this a mandatory field for the bindings? I would expect one being
> the default and used while the other one being an optional property
> and only used when the hardware setup needs this. Does this make sense?
I'm afraid not, my understanding is that the spi-cpol property is used
by the spi master driver rather than the device driver, so if I need
CPOL=1 (which the CA8210 always does) then the user doesn't have a
choice but to specify spi-cpol here? As omitting would cause the spi
master to use CPOL=0 for the attached slave.
Regards,
Harry
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] ieee802154: Add CA8210 IEEE 802.15.4 device driver
2016-10-25 15:54 ` [PATCH 1/3] ieee802154: Add CA8210 IEEE 802.15.4 device driver Stefan Schmidt
@ 2016-10-26 9:59 ` Harry Morris
0 siblings, 0 replies; 13+ messages in thread
From: Harry Morris @ 2016-10-26 9:59 UTC (permalink / raw)
To: Stefan Schmidt; +Cc: linux-wpan, alex.aring
Hi Stefan,
You're absolutely right, my apologies. I've rebuilt my patches against
the bluetooth-next tree and will re-submit a v2.
Regards,
Harry
On 25/10/2016 16:54, Stefan Schmidt wrote:
> Hello.
>
> Against what kernel did you prepare this patches?
>
> I tried to apply them against bluetooth-next and got the following:
> Applying: ieee802154: Add CA8210 IEEE 802.15.4 device driver
> error: patch failed: drivers/net/ieee802154/Kconfig:63
> error: drivers/net/ieee802154/Kconfig: patch does not apply
> error: patch failed: drivers/net/ieee802154/Makefile:3
> error: drivers/net/ieee802154/Makefile: patch does not apply
> Patch failed at 0001 ieee802154: Add CA8210 IEEE 802.15.4 device driver
> The copy of the patch that failed is found in:
> /home/stefan/Kernel/bluetooth-next-rpi/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> A guess in the blue from my side would be that you did this patches
> against the latest kernel release (4.8) and not the bluetooth-next
> kernel repo?
>
> https://git.kernel.org/cgit/linux/kernel/git/bluetooth/bluetooth-next.git/
>
>
> You would need to rebase your patches on this one so we can apply them
> cleanly and test build, run checker scripts, etc.
>
> Given the driver file itself is isolated it will likely just we some
> small conflicts in the Makefile and Kconfig.
>
> Patch 2 and 3 fail to apply as well.
>
> regards
> Stefan Schmidt
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] ieee802154: Add device tree documentation for CA8210
2016-10-25 16:36 ` Harry Morris
@ 2016-10-26 22:33 ` Stefan Schmidt
2016-10-26 22:51 ` Stefan Schmidt
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Schmidt @ 2016-10-26 22:33 UTC (permalink / raw)
To: h.morris, linux-wpan; +Cc: alex.aring
Hello.
On 25/10/16 18:36, Harry Morris wrote:
> Hi Stefan,
>
> On 25/10/2016 16:54, Stefan Schmidt wrote:
>> Start should with a capital S here as you do on all other lines. Total
>> nitpick. :)
> Perfect, I'm sure there's a few more elsewhere I've missed
>>> + - reg: Controlling chip select
>>> + - spi-max-frequency: Maximum clock speed, should be *less than*
>>> + 4000000
>>> + - spi-cpol: Invert clock polarity
>>
>> Is this a mandatory field for the bindings? I would expect one being
>> the default and used while the other one being an optional property
>> and only used when the hardware setup needs this. Does this make sense?
> I'm afraid not, my understanding is that the spi-cpol property is used
> by the spi master driver rather than the device driver, so if I need
> CPOL=1 (which the CA8210 always does) then the user doesn't have a
> choice but to specify spi-cpol here? As omitting would cause the spi
> master to use CPOL=0 for the attached slave.
I would expect there is a way to set this in the driver itself as it is
the default and the only working mode. Having to set it in the device
tree bindings seems odd as this is really no option or parameter that
can be changed.
Did you look around in other SPI driver so see how they handle such a
situation?
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] ieee802154: Add device tree documentation for CA8210
2016-10-26 22:33 ` Stefan Schmidt
@ 2016-10-26 22:51 ` Stefan Schmidt
2016-10-27 9:52 ` Harry Morris
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Schmidt @ 2016-10-26 22:51 UTC (permalink / raw)
To: h.morris, linux-wpan; +Cc: alex.aring
Hello.
On 27/10/16 00:33, Stefan Schmidt wrote:
> Hello.
>
> On 25/10/16 18:36, Harry Morris wrote:
>> Hi Stefan,
>>
>> On 25/10/2016 16:54, Stefan Schmidt wrote:
>>> Start should with a capital S here as you do on all other lines. Total
>>> nitpick. :)
>> Perfect, I'm sure there's a few more elsewhere I've missed
>>>> + - reg: Controlling chip select
>>>> + - spi-max-frequency: Maximum clock speed, should be *less than*
>>>> + 4000000
>>>> + - spi-cpol: Invert clock polarity
>>>
>>> Is this a mandatory field for the bindings? I would expect one being
>>> the default and used while the other one being an optional property
>>> and only used when the hardware setup needs this. Does this make sense?
>> I'm afraid not, my understanding is that the spi-cpol property is used
>> by the spi master driver rather than the device driver, so if I need
>> CPOL=1 (which the CA8210 always does) then the user doesn't have a
>> choice but to specify spi-cpol here? As omitting would cause the spi
>> master to use CPOL=0 for the attached slave.
>
> I would expect there is a way to set this in the driver itself as it is
> the default and the only working mode. Having to set it in the device
> tree bindings seems odd as this is really no option or parameter that
> can be changed.
>
> Did you look around in other SPI driver so see how they handle such a
> situation?
I did look around myself quickly and it seems indeed the way to set the
clock polarity. Just keep it. Maybe adjust the description to "Requires
invert clock polarity".
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] ieee802154: Add device tree documentation for CA8210
2016-10-26 22:51 ` Stefan Schmidt
@ 2016-10-27 9:52 ` Harry Morris
2016-10-27 10:12 ` Stefan Schmidt
0 siblings, 1 reply; 13+ messages in thread
From: Harry Morris @ 2016-10-27 9:52 UTC (permalink / raw)
To: Stefan Schmidt; +Cc: linux-wpan, alex.aring
Hi Stefan,
On 26/10/2016 23:51, Stefan Schmidt wrote:
> Hello.
>
> On 27/10/16 00:33, Stefan Schmidt wrote:
>> Hello.
>>
>> On 25/10/16 18:36, Harry Morris wrote:
>>> Hi Stefan,
>>>
>>> On 25/10/2016 16:54, Stefan Schmidt wrote:
>>>> Start should with a capital S here as you do on all other lines. Total
>>>> nitpick. :)
>>> Perfect, I'm sure there's a few more elsewhere I've missed
>>>>> + - reg: Controlling chip select
>>>>> + - spi-max-frequency: Maximum clock speed, should be *less
>>>>> than*
>>>>> + 4000000
>>>>> + - spi-cpol: Invert clock polarity
>>>>
>>>> Is this a mandatory field for the bindings? I would expect one being
>>>> the default and used while the other one being an optional property
>>>> and only used when the hardware setup needs this. Does this make
>>>> sense?
>>> I'm afraid not, my understanding is that the spi-cpol property is used
>>> by the spi master driver rather than the device driver, so if I need
>>> CPOL=1 (which the CA8210 always does) then the user doesn't have a
>>> choice but to specify spi-cpol here? As omitting would cause the spi
>>> master to use CPOL=0 for the attached slave.
>>
>> I would expect there is a way to set this in the driver itself as it is
>> the default and the only working mode. Having to set it in the device
>> tree bindings seems odd as this is really no option or parameter that
>> can be changed.
>>
>> Did you look around in other SPI driver so see how they handle such a
>> situation?
>
> I did look around myself quickly and it seems indeed the way to set
> the clock polarity. Just keep it. Maybe adjust the description to
> "Requires invert clock polarity".
I'm not sure which is the "correct" approach but I think you're right
actually, I can indeed set the mode in the spi_device struct from the
driver, eliminating the need to invert the polarity in the device tree.
Since the mode is fixed for the hardware I agree it makes more sense to
set it in the driver rather than the device tree. I'm happy to make the
change.
Thanks,
Harry
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] ieee802154: Add device tree documentation for CA8210
2016-10-27 9:52 ` Harry Morris
@ 2016-10-27 10:12 ` Stefan Schmidt
0 siblings, 0 replies; 13+ messages in thread
From: Stefan Schmidt @ 2016-10-27 10:12 UTC (permalink / raw)
To: h.morris; +Cc: linux-wpan, alex.aring
Hello.
On 27/10/16 11:52, Harry Morris wrote:
> Hi Stefan,
>
> On 26/10/2016 23:51, Stefan Schmidt wrote:
>> Hello.
>>
>> On 27/10/16 00:33, Stefan Schmidt wrote:
>>> Hello.
>>>
>>> On 25/10/16 18:36, Harry Morris wrote:
>>>> Hi Stefan,
>>>>
>>>> On 25/10/2016 16:54, Stefan Schmidt wrote:
>>>>> Start should with a capital S here as you do on all other lines. Total
>>>>> nitpick. :)
>>>> Perfect, I'm sure there's a few more elsewhere I've missed
>>>>>> + - reg: Controlling chip select
>>>>>> + - spi-max-frequency: Maximum clock speed, should be *less
>>>>>> than*
>>>>>> + 4000000
>>>>>> + - spi-cpol: Invert clock polarity
>>>>>
>>>>> Is this a mandatory field for the bindings? I would expect one being
>>>>> the default and used while the other one being an optional property
>>>>> and only used when the hardware setup needs this. Does this make
>>>>> sense?
>>>> I'm afraid not, my understanding is that the spi-cpol property is used
>>>> by the spi master driver rather than the device driver, so if I need
>>>> CPOL=1 (which the CA8210 always does) then the user doesn't have a
>>>> choice but to specify spi-cpol here? As omitting would cause the spi
>>>> master to use CPOL=0 for the attached slave.
>>>
>>> I would expect there is a way to set this in the driver itself as it is
>>> the default and the only working mode. Having to set it in the device
>>> tree bindings seems odd as this is really no option or parameter that
>>> can be changed.
>>>
>>> Did you look around in other SPI driver so see how they handle such a
>>> situation?
>>
>> I did look around myself quickly and it seems indeed the way to set
>> the clock polarity. Just keep it. Maybe adjust the description to
>> "Requires invert clock polarity".
>
> I'm not sure which is the "correct" approach but I think you're right
> actually, I can indeed set the mode in the spi_device struct from the
> driver, eliminating the need to invert the polarity in the device tree.
> Since the mode is fixed for the hardware I agree it makes more sense to
> set it in the driver rather than the device tree. I'm happy to make the
> change.
Keep it like you have it right now (through the bindings). It seems that
this is the way other drivers handle it as well. It just puzled me at first.
Changing the description should help to make it clear that this is
actually needed for the hardware to function.
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-10-27 14:39 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-24 15:04 [PATCH 0/3] ieee802154: add support for Cascoda CA8210 harrymorris12
2016-10-24 15:04 ` [PATCH 2/3] ieee802154: Add device tree documentation for CA8210 harrymorris12
2016-10-25 15:54 ` Stefan Schmidt
2016-10-25 16:36 ` Harry Morris
2016-10-26 22:33 ` Stefan Schmidt
2016-10-26 22:51 ` Stefan Schmidt
2016-10-27 9:52 ` Harry Morris
2016-10-27 10:12 ` Stefan Schmidt
2016-10-24 15:04 ` [PATCH 3/3] ieee802154: Add entry in MAINTAINTERS for CA8210 driver harrymorris12
2016-10-25 15:54 ` Stefan Schmidt
2016-10-25 15:16 ` [PATCH 0/3] ieee802154: add support for Cascoda CA8210 Stefan Schmidt
[not found] ` <20161024150449.11132-2-h.morris@cascoda.com>
2016-10-25 15:54 ` [PATCH 1/3] ieee802154: Add CA8210 IEEE 802.15.4 device driver Stefan Schmidt
2016-10-26 9:59 ` Harry Morris
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.