mbox series

[v4,0/5] misc: Add mikroBUS driver

Message ID 20240317193714.403132-1-ayushdevel1325@gmail.com
Headers show
Series misc: Add mikroBUS driver | expand

Message

Ayush Singh March 17, 2024, 7:37 p.m. UTC
MikroBUS is an open standard  developed by MikroElektronika for connecting
add-on boards to microcontrollers or microprocessors. It essentially
allows you to easily expand the functionality of your main boards using
these add-on boards.

This patchset adds mikroBUS as a Linux bus type and provides a driver to
parse and register the mikroBUS board using mikroBUS manifest.

The patchset is based on work originally done by Vaishnav.

Link: https://www.mikroe.com/mikrobus
Link: https://docs.beagleboard.org/latest/boards/beagleplay/
Link: https://lore.kernel.org/lkml/20240315184908.500352-1-ayushdevel1325@gmail.com/ Patch v3

Changes v4:
- Better commit messages
- Remove clickID, serdev, pwm, regulator, clocks etc. Just the basic
  mikroBUS driver.
- Fix a lot of memory leaks, unused variables, etc.
- Create accompanying PR in Greybus Spec repository
- Switch to 80 columns formatting
- Some other fixes pointed out in v3

Changes in v3:
- Use phandle instead of busname for spi
- Use spi board info for registering new device
- Convert dt bindings to yaml
- Add support for clickID
- Code cleanup and style changes
- Additions required to spi, serdev, w1 and regulator subsystems

Changes in v2:
- support for adding mikroBUS ports from DT overlays,
- remove debug sysFS interface for adding mikrobus ports,
- consider extended pin usage/deviations from mikrobus standard
  specifications
- use greybus CPort protocol enum instead of new protocol enums
- Fix cases of wrong indentation, ignoring return values, freeing allocated
  resources in case of errors and other style suggestions in v1 review.

Ayush Singh (5):
  dt-bindings: misc: Add mikrobus-connector
  spi: Make of_find_spi_controller_by_node() available
  greybus: Add mikroBUS manifest types
  mikrobus: Add mikroBUS driver
  dts: ti: k3-am625-beagleplay: Add mikroBUS

 .../connector/mikrobus-connector.yaml         | 113 +++
 MAINTAINERS                                   |   7 +
 .../arm64/boot/dts/ti/k3-am625-beagleplay.dts |  80 +-
 drivers/misc/Kconfig                          |   1 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/mikrobus/Kconfig                 |  15 +
 drivers/misc/mikrobus/Makefile                |   5 +
 drivers/misc/mikrobus/mikrobus_core.c         | 696 ++++++++++++++++++
 drivers/misc/mikrobus/mikrobus_core.h         | 151 ++++
 drivers/misc/mikrobus/mikrobus_manifest.c     | 503 +++++++++++++
 drivers/misc/mikrobus/mikrobus_manifest.h     |  29 +
 drivers/spi/spi.c                             | 206 +++---
 include/linux/greybus/greybus_manifest.h      |  49 ++
 include/linux/spi/spi.h                       |   4 +
 14 files changed, 1750 insertions(+), 110 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
 create mode 100644 drivers/misc/mikrobus/Kconfig
 create mode 100644 drivers/misc/mikrobus/Makefile
 create mode 100644 drivers/misc/mikrobus/mikrobus_core.c
 create mode 100644 drivers/misc/mikrobus/mikrobus_core.h
 create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.c
 create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.h


base-commit: 61996c073c9b070922ad3a36c981ca6ddbea19a5

Comments

Ayush Singh March 18, 2024, 5:20 p.m. UTC | #1
On 3/18/24 17:52, Michael Walle wrote:

> On Sun Mar 17, 2024 at 8:37 PM CET, 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.
>>
>> 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.
>>
>> 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.
>>
>> 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@beagleboard.org>
>> Signed-off-by: Vaishnav M A <vaishnav@beagleboard.org>
>> Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
>> ---
>>   .../connector/mikrobus-connector.yaml         | 113 ++++++++++++++++++
> See also
> https://lore.kernel.org/r/YmFo+EntwxIsco%2Ft@robh.at.kernel.org/
>
> Looks like this proposal doesn't have the subnodes. How do you
> attach a kernel driver to it's spi port for example? Only through
> the manifest files?
>
> -michael


So I looked at the Patch, and it seems the approach of fundamentally 
different than this PR. So, let me try to explain what this patch set 
does for an add-on board using SPI.

The device tree defines the SPI controller associated with mikroBUS SPI 
pins. The driver on match queries and takes a reference to the SPI 
controller but does nothing with it. Once a mikroBUS add-on board is 
detected (by passing manifest using sysfs or reading from 1-wire 
EEPROM), the driver parses the manifest, and if it detects an SPI device 
in manifest, it registers SPI device along with setting properties such 
as `chip_select`, `max_speed_hz`, `mode`, etc., which are defined in the 
manifest. On board removal, it unregisters the SPI device and waits for 
a new mikroBUS board to be detected again.

It is also possible for SPI not to be used by a device, in which case, 
no SPI device is registered to the controller. It is also possible that 
the SPI pins will be used as normal GPIOs. Everything is identified from 
the manifest.


Ayush Singh
Krzysztof Kozlowski March 19, 2024, 5:58 a.m. UTC | #2
On 18/03/2024 18:20, Ayush Singh wrote:
> On 3/18/24 17:52, Michael Walle wrote:
> 
>> On Sun Mar 17, 2024 at 8:37 PM CET, 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.
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> 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@beagleboard.org>
>>> Signed-off-by: Vaishnav M A <vaishnav@beagleboard.org>
>>> Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
>>> ---
>>>   .../connector/mikrobus-connector.yaml         | 113 ++++++++++++++++++
>> See also
>> https://lore.kernel.org/r/YmFo+EntwxIsco%2Ft@robh.at.kernel.org/
>>
>> Looks like this proposal doesn't have the subnodes. How do you
>> attach a kernel driver to it's spi port for example? Only through
>> the manifest files?
>>
>> -michael
> 
> 
> So I looked at the Patch, and it seems the approach of fundamentally 
> different than this PR. So, let me try to explain what this patch set 
> does for an add-on board using SPI.
> 
> The device tree defines the SPI controller associated with mikroBUS SPI 
> pins. The driver on match queries and takes a reference to the SPI 
> controller but does nothing with it. Once a mikroBUS add-on board is 
> detected (by passing manifest using sysfs or reading from 1-wire 
> EEPROM), the driver parses the manifest, and if it detects an SPI device 

As I understood Mikrobus does not have EEPROM.

> in manifest, it registers SPI device along with setting properties such 
> as `chip_select`, `max_speed_hz`, `mode`, etc., which are defined in the 
> manifest. On board removal, it unregisters the SPI device and waits for 
> a new mikroBUS board to be detected again.

You explained drivers, not hardware for DT.

> 
> It is also possible for SPI not to be used by a device, in which case, 
> no SPI device is registered to the controller. It is also possible that 
> the SPI pins will be used as normal GPIOs. Everything is identified from 
> the manifest.


Best regards,
Krzysztof
Ayush Singh March 19, 2024, 7:36 a.m. UTC | #3
On 3/19/24 11:28, Krzysztof Kozlowski wrote:

> On 18/03/2024 18:20, Ayush Singh wrote:
>> On 3/18/24 17:52, Michael Walle wrote:
>>
>>> On Sun Mar 17, 2024 at 8:37 PM CET, 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.
>>>>
>>>> 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.
>>>>
>>>> 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.
>>>>
>>>> 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@beagleboard.org>
>>>> Signed-off-by: Vaishnav M A <vaishnav@beagleboard.org>
>>>> Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
>>>> ---
>>>>    .../connector/mikrobus-connector.yaml         | 113 ++++++++++++++++++
>>> See also
>>> https://lore.kernel.org/r/YmFo+EntwxIsco%2Ft@robh.at.kernel.org/
>>>
>>> Looks like this proposal doesn't have the subnodes. How do you
>>> attach a kernel driver to it's spi port for example? Only through
>>> the manifest files?
>>>
>>> -michael
>>
>> So I looked at the Patch, and it seems the approach of fundamentally
>> different than this PR. So, let me try to explain what this patch set
>> does for an add-on board using SPI.
>>
>> The device tree defines the SPI controller associated with mikroBUS SPI
>> pins. The driver on match queries and takes a reference to the SPI
>> controller but does nothing with it. Once a mikroBUS add-on board is
>> detected (by passing manifest using sysfs or reading from 1-wire
>> EEPROM), the driver parses the manifest, and if it detects an SPI device
> As I understood Mikrobus does not have EEPROM.

mikroBUS add-on boards do not need to have EEPROM, but they can have it. 
Simply put, EEPROM is not part of mikroBUS specification, but you will 
find a lot (especially newer) addon boards with support for EEPROM manifest.

Regardless, this patch actually does not contain any code for EEPROM 
support I have just mentioned it to give more context on why mikroBUS 
manifest is the focus of this patch instead of DT overlay or something 
else.

