Re: [PATCH v4 1/5] dt-bindings: misc: Add mikrobus-connector

Krzysztof Kozlowski
2024-03-19T06:03:24Z

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