iio: trx-rf: adrv903x: Add AGC debugfs config

[danmois]

PR Description

Add per-channel AGC configuration support via debugfs, following the shadow-struct + commit pattern.

Each RX channel exposes ~35 AGC parameters as individual debugfs files (u8/u16/u32) plus a summary file (rxN_agc_config) that seeds the shadow on read and commits to hardware on write.

The AGC shadow is stored per-channel in the phy struct (phy->rx_agc[]) to support multiple device instances.

Clamp four fields before commit where firmware defaults fall outside the API's own documented valid ranges:
- adcOvldLowThresh: max 8
- adcOvldLowerThreshPeakExceededCnt: min 2
- adcOvldUpperThreshPeakExceededCnt: min 2
- agcPeakWaitTime: max 3

Make adrv903x_debugfs.o and adrv903x_agc.o conditional on CONFIG_DEBUG_FS to avoid compiling dead code when debugfs is disabled.

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 compiled my changes, including the documentation
• I have tested the changes on the relevant hardware
• I have updated the documentation outside this repo accordingly
• I have provided links for the relevant upstream lore

[nunojsa]

There is a clamp() API

[danmois]

Thank you for looking into this. Wouldn't min/max be more appropriate since these are all one-sided constraints?

[nunojsa]

Don't think so! The above seems like a normal clamp() to me! Even your comment agrees :)

[nunojsa]

This should be squashed in the first patch

[danmois]

V2

• squash commits
• use min/max instead of if constructs for constraints

[stefpopa]

To me it looks good overall, but we should clarify who is this feature for?
Is it for internal bring-up/tuning? If so, then debugfs is fine, just mention this in the commit message.
If this feature is needed by customers who need to set AGC parameters in their product, maybe we should be thinking about doing it like adrv9009's adi,rxagc-*

[tfcollins]

To me it looks good overall, but we should clarify who is this feature for? Is it for internal bring-up/tuning? If so, then debugfs is fine, just mention this in the commit message. If this feature is needed by customers who need to set AGC parameters in their product, maybe we should be thinking about doing it like adrv9009's adi,rxagc-*

Is the question this should be devicetree config only or debugfs only? I would argue for both. This is something advanced that you want to tune at runtime but only during use case bring up.

[stefpopa]

To me it looks good overall, but we should clarify who is this feature for? Is it for internal bring-up/tuning? If so, then debugfs is fine, just mention this in the commit message. If this feature is needed by customers who need to set AGC parameters in their product, maybe we should be thinking about doing it like adrv9009's adi,rxagc-*

Is the question this should be devicetree config only or debugfs only? I would argue for both. This is something advanced that you want to tune at runtime but only during use case bring up.

Yes, they can be devicetree properties. The board's DT carries the values and the driver reads them at probe and applies them during init, like it is done for adrv9009.

[danmois]

To me it looks good overall, but we should clarify who is this feature for? Is it for internal bring-up/tuning? If so, then debugfs is fine, just mention this in the commit message. If this feature is needed by customers who need to set AGC parameters in their product, maybe we should be thinking about doing it like adrv9009's adi,rxagc-*

Is the question this should be devicetree config only or debugfs only? I would argue for both. This is something advanced that you want to tune at runtime but only during use case bring up.

Yes, they can be devicetree properties. The board's DT carries the values and the driver reads them at probe and applies them during init, like it is done for adrv9009.

Would then be appropriate to specify in this commit messages and PR title that here we add debugf support and come with a subsequent PR adding DT properties for initialization?

[nunojsa]

Yes, they can be devicetree properties. The board's DT carries the values and the driver reads them at probe and applies them during init, like it is done for adrv9009.

Yeah, typically we have them both in DT and debugfs but, in fact, the only "correct" one is debugfs as we don't care much about it and debugfs is very permissive! DT is an hack we need to use (and abuse) in these drivers to have a proper configuration path for this kind of settings (but note this is really abusing devicetree) given that one should not have to rely on debugfs being mounted to have the feature. Ideally, we would have this either with configfs (could be painful but in theory same experience as debugfs) or ioctls (could be painful to maintain our own but likely the correct approach).

[nunojsa]

use min/max instead of if constructs for constraints

I do think clamp() is the proper one to use

[danmois]

V3

• use clamp() API for making sure allowable values are set

[nunojsa]

Don't think you need clamp_t(). Also, please split lines on the argument level rather than this

[danmois]

V4

• use clamp() instead of clamp_t()
• split line on the argument level

[nunojsa]

uHu? Not what I meant, sorry... Spitting either after = or ( is very weird and not typical style. Why not

agc->agcPeak.adcOvldLowerThreshPeakExceededCnt = clamp(agc->agcPeak.adcOvldLowThresh, (u8)0,(u8)8);

I don't even think it passes 101 col limit? If it passes, do:

agc->agcPeak.adcOvldLowerThreshPeakExceededCnt = clamp(agc->agcPeak.adcOvldLowerThreshPeakExceededCnt,
                                                      (u8)2, (u8)255);

But also, remove all the unneeded casts

[danmois]

V5

• remove unnecessary casts
• add intermediary value to keep lines in 101 column limit