Hello Vaishnav, I should say, an excellent work on the greybus_manifest.h file.
Actually, my thoughts will be to have a two-stage commit of the whole MikroBUS patch. The first one are these changes with greybus_manifest.h, followed by dependent mikrobus_core.h and mikrobus_manifest.h. These two should have included #include <linux/greybus/greybus_manifest.h> to reflect the correct hierarchical structure. The rest is with the mikrobus driver .c code. It is just an observation from me, I guess, it is obvious. My two cent worth comment, Zoran _______ On Thu, Aug 20, 2020 at 2:49 AM Vaishnav M A <vaish...@beagleboard.org> wrote: > > Hi, > > Trying to add more information regarding the newly added > descriptors and describe how they are used now within the > mikroBUS driver. > > On Tue, Aug 18, 2020 at 6:18 PM Vaishnav M A <vaish...@beagleboard.org> wrote: > > > > This patch adds new descriptors used in the manifest parsing inside > > the mikrobus driver, the device descriptor help to describe the > > devices on a mikroBUS port, mikrobus descriptor is used to set up > > the mikrobus port pinmux and GPIO states and property descriptor > > to pass named properties to device drivers through the Unified > > Properties API under linux/property.h > > > > The corresponding pull request for manifesto is updated > > at : https://github.com/projectara/manifesto/pull/2 > > > > Signed-off-by: Vaishnav M A <vaish...@beagleboard.org> > > --- > > include/linux/greybus/greybus_manifest.h | 47 ++++++++++++++++++++++++ > > 1 file changed, 47 insertions(+) > > > > diff --git a/include/linux/greybus/greybus_manifest.h > > b/include/linux/greybus/greybus_manifest.h > > index 6e62fe478712..821661ea7f01 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, > > The mikrobus descriptor is used to pass information about > the specific pinmux settings and the default GPIO states on > the mikrobus port to be set up for the add-on board to work > correctly, this descriptor has 12 u8 fields(corresponding to the > 12 pins on the mikrobus port) which includes information > about the prior setup required on the mikroBUS port for the > device(s) on the add-on board to work correctly. The mikrobus > descriptor is a fixed-length descriptor and there will be only a > single instance of mikrobus descriptor per add-on board manifest. > > > + GREYBUS_TYPE_PROPERTY = 0x06, > > The property descriptors are used to pass named properties > to the device drivers through the Unified device property interface > under linux/property.h , so that device drivers using the > device_property_read_* call can get the named properties, > the mikrobus driver fetches the information from the manifest > binary and forms a corresponding `struct property_entry` which > will be attached to the `struct device`. > The property descriptor is a variable-length descriptor similar > to the string descriptor and there can be multiple instances of > property descriptor per add-on board manifest. > > > + GREYBUS_TYPE_DEVICE = 0x07, > > The device descriptor is used to describe a device on the > mikrobus port and has necessary fields from `struct i2c_board_info` > and `struct spi_board_info` to describe a device on these buses > in a mikrobus port, even though SPI/I2C device info structs are used > this descriptor has enough information to describe other kinds of > devices relevant to mikrobus as well.(serdev/platform devices). > 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. > > > }; > > > > enum greybus_protocol { > > @@ -151,6 +154,47 @@ 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; > > + > > These 12 u8 fields describe the state of the pins in the > mikrobus port(in clock wise order starting from the PWM > pin) > mikrobus v2 standard specification : > https://download.mikroe.com/documents/standards/mikrobus/mikrobus-standard-specification-v200.pdf > This struct is filled from the mikrobus-descriptor > in the manifest and can have one of the values > for each pin group: > MIKROBUS_STATE_INPUT = 0x01, > MIKROBUS_STATE_OUTPUT_HIGH = 0x02, > MIKROBUS_STATE_OUTPUT_LOW = 0x03, > MIKROBUS_STATE_PWM = 0x04, ( applicable only to PWM pin) > MIKROBUS_STATE_SPI = 0x05, ( applicable only to > the group of MOSI, MISO, SCK , CS pins on mikroBUS port) > MIKROBUS_STATE_I2C = 0x06, (applicable only to the SCL, SDA > pins on the mikrobus port) > MIKROBUS_STATE_UART = 0x07,(applicable only to the RX, TX > pins on the mikrobus port) > There are two purposes for adding this descriptor, > 1) for some add-on boards some of the pins might need to > be configured as GPIOs deviating from their reserved purposes > An example for this case is an SHT15 Click > (https://www.mikroe.com/sht1x-click), > where the SCL and SDA Pins need to be configured as GPIOs > for the driver (drivers/hwmon/sht15.c) to work. The mikrobus > descriptor for this case would look like this : > [mikrobus-descriptor] > pwm-state = 4 (default, pwm) > int-state = 1 (default, input) > rx-state = 7 (default, uart) > tx-state = 7 (default, uart) > scl-state = 3 (note the SCL Pin configured as GPIO) > sda-state = 3 (note the SCL Pin configured as GPIO) > mosi-state = 5 (default, spi) > miso-state = 5 (default, spi) > sck-state = 5 (default, spi) > cs-state = 5 (default, spi) > rst-state = 2 (default, GPIO) > an-state = 1 (default, input) > 2) for some add-on boards the driver may not take care > of some additional signals like reset/wake-up/other thus > the mikrobus driver can set-up these GPIOs to a required > default state from the information from the manifest, a good > example for this is the ENC28J60 click (https://www.mikroe.com/eth-click) > where the reset line(RST pin on the mikrobus port) needs to be > pulled high. The manifest example for this add-on board can > be found here : > https://github.com/vaishnav98/manifesto/blob/mikrobusv3/manifests/ETH-CLICK.mnfs > > > +/* > > + * 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[0]; > > +} __packed; > > + > > This descriptor is used to fill in `struct property_entry` > (linux/property.h), the propname_stringid > field is used to map to the corresponding string descriptor > which has the property name, the type field has the types > under dev_prop_type (linux/property.h) and there are > some new types which are used within the mikrobus > driver, these are the new types : > MIKROBUS_PROPERTY_TYPE_LINK = 0x01 > MIKROBUS_PROPERTY_TYPE_GPIO = 0x02 > > The property-link type is used to attach an array of properties > to the corresponding device, for example, consider an SPI > EEPROM device which works with the AT25 driver( > drivers/misc/eeprom/at25.c), The device and property > descriptor parts of the manifest will look like this. > > [device-descriptor 1] > driver-string-id = 3 > prop-link = 1 (The ID of the property-descriptor which > contains the list of IDs of the actual properties to attach with > the device) > protocol = 0xb > reg = 0 > mode = 0x3 > max-speed-hz = 5000000 > [string-descriptor 3] > string = at25 (driver string) > > [property-descriptor 1] > name-string-id = 4 > type = 0x01 (type is property-link) > value = <2 3 4>(attach properties with id 2,3,4 to the device) > [string-descriptor 4] > string = prop-link > > [property-descriptor 2] > name-string-id = 5 (string id for the property name string) > type = 0x05 (U32, driver uses device_property_read_u32 call > to read the value) > value = <262144> > [string-descriptor 5] > string = size (property name string) > > [property-descriptor 3] > name-string-id = 6 > type = 0x05 > value = <256> > [string-descriptor 6] > string = pagesize > > [property-descriptor 4] > name-string-id = 7 > type = 0x05 > value = <24> > [string-descriptor 7] > string = address-width > > The gpio-link type is very similar to property descriptor and is used to > pass an array of named gpios to the device driver through GPIO lookup tables, > consider an example for a SHT15 device (drivers/hwmon/sht15.c), > the device and the property(gpio) descriptors are as follows : > > [device-descriptor 1] > driver-string-id = 3 > protocol = 0xfe > reg = 0 > gpio-link = 1 (The ID of the property-descriptor which > contains the list of IDs of the named gpio properties to attach with > the device) > > [string-descriptor 3] > string = sht11 (device_id string) > > [property-descriptor 1] > name-string-id = 4 > type = 0x02 (gpio-link) > value = <2 3> (attach properties with id 2,3 as named gpios to the device) > [string-descriptor 4] > string = gpio-link > > [property-descriptor 2] > name-string-id = 5 > type = 0x03 > value = <4> > [string-descriptor 5] > string = clk (name of the GPIO, the driver uses > devm_gpiod_get or similar calls to get the GPIO) > > [property-descriptor 3] > name-string-id = 6 > type = 0x03 > value = <5> > [string-descriptor 6] > string = data > > Note that the values here 4 and 5 for the GPIOs are > the offset numbers(clockwise starting from PWM pin) > within a mikrobus port, the mikrobus drivers translates this > offset information to the actual GPIO while creating the GPIO > lookup table, this ensures that the manifest doesn't have any > port-specific information and a single manifest can be used for > an add-on board over different platforms/sockets. > > > +/* > > + * 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 pad[3]; > > +} __packed; > > + > > The device descriptor is used to describe a device on the > mikrobus port and has necessary fields from `struct i2c_board_info` > and `struct spi_board_info`, of these fields, the irq field is similar to > the gpio descriptor value above in that the value under irq is also > the pin offset within the mikrobus port which will be translated to the > actual GPIO within the mikrobus driver and the irq-type takes types > defined under linux/interrupt.h . For a device with a > IRQF_TRIGGER_RISING interrupt on the INT pin on the mikrobus port > the fields will be : > irq = 1 (offset of INT pin) > irq_type = 1 ( IRQF_TRIGGER_RISING) > > > struct greybus_descriptor_header { > > __le16 size; > > __u8 type; /* enum greybus_descriptor_type */ > > @@ -164,6 +208,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; > > > > -- > > 2.25.1 > > > Thanks and Regards, > Vaishnav
