On Wed, Oct 5, 2016 at 9:09 AM, Gerald Baeza <gerald.baeza@xxxxxx> wrote:
> On 09/23/2016 05:29 PM, Rob Herring wrote:
>>
>> On Thu, Sep 15, 2016 at 06:42:34PM +0200, Alexandre TORGUE wrote:
>>>
>>> This adds documentation of device tree bindings for the
>>> STM32 USART
>>
>>
>> Please make your subject prefixes consistent and drop "DOCUMENTATION".
>>
>
> Ok, thanks
>
>>>
>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@xxxxxxxxx>
>>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@xxxxxx>
>>>
>>> diff --git a/Documentation/devicetree/bindings/serial/st,stm32-usart.txt
>>> b/Documentation/devicetree/bindings/serial/st,stm32-usart.txt
>>> new file mode 100644
>>> index 0000000..75b1400
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/st,stm32-usart.txt
>>> @@ -0,0 +1,34 @@
>>> +* STMicroelectronics STM32 USART
>>> +
>>> +Required properties:
>>> +- compatible: Can be either "st,stm32-usart", "st,stm32-uart",
>>> +"st,stm32f7-usart" or "st,stm32f7-uart" depending on whether
>>> +the device supports synchronous mode and is compatible with
>>> +stm32(f4) or stm32f7.
>>
>>
>> Why not put f4 in the compatible string. stm32 is too generic.
>
>
> The initial binding is not in current kernel so it has been put in this
> serie as PATCH 07/11. It will be squashed with this one, as you requested.
>
> But the driver tty/serial/stm32-usart.c was already upstreamed and it
> already mentions the "st,stm32-usart" and "st,stm32-uart" for stm32f4 so I
> kept this as it for backward compatibility for those who already use the
> driver.
>
> I do not have the history to explain this inconsistency but can you confirm
> that keeping the existing compatible values from the driver is the good
> approach please?
Yes, keep it as it. Please reformat 1 valid combination per line.
>> What determines sync mode or not? If it is IP configuration fixed in the
>> design, then this is fine. If it is user choice or board dependent, then
>> use a separate property.
>
>
> This is IP configuration fixed in the design, indeed.
>
>>> +- reg: The address and length of the peripheral registers space
>>> +- interrupts: The interrupt line of the USART instance
>>> +- clocks: The input clock of the USART instance
>>> +
>>> +Optional properties:
>>> +- pinctrl: The reference on the pins configuration
>>> +- st,hw-flow-ctrl: bool flag to enable hardware flow control.
>>> +
>>> +Examples:
>>> +usart4: serial@40004c00 {
>>> + compatible = "st,stm32-uart";
>>> + reg = <0x40004c00 0x400>;
>>> + interrupts = <52>;
>>> + clocks = <&clk_pclk1>;
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&pinctrl_usart4>;
>>> +};
>>> +
>>> +usart2: serial@40004400 {
>>> + compatible = "st,stm32-usart", "st,stm32-uart";
>>
>>
>> What are valid combinations? usart is sync only, not sync and async?
>
>
> usart (sync and async) is a superset of uart (async).
> But the current driver does not use the synchronous mode, so the distinction
> is just here to be consistent with the reference manual instances naming (so
> configuration).
Okay, but this point is not clear in the compatible text. The
description should allow me to validate the example or a dts file.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
