Hello,
Thanks for your comments
On 05/08/2016 15:39, Mark Rutland wrote:
On Fri, Aug 05, 2016 at 03:31:25PM +0200, Fabien Lahoudere wrote:
+/*
+ * Hardware uses CKO2 at 24MHz at several places. Set the parent clock of
+ * CKO2 to OSC.
+ */
+#define CKO2_CONFIGURATION \
+ clock-frequency = <24000000>; \
+ clocks = <&clks IMX5_CLK_CKO2>; \
+ assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>; \
+ assigned-clock-parents = <&clks IMX5_CLK_OSC>
This makes the DT harder to read.
Please place these explicitly where used.
+
+/ {
+ model = "Freescale i.MX53 CPUV0 PPD rev6";
+ compatible = "fsl,imx53-ppd", "fsl,imx53";
+
+ aliases {
+ spi0 = &cspi;
+ spi1 = &ecspi1;
+ spi2 = &ecspi2;
+ };
+
+ chosen {
+ stdout-path = &uart1;
+ };
It would be good if you could also define the rate, using an alias if
necessary.
+
+ memory {
+ reg = <0x70000000 0x20000000>,
+ <0xb0000000 0x20000000>;
+ };
+
+ clocks {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cko2_11M: sgtl_clock_cko2 {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <11289600>;
+ };
+ };
Get rid of the clocks node, and place the clock directly under the root
node.
+
+ regulators {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg_sgtl5k: regulator@0 {
+ compatible = "regulator-fixed";
+ reg = <0>;
+ regulator-name = "reg-sgtl5k";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
Likewise for these. Get rid of the regulators container node and place
these directly under the root node.
Please don't make fake up reg and unit-address values. Just suffix the
node name, e.g. regulator-sgtl5k.
Ok for all above.
+&ecspi1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ecspi1>;
+ fsl,spi-num-chipselects = <4>;
+ /* Fake this GPIO, this has special handling in userspace. */
NAK. The DT should reflect reality, and should not be specific to a
given userspace.
+ cs-gpios = <&gpio5 1 GPIO_ACTIVE_LOW
+ &gpio4 10 GPIO_ACTIVE_LOW
+ &gpio4 11 GPIO_ACTIVE_LOW
+ &gpio4 12 GPIO_ACTIVE_LOW>;
+ status = "okay";
+
+ spidev0: spi@0 {
+ compatible = "spidev";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ status = "okay";
+ };
+
+ spidev1: spi@1 {
+ compatible = "spidev";
+ reg = <1>;
+ spi-max-frequency = <1000000>;
+ status = "okay";
+ };
+
+ spidev2: spi@2 {
+ compatible = "spidev";
+ reg = <2>;
+ spi-max-frequency = <1000000>;
+ status = "okay";
+ };
+
+ spidev3: spi@3 {
+ compatible = "spidev";
+ reg = <3>;
+ spi-max-frequency = <1000000>;
+ status = "okay";
+ };
+};
What are these SPI devices?
Please fully describe them, with a complete compatible list.
Thanks,
Mark.
I understand what you mean and I agree but spidev0 and spidev1
communicate with another micro with an OS (cortex M4) so it is ok if I
add 'compatible = "fsl,mk20fn1m0vmd12"' in the dts and in spidev.c ?
The 2 other devices are EXAR xra1403 (gpio controller).
--
Fabien Lahoudere
Collabora Ltd.
