Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v3 1/4] dt-bindings: mediatek: add support for mt6765 reference board
From: Mars Cheng @ 2018-07-04  1:52 UTC (permalink / raw)
  To: Matthias Brugger, Rob Herring, Marc Zyngier, Greg Kroah-Hartman
  Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
	wsd_upstream, linux-serial, linux-arm-kernel, Mars Cheng
In-Reply-To: <1530669174-17623-1-git-send-email-mars.cheng@mediatek.com>

Update binding document for mt6765 reference board

Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
---
 Documentation/devicetree/bindings/arm/mediatek.txt |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.txt b/Documentation/devicetree/bindings/arm/mediatek.txt
index 7d21ab3..48fac4e 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek.txt
@@ -11,6 +11,7 @@ compatible: Must contain one of
    "mediatek,mt6589"
    "mediatek,mt6592"
    "mediatek,mt6755"
+   "mediatek,mt6765"
    "mediatek,mt6795"
    "mediatek,mt6797"
    "mediatek,mt7622"
@@ -41,6 +42,9 @@ Supported boards:
 - Evaluation phone for MT6755(Helio P10):
     Required root node properties:
       - compatible = "mediatek,mt6755-evb", "mediatek,mt6755";
+- Evaluation board for MT6765(Helio P22):
+    Required root node properties:
+      - compatible = "mediatek,mt6765-evb", "mediatek,mt6765";
 - Evaluation board for MT6795(Helio X10):
     Required root node properties:
       - compatible = "mediatek,mt6795-evb", "mediatek,mt6795";
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 0/4] Add basic SoC support for mt6765
From: Mars Cheng @ 2018-07-04  1:52 UTC (permalink / raw)
  To: Matthias Brugger, Rob Herring, Marc Zyngier, Greg Kroah-Hartman
  Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
	wsd_upstream, linux-serial, linux-arm-kernel

This patch adds basic SoC support for Mediatek's new 8-core SoC,
MT6765, which is mainly for smartphone application.

Changes in V3:
1. split dt-binding document patchs
2. fix mt6765.dtsi warnings with W=12
3. remove uncessary PPI affinity for timer
4. add gicc base for gic dt node

Changes in V2:
1. fix clk properties in uart dts node
2. fix typo in submit title
3. add simple-bus in mt6765.dtsi
4. use correct SPDX license format


Mars Cheng (4):
  dt-bindings: mediatek: add support for mt6765 reference board
  dt-bindings: mtk-uart: add mt6765 uart bindings
  dt-bindings: interrupt-controller: add binding for mt6765
  arm64: dts: mediatek: add mt6765 support

 Documentation/devicetree/bindings/arm/mediatek.txt |    4 +
 .../interrupt-controller/mediatek,sysirq.txt       |    1 +
 .../devicetree/bindings/serial/mtk-uart.txt        |    1 +
 arch/arm64/boot/dts/mediatek/Makefile              |    1 +
 arch/arm64/boot/dts/mediatek/mt6765-evb.dts        |   33 +++++
 arch/arm64/boot/dts/mediatek/mt6765.dtsi           |  155 ++++++++++++++++++++
 6 files changed, 195 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi

^ permalink raw reply

* Re: [PATCH v2 2/2] arm64: dts: mediatek: add mt6765 support
From: Mars Cheng @ 2018-07-04  0:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Matthias Brugger, Marc Zyngier, CC Hwang, Loda Choui, Miles Chen,
	Jades Shih, Yingjoe Chen, My Chuang, linux-kernel@vger.kernel.org,
	linux-mediatek, devicetree, wsd_upstream,
	open list:SERIAL DRIVERS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_JsqKxvCxn3eb0sZVFwhUudDAkoHH5uyUgt1NY06zt-yKkQQ@mail.gmail.com>

Hi Rob