>> in manifest, it registers SPI device along with setting properties such
>> as `chip_select`, `max_speed_hz`, `mode`, etc., which are defined in the
>> manifest. On board removal, it unregisters the SPI device and waits for
>> a new mikroBUS board to be detected again.
> You explained drivers, not hardware for DT.


Yes, I was replying to the question posed by Michael. Since this happens 
in the driver and not in the devicetree, I needed to explain the working 
of the driver:


 > How do you attach a kernel driver to it's spi port for example?


For more hardware side, the bindings are for mikrobus connector rather 
than for any addon board. Thus, while an addon board might not use some 
of the pins, the connector still needs to have all the pins and 
associated controllers.

>> It is also possible for SPI not to be used by a device, in which case,
>> no SPI device is registered to the controller. It is also possible that
>> the SPI pins will be used as normal GPIOs. Everything is identified from
>> the manifest.
>
> Best regards,
> Krzysztof
>

Ayush Singh
Ayush Singh March 19, 2024, 1:03 p.m. UTC | #4
On 3/19/24 17:38, Michael Walle wrote:

> On Tue Mar 19, 2024 at 12:36 PM CET, Ayush Singh wrote:
>>>> Regardless, this patch actually does not contain any code for EEPROM
>>>> support I have just mentioned it to give more context on why mikroBUS
>>>> manifest is the focus of this patch instead of DT overlay or something
>>>> else.
>>> Right, and I think this is the crux here. Why can't you use DT
>>> overlays? The manifest files, seem to be yet another hardware
>>> description (method) and we already have DT. Can't we have some kind
>>> of userspace helper that could translate them to DT overlays? That
>>> way, you could also handle the EEPROM vs non-EEPROM case, or have
>>> some other kind of method to load a DT overlay.
>>>
>>> Admittedly, I've never worked with in-kernel overlays, but AFAIK
>>> they work with some subsystems.
>>>
>>> -michael
>>
>> So let me 1st go over 3 cases that the driver needs to support:
>>
>> 1. Non EEPROM boards:
>>
>> Using overlays should be pretty similar to current solution. If the
>> manifest is converted to overlay in userspace, then we do not even need
>> to do manifest parsing, setting up spi, i2c etc in the kernel driver.
>>
>>
>> 2. EEPROM boards
>>
>> How do you propose handling these. If you are proposing storing dt
>> overlay in EEPROM, then this raises some questions regarding support
>> outside of Linux.
>>
>> The other option would be generating overlay from manifest in the kernel
>> driver, which I'm not sure is significantly better than registering the
>> i2c, spi, etc. interfaces separately using standard kernel APIs.
> You did answer that yourself in (1): you could use a user space
> helper to translate it to a DT overlay, I don't think this has to be
> done in the kernel.

I do not understand what you mean. For EEPROM supported boards, user 
space is not involved. The driver can directly read the manifest from 
add-on board and setup everything, so it is plug and play.

The manual involvement of user space is only for non EEPROM boards since 
we do not have a way to identify the board without the user needing to 
provide the manifest.


> Also how do you know whether there is an EEPROM
> or not?

Set RST GPIO to low. clickID supported board will enter ID MODE, Then 
check if CS line has a w1 gpio bus.

>> 3. Over Greybus
>>
>> It is quite important to have mikroBUS over greybus for BeagleConnect.
>> This is one of the major reasons why greybus manifest was chosen for the
>> manifest format.
>>
>> Also, it is important to note that mikroBUS manifest is being used since
>> 2020 now and thus manifests for a lot of boards (both supporting clickID
>> and not supporting it exist). So I would prefer using it, unless of
>> course there are strong reasons not to.
> And also here, I'm not really familiar with greybus. Could you give
> a more complex example? My concern is that some driver might need
> additional properties from DT (or software nodes) to function
> properly. It might not only be a node with a compatible string but
> also more advanced bindings. How would that play together with this?
> My gut feeling is that you can handle any missing properties
> easier/better (eg. for existing modules) in user space. But maybe
> that is already solved in/with greybus?

Greybus is a communication protocol designed for modular electronic 
devices. It allows different parts of a device to be hot plugged (added 
or removed) while the device is still running. Greybus manifest is used 
to describe the capabilities of a module in the greybus network. The 
host then creates appropriate bidirectional unipro connections with the 
module based on the cports described in the manifest. I have added a 
link to lwn article that goes into more detail.

BeagleConnect simply allows using greybus over any bidirectional 
transport, instead of just Unipro.

I cannot comment much about how greybus handles missing properties. 
While greybus also works just in kernel space, greybus protocols are 
inherently higher level than kernel driver, so it might have an easier 
time with this.

I have also added a link to eLInux page which provides rational for the 
mikroBUS manifest. But the crux seems to be that dynamic overlays were 
not well-supported back then. Also, the use of mikroBUS using greybus 
subsystem was already used. Hence the mikroBUS driver.

Greybus is not a big blocker from my perspective, since it is always 
possible to introduce a new protocol for mikroBUS in Greybus spec. I 
think as long as both EEPROM and non EEPROM boards can be supported by 
mikroBUS driver and dt-bindings, are can be used outside of Linux (eg: 
ZephyrRTOS, nuttx, etc), it is fine.

> Here's a random one: the manifest [1] just lists the compatible
> string apparently, but the actual DT binding has also reset-gpios,
> some -supply and interrupt properties.
>
> -michael
>
> [1] https://github.com/MikroElektronika/click_id/blob/main/manifests/WEATHER-CLICK.mnfs


Yes, the concern is valid. Support for validating the manifest is 
nowhere near as good as devicetree overlays. But I think that would be a 
problem with the device rather than the responsibility of the kernel. It 
is up to the manufacturer to have valid manifests.


Link: https://lwn.net/Articles/715955/ Greybus

Link https://elinux.org/Mikrobus eLinux article


Ayush Singh
Michael Walle March 19, 2024, 2:21 p.m. UTC | #5
On Tue Mar 19, 2024 at 2:03 PM CET, Ayush Singh wrote:
> >>>> Regardless, this patch actually does not contain any code for EEPROM
> >>>> support I have just mentioned it to give more context on why mikroBUS
> >>>> manifest is the focus of this patch instead of DT overlay or something
> >>>> else.
> >>> Right, and I think this is the crux here. Why can't you use DT
> >>> overlays? The manifest files, seem to be yet another hardware
> >>> description (method) and we already have DT. Can't we have some kind
> >>> of userspace helper that could translate them to DT overlays? That
> >>> way, you could also handle the EEPROM vs non-EEPROM case, or have
> >>> some other kind of method to load a DT overlay.
> >>>
> >>> Admittedly, I've never worked with in-kernel overlays, but AFAIK
> >>> they work with some subsystems.
> >>>
> >>> -michael
> >>
> >> So let me 1st go over 3 cases that the driver needs to support:
> >>
> >> 1. Non EEPROM boards:
> >>
> >> Using overlays should be pretty similar to current solution. If the
> >> manifest is converted to overlay in userspace, then we do not even need
> >> to do manifest parsing, setting up spi, i2c etc in the kernel driver.
> >>
> >>
> >> 2. EEPROM boards
> >>
> >> How do you propose handling these. If you are proposing storing dt
> >> overlay in EEPROM, then this raises some questions regarding support
> >> outside of Linux.
> >>
> >> The other option would be generating overlay from manifest in the kernel
> >> driver, which I'm not sure is significantly better than registering the
> >> i2c, spi, etc. interfaces separately using standard kernel APIs.
> > You did answer that yourself in (1): you could use a user space
> > helper to translate it to a DT overlay, I don't think this has to be
> > done in the kernel.
>
> I do not understand what you mean. For EEPROM supported boards, user 
> space is not involved. The driver can directly read the manifest from 
> add-on board and setup everything, so it is plug and play.

A driver could call a user-space helper, which will read the EEPROM
content (or maybe the driver already passes the content to the
helper), translate it to a DT overlay, and load it. Wouldn't that
work?

I'm not saying that is the way to go, just evaluate some ideas.

> The manual involvement of user space is only for non EEPROM boards since 
> we do not have a way to identify the board without the user needing to 
> provide the manifest.

FWIW, I'm not talking about manual steps here. But more of
call_usermodehelper(). Or maybe udev can do it?

Btw, [1] mentions hot-plugging. Is that really hot-plugging while
the system is running? How would that work?

> > Also how do you know whether there is an EEPROM
> > or not?
>
> Set RST GPIO to low. clickID supported board will enter ID MODE, Then 
> check if CS line has a w1 gpio bus.

Ok.

