Re: [PATCH RFC 3/3] arm64: dts: ti: grove: Add Grove Sunlight Sensor overlay

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

 




On 7/4/24 22:25, Andrew Davis wrote:
On 7/3/24 9:15 AM, Ayush Singh wrote:
On 7/2/24 22:14, Andrew Davis wrote:

Add DT overlay for the Grove Sunlight Sensor[0].

[0] https://wiki.seeedstudio.com/Grove-Sunlight_Sensor/

Signed-off-by: Andrew Davis <afd@xxxxxx>
---
  arch/arm64/boot/dts/ti/Makefile               |  3 ++
  .../boot/dts/ti/grove-sunlight-sensor.dtso    | 31 +++++++++++++++++++
  2 files changed, 34 insertions(+)
  create mode 100644 arch/arm64/boot/dts/ti/grove-sunlight-sensor.dtso

diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index a859629a6072c..7d1ce7a5d97bc 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -8,6 +8,9 @@
  # Entries are grouped as per SoC present on the board. Groups are sorted
  # alphabetically.
+# This needs a better directory location
+dtb-$(CONFIG_OF_OVERLAY) += grove-sunlight-sensor.dtbo
+
  # Boards with AM62x SoC
  dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay.dtb
  dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay-csi2-ov5640.dtbo
diff --git a/arch/arm64/boot/dts/ti/grove-sunlight-sensor.dtso b/arch/arm64/boot/dts/ti/grove-sunlight-sensor.dtso
new file mode 100644
index 0000000000000..ab2f102e1f8ab
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/grove-sunlight-sensor.dtso
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/**
+ * Grove - Sunlight Sensor v1.0
+ *
+ * https://wiki.seeedstudio.com/Grove-Sunlight_Sensor/
+ *
+ * Copyright (C) 2024 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+/dts-v1/;
+/plugin/;
+
+&GROVE_CONNECTOR {
+    status = "okay";
+    pinctrl-names = "default";
+    pinctrl-0 = <&GROVE_PIN1_MUX_I2C_SCL>,
+                <&GROVE_PIN2_MUX_I2C_SDA>;
+};

On setting pinctrl in the mikrobus connector, I seem to encounter problem with the SPI driver trying to use the device before the pins are ready. So I think, the pinctrl should probably be defined in the respective i2c, spi, etc nodes instead of connector.


Maybe, I originally did that but the issue there is it can overwrite
any existing pinmux for that IP node. For instance if you add the
pinmux to a GPIO node, any other users of that GPIO lose their mux.

But you are right, they belong in the IP node. Maybe even in the
specific consumer device node (si1145@60 in this case).

The general idea with all of this is that if we have a board in a
static state (with add-ons already attached) we could write a DTS
that fully describes that steady state. Our challenge is to create
an overlay that transforms the base board into what we would have
written in the static case. In the static case we would have added
the pinmux to the IP node, so that is where it belongs.

The issue then is the overlay mechanism is not complete. We
can add properties to nodes, and add nodes to nodes, and append
properties to nodes, but cannot append values to existing properties,
only replace them completely. This gap in the overlay system will
prevent a general solution. So I've started to work on adding
that property appending ability to the overlay system. I should
have some patches posted against the upstream dtc/libfdt here
in the next week or so.

Sure. Will look forward to testing it.



+
+&GROVE_PIN1_I2C {
+    status = "okay";
+    #address-cells = <1>;
+    #size-cells = <0>;
+
+    clock-frequency = <100000>;
+
+    si1145@60 {
+        compatible = "si,si1145";
+        reg = <0x60>;
+    };
+};


I also have question regarding how to define reg property in SPI (chipselect). Ideally, we want to define it relative to the connector pins, but since the SPI device(s) is a child of SPI controller, I am not sure how I can do remapping.


Could you give me an example? Sounds like the interrupt issue, where
we want say the interrupt belonging to "pin 5", but need to specify
it relative to the base interrupt controller, which we cannot know
anything about in the general case. Instead we need a map, from
pin number to both interrupt controller and IRQ number (or SPI
controller and chipselect number, etc..). I think you are on to
something with the GPIO names, select the GPIO by generic name,
not by board specific number. That might be extendable to IRQs
and other numbered items (but yes, that is still an open item
and I'm waiting on some add-on boards to arrive before I can
start testing ideas on this..).


Yes, the same problem will also occur for interrupt. What I am referring to here is the SPI chipselect. In a child of SPI controller, the reg property in child specifies the logical chipselect using a u32 number which is then mapped to the physical chipselect by the controller. So the Mikrobus CS pin might be SPI_CS0 on one system while it might be SPI_CS1 on another. This means we need a way to do the following:


&MIKROBUS_SCK_SPI {
    status = "okay";

    #address-cells = <1>;
    #size-cells = <0>;

    lsm6dsl_click: lsm6dsl-click@MIKROBUS_CS_SPI_REG {
        reg = <MIKROBUS_CS_SPI_REG>;
        compatible = "st,lsm6ds3";
        spi-max-frequency = <1000000>;
    };
};


Additionally, other pins can also be used as chipselect (Eg, SPI Extend Click uses RST and AN as chipselect). Some chipselect might be directly controlled by SPI controller while others might be normal GPIOs which were added using `cs-gpios` property in the controller.


of_spi_parse_dt(): https://github.com/torvalds/linux/blob/795c58e4c7fc6163d8fb9f2baa86cfe898fa4b19/drivers/spi/spi.c#L2468



Andrew


Ayush Singh





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux