iio: adc: adrv9025: fix hardwaregain attribute when working with dual-channel 4-pin mode. - #3525
Open
RaulGeo289 wants to merge 3 commits into
Open
RaulGeo289 wants to merge 3 commits into
RaulGeo289 wants to merge 3 commits into
Conversation
RaulGeo289
requested review from
CiprianRegus,
amiclaus,
btogorean,
buha,
chegbeli,
danmois,
dbogdan,
machschmitt,
mhennerich,
nunojsa,
rbolboac and
stefpopa
as code owners
September 7, 2026 09:36
RaulGeo289
force-pushed
the
staging/madura_adapt_to_dual_channel_mode
branch
3 times, most recently
from
September 7, 2026 10:43
edf5f70 to
14f6789
Compare
stefpopa
requested changes
Sep 11, 2026
stefpopa
left a comment
stefpopa
left a comment
Collaborator
There was a problem hiding this comment.
A few comments for this first round. The most important is splitting the commit.
| chan_no = CHAN_OBS_RX2; | ||
| } else { | ||
| ret = -EINVAL; | ||
| /* |
Collaborator
There was a problem hiding this comment.
All the HARDWAREGAIN read/write logic looks for resolving the ORx channel duplicated in both adrv9025_phy_read_raw and adrv9025_phy_write_raw. Consider extracting to a helper like:
static int adrv9025_resolve_obs_channel(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
int *out_chan_no)
| rxGain.gainIndex = code; | ||
| rxGain.rxChannelMask = 1 << chan_no; | ||
|
|
||
| rxGain.rxChannelMask = 1 << chan_no; |
Collaborator
There was a problem hiding this comment.
inconsistent spacing. Was this change necessary?
| @@ -1093,18 +1093,17 @@ | |||
| }; | |||
Collaborator
There was a problem hiding this comment.
please split this commit in 2, as it is pretty difficult to review:
Commit A: Introduce adrv9025_orx_get_mode() helper + convert existing adrv9025_orx_dual_4pin() callers to use the new switch pattern (no functional change)
Commit B: Fix HARDWAREGAIN read/write to use adrv9025_get_obs_rx_path() in 4-pin mode (the actual bug fix)
| @@ -1088,7 +1088,7 @@ static const char * const adrv9025_obs2_rx_port[] = { | |||
| "ORX3_ON_ORX4_OFF", "ORX3_OFF_ORX4_ON", | |||
Collaborator
There was a problem hiding this comment.
In the commit message, you forgot the space after the colon
Fixes:ce4db4d should be Fixes: ce4db4d
The driver only distinguished dual-channel 4-pin ORx mode from everything else, through adrv9025_orx_dual_4pin(), which is not considered a clean approach, in case there is the need for the other modes to be implemented. Replace it with adrv9025_orx_get_mode(), which returns the orxEnableMode cached from the init profile at probe, and convert every caller to switch on that enum: adrv9025_set_obs_rx_path(), adrv9025_get_obs_rx_path() and the IIO_CHAN_INFO_ENABLE paths of adrv9025_phy_read_raw() and adrv9025_phy_write_raw(). The IIO_CHAN_INFO_HARDWAREGAIN paths of both functions carried an identical copy of the code that resolves an OBS channel to the physical ORx of the pair. Move it into a single adrv9025_resolve_obs_channel() helper that switches on the same enum, so the resolution only has to be taught about a new mode once. The GPIO presence check moves into the dual-channel 4-pin case, which now returns -ENODEV when the channel-select pin is not wired, and unhandled modes return -EINVAL instead of falling through to the SPI path. Behaviour of the two modes the driver supports is otherwise unchanged. This is preparation for fixing the HARDWAREGAIN attribute in dual-channel 4-pin mode. Signed-off-by: Georgian Raul <Raul.Georgian@analog.com>
In dual-channel 4-pin ORx mode the observation channels are organised in pairs driven by four GPIO pins: ORX_CTRL A and ORX_CTRL C enable the first and the second pair, ORX_CTRL B selects between physical channels 1 and 2 and ORX_CTRL D between physical channels 3 and 4. Reading and writing the HARDWAREGAIN attribute still resolved the physical ORx channel from the 0x106 SPI enable bits, which are not used in this mode. The resolution therefore either picked the wrong channel or failed with -EINVAL, and the gain reads and writes did not take effect. Teach adrv9025_resolve_obs_channel() about the mode: read the channel-select pin back through adrv9025_get_obs_rx_path() and map it onto the physical ORx of the pair. Only the readback is used, because a gain access must never drive the select pin. Fixes: b0a20b8 ("iio: adc: adrv902x: Add dual-channel 4 pin mode crossbar and select fix") Signed-off-by: Georgian Raul <Raul.Georgian@analog.com>
Fix the naming for this LUT. Fixes: ce4db4d ("iio: adc: adrv902x: Add ORx support in the driver") Signed-off-by: Georgian Raul <Raul.Georgian@analog.com>
RaulGeo289
force-pushed
the
staging/madura_adapt_to_dual_channel_mode
branch
from
September 18, 2026 09:21
14f6789 to
7f6775b
Compare
Collaborator
Author
|
Changelog V1: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The PR fixes the writes and reads to the HARDWAREGAIN attribute,
caused by my commit from a previous PR. The commit has the Fixes tag
in its description.
This PR also does some cleanup, in regards to how the SPI and
dual-channel 4-pin mode are treated. I made a helper function that
returns the ORX enable mode, so in each function where i have a
different datapath for SPI mode and dual-channel 4-pin mode, i use a
switch, in case we want to add support for the other GPIO modes later
on.
I tried to split the commit into 2 commits, one for HARDWAREGAIN fix
and one for cleanup but they rely on each other. For example, the
hardwaregain commit uses the function adrv9025_orx_get_mode, but that
is used for cleanup as well.
PR Type
PR Checklist