| Message ID | 20240702164403.29067-4-afd@ti.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add generic Overlay for Grove Sunlight Sensor | expand |
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@ti.com> > --- > 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. > + > +&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. Ayush Singh
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@ti.com> >> --- >> 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. >> + >> +&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..). Andrew > > Ayush Singh >
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@ti.com> >>> --- >>> 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 >>
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>; +}; + +&GROVE_PIN1_I2C { + status = "okay"; + #address-cells = <1>; + #size-cells = <0>; + + clock-frequency = <100000>; + + si1145@60 { + compatible = "si,si1145"; + reg = <0x60>; + }; +};
Add DT overlay for the Grove Sunlight Sensor[0]. [0] https://wiki.seeedstudio.com/Grove-Sunlight_Sensor/ Signed-off-by: Andrew Davis <afd@ti.com> --- 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