2018-11-01 01:53:53

by Marcel Ziswiler

[permalink] [raw]
Subject: [PATCH v1 0/3] clk/serial tegra: uart related fixes


This series features some UART related clock issue fix and clean-up.


Marcel Ziswiler (3):
clk: tegra: get rid of duplicate defines
clk: tegra: ignore unused vfir clock shared with uartb
serial: tegra: fix some spelling mistakes

drivers/clk/tegra/clk-tegra-periph.c | 5 +----
drivers/tty/serial/serial-tegra.c | 10 +++++-----
2 files changed, 6 insertions(+), 9 deletions(-)

--
2.14.5



2018-11-01 01:53:38

by Marcel Ziswiler

[permalink] [raw]
Subject: [PATCH v1 3/3] serial: tegra: fix some spelling mistakes

From: Marcel Ziswiler <[email protected]>

Fix a few spelling mistakes I stumbled upon while debugging a customers
UART issues.

Signed-off-by: Marcel Ziswiler <[email protected]>

---

drivers/tty/serial/serial-tegra.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index af2a29cfbbe9..d5269aaaf9b2 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -746,7 +746,7 @@ static void tegra_uart_stop_rx(struct uart_port *u)
if (!tup->rx_in_progress)
return;

- tegra_uart_wait_sym_time(tup, 1); /* wait a character interval */
+ tegra_uart_wait_sym_time(tup, 1); /* wait one character interval */

ier = tup->ier_shadow;
ier &= ~(UART_IER_RDI | UART_IER_RLSI | UART_IER_RTOIE |
@@ -887,7 +887,7 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
*
* EORD is different interrupt than RX_TIMEOUT - RX_TIMEOUT occurs when
* the DATA is sitting in the FIFO and couldn't be transferred to the
- * DMA as the DMA size alignment(4 bytes) is not met. EORD will be
+ * DMA as the DMA size alignment (4 bytes) is not met. EORD will be
* triggered when there is a pause of the incomming data stream for 4
* characters long.
*
@@ -1079,7 +1079,7 @@ static void tegra_uart_set_termios(struct uart_port *u,
if (tup->rts_active)
set_rts(tup, false);

- /* Clear all interrupts as configuration is going to be change */
+ /* Clear all interrupts as configuration is going to be changed */
tegra_uart_write(tup, tup->ier_shadow | UART_IER_RDI, UART_IER);
tegra_uart_read(tup, UART_IER);
tegra_uart_write(tup, 0, UART_IER);
@@ -1165,10 +1165,10 @@ static void tegra_uart_set_termios(struct uart_port *u,
/* update the port timeout based on new settings */
uart_update_timeout(u, termios->c_cflag, baud);

- /* Make sure all write has completed */
+ /* Make sure all writes have completed */
tegra_uart_read(tup, UART_IER);

- /* Reenable interrupt */
+ /* Re-enable interrupt */
tegra_uart_write(tup, tup->ier_shadow, UART_IER);
tegra_uart_read(tup, UART_IER);

--
2.14.5


2018-11-01 01:54:42

by Marcel Ziswiler

[permalink] [raw]
Subject: [PATCH v1 1/3] clk: tegra: get rid of duplicate defines

From: Marcel Ziswiler <[email protected]>

Get rid of 3 duplicate defines.

Signed-off-by: Marcel Ziswiler <[email protected]>

---

drivers/clk/tegra/clk-tegra-periph.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index 38c4eb28c8bf..cc5275ec2c01 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -79,7 +79,6 @@
#define CLK_SOURCE_3D 0x158
#define CLK_SOURCE_2D 0x15c
#define CLK_SOURCE_MPE 0x170
-#define CLK_SOURCE_UARTE 0x1c4
#define CLK_SOURCE_VI_SENSOR 0x1a8
#define CLK_SOURCE_VI 0x148
#define CLK_SOURCE_EPP 0x16c
@@ -117,8 +116,6 @@
#define CLK_SOURCE_ISP 0x144
#define CLK_SOURCE_SOR0 0x414
#define CLK_SOURCE_DPAUX 0x418
-#define CLK_SOURCE_SATA_OOB 0x420
-#define CLK_SOURCE_SATA 0x424
#define CLK_SOURCE_ENTROPY 0x628
#define CLK_SOURCE_VI_SENSOR2 0x658
#define CLK_SOURCE_HDMI_AUDIO 0x668
--
2.14.5


2018-11-01 01:54:57

by Marcel Ziswiler

[permalink] [raw]
Subject: [PATCH v1 2/3] clk: tegra: ignore unused vfir clock shared with uartb

From: Marcel Ziswiler <[email protected]>

As UARTB and VFIR share their clock enable bit it is rather unwise for
the kernel to turn off the VFIR one should that be unused (and
potentially vice versa but so far there anyway is no VFIR driver).

Without this patch trying to use UARTB with the regular 8250 driver
will freeze as soon as ttyS1 is accessed after boot. Luckily, using the
high-speed Tegra serial driver won't exhibit the issue as clocks are
dynamically enabled/disabled on every access.

This has been reproduced both on Apalis T30 as well as Apalis TK1 but
may be an issue on all Tegra UARTB's which share the clock enable with
VFIR.

Reported-by: Kory Swain <[email protected]>
Signed-off-by: Marcel Ziswiler <[email protected]>

---

drivers/clk/tegra/clk-tegra-periph.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index cc5275ec2c01..116c74340fb7 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -668,7 +668,7 @@ static struct tegra_periph_init_data periph_clks[] = {
MUX("hda", mux_pllp_pllc_clkm, CLK_SOURCE_HDA, 125, TEGRA_PERIPH_ON_APB, tegra_clk_hda_8),
MUX("hda2codec_2x", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_HDA2CODEC_2X, 111, TEGRA_PERIPH_ON_APB, tegra_clk_hda2codec_2x),
MUX8("hda2codec_2x", mux_pllp_pllc_plla_clkm, CLK_SOURCE_HDA2CODEC_2X, 111, TEGRA_PERIPH_ON_APB, tegra_clk_hda2codec_2x_8),
- MUX("vfir", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VFIR, 7, TEGRA_PERIPH_ON_APB, tegra_clk_vfir),
+ MUX_FLAGS("vfir", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VFIR, 7, TEGRA_PERIPH_ON_APB, tegra_clk_vfir, CLK_IGNORE_UNUSED),
MUX("sdmmc1", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SDMMC1, 14, TEGRA_PERIPH_ON_APB, tegra_clk_sdmmc1),
MUX("sdmmc2", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SDMMC2, 9, TEGRA_PERIPH_ON_APB, tegra_clk_sdmmc2),
MUX("sdmmc3", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SDMMC3, 69, TEGRA_PERIPH_ON_APB, tegra_clk_sdmmc3),
--
2.14.5


2018-11-01 08:42:43

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] clk: tegra: ignore unused vfir clock shared with uartb

On Thu, Nov 01, 2018 at 02:52:29AM +0100, Marcel Ziswiler wrote:
> From: Marcel Ziswiler <[email protected]>
>
> As UARTB and VFIR share their clock enable bit it is rather unwise for
> the kernel to turn off the VFIR one should that be unused (and
> potentially vice versa but so far there anyway is no VFIR driver).
>
> Without this patch trying to use UARTB with the regular 8250 driver
> will freeze as soon as ttyS1 is accessed after boot. Luckily, using the
> high-speed Tegra serial driver won't exhibit the issue as clocks are
> dynamically enabled/disabled on every access.
>
> This has been reproduced both on Apalis T30 as well as Apalis TK1 but
> may be an issue on all Tegra UARTB's which share the clock enable with
> VFIR.
>

Ah.. the correct fix for this is to initialize the enable_refcnt based on the
hw state. This is done in 9619dba8325fce098bbc9ee2911d1b0150fec0c9 for
periph gate clocks, but obviously also applies to normal periph clocks.

Peter.

> Reported-by: Kory Swain <[email protected]>
> Signed-off-by: Marcel Ziswiler <[email protected]>
>
> ---
>
> drivers/clk/tegra/clk-tegra-periph.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
> index cc5275ec2c01..116c74340fb7 100644
> --- a/drivers/clk/tegra/clk-tegra-periph.c
> +++ b/drivers/clk/tegra/clk-tegra-periph.c
> @@ -668,7 +668,7 @@ static struct tegra_periph_init_data periph_clks[] = {
> MUX("hda", mux_pllp_pllc_clkm, CLK_SOURCE_HDA, 125, TEGRA_PERIPH_ON_APB, tegra_clk_hda_8),
> MUX("hda2codec_2x", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_HDA2CODEC_2X, 111, TEGRA_PERIPH_ON_APB, tegra_clk_hda2codec_2x),
> MUX8("hda2codec_2x", mux_pllp_pllc_plla_clkm, CLK_SOURCE_HDA2CODEC_2X, 111, TEGRA_PERIPH_ON_APB, tegra_clk_hda2codec_2x_8),
> - MUX("vfir", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VFIR, 7, TEGRA_PERIPH_ON_APB, tegra_clk_vfir),
> + MUX_FLAGS("vfir", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_VFIR, 7, TEGRA_PERIPH_ON_APB, tegra_clk_vfir, CLK_IGNORE_UNUSED),
> MUX("sdmmc1", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SDMMC1, 14, TEGRA_PERIPH_ON_APB, tegra_clk_sdmmc1),
> MUX("sdmmc2", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SDMMC2, 9, TEGRA_PERIPH_ON_APB, tegra_clk_sdmmc2),
> MUX("sdmmc3", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SDMMC3, 69, TEGRA_PERIPH_ON_APB, tegra_clk_sdmmc3),
> --
> 2.14.5
>

2018-11-28 09:20:19

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v1 1/3] clk: tegra: get rid of duplicate defines

On Thu, Nov 01, 2018 at 02:52:28AM +0100, Marcel Ziswiler wrote:
> From: Marcel Ziswiler <[email protected]>
>
> Get rid of 3 duplicate defines.
>
> Signed-off-by: Marcel Ziswiler <[email protected]>
>
> ---
>
> drivers/clk/tegra/clk-tegra-periph.c | 3 ---
> 1 file changed, 3 deletions(-)

Acked-by: Thierry Reding <[email protected]>


Attachments:
(No filename) (376.00 B)
signature.asc (849.00 B)
Download all attachments

2018-11-28 09:21:05

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] serial: tegra: fix some spelling mistakes

On Thu, Nov 01, 2018 at 02:52:30AM +0100, Marcel Ziswiler wrote:
> From: Marcel Ziswiler <[email protected]>
>
> Fix a few spelling mistakes I stumbled upon while debugging a customers
> UART issues.
>
> Signed-off-by: Marcel Ziswiler <[email protected]>
>
> ---
>
> drivers/tty/serial/serial-tegra.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index af2a29cfbbe9..d5269aaaf9b2 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -746,7 +746,7 @@ static void tegra_uart_stop_rx(struct uart_port *u)
> if (!tup->rx_in_progress)
> return;
>
> - tegra_uart_wait_sym_time(tup, 1); /* wait a character interval */
> + tegra_uart_wait_sym_time(tup, 1); /* wait one character interval */
>
> ier = tup->ier_shadow;
> ier &= ~(UART_IER_RDI | UART_IER_RLSI | UART_IER_RTOIE |
> @@ -887,7 +887,7 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
> *
> * EORD is different interrupt than RX_TIMEOUT - RX_TIMEOUT occurs when
> * the DATA is sitting in the FIFO and couldn't be transferred to the
> - * DMA as the DMA size alignment(4 bytes) is not met. EORD will be
> + * DMA as the DMA size alignment (4 bytes) is not met. EORD will be
> * triggered when there is a pause of the incomming data stream for 4
> * characters long.
> *
> @@ -1079,7 +1079,7 @@ static void tegra_uart_set_termios(struct uart_port *u,
> if (tup->rts_active)
> set_rts(tup, false);
>
> - /* Clear all interrupts as configuration is going to be change */
> + /* Clear all interrupts as configuration is going to be changed */
> tegra_uart_write(tup, tup->ier_shadow | UART_IER_RDI, UART_IER);
> tegra_uart_read(tup, UART_IER);
> tegra_uart_write(tup, 0, UART_IER);
> @@ -1165,10 +1165,10 @@ static void tegra_uart_set_termios(struct uart_port *u,
> /* update the port timeout based on new settings */
> uart_update_timeout(u, termios->c_cflag, baud);
>
> - /* Make sure all write has completed */
> + /* Make sure all writes have completed */
> tegra_uart_read(tup, UART_IER);
>
> - /* Reenable interrupt */
> + /* Re-enable interrupt */

I think both forms are valid. Either way:

Acked-by: Thierry Reding <[email protected]>


Attachments:
(No filename) (2.34 kB)
signature.asc (849.00 B)
Download all attachments

2018-11-28 09:26:20

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] clk: tegra: ignore unused vfir clock shared with uartb

On Thu, Nov 01, 2018 at 10:41:52AM +0200, Peter De Schrijver wrote:
> On Thu, Nov 01, 2018 at 02:52:29AM +0100, Marcel Ziswiler wrote:
> > From: Marcel Ziswiler <[email protected]>
> >
> > As UARTB and VFIR share their clock enable bit it is rather unwise for
> > the kernel to turn off the VFIR one should that be unused (and
> > potentially vice versa but so far there anyway is no VFIR driver).
> >
> > Without this patch trying to use UARTB with the regular 8250 driver
> > will freeze as soon as ttyS1 is accessed after boot. Luckily, using the
> > high-speed Tegra serial driver won't exhibit the issue as clocks are
> > dynamically enabled/disabled on every access.
> >
> > This has been reproduced both on Apalis T30 as well as Apalis TK1 but
> > may be an issue on all Tegra UARTB's which share the clock enable with
> > VFIR.
> >
>
> Ah.. the correct fix for this is to initialize the enable_refcnt based on the
> hw state. This is done in 9619dba8325fce098bbc9ee2911d1b0150fec0c9 for
> periph gate clocks, but obviously also applies to normal periph clocks.

Hi Marcel,

were you going to send a new version with the alternative fix as
suggested by Peter?

Thierry


Attachments:
(No filename) (1.19 kB)
signature.asc (849.00 B)
Download all attachments

2018-12-10 22:12:35

by Marcel Ziswiler

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] clk: tegra: ignore unused vfir clock shared with uartb



On November 28, 2018 10:24:04 AM GMT+01:00, Thierry Reding <[email protected]> wrote:
>On Thu, Nov 01, 2018 at 10:41:52AM +0200, Peter De Schrijver wrote:
>> On Thu, Nov 01, 2018 at 02:52:29AM +0100, Marcel Ziswiler wrote:
>> > From: Marcel Ziswiler <[email protected]>
>> >
>> > As UARTB and VFIR share their clock enable bit it is rather unwise
>for
>> > the kernel to turn off the VFIR one should that be unused (and
>> > potentially vice versa but so far there anyway is no VFIR driver).
>> >
>> > Without this patch trying to use UARTB with the regular 8250 driver
>> > will freeze as soon as ttyS1 is accessed after boot. Luckily, using
>the
>> > high-speed Tegra serial driver won't exhibit the issue as clocks
>are
>> > dynamically enabled/disabled on every access.
>> >
>> > This has been reproduced both on Apalis T30 as well as Apalis TK1
>but
>> > may be an issue on all Tegra UARTB's which share the clock enable
>with
>> > VFIR.
>> >
>>
>> Ah.. the correct fix for this is to initialize the enable_refcnt
>based on the
>> hw state. This is done in 9619dba8325fce098bbc9ee2911d1b0150fec0c9
>for
>> periph gate clocks, but obviously also applies to normal periph
>clocks.
>
>Hi Marcel,
>
>were you going to send a new version with the alternative fix as
>suggested by Peter?

Yes, sorry. Let me look at that now.

>Thierry

2018-12-10 23:14:56

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v1 1/3] clk: tegra: get rid of duplicate defines

Quoting Marcel Ziswiler (2018-10-31 18:52:28)
> From: Marcel Ziswiler <[email protected]>
>
> Get rid of 3 duplicate defines.
>
> Signed-off-by: Marcel Ziswiler <[email protected]>
>
> ---

Applied to clk-next