Linux-SPI Archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Ayush Singh <ayushdevel1325@gmail.com>,
	linux-kernel@vger.kernel.org, jkridner@beagleboard.org,
	robertcnelson@beagleboard.org,
	Vaishnav M A <vaishnav@beagleboard.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>, Nishanth Menon <nm@ti.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tero Kristo <kristo@kernel.org>,
	Derek Kiernan <derek.kiernan@amd.com>,
	Dragan Cvetic <dragan.cvetic@amd.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Johan Hovold <johan@kernel.org>, Alex Elder <elder@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-spi@vger.kernel.org, linux-serial@vger.kernel.org,
	greybus-dev@lists.linaro.org
Subject: Re: [PATCH v3 1/8] dt-bindings: misc: Add mikrobus-connector
Date: Fri, 15 Mar 2024 21:00:49 +0000	[thread overview]
Message-ID: <ZfS3AcIyXd3Lli/w@shell.armlinux.org.uk> (raw)
In-Reply-To: <5723478b-1717-4f83-959f-14bfce309bcf@linaro.org>

On Fri, Mar 15, 2024 at 09:40:13PM +0100, Krzysztof Kozlowski wrote:
> On 15/03/2024 21:20, Russell King (Oracle) wrote:
> > On Fri, Mar 15, 2024 at 09:09:11PM +0100, Krzysztof Kozlowski wrote:
> >>> +properties:
> >>> +  compatible:
> >>> +    const: mikrobus-connector
> >>
> >> Hm, why do you create binding for the connector, not for some sort of
> >> controller? Please provide some rationale for this in commit msg.
> > 
> > I think you have a distorted view. I refer you to the Mikroe mikroBUS
> > specification - it's _just_ a connector which provides a fairly
> > standardised purpose for each pin and the electrical specifications.
> > For example of the pins: power, UART, SPIs, I2C, PWM, and analogue
> > pins.
> 
> I refer to the commit msg or description in the binding and there is
> nothing explained like this. Yeah, true, I could google every possible
> bus specification, but I also expect some sort of help here by the patch
> submitter.
> 
> The binding looks like binding for a connector, not for some sort of
> controller, then are you saying the control part it is purely in
> software? That's how DTS looks like, but then my question is are there
> some sort of controller which would also perform this?

There is, as far as I'm aware, no "controller" for a mikroBUS. As I
tried to explain, it's a bunch of pins with defined standard functions.
The idea seems to be they're connected to a SoC with a pinmux that can
reconfigure the function of the pin.

At least that's how the hardware implementations I've seen do it.

> > This isn't a choice at all. Here's the list of pins:
> > 
> > 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
> > 
> > Any data pin can be a GPIO if e.g. a relay board is plugged in, even
> > if some of the other pins are used for e.g. UART purposes. For example,
> > a GPS board that provides the GPS data over the UART pins, and the
> > PPS signal through a different pin.
> 
> And could you not have some certain features supported? Could have some
> pins just pull down / not connected?

A board plugged in doesn't have to use all the functions. I gave two
examples. Apart from the power pins,

The GPS board as I stated uses the two UART pins, and some GPS boards
route the PPS signal to another pin on the connector, but that's
entirely optional. Another pin might be used as a GPIO as an enable.

In the case of a relay board I've had, the SPI CS pin is used for one
relay, and the PWM pin for the other relay.

I also have a BME280 humidity/pressure sensor board, which just uses
the two I2C pins.

What is supported by a board is down to the board. Which pins it
connects to is down to the board. Which board is plugged in is up
to the user.

That is essentially the long and short of mikroBUS - I hope I've
given a good overview of it.

I am slightly confused by this series, because it seems the Linux
"mikroBUS" layer requires a one-wire EEPROM on the boards, but the
official mikroBUS specification doesn't state this. The author
really needs to clarify what they are implementing here. Are they
truly implementing mikroBUS as defined by Mikroe, or are they
implementing someone's custom extension to it that adds an
identification EEPROM - in which case I would argue that this
code as it stands is not suitable for mainline as long as it
purports to be implementing support for Mikroe's mikroBUS.

Hence, I think we should back off on reviewing this until we have
that clarification.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2024-03-15 21:01 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-15 18:48 [PATCH v3 0/8] misc: Add mikroBUS driver Ayush Singh
2024-03-15 18:48 ` [PATCH v3 1/8] dt-bindings: misc: Add mikrobus-connector Ayush Singh
2024-03-15 20:09   ` Krzysztof Kozlowski
2024-03-15 20:20     ` Russell King (Oracle)
2024-03-15 20:40       ` Krzysztof Kozlowski
2024-03-15 21:00         ` Russell King (Oracle) [this message]
2024-03-17 20:59   ` Rob Herring
2024-03-18 12:37     ` Ayush Singh
2024-03-15 18:49 ` [PATCH v3 2/8] w1: Add w1_find_master_device Ayush Singh
2024-03-15 19:40   ` Russell King (Oracle)
2024-03-15 20:14   ` Krzysztof Kozlowski
2024-03-15 18:49 ` [PATCH v3 3/8] spi: Make of_find_spi_controller_by_node() available Ayush Singh
2024-03-15 18:49 ` [PATCH v3 4/8] serdev: add of_ helper to get serdev controller Ayush Singh
2024-03-15 20:16   ` Krzysztof Kozlowski
2024-03-15 18:49 ` [PATCH v3 5/8] regulator: fixed-helper: export regulator_register_always_on Ayush Singh
2024-03-15 18:49 ` [PATCH v3 6/8] greybus: Add mikroBUS manifest types Ayush Singh
2024-04-11 12:03   ` Greg Kroah-Hartman
2024-03-15 18:49 ` [PATCH v3 7/8] mikrobus: Add mikrobus driver Ayush Singh
2024-03-15 19:03   ` Mark Brown
2024-03-15 19:32   ` Russell King (Oracle)
     [not found]     ` <46ba778a-5966-4b99-b820-f0d047a56227@gmail.com>
2024-03-15 21:19       ` Russell King (Oracle)
2024-03-15 22:10         ` Vaishnav Achath
2024-03-15 20:35   ` Krzysztof Kozlowski
2024-03-16 13:06     ` Ayush Singh
2024-03-19  5:32       ` Krzysztof Kozlowski
2024-03-19  6:59         ` Ayush Singh
2024-03-20 11:56           ` Krzysztof Kozlowski
2024-03-16  8:18   ` kernel test robot
2024-03-16  9:00   ` kernel test robot
2024-03-15 18:49 ` [PATCH v3 8/8] dts: ti: k3-am625-beagleplay: Add mikroBUS Ayush Singh
2024-03-15 20:20   ` Krzysztof Kozlowski
2024-03-15 21:20 ` [PATCH v3 0/8] misc: Add mikroBUS driver Vaishnav M A
2024-03-15 21:41   ` Ayush Singh
2024-03-15 22:24     ` Vaishnav Achath
2024-03-17 14:41     ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZfS3AcIyXd3Lli/w@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=arnd@arndb.de \
    --cc=ayushdevel1325@gmail.com \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=derek.kiernan@amd.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dragan.cvetic@amd.com \
    --cc=elder@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=greybus-dev@lists.linaro.org \
    --cc=jirislaby@kernel.org \
    --cc=jkridner@beagleboard.org \
    --cc=johan@kernel.org \
    --cc=kristo@kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=robertcnelson@beagleboard.org \
    --cc=robh@kernel.org \
    --cc=vaishnav@beagleboard.org \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).