diff mbox series

[2/2] dt-bindings: serial: sc16is7xx: remove 'clock-frequency' property

Message ID 20231013141925.3427158-3-hugo@hugovil.com
State New
Headers
Series serial: sc16is7xx: device tree improvements |

Commit Message

Hugo Villeneuve Oct. 13, 2023, 2:19 p.m. UTC 
  From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

The 'clock-frequency' property is supported but mainly in ACPI-based
configurations, for example.

This property has therefore no place in the sc16is7xx YAML binding.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../devicetree/bindings/serial/nxp,sc16is7xx.yaml  | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

Comments

Krzysztof Kozlowski Oct. 16, 2023, 6:09 a.m. UTC | #1 
On 13/10/2023 16:19, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> The 'clock-frequency' property is supported but mainly in ACPI-based
> configurations, for example.
> 
> This property has therefore no place in the sc16is7xx YAML binding.

Please reference commit which you question - 0d447e927. I don't
understand why do you remove it. The property was in the original binding.

Best regards,
Krzysztof
Hugo Villeneuve Oct. 16, 2023, 1:58 p.m. UTC | #2 
On Mon, 16 Oct 2023 08:09:56 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 13/10/2023 16:19, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > The 'clock-frequency' property is supported but mainly in ACPI-based
> > configurations, for example.
> > 
> > This property has therefore no place in the sc16is7xx YAML binding.
> 
> Please reference commit which you question - 0d447e927. I don't
> understand why do you remove it. The property was in the original binding.

Hi,
in the max310x YAML conversion review (last week), you told me to
drop the clock-frequency from the binding, even
if it is supported by the driver, since it is related to ACPI
configuration, not DT.

The sc16is7xx driver (IC) is very similar to the max310x, and it also
supports the clock-frequency property, and I just assumed that its
presence in the original text binding was some kind of error or legacy
leftover, and would need to be removed based on your comments.

Just as a reference, here are the original commits in both drivers that
added support for the clock-frequency property:

-----------------------
commit d4d6f03c4fb3a91dadfe147b47edd40e4d7e4d36
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date:   Mon May 17 20:29:30 2021 +0300

    serial: max310x: Try to get crystal clock rate from property
    
    In some configurations, mainly ACPI-based, the clock frequency of
the device is supplied by very well established 'clock-frequency'
property. Hence, try to get it from the property at last if no other
providers are available.


commit 24bc6e68efa00f95034dbef0ba91661dd80bd37d
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date:   Mon Mar 18 12:29:15 2019 +0200

    serial: sc16is7xx: Respect clock-frequency property
    
    If the property is provided and there are no other possibilities to
detect UART clock frequency, use it as a fallback.
-----------------------

Hugo.
Krzysztof Kozlowski Oct. 16, 2023, 2:18 p.m. UTC | #3 
On 16/10/2023 15:58, Hugo Villeneuve wrote:
> On Mon, 16 Oct 2023 08:09:56 +0200
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 13/10/2023 16:19, Hugo Villeneuve wrote:
>>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>>>
>>> The 'clock-frequency' property is supported but mainly in ACPI-based
>>> configurations, for example.
>>>
>>> This property has therefore no place in the sc16is7xx YAML binding.
>>
>> Please reference commit which you question - 0d447e927. I don't
>> understand why do you remove it. The property was in the original binding.
> 
> Hi,
> in the max310x YAML conversion review (last week), you told me to
> drop the clock-frequency from the binding, even
> if it is supported by the driver, since it is related to ACPI
> configuration, not DT.

Yes, when we talked about max310x.

> 
> The sc16is7xx driver (IC) is very similar to the max310x, and it also
> supports the clock-frequency property, and I just assumed that its
> presence in the original text binding was some kind of error or legacy
> leftover, and would need to be removed based on your comments.

But that is not the same and not accurate. clock-frequency was described
by the binding.

> 
> Just as a reference, here are the original commits in both drivers that
> added support for the clock-frequency property:
> 
> -----------------------
> commit d4d6f03c4fb3a91dadfe147b47edd40e4d7e4d36
> Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date:   Mon May 17 20:29:30 2021 +0300
> 
>     serial: max310x: Try to get crystal clock rate from property
>     
>     In some configurations, mainly ACPI-based, the clock frequency of
> the device is supplied by very well established 'clock-frequency'
> property. Hence, try to get it from the property at last if no other
> providers are available.
> 
> 
> commit 24bc6e68efa00f95034dbef0ba91661dd80bd37d
> Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date:   Mon Mar 18 12:29:15 2019 +0200
> 
>     serial: sc16is7xx: Respect clock-frequency property
>     
>     If the property is provided and there are no other possibilities to
> detect UART clock frequency, use it as a fallback.
> -----------------------

These are driver changes.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
index 5dec15b7e7c3..5050176e8b80 100644
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
@@ -28,13 +28,6 @@  properties:
   clocks:
     maxItems: 1
 
-  clock-frequency:
-    description:
-      When there is no clock provider visible to the platform, this
-      is the source crystal or external clock frequency for the IC in Hz.
-    minimum: 1
-    maximum: 80000000
-
   gpio-controller: true
 
   "#gpio-cells":
@@ -74,18 +67,13 @@  required:
   - compatible
   - reg
   - interrupts
+  - clocks
 
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
   - $ref: /schemas/serial/serial.yaml#
   - $ref: /schemas/serial/rs485.yaml#
 
-oneOf:
-  - required:
-      - clocks
-  - required:
-      - clock-frequency
-
 unevaluatedProperties: false
 
 examples: