Skip to content

iio: frequency: ad9172: implement sysref oneshot sequence #2906

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

iio: frequency: ad9172: implement sysref oneshot sequence #2906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
643046a
bindings: iio: frequency: adi,ad9172: sysref-error-window
vai-tomme Nov 23, 2023
063e015
iio: frequency: ad9172: implement sysref oneshot sequence
vai-tomme Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Documentation/devicetree/bindings/iio/frequency/adi,ad9172.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ Optional properties:
SYNCOUTB signal, default is CMOS.
- adi,sysref-coupling-dc-enable: Enables SYSREF signal DC coupling,
default is AC coupling.
- adi,sysref-error-window: Amount of jitter allowed on the sysref
input pins. Unit is in DAC clock cycles.
- reset-gpio: a GPIO spec for the reset pin.
- txen0-gpio: a GPIO spec for the TXEN0 pin.
- txen1-gpio: a GPIO spec for the TXEN1 pin.
Expand Down
28 changes: 28 additions & 0 deletions drivers/iio/frequency/ad9172.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// SPDX-License-Identifier: GPL-2.0
/*
* AD9172 SPI DAC driver for AXI DDS PCORE/COREFPGA Module
Expand Down Expand Up @@ -67,6 +67,7 @@
u32 clock_output_config;
u32 scrambling;
u32 sysref_mode;
u32 sysref_err_win;
bool pll_bypass;
signal_type_t syncoutb_type;
signal_coupling_t sysref_coupling;
Expand Down Expand Up @@ -928,6 +929,9 @@
if (of_property_read_u32(np, "adi,sysref-mode", &st->sysref_mode))
st->sysref_mode = SYSREF_CONT;

st->sysref_err_win = 0;
of_property_read_u32(np, "adi,sysref-error-window", &st->sysref_err_win);

/*Logic lane configuration*/
ret = of_property_read_u8_array(np,"adi,logic-lanes-mapping",
st->logic_lanes, sizeof(st->logic_lanes));
Expand All @@ -942,7 +946,7 @@
enum jesd204_state_op_reason reason,
struct jesd204_link *lnk)
{
struct device *dev = jesd204_dev_to_device(jdev);

Check warning on line 949 in drivers/iio/frequency/ad9172.c

View workflow job for this annotation

GitHub Actions / build_llvm_x86_64 / build 

clang_analyzer: Value stored to 'dev' during its initialization is never read [deadcode.DeadStores]
949 | struct device *dev = jesd204_dev_to_device(jdev);
| ^~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
struct ad9172_jesd204_priv *priv = jesd204_dev_priv(jdev);
struct ad9172_state *st = priv->st;
u64 rate;
Expand Down Expand Up @@ -1001,6 +1005,15 @@

ad917x_jesd_set_sysref_enable(&st->dac_h, !!st->jesd_subclass);

if (lnk->subclass == JESD204_SUBCLASS_1 &&
st->sysref_mode == SYSREF_ONESHOT) {
ret = ad917x_jesd_oneshot_sync(&st->dac_h, st->sysref_err_win);
if (ret) {
dev_err(dev, "Failed to set oneshot sync (%d)\n", ret);
return ret;
}
}

/*Enable Link*/
ret = ad917x_jesd_enable_link(&st->dac_h, JESD_LINK_ALL,
reason == JESD204_STATE_OP_REASON_INIT);
Expand All @@ -1021,6 +1034,7 @@
struct ad9172_state *st = priv->st;
unsigned long lane_rate_khz;
int ret;
bool done;

if (reason != JESD204_STATE_OP_REASON_INIT)
return JESD204_STATE_CHANGE_DONE;
Expand All @@ -1040,6 +1054,20 @@
return ret;
}

if (lnk->subclass == JESD204_SUBCLASS_1 &&
st->sysref_mode == SYSREF_ONESHOT) {
ret = ad917x_jesd_get_sync_rotation_done(&st->dac_h, &done);
if (ret) {
dev_err(dev, "Failed sync rotation read (%d)\n", ret);
return ret;
}

if (!done) {
dev_err(dev, "JESD204 sync rotation check failed\n");
return JESD204_STATE_CHANGE_ERROR;
}
}

return JESD204_STATE_CHANGE_DONE;
}

Expand Down
21 changes: 21 additions & 0 deletions drivers/iio/frequency/ad917x/AD917x.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,27 @@ int ad917x_jesd_set_sysref_enable(ad917x_handle_t *h, uint8_t en);
*/
int ad917x_jesd_get_sysref_enable(ad917x_handle_t *h, uint8_t *en);

/**
* \brief Configure SYSREF for oneshot sync
*
* Configure SYSREF oneshot sequence to align LMFC on next SYSREF
* rising edge signal.
*
* \param h Pointer to the AD917X device reference handle.
* \param err_window Error window in DAC clock cycles for SYSREF
* jitter.
*/
int ad917x_jesd_oneshot_sync(ad917x_handle_t *h, u8 err_window);

