8250: Add driver for Broadcom UART
Al Cooper
maintainerv4 cover 8250: Add driver for Broadcom UART
v5 - Changes based on requests from Greg Kroah-Hartman
- Move sysfs stats to debugfs.
- Merge the .h file into the .c file.
- Convert "flags" with only one flag to bool
- Remove most dev_dbg() messages. Some can be replaced by using ftrace and some would cause a crash if the driver was used for the console.
v4 - Fix MAINTAINERS
- Make all changes requested by Jira Slaby. The only functional change was to handle errors returned by brcmuart_arbitration() in brcmuart_resume().
v3 - remove "disable_dma" module param because it can be done
by modifying the device tree node instead. Reduce size by removing some debug fuctionality that was no longer used.
- Fix error from yaml compiler in bindings
v2 - remove the patch that modified 8250_of.c to keep it from
registering before this driver when this driver was deferred as it was getting it's "clocks". This was fixed by changing the Device Tree entry to remove "clock-frequency". This results in both drivers getting "clocks" and getting same the deferral.
Al Cooper (2):
dt-bindings: Add support for the Broadcom UART driver
serial: 8250: Add new 8250-core based Broadcom STB driver
.../bindings/serial/brcm,bcm7271-uart.yaml | 96 ++ MAINTAINERS | 8 + drivers/tty/serial/8250/8250_bcm7271.c | 1207 +++++++++++++++++ drivers/tty/serial/8250/Kconfig | 10 + drivers/tty/serial/8250/Makefile | 1 + 5 files changed, 1322 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/brcm,bcm7271-uart.yaml create mode 100644 drivers/tty/serial/8250/8250_bcm7271.c
Signature · 1 line
2.17.1
Al Cooper
maintainerPATCH v4 1/2 dt-bindings: Add support for the Broadcom UART driver
Add DT bindings for the Broadcom 8250 based UART driver. This UART is based on an 8250 but adds additional functionality. The additional features include the ability to use DMA for transfers and a baud rate clock system that is more accurate at high baud rates. This UART is backward compatible with the standard 8250 UART.
--- .../bindings/serial/brcm,bcm7271-uart.yaml | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/brcm,bcm7271-uart.yaml
Documentation/devicetree/bindings/serial/brcm,bcm7271-uart.yaml new file +96 -0
@@ -0,0 +1,96 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/serial/brcm,bcm7271-uart.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Broadcom 8250 based serial port devicetree bindings + +maintainers: + - Al Cooper <[email protected]> + +description: |+ + The Broadcom UART is based on the basic 8250 UART but with + enhancements for more accurate high speed baud rates and support + for DMA. + +properties: + $nodename: + pattern: "^serial@[0-9a-f]+$" + + compatible: + items: + - enum: + - brcm,bcm7271-uart + - brcm,bcm7278-uart + reg: + minItems: 1 + maxItems: 5 + + reg-names: + description: The UART register block and optionally the DMA register blocks. + oneOf: + - items: + - const: uart + - items: + - const: uart + - const: dma_arb + - const: dma_rx + - const: dma_tx + - const: dma_intr2 + + clocks: + minItems: 1 + + clock-names: + const: sw_baud + + interrupts: + minItems: 1 + maxItems: 2 + + interrupt-names: + description: The UART interrupt and optionally the DMA interrupt. + oneOf: + - items: + - const: uart + - items: + - const: uart + - const: dma + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - interrupts + - interrupt-names + +additionalProperties: false + +examples: + - | + serial@840d000 { + compatible = "brcm,bcm7271-uart"; + reg = <0x840d000 0x20>; + reg-names = "uart"; + interrupts = <0x0 0x62 0x4>; + interrupt-names = "uart"; + clocks = <&scmi_clk 190>; + clock-names = "sw_baud"; + }; + + serial@840e000 { + compatible = "brcm,bcm7271-uart"; + reg = <0x840e000 0x20>, + <0x840e080 0x8>, + <0x840e100 0xa8>, + <0x840e200 0x4c>, + <0x840e300 0x30>; + reg-names = "uart", "dma_arb", "dma_rx", "dma_tx", "dma_intr2"; + interrupts = <0x0 0x62 0x4>, <0x0 0x75 0x4>; + interrupt-names = "uart", "dma"; + clocks = <&scmi_clk 190>; + clock-names = "sw_baud"; + };
Signature · 1 line
2.17.1
Rob Herring
maintainerRe: [PATCH v4 1/2] dt-bindings: Add support for the Broadcom UART driver
Quoted 5 lines · Al Cooper
Add DT bindings for the Broadcom 8250 based UART driver. This UART is based on an 8250 but adds additional functionality. The additional features include the ability to use DMA for transfers and a baud rate clock system that is more accurate at high baud rates. This UART is backward compatible with the standard 8250 UART.
My comments on the previous v4 still apply.
Quoted 112 lines
Signed-off-by Al Cooper <[email protected]>--- .../bindings/serial/brcm,bcm7271-uart.yaml | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 Documentation/devicetree/bindings/serial/brcm,bcm7271-uart.yamlDocumentation/devicetree/bindings/serial/brcm,bcm7271-uart.yaml new file +96
-0@@ -0,0 +1,96 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/serial/brcm,bcm7271-uart.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Broadcom 8250 based serial port devicetree bindings + +maintainers: + - Al Cooper <[email protected]> + +description: |+ + The Broadcom UART is based on the basic 8250 UART but with + enhancements for more accurate high speed baud rates and support + for DMA. + +properties: + $nodename: + pattern: "^serial@[0-9a-f]+$" + + compatible: + items: + - enum: + - brcm,bcm7271-uart + - brcm,bcm7278-uart + reg: + minItems: 1 + maxItems: 5 + + reg-names: + description: The UART register block and optionally the DMA register blocks. + oneOf: + - items: + - const: uart + - items: + - const: uart + - const: dma_arb + - const: dma_rx + - const: dma_tx + - const: dma_intr2 + + clocks: + minItems: 1 + + clock-names: + const: sw_baud + + interrupts: + minItems: 1 + maxItems: 2 + + interrupt-names: + description: The UART interrupt and optionally the DMA interrupt. + oneOf: + - items: + - const: uart + - items: + - const: uart + - const: dma + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - interrupts + - interrupt-names + +additionalProperties: false + +examples: + - | + serial@840d000 { + compatible = "brcm,bcm7271-uart"; + reg = <0x840d000 0x20>; + reg-names = "uart"; + interrupts = <0x0 0x62 0x4>; + interrupt-names = "uart"; + clocks = <&scmi_clk 190>; + clock-names = "sw_baud"; + }; + + serial@840e000 { + compatible = "brcm,bcm7271-uart"; + reg = <0x840e000 0x20>, + <0x840e080 0x8>, + <0x840e100 0xa8>, + <0x840e200 0x4c>, + <0x840e300 0x30>; + reg-names = "uart", "dma_arb", "dma_rx", "dma_tx", "dma_intr2"; + interrupts = <0x0 0x62 0x4>, <0x0 0x75 0x4>; + interrupt-names = "uart", "dma"; + clocks = <&scmi_clk 190>; + clock-names = "sw_baud"; + };Signature · 1 line
2.17.1
Al Cooper
maintainerPATCH v4 2/2 serial: 8250: Add new 8250-core based Broadcom STB driver
In reply to Al CooperAdd a UART driver for the new Broadcom 8250 based STB UART. The new UART is backward compatible with the standard 8250, but has some additional features. The new features include a high accuracy baud rate clock system and DMA support.
The driver will use the new optional BAUD MUX clock to select the best one of the four master clocks (81MHz, 108MHz, 64MHz and 48MHz) to feed the baud rate selection logic for any requested baud rate. This allows for more accurate BAUD rates when high speed baud rates are selected.
The driver will use the new UART DMA hardware if the UART DMA registers are specified in Device Tree "reg" property.
The driver also sets the UPSTAT_AUTOCTS flag when hardware flow control is enabled. This flag is needed for UARTs that don't assert a CTS changed interrupt when CTS changes and AFE (Hardware Flow Control) is enabled.
The driver also contains a workaround for a bug in the Synopsis 8250 core. The problem is that at high baud rates, the RX partial FIFO timeout interrupt can occur but there is no RX data (DR not set in the LSR register). In this case the driver will not read the Receive Buffer Register, which clears the interrupt, and the system will get continuous UART interrupts until the next RX character arrives. The fix originally suggested by Synopsis was to read the Receive Buffer Register and discard the character when the DR bit in the LSR was not set, to clear the interrupt. The problem was that occasionally a character would arrive just after the DR bit check and a valid character would be discarded. The fix that was added will clear receive interrupts to stop the interrupt, deassert RTS to insure that no new data can arrive, wait for 1.5 character times for the sender to react to RTS and then check for data and either do a dummy read or a valid read. Debugfs error counters were also added and were used to help create test software that would cause the error condition. The counters can be found at:
/sys/kernel/debug/bcm7271-uart/<device-name>/stats
--- MAINTAINERS | 8 + drivers/tty/serial/8250/8250_bcm7271.c | 1207 ++++++++++++++++++++++++ drivers/tty/serial/8250/Kconfig | 10 + drivers/tty/serial/8250/Makefile | 1 + 4 files changed, 1226 insertions(+) create mode 100644 drivers/tty/serial/8250/8250_bcm7271.c
MAINTAINERS +8 -0
@@ -3558,6 +3558,14 @@ S: Supported F: Documentation/devicetree/bindings/i2c/brcm,brcmstb-i2c.yaml F: drivers/i2c/busses/i2c-brcmstb.c +BROADCOM BRCMSTB UART DRIVER +M: Al Cooper <[email protected]> +L: [email protected] +L: [email protected] +S: Maintained +F: Documentation/devicetree/bindings/serial/brcm,bcm7271-uart.yaml +F: drivers/tty/serial/8250/8250_bcm7271.c + BROADCOM BRCMSTB USB EHCI DRIVER M: Al Cooper <[email protected]> L: [email protected]
drivers/tty/serial/8250/8250_bcm7271.c new file +1207 -0
1,207 lines not on this page · raw message
drivers/tty/serial/8250/Kconfig +10 -0
@@ -510,6 +510,16 @@ config SERIAL_8250_TEGRA Select this option if you have machine with an NVIDIA Tegra SoC and wish to enable 8250 serial driver for the Tegra serial interfaces. +config SERIAL_8250_BCM7271 + bool "Broadcom 8250 based serial port" + depends on SERIAL_8250 && (ARCH_BRCMSTB || COMPILE_TEST) + default ARCH_BRCMSTB + help + If you have a Broadcom STB based board and want to use the + enhanced features of the Broadcom 8250 based serial port, + including DMA support and high accuracy BAUD rates, say + Y to this option. If unsure, say N. + config SERIAL_OF_PLATFORM tristate "Devicetree based probing for 8250 ports" depends on SERIAL_8250 && OF
drivers/tty/serial/8250/Makefile +1 -0
@@ -38,6 +38,7 @@ obj-$(CONFIG_SERIAL_8250_LPSS) += 8250_lpss.o obj-$(CONFIG_SERIAL_8250_MID) += 8250_mid.o obj-$(CONFIG_SERIAL_8250_PXA) += 8250_pxa.o obj-$(CONFIG_SERIAL_8250_TEGRA) += 8250_tegra.o +obj-$(CONFIG_SERIAL_8250_BCM7271) += 8250_bcm7271.o obj-$(CONFIG_SERIAL_OF_PLATFORM) += 8250_of.o CFLAGS_8250_ingenic.o += -I$(srctree)/scripts/dtc/libfdt
Signature · 1 line
2.17.1
Greg Kroah-Hartman
maintainerRe: [PATCH v4 2/2] serial: 8250: Add new 8250-core based Broadcom STB driver
drivers/tty/serial/8250/8250_bcm7271.c line 934
Quoted 8 lines · Al Cooper
drivers/tty/serial/8250/8250_bcm7271.c new file +8
-0+static void brcmuart_init_debugfs (struct brcmuart_priv *priv, +const char *device) +{ + priv->debugfs_dir =debugfs_create_dir (device, brcmuart_debugfs_root); + priv->debugfs_file =debugfs_create_file ("stats" ,0444 , + priv->debugfs_dir, + priv, + &debugfs_stats_fops);
No need to save this file *, you never do anything with it.
Quoted 85 lines
drivers/tty/serial/8250/8250_bcm7271.c new file +85
-0+} + + +static int brcmuart_probe (struct platform_device *pdev) +{ +struct resource *regs; +struct device_node *np = pdev->dev.of_node; +const struct of_device_id *of_id =NULL ; +struct uart_8250_port *new_port; +struct device *dev = &pdev->dev; +struct brcmuart_priv *priv; +struct clk *baud_mux_clk; +struct uart_8250_port up; +struct resource *irq; +void __iomem *membase =0 ; +resource_size_t mapbase =0 ; +u32 clk_rate =0 ; +int ret; +int x; +int dma_irq; +static const char *const reg_names[REGS_MAX ] = { +"uart" ,"dma_rx" ,"dma_tx" ,"dma_intr2" ,"dma_arb" + }; + + irq =platform_get_resource (pdev,IORESOURCE_IRQ ,0 ); +if (!irq) { +dev_err (dev,"missing irq\n" ); +return -EINVAL ; + } + priv =devm_kzalloc (dev,sizeof (struct brcmuart_priv ), +GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + + of_id =of_match_node (brcmuart_dt_ids, np); +if (!of_id || !of_id->data) + priv->rate_table = brcmstb_rate_table; +else + priv->rate_table = of_id->data; + +for (x =0 ; x <REGS_MAX ; x++) { + regs =platform_get_resource_byname (pdev,IORESOURCE_MEM , + reg_names[x]); +if (!regs) +break ; + priv->regs[x] =devm_ioremap (dev, regs->start, +resource_size (regs)); +if (IS_ERR (priv->regs[x])) +return PTR_ERR (priv->regs[x]); +if (x ==REGS_8250 ) { + mapbase = regs->start; + membase = priv->regs[x]; + } + } + +/* We should have just the uart base registers or all the registers */ +if (x !=1 && x !=REGS_MAX ) { +dev_warn (dev,"%s registers not specified\n" , reg_names[x]); +return -EINVAL ; + } + +/* if the DMA registers were specified, try to enable DMA */ +if (x >REGS_DMA_RX ) { +if (brcmuart_arbitration (priv,1 ) ==0 ) { +u32 txrev =0 ; +u32 rxrev =0 ; + + txrev =udma_readl (priv,REGS_DMA_RX ,UDMA_RX_REVISION ); + rxrev =udma_readl (priv,REGS_DMA_TX ,UDMA_TX_REVISION ); +if ((txrev >=UDMA_TX_REVISION_REQUIRED ) && + (rxrev >=UDMA_RX_REVISION_REQUIRED )) { + +/* Enable the use of the DMA hardware */ + priv->dma_enabled = true; + }else { +brcmuart_arbitration (priv,0 ); +dev_err (dev, +"Unsupported DMA Hardware Revision\n" ); + } + }else { +dev_err (dev, +"Timeout arbitrating for UART DMA hardware\n" ); + } + } +
drivers/tty/serial/8250/8250_bcm7271.c line 1029
Quoted 10 lines
drivers/tty/serial/8250/8250_bcm7271.c new file +10
-0+of_property_read_u32 (np,"clock-frequency" , &clk_rate); + +/* See if a Baud clock has been specified */ + baud_mux_clk =of_clk_get_by_name (np,"sw_baud" ); +if (IS_ERR (baud_mux_clk)) { +if (PTR_ERR (baud_mux_clk) == -EPROBE_DEFER ) +return -EPROBE_DEFER ; +dev_info (dev,"BAUD MUX clock not specified\n" ); + }else { +dev_info (dev,"BAUD MUX clock found\n" );
These are not "info" messages, make them dev_dbg() please.
Quoted 4 lines
drivers/tty/serial/8250/8250_bcm7271.c new file +4
-0+ ret =clk_prepare_enable (baud_mux_clk); +if (ret) +return ret; + priv->baud_mux_clk = baud_mux_clk;
drivers/tty/serial/8250/8250_bcm7271.c line 1043
Quoted 10 lines
drivers/tty/serial/8250/8250_bcm7271.c new file +10
-0+init_real_clk_rates (dev, priv); + clk_rate = priv->default_mux_rate; + } + +if (clk_rate ==0 ) { +dev_err (dev,"clock-frequency or clk not defined\n" ); +return -EINVAL ; + } + +dev_info (dev,"DMA is %senabled\n" , priv->dma_enabled ?"" :"not " );
When a driver is working properly, no need to say anything in the kernel log.
This should be dev_info().
Much better overall, thanks for making the changes.
thanks,
greg k-h
kernel test robot
build botRe: [PATCH v4 2/2] serial: 8250: Add new 8250-core based Broadcom STB driver
In reply to Al CooperI love your patch! Perhaps something to improve:
Hi Al,
I love your patch! Perhaps something to improve:
[auto build test WARNING on tty/tty-testing]
[also build test WARNING on linux/master robh/for-next usb/usb-testing linus/master v5.12-rc2 next-20210305] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Al-Cooper/8250-Add-driver-for-Broadcom-UART/20210309-010755 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing config: nios2-allyesconfig (attached as .config) compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/38e38c0d5bd1fd6e256cbb0da5cd0cd1010b9cb6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Al-Cooper/8250-Add-driver-for-Broadcom-UART/20210309-010755 git checkout 38e38c0d5bd1fd6e256cbb0da5cd0cd1010b9cb6
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
drivers/tty/serial/8250/8250_bcm7271.c: In function 'brcmuart_tx_isr':
>> drivers/tty/serial/8250/8250_bcm7271.c:547:6: warning: variable 'length' set but not used [-Wunused-but-set-variable]
547 | int length;
| ^~~~~~
drivers/tty/serial/8250/8250_bcm7271.c: At top level:
>> drivers/tty/serial/8250/8250_bcm7271.c:1190:12: warning: no previous prototype for 'brcmuart_init' [-Wmissing-prototypes]
1190 | int __init brcmuart_init(void)
| ^~~~~~~~~~~~~
>> drivers/tty/serial/8250/8250_bcm7271.c:1198:13: warning: no previous prototype for 'brcmuart_deinit' [-Wmissing-prototypes] 1198 | void __exit brcmuart_deinit(void)
| ^~~~~~~~~~~~~~~
vim +/length +547 drivers/tty/serial/8250/8250_bcm7271.c
540
541 static void brcmuart_tx_isr(struct uart_port *up, u32 isr)
542 {
543 struct brcmuart_priv *priv = up->private_data;
544 struct device *dev = up->dev;
545 struct uart_8250_port *port_8250 = up_to_u8250p(up);
546 struct circ_buf *xmit = &port_8250->port.state->xmit;
> 547 int length;
548
549 length = udma_readl(priv, REGS_DMA_TX, UDMA_TX_TRANSFER_TOTAL);
550 if (isr & UDMA_INTR_TX_ABORT) {
551 if (priv->tx_running)
552 dev_err(dev, "Unexpected TX_ABORT interrupt\n");
553 return;
554 }
555 priv->tx_running = false;
556 if (!uart_circ_empty(xmit) && !uart_tx_stopped(up))
557 brcmuart_tx_dma(port_8250);
558 }
559
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org