| Message ID | 1473957763-30629-3-git-send-email-alexandre.torgue@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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". > > Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com> > Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com> > > 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. 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. > +- 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? > + reg = <0x40004400 0x400>; > + interrupts = <38>; > + clocks = <&clk_pclk1>; > + st,hw-flow-ctrl; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_usart2 &pinctrl_usart2_rtscts>; > +}; > -- > 1.9.1 >
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@gmail.com> >> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com> >> >> 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? > 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). >> + reg = <0x40004400 0x400>; >> + interrupts = <38>; >> + clocks = <&clk_pclk1>; >> + st,hw-flow-ctrl; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_usart2 &pinctrl_usart2_rtscts>; >> +}; >> -- >> 1.9.1 >>
On Wed, Oct 5, 2016 at 9:09 AM, Gerald Baeza <gerald.baeza@st.com> 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@gmail.com> >>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com> >>> >>> 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
Hi Rob, On 10/05/2016 05:13 PM, Rob Herring wrote: > On Wed, Oct 5, 2016 at 9:09 AM, Gerald Baeza <gerald.baeza@st.com> 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@gmail.com> >>>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com> >>>> >>>> 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. Ok. Do you mean something like: - compatible: "st,stm32-usart", "st,stm32-uart" For STM32F4 SOC and if IP supports synchronous mode. - compatible: "st,stm32-uart" For STM32F4 SOC. - compatible: "st,stm32f7-usart", "st,stm32f7-uart": For STM32F7 SOC and if IP supports synchronous mode. - compatible: "st,stm32f7-uart" For STM32F7 SOC If you agree, what do you prefer to send modification ? I mean, those bindings documentation patches are already in linux-next tree. Do you want a new patch on top of linux-next or do you prefer I resend only those ones ? Regards Alex > >>> 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 >
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. +- 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"; + reg = <0x40004400 0x400>; + interrupts = <38>; + clocks = <&clk_pclk1>; + st,hw-flow-ctrl; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_usart2 &pinctrl_usart2_rtscts>; +};