Re: [PATCH v5 0/7] misc: Add mikroBUS driver

[Ayush Singh]

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@xxxxxxxxxxx/


Ayush Singh