> >> 3. Over Greybus
> >>
> >> It is quite important to have mikroBUS over greybus for BeagleConnect.
> >> This is one of the major reasons why greybus manifest was chosen for the
> >> manifest format.
> >>
> >> Also, it is important to note that mikroBUS manifest is being used since
> >> 2020 now and thus manifests for a lot of boards (both supporting clickID
> >> and not supporting it exist). So I would prefer using it, unless of
> >> course there are strong reasons not to.
> > And also here, I'm not really familiar with greybus. Could you give
> > a more complex example? My concern is that some driver might need
> > additional properties from DT (or software nodes) to function
> > properly. It might not only be a node with a compatible string but
> > also more advanced bindings. How would that play together with this?
> > My gut feeling is that you can handle any missing properties
> > easier/better (eg. for existing modules) in user space. But maybe
> > that is already solved in/with greybus?
>
> Greybus is a communication protocol designed for modular electronic 
> devices. It allows different parts of a device to be hot plugged (added 
> or removed) while the device is still running. Greybus manifest is used 
> to describe the capabilities of a module in the greybus network. The 
> host then creates appropriate bidirectional unipro connections with the 
> module based on the cports described in the manifest. I have added a 
> link to lwn article that goes into more detail.
>
> BeagleConnect simply allows using greybus over any bidirectional 
> transport, instead of just Unipro.
>
> I cannot comment much about how greybus handles missing properties. 
> While greybus also works just in kernel space, greybus protocols are 
> inherently higher level than kernel driver, so it might have an easier 
> time with this.
>
> I have also added a link to eLInux page which provides rational for the 
> mikroBUS manifest. But the crux seems to be that dynamic overlays were 
> not well-supported back then. Also, the use of mikroBUS using greybus 
> subsystem was already used. Hence the mikroBUS driver.

I see this as an opportunity to improve the in-kernel overlays :)

> Greybus is not a big blocker from my perspective, since it is always 
> possible to introduce a new protocol for mikroBUS in Greybus spec. I 
> think as long as both EEPROM and non EEPROM boards can be supported by 
> mikroBUS driver and dt-bindings, are can be used outside of Linux (eg: 
> ZephyrRTOS, nuttx, etc), it is fine.
>
> > Here's a random one: the manifest [1] just lists the compatible
> > string apparently, but the actual DT binding has also reset-gpios,
> > some -supply and interrupt properties.
> >
> > -michael
> >
> > [1] https://github.com/MikroElektronika/click_id/blob/main/manifests/WEATHER-CLICK.mnfs
>
>
> Yes, the concern is valid. Support for validating the manifest is 
> nowhere near as good as devicetree overlays. But I think that would be a 
> problem with the device rather than the responsibility of the kernel. It 
> is up to the manufacturer to have valid manifests.

But does the manifest have the capabilities to express all that
information? To me it looks like just some kind of pinmux, some
vendor strings and a (DT) compatible string.
[coming back to this after seeing [2]: there are more properties,
but it seem just be a list of property=value]

What I'd like to avoid is some kind of in-kernel mapping list from
manifest to actual driver instantiation.

I guess you'll get much of that with DT overlays already and if you
have some kind of automatic translation from manifest to DT overlay,
it will still be plug-and-play. You could fix up any missing
properties, etc. manually loading some manifests/dt overlays for
modules without EEPROMs.

Again, a more complex manifest file would really be appreciated
here. Not just a simple "there is exactly one trivial SPI device on
the bus".

FWIW, here is a more complex example [2] which uses the ssd1306
display driver. Dunno if that is a good example, as it seems to use
the fb_ssd1306 driver (at least that's what I'm deducing by reading
the driver-string-id) in staging and there is also ssd1307fb.c in
drivers/video/fbdev. But how are the additional information like
width and height translate to the properties of the driver (device
tree properties, swnode properties, platform_data*)?

On a side note, does the manifest files use the (linux) kernel
module name for the driver-string-id?

-michael

[1] https://github.com/MikroElektronika/click_id/blob/main/README.md
[2] https://github.com/MikroElektronika/click_id/blob/main/manifests/OLEDB-CLICK.mnfs

> Link: https://lwn.net/Articles/715955/ Greybus
> Link https://elinux.org/Mikrobus eLinux article
Ayush Singh March 19, 2024, 5:35 p.m. UTC | #6
On 3/19/24 22:49, Vaishnav Achath wrote:
>> A driver could call a user-space helper, which will read the EEPROM
>> content (or maybe the driver already passes the content to the
>> helper), translate it to a DT overlay, and load it. Wouldn't that
>> work?
>>
>> I'm not saying that is the way to go, just evaluate some ideas.
>>
>
> This would work in most cases when we want to instantiate devices on a 
> physical mikroBUS port on the host running Linux, but another use case 
> we need to support is to instantiate devices on a virtual/greybus 
> mikroBUS port created through greybus, this is the case when a remote 
> microcontroller board (Example BeagleConnect Freedom) has mikroBUS 
> ports and through the magic of greybus these virtual ports 
> (corresponding to the physical remote ports) appear on the Linux host 
> - now we cannot use a device tree overlay to instantiate a Weather 
> click (BME280) sensor on this port, that is why the choice of 
> extending greybus manifest was chosen, another alternative here is to 
> go and add device tree as a description mechanism for greybus, please 
> let know if that is the recommended way forward?
>
> The greybus manifest already is being used in the greybus susbystem 
> for describing an interface and there are already greybus controllers 
> (SPI/I2C .etc) being created according to the manifest contents, all 
> this driver does is to extend that format to be able to instantiate 
> devices on these buses. The primary goals for introducing the driver 
> for mikroBUS add-on boards are:
>
> 1) A way to isolate platform specific information from add-on board 
> specific information - so that each permutation of connecting the 
> add-on board on different ports on different board does not require a 
> new overlay.
> 2) A way to instantiate add-on boards on greybus created virtual 
> mikroBUS ports.
> 3) Both 1 and 2 should use the same add-on board description format.
>
> Standard device tree overlays did not help to achieve this and that is 
> why the standard interface discovery mechanism in greybus, the 
> manifest was extended even though it is not the most optimal way to 
> describe hardware.


Yes, after discussion with Vaishnav and trying to brainstorm some way to 
do the same thing with dt overlays, it seems that trying to use dt 
overlays will mean need to have completely separate implementation of 
mikroBUS for local ports and mikroBUS over greybus. Additionally, trying 
to put dt overlays in EEPROM would mean they will be incompatible with 
use in local ports and vice versa.

Also, I feel like jumping to userspace can fail for all sorts of reasons 
and undermine the plug and play support that clickID strives for.


>
>>> The manual involvement of user space is only for non EEPROM boards 
>>> since
>>> we do not have a way to identify the board without the user needing to
>>> provide the manifest.
>>
>> FWIW, I'm not talking about manual steps here. But more of
>> call_usermodehelper(). Or maybe udev can do it?
>>
>> Btw, [1] mentions hot-plugging. Is that really hot-plugging while
>> the system is running? How would that work?
>>
>
> This should be corrected, it is not recommended to hot-plug the board 
> as the connector standard does not ensure any power sequencing and can 
> cause damage.


Yes, as long as local port is concerned, hot plugging is not 
recommended. However, when using greybus, it is possible to hotplug the 
node that mikroBUS is connected to.


