ad9088: Series of fixes to enable ADI_APOLLO_NCO_CHAN_SEL_DIRECT_GPIO - #3466
ad9088: Series of fixes to enable ADI_APOLLO_NCO_CHAN_SEL_DIRECT_GPIO#3466gastmaier wants to merge 18 commits into
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
d16dde8 to
8a3c3de
Compare
4f4e19e to
351631e
Compare
|
V2:
Changes:
Squashed minor clang finding + llm fixup:
API:
|
This comment was marked as resolved.
This comment was marked as resolved.
351631e to
dc5fa3e
Compare
The device_property_count_u32() length was being overwritten by device_property_read_u32_array() return value. Add auxiliary len variable. Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
The val is unsigned, negative value never occur. Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
GPIO quick configuration profile to apply. Explicit GPIO mappings override the quick configuration mapped pins. Example usage: adi,gpio-quick-config = <ADI_APOLLO_QUICK_CFG_PROFILE_2>; Signed-off-by: Jorge Marques <jorge.marques@analog.com>
The AD9084 has 8 GPIO profiles predefined, expose this functionality by reading the devicetree property 'adi,gpio-quick-config'. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
The index is type uint8_t while IDX_NONE takes value -1, causing the check to never execute. Cast to same type to force into value 255. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
The ad9088_read_gpio_hop_array() was returning the result of device_property_read_u32_array() that is not the number of read elements on success. Fix by storing the minimum between the number of elements in the devicetree and the requested. Fewer elements in the devicetree means setting the remaining elements as ADI_APOLLO_GPIO_HOP_IDX_NONE (-1). In some systems, not all gpios can be fully routed. Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
The define values for ADI_APOLLO_CNCO_NUM, ADI_APOLLO_FNCO_NUM are already across both side for 8T8R, but multipled by ADI_APOLLO_NUM_SIDES again, doubling the allocated space. The mode is also fixed to be the number of controllers, not the number of hop profiles. For field frequency, extend by multiplying by number of controllers, since in the following commits will allow to configure the frequencies per profile per controller, instead of limiting one set of frequencies for all controllers. Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
FFH_FNCO_FREQUENCY, FFH_FNCO_SELECT and FFH_FNCO_MODE computed the hop-enable mask for a single FNCO controller with GENMASK(fnco_num + 1, fnco_num), which set 2 neighbors controllers instead of 1. The CNCO was already using BIT correctly. Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
FFH_FNCO_FREQUENCY restores the FNCO hop-enable state after loading a new frequency into a profile slot by reading phy->ffh.dir[dir].fnco.en[index], but index is the hop-profile index (0-31) just written to, while en[] is indexed by controller number. Use the already computed fnco_num instead. Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
phy->ffh.dir[dir].fnco.select[] is stored incremented by 1, with 0 meaning "not selected". For printing back, cast to int to yield -1 as "not selected". Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
adi_apollo_fnco_hop_pgm() misused ADI_APOLLO_CNCO_ALL instead of ADI_APOLLO_FNCO_ALL. Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
The FFH state arrays are sized by the number of controllers, but were indexed with the per-side map->fddc_num/cddc_num, aliasing side A onto side B. Add ad9088_ffh_fnco_num()/ad9088_ffh_cnco_num() to compute the absolute controller number. Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
fnco_hop_config and cnco_hop_config were partially assigned, so adi_apollo_fnco_hop_pgm() and adi_apollo_cnco_hop_enable(), remaining fields were undefined. Hop on frequency only, auto hop incrementing up to the last profile, and let the CNCO load the initial profile to start hopping from. Load the frequency with adi_apollo_fnco_chan_pgm(), which programs the channel NCO of a profile, instead of adi_apollo_fnco_profile_load(), and check the return of the hop enable restore. Fixes: 86f4acb ("iio: trx-rf: ad9088: Add initial FFH support") Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Switching a controller into a GPIO profile select mode requires handing the block select over to the GPIOs, but the block select is chip global, while the mode is per controller. Add ad9088_ffh_gpio_hop_enter()/exit() and only hand the block select back to SPI once the last FNCO or CNCO of either terminal left GPIO hop mode, tracked with ad9088_ffh_gpio_active(). Use adi_apollo_fnco_profile_sel_mode_set() to preserve per-controller GPIOSHARE bit and, since adi_apollo_fnco_hop_pgm() pulses HOP_CTRL_INIT, clearing the hop profiles. Drop the GPIO mask calculation from both select handlers, which only printed the masks and did not configure anything. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
At ad9088_ffh.c, update return paths to use ad9088_check_apollo_error() instead of returning a generic error code or message. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Set adi,gpio-quick-config to 1 (ADI_APOLLO_QUICK_CFG_PROFILE_1), already the default profile. Reconfigure Profile 1's profile_fcn_sel[3] from 31 to 15, since 31 is taken for aux_gpio (ADF4382_CE_LS). Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Set adi,gpio-quick-config to 1 (ADI_APOLLO_QUICK_CFG_PROFILE_1), already the default profile. Reconfigure Profile 1's profile_fcn_sel[3] from 31 to 15, since 31 is taken for aux_gpio (ADF4382_CE_LS). Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Fix parameter order, where buf_size was expected before timeout_us, but was being called as the last argument. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
dc5fa3e to
61e9c1d
Compare
| bool en[ADI_APOLLO_NUM_SIDES * ADI_APOLLO_FNCO_NUM]; | ||
| u8 mode[ADI_APOLLO_FNCO_PROFILE_NUM]; | ||
| u8 index[ADI_APOLLO_FNCO_NUM]; | ||
| u64 frequency[ADI_APOLLO_FNCO_NUM][ADI_APOLLO_FNCO_PROFILE_NUM]; |
There was a problem hiding this comment.
Hmm I guess this change makes the series non bisectable? So, ideally every commit should remain in a working state (of course to best of our knowledge) and compile. The working state sometimes is harder but being compile is a must.
So couple all the data changes with the code path that actually depend on it. If you think it still makes sense to have separate patches per field (index, frequency, etc...) fine. Or by logical change...
| ADI_APOLLO_NCO_PROFILE_PHASE_INCREMENT, | ||
| index, &ftw_u32, 1); | ||
| ret = ad9088_check_apollo_error(&phy->spi->dev, ret, | ||
| "adi_apollo_cnco_profile_load"); |
There was a problem hiding this comment.
The above handling is very annoying 😄. Not related to the current PR and just something I'm sharing if you ever want to change it. In navassa, I added this exactly to avoid handling like the above.
| const struct _ad9088_ffh *ffh; | ||
| u8 t, i; | ||
|
|
||
| for (t = 0; t < ARRAY_SIZE(phy->ffh.dir); t++) { |
There was a problem hiding this comment.
nowadays you can do for (u8 t = 0; ...) in the kernel
|
For this PR, there is a gpio collision issue with MCS, the state machine needs to incorporate 4e31fd0 @mhennerich is Apollo GPIO -> DELADJ/DELSTR configurable? does it have a devietree? How could we detect collisions?
:( https://analogdevicesinc.github.io/linux/drivers/iio-trx-rf/ad9088/mcs.html#hardware-topology |
Please see here: DELADJ/DELSTR are required beyond initial MCS, apollo constantly updates those in the background. If there are collisions with FFH, it should error out. |
PR Description
Resolved ai findings:
Tested with
PR Type
PR Checklist