Re: [PATCH 2/6] dt-bindings: serial: Add clock-frequency property as an alternative to clocks

[Andrea della Porta <andrea.porta@suse.com>]

Hi Krzysztof,

On 14:02 Tue 12 Aug     , Krzysztof Kozlowski wrote:
> On 11/08/2025 17:19, Andrea della Porta wrote:
> > The UARTA controller on BCM2712 connected to Bluetooth chip does not
> 
> Bluetooth chip does not ask...
> 
> > mandiatorily ask for a clock connected to the high speed baud generator.
> > This is, in fact, an optional clock in the driver.
> 
> ... or driver does not ask?
> 
> Please describe here hardware.
> 
> 
> > 
> > As an alternative, the call to uart_read_port_properties() ensures that
> > just a simple 'clock-frequency' property can be specified for the clock
> > value.
> 
> Don't describe drivers. Describe hardware.

I will try to test whether the driver can just work specifying clock instead of
clock-frequency in teh DTS, so there will be no need to amend the bindings.

Many thanks,
Andrea

> 
> > 
> > Amend the bindings to allow to either specify clocks or clock-frequency.
> > 
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > ---
> >  .../bindings/serial/brcm,bcm7271-uart.yaml    | 19 +++++++++++++++++--
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> 
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/brcm,bcm7271-uart.yaml b/Documentation/devicetree/bindings/serial/brcm,bcm7271-uart.yaml
> > index 89c462653e2d..96697b1428bd 100644
> > --- a/Documentation/devicetree/bindings/serial/brcm,bcm7271-uart.yaml
> > +++ b/Documentation/devicetree/bindings/serial/brcm,bcm7271-uart.yaml
> > @@ -40,7 +40,15 @@ properties:
> >            - const: dma_tx
> >            - const: dma_intr2
> >  
> > +  clock-frequency:
> > +    description:
> > +      The input clock frequency for the UART, Either this or clocks must be
> > +      specified.
> 
> Anyway, don't open-code schema in free form text.
> 
> That's legacy property. You need clear explanation why.
> 
> > +
> >    clocks:
> > +    description:
> > +      High speed baud rate clock. Either this or clock-frequency must be
> > +      specified.
> 
> Drop last sentence, Anyway, don't open-code schema in free form text.
> First sentence seems redundant anyway.
> 
> 
> >      minItems: 1
> 
> I'll fix this.
> 
> >  
> >    clock-names:
> > @@ -61,11 +69,18 @@ required:
> >    - compatible
> >    - reg
> >    - reg-names
> > -  - clocks
> > -  - clock-names
> >    - interrupts
> >    - interrupt-names
> >  
> > +oneOf:
> > +  - allOf:
> > +      - required:
> > +          - clocks
> > +      - required:
> > +          - clock-names
> > +  - required:
> > +      - clock-frequency
> > +
> >  unevaluatedProperties: false
> >  
> >  examples:
> 
> 
> Best regards,
> Krzysztof