On 03/03/2025 14:31, Sam Winchenbach wrote:
>>> 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.
>>
>> Description is confusing a bit, because it suggests the value sets the
>> corner frequency. It explicitly says this - "sets ... corner frequency"
>> and such meaning for properties we usually associate with the property
>> doing this. Here however corner frequency will be always set to rf_in
>> and you just adjust the value.
>>
>
> How about: "Sets the minimum distance (in Hz) between the fundamental
> frequency of `rf_in` and the corner frequency of the high-pass, input filter
> when operatred in 'auto' mode. The selected high-pass corner frequency will
> be less than, or equal to, `rf_in` - `hpf-margin-hz`. If not setting is found
> that satisfies this relationship the filter will be put into 'bypass'."
>
> Perhaps that is a bit more clear on the intention of this parameter?
Yes
>
>>>
>>>>
>>>>> 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>;
>>
>>
>> foo-hz is in 32-bit, so basically you have here 4 32-bit numbers which
>> indeed reported by dtschema - property is too long. Drop 64-bit here.
>>
>
> I was hoping to keep this 64 bits seeing this is a 18 GHz+ filter. I suppose
> I could change this to MHz and just lose a bit of resolution. Does that sound
> like a better approach?
Does the hardware accept Hz resolution? How many bits do you have in the
registers per each value?
Anyway, the value was 32-bit even in your original patch and your DTS
example was not correct.
>
>> Device allows multiple LPF/HPF values to be stored in LUT tables and it
>> actually has four independent filters. Shouldn't these be included here?
>> Maybe not LUT tables, but the configuration for all filters?
>>
>
> There are two filters, the input (high-pass) filter, and the output (low-pass)
> filter. Each filter has four banks, each with a different range of frequencies.
> Only one bank can be selected at a time. Each bank has 16 different possible
> cutoff/corner frequencies. That is a total of 64 distinct values for each of
> the two filters.
Hm, datasheet says:
"four independently controlled high-
pass filters (HPFs) and four independently controlled low-pass
filters (LPFs)"
so four each, not one each, but I guess they wanted to say banks as only
one filter/bank can be active in given time?
>
> The issue with setting the corner frequency directly is that in certain
> applications (such as software defined radios) the fundamental frequency
> is adjustable, necessitating that the corner frequencies of the filter are
> adjusted accordingly. When the filter is in "auto" mode it is notified via
> the clock system of frequency changes, so using this information it should be
> possible to select new corner frequencies if you know the minimum distance
> between your fundamental frequency and the corner.
I am not advocating to set the corner frequency directly, but just
pointing that your current binding seems incomplete.
Best regards,
Krzysztof
