Message ID | 20240317193714.403132-2-ayushdevel1325@gmail.com |
---|---|
State | New |
Headers | show |
Series | misc: Add mikroBUS driver | expand |
On 17/03/2024 20:37, 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. > ... > +title: mikroBUS add-on board socket > + > +maintainers: > + - Ayush Singh <ayushdevel1325@gmail.com> > + > +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: > + items: > + - const: default > + - const: pwm_default > + - const: pwm_gpio > + - const: uart_default > + - const: uart_gpio > + - const: i2c_default > + - const: i2c_gpio > + - const: spi_default > + - const: spi_gpio > + > + mikrobus-gpios: > + minItems: 11 > + maxItems: 12 I don't see any of the issues resolved which I raised at v3. I think Russell pointed that you do not have EEPROM and that some pins are optional. You do not allow that. Plus I don't see him being Cced but he had quite detailed look and comments at your patchset, so *you are supposed to Cc* him. I also do not see Rob's comments fully addressed. Do not send next versions before resolving previous discusssion. > + > + i2c-adapter: > + description: i2c adapter attached to the mikrobus socket. > + $ref: /schemas/types.yaml#/definitions/phandle > + > + spi-controller: > + description: spi bus number of the spi-master attached to the mikrobus socket. > + $ref: /schemas/types.yaml#/definitions/phandle > + > + uart: > + description: uart port attached to the mikrobus socket > + $ref: /schemas/types.yaml#/definitions/phandle > + > + pwms: > + description: the pwm-controller corresponding to the mikroBUS PWM pin. > + maxItems: 1 > + > + spi-cs: > + description: spi chip-select numbers corresponding to the chip-selects on the mikrobus socket. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + items: > + - description: chip select corresponding to CS pin > + - description: chip select corresponding to RST pin > + > +required: > + - compatible > + - pinctrl-0 > + - pinctrl-1 > + - pinctrl-2 > + - pinctrl-3 > + - pinctrl-4 > + - pinctrl-5 > + - pinctrl-6 > + - pinctrl-7 > + - pinctrl-8 > + - i2c-adapter > + - spi-controller > + - spi-cs > + - uart > + - pwms > + - mikrobus-gpios > + > +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", Please properly wrap your code according to Linux and DTS coding style documents. Best regards, Krzysztof
On Mon, Mar 18, 2024 at 01:07:09AM +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 / > registering uart, spi, i2c, pwm and other buses. Thus it needs a way to > get uart, spi, i2c, pwm and gpio controllers / adapters. > > A mikroBUS addon board is free to leave some of the pins unused which > are marked as NC or Not Connected. But your binding makes everything required. Why would I need to instantiate and connect an i2c controller to the mikroebus header if I don't intend connecting any mikroebus devices that need one? > 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. What drivers do is not relevant to the binding. Describe the hardware. > Here's the list of pins in mikroBUS connector: > Analog - AN > Reset - RST > SPI Chip Select - CS > SPI Clock - SCK > SPI Master Input Slave Output - MISO > SPI Master Output Slave Input - MOSI > VCC-3.3V power - +3.3V > Reference Ground - GND > 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 > > Additionally, some new mikroBUS boards contain 1-wire EEPROM that contains > a manifest to describe the addon board to provide plug and play > capabilities. My problem with this is that it purports to be some generic description of the connector but it appears to be very centred on a particular use case (this beagle product) with simple add-on boards or those that support the auto-detection eeprom. For other use cases I'm at a loss for why I'd not omit this node from my DT and treat the mikroebus connector like any other header on my boards, where I just apply an overlay that hooks up the device to the relevant spi/pwm/i2c/etc controllers. I'd almost go as far as to say that this binding is misleading, because it's worded like a complete description of the port but actually only seems to describe a (set of) use case(s). What am I missing? > 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/clickid ClickID > > Co-developed-by: Vaishnav M A <vaishnav@beagleboard.org> > Signed-off-by: Vaishnav M A <vaishnav@beagleboard.org> > Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com> > +required: > + - compatible > + - pinctrl-0 > + - pinctrl-1 > + - pinctrl-2 > + - pinctrl-3 > + - pinctrl-4 > + - pinctrl-5 > + - pinctrl-6 > + - pinctrl-7 > + - pinctrl-8 > + - i2c-adapter > + - spi-controller > + - spi-cs > + - uart > + - pwms > + - mikrobus-gpios Oh also, you're missing properties for the supplies.
On 3/18/24 01:07, 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 / > registering uart, spi, i2c, pwm and other buses. Thus it needs a way to > get uart, spi, i2c, pwm and gpio controllers / adapters. > > 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. > > Here's the list of pins in mikroBUS connector: > Analog - AN > Reset - RST > SPI Chip Select - CS > SPI Clock - SCK > SPI Master Input Slave Output - MISO > SPI Master Output Slave Input - MOSI > VCC-3.3V power - +3.3V > Reference Ground - GND > 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 > > Additionally, some new mikroBUS boards contain 1-wire EEPROM that contains > a manifest to describe the addon board to provide plug and play > capabilities. > > 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/clickid ClickID > > Co-developed-by: Vaishnav M A <vaishnav@beagleboard.org> > Signed-off-by: Vaishnav M A <vaishnav@beagleboard.org> > Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com> > --- > .../connector/mikrobus-connector.yaml | 113 ++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 119 insertions(+) > create mode 100644 Documentation/devicetree/bindings/connector/mikrobus-connector.yaml > > diff --git a/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml > new file mode 100644 > index 000000000000..ee3736add41c > --- /dev/null > +++ b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml > @@ -0,0 +1,113 @@ > +# 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 socket > + > +maintainers: > + - Ayush Singh <ayushdevel1325@gmail.com> > + > +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: > + items: > + - const: default > + - const: pwm_default > + - const: pwm_gpio > + - const: uart_default > + - const: uart_gpio > + - const: i2c_default > + - const: i2c_gpio > + - const: spi_default > + - const: spi_gpio > + > + mikrobus-gpios: > + minItems: 11 > + maxItems: 12 > + > + i2c-adapter: > + description: i2c adapter attached to the mikrobus socket. > + $ref: /schemas/types.yaml#/definitions/phandle > + > + spi-controller: > + description: spi bus number of the spi-master attached to the mikrobus socket. > + $ref: /schemas/types.yaml#/definitions/phandle > + > + uart: > + description: uart port attached to the mikrobus socket > + $ref: /schemas/types.yaml#/definitions/phandle > + > + pwms: > + description: the pwm-controller corresponding to the mikroBUS PWM pin. > + maxItems: 1 > + > + spi-cs: > + description: spi chip-select numbers corresponding to the chip-selects on the mikrobus socket. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + items: > + - description: chip select corresponding to CS pin > + - description: chip select corresponding to RST pin > + > +required: > + - compatible > + - pinctrl-0 > + - pinctrl-1 > + - pinctrl-2 > + - pinctrl-3 > + - pinctrl-4 > + - pinctrl-5 > + - pinctrl-6 > + - pinctrl-7 > + - pinctrl-8 > + - i2c-adapter > + - spi-controller > + - spi-cs > + - uart > + - pwms > + - mikrobus-gpios > + > +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-adapter = <&i2c1>; > + spi-controller = <&spi1>; > + spi-cs = <0 1>; > + uart = <&uart1>; > + 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 375d34363777..69418a058c6b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14767,6 +14767,12 @@ M: Oliver Neukum <oliver@neukum.org> > S: Maintained > F: drivers/usb/image/microtek.* > > +MIKROBUS > +M: Ayush Singh <ayushdevel1325@gmail.com> > +M: Vaishnav M A <vaishnav@beagleboard.org> > +S: Maintained > +F: Documentation/devicetree/bindings/misc/mikrobus-connector.yaml > + > MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT > M: Luka Kovacic <luka.kovacic@sartura.hr> > M: Luka Perkov <luka.perkov@sartura.hr> After going through all the discussions here, I have a few questions: 1. Is the old `*_register_device(controller, board_info)` style discouraged in favor of using device tree, at least for drivers using multiple fundamental buses (i2c, spi, etc)? Or is the problem just that these bindings do not leave open the possibility of using device tree overlays? Will it be fine if the dt bindings allow for dt overlays, but the driver still uses imperative registering of board? 2. Is the preferred way to handle virtual devices (like those created by greybus subsystem) now device tree? Is that one of the blockers for greybus i2c, spi etc to still be in staging? 3. How are virtual devices created in device tree? If I register an i2c adapter using `i2c_add_adapter`, is the device tree entry is dynamically created, which can then be used by a device tree overlay? Ayush Singh
> After going through all the discussions here, I have a few questions: > > 1. Is the old `*_register_device(controller, board_info)` style discouraged > in favor of using device tree, at least for drivers using multiple > fundamental buses (i2c, spi, etc)? Historically, they were used in board files, where you needed to write C code for every single board. That did not scale, which is why we swapped to DT. board_info is still useful, e.g. for platforms which don't have DT. I support a few amd64 boards where i need to use a platform driver to instantiate some I2C and MDIO devices. But in general DT is much easier to use. > 2. Is the preferred way to handle virtual devices (like those created by > greybus subsystem) now device tree? Is that one of the blockers for greybus > i2c, spi etc to still be in staging? I would not say they are virtual. They do exist. They are just not memory mapped like most devices, but in another address space, one which you access via RPCs. > > 3. How are virtual devices created in device tree? If I register an i2c > adapter using `i2c_add_adapter`, is the device tree entry is dynamically > created, which can then be used by a device tree overlay? As far as i'm aware, there are no examples today. You are doing something different, something new. Adding these dynamic devices to DT is just a suggestion from me, as a good way to solve your problem. You will need to look into the DT core and figure out how to do it. Andrew
diff --git a/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml new file mode 100644 index 000000000000..ee3736add41c --- /dev/null +++ b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml @@ -0,0 +1,113 @@ +# 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 socket + +maintainers: + - Ayush Singh <ayushdevel1325@gmail.com> + +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: + items: + - const: default + - const: pwm_default + - const: pwm_gpio + - const: uart_default + - const: uart_gpio + - const: i2c_default + - const: i2c_gpio + - const: spi_default + - const: spi_gpio + + mikrobus-gpios: + minItems: 11 + maxItems: 12 + + i2c-adapter: + description: i2c adapter attached to the mikrobus socket. + $ref: /schemas/types.yaml#/definitions/phandle + + spi-controller: + description: spi bus number of the spi-master attached to the mikrobus socket. + $ref: /schemas/types.yaml#/definitions/phandle + + uart: + description: uart port attached to the mikrobus socket + $ref: /schemas/types.yaml#/definitions/phandle + + pwms: + description: the pwm-controller corresponding to the mikroBUS PWM pin. + maxItems: 1 + + spi-cs: + description: spi chip-select numbers corresponding to the chip-selects on the mikrobus socket. + $ref: /schemas/types.yaml#/definitions/uint32-array + items: + - description: chip select corresponding to CS pin + - description: chip select corresponding to RST pin + +required: + - compatible + - pinctrl-0 + - pinctrl-1 + - pinctrl-2 + - pinctrl-3 + - pinctrl-4 + - pinctrl-5 + - pinctrl-6 + - pinctrl-7 + - pinctrl-8 + - i2c-adapter + - spi-controller + - spi-cs + - uart + - pwms + - mikrobus-gpios + +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-adapter = <&i2c1>; + spi-controller = <&spi1>; + spi-cs = <0 1>; + uart = <&uart1>; + 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 375d34363777..69418a058c6b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14767,6 +14767,12 @@ M: Oliver Neukum <oliver@neukum.org> S: Maintained F: drivers/usb/image/microtek.* +MIKROBUS +M: Ayush Singh <ayushdevel1325@gmail.com> +M: Vaishnav M A <vaishnav@beagleboard.org> +S: Maintained +F: Documentation/devicetree/bindings/misc/mikrobus-connector.yaml + MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT M: Luka Kovacic <luka.kovacic@sartura.hr> M: Luka Perkov <luka.perkov@sartura.hr>