| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 28 Jun 2024 12:01:47 +0530
From: Ayush Singh <ayush@...gleboard.org>
To: Mark Brown <broonie@...nel.org>, Vaishnav M A <vaishnav@...gleboard.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Derek Kiernan <derek.kiernan@....com>,
Dragan Cvetic <dragan.cvetic@....com>, Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Nishanth Menon <nm@...com>,
Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>,
Michael Walle <mwalle@...nel.org>, Andrew Lunn <andrew@...n.ch>,
jkridner@...gleboard.org, robertcnelson@...gleboard.org
Cc: linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
Ayush Singh <ayushdevel1325@...il.com>
Subject: Re: [PATCH v5 0/7] misc: Add mikroBUS driver
On 6/27/24 21:56, Ayush Singh wrote:
> 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 device tree infrastructure.
>
> 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/all/20240317193714.403132-1-ayushdevel1325@gmail.com/ Patch v4
>
> Changes v5
> - Complete rewrite to use device tree instead of mikroBUS manifest.
> - Only support for SPI.
> - Adds `mikrobus,spi` compatible property.
>
> 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.
>
> Signed-off-by: Ayush Singh <ayush@...gleboard.org>
> ---
> Ayush Singh (7):
> dt-bindings: connector: Add mikrobus-connector
> dt-bindings: mikrobus: Add mikrobus board base
> dt-bindings: mikrobus: Add mikrobus-spi binding
> spi: Make of_find_spi_controller_by_node() available
> spi: Make of_register_spi_device() available
> mikrobus: Add mikroBUS driver
> dts: ti: k3-am625-beagleplay: Add mikroBUS
>
> .../bindings/connector/mikrobus-connector.yaml | 107 ++++++
> .../bindings/mikrobus/mikrobus-board.yaml | 20 ++
> .../devicetree/bindings/mikrobus/mikrobus-spi.yaml | 37 +++
> MAINTAINERS | 9 +
> arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 94 +++++-
> drivers/misc/Kconfig | 16 +
> drivers/misc/Makefile | 1 +
> drivers/misc/mikrobus.c | 361 +++++++++++++++++++++
> drivers/spi/spi.c | 209 ++++++------
> include/linux/spi/spi.h | 7 +
> 10 files changed, 750 insertions(+), 111 deletions(-)
> ---
> base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7
> change-id: 20240627-mikrobus-scratch-spi-ad8c98dcec98
>
> Best regards,
I would just like to summarize the discussions on different patches here
to give information regarding why the board is not subnode of
mikrobus-connector along with what questions need to be answered for a
subnode based architecture.
I will be using (```) to differentiate between code section and non-code
section. It is just for seperation not for any formatting since I am
using plaintext.
Let me first summarise the goals that should be possible with any
architecture chosen.
1. Keeping the device tree properties upstream in a system independent way.
2. Editing system dt at kernel build time to add the pre-defined board
or applying dt overlay using uboot or dynamic overlays.
3. Allowing creation of sysfs entries `new_device` and `delete_device`
similar to what already exists for I2C, etc.
4. Allow using 1-wire-eeprom in a fashion that allows automatic board
discovery.
Let me now introduce the 2 architectures we will be discussing:
1. mikrobus-connector has phandle to mikrobus-board:
```
\ {
connector1 {
board = <&board1>;
};
mikrobus_boards {
board1 {
...
};
};
};
```
2. mikrobus board is a child node of mikrobus-connector:
```
\ {
connector1 {
...
spi {
board1 {
...
};
};
};
};
```
I will now go over how each of these goals might look like in both of
the architecture.
1. Keeping the device tree properties upstream in a system independent way:
a. mikrobus-connector has phandle to mikrobus-board
It is possible to create an overlay as follows which will work with any
system that defines the `mikrobus_boards` node. This node is completely
independent of mikroBUS connector and thus does not need to be rewritten
(or generated) for each board. There are no problems for system with
more than 1 mikrobus connector.
```
&mikrobus_boards {
board2 {
...
};
board3 {
...
};
};
```
b. mikrobus board is a child node of mikrobus-connector:
Not sure how to do something similar here. The overlay needs to be
rewritten (or generated) for each board. Systems with multiple mikrobus
connectors will need multiple overlays adding the boards as child node
of each connector (with status = "disabled"). Considering how many
mikrobus boards are available, this can also lead to problem (especially
in embeded Linux) with the dt binary size since each connector is
replicating the same overlay.
```
&connector1 {
spi = {
board 2 {
...
};
board 3 {
...
};
};
};
&connector2 {
spi = {
board 2 {
...
};
board 3 {
...
};
};
};
```
Maybe it is possible to have special behavior for mikrobus-connector
nodes in dt overlay but that will break compatibility with exisiting
infrastructure which isn't great.
2. Editing system dt at kernel build time to add the pre-defined board
or applying dt overlay using uboot or dynamic overlays.
a. mikrobus-connector has phandle to mikrobus-board
```
&connector1 {
board = <&board1>;
};
```
b. mikrobus board is a child node of mikrobus-connector:
```
&connector1 {
spi = {
board 2 {
...
};
};
};
```
Both the cases will need to generate these overlays at build time.
However, in case (a), the overlay will be much smaller than case (b).
This is important for embeded Linux.
3. Allowing creation of sysfs entries `new_device` and `delete_device`
similar to what already exists for I2C, etc.
a. mikrobus-connector has phandle to mikrobus-board
It is quite simple with the current changeset APIs. I have an example
implementation here:
https://github.com/Ayush1325/linux/blob/c4e3d5138b7ad5c24bdbc1dd02d89720d3a5de82/drivers/misc/mikrobus.c#L59
.
Essentially, it is possible to pass the mikroBUS board name or id to
create changeset as long as the board has been defined in dt. The boards
definition can be added using overlay in uboot of dynamic overlays using
configfs patch.
b. mikrobus board is a child node of mikrobus-connector:
Since even the board definition overlay is now dependent on the
connector, any person writing the board overlay needs to know the name's
of the connector nodes and generate overlays for all connectors. We can
toggle a `status` property to `okay` based on the board id passed
through sysfs.
4. Allow using 1-wire-eeprom in a fashion that allows automatic board
discovery.
a. mikrobus-connector has phandle to mikrobus-board
1-wire-eeprom only needs to contain the board definition overlay which
is not dependent on the connector. The connector can generate the
changeset of add `board` property to itself. The board should work
irrespective of if the dt overlay is actually present in the kernel
config since we can read the overlay from 1-wire-eeprom and apply it
using `of_overlay_fdt_apply()`.
b. mikrobus board is a child node of mikrobus-connector:
Cannot really use the normal dt overlay. Maybe we can use the mikroBUS
manifest to dynamically create the overlay, but well, I do not wish to
support both the manifest and devicetree at the same time.
Maybe we can introduce something like partial device tree which only
contains properties to be applied to a target device node? Since
`of_overlay_fdt_apply` does contain target node property, maybe it is
already possible to have an overlay that is generic over a type of node
instead of the exact node?
I will also go through the overlay kernel internals to see if there are
any better ways to use child-nodes. Feel free to chime in if you have
any ideas.
Yours Sincerely,
Ayush Singh
Powered by blists - more mailing lists