>
>>>> Also how do you know whether there is an EEPROM
>>>> or not?
>>>
>>> Set RST GPIO to low. clickID supported board will enter ID MODE, Then
>>> check if CS line has a w1 gpio bus.
>>
>> Ok.
>>
>>>>> 3. Over Greybus
>>>>>
>>>>> It is quite important to have mikroBUS over greybus for 
>>>>> BeagleConnect.
>>>>> This is one of the major reasons why greybus manifest was chosen 
>>>>> for the
>>>>> manifest format.
>>>>>
>>>>> Also, it is important to note that mikroBUS manifest is being used 
>>>>> since
>>>>> 2020 now and thus manifests for a lot of boards (both supporting 
>>>>> clickID
>>>>> and not supporting it exist). So I would prefer using it, unless of
>>>>> course there are strong reasons not to.
>>>> And also here, I'm not really familiar with greybus. Could you give
>>>> a more complex example? My concern is that some driver might need
>>>> additional properties from DT (or software nodes) to function
>>>> properly. It might not only be a node with a compatible string but
>>>> also more advanced bindings. How would that play together with this?
>>>> My gut feeling is that you can handle any missing properties
>>>> easier/better (eg. for existing modules) in user space. But maybe
>>>> that is already solved in/with greybus?
>>>
>>> Greybus is a communication protocol designed for modular electronic
>>> devices. It allows different parts of a device to be hot plugged (added
>>> or removed) while the device is still running. Greybus manifest is used
>>> to describe the capabilities of a module in the greybus network. The
>>> host then creates appropriate bidirectional unipro connections with the
>>> module based on the cports described in the manifest. I have added a
>>> link to lwn article that goes into more detail.
>>>
>>> BeagleConnect simply allows using greybus over any bidirectional
>>> transport, instead of just Unipro.
>>>
>>> I cannot comment much about how greybus handles missing properties.
>>> While greybus also works just in kernel space, greybus protocols are
>>> inherently higher level than kernel driver, so it might have an easier
>>> time with this.
>>>
>>> I have also added a link to eLInux page which provides rational for the
>>> mikroBUS manifest. But the crux seems to be that dynamic overlays were
>>> not well-supported back then. Also, the use of mikroBUS using greybus
>>> subsystem was already used. Hence the mikroBUS driver.
>>
>> I see this as an opportunity to improve the in-kernel overlays :)
>>
>>> Greybus is not a big blocker from my perspective, since it is always
>>> possible to introduce a new protocol for mikroBUS in Greybus spec. I
>>> think as long as both EEPROM and non EEPROM boards can be supported by
>>> mikroBUS driver and dt-bindings, are can be used outside of Linux (eg:
>>> ZephyrRTOS, nuttx, etc), it is fine.
>>>
>>>> Here's a random one: the manifest [1] just lists the compatible
>>>> string apparently, but the actual DT binding has also reset-gpios,
>>>> some -supply and interrupt properties.
>>>>
>>>> -michael
>>>>
>>>> [1] 
>>>> https://github.com/MikroElektronika/click_id/blob/main/manifests/WEATHER-CLICK.mnfs
>>>
>>>
>>> Yes, the concern is valid. Support for validating the manifest is
>>> nowhere near as good as devicetree overlays. But I think that would 
>>> be a
>>> problem with the device rather than the responsibility of the 
>>> kernel. It
>>> is up to the manufacturer to have valid manifests.
>>
>> But does the manifest have the capabilities to express all that
>> information? To me it looks like just some kind of pinmux, some
>> vendor strings and a (DT) compatible string.
>> [coming back to this after seeing [2]: there are more properties,
>> but it seem just be a list of property=value]
>>
>> What I'd like to avoid is some kind of in-kernel mapping list from
>> manifest to actual driver instantiation.
>
> The property descriptor is implemented to account the properties under 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/property.h#n22 
>
>
> There is no in-kernel mapping that needs to be updated per driver, but 
> a generic mapping and some specific mapping depending on the bus the 
> device is connected (I2C/SPI/.etc)
>
>>
>> I guess you'll get much of that with DT overlays already and if you
>> have some kind of automatic translation from manifest to DT overlay,
>> it will still be plug-and-play. You could fix up any missing
>> properties, etc. manually loading some manifests/dt overlays for
>> modules without EEPROMs.
>>
>> Again, a more complex manifest file would really be appreciated
>> here. Not just a simple "there is exactly one trivial SPI device on
>> the bus".
>>
>> FWIW, here is a more complex example [2] which uses the ssd1306
>> display driver. Dunno if that is a good example, as it seems to use
>> the fb_ssd1306 driver (at least that's what I'm deducing by reading
>> the driver-string-id) in staging and there is also ssd1307fb.c in
>> drivers/video/fbdev. But how are the additional information like
>> width and height translate to the properties of the driver (device
>> tree properties, swnode properties, platform_data*)?
>>
>
> The driver uses device_property_read_* helpers to fetch the 
> infromation and the mikroBUS driver populates the table of properties 
> fetching the information from manifest and combining with platform 
> information.
>
>> On a side note, does the manifest files use the (linux) kernel
>> module name for the driver-string-id?
>>
>
> The spi_device_id is used for the driver-string-id :
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/fbtft/fbtft.h#n361 
>
>
> Thanks and Regards,
> Vaishnav
>
>> -michael
>>
>> [1] https://github.com/MikroElektronika/click_id/blob/main/README.md
>> [2] 
>> https://github.com/MikroElektronika/click_id/blob/main/manifests/OLEDB-CLICK.mnfs
>>
>>> Link: https://lwn.net/Articles/715955/ Greybus
>>> Link https://elinux.org/Mikrobus eLinux article
>>

I apologize if I gave the wrong impression, but the ability to enable 
mikroBUS over Greybus is extremely important here. I left out mikroBUS 
over greybus support from this patchset to allow for easier upstreaming 
of a new driver. However, that does not mean it is a secondary goal of 
this patch.

The current mikroBUS manifest is compatible with Greybus manifest, and 
thus is easy to integrate into standard Greybus. Anything suggested here 
still needs to enable support of mikroBUS over Greybus.


Ayush Singh
Conor Dooley March 19, 2024, 6:19 p.m. UTC | #7
On Tue, Mar 19, 2024 at 11:05:37PM +0530, Vaishnav Achath wrote:
> Hi Andrew,
> 
> On 19/03/24 17:55, Andrew Lunn wrote:
> > > The device tree defines the SPI controller associated with mikroBUS SPI
> > > pins. The driver on match queries and takes a reference to the SPI
> > > controller but does nothing with it. Once a mikroBUS add-on board is
> > > detected (by passing manifest using sysfs or reading from 1-wire EEPROM),
> > > the driver parses the manifest, and if it detects an SPI device in manifest,
> > > it registers SPI device along with setting properties such as `chip_select`,
> > > `max_speed_hz`, `mode`, etc.,
> > 
> > How complex can the description of the hardware be in the manifest?
> > 
> > Could i describe an SPI to I2C converter? And then a few temperature
> > sensors, a fan controller, and a GPIO controller on that I2C bus? And
> > the GPIO controller is then used for LEDs and a push button? DT
> > overlays could describe that. Can the manifest?
> 
> No, it cannot describe such complex hardware, it can only describe simple
> devices (sensors/displays .etc) on a standard mikroBUS add-on board, we did
> a analysis on what mikroBUS add-on boards have driver support in Linux and
> then noticed that most devices does not need this kind of complex
> description to work:
> https://elinux.org/MikroEClicks_with_Linux_Support

What happens to the devices that fall outside of the "do not need a
complex description" category? Do you expect that those would be
described by a dt overlay?

> The greybus manifest already is being used in the greybus susbystem for
> describing an interface and there are already greybus controllers (SPI/I2C
> .etc) being created according to the manifest contents, all this driver does
> is to extend that format to be able to instantiate devices on these buses.
> The primary goals for introducing the driver for mikroBUS add-on boards are:
> 
> 1) A way to isolate platform specific information from add-on board specific
> information - so that each permutation of connecting the add-on board on
> different ports on different board does not require a new overlay.
> 2) A way to instantiate add-on boards on greybus created virtual mikroBUS
> ports.
> 3) Both 1 and 2 should use the same add-on board description format.
> 
> Standard device tree overlays did not help to achieve this and that is why
> the standard interface discovery mechanism in greybus, the manifest was
> extended even though it is not the most optimal way to describe hardware.
> 
> The greybus manifest extensions were made with the following things in mind
> and three new descriptor were introduced:
> 1) mikrobus descriptor - pinmux/port state
> 2) device descriptor - contains information which is a superset of struct
> i2c_board_info , struct spi_board_info .etc
> 3) property descriptor - to describe named properties of the types defined
> under https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/property.h#n22
> 
> With these we were able to test around 150 add-on boards with corresponding
> drivers in Linux :
> https://github.com/MikroElektronika/click_id/tree/main/manifests
> 
> The mechanism is not as robust a device tree and should not be compared, the

Why not? You're suggesting this as a method for describing devices and you
seem to have extended the manifest to support more complex properties, why
shouldn't someone question make that comparison?

> intent was not to create a new hardware description format, but extend the
> existing greybus manifest format to be able to instantiate devices on the
> greybus SPI/I2C/GPIO/ (mikroBUS)
Andrew Lunn March 19, 2024, 7:23 p.m. UTC | #8
On Tue, Mar 19, 2024 at 11:05:37PM +0530, Vaishnav Achath wrote:
> Hi Andrew,
> 
> On 19/03/24 17:55, Andrew Lunn wrote:
> > > The device tree defines the SPI controller associated with mikroBUS SPI
> > > pins. The driver on match queries and takes a reference to the SPI
> > > controller but does nothing with it. Once a mikroBUS add-on board is
> > > detected (by passing manifest using sysfs or reading from 1-wire EEPROM),
> > > the driver parses the manifest, and if it detects an SPI device in manifest,
> > > it registers SPI device along with setting properties such as `chip_select`,
> > > `max_speed_hz`, `mode`, etc.,
> > 
> > How complex can the description of the hardware be in the manifest?
> > 
> > Could i describe an SPI to I2C converter? And then a few temperature
> > sensors, a fan controller, and a GPIO controller on that I2C bus? And
> > the GPIO controller is then used for LEDs and a push button? DT
> > overlays could describe that. Can the manifest?
> 
> No, it cannot describe such complex hardware, it can only describe simple
> devices (sensors/displays .etc) on a standard mikroBUS add-on board, we did
> a analysis on what mikroBUS add-on boards have driver support in Linux and
> then noticed that most devices does not need this kind of complex
> description to work:
> https://elinux.org/MikroEClicks_with_Linux_Support

Is that because the current software support is too limited? Are there
manufactures who want to create more complex designed, but are limited
by what can be described in the manifest?

Do you have a list of boards without Linux support? Why do they not
have Linux support? Is there a "vendor crap" driver which makes them
work? Does it make them work by working around the manifest
limitations?

> The greybus manifest already is being used in the greybus susbystem for
> describing an interface and there are already greybus controllers (SPI/I2C
> .etc) being created according to the manifest contents, all this driver does
> is to extend that format to be able to instantiate devices on these buses.

I don't know anything about greybus, so let me ask a few background
questions. Are these SPI and I2C controller plain Linux SPI and I2C
controllers? They fit the usual device model, they appear in
/sys/class/bus etc? Are the GPIO controllers also just plain Linux
GPIO controllers? All the drivers have a bottom interface which uses
greybus to perform some sort of RPC, but the top interface is standard
Linux. So in fact they are not so different to I2C over USB, SPI over
USB, GPIO over USB?

     Andrew
