Re: [PATCH v2 tty-next 1/2] dt-bindings: serial: ni,ni16650: add bindings

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 13/04/2023 22:44, Brenda Streiff wrote:
> 
> 
> On 4/13/23 02:39, Krzysztof Kozlowski wrote:
>> On 13/04/2023 00:24, Brenda Streiff wrote:
>>> On 4/11/23 00:44, Krzysztof Kozlowski wrote:
>>>> On 10/04/2023 23:11, Brenda Streiff wrote:
>>>>> +
>>>>> +  interrupts:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  clock-frequency: true
>>>>
>>>> I missed it last time - why do you need this property? You do not have
>>>> any clock input, so which clock's frequency is it?
>>>>
>>>
>>> This is the clock frequency of the UART; on our x86-based platforms this
>>> comes from the LPC clock, on Zynq-7000 it's derived from a clock in the
>>> FPGA. This is used to set struct uart_port::uartclk in the serial core,
>>> as it is for other UARTs.
>>>
>>> This clock frequency can vary based on board design (especially on the
>>> x86 side, due to different LPC clocks) but for a given design is fixed-
>>> frequency.
>>
>> So you must have clock input - clocks property. Once you add this, use
>> assigned-clocks to get the rate you want.
>>
>>>
>>> Would you prefer this be documented further? I was following 8250.yaml's
>>> lead here with the simple `true`.
>>
>> I prefer to drop it. It is not correct and a legacy property. Without
>> clock inputs how can you even configure some clock?
> 
> Configure in what respect? Software can't change this clock; it's
> effectively a fixed oscillator.

So where is the clock located? Physically.

> 
> In practice, this would always be pointing at a compatible="fixed-clock"
> which declares a clock-frequency; this seems like "clock-frequency but
> more steps". I can add that, but I'm not clear on what value that adds.

Aren't we talking about two different things? Based on limited
informamtion, I claimed you have clock input. If you have clock input,
then you miss clocks property.

What value would that add? I don't know... the rules that bindings
describe hardware?

> 
> We also have shipping devices with ACPI tables using "clock-frequency",
> so independent of support for "clocks" and "assigned-clocks" for
> devicetree-using systems, I would still have to keep support in the
> driver for a "clock-frequency" device property for ACPI-using systems.

I don't care about ACPI and ACPI has nothing to do with Bindings. We do
not write bindings for ACPI.

What your driver has or has not to do, it's also separate question. I2C
camera sensors solved it long time ago. I don't see why this must be
special.

> 
> (Is there documentation on a standalone clock-property being a legacy
> property that I've missed? 
> I don't see anything of the sort in
> writing-bindings.rst or in dt-schema and I want to make sure that I

dtschema:
  # Legacy clock properties
  clock-frequency:
    description: Legacy property ...

> haven't missed the proper guidance here.)

Best regards,
Krzysztof