| Message ID | 20160726062233.7656-2-stefan@agner.ch |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
On 26 July 2016 at 00:22, Stefan Agner <stefan@agner.ch> wrote: > From: Stefan Agner <stefan.agner@toradex.com> > > Support instatiation through device tree. Also parse the fsl,dte-mode > property to determine whether DTE mode shall be used. > > Signed-off-by: Stefan Agner <stefan.agner@toradex.com> > --- > The kernel uses fsl,imx21-uart as "base" compatible, should we follow > that? > > doc/device-tree-bindings/serial/mxc-serial.txt | 8 ++++++++ > drivers/serial/serial_mxc.c | 28 ++++++++++++++++++++++++-- > 2 files changed, 34 insertions(+), 2 deletions(-) > create mode 100644 doc/device-tree-bindings/serial/mxc-serial.txt Reviewed-by: Simon Glass <sjg@chromium.org> Do all MX6 boards define OF_CONTROL? - Simon
Hi Simon, On 2016-07-31 18:01, Simon Glass wrote: > On 26 July 2016 at 00:22, Stefan Agner <stefan@agner.ch> wrote: >> From: Stefan Agner <stefan.agner@toradex.com> >> >> Support instatiation through device tree. Also parse the fsl,dte-mode >> property to determine whether DTE mode shall be used. >> >> Signed-off-by: Stefan Agner <stefan.agner@toradex.com> >> --- >> The kernel uses fsl,imx21-uart as "base" compatible, should we follow >> that? >> >> doc/device-tree-bindings/serial/mxc-serial.txt | 8 ++++++++ >> drivers/serial/serial_mxc.c | 28 ++++++++++++++++++++++++-- >> 2 files changed, 34 insertions(+), 2 deletions(-) >> create mode 100644 doc/device-tree-bindings/serial/mxc-serial.txt > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Do all MX6 boards define OF_CONTROL? No, I think none do actually. Also note that the conversion is done for a MX7 board, but also here, no boards yet using OF_CONTROL... -- Stefan
On 26/07/2016 08:22, Stefan Agner wrote: > From: Stefan Agner <stefan.agner@toradex.com> > > Support instatiation through device tree. Also parse the fsl,dte-mode > property to determine whether DTE mode shall be used. > > Signed-off-by: Stefan Agner <stefan.agner@toradex.com> > --- > The kernel uses fsl,imx21-uart as "base" compatible, should we follow > that? > > doc/device-tree-bindings/serial/mxc-serial.txt | 8 ++++++++ > drivers/serial/serial_mxc.c | 28 ++++++++++++++++++++++++-- > 2 files changed, 34 insertions(+), 2 deletions(-) > create mode 100644 doc/device-tree-bindings/serial/mxc-serial.txt > > diff --git a/doc/device-tree-bindings/serial/mxc-serial.txt b/doc/device-tree-bindings/serial/mxc-serial.txt > new file mode 100644 > index 0000000..ede92a4 > --- /dev/null > +++ b/doc/device-tree-bindings/serial/mxc-serial.txt > @@ -0,0 +1,8 @@ > +NXP i.MX (MXC) UART > + > +Required properties: > +- compatible: must be "fsl,imx7d-uart" > +- reg: start address and size of the registers > + > +Optional properties: > +- fsl,dte-mode: use DTE mode > diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c > index 1960bbc..bd850d7 100644 > --- a/drivers/serial/serial_mxc.c > +++ b/drivers/serial/serial_mxc.c > @@ -108,6 +108,8 @@ > #define UTS_RXFULL (1<<3) /* RxFIFO full */ > #define UTS_SOFTRST (1<<0) /* Software reset */ > > +DECLARE_GLOBAL_DATA_PTR; > + > #ifndef CONFIG_DM_SERIAL > > #ifndef CONFIG_MXC_UART_BASE > @@ -135,8 +137,6 @@ > #define UBRC 0xac /* Baud Rate Count Register */ > #define UTS 0xb4 /* UART Test Register (mx31) */ > > -DECLARE_GLOBAL_DATA_PTR; > - > #define TXTL 2 /* reset default */ > #define RXTL 1 /* reset default */ > #define RFDIV 4 /* divide input clock by 2 */ > @@ -348,9 +348,33 @@ static const struct dm_serial_ops mxc_serial_ops = { > .setbrg = mxc_serial_setbrg, > }; > > +static int mxc_serial_ofdata_to_platdata(struct udevice *dev) > +{ > + struct mxc_serial_platdata *plat = dev->platdata; > + fdt_addr_t addr; > + > + addr = dev_get_addr(dev); > + if (addr == FDT_ADDR_T_NONE) > + return -EINVAL; > + > + plat->reg = (struct mxc_uart *)addr; > + > + plat->use_dte = fdtdec_get_bool(gd->fdt_blob, dev->of_offset, > + "fsl,dte-mode"); I have applied it, I just noted a slight drawback because this breaks boards that do not have CONFIG_FIT set. Best regards, Stefano
On 2016-08-26 07:10, Stefano Babic wrote: > On 26/07/2016 08:22, Stefan Agner wrote: >> From: Stefan Agner <stefan.agner@toradex.com> >> >> Support instatiation through device tree. Also parse the fsl,dte-mode >> property to determine whether DTE mode shall be used. >> >> Signed-off-by: Stefan Agner <stefan.agner@toradex.com> >> --- >> The kernel uses fsl,imx21-uart as "base" compatible, should we follow >> that? >> >> doc/device-tree-bindings/serial/mxc-serial.txt | 8 ++++++++ >> drivers/serial/serial_mxc.c | 28 ++++++++++++++++++++++++-- >> 2 files changed, 34 insertions(+), 2 deletions(-) >> create mode 100644 doc/device-tree-bindings/serial/mxc-serial.txt >> >> diff --git a/doc/device-tree-bindings/serial/mxc-serial.txt b/doc/device-tree-bindings/serial/mxc-serial.txt >> new file mode 100644 >> index 0000000..ede92a4 >> --- /dev/null >> +++ b/doc/device-tree-bindings/serial/mxc-serial.txt >> @@ -0,0 +1,8 @@ >> +NXP i.MX (MXC) UART >> + >> +Required properties: >> +- compatible: must be "fsl,imx7d-uart" >> +- reg: start address and size of the registers >> + >> +Optional properties: >> +- fsl,dte-mode: use DTE mode >> diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c >> index 1960bbc..bd850d7 100644 >> --- a/drivers/serial/serial_mxc.c >> +++ b/drivers/serial/serial_mxc.c >> @@ -108,6 +108,8 @@ >> #define UTS_RXFULL (1<<3) /* RxFIFO full */ >> #define UTS_SOFTRST (1<<0) /* Software reset */ >> >> +DECLARE_GLOBAL_DATA_PTR; >> + >> #ifndef CONFIG_DM_SERIAL >> >> #ifndef CONFIG_MXC_UART_BASE >> @@ -135,8 +137,6 @@ >> #define UBRC 0xac /* Baud Rate Count Register */ >> #define UTS 0xb4 /* UART Test Register (mx31) */ >> >> -DECLARE_GLOBAL_DATA_PTR; >> - >> #define TXTL 2 /* reset default */ >> #define RXTL 1 /* reset default */ >> #define RFDIV 4 /* divide input clock by 2 */ >> @@ -348,9 +348,33 @@ static const struct dm_serial_ops mxc_serial_ops = { >> .setbrg = mxc_serial_setbrg, >> }; >> >> +static int mxc_serial_ofdata_to_platdata(struct udevice *dev) >> +{ >> + struct mxc_serial_platdata *plat = dev->platdata; >> + fdt_addr_t addr; >> + >> + addr = dev_get_addr(dev); >> + if (addr == FDT_ADDR_T_NONE) >> + return -EINVAL; >> + >> + plat->reg = (struct mxc_uart *)addr; >> + >> + plat->use_dte = fdtdec_get_bool(gd->fdt_blob, dev->of_offset, >> + "fsl,dte-mode"); > > I have applied it, I just noted a slight drawback because this breaks > boards that do not have CONFIG_FIT set. Hm, maybe due to missing CONFIG_OF_LIBFDT? Do you want me to fix it, do you have a certain board you can reproduce it? -- Stefan
Hi Stefan, On 29/08/2016 02:00, Stefan Agner wrote: >> >> I have applied it, I just noted a slight drawback because this breaks >> boards that do not have CONFIG_FIT set. > > Hm, maybe due to missing CONFIG_OF_LIBFDT? Do you want me to fix it, do > you have a certain board you can reproduce it? No, I have found it. The patchset breaks two boards (gwventana and cmx6), and the reason is that lib/fdtdec.c is not compiled. This is because CONFIG_OF_CONTROL is not set for these two boards, but as far as I understand this should be not set, because there is no device tree for these two boards. The issue is generate by the feature use_dte: in fact: plat->use_dte = fdtdec_get_bool(gd->fdt_blob, dev->of_offset, "fsl,dte-mode"); but for boards without DT, fdtdec is not built and gd->fdt_blob is maybe not set. Can you take a look ? What do you think about it ? The second issue is related to CONFIG_CUSTOM_BOARDINFO: arm: + colibri_imx7 +Error: You must add new CONFIG options using Kconfig +The following new ad-hoc CONFIG options were detected: +CONFIG_CUSTOM_BOARDINFO + +Please add these via Kconfig instead. Find a suitable Kconfig +file and add a 'config' or 'menuconfig' option. This is related to: Author: Stefan Agner <stefan.agner@toradex.com> Date: Mon Aug 1 22:50:24 2016 -0700 configs: enable device tree for Colibri iMX7 Enable device tree configuration and specify default device tree for Toradex Colibri iMX7. Also configure CONFIG_CUSTOM_BOARDINFO to avoid that board info get printed twice (once from the device tree and one from the runtime detection in board specific code). Signed-off-by: Stefan Agner <stefan.agner@toradex.com> What about to split it ? I will let this patch to just enable the device tree, and let fix the double output with a follow up patch. What do you think ? Best regards, Stefano
On 2016-10-04 06:02, Stefano Babic wrote: > Hi Stefan, > > On 29/08/2016 02:00, Stefan Agner wrote: >>> >>> I have applied it, I just noted a slight drawback because this breaks >>> boards that do not have CONFIG_FIT set. >> >> Hm, maybe due to missing CONFIG_OF_LIBFDT? Do you want me to fix it, do >> you have a certain board you can reproduce it? > > No, I have found it. The patchset breaks two boards (gwventana and > cmx6), and the reason is that lib/fdtdec.c is not compiled. This is > because CONFIG_OF_CONTROL is not set for these two boards, but as far as > I understand this should be not set, because there is no device tree for > these two boards. > > The issue is generate by the feature use_dte: in fact: > > plat->use_dte = fdtdec_get_bool(gd->fdt_blob, dev->of_offset, > "fsl,dte-mode"); > > but for boards without DT, fdtdec is not built and gd->fdt_blob is maybe > not set. > > Can you take a look ? What do you think about it ? Hm, I see... I guess we need to add a CONFIG_IS_ENABLED(OF_CONTROL) there, that is what other drivers also do (e.g. drivers/gpio/mpc85xx_gpio.c). > > The second issue is related to CONFIG_CUSTOM_BOARDINFO: > > arm: + colibri_imx7 > +Error: You must add new CONFIG options using Kconfig > +The following new ad-hoc CONFIG options were detected: > +CONFIG_CUSTOM_BOARDINFO > + > +Please add these via Kconfig instead. Find a suitable Kconfig > +file and add a 'config' or 'menuconfig' option. Yeah I saw that and we either drop that config or will convert it to a proper Kconfig soon, see also this thread: http://lists.denx.de/pipermail/u-boot/2016-October/268669.html > > This is related to: > > Author: Stefan Agner <stefan.agner@toradex.com> > Date: Mon Aug 1 22:50:24 2016 -0700 > > configs: enable device tree for Colibri iMX7 > > Enable device tree configuration and specify default device tree > for Toradex Colibri iMX7. Also configure CONFIG_CUSTOM_BOARDINFO > to avoid that board info get printed twice (once from the device > tree and one from the runtime detection in board specific code). > > Signed-off-by: Stefan Agner <stefan.agner@toradex.com> > > What about to split it ? I will let this patch to just enable the device > tree, and let fix the double output with a follow up patch. What do you > think ? Sure, lets just not add CONFIG_CUSTOM_BOARDINFO for now and fix that once we figure out what we do with that config. I'll send new version of these two patches, or do you want a new patch which works ontop of your next branch? -- Stefan
Hi Stefan, On 05/10/2016 23:58, Stefan Agner wrote: > On 2016-10-04 06:02, Stefano Babic wrote: >> Hi Stefan, >> >> On 29/08/2016 02:00, Stefan Agner wrote: >>>> >>>> I have applied it, I just noted a slight drawback because this breaks >>>> boards that do not have CONFIG_FIT set. >>> >>> Hm, maybe due to missing CONFIG_OF_LIBFDT? Do you want me to fix it, do >>> you have a certain board you can reproduce it? >> >> No, I have found it. The patchset breaks two boards (gwventana and >> cmx6), and the reason is that lib/fdtdec.c is not compiled. This is >> because CONFIG_OF_CONTROL is not set for these two boards, but as far as >> I understand this should be not set, because there is no device tree for >> these two boards. >> >> The issue is generate by the feature use_dte: in fact: >> >> plat->use_dte = fdtdec_get_bool(gd->fdt_blob, dev->of_offset, >> "fsl,dte-mode"); >> >> but for boards without DT, fdtdec is not built and gd->fdt_blob is maybe >> not set. >> >> Can you take a look ? What do you think about it ? > > Hm, I see... I guess we need to add a CONFIG_IS_ENABLED(OF_CONTROL) > there, that is what other drivers also do (e.g. > drivers/gpio/mpc85xx_gpio.c). Yes, agree, this is the best solution. > >> >> The second issue is related to CONFIG_CUSTOM_BOARDINFO: >> >> arm: + colibri_imx7 >> +Error: You must add new CONFIG options using Kconfig >> +The following new ad-hoc CONFIG options were detected: >> +CONFIG_CUSTOM_BOARDINFO >> + >> +Please add these via Kconfig instead. Find a suitable Kconfig >> +file and add a 'config' or 'menuconfig' option. > > Yeah I saw that and we either drop that config or will convert it to a > proper Kconfig soon, see also this thread: > http://lists.denx.de/pipermail/u-boot/2016-October/268669.html > >> >> This is related to: >> >> Author: Stefan Agner <stefan.agner@toradex.com> >> Date: Mon Aug 1 22:50:24 2016 -0700 >> >> configs: enable device tree for Colibri iMX7 >> >> Enable device tree configuration and specify default device tree >> for Toradex Colibri iMX7. Also configure CONFIG_CUSTOM_BOARDINFO >> to avoid that board info get printed twice (once from the device >> tree and one from the runtime detection in board specific code). >> >> Signed-off-by: Stefan Agner <stefan.agner@toradex.com> >> >> What about to split it ? I will let this patch to just enable the device >> tree, and let fix the double output with a follow up patch. What do you >> think ? > > Sure, lets just not add CONFIG_CUSTOM_BOARDINFO for now and fix that > once we figure out what we do with that config. I'll send new version of > these two patches, or do you want a new patch which works ontop of your > next branch? It is ok with the new version you have sent, I want to apply them to -master before sending my PR to Tom. Thanks ! Best regards, Stefano Babic
diff --git a/doc/device-tree-bindings/serial/mxc-serial.txt b/doc/device-tree-bindings/serial/mxc-serial.txt new file mode 100644 index 0000000..ede92a4 --- /dev/null +++ b/doc/device-tree-bindings/serial/mxc-serial.txt @@ -0,0 +1,8 @@ +NXP i.MX (MXC) UART + +Required properties: +- compatible: must be "fsl,imx7d-uart" +- reg: start address and size of the registers + +Optional properties: +- fsl,dte-mode: use DTE mode diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 1960bbc..bd850d7 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -108,6 +108,8 @@ #define UTS_RXFULL (1<<3) /* RxFIFO full */ #define UTS_SOFTRST (1<<0) /* Software reset */ +DECLARE_GLOBAL_DATA_PTR; + #ifndef CONFIG_DM_SERIAL #ifndef CONFIG_MXC_UART_BASE @@ -135,8 +137,6 @@ #define UBRC 0xac /* Baud Rate Count Register */ #define UTS 0xb4 /* UART Test Register (mx31) */ -DECLARE_GLOBAL_DATA_PTR; - #define TXTL 2 /* reset default */ #define RXTL 1 /* reset default */ #define RFDIV 4 /* divide input clock by 2 */ @@ -348,9 +348,33 @@ static const struct dm_serial_ops mxc_serial_ops = { .setbrg = mxc_serial_setbrg, }; +static int mxc_serial_ofdata_to_platdata(struct udevice *dev) +{ + struct mxc_serial_platdata *plat = dev->platdata; + fdt_addr_t addr; + + addr = dev_get_addr(dev); + if (addr == FDT_ADDR_T_NONE) + return -EINVAL; + + plat->reg = (struct mxc_uart *)addr; + + plat->use_dte = fdtdec_get_bool(gd->fdt_blob, dev->of_offset, + "fsl,dte-mode"); + return 0; +} + +static const struct udevice_id mxc_serial_ids[] = { + { .compatible = "fsl,imx7d-uart" }, + { } +}; + U_BOOT_DRIVER(serial_mxc) = { .name = "serial_mxc", .id = UCLASS_SERIAL, + .of_match = mxc_serial_ids, + .ofdata_to_platdata = mxc_serial_ofdata_to_platdata, + .platdata_auto_alloc_size = sizeof(struct mxc_serial_platdata), .probe = mxc_serial_probe, .ops = &mxc_serial_ops, .flags = DM_FLAG_PRE_RELOC,