Ayush Singh March 20, 2024, 4:39 p.m. UTC | #9
On 3/20/24 01:02, Andrew Lunn wrote:

>> Yes, after discussion with Vaishnav and trying to brainstorm some way to do
>> the same thing with dt overlays, it seems that trying to use dt overlays
>> will mean need to have completely separate implementation of mikroBUS for
>> local ports and mikroBUS over greybus.
> Could you explain why please?
>
> Are greybus I2C bus masters different from physical I2C bus masters?
> Are greybus SPI bus masters different from physical SPI bus masters?

Well, they are virtual, so they are not declared in the device tree. I 
have linked the greybus i2c implementation. It basically allocates an 
i2c_adpater and then adds it using `i2c_add_adapter` method. This 
adapter can then be passed to say mikroBUS driver where it can be used 
as a normal i2c_adapter, and we can register the device to it.

>> Additionally, trying to put dt overlays in EEPROM would mean they
>> will be incompatible with use in local ports and vice versa.
> I don't think you need to put the DT overlay in the EEPROM. All you
> need to do is translate the manifest into DT for those simple devices
> which can be described by the limited manifest format. For more
> complex devices, you use the ID to go find a DT fragment which
> describes the board, and skip the manifest to DT transformation.
>
> 	Andrew

I am not familiar enough to know if the device tree can work with 
virtual devices created by greybus subsystem.

Maybe the problem stems from the fact that mikroBUS does not have a 
physical controller (and my inability to explain the patch properly). 
However, the purpose of this patchset is to in fact provide a virtual 
mikroBUS controller to allow us to register a mikroBUS addon board 
described by board_info struct similar to how it is possible to create 
and register an i2c device on an i2c adapter using 
`i2c_new_client_device` or spi device using `spi_new_device`. The 
manifest is used to populate this board_info struct, but it will be 
possible to use something other than mikroBUS manifest if someone wants 
to. I can make the necessary adjustments by moving manifest support to 
its own config option.


Link: 
https://elixir.bootlin.com/linux/latest/source/drivers/staging/greybus/i2c.c#L230 
Greybus i2c


Ayush Singh
Andrew Lunn March 20, 2024, 6:44 p.m. UTC | #10
On Wed, Mar 20, 2024 at 10:09:05PM +0530, Ayush Singh wrote:
> On 3/20/24 01:02, Andrew Lunn wrote:
> 
> > > Yes, after discussion with Vaishnav and trying to brainstorm some way to do
> > > the same thing with dt overlays, it seems that trying to use dt overlays
> > > will mean need to have completely separate implementation of mikroBUS for
> > > local ports and mikroBUS over greybus.
> > Could you explain why please?
> > 
> > Are greybus I2C bus masters different from physical I2C bus masters?
> > Are greybus SPI bus masters different from physical SPI bus masters?
> 
> Well, they are virtual, so they are not declared in the device tree. I have
> linked the greybus i2c implementation. It basically allocates an i2c_adpater
> and then adds it using `i2c_add_adapter` method. This adapter can then be
> passed to say mikroBUS driver where it can be used as a normal i2c_adapter,
> and we can register the device to it.

Being virtual does not really stop it being added to the DT.

I'm making this all up, but i assume it will look something like this:

greybus@42 {
        compatible = "acme,greybus";
        reg = <0x42 0x100>;

This would represent the greybus host controller.

	module@0 {
		 reg = <0>;

This would represent a module discovered on the bus. I assume there is
some sort of addressing? The greybus core code dynamically creates the
node in DT to describe the modules it has discovered. This is not too
different to USB. You can already describe USB devices in DT, but the
assumption is you know they exists, e.g. because they are hard wired,
not hot-plugable. The USB core will associate the USB device with the
node in DT. But actually creating a node in DT is not too big a jump.

		interface@0 {
     			compatible = "greybus,i2c";
			reg = <0>;
		}
		interface@1 {
     			compatible = "greybus,spi";
			reg = <1>;
		}
		interface@10 {
     			compatible = "greybus,gpio";
			reg = <10>;
		}

It can then enumerate the interfaces on the module, and create the I2C
node, SPI bus node, the gpio controller etc. Again, the greybus core
can add nodes to DT to described the discovered hardware, and
associate them to the linux devices which are created.

That gives you what you need to load a DT overlay to make use of these
devices. That overlay would contain one of your virtual mikroBUS
controllers. This virtual controller is basically a phandle-proxy. The
virtual mikroBUS controllers is a consumer of phandles to an I2C bus,
an SPI bus, GPIO bus which makes up the pins routed to the mikroBUS
connector. The virtual mikroBUS controllers is also a provider of an
I2C bus, an SPI bus, GPIO controller. The mikroBUS device consumes
these I2C bus, SPI bus etc. The virtual mikroBUS controllers makes it
simpler for the device to find the resources it needs, since they are
all in one place. For a physical mikroBUS you have a DT node with
phandles to the physical devices. For greybus you create a virtual
device with phandles to the virtual devices added to the DT bus.

You then have everything you need to describe the mikroBUS
devices. For very simple devices you convert the manifest to a DT
overlay and load it. For complex devices you directly use a DT
overlay.

I also don't see any need to do the manifest to DT overlay conversion
on the fly. You have a database of manifests. They could be converted
to DT and then added to the linux-firmware repo, for example. If
device with an unknown manifest is found, it should be possible to
read the manifest in userspace via its eeprom in /sys/class/. An tool
could create DT blob and add it to /lib/firmware to get it working
locally, and provide suggestions how to contribute it to the linux
firmware project?

   Andrew
Vaishnav Achath March 21, 2024, 6:30 a.m. UTC | #11
Hi Conor,

On 19/03/24 23:49, Conor Dooley wrote:
> On Tue, Mar 19, 2024 at 11:05:37PM +0530, Vaishnav Achath wrote:
>> Hi Andrew,
>>
>> On 19/03/24 17:55, Andrew Lunn wrote:
>>>> The device tree defines the SPI controller associated with mikroBUS SPI
>>>> pins. The driver on match queries and takes a reference to the SPI
>>>> controller but does nothing with it. Once a mikroBUS add-on board is
>>>> detected (by passing manifest using sysfs or reading from 1-wire EEPROM),
>>>> the driver parses the manifest, and if it detects an SPI device in manifest,
>>>> it registers SPI device along with setting properties such as `chip_select`,
>>>> `max_speed_hz`, `mode`, etc.,
>>>
>>> How complex can the description of the hardware be in the manifest?
>>>
>>> Could i describe an SPI to I2C converter? And then a few temperature
>>> sensors, a fan controller, and a GPIO controller on that I2C bus? And
>>> the GPIO controller is then used for LEDs and a push button? DT
>>> overlays could describe that. Can the manifest?
>>
>> No, it cannot describe such complex hardware, it can only describe simple
>> devices (sensors/displays .etc) on a standard mikroBUS add-on board, we did
>> a analysis on what mikroBUS add-on boards have driver support in Linux and
>> then noticed that most devices does not need this kind of complex
>> description to work:
>> https://elinux.org/MikroEClicks_with_Linux_Support
> 
> What happens to the devices that fall outside of the "do not need a
> complex description" category? Do you expect that those would be
> described by a dt overlay?
> 

Yes, those would need a device tree overlay, but most mikroBUS add-on 
boards does not need this, almost all the boards need the standard bus 
properties (SPI/I2C properties), IRQ, named-gpios, named properties, 
regulators, clocks and the current implementation supports this.

Looking at the example Andrew provided above (SPI-I2C converter with 
sensors .etc on the I2C bus and GPIO controller) - usually you will not 
find such a mikroBUS add-on board, because if there are I2C devices they 
would directly be on the mikroBUS I2C bus rather than on the converter, 
now someone can do this in their custom solution but then it is no 
different than an I2C adapter on USB/PCIe, does the standard discovery 
mechanism on those buses help instantiate devices on the I2C? my 
understanding is NO and you will need to write a custom device tree 
overlay for the same and same will be the case here.

>> The greybus manifest already is being used in the greybus susbystem for
>> describing an interface and there are already greybus controllers (SPI/I2C
>> .etc) being created according to the manifest contents, all this driver does
>> is to extend that format to be able to instantiate devices on these buses.
>> The primary goals for introducing the driver for mikroBUS add-on boards are:
>>
>> 1) A way to isolate platform specific information from add-on board specific
>> information - so that each permutation of connecting the add-on board on
>> different ports on different board does not require a new overlay.
>> 2) A way to instantiate add-on boards on greybus created virtual mikroBUS
>> ports.
>> 3) Both 1 and 2 should use the same add-on board description format.
>>
>> Standard device tree overlays did not help to achieve this and that is why
>> the standard interface discovery mechanism in greybus, the manifest was
>> extended even though it is not the most optimal way to describe hardware.
>>
>> The greybus manifest extensions were made with the following things in mind
>> and three new descriptor were introduced:
>> 1) mikrobus descriptor - pinmux/port state
>> 2) device descriptor - contains information which is a superset of struct
>> i2c_board_info , struct spi_board_info .etc
>> 3) property descriptor - to describe named properties of the types defined
>> under https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/property.h#n22
>>
>> With these we were able to test around 150 add-on boards with corresponding
>> drivers in Linux :
>> https://github.com/MikroElektronika/click_id/tree/main/manifests
>>
>> The mechanism is not as robust a device tree and should not be compared, the
> 
> Why not? You're suggesting this as a method for describing devices and you
> seem to have extended the manifest to support more complex properties, why
> shouldn't someone question make that comparison?
> 

Agreed, but the comparison need to limited to the expansion interface 
(mikroBUS) under discussion as the idea is not to create an alternate 
interface for describing generic devices, the   class of add-on boards 
that can fit in the mikroBUS add-on board form factor and on the buses 
exposed by mikroBUS requires only simple descriptions - namely standard 
bus properties (SPI/I2C properties), IRQ, named-gpios, named properties, 
regulators, clocks and the extensions to manifest were made for those 
properties only. Also the extensions were done to support the properties 
under unified device property interface under include/linux/property.h

Thanks and Regards,
Vaishnav

>> intent was not to create a new hardware description format, but extend the
>> existing greybus manifest format to be able to instantiate devices on the
>> greybus SPI/I2C/GPIO/ (mikroBUS)
>
Vaishnav Achath March 21, 2024, 7:07 a.m. UTC | #12
Hi Andrew,

On 20/03/24 00:53, Andrew Lunn wrote:
> On Tue, Mar 19, 2024 at 11:05:37PM +0530, Vaishnav Achath wrote:
>> Hi Andrew,
>>
>> On 19/03/24 17:55, Andrew Lunn wrote:
>>>> The device tree defines the SPI controller associated with mikroBUS SPI
>>>> pins. The driver on match queries and takes a reference to the SPI
>>>> controller but does nothing with it. Once a mikroBUS add-on board is
>>>> detected (by passing manifest using sysfs or reading from 1-wire EEPROM),
>>>> the driver parses the manifest, and if it detects an SPI device in manifest,
>>>> it registers SPI device along with setting properties such as `chip_select`,
>>>> `max_speed_hz`, `mode`, etc.,
>>>
>>> How complex can the description of the hardware be in the manifest?
>>>
>>> Could i describe an SPI to I2C converter? And then a few temperature
>>> sensors, a fan controller, and a GPIO controller on that I2C bus? And
>>> the GPIO controller is then used for LEDs and a push button? DT
>>> overlays could describe that. Can the manifest?
>>
>> No, it cannot describe such complex hardware, it can only describe simple
>> devices (sensors/displays .etc) on a standard mikroBUS add-on board, we did
>> a analysis on what mikroBUS add-on boards have driver support in Linux and
>> then noticed that most devices does not need this kind of complex
>> description to work:
>> https://elinux.org/MikroEClicks_with_Linux_Support
> 
> Is that because the current software support is too limited? Are there
> manufactures who want to create more complex designed, but are limited
> by what can be described in the manifest?
> 

most mikroBUS add-on boards in production lies in the category of 
sensors, displays, connectivity, mixed signal (ADC/DAC .etc) and if you 
look at the existing bindings under bindings/iio/ , most devices need 
only simple descriptions and the properties are mainly standard bus 
properties (SPI/I2C properties), IRQ, named-gpios, named properties, 
regulators, clocks the extension to manifest was made taking this into 
account and the named property description interface just maps the 
manifest entries to the unified device property interface under 
include/linux/property.h

> Do you have a list of boards without Linux support? Why do they not
> have Linux support? Is there a "vendor crap" driver which makes them
> work? Does it make them work by working around the manifest
> limitations?
> 

I did the survey in 2020, close to 600 board did not have Linux support 
and 150 board had support then, the boards which did not have Linux 
support was mostly because there was no corresponding driver present and 
a lot of these were similar to the 150 that had support (IIO sensors, 
ADC, DACs .etc), there is no vendor(Example MikroElektronika) drivers 
being maintained, so I am not sure if there are drivers working around 
limitations of manifests , but for the 150 boards that we have tested 
support we never had to make any changes to the underlying device 
drivers to be supported.

>> The greybus manifest already is being used in the greybus susbystem for
>> describing an interface and there are already greybus controllers (SPI/I2C
>> .etc) being created according to the manifest contents, all this driver does
>> is to extend that format to be able to instantiate devices on these buses.
> 
> I don't know anything about greybus, so let me ask a few background
> questions. Are these SPI and I2C controller plain Linux SPI and I2C
> controllers? They fit the usual device model, they appear in
> /sys/class/bus etc? Are the GPIO controllers also just plain Linux
> GPIO controllers? All the drivers have a bottom interface which uses
> greybus to perform some sort of RPC, but the top interface is standard
> Linux. So in fact they are not so different to I2C over USB, SPI over
> USB, GPIO over USB?

They are very similar and all the details you mentioned are correct, I 
will provide some comments on the DT proposal you made and why we could 
not implement that approach initially, primarily it is because PCIe and 
USB has OF device tree support and USB interface nodes are children of 
USB device nodes and there is some hardware parent we can tie USB 
interface to and share/derive the of_node, but in case of greybus we 
could not find such mapping - looking at your proposal that is more 
maintainable in the long term, have some doubts regarding the proposal 
will post in the other thread.

Thanks and Regards,
Vaishnav

> 
>       Andrew
Vaishnav Achath March 21, 2024, 7:35 a.m. UTC | #13
Hi Andrew,

