Re: [PATCH 2/3] ieee802154: Add device tree documentation for CA8210

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux