Re: [RFC PATCH v2 0/3] mikroBUS driver for add-on boards

Re: [RFC PATCH v2 0/3] mikroBUS driver for add-on boards

[Frank Rowand]

Hi Vaishnav,

+me +devicetree

Please add these two recipients to future versions.

I will comment more after reading the first version and v2.

-Frank


On 2020-08-18 07:48, Vaishnav M A wrote:
> Hi,
> 
> This Patch series is an update to the mikroBUS driver
> RFC v1 Patch : https://lkml.org/lkml/2020/7/24/518 .
> The mikrobus driver is updated to add mikrobus ports from device-tree
> overlays, the debug interfaces for adding mikrobus ports through sysFS
> is removed, and the driver considers the extended usage of mikrobus
> port pins from their standard purposes.
> 
> change log:
>         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 protcol enums
>         Fix cases of wrong indendation, ignoring return values, freeing
>         allocated resources in case of errors and other style suggestions
>         in v1 review.
> 
> Vaishnav M A (3):
>   add mikrobus descriptors to greybus_manifest
>   mikroBUS driver for add-on boards on mikrobus ports
>   Add Device Tree Bindings for mikroBUS port
> 
>  .../bindings/misc/linux,mikrobus.txt          |  81 ++
>  MAINTAINERS                                   |   6 +
>  drivers/misc/Kconfig                          |   1 +
>  drivers/misc/Makefile                         |   1 +
>  drivers/misc/mikrobus/Kconfig                 |  16 +
>  drivers/misc/mikrobus/Makefile                |   7 +
>  drivers/misc/mikrobus/mikrobus_core.c         | 692 ++++++++++++++++++
>  drivers/misc/mikrobus/mikrobus_core.h         | 191 +++++
>  drivers/misc/mikrobus/mikrobus_manifest.c     | 444 +++++++++++
>  drivers/misc/mikrobus/mikrobus_manifest.h     |  21 +
>  drivers/misc/mikrobus/mikrobus_port.c         | 171 +++++
>  include/linux/greybus/greybus_manifest.h      |  47 ++
>  12 files changed, 1678 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/linux,mikrobus.txt
>  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
>  create mode 100644 drivers/misc/mikrobus/mikrobus_port.c
> 

Re: [RFC PATCH v2 0/3] mikroBUS driver for add-on boards

[Vaishnav M A]

On Wed, Aug 19, 2020 at 2:08 AM Frank Rowand <frowand.list@gmail.com> wrote:
>
> Hi Vaishnav,
>
> +me +devicetree
>
> Please add these two recipients to future versions.
>
> I will comment more after reading the first version and v2.
>
> -Frank
>
Hi Frank,

Sorry, I missed to run get_maintainer.pl after making the changes
will add both recipients in future versions.

Thanks and Regards,
Vaishnav M A
>
> On 2020-08-18 07:48, Vaishnav M A wrote:
> > Hi,
> >
> > This Patch series is an update to the mikroBUS driver
> > RFC v1 Patch : https://lkml.org/lkml/2020/7/24/518 .
> > The mikrobus driver is updated to add mikrobus ports from device-tree
> > overlays, the debug interfaces for adding mikrobus ports through sysFS
> > is removed, and the driver considers the extended usage of mikrobus
> > port pins from their standard purposes.
> >
> > change log:
> >         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 protcol enums
> >         Fix cases of wrong indendation, ignoring return values, freeing
> >         allocated resources in case of errors and other style suggestions
> >         in v1 review.
> >
> > Vaishnav M A (3):
> >   add mikrobus descriptors to greybus_manifest
> >   mikroBUS driver for add-on boards on mikrobus ports
> >   Add Device Tree Bindings for mikroBUS port
> >
> >  .../bindings/misc/linux,mikrobus.txt          |  81 ++
> >  MAINTAINERS                                   |   6 +
> >  drivers/misc/Kconfig                          |   1 +
> >  drivers/misc/Makefile                         |   1 +
> >  drivers/misc/mikrobus/Kconfig                 |  16 +
> >  drivers/misc/mikrobus/Makefile                |   7 +
> >  drivers/misc/mikrobus/mikrobus_core.c         | 692 ++++++++++++++++++
> >  drivers/misc/mikrobus/mikrobus_core.h         | 191 +++++
> >  drivers/misc/mikrobus/mikrobus_manifest.c     | 444 +++++++++++
> >  drivers/misc/mikrobus/mikrobus_manifest.h     |  21 +
> >  drivers/misc/mikrobus/mikrobus_port.c         | 171 +++++
> >  include/linux/greybus/greybus_manifest.h      |  47 ++
> >  12 files changed, 1678 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/linux,mikrobus.txt
> >  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
> >  create mode 100644 drivers/misc/mikrobus/mikrobus_port.c
> >
>

