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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 19 Mar 2024 19:37:49 +0000
From: Conor Dooley <conor@...nel.org>
To: Ayush Singh <ayushdevel1325@...il.com>
Cc: open list <linux-kernel@...r.kernel.org>, jkridner@...gleboard.org,
	robertcnelson@...gleboard.org, lorforlinux@...gleboard.org,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>, Nishanth Menon <nm@...com>,
	Vignesh Raghavendra <vigneshr@...com>,
	Tero Kristo <kristo@...nel.org>,
	Derek Kiernan <derek.kiernan@....com>,
	Dragan Cvetic <dragan.cvetic@....com>,
	Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Vaishnav M A <vaishnav.a@...com>, Mark Brown <broonie@...nel.org>,
	Johan Hovold <johan@...nel.org>, Alex Elder <elder@...nel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>,
	"moderated list:ARM/TEXAS INSTRUMENTS K3 ARCHITECTURE" <linux-arm-kernel@...ts.infradead.org>,
	"open list:SPI SUBSYSTEM" <linux-spi@...r.kernel.org>,
	"moderated list:GREYBUS SUBSYSTEM" <greybus-dev@...ts.linaro.org>,
	Vaishnav M A <vaishnav@...gleboard.org>
Subject: Re: [PATCH v4 1/5] dt-bindings: misc: Add mikrobus-connector

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@...gleboard.org>
> Signed-off-by: Vaishnav M A <vaishnav@...gleboard.org>
> Signed-off-by: Ayush Singh <ayushdevel1325@...il.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.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)