Joule shield with ADC detection

[Propanu]

Description pretty much sums it up. If you have a Grove or DFRobot Shield for Joule and are running the latest kernel, this will detect it and load the appropriate IIO driver, configure it, and finally, add the AIO pins to the pinmap.

A couple of other minor changes:

• reordered I2C buses
• added UART buses + device paths

[arfoll]

It's absolutely what our internal i2-dev header is for and pretty sure it contains all you need see https://github.com/intel-iot-devkit/mraa/blob/master/include/linux/i2c-dev.h

On 14 Mar 2017 04:13, "Mihai Tudor Panu" [email protected] wrote:

@Propanu commented on this pull request.

In src/x86/intel_joule_expansion.c https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105819846:

#include "common.h" #include "x86/intel_joule_expansion.h"

#define PLATFORM_NAME "INTEL JOULE EXPANSION"

+typedef union i2c_smbus_data_union {

• uint8_t byte; ///< data byte
• unsigned short word; ///< data short word
• uint8_t block[I2C_SMBUS_BLOCK_MAX + 2];
• ///< block[0] is used for length and one more for PEC +} i2c_smbus_data_t;

+typedef struct i2c_smbus_ioctl_data_struct {

• uint8_t read_write; ///< operation direction
• uint8_t command; ///< ioctl command
• int size; ///< data size
• i2c_smbus_data_t* data; ///< data +} i2c_smbus_ioctl_data_t;

We probably don't, I'll see if it can be removed with the proper includes added.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105819846, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfZZsvwIfvNfEP1-G0LDjpCnvLUHoeEks5rlgXegaJpZM4MY1cz .

[arfoll]

Sure but we can use platform version for this and then expose extra info to users

On 14 Mar 2017 04:15, "Mihai Tudor Panu" [email protected] wrote:

@Propanu commented on this pull request.

In src/x86/intel_joule_expansion.c https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105819984:

#include "common.h" #include "x86/intel_joule_expansion.h"

#define PLATFORM_NAME "INTEL JOULE EXPANSION"

+typedef union i2c_smbus_data_union {

• uint8_t byte; ///< data byte
• unsigned short word; ///< data short word
• uint8_t block[I2C_SMBUS_BLOCK_MAX + 2];
• ///< block[0] is used for length and one more for PEC +} i2c_smbus_data_t;

+typedef struct i2c_smbus_ioctl_data_struct {

• uint8_t read_write; ///< operation direction
• uint8_t command; ///< ioctl command
• int size; ///< data size
• i2c_smbus_data_t* data; ///< data +} i2c_smbus_ioctl_data_t;

+typedef enum {NO_SHIELD, DFROBOT, GROVE} shield_t;

Not sure I follow here. This is only used internally to keep track of how many ADC channels we have.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105819984, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfZZgf6h5-VUzKadFPzAnqeqx_pkLYgks5rlgZMgaJpZM4MY1cz .

[arfoll]

Agreed it's not great, but we can just make that iio device string platform mappable and then use it. That shouldn't be a big fix and remove a load of code from this...

On 14 Mar 2017 04:24, "Mihai Tudor Panu" [email protected] wrote:

@Propanu commented on this pull request.

In src/x86/intel_joule_expansion.c https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105820837:

+typedef struct i2c_smbus_ioctl_data_struct {

• uint8_t read_write; ///< operation direction
• uint8_t command; ///< ioctl command
• int size; ///< data size
• i2c_smbus_data_t* data; ///< data +} i2c_smbus_ioctl_data_t;

+typedef enum {NO_SHIELD, DFROBOT, GROVE} shield_t; + +static mraa_result_t +mraa_joule_expansion_board_get_valid_fp(mraa_aio_context dev) +{

• char file_path[64] = "";
• // 4 channels per ADC
• snprintf(file_path, 64, "/sys/bus/iio/devices/iio:device%d/in_voltage%d_raw", dev->channel / 4, dev->channel % 4);

The aio module doesn't handle multiple devices. In fact, it assumes iio:device0 is the ADC. Proper way would be to actually query all the IIO devices and figure out which ones are the ADCs. That's a pretty substantial "fix".

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105820837, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfZZlyd9HX3Z1sY4iW4Ar01ALOXTYNRks5rlgh9gaJpZM4MY1cz .

[arfoll]

Uhm yes not sure what i saw there either my bad :)

On 14 Mar 2017 04:29, "Mihai Tudor Panu" [email protected] wrote:

@Propanu commented on this pull request.

In src/x86/intel_joule_expansion.c https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105821489:

 b->pwm_default_period = 5000;

 b->pwm_max_period = 218453;
 b->pwm_min_period = 1;

 b->i2c_bus_count = 0;

• int i2c_bus_num = -1;
• i2c_bus_num = mraa_find_i2c_bus_pci("0000:00", "0000:00:16.0", "i2c_designware.0");

• //int i2c_bus_num = -1;
• int i2c_bus_num = mraa_find_i2c_bus_pci("0000:00", "0000:00:16.0", "i2c_designware.0");

I'm a bit lost here, are you thinking about i2c_bus_count?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105821489, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfZZuzjKzOMhAN_hgKlfCzLkcXOrqkbks5rlgmngaJpZM4MY1cz .

[arfoll]

We have lots of platform defs that use the gpio raw API i see no difference with that and the i2c API. Please use it

On 14 Mar 2017 04:39, "Mihai Tudor Panu" [email protected] wrote:

@Propanu commented on this pull request.

In src/x86/intel_joule_expansion.c https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105822199:

•    syslog(LOG_NOTICE, "Attempting shield autodetection on I2C bus %d...", i2c_aio_bus);
  

•    i2c_smbus_ioctl_data_t sm;
  

•    i2c_smbus_data_t d;
  

•    int fp;
  

•    char i2c_fh[64];
  

•    snprintf(i2c_fh, 64, "/dev/i2c-%d", i2c_aio_bus);
  

•    sm.read_write = I2C_SMBUS_READ;
  

•    sm.command = 0x01;
  

•    sm.size = I2C_SMBUS_WORD_DATA; // LSB first
  

•    sm.data = &d;
  

•    if ((fp = open(i2c_fh, O_RDWR)) < 1)
  

I2C raw would do exactly what's needed, however there isn't a single board definition that relies on the API headers. If that's something you encourage, I see no problem using the raw functions instead. It would also get rid of the smbus data structs since they would be pulled from MRAA's i2c.h...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/intel-iot-devkit/mraa/pull/667#discussion_r105822199, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfZZl8djrr3hYNU8DCwy-XnZL1u3ohwks5rlgvkgaJpZM4MY1cz .

[alext-mkrs]

I also wonder if sub-platform functionality would be a better fit for this, rather then packing the shield-related code into base expansion board file? Or maybe a copy of this file, with this additional logic, could be treated as a separate platform...

[Propanu]

Cleaned up and resolved most of the suggestions. Additional change:

• Enabled use of mraa_i2c_init_raw() during platform detection.

I think the mraa_joule_expansion_board_get_valid_fp() replacement is valid for now, but can be improved by expanding on the new aio_dev struct that was recently added. Will spend some time on that, since the ADC class needs more flexibility.

On the other hand, loading and exporting drivers would be better off in the iio class as a new feature, but I doubt I'll find any time for that soon.

@arfoll & @alext-mkrs what are your thoughts?