On 21/03/24 00:14, Andrew Lunn wrote:
> On Wed, Mar 20, 2024 at 10:09:05PM +0530, Ayush Singh wrote:
>> On 3/20/24 01:02, Andrew Lunn wrote:
>>
>>>> Yes, after discussion with Vaishnav and trying to brainstorm some way to do
>>>> the same thing with dt overlays, it seems that trying to use dt overlays
>>>> will mean need to have completely separate implementation of mikroBUS for
>>>> local ports and mikroBUS over greybus.
>>> Could you explain why please?
>>>
>>> Are greybus I2C bus masters different from physical I2C bus masters?
>>> Are greybus SPI bus masters different from physical SPI bus masters?
>>
>> Well, they are virtual, so they are not declared in the device tree. I have
>> linked the greybus i2c implementation. It basically allocates an i2c_adpater
>> and then adds it using `i2c_add_adapter` method. This adapter can then be
>> passed to say mikroBUS driver where it can be used as a normal i2c_adapter,
>> and we can register the device to it.
> 
> Being virtual does not really stop it being added to the DT.
> 
> I'm making this all up, but i assume it will look something like this:
> 
> greybus@42 {
>          compatible = "acme,greybus";
>          reg = <0x42 0x100>;
> 
> This would represent the greybus host controller.
> 
> 	module@0 {
> 		 reg = <0>;
> 
> This would represent a module discovered on the bus. I assume there is
> some sort of addressing? The greybus core code dynamically creates the
> node in DT to describe the modules it has discovered. This is not too
> different to USB. You can already describe USB devices in DT, but the
> assumption is you know they exists, e.g. because they are hard wired,
> not hot-plugable. The USB core will associate the USB device with the
> node in DT. But actually creating a node in DT is not too big a jump.
> 
> 		interface@0 {
>       			compatible = "greybus,i2c";
> 			reg = <0>;
> 		}
> 		interface@1 {
>       			compatible = "greybus,spi";
> 			reg = <1>;
> 		}
> 		interface@10 {
>       			compatible = "greybus,gpio";
> 			reg = <10>;
> 		}
> 
> It can then enumerate the interfaces on the module, and create the I2C
> node, SPI bus node, the gpio controller etc. Again, the greybus core
> can add nodes to DT to described the discovered hardware, and
> associate them to the linux devices which are created.
> 

This proposal looks great and would be the ideal solution, but we met 
with few challenges when initially trying to implement something like 
this and had to drop and take the route with minimal development effort 
to just instantiate mikroBUS devices.

 From what we understand, you are recommending to change the manifest 
description format used by greybus to device tree and also add of_bus 
support for greybus - now this will not only solve instantiating 
mikrobus devices on greybus but even complex devices on greybus making 
it a robust solution and using standard tools and support DT offers.

However we have a few doubts:
* For USB or PCIe, to add OF device tree support the parent devices are 
physically present, for example USB device is a child node of USB 
controller (physically description available in a SoC DT) and USB 
interfaces are child of USB devices, how would that hierarchy look for 
greybus devices?
Would it be
USB/UART/transport controller -> AP Bridge host controller -> Module -> 
interface -> bundle -> CPort ?

When this mikrobus driver was initially implemented we could not think 
of such an approach as the SVC and Control functionality were 
implemented in userspace with gbridge ( 
https://github.com/anobli/gbridge ) with a netlink interface to kernel 
greybus, but today there are references to do it completely in kernel ( 
drivers/greybus/gb-beagleplay.c) and your proposal is implementable.

Also with this the manifesto tool which is not very well maintained is 
not necessary : https://github.com/projectara/manifesto

> That gives you what you need to load a DT overlay to make use of these
> devices. That overlay would contain one of your virtual mikroBUS
> controllers. This virtual controller is basically a phandle-proxy. The
> virtual mikroBUS controllers is a consumer of phandles to an I2C bus,
> an SPI bus, GPIO bus which makes up the pins routed to the mikroBUS
> connector. The virtual mikroBUS controllers is also a provider of an
> I2C bus, an SPI bus, GPIO controller. The mikroBUS device consumes
> these I2C bus, SPI bus etc. The virtual mikroBUS controllers makes it
> simpler for the device to find the resources it needs, since they are
> all in one place. For a physical mikroBUS you have a DT node with
> phandles to the physical devices. For greybus you create a virtual
> device with phandles to the virtual devices added to the DT bus.
> 
> You then have everything you need to describe the mikroBUS
> devices. For very simple devices you convert the manifest to a DT
> overlay and load it. For complex devices you directly use a DT
> overlay.
> 
> I also don't see any need to do the manifest to DT overlay conversion
> on the fly. You have a database of manifests. They could be converted
> to DT and then added to the linux-firmware repo, for example. If
> device with an unknown manifest is found,

  How do we know if we found a device with unknown manifest if we don't 
read the EEPROM?

  it should be possible to
> read the manifest in userspace via its eeprom in /sys/class/. An tool
> could create DT blob and add it to /lib/firmware to get it working
> locally, and provide suggestions how to contribute it to the linux
> firmware project?

Agreed, but on what basis will you load the particular manifest for a 
add-on board if you are not reading the DT overlay (or manifest blob) 
from the EEPROM?

Thanks and Regards,
Vaishnav

> 
>     Andrew
Michael Walle March 21, 2024, 9:38 a.m. UTC | #14
Hi,

> > Is that because the current software support is too limited? Are there
> > manufactures who want to create more complex designed, but are limited
> > by what can be described in the manifest?
> > 
>
> most mikroBUS add-on boards in production lies in the category of 
> sensors, displays, connectivity, mixed signal (ADC/DAC .etc) and if you 
> look at the existing bindings under bindings/iio/ , most devices need 
> only simple descriptions and the properties are mainly standard bus 
> properties (SPI/I2C properties), IRQ, named-gpios, named properties, 
> regulators, clocks the extension to manifest was made taking this into 
> account and the named property description interface just maps the 
> manifest entries to the unified device property interface under 
> include/linux/property.h

How will the ethernet boards ([1], [2]) work? Where do they get
their MAC address from, for example. The DT has some nice properties
for that, but I doubt that will be possible with the manifest files.
I've looked at the manifest file for the w5500 board [3] and to me
it looks like that board will come up with a random MAC address on
each start. Thus, even today, you have some boards which require
a more complex description.

Apart from the discussion whether the manifest is a suitable or
sufficient mechanism to describe the hardware, I think the main
problem with the proposed binding, is that it doesn't follow the
binding Rob was proposing for a socket. If I want to use DT
overlays, how would you describe an add-on board?

The proposal was that the base board has something like

mikrobus: socket {
	compatible = "mikrobus-socket";
	i2c-parent = <&i2c0>;
	spi-parent = <&spi0>;

	i2c {};
	spi {};
};

an add-on board can then have a DT snippet/overlay like the
following:

&mikrobus {
	i2c {
		eeprom@52: {
			reg = <52>;
			compatible = <atmel,at24..>;
		}
	};

	spi {
		sensor@0: {
			reg = <0>;
			compatible = <foobar>;
		};
	};
};

That should be possible with a binding for the mikrobus, which
in fact it is just a pin header with a standard pinout. Also as
Russell pointed out in v3, the EEPROM/manifest is not part of the
mikrobus standard. So maybe that deserves an own compatible, like

   compatible = "mikroe,click", "mikrobus-socket";

Or maybe click-eeprom? Although click seems to be the brand name of
MikroElektronika.

-michael

[1] https://www.mikroe.com/eth-3-click
[2] https://www.mikroe.com/eth-wiz-click
[3] https://github.com/MikroElektronika/click_id/blob/main/manifests/ETH-WIZ-CLICK.mnfs
Vaishnav Achath March 21, 2024, 11:55 a.m. UTC | #15
On 21/03/24 15:08, Michael Walle wrote:
> Hi,
> 
>>> Is that because the current software support is too limited? Are there
>>> manufactures who want to create more complex designed, but are limited
>>> by what can be described in the manifest?
>>>
>>
>> most mikroBUS add-on boards in production lies in the category of
>> sensors, displays, connectivity, mixed signal (ADC/DAC .etc) and if you
>> look at the existing bindings under bindings/iio/ , most devices need
>> only simple descriptions and the properties are mainly standard bus
>> properties (SPI/I2C properties), IRQ, named-gpios, named properties,
>> regulators, clocks the extension to manifest was made taking this into
>> account and the named property description interface just maps the
>> manifest entries to the unified device property interface under
>> include/linux/property.h
> 
> How will the ethernet boards ([1], [2]) work? Where do they get
> their MAC address from, for example. The DT has some nice properties
> for that, but I doubt that will be possible with the manifest files.
> I've looked at the manifest file for the w5500 board [3] and to me
> it looks like that board will come up with a random MAC address on
> each start. Thus, even today, you have some boards which require
> a more complex description.
> 

Agreed, this is a limitation, unless the corresponding 
drivers/subsystems use device_property_read_* helper to fetch 
properties, it will not work and net/core/of_net.c only implements 
of_get_* helpers even though the underlying functions can be implemented 
with equivalent device_property_read_* equivalent as well.

> Apart from the discussion whether the manifest is a suitable or
> sufficient mechanism to describe the hardware, I think the main
> problem with the proposed binding, is that it doesn't follow the
> binding Rob was proposing for a socket. If I want to use DT
> overlays, how would you describe an add-on board?
> 
> The proposal was that the base board has something like
> 
> mikrobus: socket {
> 	compatible = "mikrobus-socket";
> 	i2c-parent = <&i2c0>;
> 	spi-parent = <&spi0>;
> 
> 	i2c {};
> 	spi {};
> };
> 
> an add-on board can then have a DT snippet/overlay like the
> following:
> 
> &mikrobus {
> 	i2c {
> 		eeprom@52: {
> 			reg = <52>;
> 			compatible = <atmel,at24..>;
> 		}
> 	};
> 
> 	spi {
> 		sensor@0: {
> 			reg = <0>;
> 			compatible = <foobar>;
> 		};
> 	};
> };
> 
> That should be possible with a binding for the mikrobus, which
> in fact it is just a pin header with a standard pinout. Also as
> Russell pointed out in v3, the EEPROM/manifest is not part of the
> mikrobus standard. So maybe that deserves an own compatible, like
> 
>     compatible = "mikroe,click", "mikrobus-socket";
> 
> Or maybe click-eeprom? Although click seems to be the brand name of
> MikroElektronika.

Agreed, there is nothing preventing us to convert the binding and update 
the driver to follow the above proposed format and will be done in next 
revision. Click is brand name of MikroElektronika and they don't allow 
custom boards to use that branding, however clickid can be used in the 
case where EEPROM is present/enable the socket to be probeable.

Thanks and Regards,
Vaishnav

> 
> -michael
> 
> [1] https://www.mikroe.com/eth-3-click
> [2] https://www.mikroe.com/eth-wiz-click
> [3] https://github.com/MikroElektronika/click_id/blob/main/manifests/ETH-WIZ-CLICK.mnfs
Andrew Lunn March 21, 2024, 12:31 p.m. UTC | #16
On Thu, Mar 21, 2024 at 01:05:14PM +0530, Vaishnav Achath wrote:
> Hi Andrew,
> 
> On 21/03/24 00:14, Andrew Lunn wrote:
> > On Wed, Mar 20, 2024 at 10:09:05PM +0530, Ayush Singh wrote:
> > > On 3/20/24 01:02, Andrew Lunn wrote:
> > > 
> > > > > Yes, after discussion with Vaishnav and trying to brainstorm some way to do
> > > > > the same thing with dt overlays, it seems that trying to use dt overlays
> > > > > will mean need to have completely separate implementation of mikroBUS for
> > > > > local ports and mikroBUS over greybus.
> > > > Could you explain why please?
> > > > 
> > > > Are greybus I2C bus masters different from physical I2C bus masters?
> > > > Are greybus SPI bus masters different from physical SPI bus masters?
> > > 
> > > Well, they are virtual, so they are not declared in the device tree. I have
> > > linked the greybus i2c implementation. It basically allocates an i2c_adpater
> > > and then adds it using `i2c_add_adapter` method. This adapter can then be
> > > passed to say mikroBUS driver where it can be used as a normal i2c_adapter,
> > > and we can register the device to it.
> > 
> > Being virtual does not really stop it being added to the DT.
> > 
> > I'm making this all up, but i assume it will look something like this:
> > 
> > greybus@42 {
> >          compatible = "acme,greybus";
> >          reg = <0x42 0x100>;
> > 
> > This would represent the greybus host controller.
> > 
> > 	module@0 {
> > 		 reg = <0>;
> > 
> > This would represent a module discovered on the bus. I assume there is
> > some sort of addressing? The greybus core code dynamically creates the
> > node in DT to describe the modules it has discovered. This is not too
> > different to USB. You can already describe USB devices in DT, but the
> > assumption is you know they exists, e.g. because they are hard wired,
> > not hot-plugable. The USB core will associate the USB device with the
> > node in DT. But actually creating a node in DT is not too big a jump.
> > 
> > 		interface@0 {
> >       			compatible = "greybus,i2c";
> > 			reg = <0>;
> > 		}
> > 		interface@1 {
> >       			compatible = "greybus,spi";
> > 			reg = <1>;
> > 		}
> > 		interface@10 {
> >       			compatible = "greybus,gpio";
> > 			reg = <10>;
> > 		}
> > 
> > It can then enumerate the interfaces on the module, and create the I2C
> > node, SPI bus node, the gpio controller etc. Again, the greybus core
> > can add nodes to DT to described the discovered hardware, and
> > associate them to the linux devices which are created.
> > 
> 
> This proposal looks great and would be the ideal solution, but we met with
> few challenges when initially trying to implement something like this and
> had to drop and take the route with minimal development effort to just
> instantiate mikroBUS devices.
> 
> From what we understand, you are recommending to change the manifest
> description format used by greybus to device tree and also add of_bus
> support for greybus - now this will not only solve instantiating mikrobus
> devices on greybus but even complex devices on greybus making it a robust
> solution and using standard tools and support DT offers.

I would not discard the manifest. It exists, it appears to be used by
a lot of devices. So use it. But consider it an 'intermediate
representation'. Take it and transform it to DT.

Looking at:

https://libstock.mikroe.com/projects/view/5435/clickid

there appears to be a name, and hardware revision in the
manifest. Convert that to a string. Run a checksum over the rest of
the manifest, and append that to the string. You can then look in
/lib/firmware for a DT representation which matches.

> However we have a few doubts:
> * For USB or PCIe, to add OF device tree support the parent devices are
> physically present, for example USB device is a child node of USB controller
> (physically description available in a SoC DT) and USB interfaces are child
> of USB devices, how would that hierarchy look for greybus devices?

So DT support for USB, serial and PCIe devices already exists. When
the core enumerates a PCIe or USB bus, it looks in the DT blob for the
vendor:product ID, and associates any node it finds with the
device. It is not used very often, but if you search the .dts files,
you can find examples.

> Would it be
> USB/UART/transport controller -> AP Bridge host controller -> Module ->
> interface -> bundle -> CPort ?

That is for you to decide. I don't know the architecture well enough.

It is rather old, but:

https://lwn.net/Articles/715955/

There is a diagram of the sysfs tree, which looks pretty similar to
what you say above. What is however missing from the diagram is the
Linux devices themselves. Where do the I2C bus, the SPI bus, the GPIO
controller etc appear in the tree?

Maybe look at PCI and USB. Does the device tree representation include
all the bridges and hubs etc. Or does it simply the representation?

You need something to represent the controller. You need modules. Do
you need the details of interface & bundle and cport? Could you
represent them as an address tuple, and just have the devices under
module?

> When this mikrobus driver was initially implemented we could not think of
> such an approach as the SVC and Control functionality were implemented in
> userspace with gbridge ( https://github.com/anobli/gbridge ) with a netlink
> interface to kernel greybus, but today there are references to do it
> completely in kernel ( drivers/greybus/gb-beagleplay.c) and your proposal is
> implementable.

Does gb-beagleplay.c work together with the code in staging? It looks
like the GPIO controller, I2C controller, SPI controller, etc are
still in GregKH "Tree of crap". I don't think it is wise to build on
top of something in staging. So you probably need to spend time to
cleanup that code and moving it into the mainline proper. As you do
that, you could add the code needed to dynamically add nodes to device
tree.

	Andrew
Michael Walle March 21, 2024, 12:44 p.m. UTC | #17
Hi,

> >>> Is that because the current software support is too limited? Are there
> >>> manufactures who want to create more complex designed, but are limited
> >>> by what can be described in the manifest?
> >>>
> >>
> >> most mikroBUS add-on boards in production lies in the category of
> >> sensors, displays, connectivity, mixed signal (ADC/DAC .etc) and if you
> >> look at the existing bindings under bindings/iio/ , most devices need
> >> only simple descriptions and the properties are mainly standard bus
> >> properties (SPI/I2C properties), IRQ, named-gpios, named properties,
> >> regulators, clocks the extension to manifest was made taking this into
> >> account and the named property description interface just maps the
> >> manifest entries to the unified device property interface under
> >> include/linux/property.h
> > 
> > How will the ethernet boards ([1], [2]) work? Where do they get
> > their MAC address from, for example. The DT has some nice properties
> > for that, but I doubt that will be possible with the manifest files.
> > I've looked at the manifest file for the w5500 board [3] and to me
> > it looks like that board will come up with a random MAC address on
> > each start. Thus, even today, you have some boards which require
> > a more complex description.
> > 
>
> Agreed, this is a limitation, unless the corresponding 
> drivers/subsystems use device_property_read_* helper to fetch 
> properties, it will not work and net/core/of_net.c only implements 
> of_get_* helpers even though the underlying functions can be implemented 
> with equivalent device_property_read_* equivalent as well.

And I don't think this is a good idea given that the alternative is
just working.

> > Apart from the discussion whether the manifest is a suitable or
> > sufficient mechanism to describe the hardware, I think the main
> > problem with the proposed binding, is that it doesn't follow the
> > binding Rob was proposing for a socket. If I want to use DT
> > overlays, how would you describe an add-on board?
> > 
> > The proposal was that the base board has something like
> > 
> > mikrobus: socket {
> > 	compatible = "mikrobus-socket";
> > 	i2c-parent = <&i2c0>;
> > 	spi-parent = <&spi0>;
> > 
> > 	i2c {};
> > 	spi {};
> > };
> > 
> > an add-on board can then have a DT snippet/overlay like the
> > following:
> > 
> > &mikrobus {
> > 	i2c {
> > 		eeprom@52: {
> > 			reg = <52>;
> > 			compatible = <atmel,at24..>;
> > 		}
> > 	};
> > 
> > 	spi {
> > 		sensor@0: {
> > 			reg = <0>;
> > 			compatible = <foobar>;
> > 		};
> > 	};
> > };
> > 
> > That should be possible with a binding for the mikrobus, which
> > in fact it is just a pin header with a standard pinout. Also as
> > Russell pointed out in v3, the EEPROM/manifest is not part of the
> > mikrobus standard. So maybe that deserves an own compatible, like
> > 
> >     compatible = "mikroe,click", "mikrobus-socket";
> > 
> > Or maybe click-eeprom? Although click seems to be the brand name of
> > MikroElektronika.
>
> Agreed, there is nothing preventing us to convert the binding and update 
> the driver to follow the above proposed format and will be done in next 
> revision. Click is brand name of MikroElektronika and they don't allow 
> custom boards to use that branding, however clickid can be used in the 
> case where EEPROM is present/enable the socket to be probeable.

Thinking about this again. I'm not sure this additional compatible
really helps the discovery. It's a chicken egg problem. Only the
modules knows if it has an EEPROM, but then, we already have to
know it's in the socket. So while it might help for a static
configuration, it does not for the discovery.

-michael

> > [1] https://www.mikroe.com/eth-3-click
> > [2] https://www.mikroe.com/eth-wiz-click
> > [3] https://github.com/MikroElektronika/click_id/blob/main/manifests/ETH-WIZ-CLICK.mnfs
Andrew Lunn March 21, 2024, 12:55 p.m. UTC | #18
> > How will the ethernet boards ([1], [2]) work? Where do they get
> > their MAC address from, for example. The DT has some nice properties
> > for that, but I doubt that will be possible with the manifest files.
> > I've looked at the manifest file for the w5500 board [3] and to me
> > it looks like that board will come up with a random MAC address on
> > each start. Thus, even today, you have some boards which require
> > a more complex description.
> > 
> 
> Agreed, this is a limitation, unless the corresponding drivers/subsystems
> use device_property_read_* helper to fetch properties, it will not work and
> net/core/of_net.c only implements of_get_* helpers even though the
> underlying functions can be implemented with equivalent
> device_property_read_* equivalent as well.

I think this is a good example of the limitations. For an Ethernet
NIC, you often want to describe properties of both the MAC and the
PHY. What phy-mode should be used, what delays on the RGMII bus, what
LEDs are there etc. This is a pretty much solved problem with DT, we
have a well defined sub tree to represent the MAC, the MDIO bus and
the PHY on the bus.

But we do have two classes of properties here. The MAC address is
unique to a board. So that does need to be stored in the EEPROM, and
cannot be in a one time converted manifest to DT file stored in
/lib/firmware. However, to some extent, this is a solved problem. We
have a DT representation of how to look in an EEPROM to find the MAC
address. So the DT just needs to point to some bytes in the manifest
in the EEPROM.

	Andrew