| Message ID | 20250225134612.577022-2-sam.winchenbach@framepointer.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/2] iio: filter: admv8818: fix range calculation | expand |
On Tue, Feb 25, 2025 at 08:46:12AM -0500, Sam Winchenbach wrote: > Adds two properties to add a margin when automatically finding the > corner frequencies. > > Signed-off-by: Sam Winchenbach <sam.winchenbach@framepointer.org> > --- > .../bindings/iio/filter/adi,admv8818.yaml | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) Bindings are before users (see DT submitting patches), so this should be re-ordered. > > diff --git a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > index b77e855bd594..2acdbd8d84cb 100644 > --- a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > @@ -44,6 +44,18 @@ properties: > '#clock-cells': > const: 0 > > + adi,lpf-margin-hz: > + description: > + Sets minimum low-pass corner frequency to the frequency of rf_in plus > + this value when in auto mode. > + default: 0 > + > + adi,hpf-margin-hz: > + description: > + Sets maximum high-pass corner frequency to the frequency of rf_in minus > + this value when in auto mode. IIUC, these are two bounds - lower and upper - in relation to something else (like rf_in frequency)? If so, make it an array (naming to be discuss, I assume you know better what's that): adi,filter-margin-hz: items: - description: low-pass corner frequency to the freq..... minimum: xxxx? maximum: xxxx? default: 0 - description: high-pass .... minimum: xxxx? maximum: xxxx? default: 0 Best regards, Krzysztof
On Wed, Feb 26, 2025 at 09:03:13AM +0100, Krzysztof Kozlowski wrote: > On Tue, Feb 25, 2025 at 08:46:12AM -0500, Sam Winchenbach wrote: > > Adds two properties to add a margin when automatically finding the > > corner frequencies. > > > > Signed-off-by: Sam Winchenbach <sam.winchenbach@framepointer.org> > > --- > > .../bindings/iio/filter/adi,admv8818.yaml | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > Bindings are before users (see DT submitting patches), so this should be > re-ordered. > > > > > diff --git a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > > index b77e855bd594..2acdbd8d84cb 100644 > > --- a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > > +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > > @@ -44,6 +44,18 @@ properties: > > '#clock-cells': > > const: 0 > > > > + adi,lpf-margin-hz: > > + description: > > + Sets minimum low-pass corner frequency to the frequency of rf_in plus > > + this value when in auto mode. > > + default: 0 > > + > > + adi,hpf-margin-hz: > > + description: > > + Sets maximum high-pass corner frequency to the frequency of rf_in minus > > + this value when in auto mode. > > IIUC, these are two bounds - lower and upper - in relation to something > else (like rf_in frequency)? If so, make it an array (naming to be > discuss, I assume you know better what's that): It is true that these are both related to rf_in but both the low and high pass filters can operate independently. Logically, IMO, it makes more sense to have them as separate controls but I am happy to put them into an array if that is the idiomatic approach to situations like this. That said, I am having a difficult time getting dt_binding_check to pass when I have an array of uint64. When listing two items, as in your example below, I get the following: adi,admv8818.example.dtb: admv8818@0: adi,filter-margins-hz: [[0, 30000000], [0, 30000000]] is too long I have tried specifying the scheme for each item, setting minItems/maxItems. Any advice on this would be appreciated. Thanks. > > adi,filter-margin-hz: > items: > - description: low-pass corner frequency to the freq..... > minimum: xxxx? > maximum: xxxx? > default: 0 > - description: high-pass .... > minimum: xxxx? > maximum: xxxx? > default: 0 > > Best regards, > Krzysztof >
On 26/02/2025 18:10, Sam Winchenbach wrote: > On Wed, Feb 26, 2025 at 09:03:13AM +0100, Krzysztof Kozlowski wrote: >> On Tue, Feb 25, 2025 at 08:46:12AM -0500, Sam Winchenbach wrote: >>> Adds two properties to add a margin when automatically finding the >>> corner frequencies. >>> >>> Signed-off-by: Sam Winchenbach <sam.winchenbach@framepointer.org> >>> --- >>> .../bindings/iio/filter/adi,admv8818.yaml | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >> >> Bindings are before users (see DT submitting patches), so this should be >> re-ordered. >> >>> >>> diff --git a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml >>> index b77e855bd594..2acdbd8d84cb 100644 >>> --- a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml >>> +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml >>> @@ -44,6 +44,18 @@ properties: >>> '#clock-cells': >>> const: 0 >>> >>> + adi,lpf-margin-hz: >>> + description: >>> + Sets minimum low-pass corner frequency to the frequency of rf_in plus >>> + this value when in auto mode. >>> + default: 0 >>> + >>> + adi,hpf-margin-hz: >>> + description: >>> + Sets maximum high-pass corner frequency to the frequency of rf_in minus >>> + this value when in auto mode. >> >> IIUC, these are two bounds - lower and upper - in relation to something >> else (like rf_in frequency)? If so, make it an array (naming to be >> discuss, I assume you know better what's that): > > It is true that these are both related to rf_in but both the low and high pass > filters can operate independently. Logically, IMO, it makes more sense to have You mean you can set only low or high pass and keep other as default? But what is the default then - something from reset value or "0" means disabled? > them as separate controls but I am happy to put them into an array if that is > the idiomatic approach to situations like this. That said, I am having a > difficult time getting dt_binding_check to pass when I have an array of uint64. > > When listing two items, as in your example below, I get the following: > adi,admv8818.example.dtb: admv8818@0: adi,filter-margins-hz: [[0, 30000000], [0, 30000000]] is too long Tricky to say without seeing your code. Magic crystal ball had malfunction today. Best regards, Krzysztof
On Wed, Feb 26, 2025 at 10:22:03PM +0100, Krzysztof Kozlowski wrote: > On 26/02/2025 18:10, Sam Winchenbach wrote: > > On Wed, Feb 26, 2025 at 09:03:13AM +0100, Krzysztof Kozlowski wrote: > >> On Tue, Feb 25, 2025 at 08:46:12AM -0500, Sam Winchenbach wrote: > >>> Adds two properties to add a margin when automatically finding the > >>> corner frequencies. > >>> > >>> Signed-off-by: Sam Winchenbach <sam.winchenbach@framepointer.org> > >>> --- > >>> .../bindings/iio/filter/adi,admv8818.yaml | 14 ++++++++++++++ > >>> 1 file changed, 14 insertions(+) > >> > >> Bindings are before users (see DT submitting patches), so this should be > >> re-ordered. > >> > >>> > >>> diff --git a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > >>> index b77e855bd594..2acdbd8d84cb 100644 > >>> --- a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > >>> +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml > >>> @@ -44,6 +44,18 @@ properties: > >>> '#clock-cells': > >>> const: 0 > >>> > >>> + adi,lpf-margin-hz: > >>> + description: > >>> + Sets minimum low-pass corner frequency to the frequency of rf_in plus > >>> + this value when in auto mode. > >>> + default: 0 > >>> + > >>> + adi,hpf-margin-hz: > >>> + description: > >>> + Sets maximum high-pass corner frequency to the frequency of rf_in minus > >>> + this value when in auto mode. > >> > >> IIUC, these are two bounds - lower and upper - in relation to something > >> else (like rf_in frequency)? If so, make it an array (naming to be > >> discuss, I assume you know better what's that): > > > > It is true that these are both related to rf_in but both the low and high pass > > filters can operate independently. Logically, IMO, it makes more sense to have > > > You mean you can set only low or high pass and keep other as default? > But what is the default then - something from reset value or "0" means > disabled? This value isn't setting the corner frequency of the filter, but the minimum distance the corner must be from the fundamental frequency. So, for example, if rf_in is 3.35 GHz and you set lpf-margin-hz to 0 then the corner frequency will be set to 3.35 GHz because that is an exact value supported by the device. If lpf-margin-hz is set to 30 MHz (for example), then corner frequency would be at least 3.35 GHz + 30 MHz = 3.38 GHz. 3.49 GHz is the closest corner frequency without going below 3.38 GHz that is supported by the device, so that is what will be selected. This prevents the situation where your fundamental frequency falls on, or close to, a corner frequency which could result in 3dB (half power) loss in your signal. This is all completely indepent of the high-pass filter. > > > them as separate controls but I am happy to put them into an array if that is > > the idiomatic approach to situations like this. That said, I am having a > > difficult time getting dt_binding_check to pass when I have an array of uint64. > > > > When listing two items, as in your example below, I get the following: > > adi,admv8818.example.dtb: admv8818@0: adi,filter-margins-hz: [[0, 30000000], [0, 30000000]] is too long > > Tricky to say without seeing your code. Magic crystal ball had > malfunction today. This is the property: adi,filter-margins-hz: items: - description: | The minimum distance, in Hz, between rf_in and the low-pass corner frequency when the device is used in "auto" mode. If the sum of rf_in and this value is greater than 18.85 GHz then the low-pass filter will be put into bypass mode, otherwise the closest corner frequency that is greater than or equal to the sum of rf_in plus this value will be used. minimum: 0 maximum: 0xFFFFFFFFFFFFFFFF default: 0 - description: | The minimum distance, in Hz, between rf_in and the high-pass corner frequency when the device is used in "auto" mode. If the difference between rf_in and this value is less than 1.75 GHz then the high-pass filter will be put into bypass mode, otherwise the closest corner frequency that is less than or equal to the difference of rf_in and this value will be used. minimum: 0 maximum: 0xFFFFFFFFFFFFFFFF default: 0 And this is the example: examples: - | spi { #address-cells = <1>; #size-cells = <0>; admv8818@0 { compatible = "adi,admv8818"; reg = <0>; spi-max-frequency = <10000000>; clocks = <&admv8818_rfin>; clock-names = "rf_in"; adi,filter-margins-hz = /bits/ 64 <30000000 30000000>; }; }; ... Thank you for taking the time to go through this, -Sam > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml index b77e855bd594..2acdbd8d84cb 100644 --- a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml @@ -44,6 +44,18 @@ properties: '#clock-cells': const: 0 + adi,lpf-margin-hz: + description: + Sets minimum low-pass corner frequency to the frequency of rf_in plus + this value when in auto mode. + default: 0 + + adi,hpf-margin-hz: + description: + Sets maximum high-pass corner frequency to the frequency of rf_in minus + this value when in auto mode. + default: 0 + required: - compatible - reg @@ -61,6 +73,8 @@ examples: spi-max-frequency = <10000000>; clocks = <&admv8818_rfin>; clock-names = "rf_in"; + adi,lpf-margin-hz = /bits/ 64 <30000000>; + adi,hpf-margin-hz = /bits/ 64 <30000000>; }; }; ...
Adds two properties to add a margin when automatically finding the corner frequencies. Signed-off-by: Sam Winchenbach <sam.winchenbach@framepointer.org> --- .../bindings/iio/filter/adi,admv8818.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)