Re: [EXTERNAL] Re: [PATCH 4/5] inmates: uart-8250: Add MDR quirk for enabling UART
On 6/12/19 6:24 PM, 'Nikhil Devshatwar' via Jailhouse wrote: > On 12/06/19 9:30 PM, Ralf Ramsauer wrote: >> >> >> On 6/12/19 5:48 PM, Nikhil Devshatwar wrote: >>> On 24/05/19 4:15 AM, Ralf Ramsauer wrote: Hi Lokesh, On 5/23/19 11:16 PM, 'Nikhil Devshatwar' via Jailhouse wrote: > UART is disabled by default on TI platforms and must be enabled > on some platforms via the MDR register. > Do this as part of uart_init for 8250 driver > > Signed-off-by: Nikhil Devshatwar > Signed-off-by: Lokesh Vutla > --- > inmates/lib/uart-8250.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/inmates/lib/uart-8250.c b/inmates/lib/uart-8250.c > index fb7940d2..42b0979c 100644 > --- a/inmates/lib/uart-8250.c > +++ b/inmates/lib/uart-8250.c > @@ -49,6 +49,7 @@ > #define UART_LCR_DLAB 0x80 > #define UART_LSR 0x5 > #define UART_LSR_THRE 0x20 > +#define UART_MDR1 0x8 > static void reg_out_mmio32(struct uart_chip *chip, unsigned int > reg, u32 value) > { > @@ -67,6 +68,9 @@ static void uart_8250_init(struct uart_chip *chip) > chip->reg_out(chip, UART_DLL, chip->divider); > chip->reg_out(chip, UART_DLM, 0); > chip->reg_out(chip, UART_LCR, UART_LCR_8N1); > +#ifdef CONFIG_TI_16550_MDR_QUIRK >> >> if (comm_region->console.flags & JAILHOUSE_CON_MDS_QUIRK) >> > + chip->reg_out(chip, UART_MDR1, 0); > +#endif I think it's better to encode this in struct uart's flags. We still have some bits free there. >> >> s/struct uart/struct jailhouse_console/ > > Oh okay. > Using console flag for setting up uart didn't sound right to me. > Here the inmate has a dedicated uart instance, not shared with the root > cell. But that's okay. The inmate also has a dedicated struct jailhouse_console that is not shared with the root cell config. Am I missing something? Ralf > > If that's the preferred option, I'll do that. > Of course both uarts have the same behavior. > > Nikhil D > >> >>> >>> I looked up in the source. >>> The flags are only for the console. >>> Here we need this to be written from the inmate, which doesn't know >>> about the console flags. >> >> We do pass the flags to the inmate via the communication region. See >> hypervisor/control.c:657. Just pick a reserved bit. >> >> Ralf >> >>> Which struct uart are you referring to? >>> >>> Nikhil D >>> It's better to not reintroduce compile time switches, it took a while to get rid of (most of) them. Ralf > } > } > >>> > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/4a3f2a5d-1916-3a0f-763e-dfd865229397%40oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
Re: [EXTERNAL] Re: [PATCH 4/5] inmates: uart-8250: Add MDR quirk for enabling UART
On 12/06/19 9:30 PM, Ralf Ramsauer wrote: On 6/12/19 5:48 PM, Nikhil Devshatwar wrote: On 24/05/19 4:15 AM, Ralf Ramsauer wrote: Hi Lokesh, On 5/23/19 11:16 PM, 'Nikhil Devshatwar' via Jailhouse wrote: UART is disabled by default on TI platforms and must be enabled on some platforms via the MDR register. Do this as part of uart_init for 8250 driver Signed-off-by: Nikhil Devshatwar Signed-off-by: Lokesh Vutla --- inmates/lib/uart-8250.c | 4 1 file changed, 4 insertions(+) diff --git a/inmates/lib/uart-8250.c b/inmates/lib/uart-8250.c index fb7940d2..42b0979c 100644 --- a/inmates/lib/uart-8250.c +++ b/inmates/lib/uart-8250.c @@ -49,6 +49,7 @@ #define UART_LCR_DLAB 0x80 #define UART_LSR 0x5 #define UART_LSR_THRE 0x20 +#define UART_MDR1 0x8 static void reg_out_mmio32(struct uart_chip *chip, unsigned int reg, u32 value) { @@ -67,6 +68,9 @@ static void uart_8250_init(struct uart_chip *chip) chip->reg_out(chip, UART_DLL, chip->divider); chip->reg_out(chip, UART_DLM, 0); chip->reg_out(chip, UART_LCR, UART_LCR_8N1); +#ifdef CONFIG_TI_16550_MDR_QUIRK if (comm_region->console.flags & JAILHOUSE_CON_MDS_QUIRK) + chip->reg_out(chip, UART_MDR1, 0); +#endif I think it's better to encode this in struct uart's flags. We still have some bits free there. s/struct uart/struct jailhouse_console/ Oh okay. Using console flag for setting up uart didn't sound right to me. Here the inmate has a dedicated uart instance, not shared with the root cell. If that's the preferred option, I'll do that. Of course both uarts have the same behavior. Nikhil D I looked up in the source. The flags are only for the console. Here we need this to be written from the inmate, which doesn't know about the console flags. We do pass the flags to the inmate via the communication region. See hypervisor/control.c:657. Just pick a reserved bit. Ralf Which struct uart are you referring to? Nikhil D It's better to not reintroduce compile time switches, it took a while to get rid of (most of) them. Ralf } } -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/e6cd860e-f4c8-47d3-ea32-08d6520bcaf0%40ti.com. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 4/5] inmates: uart-8250: Add MDR quirk for enabling UART
On 6/12/19 5:48 PM, Nikhil Devshatwar wrote: > On 24/05/19 4:15 AM, Ralf Ramsauer wrote: >> Hi Lokesh, >> >> On 5/23/19 11:16 PM, 'Nikhil Devshatwar' via Jailhouse wrote: >>> UART is disabled by default on TI platforms and must be enabled >>> on some platforms via the MDR register. >>> Do this as part of uart_init for 8250 driver >>> >>> Signed-off-by: Nikhil Devshatwar >>> Signed-off-by: Lokesh Vutla >>> --- >>> inmates/lib/uart-8250.c | 4 >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/inmates/lib/uart-8250.c b/inmates/lib/uart-8250.c >>> index fb7940d2..42b0979c 100644 >>> --- a/inmates/lib/uart-8250.c >>> +++ b/inmates/lib/uart-8250.c >>> @@ -49,6 +49,7 @@ >>> #define UART_LCR_DLAB 0x80 >>> #define UART_LSR 0x5 >>> #define UART_LSR_THRE 0x20 >>> +#define UART_MDR1 0x8 >>> static void reg_out_mmio32(struct uart_chip *chip, unsigned int >>> reg, u32 value) >>> { >>> @@ -67,6 +68,9 @@ static void uart_8250_init(struct uart_chip *chip) >>> chip->reg_out(chip, UART_DLL, chip->divider); >>> chip->reg_out(chip, UART_DLM, 0); >>> chip->reg_out(chip, UART_LCR, UART_LCR_8N1); >>> +#ifdef CONFIG_TI_16550_MDR_QUIRK if (comm_region->console.flags & JAILHOUSE_CON_MDS_QUIRK) >>> + chip->reg_out(chip, UART_MDR1, 0); >>> +#endif >> >> I think it's better to encode this in struct uart's flags. We still have >> some bits free there. s/struct uart/struct jailhouse_console/ > > I looked up in the source. > The flags are only for the console. > Here we need this to be written from the inmate, which doesn't know > about the console flags. We do pass the flags to the inmate via the communication region. See hypervisor/control.c:657. Just pick a reserved bit. Ralf > Which struct uart are you referring to? > > Nikhil D > >> >> It's better to not reintroduce compile time switches, it took a while to >> get rid of (most of) them. >> >> Ralf >> >>> } >>> } >>> > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/6b51d8be-2267-32df-3ee0-1bfb07c81851%40oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 4/5] inmates: uart-8250: Add MDR quirk for enabling UART
On 24/05/19 4:15 AM, Ralf Ramsauer wrote: Hi Lokesh, On 5/23/19 11:16 PM, 'Nikhil Devshatwar' via Jailhouse wrote: UART is disabled by default on TI platforms and must be enabled on some platforms via the MDR register. Do this as part of uart_init for 8250 driver Signed-off-by: Nikhil Devshatwar Signed-off-by: Lokesh Vutla --- inmates/lib/uart-8250.c | 4 1 file changed, 4 insertions(+) diff --git a/inmates/lib/uart-8250.c b/inmates/lib/uart-8250.c index fb7940d2..42b0979c 100644 --- a/inmates/lib/uart-8250.c +++ b/inmates/lib/uart-8250.c @@ -49,6 +49,7 @@ #define UART_LCR_DLAB0x80 #define UART_LSR 0x5 #define UART_LSR_THRE0x20 +#define UART_MDR1 0x8 static void reg_out_mmio32(struct uart_chip *chip, unsigned int reg, u32 value) { @@ -67,6 +68,9 @@ static void uart_8250_init(struct uart_chip *chip) chip->reg_out(chip, UART_DLL, chip->divider); chip->reg_out(chip, UART_DLM, 0); chip->reg_out(chip, UART_LCR, UART_LCR_8N1); +#ifdef CONFIG_TI_16550_MDR_QUIRK + chip->reg_out(chip, UART_MDR1, 0); +#endif I think it's better to encode this in struct uart's flags. We still have some bits free there. I looked up in the source. The flags are only for the console. Here we need this to be written from the inmate, which doesn't know about the console flags. Which struct uart are you referring to? Nikhil D It's better to not reintroduce compile time switches, it took a while to get rid of (most of) them. Ralf } } -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/8cb3e54a-a0f6-38c3-626d-0dbece8b2025%40ti.com. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 4/5] inmates: uart-8250: Add MDR quirk for enabling UART
Hi Lokesh, On 5/23/19 11:16 PM, 'Nikhil Devshatwar' via Jailhouse wrote: > UART is disabled by default on TI platforms and must be enabled > on some platforms via the MDR register. > Do this as part of uart_init for 8250 driver > > Signed-off-by: Nikhil Devshatwar > Signed-off-by: Lokesh Vutla > --- > inmates/lib/uart-8250.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/inmates/lib/uart-8250.c b/inmates/lib/uart-8250.c > index fb7940d2..42b0979c 100644 > --- a/inmates/lib/uart-8250.c > +++ b/inmates/lib/uart-8250.c > @@ -49,6 +49,7 @@ > #define UART_LCR_DLAB 0x80 > #define UART_LSR 0x5 > #define UART_LSR_THRE 0x20 > +#define UART_MDR1 0x8 > > static void reg_out_mmio32(struct uart_chip *chip, unsigned int reg, u32 > value) > { > @@ -67,6 +68,9 @@ static void uart_8250_init(struct uart_chip *chip) > chip->reg_out(chip, UART_DLL, chip->divider); > chip->reg_out(chip, UART_DLM, 0); > chip->reg_out(chip, UART_LCR, UART_LCR_8N1); > +#ifdef CONFIG_TI_16550_MDR_QUIRK > + chip->reg_out(chip, UART_MDR1, 0); > +#endif I think it's better to encode this in struct uart's flags. We still have some bits free there. It's better to not reintroduce compile time switches, it took a while to get rid of (most of) them. Ralf > } > } > > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/ec13c47e-9ff0-8fe2-f895-27213a5b7b6a%40oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
[PATCH 4/5] inmates: uart-8250: Add MDR quirk for enabling UART
UART is disabled by default on TI platforms and must be enabled on some platforms via the MDR register. Do this as part of uart_init for 8250 driver Signed-off-by: Nikhil Devshatwar Signed-off-by: Lokesh Vutla --- inmates/lib/uart-8250.c | 4 1 file changed, 4 insertions(+) diff --git a/inmates/lib/uart-8250.c b/inmates/lib/uart-8250.c index fb7940d2..42b0979c 100644 --- a/inmates/lib/uart-8250.c +++ b/inmates/lib/uart-8250.c @@ -49,6 +49,7 @@ #define UART_LCR_DLAB 0x80 #define UART_LSR 0x5 #define UART_LSR_THRE 0x20 +#define UART_MDR1 0x8 static void reg_out_mmio32(struct uart_chip *chip, unsigned int reg, u32 value) { @@ -67,6 +68,9 @@ static void uart_8250_init(struct uart_chip *chip) chip->reg_out(chip, UART_DLL, chip->divider); chip->reg_out(chip, UART_DLM, 0); chip->reg_out(chip, UART_LCR, UART_LCR_8N1); +#ifdef CONFIG_TI_16550_MDR_QUIRK + chip->reg_out(chip, UART_MDR1, 0); +#endif } } -- 2.17.1 -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190523211623.9718-5-nikhil.nd%40ti.com. For more options, visit https://groups.google.com/d/optout.