Re: [RFC PATCH v2 0/3] mikroBUS driver for add-on boards

[Frank Rowand]

Hi Vaishnav,

Apologies in advance -- I expect to be very slow in responding this
week.  Linux Plumbers will take some of my time and I am moving to
a new home.

-Frank


On 2020-08-18 15:38, Frank Rowand wrote:
> Hi Vaishnav,
> 
> +me +devicetree
> 
> Please add these two recipients to future versions.
> 
> I will comment more after reading the first version and v2.
> 
> -Frank
> 
> 
> On 2020-08-18 07:48, Vaishnav M A wrote:
>> Hi,
>>
>> This Patch series is an update to the mikroBUS driver
>> RFC v1 Patch : https://lkml.org/lkml/2020/7/24/518 .
>> The mikrobus driver is updated to add mikrobus ports from device-tree
>> overlays, the debug interfaces for adding mikrobus ports through sysFS
>> is removed, and the driver considers the extended usage of mikrobus
>> port pins from their standard purposes.
>>
>> change log:
>>         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 protcol enums
>>         Fix cases of wrong indendation, ignoring return values, freeing
>>         allocated resources in case of errors and other style suggestions
>>         in v1 review.
>>
>> Vaishnav M A (3):
>>   add mikrobus descriptors to greybus_manifest
>>   mikroBUS driver for add-on boards on mikrobus ports
>>   Add Device Tree Bindings for mikroBUS port
>>
>>  .../bindings/misc/linux,mikrobus.txt          |  81 ++
>>  MAINTAINERS                                   |   6 +
>>  drivers/misc/Kconfig                          |   1 +
>>  drivers/misc/Makefile                         |   1 +
>>  drivers/misc/mikrobus/Kconfig                 |  16 +
>>  drivers/misc/mikrobus/Makefile                |   7 +
>>  drivers/misc/mikrobus/mikrobus_core.c         | 692 ++++++++++++++++++
>>  drivers/misc/mikrobus/mikrobus_core.h         | 191 +++++
>>  drivers/misc/mikrobus/mikrobus_manifest.c     | 444 +++++++++++
>>  drivers/misc/mikrobus/mikrobus_manifest.h     |  21 +
>>  drivers/misc/mikrobus/mikrobus_port.c         | 171 +++++
>>  include/linux/greybus/greybus_manifest.h      |  47 ++
>>  12 files changed, 1678 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/misc/linux,mikrobus.txt
>>  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
>>  create mode 100644 drivers/misc/mikrobus/mikrobus_port.c
>>
> 

Re: [RFC PATCH v2 1/3] add mikrobus descriptors to greybus_manifest

[Vaishnav M A]

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 <vaishnav@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 <vaishnav@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

Re: [RFC PATCH v2 1/3] add mikrobus descriptors to greybus_manifest

[Zoran Stojsavljevic]

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 <vaishnav@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 <vaishnav@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 <vaishnav@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

Re: [RFC PATCH v2 1/3] add mikrobus descriptors to greybus_manifest

[Vaishnav M A]

Hi Zoran,

Thank you for your review,
On Mon, Aug 31, 2020 at 11:56 AM Zoran Stojsavljevic
<zoran.stojsavljevic@gmail.com> wrote:
>
> 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.
>
Sure, we can split up the mikrobus driver patch into two parts and
still ensure that each patch builds without errors, will fix this in the
next version.
> My two cent worth comment,
> Zoran
> _______
>
> On Thu, Aug 20, 2020 at 2:49 AM Vaishnav M A <vaishnav@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 <vaishnav@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 <vaishnav@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
Thanks and Regards,
Vaishnav