| Message ID | 20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org |
|---|---|
| Headers | show |
| Series | misc: Add mikroBUS driver | expand |
Hi, Could you give us a DT snippet of how this should look like with a board? On Thu Jun 27, 2024 at 6:26 PM CEST, Ayush Singh wrote: > + board: > + description: board attached to mikrobus connector > + $ref: /schemas/types.yaml#/definitions/phandle-array Shouldn't this be a subnode of the connector? i.e. connector { compatible = "mikrobus-connector"; // phandles to the parent controllers spi { temp-sensor@0 { compatible = "maxim,max31855k"; reg = <0>; }; }; i2c { .. }; }; I don't think you can introduce a new compatible = "maxim,max31855k", "mikrobus,spi"; if there is already a binding for "maxim,max31855k". But I might be wrong. Why is this compatible needed at all? Also, the mikrobus-connector driver could translate the chipselects. -michael
On 6/27/24 22:42, Michael Walle wrote: > Hi, > > Could you give us a DT snippet of how this should look like with a > board? > > On Thu Jun 27, 2024 at 6:26 PM CEST, Ayush Singh wrote: >> + board: >> + description: board attached to mikrobus connector >> + $ref: /schemas/types.yaml#/definitions/phandle-array > Shouldn't this be a subnode of the connector? > > i.e. > > connector { > compatible = "mikrobus-connector"; > > // phandles to the parent controllers > > spi { > temp-sensor@0 { > compatible = "maxim,max31855k"; > reg = <0>; > }; > }; > > i2c { > .. > }; > }; > > I don't think you can introduce a new > compatible = "maxim,max31855k", "mikrobus,spi"; > if there is already a binding for "maxim,max31855k". But I might be > wrong. Why is this compatible needed at all? So I did consider the design you just proposed, but I was not able to solve a few issues. 1. How to deal with say 2 mikrobus connectors in a single system? My goal is to have only 1 overlay required for the board config at most. Ideally, I would actually like to add the dt for most mikroBUS boards to upstream and thus only the following overlay would be required: ``` &connector0 { board = <&temp-board>; }; ``` The problem with making it children is that each connector will require seperate overlays for board configs. Additionally, there are boards with 1 wire eeprom available which can theselves store the overlay. In the current setup it will look as follows: ``` &mikrobus_board { thermo-sensor { ... }; }; ``` Which is completely independent of the connector. If the same can be achieved with child-node as well, then I am open to doing that. > > Also, the mikrobus-connector driver could translate the chipselects. > > -michael Yes, so it is currently doing that. Translating chip select name to the actual number. I am doing it the name way since some boards might use pins other than CS as chipselect and not all connectors will allow all pins to be used as chipselect. Ayush Singh
On 6/27/24 22:51, Michael Walle wrote: > On Thu Jun 27, 2024 at 7:07 PM CEST, Andrew Davis wrote: >>> + mikrobus_boards { >>> + thermo_click: thermo-click { >>> + compatible = "maxim,max31855k", "mikrobus-spi"; >> I might be missing something, but your solution cannot possibly be >> to list every click board that could be connected (all 1500+ of them) >> to every mikroBUS connector on every device's DT file.. >> >> Each click board should have a single DTSO overlay file to describe the >> click board, one per click board total. And then that overlay should >> apply cleanly to any device that has a mikroBUS interface. >> >> Which means you have not completely solved the fundamental problem of >> abstracting the mikroBUS connector in DT. Each of these click device child >> nodes has to be under the parent connector node. Which means a phandle >> to the parent node, which is not generically named. For instance >> if my board has 2 connectors, I would have mikrobus0 and mikrobus1, >> the click board's overlay would look like this: >> >> /dts-v1/; >> /plugin/; >> >> &mikrobus0 { >> status = "okay"; >> >> mikrobus_board { >> thermo-click { >> compatible = "maxim,max31855k", "mikrobus-spi"; >> spi-max-frequency = <1000000>; >> pinctrl-apply = "spi_default"; >> }; >> }; >> }; > If there should only be one DT overlay per click board, how would > you apply that to to different connectors? See my other two replies for more context: https://lore.kernel.org/linux-arm-kernel/cef08d49-a462-4167-8b9d-bf09e8aac92f@beagleboard.org/ https://lore.kernel.org/linux-arm-kernel/e0f9754e-4d84-4ab4-82a4-34cb12800927@beagleboard.org/ My ideal design is that most mikroBUS board configs will be defined in a `dtsi` file which can be included by any system with mikroBUS support. This file might look as follows: ``` /dts-v1/; /plugin/; / { mikrobus_boards { thermo_click: thermo-click { compatible = "maxim,max31855k", "mikrobus-spi"; spi-max-frequency = <1000000>; pinctrl-apply = "spi_default"; }; lsm6dsl_click: lsm6dsl-click { compatible = "st,lsm6ds3", "mikrobus-spi"; spi-max-frequency = <1000000>; pinctrl-apply = "spi_default"; }; }; }; ``` And the board overlay will look as follows: ``` &conector { board = <&lsm6dsl_click>; }; ``` I do have an experimental configfs entry that passes the mikroBUS board id(s) and creates and applies the dt changeset to the connector dynamically. > >> I think this solution is almost there, but once you solve the above >> issue, we could just apply the right overlay for our attached click >> board ahead of time and not need the mikroBUS bus driver at all. > The bus driver would still be needed to do the enumeration of the > children, no? And it could make the chip select translations etc. So > you can use the normal bindings of these devices. > > -michael If static dt was the only method to detect the board, then it would be fine. But boards with 1-wire-eeprom can allow for for dynamic detection if the overlay can be system agnostic. To make the dt system agnostic, it should be possible for the board to specify the following information: 1. If a pin is supposed to perform its normal function (UART TX should actually do UART TX), or if it should work as say GPIO (Eg, using RST pin as SPI chipselect). 2. Which pin(s) are the SPI chipselect. 3. If a particular pin needs to be pulled high or low for the board to function, etc. So while most of the normal properties of SPI devices can be reused, there is a need to introduce new properties for additional information. In the previous patches, MikroBUS manifest was being used for this purpose, but for a full dt driver, the device tree needs to be extended to specify these extra properties that are not relevant to the non-mikrobus variant of the device. Ayush Singh
On 6/27/24 12:16 PM, Ayush Singh wrote: > > On 6/27/24 22:37, Andrew Davis wrote: >> On 6/27/24 11:26 AM, Ayush Singh wrote: >>> DONOTMERGE >>> >>> Add mikroBUS connector and some mikroBUS boards support for Beagleplay. >>> The mikroBUS boards node should probably be moved to a more appropriate >>> location but I am not quite sure where it should go since it is not >>> dependent on specific arch. >>> >>> Signed-off-by: Ayush Singh <ayush@beagleboard.org> >>> --- >>> arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 94 +++++++++++++++++++++++--- >>> 1 file changed, 86 insertions(+), 8 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts >>> index 70de288d728e..3f3cd70345c4 100644 >>> --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts >>> +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts >>> @@ -38,6 +38,7 @@ aliases { >>> serial2 = &main_uart0; >>> usb0 = &usb0; >>> usb1 = &usb1; >>> + mikrobus0 = &mikrobus0; >>> }; >>> chosen { >>> @@ -227,6 +228,56 @@ simple-audio-card,codec { >>> }; >>> }; >>> + mikrobus0: mikrobus-connector { >>> + compatible = "mikrobus-connector"; >>> + pinctrl-names = "default", "pwm_default", "pwm_gpio", >>> + "uart_default", "uart_gpio", "i2c_default", >>> + "i2c_gpio", "spi_default", "spi_gpio"; >>> + pinctrl-0 = <&mikrobus_gpio_pins_default>; >>> + pinctrl-1 = <&mikrobus_pwm_pins_default>; >>> + pinctrl-2 = <&mikrobus_pwm_pins_gpio>; >>> + pinctrl-3 = <&mikrobus_uart_pins_default>; >>> + pinctrl-4 = <&mikrobus_uart_pins_gpio>; >>> + pinctrl-5 = <&mikrobus_i2c_pins_default>; >>> + pinctrl-6 = <&mikrobus_i2c_pins_gpio>; >>> + pinctrl-7 = <&mikrobus_spi_pins_default>; >>> + pinctrl-8 = <&mikrobus_spi_pins_gpio>; >>> + >>> + mikrobus-gpio-names = "pwm", "int", "rx", "tx", "scl", "sda", >>> + "mosi", "miso", "sck", "cs", "rst", "an"; >>> + mikrobus-gpios = <&main_gpio1 11 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 9 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 24 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 25 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 22 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 23 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 7 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 8 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 14 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 13 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 12 GPIO_ACTIVE_HIGH>, >>> + <&main_gpio1 10 GPIO_ACTIVE_HIGH>; >>> + >>> + spi-controller = <&main_spi2>; >>> + spi-cs = <0>; >>> + spi-cs-names = "default"; >>> + >>> + board = <&lsm6dsl_click>; >>> + }; >>> + >>> + mikrobus_boards { >>> + thermo_click: thermo-click { >>> + compatible = "maxim,max31855k", "mikrobus-spi"; >> >> I might be missing something, but your solution cannot possibly be >> to list every click board that could be connected (all 1500+ of them) >> to every mikroBUS connector on every device's DT file.. > > > I think you missed something. `mikrobus-boards` is not a child node of `mikrobus0`. See the `board` property in `mikrobus0`. That is what selects the board attached to the connector. > That seems even worse.. That means the board file needs to know about the attached board, which is not how DT works. It describes hardware in a hierarchical/acyclic graph. For instance, take an I2C device, its node is a child of the I2C bus, and has phandle pointers to the IRQ it uses (or whatever else provider it needs). What you have here is like the I2C bus node phandle pointing to the connected child devices. > The `mikcrobus-boards` node itself should be moved to some independent location and included from a system that wants to support mikrobus boards. The connector will only have a phandle to the board (or boards in case a single mikroBUS board has 1 i2c and 1 spi sensor or some combination). > How about providing the full/final example then (this series should be marked as RFC as it is now has missing parts). Move the click board node into a DTSO file and put that in a common location (click boards are common to all boards right, so lets say in drivers/of/click for now just for the RFC). > >> >> Each click board should have a single DTSO overlay file to describe the >> click board, one per click board total. And then that overlay should >> apply cleanly to any device that has a mikroBUS interface. > > > Yes, that is the goal. > > >> >> Which means you have not completely solved the fundamental problem of >> abstracting the mikroBUS connector in DT. Each of these click device child >> nodes has to be under the parent connector node. Which means a phandle >> to the parent node, which is not generically named. For instance >> if my board has 2 connectors, I would have mikrobus0 and mikrobus1, >> the click board's overlay would look like this: >> >> /dts-v1/; >> /plugin/; >> >> &mikrobus0 { >> status = "okay"; >> >> mikrobus_board { >> thermo-click { >> compatible = "maxim,max31855k", "mikrobus-spi"; >> spi-max-frequency = <1000000>; >> pinctrl-apply = "spi_default"; >> }; >> }; >> }; > > > No, it will look as follows: > > ``` > > &mikrobus0 { ^^^ So same issue, what if I want to attach this click board to the second mikrobus connector on my board (i.e. mikrobus1), I'd have to modify the overlay.. Or have an overlay for every possible connector instance number. > status = "okay"; > > board = <&thermo-click>; > > }; > > > &mikrobus_board { > thermo-click { > compatible = "maxim,max31855k", "mikrobus-spi"; > spi-max-frequency = <1000000>; > pinctrl-apply = "spi_default"; > }; > }; > > ``` > > >> >> I think this solution is almost there, but once you solve the above >> issue, we could just apply the right overlay for our attached click >> board ahead of time and not need the mikroBUS bus driver at all. > > > Well, the driver is still needed because some things cannot be done generically in dt. Eg: > > 1. SPI chipselect. Each connector will have different chipselect number mapped to CS pin. In fact a mikrobus board might use other pins like RST as chipselect as well. > > 2. Using pins other than their intended purpose like GPIO. > Then these are two things you'll need to solve. We can add these functions to the base DT/OF support if they are generic problems (which they are, other connectors/daughterboards have this same issue, RPi header, Seeed Grove connector, Sparkfun QWIIC headers, etc..). Andrew > >> >> Andrew >> >>> + spi-max-frequency = <1000000>; >>> + pinctrl-apply = "spi_default"; >>> + }; >>> + >>> + lsm6dsl_click: lsm6dsl-click { >>> + compatible = "st,lsm6ds3", "mikrobus-spi"; >>> + spi-max-frequency = <1000000>; >>> + pinctrl-apply = "spi_default"; >>> + }; >>> + }; >>> }; >>> &main_pmx0 { >>> @@ -387,6 +438,18 @@ AM62X_IOPAD(0x01f0, PIN_OUTPUT, 5) /* (A18) EXT_REFCLK1.CLKOUT0 */ >>> >; >>> }; >>> + mikrobus_pwm_pins_default: mikrobus-pwm-default-pins { >>> + pinctrl-single,pins = < >>> + AM62X_IOPAD(0x01a4, PIN_INPUT, 2) /* (B20) MCASP0_ACLKX.ECAP2_IN_APWM_OUT */ >>> + >; >>> + }; >>> + >>> + mikrobus_pwm_pins_gpio: mikrobus-pwm-gpio-pins { >>> + pinctrl-single,pins = < >>> + AM62X_IOPAD(0x01a4, PIN_INPUT, 7) /* (B20) MCASP0_ACLKX.GPIO1_11 */ >>> + >; >>> + }; >>> + >>> mikrobus_i2c_pins_default: mikrobus-i2c-default-pins { >>> pinctrl-single,pins = < >>> AM62X_IOPAD(0x01d0, PIN_INPUT_PULLUP, 2) /* (A15) UART0_CTSn.I2C3_SCL */ >>> @@ -394,6 +457,13 @@ AM62X_IOPAD(0x01d4, PIN_INPUT_PULLUP, 2) /* (B15) UART0_RTSn.I2C3_SDA */ >>> >; >>> }; >>> + mikrobus_i2c_pins_gpio: mikrobus-i2c-gpio-pins { >>> + pinctrl-single,pins = < >>> + AM62X_IOPAD(0x01d0, PIN_INPUT, 7) /* (A15) UART0_CTSn.GPIO1_22 */ >>> + AM62X_IOPAD(0x01d4, PIN_INPUT, 7) /* (B15) UART0_RTSn.GPIO1_23 */ >>> + >; >>> + }; >>> + >>> mikrobus_uart_pins_default: mikrobus-uart-default-pins { >>> pinctrl-single,pins = < >>> AM62X_IOPAD(0x01d8, PIN_INPUT, 1) /* (C15) MCAN0_TX.UART5_RXD */ >>> @@ -401,6 +471,13 @@ AM62X_IOPAD(0x01dc, PIN_OUTPUT, 1) /* (E15) MCAN0_RX.UART5_TXD */ >>> >; >>> }; >>> + mikrobus_uart_pins_gpio: mikrobus-uart-gpio-pins { >>> + pinctrl-single,pins = < >>> + AM62X_IOPAD(0x01d8, PIN_INPUT, 7) /* (C15) MCAN0_TX.GPIO1_24 */ >>> + AM62X_IOPAD(0x01dc, PIN_INPUT, 7) /* (E15) MCAN0_RX.GPIO1_25 */ >>> + >; >>> + }; >>> + >>> mikrobus_spi_pins_default: mikrobus-spi-default-pins { >>> pinctrl-single,pins = < >>> AM62X_IOPAD(0x01b0, PIN_INPUT, 1) /* (A20) MCASP0_ACLKR.SPI2_CLK */ >>> @@ -410,6 +487,15 @@ AM62X_IOPAD(0x0198, PIN_INPUT, 1) /* (A19) MCASP0_AXR2.SPI2_D1 */ >>> >; >>> }; >>> + mikrobus_spi_pins_gpio: mikrobus-spi-gpio-pins { >>> + pinctrl-single,pins = < >>> + AM62X_IOPAD(0x0194, PIN_INPUT, 7) /* (B19) MCASP0_AXR3.GPIO1_7 */ >>> + AM62X_IOPAD(0x0198, PIN_INPUT, 7) /* (A19) MCASP0_AXR2.GPIO1_8 */ >>> + AM62X_IOPAD(0x01ac, PIN_INPUT, 7) /* (E19) MCASP0_AFSR.GPIO1_13 */ >>> + AM62X_IOPAD(0x01b0, PIN_INPUT, 7) /* (A20) MCASP0_ACLKR.GPIO1_14 */ >>> + >; >>> + }; >>> + >>> mikrobus_gpio_pins_default: mikrobus-gpio-default-pins { >>> bootph-all; >>> pinctrl-single,pins = < >>> @@ -630,8 +716,6 @@ &main_gpio0 { >>> &main_gpio1 { >>> bootph-all; >>> - pinctrl-names = "default"; >>> - pinctrl-0 = <&mikrobus_gpio_pins_default>; >>> gpio-line-names = "", "", "", "", "", /* 0-4 */ >>> "SPE_RSTN", "SPE_INTN", "MIKROBUS_GPIO1_7", /* 5-7 */ >>> "MIKROBUS_GPIO1_8", "MIKROBUS_GPIO1_9", /* 8-9 */ >>> @@ -804,15 +888,11 @@ it66121_out: endpoint { >>> }; >>> &main_i2c3 { >>> - pinctrl-names = "default"; >>> - pinctrl-0 = <&mikrobus_i2c_pins_default>; >>> clock-frequency = <400000>; >>> status = "okay"; >>> }; >>> &main_spi2 { >>> - pinctrl-names = "default"; >>> - pinctrl-0 = <&mikrobus_spi_pins_default>; >>> status = "okay"; >>> }; >>> @@ -876,8 +956,6 @@ &main_uart1 { >>> }; >>> &main_uart5 { >>> - pinctrl-names = "default"; >>> - pinctrl-0 = <&mikrobus_uart_pins_default>; >>> status = "okay"; >>> }; >>> > > Ayush Singh >
> That's then per board, per click board. right? > > > > > ``` > > > > > > The problem with making it children is that each connector will require > > seperate overlays for board configs. > > Right. For somebody who has not used overlays, could you expand on that. Is it not possible to say "Overlay this DT fragment on point X of the tree". So you populate children on a node. It should not matter if you have the same children somewhere else, they have different parents? Andrew
On Thu, 27 Jun 2024 21:56:13 +0530, Ayush Singh wrote: > Add bindings for MikroBUS boards using SPI interface. > > Almost all of the properties that are valid for SPI devices can be used > except reg. Since the goal is to allow use of the same MikroBUS board > across different connectors, config needs to be independent of the actual > SPI controller in mikroBUS port(s), it is not possible to define the > chipselect by number in advance. Thus, `spi-cs-apply` property is used to > specify the chipselect(s) by name. > > Another important fact is that while there is a CS pin in the mikroBUS > connector, some boards (eg SPI Extend Click) use additional pins as > chipselect. Thus we need a way to specify the CS pin(s) in terms of > mikcrobus-connector which can then handle bindings the actual CS pin(s). > > Link: https://www.mikroe.com/spi-extend-click SPI Extend Click > > Signed-off-by: Ayush Singh <ayush@beagleboard.org> > --- > .../devicetree/bindings/mikrobus/mikrobus-spi.yaml | 37 ++++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 38 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mikrobus/mikrobus-spi.example.dtb: thermo-click: compatible: ['maxim,max31855k', 'mikrobus,spi'] is too long from schema $id: http://devicetree.org/schemas/iio/temperature/maxim,max31855k.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mikrobus/mikrobus-spi.example.dtb: thermo-click: 'reg' is a required property from schema $id: http://devicetree.org/schemas/iio/temperature/maxim,max31855k.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mikrobus/mikrobus-spi.example.dtb: thermo-click: Unevaluated properties are not allowed ('compatible', 'pinctrl-apply', 'spi-cs-apply' were unexpected) from schema $id: http://devicetree.org/schemas/iio/temperature/maxim,max31855k.yaml# Documentation/devicetree/bindings/mikrobus/mikrobus-spi.example.dtb: /example-0/thermo-click: failed to match any schema with compatible: ['maxim,max31855k', 'mikrobus,spi'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240627-mikrobus-scratch-spi-v5-3-9e6c148bf5f0@beagleboard.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 6/27/24 21:56, Ayush Singh wrote: > MikroBUS is an open standard developed by MikroElektronika for connecting > add-on boards to microcontrollers or microprocessors. It essentially > allows you to easily expand the functionality of your main boards using > these add-on boards. > > This patchset adds mikroBUS as a Linux bus type and provides a driver to > parse and register the mikroBUS board using device tree infrastructure. > > The patchset is based on work originally done by Vaishnav. > > Link: https://www.mikroe.com/mikrobus > Link: https://docs.beagleboard.org/latest/boards/beagleplay/ > Link: https://lore.kernel.org/all/20240317193714.403132-1-ayushdevel1325@gmail.com/ Patch v4 > > Changes v5 > - Complete rewrite to use device tree instead of mikroBUS manifest. > - Only support for SPI. > - Adds `mikrobus,spi` compatible property. > > Changes v4: > - Better commit messages > - Remove clickID, serdev, pwm, regulator, clocks etc. Just the basic > mikroBUS driver. > - Fix a lot of memory leaks, unused variables, etc. > - Create accompanying PR in Greybus Spec repository > - Switch to 80 columns formatting > - Some other fixes pointed out in v3 > > Changes in v3: > - Use phandle instead of busname for spi > - Use spi board info for registering new device > - Convert dt bindings to yaml > - Add support for clickID > - Code cleanup and style changes > - Additions required to spi, serdev, w1 and regulator subsystems > > Changes in v2: > - support for adding mikroBUS ports from DT overlays, > - remove debug sysFS interface for adding mikrobus ports, > - consider extended pin usage/deviations from mikrobus standard > specifications > - use greybus CPort protocol enum instead of new protocol enums > - Fix cases of wrong indentation, ignoring return values, freeing allocated > resources in case of errors and other style suggestions in v1 review. > > Signed-off-by: Ayush Singh <ayush@beagleboard.org> > --- > Ayush Singh (7): > dt-bindings: connector: Add mikrobus-connector > dt-bindings: mikrobus: Add mikrobus board base > dt-bindings: mikrobus: Add mikrobus-spi binding > spi: Make of_find_spi_controller_by_node() available > spi: Make of_register_spi_device() available > mikrobus: Add mikroBUS driver > dts: ti: k3-am625-beagleplay: Add mikroBUS > > .../bindings/connector/mikrobus-connector.yaml | 107 ++++++ > .../bindings/mikrobus/mikrobus-board.yaml | 20 ++ > .../devicetree/bindings/mikrobus/mikrobus-spi.yaml | 37 +++ > MAINTAINERS | 9 + > arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 94 +++++- > drivers/misc/Kconfig | 16 + > drivers/misc/Makefile | 1 + > drivers/misc/mikrobus.c | 361 +++++++++++++++++++++ > drivers/spi/spi.c | 209 ++++++------ > include/linux/spi/spi.h | 7 + > 10 files changed, 750 insertions(+), 111 deletions(-) > --- > base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7 > change-id: 20240627-mikrobus-scratch-spi-ad8c98dcec98 > > Best regards, I would just like to summarize the discussions on different patches here to give information regarding why the board is not subnode of mikrobus-connector along with what questions need to be answered for a subnode based architecture. I will be using (```) to differentiate between code section and non-code section. It is just for seperation not for any formatting since I am using plaintext. Let me first summarise the goals that should be possible with any architecture chosen. 1. Keeping the device tree properties upstream in a system independent way. 2. Editing system dt at kernel build time to add the pre-defined board or applying dt overlay using uboot or dynamic overlays. 3. Allowing creation of sysfs entries `new_device` and `delete_device` similar to what already exists for I2C, etc. 4. Allow using 1-wire-eeprom in a fashion that allows automatic board discovery. Let me now introduce the 2 architectures we will be discussing: 1. mikrobus-connector has phandle to mikrobus-board: ``` \ { connector1 { board = <&board1>; }; mikrobus_boards { board1 { ... }; }; }; ``` 2. mikrobus board is a child node of mikrobus-connector: ``` \ { connector1 { ... spi { board1 { ... }; }; }; }; ``` I will now go over how each of these goals might look like in both of the architecture. 1. Keeping the device tree properties upstream in a system independent way: a. mikrobus-connector has phandle to mikrobus-board It is possible to create an overlay as follows which will work with any system that defines the `mikrobus_boards` node. This node is completely independent of mikroBUS connector and thus does not need to be rewritten (or generated) for each board. There are no problems for system with more than 1 mikrobus connector. ``` &mikrobus_boards { board2 { ... }; board3 { ... }; }; ``` b. mikrobus board is a child node of mikrobus-connector: Not sure how to do something similar here. The overlay needs to be rewritten (or generated) for each board. Systems with multiple mikrobus connectors will need multiple overlays adding the boards as child node of each connector (with status = "disabled"). Considering how many mikrobus boards are available, this can also lead to problem (especially in embeded Linux) with the dt binary size since each connector is replicating the same overlay. ``` &connector1 { spi = { board 2 { ... }; board 3 { ... }; }; }; &connector2 { spi = { board 2 { ... }; board 3 { ... }; }; }; ``` Maybe it is possible to have special behavior for mikrobus-connector nodes in dt overlay but that will break compatibility with exisiting infrastructure which isn't great. 2. Editing system dt at kernel build time to add the pre-defined board or applying dt overlay using uboot or dynamic overlays. a. mikrobus-connector has phandle to mikrobus-board ``` &connector1 { board = <&board1>; }; ``` b. mikrobus board is a child node of mikrobus-connector: ``` &connector1 { spi = { board 2 { ... }; }; }; ``` Both the cases will need to generate these overlays at build time. However, in case (a), the overlay will be much smaller than case (b). This is important for embeded Linux. 3. Allowing creation of sysfs entries `new_device` and `delete_device` similar to what already exists for I2C, etc. a. mikrobus-connector has phandle to mikrobus-board It is quite simple with the current changeset APIs. I have an example implementation here: https://github.com/Ayush1325/linux/blob/c4e3d5138b7ad5c24bdbc1dd02d89720d3a5de82/drivers/misc/mikrobus.c#L59 . Essentially, it is possible to pass the mikroBUS board name or id to create changeset as long as the board has been defined in dt. The boards definition can be added using overlay in uboot of dynamic overlays using configfs patch. b. mikrobus board is a child node of mikrobus-connector: Since even the board definition overlay is now dependent on the connector, any person writing the board overlay needs to know the name's of the connector nodes and generate overlays for all connectors. We can toggle a `status` property to `okay` based on the board id passed through sysfs. 4. Allow using 1-wire-eeprom in a fashion that allows automatic board discovery. a. mikrobus-connector has phandle to mikrobus-board 1-wire-eeprom only needs to contain the board definition overlay which is not dependent on the connector. The connector can generate the changeset of add `board` property to itself. The board should work irrespective of if the dt overlay is actually present in the kernel config since we can read the overlay from 1-wire-eeprom and apply it using `of_overlay_fdt_apply()`. b. mikrobus board is a child node of mikrobus-connector: Cannot really use the normal dt overlay. Maybe we can use the mikroBUS manifest to dynamically create the overlay, but well, I do not wish to support both the manifest and devicetree at the same time. Maybe we can introduce something like partial device tree which only contains properties to be applied to a target device node? Since `of_overlay_fdt_apply` does contain target node property, maybe it is already possible to have an overlay that is generic over a type of node instead of the exact node? I will also go through the overlay kernel internals to see if there are any better ways to use child-nodes. Feel free to chime in if you have any ideas. Yours Sincerely, Ayush Singh
On Thu, Jun 27, 2024 at 09:56:17PM +0530, Ayush Singh wrote: > + mikrobus_boards { > + thermo_click: thermo-click { > + compatible = "maxim,max31855k", "mikrobus-spi"; > + spi-max-frequency = <1000000>; > + pinctrl-apply = "spi_default"; > + }; > + > + lsm6dsl_click: lsm6dsl-click { > + compatible = "st,lsm6ds3", "mikrobus-spi"; > + spi-max-frequency = <1000000>; > + pinctrl-apply = "spi_default"; So every single device that could possibly go on a mikrobus board will need a new binding (or significant binding modifications)? tbh, I was expecting something where the mikrobus connector looks like what "spi-mux" does. Ditto "i2c-mux" for the i2c component.
On Thu, 27 Jun 2024 21:56:10 +0530, Ayush Singh wrote: > MikroBUS is an open standard developed by MikroElektronika for connecting > add-on boards to microcontrollers or microprocessors. It essentially > allows you to easily expand the functionality of your main boards using > these add-on boards. > > This patchset adds mikroBUS as a Linux bus type and provides a driver to > parse and register the mikroBUS board using device tree infrastructure. > > The patchset is based on work originally done by Vaishnav. > > Link: https://www.mikroe.com/mikrobus > Link: https://docs.beagleboard.org/latest/boards/beagleplay/ > Link: https://lore.kernel.org/all/20240317193714.403132-1-ayushdevel1325@gmail.com/ Patch v4 > > Changes v5 > - Complete rewrite to use device tree instead of mikroBUS manifest. > - Only support for SPI. > - Adds `mikrobus,spi` compatible property. > > Changes v4: > - Better commit messages > - Remove clickID, serdev, pwm, regulator, clocks etc. Just the basic > mikroBUS driver. > - Fix a lot of memory leaks, unused variables, etc. > - Create accompanying PR in Greybus Spec repository > - Switch to 80 columns formatting > - Some other fixes pointed out in v3 > > Changes in v3: > - Use phandle instead of busname for spi > - Use spi board info for registering new device > - Convert dt bindings to yaml > - Add support for clickID > - Code cleanup and style changes > - Additions required to spi, serdev, w1 and regulator subsystems > > Changes in v2: > - support for adding mikroBUS ports from DT overlays, > - remove debug sysFS interface for adding mikrobus ports, > - consider extended pin usage/deviations from mikrobus standard > specifications > - use greybus CPort protocol enum instead of new protocol enums > - Fix cases of wrong indentation, ignoring return values, freeing allocated > resources in case of errors and other style suggestions in v1 review. > > Signed-off-by: Ayush Singh <ayush@beagleboard.org> > --- > Ayush Singh (7): > dt-bindings: connector: Add mikrobus-connector > dt-bindings: mikrobus: Add mikrobus board base > dt-bindings: mikrobus: Add mikrobus-spi binding > spi: Make of_find_spi_controller_by_node() available > spi: Make of_register_spi_device() available > mikrobus: Add mikroBUS driver > dts: ti: k3-am625-beagleplay: Add mikroBUS > > .../bindings/connector/mikrobus-connector.yaml | 107 ++++++ > .../bindings/mikrobus/mikrobus-board.yaml | 20 ++ > .../devicetree/bindings/mikrobus/mikrobus-spi.yaml | 37 +++ > MAINTAINERS | 9 + > arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 94 +++++- > drivers/misc/Kconfig | 16 + > drivers/misc/Makefile | 1 + > drivers/misc/mikrobus.c | 361 +++++++++++++++++++++ > drivers/spi/spi.c | 209 ++++++------ > include/linux/spi/spi.h | 7 + > 10 files changed, 750 insertions(+), 111 deletions(-) > --- > base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7 > change-id: 20240627-mikrobus-scratch-spi-ad8c98dcec98 > > Best regards, > -- > Ayush Singh <ayush@beagleboard.org> > > > My bot found new DTB warnings on the .dts files added or changed in this series. Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments. If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date: pip3 install dtschema --upgrade New warnings running 'make CHECK_DTBS=y ti/k3-am625-beagleplay.dtb' for 20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org: arch/arm64/boot/dts/ti/k3-am625-beagleplay.dtb: mikrobus-connector: mikrobus-gpio-names:6: 'mosi' is not one of ['pwm', 'int', 'rx', 'tx', 'scl', 'sda', 'an', 'rst', 'cs', 'sck', 'cipo', 'copi'] from schema $id: http://devicetree.org/schemas/connector/mikrobus-connector.yaml# arch/arm64/boot/dts/ti/k3-am625-beagleplay.dtb: mikrobus-connector: mikrobus-gpio-names:7: 'miso' is not one of ['pwm', 'int', 'rx', 'tx', 'scl', 'sda', 'an', 'rst', 'cs', 'sck', 'cipo', 'copi'] from schema $id: http://devicetree.org/schemas/connector/mikrobus-connector.yaml# arch/arm64/boot/dts/ti/k3-am625-beagleplay.dtb: thermo-click: compatible: ['maxim,max31855k', 'mikrobus-spi'] is too long from schema $id: http://devicetree.org/schemas/iio/temperature/maxim,max31855k.yaml# arch/arm64/boot/dts/ti/k3-am625-beagleplay.dtb: thermo-click: 'reg' is a required property from schema $id: http://devicetree.org/schemas/iio/temperature/maxim,max31855k.yaml# arch/arm64/boot/dts/ti/k3-am625-beagleplay.dtb: thermo-click: Unevaluated properties are not allowed ('compatible', 'pinctrl-apply' were unexpected) from schema $id: http://devicetree.org/schemas/iio/temperature/maxim,max31855k.yaml# arch/arm64/boot/dts/ti/k3-am625-beagleplay.dtb: lsm6dsl-click: compatible: 'oneOf' conditional failed, one must be fixed: ['st,lsm6ds3', 'mikrobus-spi'] is too long 'st,lsm6ds3' is not one of ['st,asm330lhhx', 'st,asm330lhhxg1'] 'st,lsm6dstx' was expected 'st,lsm6dsv16x' was expected 'st,ism330is' was expected 'st,asm330lhb' was expected 'st,lsm6dsr' was expected 'st,lsm6dst' was expected 'st,lsm6dsv' was expected 'st,lsm6dso16is' was expected 'st,asm330lhh' was expected from schema $id: http://devicetree.org/schemas/iio/imu/st,lsm6dsx.yaml# arch/arm64/boot/dts/ti/k3-am625-beagleplay.dtb: lsm6dsl-click: 'reg' is a required property from schema $id: http://devicetree.org/schemas/iio/imu/st,lsm6dsx.yaml# arch/arm64/boot/dts/ti/k3-am625-beagleplay.dtb: lsm6dsl-click: Unevaluated properties are not allowed ('compatible', 'pinctrl-apply' were unexpected) from schema $id: http://devicetree.org/schemas/iio/imu/st,lsm6dsx.yaml#
On Thu, Jun 27, 2024 at 09:56:11PM +0530, Ayush Singh wrote: > Add DT bindings for mikroBUS interface. MikroBUS is an open standard > developed by MikroElektronika for connecting add-on boards to > microcontrollers or microprocessors. > > mikroBUS is a connector and does not have a controller. Instead the > software is responsible for identification of board and setting up uart, > spi, i2c, pwm and other buses. Additionally, some new mikroBUS boards > contain 1-wire EEPROM that contains a manifest to describe the addon > board to provide plug and play capabilities. > > A mikroBUS addon board is free to leave some of the pins unused which > are marked as NC or Not Connected. > > Some of the pins might need to be configured as GPIOs deviating from their > reserved purposes Eg: SHT15 Click where the SCL and SDA Pins need to be > configured as GPIOs for the driver (drivers/hwmon/sht15.c) to work. > > For some add-on boards the driver may not take care of some additional > signals like reset/wake-up/other. Eg: ENC28J60 click where the reset line > (RST pin on the mikrobus port) needs to be pulled high. > > Some SPI addon boards use other pins like RST, AN etc as chipselect (eg. > SPI Extend Click). Thus, `spi-cs` and `spi-cs-names` property is added > to allow mikroBUS addon board to specify chipselect by name. > > Here's the list of pins in mikroBUS connector: > AN - Analog > RST - Reset > CS - SPI Chip Select > SCK - SPI Clock > MISO - SPI Master Input Slave Output > MOSI - SPI Master Output Slave Input > +3.3V - VCC-3.3V power > GND - Reference Ground > PWM - PWM output > INT - Hardware Interrupt > RX - UART Receive > TX - UART Transmit > SCL - I2C Clock > SDA - I2C Data > +5V - VCC-5V power > GND - Reference Ground > > Link: https://www.mikroe.com/mikrobus > Link: > https://download.mikroe.com/documents/standards/mikrobus/mikrobus-standard-specification-v200.pdf > mikroBUS specification > Link: https://www.mikroe.com/sht1x-click SHT15 Click > Link: https://www.mikroe.com/eth-click ENC28J60 Click > Link: https://www.mikroe.com/spi-extend-click SPI Extend Click > > Co-developed-by: Vaishnav M A <vaishnav@beagleboard.org> > Signed-off-by: Vaishnav M A <vaishnav@beagleboard.org> > Signed-off-by: Ayush Singh <ayush@beagleboard.org> > --- > .../bindings/connector/mikrobus-connector.yaml | 107 +++++++++++++++++++++ > MAINTAINERS | 6 ++ > 2 files changed, 113 insertions(+) > > diff --git a/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml > new file mode 100644 > index 000000000000..033479f8604f > --- /dev/null > +++ b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml > @@ -0,0 +1,107 @@ > +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/connector/mikrobus-connector.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: mikroBUS add-on board connector > + > +maintainers: > + - Ayush Singh <ayush@beagleboard.org> > + > +properties: > + compatible: > + const: mikrobus-connector > + > + pinctrl-0: true > + pinctrl-1: true > + pinctrl-2: true > + pinctrl-3: true > + pinctrl-4: true > + pinctrl-5: true > + pinctrl-6: true > + pinctrl-7: true > + pinctrl-8: true > + > + pinctrl-names: > + minItems: 1 > + maxItems: 9 > + items: > + enum: [default, pwm_default, pwm_gpio, uart_default, uart_gpio, i2c_default, i2c_gpio, spi_default, > + spi_gpio] Generally, each pinctrl-N is mutually exclusive. It looks like here you want multiple states active at one time. Does this work? > + > + spi-controller: > + description: spi-controller of mikroBUS SPI pins along with cs pins. > + $ref: /schemas/types.yaml#/definitions/phandle > + > + spi-cs: > + description: spi chip-select corresponding to the chip-selects on the mikrobus socket. Wrap lines at 80 char. The array index is the chip-select number on the connector and the value is the host SPI controller CS numbers? Or the other way around? This needs a better description. > + $ref: /schemas/types.yaml#/definitions/uint32-array Maximum number of entries? > + > + spi-cs-names: > + minItems: 1 > + maxItems: 12 > + items: > + enum: [default, pwm, int, rx, tx, scl, sda, an, rst, sck, cipo, copi] > + > + i2c-controller: > + description: i2c controller attached to the mikrobus socket. > + $ref: /schemas/types.yaml#/definitions/phandle i2c-bus is the somewhat standard property for this. Really, I'd expect connectors to look something like this: connector { i2c-0 { i2c-bus = <&i2c3>; #address-cells = <1>; #size-cells = <0>; device@12 { compatible = "some-i2c-device"; }; }; }; That form allows for multiple buses (of the same type or different) on the connector. > + > + uart-controller: > + description: uart controller attached to the mikrobus socket > + $ref: /schemas/types.yaml#/definitions/phandle > + > + pwms: > + description: the pwm-controller corresponding to the mikroBUS PWM pin. > + maxItems: 1 > + > + mikrobus-gpios: > + minItems: 1 > + maxItems: 12 > + > + mikrobus-gpio-names: The GPIO binding does not work this way as the name is in the property. Either drop if you want to keep the array or you have to do something like this: pwm-gpios int-gpios rx-gpios Really, the intention was for connectors to use gpio-map property to renumber GPIOs relative to the connector. > + minItems: 1 > + maxItems: 12 > + items: > + enum: [pwm, int, rx, tx, scl, sda, an, rst, cs, sck, cipo, copi] > + > + board: > + description: board attached to mikrobus connector > + $ref: /schemas/types.yaml#/definitions/phandle-array What is this for? > + > +required: > + - compatible > + - pinctrl-0 > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + mikrobus { > + compatible = "mikrobus-connector"; > + pinctrl-names = "default", "pwm_default", "pwm_gpio","uart_default", "uart_gpio", "i2c_default", > + "i2c_gpio", "spi_default", "spi_gpio"; > + pinctrl-0 = <&P2_03_gpio_input_pin &P1_04_gpio_pin &P1_02_gpio_pin>; > + pinctrl-1 = <&P2_01_pwm_pin>; > + pinctrl-2 = <&P2_01_gpio_pin>; > + pinctrl-3 = <&P2_05_uart_pin &P2_07_uart_pin>; > + pinctrl-4 = <&P2_05_gpio_pin &P2_07_gpio_pin>; > + pinctrl-5 = <&P2_09_i2c_pin &P2_11_i2c_pin>; > + pinctrl-6 = <&P2_09_gpio_pin &P2_11_gpio_pin>; > + pinctrl-7 = <&P1_12_spi_pin &P1_10_spi_pin &P1_08_spi_sclk_pin &P1_06_spi_cs_pin>; > + pinctrl-8 = <&P1_12_gpio_pin &P1_10_gpio_pin &P1_08_gpio_pin &P1_06_gpio_pin>; > + pwms = <&ehrpwm1 0 500000 0>; > + i2c-controller = <&i2c1>; > + uart-controller = <&uart1>; > + spi-controller = <&spi1>; > + spi-cs = <0 1>; > + spi-cs-names = "default", "rst"; > + mikrobus-gpios = <&gpio1 18 GPIO_ACTIVE_HIGH>, <&gpio0 23 GPIO_ACTIVE_HIGH>, > + <&gpio0 30 GPIO_ACTIVE_HIGH>, <&gpio0 31 GPIO_ACTIVE_HIGH>, > + <&gpio0 15 GPIO_ACTIVE_HIGH>, <&gpio0 14 GPIO_ACTIVE_HIGH>, > + <&gpio0 4 GPIO_ACTIVE_HIGH>, <&gpio0 3 GPIO_ACTIVE_HIGH>, > + <&gpio0 2 GPIO_ACTIVE_HIGH>, <&gpio0 5 GPIO_ACTIVE_HIGH>, > + <&gpio2 25 GPIO_ACTIVE_HIGH>, <&gpio2 3 GPIO_ACTIVE_HIGH>; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 807feae089c4..8e4115e93aeb 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15108,6 +15108,12 @@ M: Oliver Neukum <oliver@neukum.org> > S: Maintained > F: drivers/usb/image/microtek.* > > +MIKROBUS > +M: Ayush Singh <ayush@beagleboard.org> > +M: Vaishnav M A <vaishnav@beagleboard.org> > +S: Maintained > +F: Documentation/devicetree/bindings/connector/mikrobus-connector.yaml > + > MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT > M: Luka Kovacic <luka.kovacic@sartura.hr> > M: Luka Perkov <luka.perkov@sartura.hr> > > -- > 2.45.2 >
On Thu, Jun 27, 2024 at 10:59:46PM +0530, Ayush Singh wrote: > On 6/27/24 22:42, Michael Walle wrote: > > > Hi, > > > > Could you give us a DT snippet of how this should look like with a > > board? > > > > On Thu Jun 27, 2024 at 6:26 PM CEST, Ayush Singh wrote: > > > + board: > > > + description: board attached to mikrobus connector > > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > Shouldn't this be a subnode of the connector? > > > > i.e. > > > > connector { > > compatible = "mikrobus-connector"; > > > > // phandles to the parent controllers These are per bus, so put them in the child bus nodes: > > > > spi { spi-bus = <&spiN>; spi-cs = ... The base DT would have the spi node and these properties. The overlay would still apply to the connector node, but also have the 'spi' node along with the devices. Note that whatever is done here, I expect to work on any connector with SPI, I2C, etc. So structuring the bindings for that would be nice. There is also this effort which needs the same bindings[1]. > > temp-sensor@0 { > > compatible = "maxim,max31855k"; > > reg = <0>; > > }; > > }; > > > > i2c { > > .. > > }; > > }; > > > > I don't think you can introduce a new > > compatible = "maxim,max31855k", "mikrobus,spi"; > > if there is already a binding for "maxim,max31855k". But I might be > > wrong. Why is this compatible needed at all? > > So I did consider the design you just proposed, but I was not able to solve > a few issues. > > 1. How to deal with say 2 mikrobus connectors in a single system? I don't understand why that's a problem? It's no different than the same overlay working on multiple vendor's boards which I imagine you want too. The connector node in the base DT has to remap everything from base DT into a mikrobus defined number/name space. For example, host GPIO N is mapped to mikrobus connector GPIO 0 and so on. There is one issue in knowing what the target node is. Standardizing the target path or connector node label only works for 1 connector per system. You can have an empty target path in the overlay and something else can decide the target. This is what's being done for overlays with the dynamic PCI nodes. For example, maybe an eeprom tells the driver what overlay to apply. Rob [1] https://lore.kernel.org/all/20240510-hotplug-drm-bridge-v2-0-ec32f2c66d56@bootlin.com/
On 6/28/24 19:22, Andrew Lunn wrote: >> 3. Allowing creation of sysfs entries `new_device` and `delete_device` >> similar to what already exists for I2C, etc. > On the I2C bus, these operate at the device level, you instantiate a > new I2C device. I assume here you are actually talking about board > level operations? So they would be 'new_board', and 'delete_board' > files in sysfs? > >> 4. Allow using 1-wire-eeprom in a fashion that allows automatic board >> discovery. >> >> >> Let me now introduce the 2 architectures we will be discussing: >> >> 1. mikrobus-connector has phandle to mikrobus-board: >> >> ``` >> >> \ { >> >> connector1 { >> >> board = <&board1>; >> >> }; >> >> >> mikrobus_boards { >> >> board1 { >> >> ... >> >> }; >> >> }; >> >> }; >> >> ``` >> >> >> 2. mikrobus board is a child node of mikrobus-connector: >> >> ``` >> >> \ { >> >> connector1 { >> >> ... >> >> spi { > So there would actually be multiple child nodes, one per bus, and then > maybe a simple-bus for nodes which do not correspond to a bus, > e.g. gpio-key, gpio-leds, etc., > >> board1 { >> >> ... >> >> }; >> >> }; >> >> }; >> >> }; >> >> ``` >> >> >> I will now go over how each of these goals might look like in both of the >> architecture. >> >> 1. Keeping the device tree properties upstream in a system independent way: >> >> a. mikrobus-connector has phandle to mikrobus-board >> >> It is possible to create an overlay as follows which will work with any >> system that defines the `mikrobus_boards` node. This node is completely >> independent of mikroBUS connector and thus does not need to be rewritten (or >> generated) for each board. There are no problems for system with more than 1 >> mikrobus connector. >> >> ``` >> >> &mikrobus_boards { >> >> board2 { >> >> ... >> >> }; >> >> >> board3 { >> >> ... >> >> }; >> >> }; > So by default, you have an empty mikrobus_boards node? You then use DT > overlay to load the needed board into this node, and then update the > phandle in the connection node to point to the newly loaded node? > >> b. mikrobus board is a child node of mikrobus-connector: >> >> Not sure how to do something similar here. The overlay needs to be rewritten >> (or generated) for each board. > It would be good to explain why... > >> Systems with multiple mikrobus connectors >> will need multiple overlays adding the boards as child node of each >> connector (with status = "disabled"). > Why? Just load the one overlay actually required. > >> &connector1 { >> >> spi = { >> >> board 2 { >> >> ... >> >> }; >> >> board 3 { >> >> ... >> >> }; >> >> }; >> >> }; > I don't actually understand this description. I was expecting more > like: > > connector1: { > > spi = { > /* Optional TI TSC2046 touchscreen controller */ > opt_touch: touchscreen@0 { > compatible = "ti,tsc2046"; > spi-max-frequency = <2500000>; > reg = <0>; > pinctrl-0 = <&pmx_gpio_13>; > pinctrl-names = "default"; > interrupts-extended = <&gpio0 13 IRQ_TYPE_EDGE_FALLING>; > }; > }; > > i2c = { > opt_audio: audio@1a { > compatible = "ti,tlv320aic23"; > reg = <0x1a>; > }; > > the_rest = { > gpio_keys { > compatible = "gpio-keys"; > #address-cells = <1>; > #size-cells = <0>; > pinctrl-0 = <&pmx_reset_button &pmx_USB_copy_button>; > pinctrl-names = "default"; > > copy { > label = "USB Copy"; > linux,code = <KEY_COPY>; > gpios = <&gpio0 15 GPIO_ACTIVE_LOW>; > }; > reset { > label = "Reset"; > linux,code = <KEY_RESTART>; > gpios = <&gpio0 16 GPIO_ACTIVE_LOW>; > }; > }; > > This is completely made up. You probably should use an example of a > real complex board using multiple busses. > > So for each actual bus on Mikrobus, you have a bus node, and then a > node for everything which is not bus orientated, like gpio-keys. > > So the overlay would simply populate these child nodes. I think I had a fundamental misunderstanding of what dt overlays can do. My understanding was that to say add thermo click in the above style with child nodes, the overlay needs to look as follows &connector1 { spi { thermo_click: { compatible = "maxim,max31855k", "mikrobus-spi"; spi-max-frequency = <1000000>; pinctrl-apply = "spi_default"; }; }; }; However, after going through the drm PR pointed by Rob and your description, it seems it is possible to create an overlay as follows: &spi { thermo_click: { compatible = "maxim,max31855k"; spi-max-frequency = <1000000>; pinctrl-apply = "spi_default"; }; }; and apply it to the particular connector node (say connector1), at least from the driver. If that is the case, then yes, all of my reasons for not wanting to use child nodes become irrelevant. DRM PR: https://lore.kernel.org/all/20240510-hotplug-drm-bridge-v2-0-ec32f2c66d56@bootlin.com/ Ayush Singh
On 6/28/24 21:58, Rob Herring wrote: > On Thu, Jun 27, 2024 at 09:56:11PM +0530, Ayush Singh wrote: >> Add DT bindings for mikroBUS interface. MikroBUS is an open standard >> developed by MikroElektronika for connecting add-on boards to >> microcontrollers or microprocessors. >> >> mikroBUS is a connector and does not have a controller. Instead the >> software is responsible for identification of board and setting up uart, >> spi, i2c, pwm and other buses. Additionally, some new mikroBUS boards >> contain 1-wire EEPROM that contains a manifest to describe the addon >> board to provide plug and play capabilities. >> >> A mikroBUS addon board is free to leave some of the pins unused which >> are marked as NC or Not Connected. >> >> Some of the pins might need to be configured as GPIOs deviating from their >> reserved purposes Eg: SHT15 Click where the SCL and SDA Pins need to be >> configured as GPIOs for the driver (drivers/hwmon/sht15.c) to work. >> >> For some add-on boards the driver may not take care of some additional >> signals like reset/wake-up/other. Eg: ENC28J60 click where the reset line >> (RST pin on the mikrobus port) needs to be pulled high. >> >> Some SPI addon boards use other pins like RST, AN etc as chipselect (eg. >> SPI Extend Click). Thus, `spi-cs` and `spi-cs-names` property is added >> to allow mikroBUS addon board to specify chipselect by name. >> >> Here's the list of pins in mikroBUS connector: >> AN - Analog >> RST - Reset >> CS - SPI Chip Select >> SCK - SPI Clock >> MISO - SPI Master Input Slave Output >> MOSI - SPI Master Output Slave Input >> +3.3V - VCC-3.3V power >> GND - Reference Ground >> PWM - PWM output >> INT - Hardware Interrupt >> RX - UART Receive >> TX - UART Transmit >> SCL - I2C Clock >> SDA - I2C Data >> +5V - VCC-5V power >> GND - Reference Ground >> >> Link: https://www.mikroe.com/mikrobus >> Link: >> https://download.mikroe.com/documents/standards/mikrobus/mikrobus-standard-specification-v200.pdf >> mikroBUS specification >> Link: https://www.mikroe.com/sht1x-click SHT15 Click >> Link: https://www.mikroe.com/eth-click ENC28J60 Click >> Link: https://www.mikroe.com/spi-extend-click SPI Extend Click >> >> Co-developed-by: Vaishnav M A <vaishnav@beagleboard.org> >> Signed-off-by: Vaishnav M A <vaishnav@beagleboard.org> >> Signed-off-by: Ayush Singh <ayush@beagleboard.org> >> --- >> .../bindings/connector/mikrobus-connector.yaml | 107 +++++++++++++++++++++ >> MAINTAINERS | 6 ++ >> 2 files changed, 113 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml >> new file mode 100644 >> index 000000000000..033479f8604f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml >> @@ -0,0 +1,107 @@ >> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/connector/mikrobus-connector.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: mikroBUS add-on board connector >> + >> +maintainers: >> + - Ayush Singh <ayush@beagleboard.org> >> + >> +properties: >> + compatible: >> + const: mikrobus-connector >> + >> + pinctrl-0: true >> + pinctrl-1: true >> + pinctrl-2: true >> + pinctrl-3: true >> + pinctrl-4: true >> + pinctrl-5: true >> + pinctrl-6: true >> + pinctrl-7: true >> + pinctrl-8: true >> + >> + pinctrl-names: >> + minItems: 1 >> + maxItems: 9 >> + items: >> + enum: [default, pwm_default, pwm_gpio, uart_default, uart_gpio, i2c_default, i2c_gpio, spi_default, >> + spi_gpio] > Generally, each pinctrl-N is mutually exclusive. It looks like here you > want multiple states active at one time. Does this work? I see. In mikrobus case, these pinctrl are not mutually exclusive. The ones that are mutually exclusive are as follows: - pwm_default and pwm_gpio - uart_default and uart_gpio - i2c_default and i2c_gpio - spi_default and spi_gpio It still does lead to 16 combinations so not sure if it is the best approach. >> + >> + spi-controller: >> + description: spi-controller of mikroBUS SPI pins along with cs pins. >> + $ref: /schemas/types.yaml#/definitions/phandle >> + >> + spi-cs: >> + description: spi chip-select corresponding to the chip-selects on the mikrobus socket. > Wrap lines at 80 char. > > The array index is the chip-select number on the connector and the > value is the host SPI controller CS numbers? Or the other way around? > This needs a better description. > >> + $ref: /schemas/types.yaml#/definitions/uint32-array > Maximum number of entries? > >> + >> + spi-cs-names: >> + minItems: 1 >> + maxItems: 12 >> + items: >> + enum: [default, pwm, int, rx, tx, scl, sda, an, rst, sck, cipo, copi] >> + >> + i2c-controller: >> + description: i2c controller attached to the mikrobus socket. >> + $ref: /schemas/types.yaml#/definitions/phandle > i2c-bus is the somewhat standard property for this. > > Really, I'd expect connectors to look something like this: > > connector { > i2c-0 { > i2c-bus = <&i2c3>; > #address-cells = <1>; > #size-cells = <0>; > device@12 { > compatible = "some-i2c-device"; > }; > }; > }; > > That form allows for multiple buses (of the same type or different) on > the connector. > >> + >> + uart-controller: >> + description: uart controller attached to the mikrobus socket >> + $ref: /schemas/types.yaml#/definitions/phandle >> + >> + pwms: >> + description: the pwm-controller corresponding to the mikroBUS PWM pin. >> + maxItems: 1 >> + >> + mikrobus-gpios: >> + minItems: 1 >> + maxItems: 12 >> + >> + mikrobus-gpio-names: > The GPIO binding does not work this way as the name is in the property. > Either drop if you want to keep the array or you have to do something > like this: > > pwm-gpios > int-gpios > rx-gpios > > Really, the intention was for connectors to use gpio-map property to > renumber GPIOs relative to the connector. Can you point me to what you mean by gpio-map property? >> + minItems: 1 >> + maxItems: 12 >> + items: >> + enum: [pwm, int, rx, tx, scl, sda, an, rst, cs, sck, cipo, copi] >> + >> + board: >> + description: board attached to mikrobus connector >> + $ref: /schemas/types.yaml#/definitions/phandle-array > What is this for? > >> + >> +required: >> + - compatible >> + - pinctrl-0 >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + mikrobus { >> + compatible = "mikrobus-connector"; >> + pinctrl-names = "default", "pwm_default", "pwm_gpio","uart_default", "uart_gpio", "i2c_default", >> + "i2c_gpio", "spi_default", "spi_gpio"; >> + pinctrl-0 = <&P2_03_gpio_input_pin &P1_04_gpio_pin &P1_02_gpio_pin>; >> + pinctrl-1 = <&P2_01_pwm_pin>; >> + pinctrl-2 = <&P2_01_gpio_pin>; >> + pinctrl-3 = <&P2_05_uart_pin &P2_07_uart_pin>; >> + pinctrl-4 = <&P2_05_gpio_pin &P2_07_gpio_pin>; >> + pinctrl-5 = <&P2_09_i2c_pin &P2_11_i2c_pin>; >> + pinctrl-6 = <&P2_09_gpio_pin &P2_11_gpio_pin>; >> + pinctrl-7 = <&P1_12_spi_pin &P1_10_spi_pin &P1_08_spi_sclk_pin &P1_06_spi_cs_pin>; >> + pinctrl-8 = <&P1_12_gpio_pin &P1_10_gpio_pin &P1_08_gpio_pin &P1_06_gpio_pin>; >> + pwms = <&ehrpwm1 0 500000 0>; >> + i2c-controller = <&i2c1>; >> + uart-controller = <&uart1>; >> + spi-controller = <&spi1>; >> + spi-cs = <0 1>; >> + spi-cs-names = "default", "rst"; >> + mikrobus-gpios = <&gpio1 18 GPIO_ACTIVE_HIGH>, <&gpio0 23 GPIO_ACTIVE_HIGH>, >> + <&gpio0 30 GPIO_ACTIVE_HIGH>, <&gpio0 31 GPIO_ACTIVE_HIGH>, >> + <&gpio0 15 GPIO_ACTIVE_HIGH>, <&gpio0 14 GPIO_ACTIVE_HIGH>, >> + <&gpio0 4 GPIO_ACTIVE_HIGH>, <&gpio0 3 GPIO_ACTIVE_HIGH>, >> + <&gpio0 2 GPIO_ACTIVE_HIGH>, <&gpio0 5 GPIO_ACTIVE_HIGH>, >> + <&gpio2 25 GPIO_ACTIVE_HIGH>, <&gpio2 3 GPIO_ACTIVE_HIGH>; >> + }; >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 807feae089c4..8e4115e93aeb 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -15108,6 +15108,12 @@ M: Oliver Neukum <oliver@neukum.org> >> S: Maintained >> F: drivers/usb/image/microtek.* >> >> +MIKROBUS >> +M: Ayush Singh <ayush@beagleboard.org> >> +M: Vaishnav M A <vaishnav@beagleboard.org> >> +S: Maintained >> +F: Documentation/devicetree/bindings/connector/mikrobus-connector.yaml >> + >> MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT >> M: Luka Kovacic <luka.kovacic@sartura.hr> >> M: Luka Perkov <luka.perkov@sartura.hr> >> >> -- >> 2.45.2 >> I am switching to child-node based structure from the next patch since I was able to replicate applying board overlay on a generic connector with child node. Ayush Singh
On Tue, Jul 2, 2024 at 9:14 AM Ayush Singh <ayush@beagleboard.org> wrote: > > On 6/28/24 21:58, Rob Herring wrote: > > > On Thu, Jun 27, 2024 at 09:56:11PM +0530, Ayush Singh wrote: > >> Add DT bindings for mikroBUS interface. MikroBUS is an open standard > >> developed by MikroElektronika for connecting add-on boards to > >> microcontrollers or microprocessors. [...] > >> + mikrobus-gpios: > >> + minItems: 1 > >> + maxItems: 12 > >> + > >> + mikrobus-gpio-names: > > The GPIO binding does not work this way as the name is in the property. > > Either drop if you want to keep the array or you have to do something > > like this: > > > > pwm-gpios > > int-gpios > > rx-gpios > > > > Really, the intention was for connectors to use gpio-map property to > > renumber GPIOs relative to the connector. > > Can you point me to what you mean by gpio-map property? It is defined in the DT specification. Rob
On Fri, Jun 28, 2024 at 6:48 PM Conor Dooley <conor@kernel.org> wrote: > On Thu, Jun 27, 2024 at 09:56:13PM +0530, Ayush Singh wrote: > > Add bindings for MikroBUS boards using SPI interface. > > > > Almost all of the properties that are valid for SPI devices can be used > > except reg. Since the goal is to allow use of the same MikroBUS board > > across different connectors, config needs to be independent of the actual > > SPI controller in mikroBUS port(s), it is not possible to define the > > chipselect by number in advance. Thus, `spi-cs-apply` property is used to > > specify the chipselect(s) by name. > > > > Another important fact is that while there is a CS pin in the mikroBUS > > connector, some boards (eg SPI Extend Click) use additional pins as > > chipselect. Thus we need a way to specify the CS pin(s) in terms of > > mikcrobus-connector which can then handle bindings the actual CS pin(s). > > > > Link: https://www.mikroe.com/spi-extend-click SPI Extend Click > > > > Signed-off-by: Ayush Singh <ayush@beagleboard.org> Thanks for your patch! > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mikrobus/mikrobus-spi.yaml > > + > > +required: > > + - compatible > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + thermo-click { > > + compatible = "maxim,max31855k", "mikrobus,spi"; > > I am really not keen on what this implies, as I think Rob and I have > already mentioned, the connector should handle the "mapping" and the > regular SPI/I2C/whatever bindings for the SPI devices themselves > should be usable. Indeed. The (thermocouple component on the) click itself is not compatible with "mikrobus,spi", but with "maxim,max31855k". "mikrobus,spi" here means SPI is used as the transport layer. I guess you need "mikrobus,spi" because the click is pointed to by the "board" phandle in the connector, instead of being a subnode of the connector, like it should be? Gr{oetje,eeting}s, Geert
Hi Michael, On Thu, 27 Jun 2024, Michael Walle wrote: > On Thu Jun 27, 2024 at 7:07 PM CEST, Andrew Davis wrote: >>> + mikrobus_boards { >>> + thermo_click: thermo-click { >>> + compatible = "maxim,max31855k", "mikrobus-spi"; >> >> I might be missing something, but your solution cannot possibly be >> to list every click board that could be connected (all 1500+ of them) >> to every mikroBUS connector on every device's DT file.. >> >> Each click board should have a single DTSO overlay file to describe the >> click board, one per click board total. And then that overlay should >> apply cleanly to any device that has a mikroBUS interface. >> >> Which means you have not completely solved the fundamental problem of >> abstracting the mikroBUS connector in DT. Each of these click device child >> nodes has to be under the parent connector node. Which means a phandle >> to the parent node, which is not generically named. For instance >> if my board has 2 connectors, I would have mikrobus0 and mikrobus1, >> the click board's overlay would look like this: >> >> /dts-v1/; >> /plugin/; >> >> &mikrobus0 { Let's use just "&mikrobus" instead... >> status = "okay"; >> >> mikrobus_board { >> thermo-click { >> compatible = "maxim,max31855k", "mikrobus-spi"; >> spi-max-frequency = <1000000>; >> pinctrl-apply = "spi_default"; >> }; >> }; >> }; > > If there should only be one DT overlay per click board, how would > you apply that to to different connectors? You teach fdtoverlay[*] to translate anchors, e.g. fdtoverlay -i base.dtb -o final.dtb \ -t mikrobus=mikrobus0 click1.dtbo \ -t mikrobus=mikrobus1 click2.dtbo I believe the Raspberry Pi people already have something like that. The mikrobus node handles all other translations (e.g. mapping from Mikrobus pins to GPIO numbers), so you do not have to handle these explicitly when adding an overlay. [*] And anything else that handles overlays (U-Boot, out-of-tree OF_CONFIGFS, ...) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, Jul 5, 2024 at 10:01 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, 27 Jun 2024, Michael Walle wrote: > > On Thu Jun 27, 2024 at 7:07 PM CEST, Andrew Davis wrote: > >>> + mikrobus_boards { > >>> + thermo_click: thermo-click { > >>> + compatible = "maxim,max31855k", "mikrobus-spi"; > >> > >> I might be missing something, but your solution cannot possibly be > >> to list every click board that could be connected (all 1500+ of them) > >> to every mikroBUS connector on every device's DT file.. > >> > >> Each click board should have a single DTSO overlay file to describe the > >> click board, one per click board total. And then that overlay should > >> apply cleanly to any device that has a mikroBUS interface. > >> > >> Which means you have not completely solved the fundamental problem of > >> abstracting the mikroBUS connector in DT. Each of these click device child > >> nodes has to be under the parent connector node. Which means a phandle > >> to the parent node, which is not generically named. For instance > >> if my board has 2 connectors, I would have mikrobus0 and mikrobus1, > >> the click board's overlay would look like this: > >> > >> /dts-v1/; > >> /plugin/; > >> > >> &mikrobus0 { > > Let's use just "&mikrobus" instead... > > >> status = "okay"; > >> > >> mikrobus_board { > >> thermo-click { > >> compatible = "maxim,max31855k", "mikrobus-spi"; Max31855k is an SPI device, so its device node should be under an "spi" subnode (with proper #{address,size}-cells) of the mikrobus connector, and use a suitable unit-address and "reg" property, pointing to the right SPI chip select. > >> spi-max-frequency = <1000000>; This belongs to the "spi" subnode, not the Max31855k device node. > >> pinctrl-apply = "spi_default"; This belongs to the mikrobus connector node. > >> }; > >> }; > >> }; Gr{oetje,eeting}s, Geert
On 7/5/24 3:01 AM, Geert Uytterhoeven wrote: > Hi Michael, > > On Thu, 27 Jun 2024, Michael Walle wrote: >> On Thu Jun 27, 2024 at 7:07 PM CEST, Andrew Davis wrote: >>>> + mikrobus_boards { >>>> + thermo_click: thermo-click { >>>> + compatible = "maxim,max31855k", "mikrobus-spi"; >>> >>> I might be missing something, but your solution cannot possibly be >>> to list every click board that could be connected (all 1500+ of them) >>> to every mikroBUS connector on every device's DT file.. >>> >>> Each click board should have a single DTSO overlay file to describe the >>> click board, one per click board total. And then that overlay should >>> apply cleanly to any device that has a mikroBUS interface. >>> >>> Which means you have not completely solved the fundamental problem of >>> abstracting the mikroBUS connector in DT. Each of these click device child >>> nodes has to be under the parent connector node. Which means a phandle >>> to the parent node, which is not generically named. For instance >>> if my board has 2 connectors, I would have mikrobus0 and mikrobus1, >>> the click board's overlay would look like this: >>> >>> /dts-v1/; >>> /plugin/; >>> >>> &mikrobus0 { > > Let's use just "&mikrobus" instead... > >>> status = "okay"; >>> >>> mikrobus_board { >>> thermo-click { >>> compatible = "maxim,max31855k", "mikrobus-spi"; >>> spi-max-frequency = <1000000>; >>> pinctrl-apply = "spi_default"; >>> }; >>> }; >>> }; >> >> If there should only be one DT overlay per click board, how would >> you apply that to to different connectors? > > You teach fdtoverlay[*] to translate anchors, e.g. > > fdtoverlay -i base.dtb -o final.dtb \ > -t mikrobus=mikrobus0 click1.dtbo \ > -t mikrobus=mikrobus1 click2.dtbo > This basic idea is where I started also, the result is we end up needing a huge number of "anchor" points. And they would also be board specific. So we would want to store all these anchor points in a file, and what better file than another DT file. Putting all the translations in a DT file to allow the DT overlay to become generic is the core idea of this series[0] (looks like you already found it, linking for other following along). And as you note, the symbol table trick allows us to do this without teaching fdtoverlay anything new, so it should work as-is today for any project already supporting overlays. > I believe the Raspberry Pi people already have something like that. > > The mikrobus node handles all other translations (e.g. mapping from > Mikrobus pins to GPIO numbers), so you do not have to handle these > explicitly when adding an overlay. This part seems to still be an open item. For pinmux we can name the pinmux nodes such that their phandles are resolved on overlay application. For Pin number/name to GPIO number we have "gpio-names", and the names can also be generic. But for Interrupts and a couple others, we are still missing a good way to provide a generic mapping from pin name to number. Andrew [0] https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@ti.com/ > > [*] And anything else that handles overlays (U-Boot, out-of-tree > OF_CONFIGFS, ...) > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Andrew, On Fri, Jul 5, 2024 at 6:34 PM Andrew Davis <afd@ti.com> wrote: > On 7/5/24 3:01 AM, Geert Uytterhoeven wrote: > > On Thu, 27 Jun 2024, Michael Walle wrote: > >> On Thu Jun 27, 2024 at 7:07 PM CEST, Andrew Davis wrote: > >>>> + mikrobus_boards { > >>>> + thermo_click: thermo-click { > >>>> + compatible = "maxim,max31855k", "mikrobus-spi"; > >>> > >>> I might be missing something, but your solution cannot possibly be > >>> to list every click board that could be connected (all 1500+ of them) > >>> to every mikroBUS connector on every device's DT file.. > >>> > >>> Each click board should have a single DTSO overlay file to describe the > >>> click board, one per click board total. And then that overlay should > >>> apply cleanly to any device that has a mikroBUS interface. > >>> > >>> Which means you have not completely solved the fundamental problem of > >>> abstracting the mikroBUS connector in DT. Each of these click device child > >>> nodes has to be under the parent connector node. Which means a phandle > >>> to the parent node, which is not generically named. For instance > >>> if my board has 2 connectors, I would have mikrobus0 and mikrobus1, > >>> the click board's overlay would look like this: > >>> > >>> /dts-v1/; > >>> /plugin/; > >>> > >>> &mikrobus0 { > > > > Let's use just "&mikrobus" instead... > > > >>> status = "okay"; > >>> > >>> mikrobus_board { > >>> thermo-click { > >>> compatible = "maxim,max31855k", "mikrobus-spi"; > >>> spi-max-frequency = <1000000>; > >>> pinctrl-apply = "spi_default"; > >>> }; > >>> }; > >>> }; > >> > >> If there should only be one DT overlay per click board, how would > >> you apply that to to different connectors? > > > > You teach fdtoverlay[*] to translate anchors, e.g. > > > > fdtoverlay -i base.dtb -o final.dtb \ > > -t mikrobus=mikrobus0 click1.dtbo \ > > -t mikrobus=mikrobus1 click2.dtbo > > > > This basic idea is where I started also, the result is we end > up needing a huge number of "anchor" points. And they would > also be board specific. So we would want to store all these > anchor points in a file, and what better file than another > DT file. Why wouldn't a single anchor point suffice? For Mikrobus, the anchor point should have well-known subnodes like "spi", "i2c", ... which would take care of the translation. > Putting all the translations in a DT file to allow the DT overlay > to become generic is the core idea of this series[0] (looks like > you already found it, linking for other following along). > > And as you note, the symbol table trick allows us to do this > without teaching fdtoverlay anything new, so it should work > as-is today for any project already supporting overlays. Yes, and it sounds cool! > > I believe the Raspberry Pi people already have something like that. > > > > The mikrobus node handles all other translations (e.g. mapping from > > Mikrobus pins to GPIO numbers), so you do not have to handle these > > explicitly when adding an overlay. > > This part seems to still be an open item. For pinmux we can name > the pinmux nodes such that their phandles are resolved on overlay > application. For Pin number/name to GPIO number we have "gpio-names", > and the names can also be generic. But for Interrupts and a couple > others, we are still missing a good way to provide a generic mapping > from pin name to number. Isn't that the purpose of nexus nodes in the DT spec? Gr{oetje,eeting}s, Geert
>> But here you can have subnodes, no? These could then be just >> enumerated as usual. >> >> &mikrobus_board { >> mikrobus_gpio: gpio { >> gpio-controller; >> #gpio-cells = <1>; >> }; >> >> spi { >> cs-gpios = <&mikrobus_gpio 1>; >> >> spi@0 { >> compatible = "mydevice"; >> reg = <0>; >> }; >> }; >> }; >> Hi, I am now working on an approach for mikroBUS based on the apprach described here: [1] I am thinking of the gpio-controller approach you seem to have used here. So I wanted to inquire if there already exists a gpio-controller driver that can create a proxy controller that forwards stuff to the underlying actual controller. So something like the following: &mikrobus_gpio: gpio { gpio-controller; #gpio-cells = <2>; gpios = <&gpio1 0>, <&gpi2 1>; }; spi { cs-gpios = <&mikrobus_gpio 1 GPIO_ACTIVE_HIGH>; }; There does exist gpio0-virtio, but that seems to be for vm context. [1]: https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@ti.com/ Ayush Singh
On 9/4/24 23:19, Rob Herring wrote: > On Wed, Sep 4, 2024 at 12:09 PM Ayush Singh <ayush@beagleboard.org> wrote: >>>> gpio-map is what you are looking for. It's documented in the DT spec. >>>> It was created exactly for this purpose of remapping GPIO lines on a >>>> connector. >>>> >>>> Rob >> >> Hi. I found docs on nexus nodes [1] and tried using it for mikroBUS, but >> it does not seem to be working. Here is my connector: >> >> ``` >> >> mikrobus_gpio0: mikrobus-gpio0 { >> #gpio-cells = <2>; >> gpio-map = >> <0 0 &main_gpio1 11 0>, <1 0 &main_gpio1 9 0>, >> <2 0 &main_gpio1 24 0>, <3 0 &main_gpio1 25 0>, >> <4 0 &main_gpio1 22 0>, <5 0 &main_gpio1 23 0>, >> <6 0 &main_gpio1 7 0>, <7 0 &main_gpio1 8 0>, >> <8 0 &main_gpio1 14 0>, <9 0 &main_gpio1 13 0>, >> <10 0 &main_gpio1 12 0>, <11 0 &main_gpio1 10 0>; >> gpio-map-mask = <0xf 0x0>; >> gpio-map-pass-thru = <0x0 0x1>; >> }; >> >> ... >> >> &main_uart5 { >> status = "okay"; >> pinctrl-names = "default"; >> pinctrl-0 = <&mikrobus_uart_pins_default>; >> >> gnss { >> compatible = "u-blox,neo-8"; >> reset-gpios = <&mikrobus_gpio0 10 GPIO_ACTIVE_LOW>; >> }; >> }; >> >> ``` >> >> >> After some fdtdump, I can see that at least the dtc compiler does not >> seem to do the forwarding at dt compile time. Here is the dump: > dtc knows nothing about it. > >> ``` >> >> mikrobus-gpio0 { >> #gpio-cells = <0x00000002>; >> gpio-map = <0x00000000 0x00000000 0x00000025 0x0000000b >> 0x00000000 0x00000001 0x00000000 0x00000025 0x00000009 0x00000000 >> 0x00000002 0x00000000 0x00000025 0x00000018 0x00000000 0x00000003 >> 0x00000000 0x00000025 0x00000019 0x00000000 0x00000004 0x00000000 >> 0x00000025 0x00000016 0x00000000 0x00000005 0x00000000 0x00000025 >> 0x00000017 0x00000000 0x00000006 0x00000000 0x00000025 0x00000007 >> 0x00000000 0x00000007 0x00000000 0x00000025 0x00000008 0x00000000 >> 0x00000008 0x00000000 0x00000025 0x0000000e 0x00000000 0x00000009 >> 0x00000000 0x00000025 0x0000000d 0x00000000 0x0000000a 0x00000000 >> 0x00000025 0x0000000c 0x00000000 0x0000000b 0x00000000 0x00000025 >> 0x0000000a 0x00000000>; >> gpio-map-mask = <0x0000000f 0x00000000>; >> gpio-map-pass-thru = <0x00000000 0x00000001>; >> phandle = <0x0000000e>; >> }; > You might need "gpio-controller" here. Though if you do, I think > that's a mistake in the kernel. It should work like interrupt-map and > generally you have either interrupt-controller or interrupt-map, but > not both (though that is allowed now too). > >> ... >> >> serial@2850000 { >> compatible = "ti,am64-uart", "ti,am654-uart"; >> reg = <0x00000000 0x02850000 0x00000000 0x00000100>; >> interrupts = <0x00000000 0x000000b7 0x00000004>; >> power-domains = <0x00000003 0x0000009c 0x00000001>; >> clocks = <0x00000002 0x0000009c 0x00000000>; >> clock-names = "fclk"; >> status = "okay"; >> pinctrl-names = "default"; >> pinctrl-0 = <0x0000000d>; >> phandle = <0x00000081>; >> gnss { >> compatible = "u-blox,neo-8"; >> reset-gpios = <0x0000000e 0x0000000a 0x00000001>; >> }; >> }; >> >> ``` >> >> >> So I am a bit unsure. Is the dtc parser in the kernel supposed to do the > No such thing as "dtc parser in the kernel". > >> mapping, or is it supposed to be done by `dtc` at compile time? > No. > >> Maybe we >> do not have support for it in upstream kernel yet? > Yes, there is upstream support. Grep for of_parse_phandle_with_args_map. > > Rob So, after a bit of troubleshooting, it seems that a nexus node should not be present at root level (unless it also has an actual driver). If the nexus node is a root node without an actual driver, anything referring to the node is deferred. I am still a bit unsure if I should make the `mikrobus-connector` itself a nexus node or if I should have a subnode named `mikrobus_gpio`, but at least things seem to be working now. So, thanks for your help. Ayush Singh
MikroBUS is an open standard developed by MikroElektronika for connecting add-on boards to microcontrollers or microprocessors. It essentially allows you to easily expand the functionality of your main boards using these add-on boards. This patchset adds mikroBUS as a Linux bus type and provides a driver to parse and register the mikroBUS board using device tree infrastructure. The patchset is based on work originally done by Vaishnav. Link: https://www.mikroe.com/mikrobus Link: https://docs.beagleboard.org/latest/boards/beagleplay/ Link: https://lore.kernel.org/all/20240317193714.403132-1-ayushdevel1325@gmail.com/ Patch v4 Changes v5 - Complete rewrite to use device tree instead of mikroBUS manifest. - Only support for SPI. - Adds `mikrobus,spi` compatible property. Changes v4: - Better commit messages - Remove clickID, serdev, pwm, regulator, clocks etc. Just the basic mikroBUS driver. - Fix a lot of memory leaks, unused variables, etc. - Create accompanying PR in Greybus Spec repository - Switch to 80 columns formatting - Some other fixes pointed out in v3 Changes in v3: - Use phandle instead of busname for spi - Use spi board info for registering new device - Convert dt bindings to yaml - Add support for clickID - Code cleanup and style changes - Additions required to spi, serdev, w1 and regulator subsystems Changes in v2: - support for adding mikroBUS ports from DT overlays, - remove debug sysFS interface for adding mikrobus ports, - consider extended pin usage/deviations from mikrobus standard specifications - use greybus CPort protocol enum instead of new protocol enums - Fix cases of wrong indentation, ignoring return values, freeing allocated resources in case of errors and other style suggestions in v1 review. Signed-off-by: Ayush Singh <ayush@beagleboard.org> --- Ayush Singh (7): dt-bindings: connector: Add mikrobus-connector dt-bindings: mikrobus: Add mikrobus board base dt-bindings: mikrobus: Add mikrobus-spi binding spi: Make of_find_spi_controller_by_node() available spi: Make of_register_spi_device() available mikrobus: Add mikroBUS driver dts: ti: k3-am625-beagleplay: Add mikroBUS .../bindings/connector/mikrobus-connector.yaml | 107 ++++++ .../bindings/mikrobus/mikrobus-board.yaml | 20 ++ .../devicetree/bindings/mikrobus/mikrobus-spi.yaml | 37 +++ MAINTAINERS | 9 + arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 94 +++++- drivers/misc/Kconfig | 16 + drivers/misc/Makefile | 1 + drivers/misc/mikrobus.c | 361 +++++++++++++++++++++ drivers/spi/spi.c | 209 ++++++------ include/linux/spi/spi.h | 7 + 10 files changed, 750 insertions(+), 111 deletions(-) --- base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7 change-id: 20240627-mikrobus-scratch-spi-ad8c98dcec98 Best regards,