Message-ID: <20240911170245.GA915638-robh@kernel.org>
Date: Wed, 11 Sep 2024 12:02:45 -0500
From: Rob Herring <robh@...nel.org>
To: Ayush Singh <ayush@...gleboard.org>
Cc: fabien.parent@...aro.org, d-gole@...com, lorforlinux@...gleboard.org,
	jkridner@...gleboard.org, robertcnelson@...gleboard.org,
	Andrew Davis <afd@...com>, Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Derek Kiernan <derek.kiernan@....com>,
	Dragan Cvetic <dragan.cvetic@....com>,
	Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Nishanth Menon <nm@...com>, Vignesh Raghavendra <vigneshr@...com>,
	Tero Kristo <kristo@...nel.org>, linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 0/8] Add generic overlay for MikroBUS addon boards

On Wed, Sep 11, 2024 at 07:57:17PM +0530, Ayush Singh wrote:
> Hello all,
> 
> This is an attempt to add MikroBUS addon support using the approach
> described by Grove connector patch series [0].
> 
> The patch series tests out 2 addon boards + pwm and GPIO on the MikroBUS
> connector. The boards used are GPS3 Click (for UART) [1] and Weather
> Click (for I2C) [2]. Additionally, I have tested relative GPIO numbering
> using devicetree nexus nodes [3].
> 
> The patch series does not attempt to do any dynamic discovery for 1-wire
> eeprom MikroBUS addon boards, nor does it provide any sysfs entry for
> board addition/removal. The connector driver might include them after
> the basic support is ironed out, but the existing patches for dynamic
> overlays work fine.
> 
> The patch series has been tested on BeaglePlay [4].
> 
> I will now go over individual parts of the patch series, but for a
> better picture of the approach, it would be best to checkout [0] first.
> 
> MikroBUS connector driver
> -------------------------
> 
> Just a stub platform driver for now. Will be extended in the future for
> dynamic board discovery using 1-wire eeprom present in some MikroBUS
> addon boards.
> 
> While it is a stub driver, disabling it will make the GPIO connector
> nexus node unreachable (any driver attempting to use it will enter
> differed probing). So it is required.
> 
> MikroBUS connector Devicetree
> ------------------------------
> 
> The connector devicetree defines the MikroBUS GPIO nexus node. This
> allows using pin numbering relative to MikroBUS connector pins in the
> addon boards overlay. Currently, the patch uses a clockwise numbering:
> 
>   0: PWM
>   1: INT
>   2: RX
>   3: TX
>   4: SCL
>   5: SDA
>   6: MOSI
>   7: MISO
>   8: SCK
>   9: CS
>   10: RST
>   11: AN
> 
> Additionally, in case PWM pin is not using channel 0, a nexus node for pwm
> should also be used and go in the connector devicetree.
> 
> MikroBUS connector symbols overlay
> ----------------------------------
> 
> To make an overlay generic we need a standard name scheme which we
> use across base boards. For the connector pins the pinmux phandle
> shall be:
> 
> <connector-name>_<pin-name>_mux_<pin-function>
> 
> For the parent provider phandle, we use a similar naming scheme:
> 
> <connector-name>_<pin-name>_<pin-function>
> 
> For GPIO controller, I am using `MIKROBUS_GPIO` name since with nexus
> nodes, we do not need to define individual pin gpio controllers.
> 
> The string symbols can be replaced with phandles once [5] is accepted.
> That will make connector stacking much simpler.
> 
> MikroBUS addon board overlay
> ----------------------------
> 
> The patch puts the addon board overlays in their own directory. I am
> using the following naming scheme for MikroBUS addon boards:
> 
> <vendor>-<product_id>.dtso
> 
> Mikroe [6] lists this for all boards in their website, but I am not sure
> if other vendors have a product_id.
> 
> This naming also makes future dynamic discovery easier, since click id
> spec [7] contains vendor_id and product_id in the header.
> 
> Usage
> -----
> 
> So what does this all look like? Let's take an example of a BeaglePlay
> with one MikroBUS connectors for which we have physically attached a
> Wather click module to the first connector. Doing ahead of time
> command-line DT overlay application:
> 
> ./fdtoverlay \
> 	-o output.dtb \
> 	-i k3-am625-beagleplay.dtb \
> 		k3-am625-beagleplay-mikrobus-connector0.dtbo mikroe-5761.dtbo
> 
> Open Items
> ----------
> 
> - SPI Support: 
>   Currently SPI dt requires `reg` property to specify the
>   chipselect to use. This, makes the SPI device overlay dependent on the
>   SPI controller. Thus for SPI support, we need a way to allow defining
>   SPI chipselect relative to MikroBUS pins, and not the actual device
>   controller.
> 
>   One possible solution is to introduce something like `named-reg` and
>   allow selecting the chipselect by string array. But this probably
>   requires more discussion so I have left out SPI support for now.
> 
>   NOTE: pins other than the CS MikroBUS pin can be used as chipselect.
>   See [8].
> 
> - Controller symbol duplication
>   The current symbol scheme has multiple symbols for the same underlying
>   controller (Eg: MIKROBUS_SCL_MUX_I2C_SCL and MIKROBUS_SDA_MUX_I2C_SDA)
>   point to the same i2c controller.
> 
>   I think both of them will always use the same i2c controller, but
>   maybe there are some exceptions? So I have left it as it is for this
>   patch series. The same thing also applies to UART and SPI.
> 
>   NOTE: with the use of nexus node for GPIO, the GPIO controller symbol
>   will be the same for all pins.
> 
> - Nexus node dt-bindings
>   I am not quite sure how to deal with the nexus node properties
>   (#gpio-cells, gpio-map, gpio-map-mask, gpio-map-pass-thru) since they
>   seem to conflict with upstream gpio schema (gpio-controller is a
>   dependency of #gpio-cells).

Please submit a fix to dtschema. Either GH PR or patch to 
devicetree-spec list.

Rob

Powered by blists - more mailing lists