Skip to content

axi_jesd204_rx: Fixup write to up_cfg_buffer_delay #1186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

gastmaier
Copy link
Contributor

Change up_cfg_buffer_delay assignment from up_wdata [7:0] to [9:2].

PR Description

Test axi_jesd204_rx_regmap_tb.v was failing at register x90 with output:

Address 00000240, Expected 000103fc, Found 000103f0

The issue is at fields Buffer release delay [9:2] and Data path width alignment [1:0], the latter is always 2'b00.
Investigating jesd204_up_rx.v, I noticed that up_cfg_buffer_delay is assigned up_wdata[7:0] instead of [9:2], incoherent with the read instruction.
I ran all tests at tb and now all succeeds (xsim, 2023.1).

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have followed the code style guidelines
  • I have performed a self-review of changes
  • I have compiled all hdl projects and libraries affected by this PR
  • I have tested in hardware affected projects, at least on relevant boards
  • I have commented my code, at least hard-to-understand parts
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe files, Copyright etc)
  • I have not introduced new Warnings/Critical Warnings on compilation
  • I have added new hdl testbenches or updated existing ones

@gastmaier
Copy link
Contributor Author

This change was introduced at #993 (direct link). looks like a mistake.
However there are more things in the regmap that I want to look into later, that can be added to this pr.

@gastmaier gastmaier marked this pull request as draft January 11, 2024 17:19
Change up_cfg_buffer_delay assignment from up_wdata [7:0] to [9:2].
At register x90, up_cfg_buffer_delay occupies [9:2],
but was being assigned up_wdata[7:0].
Due to this, tb/axi_jesd204_rx_regmap_tb was failing.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Change up_cfg_buffer_delay bits to 9:2.
Set 1:0 as RESERVED, since no driver implements it and is locked in 2'b00.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
@gastmaier gastmaier force-pushed the jesd204_rx_cfg_fixup branch from f20fea1 to 9f9f309 Compare July 23, 2024 09:16
@gastmaier
Copy link
Contributor Author

Looking into https://github.com/analogdevicesinc/linux/blob/main/drivers/iio/jesd204/axi_jesd204_rx.c#L53 (DWORD 90, BYTE 240)
Only bit 16 is writable with JESD204_RX_LINK_CONF2_BUFFER_EARLY_RELEASE at the subclass config.
Therefore, the up_cfg_buffer_delay (bits 9:2) are unused by the driver (same for no-OS.
On the HDL side, the write bits should match the read bits and the regmap
Updated the regmap to add the Data path width alignment as Reserved in the regmap (I followed the coding style of the current file to mitigate further changes).

Initially I expected to add more commits to this pr, but since it stayed untouched for so long, let's merge only this change.

@gastmaier gastmaier marked this pull request as ready for review July 23, 2024 09:18
@bluncan bluncan removed their request for review September 11, 2024 08:44
Copy link
Contributor

@IstvanZsSzekely IstvanZsSzekely left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, working

@gastmaier gastmaier merged commit 93e2fca into main Sep 11, 2024
0 of 3 checks passed
@gastmaier gastmaier deleted the jesd204_rx_cfg_fixup branch September 11, 2024 13:48
IstvanZsSzekely pushed a commit that referenced this pull request Jan 13, 2025
Change up_cfg_buffer_delay assignment from up_wdata [7:0] to [9:2].
At register x90, up_cfg_buffer_delay occupies [9:2],
but was being assigned up_wdata[7:0].
Due to this, tb/axi_jesd204_rx_regmap_tb was failing.
Set 1:0 as RESERVED, since no driver implements it and is locked in 2'b00.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants