Re: [EXTERNAL] Re: [PATCH 4/5] inmates: uart-8250: Add MDR quirk for enabling UART

2019-06-12 Thread Ralf Ramsauer
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

2019-06-12 Thread 'Nikhil Devshatwar' via Jailhouse

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

2019-06-12 Thread Ralf Ramsauer



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

2019-06-12 Thread 'Nikhil Devshatwar' via Jailhouse

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

2019-05-23 Thread Ralf Ramsauer
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

2019-05-23 Thread 'Nikhil Devshatwar' via Jailhouse
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.