/**
* \brief Check if sync rotation has happened
*
* Checks if SYSREF to LMFC synchronization logic has completed.
*
* \param h Pointer to the AD917X device reference handle.
* \param *done Return value indicating if rotation has completed.
*/
int ad917x_jesd_get_sync_rotation_done(ad917x_handle_t *h, bool *done);

/**
* \brief Set the LMFC Delay and Variance for the JESD Links
Expand Down
45 changes: 45 additions & 0 deletions drivers/iio/frequency/ad917x/ad917x_jesd_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,51 @@
return API_ERROR_OK;
}

int ad917x_jesd_oneshot_sync(ad917x_handle_t *h, u8 err_window)
{
int err;
u8 mode;

if (!h)
return API_ERROR_INVALID_HANDLE_PTR;

err = ad917x_register_write(h, AD917X_SYSREF_ERR_WINDOW_REG,
AD917X_SYSREF_ERR_WIN(err_window));
if (err)
return err;

err = ad917x_register_read(h, AD917X_SYSREF_MODE_REG, &mode);
if (err)
return err;

if (mode & AD917X_SYSREF_MODE_ONESHOT) {
err = ad917x_register_write(h, AD917X_SYSREF_MODE_REG,
mode & ~AD917X_SYSREF_MODE_ONESHOT);
if (err)
return err;
}

return ad917x_register_write(h, AD917X_SYSREF_MODE_REG,
mode | AD917X_SYSREF_MODE_ONESHOT);
}

int ad917x_jesd_get_sync_rotation_done(ad917x_handle_t *h, bool *done)
{
int err;
u8 mode;

if (!h)
return API_ERROR_INVALID_HANDLE_PTR;

err = ad917x_register_read(h, AD917X_SYSREF_MODE_REG, &mode);
if (err)
return err;

*done = (mode & AD917X_SYNC_ROTATION_DONE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure if having *done brings much value. Why not just

if (!(mode & AD917X_SYNC_ROTATION_DONE))
    return API_SOME_ERROR;

return API_ERROR_OK; //or 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does provide chance to differentiate between error handling read request and the actual done value.
Similar approach is used also in other locations in this driver, see e.g. ad917x_jesd_get_sysref_enable.
I propose keeping done value and following current approach.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I still don't see the point tbh. Put the logs inside the API and you have the same effect. That's pretty much the reason you have for having done as an output parameter. To print an error log, nothing else.

Having said the above, I don't really have strong feelings it so up to you.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm ok, since we this API it would not be straight to have the logs inside. Does not make it better though :)


return 0;
}

int ad917x_jesd_set_sysref_enable(ad917x_handle_t *h, uint8_t en)
{
int err;
Expand Down Expand Up @@ -385,7 +430,7 @@
err = ad917x_register_write(h,
AD917X_MASTER_PD_REG, AD917X_SERDES_PDN(0));
if (err != API_ERROR_OK)
return err;;

Check warning on line 433 in drivers/iio/frequency/ad917x/ad917x_jesd_api.c

View workflow job for this annotation

GitHub Actions / checks / checks 

coccicheck: 14-15  Unneeded semicolon

/*Config SERDES PLL with ADI RECOMMENDED Settings*/
err = ad917x_register_write_tbl(h, &ADI_REC_ES_SERDES_INIT_TBL_2[0],
Expand Down Expand Up @@ -417,7 +462,7 @@
err = ad917x_register_write(h,
AD917X_MASTER_PD_REG, AD917X_SERDES_PDN(1));
if (err != API_ERROR_OK)
return err;;

Check warning on line 465 in drivers/iio/frequency/ad917x/ad917x_jesd_api.c

View workflow job for this annotation

GitHub Actions / checks / checks 

coccicheck: 14-15  Unneeded semicolon
/*Power Down ALL Lanes*/
err = ad917x_register_write(h, AD917X_PHY_PD_REG, 0xFF);
if (err != API_ERROR_OK)
Expand Down
8 changes: 8 additions & 0 deletions drivers/iio/frequency/ad917x/ad917x_reg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ int ad917x_register_read_block(ad917x_handle_t *h,
#define AD917X_MAINDAC_PAGE_0 BIT(6)
#define AD917X_MAINDAC_PAGE_1 BIT(7)

#define AD917X_SYSREF_ERR_WINDOW_REG 0x039
#define AD917X_SYSREF_ERR_WIN(x) (((x) & 0x7F) << 0)

#define AD917X_SYSREF_MODE_REG 0x03A
#define AD917X_SYNC_ROTATION_DONE BIT(4)
#define AD917X_SYSREF_MODE_ONESHOT BIT(1)
#define AD917X_SYSREF_MODE_CONTINUOUS BIT(0)

#define AD917X_SYSREF_ROTATION_REG 0x03B
#define AD917X_SYNC_LOGIC_EN BIT(7)
#define AD917X_SYNC_RSV_EN BIT(6)
Expand Down
Loading