analogdevicesinc / precision-converters-firmware

Precision Converters Embedded Firmware Repository
https://analogdevicesinc.github.io/precision-converters-firmware/
Apache License 2.0
13 stars 16 forks source link

Add support for AD2S1210 #2

Open pamolloy opened 1 year ago

pamolloy commented 1 year ago

Add support for the serial interface to the AD2S1210 using Mbed.

Tasks

Dependencies

ahaslam2 commented 1 year ago

Progress so far:

SDPK1 is not pin compatible with the eval board of 2s1210 Had to use the SDPK1 arduino pins for SPI but could not jump wires directly to the eval board because voltage domain is different.

so i used the breakout board connected to the eval 2s1210 and jumped wires to the arduino pins of the SDPK1 to get SPI comunication.

SPI MODE 3 of Mbed seems to be sampling on rising edge and shifing on falling edge? so had to use SPI mode 2 to make it work. This is contrary to the kernel diver and what spi modes should do.. need to investigate this a bit more.

the spi speed was reduced to 100/500khz.. i guess the limit comes from the jump wires im using, not sure what the max speed i can do is with this setup.. but in theory the chip should be able to do 20/15/10Mhz

Here is the ouput of the iio_info (reading the control register attribute default 0x7E)

axelh@axelh-s30:~$ iio_info -u serial:/dev/ttyACM1,115200,8n1
Library version: 0.24 (git tag: accb7b5)
Compiled with backends: local xml ip usb serial
IIO context created with serial backend.
Backend version: 1.1 (git tag: 0000000)
Backend description string: /dev/ttyACM1: CDC DEVICE - ad2s1210_iio_DEV_AD2S1210_SDP_K1
IIO context has 2 attributes:
    hw_carrier: SDP_K1
    uri: serial:/dev/ttyACM1,115200,8n1n
IIO context has 1 devices:
    iio:device0: ad2s1210 (buffer capable)
        2 channels found:
            angl0: angle (input, index: 0, format: le:U16/16>>0)
            anglvel0: angular_velocity (input, index: 1, format: le:U16/16>>0)
        5 device-specific attributes found:
                attr  0: fclkin value: 0
                attr  1: fexcit value: 0
                attr  2: control value: 0x7e
                attr  3: resolution value: 0
                attr  4: fault value: 0
        1 debug attributes found:
                debug attr  0: direct_reg_access value: 0
        No trigger on this device
ahaslam2 commented 1 year ago

Im able to read angle and velocity, and the values make sense:

iio_info:

Library version: 0.24 (git tag: accb7b5)
Compiled with backends: local xml ip usb serial
IIO context created with serial backend.
Backend version: 1.1 (git tag: 0000000)
Backend description string: /dev/ttyACM1: CDC DEVICE - ad2s1210_iio_DEV_AD2S1210_SDP_K1
IIO context has 4 attributes:
    hw_carrier: SDP_K1
    hw_mezzanine: SDP1Z Adaptor
    hw_name: SDP Adaptor Debug Board
    uri: serial:/dev/ttyACM1,115200,8n1n
IIO context has 1 devices:
    iio:device0: ad2s1210 (buffer capable)
        2 channels found:
            angl0: angle (input, index: 0, format: le:U16/16>>0)
            3 channel-specific attributes found:
                attr  0: raw value: 26767
                attr  1: scale value:   0.100000
                attr  2: offset value: 0
            anglvel1: angular_velocity (input, index: 1, format: le:U16/16>>0)
            3 channel-specific attributes found:
                attr  0: raw value: 0
                attr  1: scale value:   0.100000
                attr  2: offset value: 0
        5 device-specific attributes found:
                attr  0: fclkin value: 0
                attr  1: fexcit value: 0
                attr  2: control value: 0x7e

                attr  3: resolution value: 0

                attr  4: fault value: 0x0

        1 debug attributes found:
                debug attr  0: direct_reg_access value: 48
        No trigger on this device

iio_attr

axelh@axelh-s30:~$ while true; do iio_attr -u serial:/dev/ttyACM1,115200,8n1  -c iio:device0 . raw;sleep 2 ;done
dev 'ad2s1210', channel 'angl0' (input), id 'angle', attr 'raw', value '27609'
dev 'ad2s1210', channel 'anglvel1' (input), id 'angular_velocity', attr 'raw', value '0'
dev 'ad2s1210', channel 'angl0' (input), id 'angle', attr 'raw', value '29713'
dev 'ad2s1210', channel 'anglvel1' (input), id 'angular_velocity', attr 'raw', value '5'
dev 'ad2s1210', channel 'angl0' (input), id 'angle', attr 'raw', value '29716'
dev 'ad2s1210', channel 'anglvel1' (input), id 'angular_velocity', attr 'raw', value '25'
dev 'ad2s1210', channel 'angl0' (input), id 'angle', attr 'raw', value '5807'
dev 'ad2s1210', channel 'anglvel1' (input), id 'angular_velocity', attr 'raw', value '11'
dev 'ad2s1210', channel 'angl0' (input), id 'angle', attr 'raw', value '8402
ahaslam2 commented 1 year ago

first sign of resolver working on the osc app: image

mhennerich commented 1 year ago

not sure how to use the debug attribute register read/write with iio_attr

Please see here: https://wiki.analog.com/software/linux/docs/iio/iio_snippets#low_level_register_access_via_debugfs_direct_reg_access

Well you would use iio_reg for that.

ahaslam2 commented 1 year ago

Hi @mhennerich @mhennerich

iio_reg is working as expected.. thanks. Only issue is that on dumping iio_info, it tries to read address 0x0 which actually puts the chip on bad state (need to power cycle to recover) i had to add a check to return invalid in case of reading a bad address.

I made some more cleanups and testing. I was able in fact to read/write SPI at 20Mhz and do 16k sampling rate. Not sure what was going on before that i had to reduce the speed. but seems to be working OK now. The osc ouput is looking nice when removing auto scale and setting reasonable scale.

