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

vai-tomme · 2025-08-22T10:48:20Z

PR Description

Support for performing oneshot sequence as laid out in section "jesd204b serial data interface" and sync procedure of datasheet rev C [1] for device. In addition to the already implemented sysref_enable step, the other steps are also required for a sync to take place.

Feature is here implemented for subclass 1 operation with jesd-fsm enabled.

[1] https://www.analog.com/media/en/technical-documentation/data-sheets/ad9171.pdf

PR Type

Bug fix (a change that fixes an issue)
New feature (a change that adds new functionality)
Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

I have conducted a self-review of my own code changes
I have tested the changes on the relevant hardware
I have updated the documentation outside this repo accordingly (if there is the case)

Testing

Tested with zcu102 + ad9171-fmc-ebz combination and branch xilinx-v2023.2 with these patches applied.
For testing, custom built hdl with jesd mode 3 and configured devicetree accordingly since prebuilt hdl and dt for ad9171 eval board is not readily available.

devicetree settings for zynqmp-zcu102-rev10-ad9171-fmc-ebz-mode3.dts

&dac0_ad9172 {
        adi,dac-rate-khz = <4423680>; /* 4.4 GSPS */
        adi,jesd-link-mode = <3>;

        adi,jesd-subclass = <1>;
        adi,dac-interpolation = <6>;
        adi,channel-interpolation = <2>;
        adi,clock-output-divider = <4>;
        adi,syncoutb-signal-type-lvds-enable;
        adi,sysref-mode = <1>; /*  oneshot sysref mode */ 
        adi,sysref-error-window = <4>;
};

Add devicetree property for setting allowed sysref jitter. Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>

gastmaier · 2025-08-28T09:24:18Z

Hi @vai-tomme, thank you for the contribution! Please ignore the modpost error, was fixed on #2920 (1)!
But please go in the Files changed tab and apply the CI commented suggestion for your touched lines.
Minor kernel style stuff, using u* instead of uint*_t, direct null check if (!var)

(1) a cleaner run can be then created by either closing and opening the pr, or pushing a commit, to update the base branch head.

vai-tomme · 2025-08-29T08:34:29Z

Hi @gastmaier, thanks! I updated one occurance of uint8 to u8, but I kept the h == NULL style since that is the style used througout the file for that particular driver.
Please let me know if you still prefer me to update the null checks to the more direct variant.

vai-tomme · 2025-09-18T08:01:46Z

Update description with further details about testing. Please let me know if more information or any changes still required

gastmaier

Please just do kernel style null and error checking where you can

drivers/iio/frequency/ad917x/ad917x_jesd_api.c

gastmaier · 2025-09-24T12:56:04Z

drivers/iio/frequency/ad9172.c


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

+	if (lnk->subclass == JESD204_SUBCLASS_1) {


I was going to suggest to add a devicetree entry to oneshot_sync, but I see that for ad9081 it is done as well

gastmaier · 2025-09-24T12:56:45Z

drivers/iio/frequency/ad9172.c


+	if (lnk->subclass == JESD204_SUBCLASS_1) {
+		ret = ad917x_jesd_oneshot_sync(&st->dac_h, st->sysref_err_win);
+		if (ret != 0) {


if (!ret)

updated as suggested, assuming if (ret) format was proposed here

gastmaier · 2025-09-24T12:57:56Z

drivers/iio/frequency/ad917x/ad917x_jesd_api.c

+
+	*done = (mode & AD917X_SYNC_ROTATION_DONE);
+
+	return API_ERROR_OK;


Suggested change

return API_ERROR_OK;

return 0;

We avoid the api defines at the linux driver layer.

Now returning 0.

drivers/iio/frequency/ad917x/ad917x_jesd_api.c

nunojsa · 2025-09-24T14:01:37Z

drivers/iio/frequency/ad917x/ad917x_jesd_api.c

+	if (err != API_ERROR_OK)
+		return err;
+
+	*done = (mode & AD917X_SYNC_ROTATION_DONE);


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

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.

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.

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

vai-tomme · 2025-09-26T10:16:40Z

Addressed comments up til now.

Support for performing oneshot sequence as laid out in section "jesd204b serial data interface" and sync procedure of datasheet rev C [1] for device. In addition to the already implemented sysref_enable step, the other steps are also required for a sync to take place. Feature is here implemented for subclass 1 operation with jesd-fsm enabled. [1] https://www.analog.com/media/en/technical-documentation/data-sheets/ad9171.pdf Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>

nunojsa

Still don't agree with that done argument but as I said, up to you :)

vai-tomme · 2025-09-29T10:21:55Z

Please just do kernel style null and error checking where you can

@gastmaier now implemented, please check!

bindings: iio: frequency: adi,ad9172: sysref-error-window

643046a

Add devicetree property for setting allowed sysref jitter. Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>

vai-tomme requested review from nunojsa, mhennerich, dbogdan, btogorean, chegbeli, buha, amiclaus, rbolboac, danmois, CiprianRegus and machschmitt as code owners August 22, 2025 10:48

gastmaier mentioned this pull request Aug 28, 2025

axi_cfd: Fix depends for regmap_spi #2920

Merged

6 tasks

vai-tomme force-pushed the upstream/ad9172_0 branch from db276db to baf8214 Compare August 29, 2025 08:28

gastmaier requested changes Sep 24, 2025

View reviewed changes

nunojsa reviewed Sep 24, 2025

View reviewed changes

vai-tomme force-pushed the upstream/ad9172_0 branch from baf8214 to 0695937 Compare September 26, 2025 10:10

vai-tomme force-pushed the upstream/ad9172_0 branch from 0695937 to 063e015 Compare September 26, 2025 10:20

nunojsa approved these changes Sep 26, 2025

View reviewed changes


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

		if (lnk->subclass == JESD204_SUBCLASS_1) {


		*done = (mode & AD917X_SYNC_ROTATION_DONE);

		return API_ERROR_OK;

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

vai-tomme commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

PR Type

PR Checklist

Testing

Uh oh!

gastmaier commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vai-tomme commented Aug 29, 2025

Uh oh!

vai-tomme commented Sep 18, 2025

Uh oh!

gastmaier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vai-tomme Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vai-tomme commented Sep 26, 2025

Uh oh!

nunojsa left a comment

Choose a reason for hiding this comment

Uh oh!

vai-tomme commented Sep 29, 2025

Uh oh!

Uh oh!

vai-tomme commented Aug 22, 2025 •

edited

Loading

gastmaier commented Aug 28, 2025 •

edited

Loading

vai-tomme Sep 26, 2025 •

edited

Loading