On Mon, 2018-07-02 at 15:50 -0600, Rob Herring wrote:
> On Mon, Jun 25, 2018 at 8:04 PM Mars Cheng <mars.cheng@mediatek.com> wrote:
> >
> > This adds basic chip support for MT6765 SoC.
> >
> > Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/Makefile       |    1 +
> >  arch/arm64/boot/dts/mediatek/mt6765-evb.dts |   33 ++++++
> >  arch/arm64/boot/dts/mediatek/mt6765.dtsi    |  158 +++++++++++++++++++++++++++
> >  3 files changed, 192 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> >  create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> > index ac17f60..7506b0d 100644
> > --- a/arch/arm64/boot/dts/mediatek/Makefile
> > +++ b/arch/arm64/boot/dts/mediatek/Makefile
> > @@ -1,6 +1,7 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
> >  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6765-evb.dtb
> >  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
> >  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
> >  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
> > diff --git a/arch/arm64/boot/dts/mediatek/mt6765-evb.dts b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> > new file mode 100644
> > index 0000000..36dddff2
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> > @@ -0,0 +1,33 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * dts file for Mediatek MT6765
> > + *
> > + * (C) Copyright 2018. Mediatek, Inc.
> > + *
> > + * Mars Cheng <mars.cheng@mediatek.com>
> > + */
> > +
> > +/dts-v1/;
> > +#include "mt6765.dtsi"
> > +
> > +/ {
> > +       model = "MediaTek MT6765 EVB";
> > +       compatible = "mediatek,mt6765-evb", "mediatek,mt6765";
> > +
> > +       aliases {
> > +               serial0 = &uart0;
> > +       };
> > +
> > +       memory@40000000 {
> > +               device_type = "memory";
> > +               reg = <0 0x40000000 0 0x1e800000>;
> > +       };
> > +
> > +       chosen {
> > +               stdout-path = "serial0:921600n8";
> > +       };
> > +};
> > +
> > +&uart0 {
> > +       status = "okay";
> > +};
> > diff --git a/arch/arm64/boot/dts/mediatek/mt6765.dtsi b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
> > new file mode 100644
> > index 0000000..ab34c0f
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
> > @@ -0,0 +1,158 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * dts file for Mediatek MT6765
> > + *
> > + * (C) Copyright 2018. Mediatek, Inc.
> > + *
> > + * Mars Cheng <mars.cheng@mediatek.com>
> > + */
> > +
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +/ {
> > +       compatible = "mediatek,mt6765";
> > +       interrupt-parent = <&sysirq>;
> > +       #address-cells = <2>;
> > +       #size-cells = <2>;
> > +
> > +       psci {
> > +               compatible = "arm,psci-0.2";
> > +               method = "smc";
> > +       };
> > +
> > +       cpus {
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +
> > +               cpu0: cpu@0 {
> 
> Really need labels for cpu nodes?
> 

Yes, I can drop them.

> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a53";
> > +                       enable-method = "psci";
> > +                       reg = <0x000>;
> > +               };
> > +
> > +               cpu1: cpu@1 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a53";
> > +                       enable-method = "psci";
> > +                       reg = <0x001>;
> > +               };
> > +
> > +               cpu2: cpu@2 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a53";
> > +                       enable-method = "psci";
> > +                       reg = <0x002>;
> > +               };
> > +
> > +               cpu3: cpu@3 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a53";
> > +                       enable-method = "psci";
> > +                       reg = <0x003>;
> > +               };
> > +
> > +               cpu4: cpu@100 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a53";
> > +                       enable-method = "psci";
> > +                       reg = <0x100>;
> > +               };
> > +
> > +               cpu5: cpu@101 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a53";
> > +                       enable-method = "psci";
> > +                       reg = <0x101>;
> > +               };
> > +
> > +               cpu6: cpu@102 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a53";
> > +                       enable-method = "psci";
> > +                       reg = <0x102>;
> > +               };
> > +
> > +               cpu7: cpu@103 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a53";
> > +                       enable-method = "psci";
> > +                       reg = <0x103>;
> > +               };
> > +       };
> > +
> > +       baud_clk: dummy26m {
> > +               compatible = "fixed-clock";
> > +               clock-frequency = <26000000>;
> > +               #clock-cells = <0>;
> > +       };
> > +
> > +       sys_clk: dummyclk {
> > +               compatible = "fixed-clock";
> > +               clock-frequency = <26000000>;
> > +               #clock-cells = <0>;
> > +       };
> > +
> > +       timer {
> > +               compatible = "arm,armv8-timer";
> > +               interrupt-parent = <&gic>;
> > +               interrupts = <GIC_PPI 13
> > +                            (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > +                            <GIC_PPI 14
> > +                            (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > +                            <GIC_PPI 11
> > +                            (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > +                            <GIC_PPI 10
> > +                            (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
> > +       };
> > +
> > +       soc {
> > +               #address-cells = <2>;
> > +               #size-cells = <2>;
> > +               compatible = "simple-bus";
> > +               ranges;
> > +
> > +               sysirq: intpol-controller@10200a80 {
> 
> interrupt-controller@...


My mistake, did not fix this in v2, will correct it. 

> 
> > +                       compatible = "mediatek,mt6765-sysirq",
> > +                                    "mediatek,mt6577-sysirq";
> > +                       interrupt-controller;
> > +                       #interrupt-cells = <3>;
> > +                       interrupt-parent = <&gic>;
> > +                       reg = <0 0x10200a80 0 0x50>;
> > +               };
> > +
> > +               gic: interrupt-controller@0c000000 {
> 
> Drop the leading 0. Build your dts with W=12 and fix the warnings like this.

Thanks for suggestion. Will avoid this kind of errors.

> 
> > +                       compatible = "arm,gic-v3";
> > +                       #interrupt-cells = <3>;
> > +                       #address-cells = <2>;
> > +                       #size-cells = <2>;
> > +                       #redistributor-regions = <1>;
> > +                       interrupt-parent = <&gic>;
> > +                       interrupt-controller;
> > +                       reg = <0 0x0c000000 0 0x40000>, // distributor
> > +                             <0 0x0c100000 0 0x200000>; // redistributor
> > +                       interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > +               };
> > +
> > +               uart0: serial@11002000 {
> > +                       compatible = "mediatek,mt6765-uart",
> > +                                    "mediatek,mt6577-uart";
> > +                       reg = <0 0x11002000 0 0x400>;
> > +                       interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
> > +                       clocks = <&baud_clk>, <&sys_clk>;
> > +                       clock-names = "baud", "bus";
> > +                       status = "disabled";
> > +               };
> > +
> > +               uart1: serial@11003000 {
> > +                       compatible = "mediatek,mt6765-uart",
> > +                                    "mediatek,mt6577-uart";
> > +                       reg = <0 0x11003000 0 0x400>;
> > +                       interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
> > +                       clocks = <&baud_clk>, <&sys_clk>;
> > +                       clock-names = "baud", "bus";
> > +                       status = "disabled";
> > +               };
> > +       }; /* end of soc */
> > +};
> > --
> > 1.7.9.5
> >

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: mediatek: Add bindings for mediatek MT6765 Platform
From: Mars Cheng @ 2018-07-04  0:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: Matthias Brugger, Marc Zyngier, CC Hwang, Loda Chou, Miles Chen,
	Jades Shih, Yingjoe Chen, My Chuang, linux-kernel, linux-mediatek,
	devicetree, wsd_upstream, linux-serial, linux-arm-kernel
In-Reply-To: <20180703221101.GA6829@rob-hp-laptop>

Hi Rob

On Tue, 2018-07-03 at 16:11 -0600, Rob Herring wrote:
> On Tue, Jun 26, 2018 at 10:04:05AM +0800, Mars Cheng wrote:
> > This adds dt-binding documentation for Mediatek MT6765. Only
> > include very basic items, gic, uart timer and cpu.
> > 
> > Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/arm/mediatek.txt |    4 ++++
> >  .../interrupt-controller/mediatek,sysirq.txt       |    1 +
> >  .../devicetree/bindings/serial/mtk-uart.txt        |    1 +
> 
> These can all go thru different trees, so should be split.
> 

Got it, will split them.

> >  3 files changed, 6 insertions(+)

^ permalink raw reply

* Re: [PATCH v2] dt-bindings: serial: imx: clarify rs485 support usage
From: Rob Herring @ 2018-07-03 23:10 UTC (permalink / raw)
  To: Baruch Siach
  Cc: devicetree, linux-serial, Greg Kroah-Hartman, Sascha Hauer,
	NXP Linux Team, Pengutronix Kernel Team, Uwe Kleine-König,
	Fabio Estevam, Shawn Guo, linux-arm-kernel, Lothar Waßmann
In-Reply-To: <e9beff38e9035c2f33d691f362e37d8e62aaa660.1530170732.git.baruch@tkos.co.il>

On Thu, Jun 28, 2018 at 10:25:32AM +0300, Baruch Siach wrote:
> The i.MX UART peripheral uses the RST_B signal as input, and CTS_B as

s/RST_B/RTS_B/

> output. This is just like the DCE role in RS-232. This is true
> regardless of the "DTE mode" setting of this peripheral.
> 
> As a result, rs485 support hardware must use the CTS_B signal to control
> the RS-485 transceiver. This is in contrast to generic rs485 kernel
> code, documentation, and DT property names that consistently refer to
> the RTS as transceiver control signal.
> 
> Add a note in the DT binding document about that, to reduce the
> confusion somewhat.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: Fix commit log typos (Lothar Waßmann)
> ---
>  Documentation/devicetree/bindings/serial/fsl-imx-uart.txt | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

With that,

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: mediatek: Add bindings for mediatek MT6765 Platform
From: Rob Herring @ 2018-07-03 22:11 UTC (permalink / raw)
  To: Mars Cheng
  Cc: Matthias Brugger, Marc Zyngier, CC Hwang, Loda Chou, Miles Chen,
	Jades Shih, Yingjoe Chen, My Chuang, linux-kernel, linux-mediatek,
	devicetree, wsd_upstream, linux-serial, linux-arm-kernel
In-Reply-To: <1529978646-28976-2-git-send-email-mars.cheng@mediatek.com>

On Tue, Jun 26, 2018 at 10:04:05AM +0800, Mars Cheng wrote:
> This adds dt-binding documentation for Mediatek MT6765. Only
> include very basic items, gic, uart timer and cpu.
> 
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> ---
>  Documentation/devicetree/bindings/arm/mediatek.txt |    4 ++++
>  .../interrupt-controller/mediatek,sysirq.txt       |    1 +
>  .../devicetree/bindings/serial/mtk-uart.txt        |    1 +

These can all go thru different trees, so should be split.

>  3 files changed, 6 insertions(+)

^ permalink raw reply

* Re: [PATCH 0/2] serial: 8250_dw: add fractional divisor support
From: Andy Shevchenko @ 2018-07-03 13:03 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20180703104816.031abbfc@xhacker.debian>

On Tue, 2018-07-03 at 10:48 +0800, Jisheng Zhang wrote:
> On Tue, 3 Jul 2018 10:22:57 +0800 Jisheng Zhang wrote:

> patching struct uart_port seems a bit overhead. After reading the code
> again, I propose another solution, similar as what dl_write() is used
> in
> 8250 core:
> 
> 1.introduce the hook to struct uart_8250_port as my previous patches
> do, 
> 
> 2.rename current serial8250_set_divisor() as
> default_serial_get_divisor()
> then introduce a new serial8250_set_divisor() as:
> static inline void serial8250_set_divisor(struct uart_8250_port
> *up,....)
> {
> 	up->set_divisor();
> }
> 
> and point up->set-divisor to default_serial_get_divisor
> 
> what do you think about this solution?

Disagree. See my previous answer for details.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH 0/2] serial: 8250_dw: add fractional divisor support
From: Andy Shevchenko @ 2018-07-03 13:02 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20180703102257.678db6a8@xhacker.debian>

On Tue, 2018-07-03 at 10:22 +0800, Jisheng Zhang wrote:
> Hi,
> 
> On Mon, 2 Jul 2018 14:51:03 +0300 Andy Shevchenko wrote:
> 
> > On Mon, 2018-07-02 at 13:18 +0300, Andy Shevchenko wrote:
> > > On Mon, 2018-07-02 at 18:04 +0800, Jisheng Zhang wrote:  
> > > > For Synopsys DesignWare 8250 uart which version >= 4.00a,
> > > > there's a
> > > > valid divisor latch fraction register. The fractional divisor
> > > > width
> > > > is
> > > > 4bits ~ 6bits.
> > > >   
> > > 
> > > There are several serial IPs that have fractional divider built-
> > > in.
> > > None
> > > is using any specific hooks. Why do you need in your case, esp.
> > > taking
> > > into consideration that we have a custom ->set_termios()
> > > callback?  
> > 
> > Okay, I see that in 8250 we have hooks embedded into 8250_port.c
> > which
> > is not the best solution.
> > 
> > For example it prevents better splitting Exar code.
> > So, we would need these hooks, but better to integrate them in the
> > same
> > way like it's done for the rest of 8250 ones, i.e.
> > - rename existing to have a "do" word
> > - create new functions which would be a replacement that choose
> > between
> > "do" variant and custom one
> > - not sure if we need to export "do" variants (at least for now)
> 
> So you mean add the support as following:
> 
> 1.rename current serial8250_set_divisor as serial8250_do_set_divisor
> 

Yes

> 2.add a new serial8250_set_divisor which will be as simple as

Yes and no. 

> 
> static void serial8250_set_divisor(struct uart_port *port, ...)
> {
> 	struct uart_8250_port *up = up_to_u8250p(port);
> 
> 	if (up->set_divisor)

port->

> 		up->set_divisor(...);

port->

> 	else
> 		serial8250_do_set_divisor(...);
> }
> 
> could you please confirm?

Yes.

> 
> Another issue is I'm not sure which struct to add the hook,
> struct uart_port or struct uart_8250_port? Currently, it seems that
> only
> uart_8250_port needs this hook,

Define "needs". 8250 _uses_ it, the rest which I easy grepped would be
able to use it if we provide such a possibility.

lantiq.c is one example (currently they don't use it), or
drivers/tty/serial/digicolor-usart.c limits baud rate choice b/c of
absence of a common way.

>  sure, adding the hook to struct uart_port
> is fine either. Could you please kindly give some suggestions?

See above. Thanks for doing this!

> 
> Thanks,
> Jisheng

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH v3 23/27] devicetree: fix name of pinctrl-bindings.txt
From: Lee Jones @ 2018-07-03  7:33 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Rob Herring, Mark Rutland, Ulf Hansson,
	Linus Walleij, Greg Kroah-Hartman, Mark Brown, devicetree,
	linux-mmc, linux-gpio, linux-serial, linux-spi
In-Reply-To: <d04d47f9fba035cf9de8589a288964c636324d58.1528990947.git.mchehab+samsung@kernel.org>

On Thu, 14 Jun 2018, Mauro Carvalho Chehab wrote:

> Rename:
> 	pinctrl-binding.txt -> pinctrl-bindings.txt
> 
> In order to match the current name of this file.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
>  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 2 +-
>  Documentation/devicetree/bindings/mfd/as3722.txt              | 2 +-

Acked-by: Lee Jones <lee.jones@linaro.org>

>  .../devicetree/bindings/mmc/microchip,sdhci-pic32.txt         | 2 +-
>  Documentation/devicetree/bindings/mmc/sdhci-st.txt            | 2 +-
>  .../devicetree/bindings/pinctrl/pinctrl-max77620.txt          | 4 ++--
>  .../devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt          | 4 ++--
>  Documentation/devicetree/bindings/pinctrl/pinctrl-rk805.txt   | 4 ++--
>  .../devicetree/bindings/serial/microchip,pic32-uart.txt       | 2 +-
>  Documentation/devicetree/bindings/spi/spi-st-ssc.txt          | 2 +-
>  9 files changed, 12 insertions(+), 12 deletions(-)

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH v3 6/8] serial: Add Tegra Combined UART driver
From: Mikko Perttunen @ 2018-07-03  6:40 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Mikko Perttunen, devicetree, gregkh, jassisinghbrar, linux-kernel,
	jonathanh, linux-serial, linux-tegra, linux-arm-kernel
In-Reply-To: <20180702134731.GI13096@ulmo>

On 02.07.2018 16:47, Thierry Reding wrote:
> On Mon, Jul 02, 2018 at 04:30:07PM +0300, Mikko Perttunen wrote:
>> On 02.07.2018 16:18, Thierry Reding wrote:
>>> On Mon, Jul 02, 2018 at 02:40:31PM +0300, Mikko Perttunen wrote:
>>>> The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
>>>> multiplexing multiple "virtual UARTs" into a single hardware serial
>>>> port. The TCU is the primary serial port on Tegra194 devices.
>>>>
>>>> Add a TCU driver utilizing the mailbox framework, as the used mailboxes
>>>> are part of Tegra HSP blocks that are already controlled by the Tegra
>>>> HSP mailbox driver.
>>>>
>>>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>>>> ---
>>>>
>>>> Notes:
>>>>       v2:
>>>>       - Removed (void) casts for unused variables.
>>>>       - Changed the uart_set_options() call to be on one line, even if its
>>>>         over 80 characters.
>>>>       - Added defines for magic numbers.
>>>>       - Style fixes.
>>>>       - Changed Kconfig entry to depend on the Tegra HSP driver instead of
>>>>         just the mailbox framework.
>>>>       v3:
>>>>       - Removed FLUSH bit, as it's unnecessary and slows down printing
>>>>       - Removed call to uart_set_options
>>>>       - Added mbox_free_channel calls to remove()
>>>>
>>>>    drivers/tty/serial/Kconfig       |   9 ++
>>>>    drivers/tty/serial/Makefile      |   1 +
>>>>    drivers/tty/serial/tegra-tcu.c   | 291 +++++++++++++++++++++++++++++++++++++++
>>>>    include/uapi/linux/serial_core.h |   3 +
>>>>    4 files changed, 304 insertions(+)
>>>>    create mode 100644 drivers/tty/serial/tegra-tcu.c
>>>
>>> The driver looks good to me. But for my own understanding, is there some
>>> way we can make use of the multiplexing? That is, could we add a
>>> mechanism to have the driver filter out only a specific stream? Could we
>>> also specify which stream to send data back to? What happens by default?
>>> Which stream is data sent to?
>>
>> There is no multiplexing on the producer/device side (i.e. what this driver
>> does). The mailbox specified in device tree specifies the stream we send
>> stuff to. The mailboxes are per-CPU (we use the CCPLEX mailbox here), so it
>> cannot really be changed.
> 
> Oh, I see, so there is one stream per pair of mailboxes? That is, the
> mailboxes we specify in the DT define which stream we receive from and
> send to?

Correct.

Mikko

> 
> Thierry
> 

^ permalink raw reply

* Re: [PATCH v5 0/7] add virt-dma support for imx-sdma
From: Robin Gong @ 2018-07-03  2:57 UTC (permalink / raw)
  To: vkoul@kernel.org
  Cc: dl-linux-imx, linux-kernel@vger.kernel.org, jslaby@suse.com,
	dan.j.williams@intel.com, dmaengine@vger.kernel.org,
	gregkh@linuxfoundation.org, linux-serial@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, l.stach@pengutronix.de,
	s.hauer@pengutronix.de
In-Reply-To: <20180702131737.GJ22377@vkoul-mobl>

On 一, 2018-07-02 at 18:47 +0530, Vinod wrote:
> On 02-07-18, 02:32, Robin Gong wrote:
> > 
> > Hi Vinod,
> > 	Do you have any comment for this patchset? Lucas and Sascha
> > acked it and tty patch already merged in.
> I was actually waiting for ACK/action on patch 1 :)
> 
> I have reviewed the series, some nitpicks nothing major, so I have
> applied this good cleanup and conversion :) along with fixes for
> typos
> in code, GFP_ fix and removed unused variable.
> 
> My buildchain noticed warns (with W=1) on this file, esp now that
> structs are moved around. Pls fix the struct descriptions.
Okay, I'll send another patch to fix such build warning issue later.
BTW, patch 1 has already been in linux-next tree :).
> 
> Thanks

^ permalink raw reply

* Re: [PATCH 0/2] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-03  2:48 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20180703102257.678db6a8@xhacker.debian>

On Tue, 3 Jul 2018 10:22:57 +0800 Jisheng Zhang wrote:

> Hi,
> 
> On Mon, 2 Jul 2018 14:51:03 +0300 Andy Shevchenko wrote:
> 
> > On Mon, 2018-07-02 at 13:18 +0300, Andy Shevchenko wrote:  
> > > On Mon, 2018-07-02 at 18:04 +0800, Jisheng Zhang wrote:    
> > > > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
> > > > valid divisor latch fraction register. The fractional divisor width
> > > > is
> > > > 4bits ~ 6bits.
> > > >     
> > > 
> > > There are several serial IPs that have fractional divider built-in.
> > > None
> > > is using any specific hooks. Why do you need in your case, esp. taking
> > > into consideration that we have a custom ->set_termios() callback?    
> > 
> > Okay, I see that in 8250 we have hooks embedded into 8250_port.c which
> > is not the best solution.
> > 
> > For example it prevents better splitting Exar code.
> > So, we would need these hooks, but better to integrate them in the same
> > way like it's done for the rest of 8250 ones, i.e.
> > - rename existing to have a "do" word
> > - create new functions which would be a replacement that choose between
> > "do" variant and custom one
> > - not sure if we need to export "do" variants (at least for now)  
> 
> So you mean add the support as following:
> 
> 1.rename current serial8250_set_divisor as serial8250_do_set_divisor
> 
> 2.add a new serial8250_set_divisor which will be as simple as
> 
> static void serial8250_set_divisor(struct uart_port *port, ...)
> {
> 	struct uart_8250_port *up = up_to_u8250p(port);
> 
> 	if (up->set_divisor)
> 		up->set_divisor(...);
> 	else
> 		serial8250_do_set_divisor(...);
> }
> 
> could you please confirm?
> 
> Another issue is I'm not sure which struct to add the hook,
> struct uart_port or struct uart_8250_port? Currently, it seems that only
> uart_8250_port needs this hook, sure, adding the hook to struct uart_port
> is fine either. Could you please kindly give some suggestions?

patching struct uart_port seems a bit overhead. After reading the code
again, I propose another solution, similar as what dl_write() is used in
8250 core:

1.introduce the hook to struct uart_8250_port as my previous patches do, 

2.rename current serial8250_set_divisor() as default_serial_get_divisor()
then introduce a new serial8250_set_divisor() as:
static inline void serial8250_set_divisor(struct uart_8250_port *up,....)
{
	up->set_divisor();
}

and point up->set-divisor to default_serial_get_divisor

what do you think about this solution?

Thanks

> 
> Thanks,
> Jisheng

^ permalink raw reply

* Re: [PATCH 0/2] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-03  2:22 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	linux-arm-kernel
In-Reply-To: <59b14f97234271a7d859fa07f27bd66ec252ccc9.camel@linux.intel.com>

Hi,

On Mon, 2 Jul 2018 14:51:03 +0300 Andy Shevchenko wrote:

> On Mon, 2018-07-02 at 13:18 +0300, Andy Shevchenko wrote:
> > On Mon, 2018-07-02 at 18:04 +0800, Jisheng Zhang wrote:  
> > > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
> > > valid divisor latch fraction register. The fractional divisor width
> > > is
> > > 4bits ~ 6bits.
> > >   
> > 
> > There are several serial IPs that have fractional divider built-in.
> > None
> > is using any specific hooks. Why do you need in your case, esp. taking
> > into consideration that we have a custom ->set_termios() callback?  
> 
> Okay, I see that in 8250 we have hooks embedded into 8250_port.c which
> is not the best solution.
> 
> For example it prevents better splitting Exar code.
> So, we would need these hooks, but better to integrate them in the same
> way like it's done for the rest of 8250 ones, i.e.
> - rename existing to have a "do" word
> - create new functions which would be a replacement that choose between
> "do" variant and custom one
> - not sure if we need to export "do" variants (at least for now)

So you mean add the support as following:

1.rename current serial8250_set_divisor as serial8250_do_set_divisor

2.add a new serial8250_set_divisor which will be as simple as

static void serial8250_set_divisor(struct uart_port *port, ...)
{
	struct uart_8250_port *up = up_to_u8250p(port);

	if (up->set_divisor)
		up->set_divisor(...);
	else
		serial8250_do_set_divisor(...);
}

could you please confirm?

Another issue is I'm not sure which struct to add the hook,
struct uart_port or struct uart_8250_port? Currently, it seems that only
uart_8250_port needs this hook, sure, adding the hook to struct uart_port
is fine either. Could you please kindly give some suggestions?

Thanks,
Jisheng

^ permalink raw reply

* Re: [PATCH v2 2/2] arm64: dts: mediatek: add mt6765 support
From: Rob Herring @ 2018-07-02 21:50 UTC (permalink / raw)
  To: Mars Cheng
  Cc: Matthias Brugger, Marc Zyngier, CC Hwang, Loda Choui, Miles Chen,
	Jades Shih, Yingjoe Chen, My Chuang, linux-kernel@vger.kernel.org,
	linux-mediatek, devicetree, wsd_upstream,
	open list:SERIAL DRIVERS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <1529978646-28976-3-git-send-email-mars.cheng@mediatek.com>

On Mon, Jun 25, 2018 at 8:04 PM Mars Cheng <mars.cheng@mediatek.com> wrote:
>
> This adds basic chip support for MT6765 SoC.
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/Makefile       |    1 +
>  arch/arm64/boot/dts/mediatek/mt6765-evb.dts |   33 ++++++
>  arch/arm64/boot/dts/mediatek/mt6765.dtsi    |  158 +++++++++++++++++++++++++++
>  3 files changed, 192 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
>
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index ac17f60..7506b0d 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6765-evb.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
> diff --git a/arch/arm64/boot/dts/mediatek/mt6765-evb.dts b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> new file mode 100644
> index 0000000..36dddff2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * dts file for Mediatek MT6765
> + *
> + * (C) Copyright 2018. Mediatek, Inc.
> + *
> + * Mars Cheng <mars.cheng@mediatek.com>
> + */
> +
> +/dts-v1/;
> +#include "mt6765.dtsi"
> +
> +/ {
> +       model = "MediaTek MT6765 EVB";
> +       compatible = "mediatek,mt6765-evb", "mediatek,mt6765";
> +
> +       aliases {
> +               serial0 = &uart0;
> +       };
> +
> +       memory@40000000 {
> +               device_type = "memory";
> +               reg = <0 0x40000000 0 0x1e800000>;
> +       };
> +
> +       chosen {
> +               stdout-path = "serial0:921600n8";
> +       };
> +};
> +
> +&uart0 {
> +       status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt6765.dtsi b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
> new file mode 100644
> index 0000000..ab34c0f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * dts file for Mediatek MT6765
> + *
> + * (C) Copyright 2018. Mediatek, Inc.
> + *
> + * Mars Cheng <mars.cheng@mediatek.com>
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> +       compatible = "mediatek,mt6765";
> +       interrupt-parent = <&sysirq>;
> +       #address-cells = <2>;
> +       #size-cells = <2>;
> +
> +       psci {
> +               compatible = "arm,psci-0.2";
> +               method = "smc";
> +       };
> +
> +       cpus {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               cpu0: cpu@0 {

Really need labels for cpu nodes?

> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       enable-method = "psci";
> +                       reg = <0x000>;
> +               };
> +
> +               cpu1: cpu@1 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       enable-method = "psci";
> +                       reg = <0x001>;
> +               };
> +
> +               cpu2: cpu@2 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       enable-method = "psci";
> +                       reg = <0x002>;
> +               };
> +
> +               cpu3: cpu@3 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       enable-method = "psci";
> +                       reg = <0x003>;
> +               };
> +
> +               cpu4: cpu@100 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       enable-method = "psci";
> +                       reg = <0x100>;
> +               };
> +
> +               cpu5: cpu@101 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       enable-method = "psci";
> +                       reg = <0x101>;
> +               };
> +
> +               cpu6: cpu@102 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       enable-method = "psci";
> +                       reg = <0x102>;
> +               };
> +
> +               cpu7: cpu@103 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       enable-method = "psci";
> +                       reg = <0x103>;
> +               };
> +       };
> +
> +       baud_clk: dummy26m {
> +               compatible = "fixed-clock";
> +               clock-frequency = <26000000>;
> +               #clock-cells = <0>;
> +       };
> +
> +       sys_clk: dummyclk {
> +               compatible = "fixed-clock";
> +               clock-frequency = <26000000>;
> +               #clock-cells = <0>;
> +       };
> +
> +       timer {
> +               compatible = "arm,armv8-timer";
> +               interrupt-parent = <&gic>;
> +               interrupts = <GIC_PPI 13
> +                            (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> +                            <GIC_PPI 14
> +                            (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> +                            <GIC_PPI 11
> +                            (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> +                            <GIC_PPI 10
> +                            (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
> +       };
> +
> +       soc {
> +               #address-cells = <2>;
> +               #size-cells = <2>;
> +               compatible = "simple-bus";
> +               ranges;
> +
> +               sysirq: intpol-controller@10200a80 {

interrupt-controller@...

> +                       compatible = "mediatek,mt6765-sysirq",
> +                                    "mediatek,mt6577-sysirq";
> +                       interrupt-controller;
> +                       #interrupt-cells = <3>;
> +                       interrupt-parent = <&gic>;
> +                       reg = <0 0x10200a80 0 0x50>;
> +               };
> +
> +               gic: interrupt-controller@0c000000 {

Drop the leading 0. Build your dts with W=12 and fix the warnings like this.

> +                       compatible = "arm,gic-v3";
> +                       #interrupt-cells = <3>;
> +                       #address-cells = <2>;
> +                       #size-cells = <2>;
> +                       #redistributor-regions = <1>;
> +                       interrupt-parent = <&gic>;
> +                       interrupt-controller;
> +                       reg = <0 0x0c000000 0 0x40000>, // distributor
> +                             <0 0x0c100000 0 0x200000>; // redistributor
> +                       interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +               };
> +
> +               uart0: serial@11002000 {
> +                       compatible = "mediatek,mt6765-uart",
> +                                    "mediatek,mt6577-uart";
> +                       reg = <0 0x11002000 0 0x400>;
> +                       interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
> +                       clocks = <&baud_clk>, <&sys_clk>;
> +                       clock-names = "baud", "bus";
> +                       status = "disabled";
> +               };
> +
> +               uart1: serial@11003000 {
> +                       compatible = "mediatek,mt6765-uart",
> +                                    "mediatek,mt6577-uart";
> +                       reg = <0 0x11003000 0 0x400>;
> +                       interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
> +                       clocks = <&baud_clk>, <&sys_clk>;
> +                       clock-names = "baud", "bus";
> +                       status = "disabled";
> +               };
> +       }; /* end of soc */
> +};
> --
> 1.7.9.5
>

^ permalink raw reply

* Re: [PATCH v3 0/8] Tegra Combined UART driver
From: Thierry Reding @ 2018-07-02 14:17 UTC (permalink / raw)
  To: Greg KH
  Cc: devicetree, jassisinghbrar, linux-kernel, jonathanh, linux-serial,
	linux-tegra, Mikko Perttunen, linux-arm-kernel
In-Reply-To: <20180702140742.GA7977@kroah.com>


[-- Attachment #1.1: Type: text/plain, Size: 1828 bytes --]

On Mon, Jul 02, 2018 at 04:07:42PM +0200, Greg KH wrote:
> On Mon, Jul 02, 2018 at 04:03:43PM +0200, Thierry Reding wrote:
> > On Mon, Jul 02, 2018 at 02:40:25PM +0300, Mikko Perttunen wrote:
> > > Hi, here's v3. Changes are in individual patches. Acks are still
> > > missing from the following patches:
> > > 
> > > mailbox: Add transmit done by blocking option
> > > mailbox: tegra-hsp: Refactor in preparation of mailboxes
> > > mailbox: tegra-hsp: Add support for shared mailboxes
> > > serial: Add Tegra Combined UART driver
> > 
> > There don't seem to be any build-time dependencies between the above, so
> > technically I guess these could be applied to the respective trees
> > separately. The only thing we need to be careful about is to not merge
> > the final patch before the others, because it will break serial console
> > if the other patches aren't applied.
> > 
> > Jassi, Greg,
> > 
> > What I could do to resolve the runtime dependency is pull the mailbox
> > patches into a separate branch, create another separate branch for the
> > TCU driver that is based on the mailbox branch and then base the DT
> > changes on that branch, or possibly send a separate pull request for
> > just the DT change late in the merge window or shortly after v4.19-rc1.
> > 
> > Obviously the latter would be equivalent to you guys merging the patches
> > through your trees for v4.19-rc1 and me sending out a late pull request
> > for the DT bits.
> 
> No need for me to take the serial driver if these types of dependancies
> are required, you are free to take that in your tree if you want to.
> 
> does that help?

It does if Jassi's okay if I take the mailbox drivers into the Tegra
tree as well. I'll wait for Jassi to comment and then we can decide what
to do.

Thanks,
Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 0/8] Tegra Combined UART driver
From: Greg KH @ 2018-07-02 14:07 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Mikko Perttunen, jassisinghbrar, jonathanh, devicetree,
	linux-serial, linux-tegra, linux-arm-kernel, linux-kernel
In-Reply-To: <20180702140343.GO13096@ulmo>

On Mon, Jul 02, 2018 at 04:03:43PM +0200, Thierry Reding wrote:
> On Mon, Jul 02, 2018 at 02:40:25PM +0300, Mikko Perttunen wrote:
> > Hi, here's v3. Changes are in individual patches. Acks are still
> > missing from the following patches:
> > 
> > mailbox: Add transmit done by blocking option
> > mailbox: tegra-hsp: Refactor in preparation of mailboxes
> > mailbox: tegra-hsp: Add support for shared mailboxes
> > serial: Add Tegra Combined UART driver
> 
> There don't seem to be any build-time dependencies between the above, so
> technically I guess these could be applied to the respective trees
> separately. The only thing we need to be careful about is to not merge
> the final patch before the others, because it will break serial console
> if the other patches aren't applied.
> 
> Jassi, Greg,
> 
> What I could do to resolve the runtime dependency is pull the mailbox
> patches into a separate branch, create another separate branch for the
> TCU driver that is based on the mailbox branch and then base the DT
> changes on that branch, or possibly send a separate pull request for
> just the DT change late in the merge window or shortly after v4.19-rc1.
> 
> Obviously the latter would be equivalent to you guys merging the patches
> through your trees for v4.19-rc1 and me sending out a late pull request
> for the DT bits.

No need for me to take the serial driver if these types of dependancies
are required, you are free to take that in your tree if you want to.

does that help?

greg k-h

^ permalink raw reply

* Re: [PATCH v3 0/8] Tegra Combined UART driver
From: Thierry Reding @ 2018-07-02 14:03 UTC (permalink / raw)
  To: Mikko Perttunen, jassisinghbrar, gregkh
  Cc: jonathanh, devicetree, linux-serial, linux-tegra,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20180702114033.15654-1-mperttunen@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 1281 bytes --]

On Mon, Jul 02, 2018 at 02:40:25PM +0300, Mikko Perttunen wrote:
> Hi, here's v3. Changes are in individual patches. Acks are still
> missing from the following patches:
> 
> mailbox: Add transmit done by blocking option
> mailbox: tegra-hsp: Refactor in preparation of mailboxes
> mailbox: tegra-hsp: Add support for shared mailboxes
> serial: Add Tegra Combined UART driver

There don't seem to be any build-time dependencies between the above, so
technically I guess these could be applied to the respective trees
separately. The only thing we need to be careful about is to not merge
the final patch before the others, because it will break serial console
if the other patches aren't applied.

Jassi, Greg,

What I could do to resolve the runtime dependency is pull the mailbox
patches into a separate branch, create another separate branch for the
TCU driver that is based on the mailbox branch and then base the DT
changes on that branch, or possibly send a separate pull request for
just the DT change late in the merge window or shortly after v4.19-rc1.

Obviously the latter would be equivalent to you guys merging the patches
through your trees for v4.19-rc1 and me sending out a late pull request
for the DT bits.

Any preference?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 6/8] serial: Add Tegra Combined UART driver
From: Thierry Reding @ 2018-07-02 13:47 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: Mikko Perttunen, devicetree, gregkh, jassisinghbrar, linux-kernel,
	jonathanh, linux-serial, linux-tegra, linux-arm-kernel
In-Reply-To: <50c4702b-3d59-e443-5075-c888f83a2615@kapsi.fi>

[-- Attachment #1: Type: text/plain, Size: 2407 bytes --]

On Mon, Jul 02, 2018 at 04:30:07PM +0300, Mikko Perttunen wrote:
> On 02.07.2018 16:18, Thierry Reding wrote:
> > On Mon, Jul 02, 2018 at 02:40:31PM +0300, Mikko Perttunen wrote:
> > > The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
> > > multiplexing multiple "virtual UARTs" into a single hardware serial
> > > port. The TCU is the primary serial port on Tegra194 devices.
> > > 
> > > Add a TCU driver utilizing the mailbox framework, as the used mailboxes
> > > are part of Tegra HSP blocks that are already controlled by the Tegra
> > > HSP mailbox driver.
> > > 
> > > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > > ---
> > > 
> > > Notes:
> > >      v2:
> > >      - Removed (void) casts for unused variables.
> > >      - Changed the uart_set_options() call to be on one line, even if its
> > >        over 80 characters.
> > >      - Added defines for magic numbers.
> > >      - Style fixes.
> > >      - Changed Kconfig entry to depend on the Tegra HSP driver instead of
> > >        just the mailbox framework.
> > >      v3:
> > >      - Removed FLUSH bit, as it's unnecessary and slows down printing
> > >      - Removed call to uart_set_options
> > >      - Added mbox_free_channel calls to remove()
> > > 
> > >   drivers/tty/serial/Kconfig       |   9 ++
> > >   drivers/tty/serial/Makefile      |   1 +
> > >   drivers/tty/serial/tegra-tcu.c   | 291 +++++++++++++++++++++++++++++++++++++++
> > >   include/uapi/linux/serial_core.h |   3 +
> > >   4 files changed, 304 insertions(+)
> > >   create mode 100644 drivers/tty/serial/tegra-tcu.c
> > 
> > The driver looks good to me. But for my own understanding, is there some
> > way we can make use of the multiplexing? That is, could we add a
> > mechanism to have the driver filter out only a specific stream? Could we
> > also specify which stream to send data back to? What happens by default?
> > Which stream is data sent to?
> 
> There is no multiplexing on the producer/device side (i.e. what this driver
> does). The mailbox specified in device tree specifies the stream we send
> stuff to. The mailboxes are per-CPU (we use the CCPLEX mailbox here), so it
> cannot really be changed.

Oh, I see, so there is one stream per pair of mailboxes? That is, the
mailboxes we specify in the DT define which stream we receive from and
send to?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 6/8] serial: Add Tegra Combined UART driver
From: Mikko Perttunen @ 2018-07-02 13:30 UTC (permalink / raw)
  To: Thierry Reding, Mikko Perttunen
  Cc: jassisinghbrar, gregkh, jonathanh, devicetree, linux-serial,
	linux-tegra, linux-arm-kernel, linux-kernel
In-Reply-To: <20180702131851.GH13096@ulmo>

On 02.07.2018 16:18, Thierry Reding wrote:
> On Mon, Jul 02, 2018 at 02:40:31PM +0300, Mikko Perttunen wrote:
>> The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
>> multiplexing multiple "virtual UARTs" into a single hardware serial
>> port. The TCU is the primary serial port on Tegra194 devices.
>>
>> Add a TCU driver utilizing the mailbox framework, as the used mailboxes
>> are part of Tegra HSP blocks that are already controlled by the Tegra
>> HSP mailbox driver.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>>
>> Notes:
>>      v2:
>>      - Removed (void) casts for unused variables.
>>      - Changed the uart_set_options() call to be on one line, even if its
>>        over 80 characters.
>>      - Added defines for magic numbers.
>>      - Style fixes.
>>      - Changed Kconfig entry to depend on the Tegra HSP driver instead of
>>        just the mailbox framework.
>>      
>>      v3:
>>      - Removed FLUSH bit, as it's unnecessary and slows down printing
>>      - Removed call to uart_set_options
>>      - Added mbox_free_channel calls to remove()
>>
>>   drivers/tty/serial/Kconfig       |   9 ++
>>   drivers/tty/serial/Makefile      |   1 +
>>   drivers/tty/serial/tegra-tcu.c   | 291 +++++++++++++++++++++++++++++++++++++++
>>   include/uapi/linux/serial_core.h |   3 +
>>   4 files changed, 304 insertions(+)
>>   create mode 100644 drivers/tty/serial/tegra-tcu.c
> 
> The driver looks good to me. But for my own understanding, is there some
> way we can make use of the multiplexing? That is, could we add a
> mechanism to have the driver filter out only a specific stream? Could we
> also specify which stream to send data back to? What happens by default?
> Which stream is data sent to?

There is no multiplexing on the producer/device side (i.e. what this 
driver does). The mailbox specified in device tree specifies the stream 
we send stuff to. The mailboxes are per-CPU (we use the CCPLEX mailbox 
here), so it cannot really be changed.

The consumer then sees the multiplexing in that it can receive from and 
transmit to all of the various CPUs in the system at the same time. AIUI 
there is also a secondary form of multiplexing where the outputs of 
different VMs can be multiplexed by the hypervisor, but I'm not very 
familiar with that.

Mikko

> 
> Thierry
> 

^ permalink raw reply

* Re: [PATCH v3 6/8] serial: Add Tegra Combined UART driver
From: Thierry Reding @ 2018-07-02 13:18 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: jassisinghbrar, gregkh, jonathanh, devicetree, linux-serial,
	linux-tegra, linux-arm-kernel, linux-kernel
In-Reply-To: <20180702114033.15654-7-mperttunen@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 1711 bytes --]

On Mon, Jul 02, 2018 at 02:40:31PM +0300, Mikko Perttunen wrote:
> The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
> multiplexing multiple "virtual UARTs" into a single hardware serial
> port. The TCU is the primary serial port on Tegra194 devices.
> 
> Add a TCU driver utilizing the mailbox framework, as the used mailboxes
> are part of Tegra HSP blocks that are already controlled by the Tegra
> HSP mailbox driver.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> 
> Notes:
>     v2:
>     - Removed (void) casts for unused variables.
>     - Changed the uart_set_options() call to be on one line, even if its
>       over 80 characters.
>     - Added defines for magic numbers.
>     - Style fixes.
>     - Changed Kconfig entry to depend on the Tegra HSP driver instead of
>       just the mailbox framework.
>     
>     v3:
>     - Removed FLUSH bit, as it's unnecessary and slows down printing
>     - Removed call to uart_set_options
>     - Added mbox_free_channel calls to remove()
> 
>  drivers/tty/serial/Kconfig       |   9 ++
>  drivers/tty/serial/Makefile      |   1 +
>  drivers/tty/serial/tegra-tcu.c   | 291 +++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/serial_core.h |   3 +
>  4 files changed, 304 insertions(+)
>  create mode 100644 drivers/tty/serial/tegra-tcu.c

The driver looks good to me. But for my own understanding, is there some
way we can make use of the multiplexing? That is, could we add a
mechanism to have the driver filter out only a specific stream? Could we
also specify which stream to send data back to? What happens by default?
Which stream is data sent to?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v5 0/7] add virt-dma support for imx-sdma
From: Vinod @ 2018-07-02 13:17 UTC (permalink / raw)
  To: Robin Gong
  Cc: l.stach@pengutronix.de, s.hauer@pengutronix.de,
	dan.j.williams@intel.com, gregkh@linuxfoundation.org,
	jslaby@suse.com, linux-serial@vger.kernel.org,
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <1530527489.15665.22.camel@nxp.com>

On 02-07-18, 02:32, Robin Gong wrote:
> Hi Vinod,
> 	Do you have any comment for this patchset? Lucas and Sascha
> acked it and tty patch already merged in.

I was actually waiting for ACK/action on patch 1 :)

I have reviewed the series, some nitpicks nothing major, so I have
applied this good cleanup and conversion :) along with fixes for typos
in code, GFP_ fix and removed unused variable.

My buildchain noticed warns (with W=1) on this file, esp now that
structs are moved around. Pls fix the struct descriptions.

Thanks
-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v3 5/8] mailbox: tegra-hsp: Add support for shared mailboxes
From: Thierry Reding @ 2018-07-02 13:13 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: jassisinghbrar, gregkh, jonathanh, devicetree, linux-serial,
	linux-tegra, linux-arm-kernel, linux-kernel
In-Reply-To: <20180702114033.15654-6-mperttunen@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

On Mon, Jul 02, 2018 at 02:40:30PM +0300, Mikko Perttunen wrote:
> The Tegra HSP block supports 'shared mailboxes' that are simple 32-bit
> registers consisting of a FULL bit in MSB position and 31 bits of data.
> The hardware can be configured to trigger interrupts when a mailbox
> is empty or full. Add support for these shared mailboxes to the HSP
> driver.
> 
> The initial use for the mailboxes is the Tegra Combined UART. For this
> purpose, we use interrupts to receive data, and spinning to wait for
> the transmit mailbox to be emptied to minimize unnecessary overhead.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> 
> Notes:
>     v3:
>     - Added define HSP_INT0_IE_FULL_SHIFT
>     - Added Jon's Reviewed-by
>     
>     v2:
>     - Added defines for some register fields
>     - Simplified bit looping logic in interrupt handler
>     - Changed write done polling to use readl_poll_timeout
>     - Removed unnecessary zero assignments
>     - Fixed two error cases in probe to do proper cleanup
> 
>  drivers/mailbox/tegra-hsp.c | 211 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 191 insertions(+), 20 deletions(-)

Looks good to me:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 4/8] mailbox: tegra-hsp: Refactor in preparation of mailboxes
From: Thierry Reding @ 2018-07-02 13:12 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: jassisinghbrar, gregkh, jonathanh, devicetree, linux-serial,
	linux-tegra, linux-arm-kernel, linux-kernel
In-Reply-To: <20180702114033.15654-5-mperttunen@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]

On Mon, Jul 02, 2018 at 02:40:29PM +0300, Mikko Perttunen wrote:
> The HSP driver is currently in many places written with the assumption
> of only supporting doorbells. Prepare for the addition of shared
> mailbox support by removing these assumptions and cleaning up the code.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> 
> Notes:
>     v2:
>     - Moved fixes for some style and other issues from the next patch
>       here, where they belong.
>     
>     v3:
>     - Added Jon's Reviewed-by.
> 
>  drivers/mailbox/tegra-hsp.c | 123 +++++++++++++++++++++++++++++---------------
>  1 file changed, 81 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
> index 0cde356c11ab..5dc21a6d01bb 100644
> --- a/drivers/mailbox/tegra-hsp.c
> +++ b/drivers/mailbox/tegra-hsp.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2016-2018, NVIDIA CORPORATION.  All rights reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms and conditions of the GNU General Public License,
> @@ -42,6 +42,7 @@ struct tegra_hsp_channel;
>  struct tegra_hsp;
>  
>  struct tegra_hsp_channel {
> +	unsigned int type;

I think it would've been slightly nicer to avoid the type member and the
differentiation made by having different mailbox operations for each
type, but since the mailbox operations are not per "channel", I don't
see how it could be done otherwise, so:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 3/8] mailbox: Add transmit done by blocking option
From: Thierry Reding @ 2018-07-02 13:09 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: jassisinghbrar, gregkh, jonathanh, devicetree, linux-serial,
	linux-tegra, linux-arm-kernel, linux-kernel
In-Reply-To: <20180702114033.15654-4-mperttunen@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 4892 bytes --]

On Mon, Jul 02, 2018 at 02:40:28PM +0300, Mikko Perttunen wrote:
> Add a new TXDONE option, TXDONE_BY_BLOCK. With this option, the
> send_data function of the mailbox driver is expected to block until
> the message has been sent. The new option is used with the Tegra
> Combined UART driver to minimize unnecessary overhead when transmitting
> data.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  drivers/mailbox/mailbox.c | 30 +++++++++++++++++++++---------
>  drivers/mailbox/mailbox.h |  1 +
>  2 files changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index 674b35f402f5..5c76b70e673c 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -53,6 +53,8 @@ static int add_to_rbuf(struct mbox_chan *chan, void *mssg)
>  	return idx;
>  }
>  
> +static void tx_tick(struct mbox_chan *chan, int r, bool submit_next);
> +
>  static void msg_submit(struct mbox_chan *chan)
>  {
>  	unsigned count, idx;
> @@ -60,10 +62,13 @@ static void msg_submit(struct mbox_chan *chan)
>  	void *data;
>  	int err = -EBUSY;
>  
> +next:
>  	spin_lock_irqsave(&chan->lock, flags);
>  
> -	if (!chan->msg_count || chan->active_req)
> -		goto exit;
> +	if (!chan->msg_count || chan->active_req) {
> +		spin_unlock_irqrestore(&chan->lock, flags);
> +		return;
> +	}
>  
>  	count = chan->msg_count;
>  	idx = chan->msg_free;
> @@ -82,15 +87,21 @@ static void msg_submit(struct mbox_chan *chan)
>  		chan->active_req = data;
>  		chan->msg_count--;
>  	}
> -exit:
> +
>  	spin_unlock_irqrestore(&chan->lock, flags);
>  
>  	if (!err && (chan->txdone_method & TXDONE_BY_POLL))
>  		/* kick start the timer immediately to avoid delays */
>  		hrtimer_start(&chan->mbox->poll_hrt, 0, HRTIMER_MODE_REL);
> +
> +	if (chan->txdone_method & TXDONE_BY_BLOCK) {
> +		tx_tick(chan, err, false);
> +		if (!err)
> +			goto next;
> +	}

This bit is slightly confusing: if check for err, but the call to the
tx_tick() function doesn't actually change err (it's passed by value).
Now, I don't have any great ideas on how to improve this, but perhaps
a simple comment would help clarify what's going on here, or maybe by
adding an extra blank line between the tx_tick() call and the
conditional would make it more obvious that these aren't related.

Also, it looks to me like this is actually modifying msg_submit() to
actually do more than just submit the next data from a message. This
effectively flushes the complete message via the mailbox, right?

Perhaps a slightly clearer implmentation would be to turn this into a
separate function that repeatedly calls msg_submit() in a loop or some
such.

>  }
>  
> -static void tx_tick(struct mbox_chan *chan, int r)
> +static void tx_tick(struct mbox_chan *chan, int r, bool submit_next)
>  {
>  	unsigned long flags;
>  	void *mssg;
> @@ -101,7 +112,8 @@ static void tx_tick(struct mbox_chan *chan, int r)
>  	spin_unlock_irqrestore(&chan->lock, flags);
>  
>  	/* Submit next message */
> -	msg_submit(chan);
> +	if (submit_next)
> +		msg_submit(chan);
>  
>  	if (!mssg)
>  		return;
> @@ -127,7 +139,7 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer)
>  		if (chan->active_req && chan->cl) {
>  			txdone = chan->mbox->ops->last_tx_done(chan);
>  			if (txdone)
> -				tx_tick(chan, 0);
> +				tx_tick(chan, 0, true);
>  			else
>  				resched = true;
>  		}
> @@ -176,7 +188,7 @@ void mbox_chan_txdone(struct mbox_chan *chan, int r)
>  		return;
>  	}
>  
> -	tx_tick(chan, r);
> +	tx_tick(chan, r, true);
>  }
>  EXPORT_SYMBOL_GPL(mbox_chan_txdone);
>  
> @@ -196,7 +208,7 @@ void mbox_client_txdone(struct mbox_chan *chan, int r)
>  		return;
>  	}
>  
> -	tx_tick(chan, r);
> +	tx_tick(chan, r, true);
>  }
>  EXPORT_SYMBOL_GPL(mbox_client_txdone);
>  
> @@ -275,7 +287,7 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
>  		ret = wait_for_completion_timeout(&chan->tx_complete, wait);
>  		if (ret == 0) {
>  			t = -ETIME;
> -			tx_tick(chan, t);
> +			tx_tick(chan, t, true);
>  		}
>  	}
>  
> diff --git a/drivers/mailbox/mailbox.h b/drivers/mailbox/mailbox.h
> index 456ba68513bb..ec68e2e28cd6 100644
> --- a/drivers/mailbox/mailbox.h
> +++ b/drivers/mailbox/mailbox.h
> @@ -10,5 +10,6 @@
>  #define TXDONE_BY_IRQ	BIT(0) /* controller has remote RTR irq */
>  #define TXDONE_BY_POLL	BIT(1) /* controller can read status of last TX */
>  #define TXDONE_BY_ACK	BIT(2) /* S/W ACK recevied by Client ticks the TX */
> +#define TXDONE_BY_BLOCK	BIT(3) /* mailbox driver send_data blocks until done */

Perhaps clarify here that it blocks until all data in the message has
been transmitted. As it is, this could mean just block until the current
datum is done transmitting.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 2/8] dt-bindings: serial: Add bindings for nvidia,tegra194-tcu
From: Thierry Reding @ 2018-07-02 12:51 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: jassisinghbrar, gregkh, jonathanh, devicetree, linux-serial,
	linux-tegra, linux-arm-kernel, linux-kernel
In-Reply-To: <20180702114033.15654-3-mperttunen@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On Mon, Jul 02, 2018 at 02:40:27PM +0300, Mikko Perttunen wrote:
> Add bindings for the Tegra Combined UART device used to talk to the
> UART console on Tegra194 systems.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> 
> Notes:
>     v2:
>     - Added Rob's Reviewed-by.
>     
>     v3:
>     - Added Jon's Acked-by.
> 
>  .../bindings/serial/nvidia,tegra194-tcu.txt        | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox