Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-23 Thread Masahiro Yamada 
2015-10-23 0:34 GMT+09:00 Rob Herring :
> On Tue, Oct 20, 2015 at 8:20 PM, Masahiro Yamada
>  wrote:
>> Hi Peter,
>> (+ Rob Herring, Stefan Agner)
>>
>> 2015-10-20 23:00 GMT+09:00 Peter Hurley :
>>> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
 The input clock frequency varies from device to device, but the
 earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
 impossible to set the correct divisor to the register.
>>>
>>> So the bootloader hasn't setup the serial port?
>>
>> It does.
>> I use U-boot and the serial port is already set up by U-boot.
>
> This problem is now moving into u-boot which is using DT for configuration.
>
>> But, earlycon setup functions update hardware registers.
>> See  early_serial8250_setup(), ingenic_early_console_setup(), etc.
>>
>>
>> Without port->uartclk set to a valid value,
>> the init code in earlycon setup does not make sense.
>>
>>
>> What I want to clarify is,
>> what should we do in the earlycon setup function?
>>
>> Currently, I see
>>  [1] set device->con->write callback
>>  [2] initialize UART port registers
>>
>>
>> For [2], we need to know baudrate and input clock frequency.
>> (and the latter is missing, that's why my patch is here.)
>
> I'm missing context of what you did, but it needs to be parse-able
> from a flattened tree. My suggestion would be use clock-frequency
> property in the uart node. We should be able to parse that. This came
> up with u-boot as well[1].

Right.

But, I think things have been going wrong
since SPL started to parse a device tree.




-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-23 Thread Masahiro Yamada 
2015-10-23 0:34 GMT+09:00 Rob Herring :
> On Tue, Oct 20, 2015 at 8:20 PM, Masahiro Yamada
>  wrote:
>> Hi Peter,
>> (+ Rob Herring, Stefan Agner)
>>
>> 2015-10-20 23:00 GMT+09:00 Peter Hurley :
>>> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
 The input clock frequency varies from device to device, but the
 earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
 impossible to set the correct divisor to the register.
>>>
>>> So the bootloader hasn't setup the serial port?
>>
>> It does.
>> I use U-boot and the serial port is already set up by U-boot.
>
> This problem is now moving into u-boot which is using DT for configuration.
>
>> But, earlycon setup functions update hardware registers.
>> See  early_serial8250_setup(), ingenic_early_console_setup(), etc.
>>
>>
>> Without port->uartclk set to a valid value,
>> the init code in earlycon setup does not make sense.
>>
>>
>> What I want to clarify is,
>> what should we do in the earlycon setup function?
>>
>> Currently, I see
>>  [1] set device->con->write callback
>>  [2] initialize UART port registers
>>
>>
>> For [2], we need to know baudrate and input clock frequency.
>> (and the latter is missing, that's why my patch is here.)
>
> I'm missing context of what you did, but it needs to be parse-able
> from a flattened tree. My suggestion would be use clock-frequency
> property in the uart node. We should be able to parse that. This came
> up with u-boot as well[1].

Right.

But, I think things have been going wrong
since SPL started to parse a device tree.




-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-22 Thread Rob Herring 
On Tue, Oct 20, 2015 at 8:20 PM, Masahiro Yamada
 wrote:
> Hi Peter,
> (+ Rob Herring, Stefan Agner)
>
> 2015-10-20 23:00 GMT+09:00 Peter Hurley :
>> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
>>> The input clock frequency varies from device to device, but the
>>> earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
>>> impossible to set the correct divisor to the register.
>>
>> So the bootloader hasn't setup the serial port?
>
> It does.
> I use U-boot and the serial port is already set up by U-boot.

This problem is now moving into u-boot which is using DT for configuration.

> But, earlycon setup functions update hardware registers.
> See  early_serial8250_setup(), ingenic_early_console_setup(), etc.
>
>
> Without port->uartclk set to a valid value,
> the init code in earlycon setup does not make sense.
>
>
> What I want to clarify is,
> what should we do in the earlycon setup function?
>
> Currently, I see
>  [1] set device->con->write callback
>  [2] initialize UART port registers
>
>
> For [2], we need to know baudrate and input clock frequency.
> (and the latter is missing, that's why my patch is here.)

I'm missing context of what you did, but it needs to be parse-able
from a flattened tree. My suggestion would be use clock-frequency
property in the uart node. We should be able to parse that. This came
up with u-boot as well[1].


> In order to be independent of a boot loader, we also need
>   [3]  pinctrl (pin-muxing)

We have to draw the line somewhere and I think this falls below it.
What if we have to turn on a regulator for the pins or RS-232
transceiver, turn on power domain, take peripheral out of reset, and
the list goes on... Hopefully a debug path would never be that
complicated.

> But, it is difficult to handle pinctrl in the earlycon framework.

earlycon is for debug. Either fix your bootloader or you need some
platform specific setup hacks to get the h/w in a working state if you
need to debug.

Rob

[1] https://patchwork.ozlabs.org/patch/492697/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-22 Thread Rob Herring 
On Tue, Oct 20, 2015 at 8:20 PM, Masahiro Yamada
 wrote:
> Hi Peter,
> (+ Rob Herring, Stefan Agner)
>
> 2015-10-20 23:00 GMT+09:00 Peter Hurley :
>> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
>>> The input clock frequency varies from device to device, but the
>>> earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
>>> impossible to set the correct divisor to the register.
>>
>> So the bootloader hasn't setup the serial port?
>
> It does.
> I use U-boot and the serial port is already set up by U-boot.

This problem is now moving into u-boot which is using DT for configuration.

> But, earlycon setup functions update hardware registers.
> See  early_serial8250_setup(), ingenic_early_console_setup(), etc.
>
>
> Without port->uartclk set to a valid value,
> the init code in earlycon setup does not make sense.
>
>
> What I want to clarify is,
> what should we do in the earlycon setup function?
>
> Currently, I see
>  [1] set device->con->write callback
>  [2] initialize UART port registers
>
>
> For [2], we need to know baudrate and input clock frequency.
> (and the latter is missing, that's why my patch is here.)

I'm missing context of what you did, but it needs to be parse-able
from a flattened tree. My suggestion would be use clock-frequency
property in the uart node. We should be able to parse that. This came
up with u-boot as well[1].


> In order to be independent of a boot loader, we also need
>   [3]  pinctrl (pin-muxing)

We have to draw the line somewhere and I think this falls below it.
What if we have to turn on a regulator for the pins or RS-232
transceiver, turn on power domain, take peripheral out of reset, and
the list goes on... Hopefully a debug path would never be that
complicated.

> But, it is difficult to handle pinctrl in the earlycon framework.

earlycon is for debug. Either fix your bootloader or you need some
platform specific setup hacks to get the h/w in a working state if you
need to debug.

Rob

[1] https://patchwork.ozlabs.org/patch/492697/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-21 Thread Masahiro Yamada 
Hi Peter,

2015-10-22 0:35 GMT+09:00 Peter Hurley :
> Hi Masahiro,
>
> On 10/21/2015 11:31 AM, Masahiro Yamada wrote:
>> If it is always correct to preserve the initialization done by boot-loader,
>> the following code in 8250_early.c does not make sense.
>
> It's not always correct to preserve the initialization by the bootloader.
> For example, on my legacy x86 workstation, the bootloader does not
> initialize the h/w so when I want 8250 earlycon, I must specify the
> baud rate.

So, the input clock on the machine matches the following macro.

#define BASE_BAUD (1843200/16)


For ARM boards, we should depend on a boot loader.
Is this what you mean?

-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-21 Thread Peter Hurley 
Hi Masahiro,

On 10/21/2015 11:31 AM, Masahiro Yamada wrote:
> If it is always correct to preserve the initialization done by boot-loader,
> the following code in 8250_early.c does not make sense.

It's not always correct to preserve the initialization by the bootloader.
For example, on my legacy x86 workstation, the bootloader does not
initialize the h/w so when I want 8250 earlycon, I must specify the
baud rate.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-21 Thread Masahiro Yamada 
Hi Peter.


2015-10-21 21:27 GMT+09:00 Peter Hurley :
> On 10/20/2015 09:20 PM, Masahiro Yamada wrote:
>> Hi Peter,
>> (+ Rob Herring, Stefan Agner)
>>
>> 2015-10-20 23:00 GMT+09:00 Peter Hurley :
>>> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
 The input clock frequency varies from device to device, but the
 earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
 impossible to set the correct divisor to the register.
>>>
>>> So the bootloader hasn't setup the serial port?
>>
>> It does.
>> I use U-boot and the serial port is already set up by U-boot.
>>
>>
>> But, earlycon setup functions update hardware registers.
>> See  early_serial8250_setup(), ingenic_early_console_setup(), etc.
>>
>>
>> Without port->uartclk set to a valid value,
>> the init code in earlycon setup does not make sense.
>>
>>
>> What I want to clarify is,
>> what should we do in the earlycon setup function?
>>
>> Currently, I see
>>  [1] set device->con->write callback
>>  [2] initialize UART port registers
>>
>>
>> For [2], we need to know baudrate and input clock frequency.
>> (and the latter is missing, that's why my patch is here.)
>>
>>
>> In order to be independent of a boot loader, we also need
>>   [3]  pinctrl (pin-muxing)
>>
>> But, it is difficult to handle pinctrl in the earlycon framework.
>>
>>
>> If we depend on a boot loader, [2] is meaningless.   [1] is enough.
>
> The 8250 earlycon doesn't try to initialize the hardware (other than
> masking interrupts) if the baud rate is uninitialized
> (!device->baud in early_serial8250_setup()).
>

Right.

If it is always correct to preserve the initialization done by boot-loader,
the following code in 8250_early.c does not make sense.

Delete?


divisor = DIV_ROUND_CLOSEST(port->uartclk, 16 * device->baud);
c = serial8250_early_in(port, UART_LCR);
serial8250_early_out(port, UART_LCR, c | UART_LCR_DLAB);
serial8250_early_out(port, UART_DLL, divisor & 0xff);
serial8250_early_out(port, UART_DLM, (divisor >> 8) & 0xff);
serial8250_early_out(port, UART_LCR, c & ~UART_LCR_DLAB);








-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-21 Thread Peter Hurley 
On 10/20/2015 09:20 PM, Masahiro Yamada wrote:
> Hi Peter,
> (+ Rob Herring, Stefan Agner)
> 
> 2015-10-20 23:00 GMT+09:00 Peter Hurley :
>> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
>>> The input clock frequency varies from device to device, but the
>>> earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
>>> impossible to set the correct divisor to the register.
>>
>> So the bootloader hasn't setup the serial port?
> 
> It does.
> I use U-boot and the serial port is already set up by U-boot.
> 
> 
> But, earlycon setup functions update hardware registers.
> See  early_serial8250_setup(), ingenic_early_console_setup(), etc.
> 
> 
> Without port->uartclk set to a valid value,
> the init code in earlycon setup does not make sense.
> 
> 
> What I want to clarify is,
> what should we do in the earlycon setup function?
> 
> Currently, I see
>  [1] set device->con->write callback
>  [2] initialize UART port registers
> 
> 
> For [2], we need to know baudrate and input clock frequency.
> (and the latter is missing, that's why my patch is here.)
> 
> 
> In order to be independent of a boot loader, we also need
>   [3]  pinctrl (pin-muxing)
> 
> But, it is difficult to handle pinctrl in the earlycon framework.
> 
> 
> If we depend on a boot loader, [2] is meaningless.   [1] is enough.

The 8250 earlycon doesn't try to initialize the hardware (other than
masking interrupts) if the baud rate is uninitialized
(!device->baud in early_serial8250_setup()).

Rather than initializing the h/w to a default of 115200 in
ingenic_early_console_setup() when device->baud == 0, it should just
mask interrupts. That way the bootloader initialization will be
preserved when the options are unspecified on the command line, such as:

earlycon=jz4740_uart,mmio32,0x43fb


Regards,
Peter Hurley

PS - Apologies for not reviewing the Ingenic 8250 driver at submission
time.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-21 Thread Masahiro Yamada 
Hi Peter,

2015-10-22 0:35 GMT+09:00 Peter Hurley :
> Hi Masahiro,
>
> On 10/21/2015 11:31 AM, Masahiro Yamada wrote:
>> If it is always correct to preserve the initialization done by boot-loader,
>> the following code in 8250_early.c does not make sense.
>
> It's not always correct to preserve the initialization by the bootloader.
> For example, on my legacy x86 workstation, the bootloader does not
> initialize the h/w so when I want 8250 earlycon, I must specify the
> baud rate.

So, the input clock on the machine matches the following macro.

#define BASE_BAUD (1843200/16)


For ARM boards, we should depend on a boot loader.
Is this what you mean?

-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-21 Thread Masahiro Yamada 
Hi Peter.


2015-10-21 21:27 GMT+09:00 Peter Hurley :
> On 10/20/2015 09:20 PM, Masahiro Yamada wrote:
>> Hi Peter,
>> (+ Rob Herring, Stefan Agner)
>>
>> 2015-10-20 23:00 GMT+09:00 Peter Hurley :
>>> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
 The input clock frequency varies from device to device, but the
 earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
 impossible to set the correct divisor to the register.
>>>
>>> So the bootloader hasn't setup the serial port?
>>
>> It does.
>> I use U-boot and the serial port is already set up by U-boot.
>>
>>
>> But, earlycon setup functions update hardware registers.
>> See  early_serial8250_setup(), ingenic_early_console_setup(), etc.
>>
>>
>> Without port->uartclk set to a valid value,
>> the init code in earlycon setup does not make sense.
>>
>>
>> What I want to clarify is,
>> what should we do in the earlycon setup function?
>>
>> Currently, I see
>>  [1] set device->con->write callback
>>  [2] initialize UART port registers
>>
>>
>> For [2], we need to know baudrate and input clock frequency.
>> (and the latter is missing, that's why my patch is here.)
>>
>>
>> In order to be independent of a boot loader, we also need
>>   [3]  pinctrl (pin-muxing)
>>
>> But, it is difficult to handle pinctrl in the earlycon framework.
>>
>>
>> If we depend on a boot loader, [2] is meaningless.   [1] is enough.
>
> The 8250 earlycon doesn't try to initialize the hardware (other than
> masking interrupts) if the baud rate is uninitialized
> (!device->baud in early_serial8250_setup()).
>

Right.

If it is always correct to preserve the initialization done by boot-loader,
the following code in 8250_early.c does not make sense.

Delete?


divisor = DIV_ROUND_CLOSEST(port->uartclk, 16 * device->baud);
c = serial8250_early_in(port, UART_LCR);
serial8250_early_out(port, UART_LCR, c | UART_LCR_DLAB);
serial8250_early_out(port, UART_DLL, divisor & 0xff);
serial8250_early_out(port, UART_DLM, (divisor >> 8) & 0xff);
serial8250_early_out(port, UART_LCR, c & ~UART_LCR_DLAB);








-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-21 Thread Peter Hurley 
Hi Masahiro,

On 10/21/2015 11:31 AM, Masahiro Yamada wrote:
> If it is always correct to preserve the initialization done by boot-loader,
> the following code in 8250_early.c does not make sense.

It's not always correct to preserve the initialization by the bootloader.
For example, on my legacy x86 workstation, the bootloader does not
initialize the h/w so when I want 8250 earlycon, I must specify the
baud rate.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-21 Thread Peter Hurley 
On 10/20/2015 09:20 PM, Masahiro Yamada wrote:
> Hi Peter,
> (+ Rob Herring, Stefan Agner)
> 
> 2015-10-20 23:00 GMT+09:00 Peter Hurley :
>> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
>>> The input clock frequency varies from device to device, but the
>>> earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
>>> impossible to set the correct divisor to the register.
>>
>> So the bootloader hasn't setup the serial port?
> 
> It does.
> I use U-boot and the serial port is already set up by U-boot.
> 
> 
> But, earlycon setup functions update hardware registers.
> See  early_serial8250_setup(), ingenic_early_console_setup(), etc.
> 
> 
> Without port->uartclk set to a valid value,
> the init code in earlycon setup does not make sense.
> 
> 
> What I want to clarify is,
> what should we do in the earlycon setup function?
> 
> Currently, I see
>  [1] set device->con->write callback
>  [2] initialize UART port registers
> 
> 
> For [2], we need to know baudrate and input clock frequency.
> (and the latter is missing, that's why my patch is here.)
> 
> 
> In order to be independent of a boot loader, we also need
>   [3]  pinctrl (pin-muxing)
> 
> But, it is difficult to handle pinctrl in the earlycon framework.
> 
> 
> If we depend on a boot loader, [2] is meaningless.   [1] is enough.

The 8250 earlycon doesn't try to initialize the hardware (other than
masking interrupts) if the baud rate is uninitialized
(!device->baud in early_serial8250_setup()).

Rather than initializing the h/w to a default of 115200 in
ingenic_early_console_setup() when device->baud == 0, it should just
mask interrupts. That way the bootloader initialization will be
preserved when the options are unspecified on the command line, such as:

earlycon=jz4740_uart,mmio32,0x43fb


Regards,
Peter Hurley

PS - Apologies for not reviewing the Ingenic 8250 driver at submission
time.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-20 Thread Masahiro Yamada 
Hi Peter,
(+ Rob Herring, Stefan Agner)

2015-10-20 23:00 GMT+09:00 Peter Hurley :
> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
>> The input clock frequency varies from device to device, but the
>> earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
>> impossible to set the correct divisor to the register.
>
> So the bootloader hasn't setup the serial port?

It does.
I use U-boot and the serial port is already set up by U-boot.


But, earlycon setup functions update hardware registers.
See  early_serial8250_setup(), ingenic_early_console_setup(), etc.


Without port->uartclk set to a valid value,
the init code in earlycon setup does not make sense.


What I want to clarify is,
what should we do in the earlycon setup function?

Currently, I see
 [1] set device->con->write callback
 [2] initialize UART port registers


For [2], we need to know baudrate and input clock frequency.
(and the latter is missing, that's why my patch is here.)


In order to be independent of a boot loader, we also need
  [3]  pinctrl (pin-muxing)

But, it is difficult to handle pinctrl in the earlycon framework.


If we depend on a boot loader, [2] is meaningless.   [1] is enough.




-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-20 Thread Peter Hurley 
On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
> The input clock frequency varies from device to device, but the
> earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
> impossible to set the correct divisor to the register.

So the bootloader hasn't setup the serial port?


> This commit allows to specify the input clock frequency from the
> kernel-parameter.
> 
> [Example]
> 
> earlycon=uart8250,mmio32,0x43fb,115200,12288000
> 
> The input clock frequency (12288000, in this case) should be specified
> after the baudrate.  If not specified, the default (BASE_BAUD * 16) is
> used.
> 
> Signed-off-by: Masahiro Yamada 
> ---
> 
> Changes in v2: None
> 
>  drivers/tty/serial/earlycon.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 07f7393..8030765 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -95,6 +95,13 @@ static int __init parse_options(struct earlycon_device 
> *device, char *options)
>   length = min(strcspn(options, " ") + 1,
>(size_t)(sizeof(device->options)));
>   strlcpy(device->options, options, length);
> + options = strchr(options, ',');
> + if (options) {
> + options++;
> + port->uartclk = simple_strtoul(options, NULL, 0);
> + }
> + if (!port->uartclk)
> + port->uartclk = BASE_BAUD * 16;
>   }
>  
>   if (port->iotype == UPIO_MEM || port->iotype == UPIO_MEM16 ||
> @@ -122,7 +129,6 @@ static int __init register_earlycon(char *buf, const 
> struct earlycon_id *match)
>   if (buf && !parse_options(_console_dev, buf))
>   buf = NULL;
>  
> - port->uartclk = BASE_BAUD * 16;
>   if (port->mapbase)
>   port->membase = earlycon_map(port->mapbase, 64);
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-20 Thread Peter Hurley 
On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
> The input clock frequency varies from device to device, but the
> earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
> impossible to set the correct divisor to the register.

So the bootloader hasn't setup the serial port?


> This commit allows to specify the input clock frequency from the
> kernel-parameter.
> 
> [Example]
> 
> earlycon=uart8250,mmio32,0x43fb,115200,12288000
> 
> The input clock frequency (12288000, in this case) should be specified
> after the baudrate.  If not specified, the default (BASE_BAUD * 16) is
> used.
> 
> Signed-off-by: Masahiro Yamada 
> ---
> 
> Changes in v2: None
> 
>  drivers/tty/serial/earlycon.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 07f7393..8030765 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -95,6 +95,13 @@ static int __init parse_options(struct earlycon_device 
> *device, char *options)
>   length = min(strcspn(options, " ") + 1,
>(size_t)(sizeof(device->options)));
>   strlcpy(device->options, options, length);
> + options = strchr(options, ',');
> + if (options) {
> + options++;
> + port->uartclk = simple_strtoul(options, NULL, 0);
> + }
> + if (!port->uartclk)
> + port->uartclk = BASE_BAUD * 16;
>   }
>  
>   if (port->iotype == UPIO_MEM || port->iotype == UPIO_MEM16 ||
> @@ -122,7 +129,6 @@ static int __init register_earlycon(char *buf, const 
> struct earlycon_id *match)
>   if (buf && !parse_options(_console_dev, buf))
>   buf = NULL;
>  
> - port->uartclk = BASE_BAUD * 16;
>   if (port->mapbase)
>   port->membase = earlycon_map(port->mapbase, 64);
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-20 Thread Masahiro Yamada 
Hi Peter,
(+ Rob Herring, Stefan Agner)

2015-10-20 23:00 GMT+09:00 Peter Hurley :
> On 10/19/2015 11:36 PM, Masahiro Yamada wrote:
>> The input clock frequency varies from device to device, but the
>> earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
>> impossible to set the correct divisor to the register.
>
> So the bootloader hasn't setup the serial port?

It does.
I use U-boot and the serial port is already set up by U-boot.


But, earlycon setup functions update hardware registers.
See  early_serial8250_setup(), ingenic_early_console_setup(), etc.


Without port->uartclk set to a valid value,
the init code in earlycon setup does not make sense.


What I want to clarify is,
what should we do in the earlycon setup function?

Currently, I see
 [1] set device->con->write callback
 [2] initialize UART port registers


For [2], we need to know baudrate and input clock frequency.
(and the latter is missing, that's why my patch is here.)


In order to be independent of a boot loader, we also need
  [3]  pinctrl (pin-muxing)

But, it is difficult to handle pinctrl in the earlycon framework.


If we depend on a boot loader, [2] is meaningless.   [1] is enough.




-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-19 Thread Masahiro Yamada 
The input clock frequency varies from device to device, but the
earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
impossible to set the correct divisor to the register.

This commit allows to specify the input clock frequency from the
kernel-parameter.

[Example]

earlycon=uart8250,mmio32,0x43fb,115200,12288000

The input clock frequency (12288000, in this case) should be specified
after the baudrate.  If not specified, the default (BASE_BAUD * 16) is
used.

Signed-off-by: Masahiro Yamada 
---

Changes in v2: None

 drivers/tty/serial/earlycon.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 07f7393..8030765 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -95,6 +95,13 @@ static int __init parse_options(struct earlycon_device 
*device, char *options)
length = min(strcspn(options, " ") + 1,
 (size_t)(sizeof(device->options)));
strlcpy(device->options, options, length);
+   options = strchr(options, ',');
+   if (options) {
+   options++;
+   port->uartclk = simple_strtoul(options, NULL, 0);
+   }
+   if (!port->uartclk)
+   port->uartclk = BASE_BAUD * 16;
}
 
if (port->iotype == UPIO_MEM || port->iotype == UPIO_MEM16 ||
@@ -122,7 +129,6 @@ static int __init register_earlycon(char *buf, const struct 
earlycon_id *match)
if (buf && !parse_options(_console_dev, buf))
buf = NULL;
 
-   port->uartclk = BASE_BAUD * 16;
if (port->mapbase)
port->membase = earlycon_map(port->mapbase, 64);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/2] serial: earlycon: allow to specify uartclk in earlycon kernel-parameter

2015-10-19 Thread Masahiro Yamada 
The input clock frequency varies from device to device, but the
earlycon uses the fixed frequency (BASE_BAUD * 16).  It makes
impossible to set the correct divisor to the register.

This commit allows to specify the input clock frequency from the
kernel-parameter.

[Example]

earlycon=uart8250,mmio32,0x43fb,115200,12288000

The input clock frequency (12288000, in this case) should be specified
after the baudrate.  If not specified, the default (BASE_BAUD * 16) is
used.

Signed-off-by: Masahiro Yamada 
---

Changes in v2: None

 drivers/tty/serial/earlycon.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 07f7393..8030765 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -95,6 +95,13 @@ static int __init parse_options(struct earlycon_device 
*device, char *options)
length = min(strcspn(options, " ") + 1,
 (size_t)(sizeof(device->options)));
strlcpy(device->options, options, length);
+   options = strchr(options, ',');
+   if (options) {
+   options++;
+   port->uartclk = simple_strtoul(options, NULL, 0);
+   }
+   if (!port->uartclk)
+   port->uartclk = BASE_BAUD * 16;
}
 
if (port->iotype == UPIO_MEM || port->iotype == UPIO_MEM16 ||
@@ -122,7 +129,6 @@ static int __init register_earlycon(char *buf, const struct 
earlycon_id *match)
if (buf && !parse_options(_console_dev, buf))
buf = NULL;
 
-   port->uartclk = BASE_BAUD * 16;
if (port->mapbase)
port->membase = earlycon_map(port->mapbase, 64);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/