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
Expand Up @@ -67,6 +67,7 @@ struct ad9172_state {
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 @@ static int ad9172_parse_dt(struct spi_device *spi, struct ad9172_state *st)
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 Down Expand Up @@ -1001,6 +1005,15 @@ static int ad9172_jesd204_link_enable(struct jesd204_dev *jdev,

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 @@ static int ad9172_jesd204_link_running(struct jesd204_dev *jdev,
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 @@ static int ad9172_jesd204_link_running(struct jesd204_dev *jdev,
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 @@ int ad917x_jesd_get_cfg_param(ad917x_handle_t *h,
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
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