Im getting ready to submit the PR have some questions before pushing:

  1. should i add the no-os driver in a new "resolver" directory, or should it be included as part of "adc"

  2. I removed the i2c and eeprom related code as im not reading the ID on the eeprom (im using the pinnout board) is this ok?

  3. Im able to set 16000 sampling rate, i see the 16Khz pwm signal on arduino D3. setting 24000 sampling rate results in timeouts on the osc app. is 16k sampling rate good enough? the pwm output triggers a gpio irq there is a hw loopback from pwm to the gpio on the same pin is this correct?. sw wise: in ad738x why some things are initialized in init_system (app_config.c) like gpio_trigger_Init but others in iio_initialize (ad738x_iio.c) like init_pwm_trigger?

  4. Can i choose/modify the API between the pcf app and the no-os driver. for exmple: ad738x on a single_conversion gets two 16 bit values, one for each channel. perhaps for the 2s1210 i can add a parameter to this function call and only return one 16 bit value corresponding to 1 channel. so that i dont need to toggle the A0/A1 gpios and get read data that is not needed in case only one channel is enabled?

  5. Do "scale" and "offset" channel attributes make sense for the 2s210 POS and VEL? do i need those on the pcf app? if yes any hints on what to set them too? seems to depend if the signal is bipolar or uniplar

  6. the OSC app is showing the position with values from +20k -40k, the raw value for position is from 0 to 0xFFFF. i would expect the osc app to show from 0 to 65k or from -32k to 32k. any idea why this -10k offset may happen? perhaps a plugin is needed for 2s1210 in the osc app? The position is unsigned while the angle is a signed value. is it left up to the final application to interpret this correctly and the pcf app only deals with raw values? does this have something to do with the "offset" attributes (Q#5)?

  7. what about the /tests and /script folders i see for example in ad717x_iio? do i need to worry about those?

  8. i guess i have to do a wiki page for 2s1210 to show how the hw setups was done. perhaps something like this: ? https://wiki.analog.com/resources/tools-software/product-support-software/AD2S1210_mbed_iio_support do i have the power to create one?

mhennerich commented 1 year ago
  1. should i add the no-os driver in a new "resolver" directory, or should it be included as part of "adc"

Please add in new resolver directory

  1. Do "scale" and "offset" channel attributes make sense for the 2s210 POS and VEL? do i need those on the pcf app? if yes any hints on what to set them too? seems to depend if the signal is bipolar or uniplar

The IIO support in No-OS must follow the Linux kernel IIO ABI. So first make sure the Linux driver uses the proper iio_chan_type for example: IIO_ANGL_VEL, IIO_ANGL I think they are correct.

https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/iio/types.h#L14

From: https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-iio

What: /sys/bus/iio/devices/iio:deviceX/in_angl_raw What: /sys/bus/iio/devices/iio:deviceX/in_anglY_raw KernelVersion: 4.17 Contact: linux-iio@vger.kernel.org Description: Angle of rotation. Units after application of scale and offset are radians.

What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw KernelVersion: 2.6.35 Contact: linux-iio@vger.kernel.org Description: Angular velocity about axis x, y or z (may be arbitrarily assigned). Has all the equivalent parameters as per voltageY. Units after application of scale and offset are radians per second.

Then scale and offset are used to convert the RAW value into these units. While handling resolution, etc. So if you want the user to change the resolution, he would pick a different scale. In this case scale is writable and there would be also a scale_available attribute.

  1. the OSC app is showing the position with values from +20k -40k,

How does the scan_element type look like for these channels? OSC typically just looks for the type and then displays the values in accordance with the type.

Adding @dbogdan and @mike-bradley as well.

ahaslam2 commented 1 year ago
  1. should i add the no-os driver in a new "resolver" directory, or should it be included as part of "adc"

Please add in new resolver directory

  1. Do "scale" and "offset" channel attributes make sense for the 2s210 POS and VEL? do i need those on the pcf app? if yes any hints on what to set them too? seems to depend if the signal is bipolar or uniplar

The IIO support in No-OS must follow the Linux kernel IIO ABI. So first make sure the Linux driver uses the proper iio_chan_type for example: IIO_ANGL_VEL, IIO_ANGL I think they are correct.

https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/iio/types.h#L14

From: https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-iio

What: /sys/bus/iio/devices/iio:deviceX/in_angl_raw What: /sys/bus/iio/devices/iio:deviceX/in_anglY_raw KernelVersion: 4.17 Contact: linux-iio@vger.kernel.org Description: Angle of rotation. Units after application of scale and offset are radians.

What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw KernelVersion: 2.6.35 Contact: linux-iio@vger.kernel.org Description: Angular velocity about axis x, y or z (may be arbitrarily assigned). Has all the equivalent parameters as per voltageY. Units after application of scale and offset are radians per second.

Then scale and offset are used to convert the RAW value into these units. While handling resolution, etc. So if you want the user to change the resolution, he would pick a different scale. In this case scale is writable and there would be also a scale_available attribute.

  1. the OSC app is showing the position with values from +20k -40k,

How does the scan_element type look like for these channels? OSC typically just looks for the type and then displays the values in accordance with the type.

Perhps its just better if i just the code for review: but i think these are the relevant bits:


#define AD2S1210_CH(_name, _idx, _type) {\
        .name = _name, \
        .ch_type = _type,\
        .ch_out = false,\
        .indexed = true,\
        .channel = _idx,\
        .scan_index = _idx,\
        .scan_type = &chn_scan,\
        .attributes = ad2s1210_iio_ch_attributes\
}

#define BYTES_PER_SAMPLE        2

/* Number of data storage bits (needed for IIO client to plot RESOLVER data) */
#define CHN_STORAGE_BITS        (BYTES_PER_SAMPLE * 8)

struct scan_type chn_scan = {
        .sign = 's',
        .realbits = CHN_STORAGE_BITS,
        .storagebits = CHN_STORAGE_BITS,
        .shift = 0,
        .is_big_endian = false
};

struct scan_type chn_scan = {
        .sign = 's',
        .realbits = CHN_STORAGE_BITS,
        .storagebits = CHN_STORAGE_BITS,
        .shift = 0,
        .is_big_endian = false
};

static struct iio_channel ad2s1210_iio_channels[RESOLVER_CHANNELS] = {
        AD2S1210_CH("ANGL", 0, IIO_ANGL),
        AD2S1210_CH("ANGL_VEL", 1, IIO_ANGL_VEL)
};

BTW i asked this question because it seems that the osc app is not taking into account the offset and scale attribute i set in the channel. its just seems to output the raw value.. is this normal? if not its perhaps some issue with my test.

Adding @dbogdan and @mike-bradley as well.

ahaslam2 commented 1 year ago

ad4130 _iio is doing what i was asking in question 4: (getting sample for a single channel) ret = ad413x_read_single_sample(ad4130_dev_inst, (uint8_t)channel->ch_num, &adc_data_raw);

ill do this and send the PR to discuss with the actual code.

ahaslam2 commented 1 year ago

Here are the pull request for the no-os and pcf code:

no-OS: https://github.com/analogdevicesinc/no-OS/pull/1906
PCF: https://github.com/analogdevicesinc/precision-converters-firmware/pull/24

I fixed comments on the no-os driver, i declared the scan value as big endian and removed a byte swap that i was doing on the driver. The swap needs to be done on the pcf app get raw attribute however for the iio_info iio_attr and DMM window of the osc app to report correct values.

mike-bradley commented 1 year ago

I removed the i2c and eeprom related code as im not reading the ID on the eeprom (im using the pinnout board) is this ok?

Yes it's fine to no read and check the EVB EEPROM contents. As the connections are fly wired, we don't want to the firmware to refuse to run if the user hasn't correctly made the I2C connections as well. If at some point in the future there is a new Arduino EVB (say) that doesn't need fly wires, we can look at adding the eeprom check back in, but not in scope here currently at least.

Im able to set 16000 sampling rate, i see the 16Khz pwm signal on arduino D3. setting 24000 sampling rate results in timeouts on the osc app. is 16k sampling rate good enough? the pwm output triggers a gpio irq there is a hw loopback from pwm to the gpio on the same pin is this correct?

16000 is fine for this part. the signal integrity due to the fly wires is likely the limiting factor for the SPI clock rate.

sw wise: in ad738x why some things are initialized in init_system (app_config.c) like gpio_trigger_Init but others in iio_initialize (ad738x_iio.c) like init_pwm_trigger?

@mphalke - can you comment here?

Can i choose/modify the API between the pcf app and the no-os driver. for exmple: ad738x on a single_conversion gets two 16 bit values, one for each channel. perhaps for the 2s1210 i can add a parameter to this function call and only return one 16 bit value corresponding to 1 channel. so that i dont need to toggle the A0/A1 gpios and get read data that is not needed in case only one channel is enabled?

If I follow the question, is the ask here in terms of providing AD2S1210 No-OS function calls for reading back 1 or more channels of data, e.g. position, velocity etc.... such that only channels enabled/request by the host are read back to save time transferring/acquiring the data, and potentially allow higher throughput on the enabled chanenls? If so that makes sense, some IIO devices do support and enable that sort of function, whereas others do not or can not. In some cases (AD7606x for example) the device always reads 8 channels and returns the 8 channels in order, so less benefit there.

what about the /tests and /script folders i see for example in ad717x_iio? do i need to worry about those? the scripts folder are really the pyadi-iio example code for those projects. That code is going to be moved at some point to the pyadi-iio repo, but it's not been prioritized yet on our side.

the tests folder contains the pytests that we run as part of ci testing of the firmware to check basic functionality. The tests to date tend to be simple go/no go tests, or DC inputs. We've not done performance level testing, but with the MX1002 available not, that is likely to change.

mphalke commented 1 year ago

sw wise: in ad738x why some things are initialized in init_system (app_config.c) like gpio_trigger_Init but others in iio_initialize (ad738x_iio.c) like init_pwm_trigger? [Mahesh >>] This is done because PWM init needs to happen after interrupt callback registration happens from the 'iio_hw_trig_init()' function in IIO library. IIO structure need us to perform interrupt init from app level and then callback registration is done from IIO layer. We have shared same PWM pin as interrupt pin as well and Mbed requires intrrupt init/callback registration to be done prior to PWM init in this case. This is issue with Mbed platform I guess.