| Message ID | 1414036962-28463-7-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Hans de Goede |
| Headers | show |
Hi, On 10/23/2014 06:02 AM, Simon Glass wrote: > Add a driver for the designware serial UART used on sunxi. This just > redirects to the normal ns16550 driver. > > Add a stdout-path to the device tree so that the correct UART is chosen. > > Signed-off-by: Simon Glass <sjg@chromium.org> Looks good: Acked-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > > Changes in v2: > - Split non-sunxi patches into a separate dependent series > > arch/arm/dts/sun7i-a20-pcduino3.dts | 4 ++++ > drivers/serial/Makefile | 1 + > drivers/serial/serial_dw.c | 39 +++++++++++++++++++++++++++++++++++++ > include/configs/sun7i.h | 3 +++ > include/configs/sunxi-common.h | 12 +++++++----- > 5 files changed, 54 insertions(+), 5 deletions(-) > create mode 100644 drivers/serial/serial_dw.c > > diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts > index 046dfc0..f7cc8e7 100644 > --- a/arch/arm/dts/sun7i-a20-pcduino3.dts > +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts > @@ -20,6 +20,10 @@ > model = "LinkSprite pcDuino3"; > compatible = "linksprite,pcduino3", "allwinner,sun7i-a20"; > > + chosen { > + stdout-path = &uart0; > + }; > + > soc@01c00000 { > mmc0: mmc@01c0f000 { > pinctrl-names = "default"; > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile > index cd87d18..588573a 100644 > --- a/drivers/serial/Makefile > +++ b/drivers/serial/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_ALTERA_UART) += altera_uart.o > obj-$(CONFIG_ALTERA_JTAG_UART) += altera_jtag_uart.o > obj-$(CONFIG_ARM_DCC) += arm_dcc.o > obj-$(CONFIG_ATMEL_USART) += atmel_usart.o > +obj-$(CONFIG_DW_SERIAL) += serial_dw.o > obj-$(CONFIG_LPC32XX_HSUART) += lpc32xx_hsuart.o > obj-$(CONFIG_MCFUART) += mcfuart.o > obj-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o > diff --git a/drivers/serial/serial_dw.c b/drivers/serial/serial_dw.c > new file mode 100644 > index 0000000..a348f29 > --- /dev/null > +++ b/drivers/serial/serial_dw.c > @@ -0,0 +1,39 @@ > +/* > + * Copyright (c) 2014 Google, Inc > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <ns16550.h> > +#include <serial.h> > + > +static const struct udevice_id dw_serial_ids[] = { > + { .compatible = "snps,dw-apb-uart" }, > + { } > +}; > + > +static int dw_serial_ofdata_to_platdata(struct udevice *dev) > +{ > + struct ns16550_platdata *plat = dev_get_platdata(dev); > + int ret; > + > + ret = ns16550_serial_ofdata_to_platdata(dev); > + if (ret) > + return ret; > + plat->clock = CONFIG_SYS_NS16550_CLK; > + > + return 0; > +} > + > +U_BOOT_DRIVER(serial_ns16550) = { > + .name = "serial_dw", > + .id = UCLASS_SERIAL, > + .of_match = dw_serial_ids, > + .ofdata_to_platdata = dw_serial_ofdata_to_platdata, > + .platdata_auto_alloc_size = sizeof(struct ns16550_platdata), > + .priv_auto_alloc_size = sizeof(struct NS16550), > + .probe = ns16550_serial_probe, > + .ops = &ns16550_serial_ops, > +}; > diff --git a/include/configs/sun7i.h b/include/configs/sun7i.h > index 2314e97..108694a 100644 > --- a/include/configs/sun7i.h > +++ b/include/configs/sun7i.h > @@ -39,6 +39,9 @@ > #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM) > # define CONFIG_CMD_DM > # define CONFIG_DM_GPIO > +# define CONFIG_DM_SERIAL > +# define CONFIG_SYS_MALLOC_F_LEN (1 << 10) > +# define CONFIG_DW_SERIAL > #endif > > /* > diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h > index 1d947d7..e26bdf9 100644 > --- a/include/configs/sunxi-common.h > +++ b/include/configs/sunxi-common.h > @@ -36,12 +36,14 @@ > #define CONFIG_SYS_NS16550 > #define CONFIG_SYS_NS16550_SERIAL > /* ns16550 reg in the low bits of cpu reg */ > -#define CONFIG_SYS_NS16550_REG_SIZE -4 > #define CONFIG_SYS_NS16550_CLK 24000000 > -#define CONFIG_SYS_NS16550_COM1 SUNXI_UART0_BASE > -#define CONFIG_SYS_NS16550_COM2 SUNXI_UART1_BASE > -#define CONFIG_SYS_NS16550_COM3 SUNXI_UART2_BASE > -#define CONFIG_SYS_NS16550_COM4 SUNXI_UART3_BASE > +#ifndef CONFIG_DM_SERIAL > +# define CONFIG_SYS_NS16550_REG_SIZE -4 > +# define CONFIG_SYS_NS16550_COM1 SUNXI_UART0_BASE > +# define CONFIG_SYS_NS16550_COM2 SUNXI_UART1_BASE > +# define CONFIG_SYS_NS16550_COM3 SUNXI_UART2_BASE > +# define CONFIG_SYS_NS16550_COM4 SUNXI_UART3_BASE > +#endif > > /* DRAM Base */ > #define CONFIG_SYS_SDRAM_BASE 0x40000000 >
On Wed, 2014-10-22 at 22:02 -0600, Simon Glass wrote: > Add a driver for the designware serial UART used on sunxi. This just > redirects to the normal ns16550 driver. > > Add a stdout-path to the device tree so that the correct UART is chosen. > > Signed-off-by: Simon Glass <sjg@chromium.org> Should the UART related code in arch/arm/cpu/armv7/sunxi/board.c:gpio_init() be nobbled in this configuration? Ian.
Hi Ian, On 24 October 2014 03:42, Ian Campbell <ijc@hellion.org.uk> wrote: > On Wed, 2014-10-22 at 22:02 -0600, Simon Glass wrote: >> Add a driver for the designware serial UART used on sunxi. This just >> redirects to the normal ns16550 driver. >> >> Add a stdout-path to the device tree so that the correct UART is chosen. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> > > Should the UART related code in > arch/arm/cpu/armv7/sunxi/board.c:gpio_init() be nobbled in this > configuration? Yes I think that needs attention. Ideally we should be able to have this handled by the serial driver (requesting the pinmux it needs) but we don't have infrastructure for that as yet. Suggestions? What does the kernel do here? Regards, Simon
On Mon, 2014-10-27 at 18:06 -0600, Simon Glass wrote: > Hi Ian, > > On 24 October 2014 03:42, Ian Campbell <ijc@hellion.org.uk> wrote: > > On Wed, 2014-10-22 at 22:02 -0600, Simon Glass wrote: > >> Add a driver for the designware serial UART used on sunxi. This just > >> redirects to the normal ns16550 driver. > >> > >> Add a stdout-path to the device tree so that the correct UART is chosen. > >> > >> Signed-off-by: Simon Glass <sjg@chromium.org> > > > > Should the UART related code in > > arch/arm/cpu/armv7/sunxi/board.c:gpio_init() be nobbled in this > > configuration? > > Yes I think that needs attention. Ideally we should be able to have > this handled by the serial driver (requesting the pinmux it needs) but > we don't have infrastructure for that as yet. Ah, I thought your earlier GPIO patches were that, but I suppose not. > Suggestions? What does the kernel do here? It has pinmux infra. In the meantime could we somehow replace/augment the #ifdef chain in gpio_init with something keyed off the stdout alias perhaps? Ian.
Hi Ian, On 29 October 2014 02:05, Ian Campbell <ijc@hellion.org.uk> wrote: > On Mon, 2014-10-27 at 18:06 -0600, Simon Glass wrote: >> Hi Ian, >> >> On 24 October 2014 03:42, Ian Campbell <ijc@hellion.org.uk> wrote: >> > On Wed, 2014-10-22 at 22:02 -0600, Simon Glass wrote: >> >> Add a driver for the designware serial UART used on sunxi. This just >> >> redirects to the normal ns16550 driver. >> >> >> >> Add a stdout-path to the device tree so that the correct UART is chosen. >> >> >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> > >> > Should the UART related code in >> > arch/arm/cpu/armv7/sunxi/board.c:gpio_init() be nobbled in this >> > configuration? >> >> Yes I think that needs attention. Ideally we should be able to have >> this handled by the serial driver (requesting the pinmux it needs) but >> we don't have infrastructure for that as yet. > > Ah, I thought your earlier GPIO patches were that, but I suppose not. > >> Suggestions? What does the kernel do here? > > It has pinmux infra. > > In the meantime could we somehow replace/augment the #ifdef chain in > gpio_init with something keyed off the stdout alias perhaps? Tegra has code to convert a device interrupt number (which uniquely identifies a peripheral in that SoC) to an internal peripheral ID, then these is a function which can enable a peripheral given the ID (funcmux). In some cases you could have multiple options for the funcmux, but there is no easy way to support this. But this approach might be good enough for sunxi. We can easily write the function to enable the pins for a particular port, and this could go in arch/arm/...sunxi/ perhaps. Regards, Simon
On Wed, 2014-10-29 at 13:28 -0600, Simon Glass wrote: > > In the meantime could we somehow replace/augment the #ifdef chain in > > gpio_init with something keyed off the stdout alias perhaps? > > Tegra has code to convert a device interrupt number (which uniquely > identifies a peripheral in that SoC) to an internal peripheral ID, > then these is a function which can enable a peripheral given the ID > (funcmux). In some cases you could have multiple options for the > funcmux, but there is no easy way to support this. I think that although there are multiple options for some functions (UARTs come to mind) we haven't yet found the need to make any dynamic choices, so it's all static right now. > But this approach > might be good enough for sunxi. We can easily write the function to > enable the pins for a particular port, and this could go in > arch/arm/...sunxi/ perhaps. I'm ok with it so long as it isn't going to stand in the way of proper dt based pinmux in the future. One way to help with that might be to use the allwinner,function property in DT as the funcmux name. Hans, what do you think? Ian.
Hi, On 10/30/2014 10:08 AM, Ian Campbell wrote: > On Wed, 2014-10-29 at 13:28 -0600, Simon Glass wrote: >>> In the meantime could we somehow replace/augment the #ifdef chain in >>> gpio_init with something keyed off the stdout alias perhaps? >> >> Tegra has code to convert a device interrupt number (which uniquely >> identifies a peripheral in that SoC) to an internal peripheral ID, >> then these is a function which can enable a peripheral given the ID >> (funcmux). In some cases you could have multiple options for the >> funcmux, but there is no easy way to support this. > > I think that although there are multiple options for some functions > (UARTs come to mind) we haven't yet found the need to make any dynamic > choices, so it's all static right now. > >> But this approach >> might be good enough for sunxi. We can easily write the function to >> enable the pins for a particular port, and this could go in >> arch/arm/...sunxi/ perhaps. > > I'm ok with it so long as it isn't going to stand in the way of proper > dt based pinmux in the future. > > One way to help with that might be to use the allwinner,function > property in DT as the funcmux name. > > Hans, what do you think? I'm not 100% sure what you're suggesting here, are you suggesting to have a 1:1 mapping between function names as stored in allwinner,function in dts and the value to pass to sunxi_gpio_set_cfgpin ? This is not going to fly very far, e.g. the "uart0" function has cfg value of 2 on portb while it has a value of 4 on portf. So we will really need a sunxi specific function to go from port-no + allwinner,function-string to a sunxi_gpio_set_cfgpin function. I think the best thing we can do to not make this function too big is do 3 things: 1) Have a table which only contains mapping where the cfg value is not 2, most ipblocks have a "primary" gpio port they are intended to be used with, and this usage maps to a cfg value of 2, and most designs use this, so have a list of exceptions and return 2 otherwise, this will make debugging of issues where the mux is not setup a bit harder, but it will keep things a lot smaller. 2) Only but things in there which are actually used by boards, or we expect to need in the near future. 3) Have #ifndef CONFIG_SPL_BUILD .. #endif around anything but mmc / uart entries in that table. Together those should keep things small enough for the SPL. Regards, Hans
On Thu, 2014-10-30 at 10:36 +0100, Hans de Goede wrote: > Hi, > > On 10/30/2014 10:08 AM, Ian Campbell wrote: > > On Wed, 2014-10-29 at 13:28 -0600, Simon Glass wrote: > >>> In the meantime could we somehow replace/augment the #ifdef chain in > >>> gpio_init with something keyed off the stdout alias perhaps? > >> > >> Tegra has code to convert a device interrupt number (which uniquely > >> identifies a peripheral in that SoC) to an internal peripheral ID, > >> then these is a function which can enable a peripheral given the ID > >> (funcmux). In some cases you could have multiple options for the > >> funcmux, but there is no easy way to support this. > > > > I think that although there are multiple options for some functions > > (UARTs come to mind) we haven't yet found the need to make any dynamic > > choices, so it's all static right now. > > > >> But this approach > >> might be good enough for sunxi. We can easily write the function to > >> enable the pins for a particular port, and this could go in > >> arch/arm/...sunxi/ perhaps. > > > > I'm ok with it so long as it isn't going to stand in the way of proper > > dt based pinmux in the future. > > > > One way to help with that might be to use the allwinner,function > > property in DT as the funcmux name. > > > > Hans, what do you think? > > I'm not 100% sure what you're suggesting here, are you suggesting to > have a 1:1 mapping between function names as stored in allwinner,function > in dts and the value to pass to sunxi_gpio_set_cfgpin ? I was imagining a function which would take the string "uart0" and would call sunxi_gpio_set_cfgpin with whatever values that would entail in order to make uart0 work, not one which would try and return something that the caller would then use. > This is not going to fly very far, e.g. the "uart0" function has cfg value > of 2 on portb while it has a value of 4 on portf. I believe we currently statically use either portb or portf (I've not looked up which, IIRC it changed recently, but I don't recall which way), so my proposed function would just DTRT. Of course if we ever find we need something more dynamic then we would have to do a proper pinmux implementation (or at least something closer to a proper one) Ian.
Hi Ian, On 30 October 2014 04:14, Ian Campbell <ijc@hellion.org.uk> wrote: > On Thu, 2014-10-30 at 10:36 +0100, Hans de Goede wrote: >> Hi, >> >> On 10/30/2014 10:08 AM, Ian Campbell wrote: >> > On Wed, 2014-10-29 at 13:28 -0600, Simon Glass wrote: >> >>> In the meantime could we somehow replace/augment the #ifdef chain in >> >>> gpio_init with something keyed off the stdout alias perhaps? >> >> >> >> Tegra has code to convert a device interrupt number (which uniquely >> >> identifies a peripheral in that SoC) to an internal peripheral ID, >> >> then these is a function which can enable a peripheral given the ID >> >> (funcmux). In some cases you could have multiple options for the >> >> funcmux, but there is no easy way to support this. >> > >> > I think that although there are multiple options for some functions >> > (UARTs come to mind) we haven't yet found the need to make any dynamic >> > choices, so it's all static right now. >> > >> >> But this approach >> >> might be good enough for sunxi. We can easily write the function to >> >> enable the pins for a particular port, and this could go in >> >> arch/arm/...sunxi/ perhaps. >> > >> > I'm ok with it so long as it isn't going to stand in the way of proper >> > dt based pinmux in the future. >> > >> > One way to help with that might be to use the allwinner,function >> > property in DT as the funcmux name. >> > >> > Hans, what do you think? >> >> I'm not 100% sure what you're suggesting here, are you suggesting to >> have a 1:1 mapping between function names as stored in allwinner,function >> in dts and the value to pass to sunxi_gpio_set_cfgpin ? > > I was imagining a function which would take the string "uart0" and would > call sunxi_gpio_set_cfgpin with whatever values that would entail in > order to make uart0 work, not one which would try and return something > that the caller would then use. If you only have a few pins that uart0 can appear on, then you could pass a parameter telling the function which combination to use. I'm not sure about passing a string for the uart0 - would not an enum defined globally for sunix not be better? > >> This is not going to fly very far, e.g. the "uart0" function has cfg value >> of 2 on portb while it has a value of 4 on portf. > > I believe we currently statically use either portb or portf (I've not > looked up which, IIRC it changed recently, but I don't recall which > way), so my proposed function would just DTRT. Of course if we ever find > we need something more dynamic then we would have to do a proper pinmux > implementation (or at least something closer to a proper one) SGTM. Regards, Simon
Hi Ian, On 30 October 2014 04:14, Ian Campbell <ijc@hellion.org.uk> wrote: > On Thu, 2014-10-30 at 10:36 +0100, Hans de Goede wrote: >> Hi, >> >> On 10/30/2014 10:08 AM, Ian Campbell wrote: >> > On Wed, 2014-10-29 at 13:28 -0600, Simon Glass wrote: >> >>> In the meantime could we somehow replace/augment the #ifdef chain in >> >>> gpio_init with something keyed off the stdout alias perhaps? >> >> >> >> Tegra has code to convert a device interrupt number (which uniquely >> >> identifies a peripheral in that SoC) to an internal peripheral ID, >> >> then these is a function which can enable a peripheral given the ID >> >> (funcmux). In some cases you could have multiple options for the >> >> funcmux, but there is no easy way to support this. >> > >> > I think that although there are multiple options for some functions >> > (UARTs come to mind) we haven't yet found the need to make any dynamic >> > choices, so it's all static right now. >> > >> >> But this approach >> >> might be good enough for sunxi. We can easily write the function to >> >> enable the pins for a particular port, and this could go in >> >> arch/arm/...sunxi/ perhaps. >> > >> > I'm ok with it so long as it isn't going to stand in the way of proper >> > dt based pinmux in the future. >> > >> > One way to help with that might be to use the allwinner,function >> > property in DT as the funcmux name. >> > >> > Hans, what do you think? >> >> I'm not 100% sure what you're suggesting here, are you suggesting to >> have a 1:1 mapping between function names as stored in allwinner,function >> in dts and the value to pass to sunxi_gpio_set_cfgpin ? > > I was imagining a function which would take the string "uart0" and would > call sunxi_gpio_set_cfgpin with whatever values that would entail in > order to make uart0 work, not one which would try and return something > that the caller would then use. I assume that it will take a string, e.g. "uart0" and a pin, since uart0 can be routed to either porta or portb, other then that having the function directly call sunxi_gpio_set_cfgpin rather then returning the value to pass to sunxi_gpio_set_cfgpin is a good idea. > >> This is not going to fly very far, e.g. the "uart0" function has cfg value >> of 2 on portb while it has a value of 4 on portf. > > I believe we currently statically use either portb or portf (I've not > looked up which, IIRC it changed recently, but I don't recall which > way), so my proposed function would just DTRT. Of course if we ever find > we need something more dynamic then we would have to do a proper pinmux > implementation (or at least something closer to a proper one) Ah, so you mainly just want to clean up the existing #ifdef mess ? I was aiming for something which we could eventually use to get the info from devicetree and not have any uart info hardcoded into the binaries at all. Regards, Hans
On Fri, 2014-10-31 at 10:07 +0100, Hans de Goede wrote: > Hi Ian, > > On 30 October 2014 04:14, Ian Campbell <ijc@hellion.org.uk> wrote: > > On Thu, 2014-10-30 at 10:36 +0100, Hans de Goede wrote: > >> Hi, > >> > >> On 10/30/2014 10:08 AM, Ian Campbell wrote: > >> > On Wed, 2014-10-29 at 13:28 -0600, Simon Glass wrote: > >> >>> In the meantime could we somehow replace/augment the #ifdef chain in > >> >>> gpio_init with something keyed off the stdout alias perhaps? > >> >> > >> >> Tegra has code to convert a device interrupt number (which uniquely > >> >> identifies a peripheral in that SoC) to an internal peripheral ID, > >> >> then these is a function which can enable a peripheral given the ID > >> >> (funcmux). In some cases you could have multiple options for the > >> >> funcmux, but there is no easy way to support this. > >> > > >> > I think that although there are multiple options for some functions > >> > (UARTs come to mind) we haven't yet found the need to make any dynamic > >> > choices, so it's all static right now. > >> > > >> >> But this approach > >> >> might be good enough for sunxi. We can easily write the function to > >> >> enable the pins for a particular port, and this could go in > >> >> arch/arm/...sunxi/ perhaps. > >> > > >> > I'm ok with it so long as it isn't going to stand in the way of proper > >> > dt based pinmux in the future. > >> > > >> > One way to help with that might be to use the allwinner,function > >> > property in DT as the funcmux name. > >> > > >> > Hans, what do you think? > >> > >> I'm not 100% sure what you're suggesting here, are you suggesting to > >> have a 1:1 mapping between function names as stored in allwinner,function > >> in dts and the value to pass to sunxi_gpio_set_cfgpin ? > > > > I was imagining a function which would take the string "uart0" and would > > call sunxi_gpio_set_cfgpin with whatever values that would entail in > > order to make uart0 work, not one which would try and return something > > that the caller would then use. > > I assume that it will take a string, e.g. "uart0" and a pin, since > uart0 can be routed to either porta or portb, other then that having the > function directly call sunxi_gpio_set_cfgpin rather then returning the > value to pass to sunxi_gpio_set_cfgpin is a good idea. Actually right now we don't actually dynamically select anything for uarts, so we could just as easily hardcode which pins to use in this new function as we do now, it's still a step in the right direction. > >> This is not going to fly very far, e.g. the "uart0" function has cfg value > >> of 2 on portb while it has a value of 4 on portf. > > > > I believe we currently statically use either portb or portf (I've not > > looked up which, IIRC it changed recently, but I don't recall which > > way), so my proposed function would just DTRT. Of course if we ever find > > we need something more dynamic then we would have to do a proper pinmux > > implementation (or at least something closer to a proper one) > > Ah, so you mainly just want to clean up the existing #ifdef mess ? I was aiming > for something which we could eventually use to get the info from devicetree > and not have any uart info hardcoded into the binaries at all. What I'm really hoping for is to enable Simon to get his DM series accepted, but in a way which won't get in the way of future work to use DT fully. Even better if it takes us a little nearer to the full DT path, at least in terms of the interfaces used. This subthreaded initially started with the suggestion from Simon: "We can easily write the function to enable the pins for a particular port, and this could go in arch/arm/...sunxi/ perhaps.". Which sounded OK to me so long as it doesn't get in the way of future work to fully use DT. So with that in mind I suggested that using the DT function name as the key passed to that function would help achieve that aim. My thinking was that this function would be a nexus point where we could independently replace the callers (individually on a driver by driver basis) with code parsing the DT to find the function name and the backend with code to lookup the correct pinmux stuff in DT and do the necessary setup. Maybe I've misjudged what the final DT pinmux thing would look like though, in which case maybe this suggestion doesn't actually achieve the aim of not getting in the way. Ian.
Hi, On 10/31/2014 10:30 AM, Ian Campbell wrote: > On Fri, 2014-10-31 at 10:07 +0100, Hans de Goede wrote: >> Hi Ian, >> >> On 30 October 2014 04:14, Ian Campbell <ijc@hellion.org.uk> wrote: >>> On Thu, 2014-10-30 at 10:36 +0100, Hans de Goede wrote: >>>> Hi, >>>> >>>> On 10/30/2014 10:08 AM, Ian Campbell wrote: >>>>> On Wed, 2014-10-29 at 13:28 -0600, Simon Glass wrote: >>>>>>> In the meantime could we somehow replace/augment the #ifdef chain in >>>>>>> gpio_init with something keyed off the stdout alias perhaps? >>>>>> >>>>>> Tegra has code to convert a device interrupt number (which uniquely >>>>>> identifies a peripheral in that SoC) to an internal peripheral ID, >>>>>> then these is a function which can enable a peripheral given the ID >>>>>> (funcmux). In some cases you could have multiple options for the >>>>>> funcmux, but there is no easy way to support this. >>>>> >>>>> I think that although there are multiple options for some functions >>>>> (UARTs come to mind) we haven't yet found the need to make any dynamic >>>>> choices, so it's all static right now. >>>>> >>>>>> But this approach >>>>>> might be good enough for sunxi. We can easily write the function to >>>>>> enable the pins for a particular port, and this could go in >>>>>> arch/arm/...sunxi/ perhaps. >>>>> >>>>> I'm ok with it so long as it isn't going to stand in the way of proper >>>>> dt based pinmux in the future. >>>>> >>>>> One way to help with that might be to use the allwinner,function >>>>> property in DT as the funcmux name. >>>>> >>>>> Hans, what do you think? >>>> >>>> I'm not 100% sure what you're suggesting here, are you suggesting to >>>> have a 1:1 mapping between function names as stored in allwinner,function >>>> in dts and the value to pass to sunxi_gpio_set_cfgpin ? >>> >>> I was imagining a function which would take the string "uart0" and would >>> call sunxi_gpio_set_cfgpin with whatever values that would entail in >>> order to make uart0 work, not one which would try and return something >>> that the caller would then use. >> >> I assume that it will take a string, e.g. "uart0" and a pin, since >> uart0 can be routed to either porta or portb, other then that having the >> function directly call sunxi_gpio_set_cfgpin rather then returning the >> value to pass to sunxi_gpio_set_cfgpin is a good idea. > > Actually right now we don't actually dynamically select anything for > uarts, so we could just as easily hardcode which pins to use in this new > function as we do now, it's still a step in the right direction. > >>>> This is not going to fly very far, e.g. the "uart0" function has cfg value >>>> of 2 on portb while it has a value of 4 on portf. >>> >>> I believe we currently statically use either portb or portf (I've not >>> looked up which, IIRC it changed recently, but I don't recall which >>> way), so my proposed function would just DTRT. Of course if we ever find >>> we need something more dynamic then we would have to do a proper pinmux >>> implementation (or at least something closer to a proper one) >> >> Ah, so you mainly just want to clean up the existing #ifdef mess ? I was aiming >> for something which we could eventually use to get the info from devicetree >> and not have any uart info hardcoded into the binaries at all. > > What I'm really hoping for is to enable Simon to get his DM series > accepted, but in a way which won't get in the way of future work to use > DT fully. Even better if it takes us a little nearer to the full DT > path, at least in terms of the interfaces used. Ah, but I plan to merge the v3 Simon has posted to u-boot-sunxi/next and then do a pull-req with that in there this weekend. IOW we don't need to solve the pinmux problem for that series to get merged (from my pov). So maybe we should just delay dealing with the pinmux issue until we really need to ? Regards, Hans
On Fri, 2014-10-31 at 10:33 +0100, Hans de Goede wrote: > Ah, but I plan to merge the v3 Simon has posted to u-boot-sunxi/next and then > do a pull-req with that in there this weekend. IOW we don't need to solve > the pinmux problem for that series to get merged (from my pov). So maybe we > should just delay dealing with the pinmux issue until we really need to ? Does it work without having done *something* about this? AFAICT it currently relies on the stdout-path in the DT precisely matching CONFIG_CONS_INDEX (due to its affect on the code in gpio_init()), doesn't it? Ian.
diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts index 046dfc0..f7cc8e7 100644 --- a/arch/arm/dts/sun7i-a20-pcduino3.dts +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts @@ -20,6 +20,10 @@ model = "LinkSprite pcDuino3"; compatible = "linksprite,pcduino3", "allwinner,sun7i-a20"; + chosen { + stdout-path = &uart0; + }; + soc@01c00000 { mmc0: mmc@01c0f000 { pinctrl-names = "default"; diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile index cd87d18..588573a 100644 --- a/drivers/serial/Makefile +++ b/drivers/serial/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_ALTERA_UART) += altera_uart.o obj-$(CONFIG_ALTERA_JTAG_UART) += altera_jtag_uart.o obj-$(CONFIG_ARM_DCC) += arm_dcc.o obj-$(CONFIG_ATMEL_USART) += atmel_usart.o +obj-$(CONFIG_DW_SERIAL) += serial_dw.o obj-$(CONFIG_LPC32XX_HSUART) += lpc32xx_hsuart.o obj-$(CONFIG_MCFUART) += mcfuart.o obj-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o diff --git a/drivers/serial/serial_dw.c b/drivers/serial/serial_dw.c new file mode 100644 index 0000000..a348f29 --- /dev/null +++ b/drivers/serial/serial_dw.c @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2014 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <ns16550.h> +#include <serial.h> + +static const struct udevice_id dw_serial_ids[] = { + { .compatible = "snps,dw-apb-uart" }, + { } +}; + +static int dw_serial_ofdata_to_platdata(struct udevice *dev) +{ + struct ns16550_platdata *plat = dev_get_platdata(dev); + int ret; + + ret = ns16550_serial_ofdata_to_platdata(dev); + if (ret) + return ret; + plat->clock = CONFIG_SYS_NS16550_CLK; + + return 0; +} + +U_BOOT_DRIVER(serial_ns16550) = { + .name = "serial_dw", + .id = UCLASS_SERIAL, + .of_match = dw_serial_ids, + .ofdata_to_platdata = dw_serial_ofdata_to_platdata, + .platdata_auto_alloc_size = sizeof(struct ns16550_platdata), + .priv_auto_alloc_size = sizeof(struct NS16550), + .probe = ns16550_serial_probe, + .ops = &ns16550_serial_ops, +}; diff --git a/include/configs/sun7i.h b/include/configs/sun7i.h index 2314e97..108694a 100644 --- a/include/configs/sun7i.h +++ b/include/configs/sun7i.h @@ -39,6 +39,9 @@ #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM) # define CONFIG_CMD_DM # define CONFIG_DM_GPIO +# define CONFIG_DM_SERIAL +# define CONFIG_SYS_MALLOC_F_LEN (1 << 10) +# define CONFIG_DW_SERIAL #endif /* diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 1d947d7..e26bdf9 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -36,12 +36,14 @@ #define CONFIG_SYS_NS16550 #define CONFIG_SYS_NS16550_SERIAL /* ns16550 reg in the low bits of cpu reg */ -#define CONFIG_SYS_NS16550_REG_SIZE -4 #define CONFIG_SYS_NS16550_CLK 24000000 -#define CONFIG_SYS_NS16550_COM1 SUNXI_UART0_BASE -#define CONFIG_SYS_NS16550_COM2 SUNXI_UART1_BASE -#define CONFIG_SYS_NS16550_COM3 SUNXI_UART2_BASE -#define CONFIG_SYS_NS16550_COM4 SUNXI_UART3_BASE +#ifndef CONFIG_DM_SERIAL +# define CONFIG_SYS_NS16550_REG_SIZE -4 +# define CONFIG_SYS_NS16550_COM1 SUNXI_UART0_BASE +# define CONFIG_SYS_NS16550_COM2 SUNXI_UART1_BASE +# define CONFIG_SYS_NS16550_COM3 SUNXI_UART2_BASE +# define CONFIG_SYS_NS16550_COM4 SUNXI_UART3_BASE +#endif /* DRAM Base */ #define CONFIG_SYS_SDRAM_BASE 0x40000000
Add a driver for the designware serial UART used on sunxi. This just redirects to the normal ns16550 driver. Add a stdout-path to the device tree so that the correct UART is chosen. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: - Split non-sunxi patches into a separate dependent series arch/arm/dts/sun7i-a20-pcduino3.dts | 4 ++++ drivers/serial/Makefile | 1 + drivers/serial/serial_dw.c | 39 +++++++++++++++++++++++++++++++++++++ include/configs/sun7i.h | 3 +++ include/configs/sunxi-common.h | 12 +++++++----- 5 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 drivers/serial/serial_dw.c