Re: [PATCH v4 3/5] greybus: Add mikroBUS manifest types

Vaishnav Achath
2024-03-19T08:27:02Z

On 18/03/24 01:07, Ayush Singh wrote:
> DONOTMERGE
>

Why?

> mikroBUS addon boards allow using same mikroBUS connector for a wide
> range of peripherals. It is also possible for the addon board not to use
> all the pins in mikroBUS socket (marked by NC or Not Connected). This
> would require the need to create an almost new overlays for each
> permutation of the hardware.
>
> To overcome this, a manifest format based on Greybus manifest
> specification was created which allows describing mikroBUS addon boards.
> The reason for choosing greybus manifest for the identifier is that so far
> we discussed only about physical mikroBUS ports on the board, but we can

you will need to reword the commit message properly in imperative mood,
here and in multiple other places.

> also have mikroBUS ports on a remote microcontroller appearing on host
> over greybus and there a device tree overlay solution does not work as the
> standard identifier mechanism.
>
> The patch introduces 3 new greybus descriptor types:
> 1. mikrobus-descriptor:
> Is a fixed-length descriptor (12 bytes), and the manifest shall have
> precisely one mikroBUS descriptor. Each byte describes a configuration
> of the corresponding pin on the mikroBUS addon board in a clockwise
> direction starting from the PWM pin omitting power (VCC and ground)
> pins as same as the default state of the pin.
> There are mikroBUS addon boards that use some dedicated SPI, UART, PWM,
> and I2C pins as GPIO pins, so it is necessary to redefine the default
> pin configuration of that pins on the host system. Also, sometimes it is
> required the pull-up on the host pin for correct functionality
> 2. property-descriptor:
> Are used to pass named properties or named GPIOs to the host. The host
> system uses this information to properly configure specific board
> drivers by passing the properties and GPIO name. There can be multiple
> instances of property descriptors per add-on board manifest.
> 3. device-descriptor:
> Describes a device on the mikroBUS port. The device descriptor is a
> fixed-length descriptor, and there can be multiple instances of device
> descriptors in an add-on board manifest in cases where the add-on board
> presents more than one device to the host.
>
> New mikroBUS addon boards also sometimes contain a 1-wire EEPROM with
> the mikroBUS manifest, thus enabling plug and play support.
>

new mikroBUS sometimes contain an EEPROM? aren't these called Click ID
compliant add-on boards? there should be clarity in the commit message.

> I have also created PR to add mikrobus descriptors in Greybus spec and I
> think the old PR on manifesto by Vaishnav should also work. However,
> both of these repositories seem to be inactive. I am guessing the
> greybus mailing list can provide more information on how I should go
> about these.

Why is information like these inside the commit message, these go below
the tear line.

>
> Here is a sample mikroBUS manifest:
> ```
> ;;
> ; PRESSURE CLICK
> ; https://www.mikroe.com/pressure-click
> ; CONFIG_IIO_ST_PRESS
> ;
> ; Copyright 2020 BeagleBoard.org Foundation
> ; Copyright 2020 Texas Instruments
> ;
>
> [manifest-header]
> version-major = 0
> version-minor = 1
>
> [interface-descriptor]
> vendor-string-id = 1
> product-string-id = 2
>
> [string-descriptor 1]
> string = MIKROE
>
> [string-descriptor 2]
> string = Pressure
>
> [mikrobus-descriptor]
> pwm-state = 4
> int-state = 1
> rx-state = 7
> tx-state = 7
> scl-state = 6
> sda-state = 6
> mosi-state = 5
> miso-state = 5
> sck-state = 5
> cs-state = 5
> rst-state = 2
> an-state = 1
>
> [device-descriptor 1]
> driver-string-id = 3
> protocol = 0x3
> reg = 0x5d
>
> [string-descriptor 3]
> string = lps331ap
> ```
>
> Link: https://www.mikroe.com/clickid ClickID
> Link:
> https://docs.beagleboard.org/latest/projects/beagleconnect/index.html
> beagleconnect
> Link: https://github.com/projectara/greybus-spec Greybus Spec
> Link: https://github.com/projectara/greybus-spec/pull/4 Greybus Spec PR
> Link: https://github.com/projectara/manifesto/pull/2 manifesto PR
>

The manifesto PR might not be updated.

Thanks and Regards,
Vaishnav

> 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>
> ---
> include/linux/greybus/greybus_manifest.h | 49 ++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/include/linux/greybus/greybus_manifest.h b/include/linux/greybus/greybus_manifest.h
> index bef9eb2093e9..83241e19d9b3 100644
> --- a/include/linux/greybus/greybus_manifest.h
> +++ b/include/linux/greybus/greybus_manifest.h
> @@ -23,6 +23,9 @@ enum greybus_descriptor_type {
> GREYBUS_TYPE_STRING = 0x02,
> GREYBUS_TYPE_BUNDLE = 0x03,
> GREYBUS_TYPE_CPORT = 0x04,
> + GREYBUS_TYPE_MIKROBUS = 0x05,
> + GREYBUS_TYPE_PROPERTY = 0x06,
> + GREYBUS_TYPE_DEVICE = 0x07,
> };
>
> enum greybus_protocol {
> @@ -151,6 +154,49 @@ struct greybus_descriptor_cport {
> __u8 protocol_id; /* enum greybus_protocol */
> } __packed;
>
> +/*
> + * A mikrobus descriptor is used to describe the details
> + * about the bus ocnfiguration for the add-on board
> + * connected to the mikrobus port.
> + */
> +struct greybus_descriptor_mikrobus {
> + __u8 pin_state[12];
> +} __packed;
> +
> +/*
> + * A property descriptor is used to pass named properties
> + * to device drivers through the unified device properties
> + * interface under linux/property.h
> + */
> +struct greybus_descriptor_property {
> + __u8 length;
> + __u8 id;
> + __u8 propname_stringid;
> + __u8 type;
> + __u8 value[];
> +} __packed;
> +
> +/*
> + * A device descriptor is used to describe the
> + * details required by a add-on board device
> + * driver.
> + */
> +struct greybus_descriptor_device {
> + __u8 id;
> + __u8 driver_stringid;
> + __u8 protocol;
> + __u8 reg;
> + __le32 max_speed_hz;
> + __u8 irq;
> + __u8 irq_type;
> + __u8 mode;
> + __u8 prop_link;
> + __u8 gpio_link;
> + __u8 reg_link;
> + __u8 clock_link;
> + __u8 pad[1];
> +} __packed;
> +
> struct greybus_descriptor_header {
> __le16 size;
> __u8 type; /* enum greybus_descriptor_type */
> @@ -164,6 +210,9 @@ struct greybus_descriptor {
> struct greybus_descriptor_interface interface;
> struct greybus_descriptor_bundle bundle;
> struct greybus_descriptor_cport cport;
> + struct greybus_descriptor_mikrobus mikrobus;
> + struct greybus_descriptor_property property;
> + struct greybus_descriptor_device device;
> };
> } __packed;
>