Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

[Florian Fainelli]

Hi,

When switching between different Multi v7 platform which all select
DEBUG_UART_8250, whichever platform managed to set
DEBUG_UART_{PHYS,VIRT} first will end-up forcing its value to the
others.

What I am seeing at the moment is for instance enabling BCM_MOBILE
will set DEBUG_UART_PHYS to 0x3e000000, disabling BCM_MOBILE and now
enabling MVEBU won't force a new DEBUG_UART_PHYS address to be
re-computed.

Should we add some sort of specific annotation to the Kconfig symbol
to force a recalculation of the UART PHYS address?

Thanks!
-- 
Florian

Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

[Arnd Bergmann]

On Wednesday 19 March 2014 16:38:45 Florian Fainelli wrote:
> Hi,
> 
> When switching between different Multi v7 platform which all select
> DEBUG_UART_8250, whichever platform managed to set
> DEBUG_UART_{PHYS,VIRT} first will end-up forcing its value to the
> others.
> 
> What I am seeing at the moment is for instance enabling BCM_MOBILE
> will set DEBUG_UART_PHYS to 0x3e000000, disabling BCM_MOBILE and now
> enabling MVEBU won't force a new DEBUG_UART_PHYS address to be
> re-computed.
> 
> Should we add some sort of specific annotation to the Kconfig symbol
> to force a recalculation of the UART PHYS address?

Russell has plans to change the Konfig logic in the future, removing the
platform specific options. With the current code, you could probably
do something along the lines of

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index f72147f..aa20929 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1013,7 +1013,7 @@ config DEBUG_UART_8250
 		ARCH_LPC32XX || ARCH_MV78XX0 || ARCH_ORION5X || ARCH_RPC
 
 config DEBUG_UART_PHYS
-	hex "Physical base address of debug UART"
+	hex
 	default 0x01c20000 if DEBUG_DAVINCI_DMx_UART0
 	default 0x01c28000 if DEBUG_SUNXI_UART0
 	default 0x01c28400 if DEBUG_SUNXI_UART1
@@ -1074,7 +1074,7 @@ config DEBUG_UART_PHYS
 		DEBUG_UART_8250 || DEBUG_UART_PL01X
 
 config DEBUG_UART_VIRT
-	hex "Virtual base address of debug UART"
+	hex
 	default 0xe0010fe0 if ARCH_RPC
 	default 0xf0000be0 if ARCH_EBSA110
 	default 0xf0009000 if DEBUG_CNS3XXX

with some extra logic to still allow you to enter the values
manually if not set by default, but that probably wouldn't work
any more when the rework is done.

	Arnd

Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

[Russell King - ARM Linux]

On Thu, Mar 20, 2014 at 08:07:52AM +0100, Arnd Bergmann wrote:
> On Wednesday 19 March 2014 16:38:45 Florian Fainelli wrote:
> > Hi,
> > 
> > When switching between different Multi v7 platform which all select
> > DEBUG_UART_8250, whichever platform managed to set
> > DEBUG_UART_{PHYS,VIRT} first will end-up forcing its value to the
> > others.
> > 
> > What I am seeing at the moment is for instance enabling BCM_MOBILE
> > will set DEBUG_UART_PHYS to 0x3e000000, disabling BCM_MOBILE and now
> > enabling MVEBU won't force a new DEBUG_UART_PHYS address to be
> > re-computed.
> > 
> > Should we add some sort of specific annotation to the Kconfig symbol
> > to force a recalculation of the UART PHYS address?
> 
> Russell has plans to change the Konfig logic in the future, removing the
> platform specific options. With the current code, you could probably
> do something along the lines of

The only reason the default values are there is to provide an easy way
for people to locate the values they need for the time being.  The
plan is to kill them off, so you have to enter them manually.  The
issue with that is it becomes quite painful if you have to keep searching
around for the right values when something breaks.

(Personally, I've ended up a number of times searching around for the
right address to generate uImage's for various target platforms which
can only load uImages... but I've now ended up with a script which has
that information stored in it...)

So yes, eventually these will have to be specified manually.  I'm
probably going to move them into file(s) in Documentation/arm/
so that we retain the information in an easily accessible form.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

[Arnd Bergmann]

On Thursday 20 March 2014, Russell King - ARM Linux wrote:
> On Thu, Mar 20, 2014 at 08:07:52AM +0100, Arnd Bergmann wrote:
> > On Wednesday 19 March 2014 16:38:45 Florian Fainelli wrote:
> > > Hi,
> > > 
> > > When switching between different Multi v7 platform which all select
> > > DEBUG_UART_8250, whichever platform managed to set
> > > DEBUG_UART_{PHYS,VIRT} first will end-up forcing its value to the
> > > others.
> > > 
> > > What I am seeing at the moment is for instance enabling BCM_MOBILE
> > > will set DEBUG_UART_PHYS to 0x3e000000, disabling BCM_MOBILE and now
> > > enabling MVEBU won't force a new DEBUG_UART_PHYS address to be
> > > re-computed.
> > > 
> > > Should we add some sort of specific annotation to the Kconfig symbol
> > > to force a recalculation of the UART PHYS address?
> > 
> > Russell has plans to change the Konfig logic in the future, removing the
> > platform specific options. With the current code, you could probably
> > do something along the lines of
> 
> The only reason the default values are there is to provide an easy way
> for people to locate the values they need for the time being.  The
> plan is to kill them off, so you have to enter them manually.  The
> issue with that is it becomes quite painful if you have to keep searching
> around for the right values when something breaks.
> 
> (Personally, I've ended up a number of times searching around for the
> right address to generate uImage's for various target platforms which
> can only load uImages... but I've now ended up with a script which has
> that information stored in it...)
> 
> So yes, eventually these will have to be specified manually.  I'm
> probably going to move them into file(s) in Documentation/arm/
> so that we retain the information in an easily accessible form.

Ok, thanks for the clarification. On a semi-related topic, we have
(once more) discussed adding an early printk implementation during
the Linaro Connect meeting this month. I think this would all become
much less painful if we had a way to handle console output in a
platform independent way before the regular console kicks in, like
a few other architectures do.

debug_ll is great for debugging the extremely early code, since
it works basically from the first instruction on, but a lot of
problems actually happen much later. The early console code
runs from parse_early_param(), which comes after setup_arch(),
so we can't use to debug that part but it means we already
have an unflattened device tree to work with for finding typical
8250 or pl011 ports. At the moment, pl011 doesn't even have a
console_initcall, so it comes up only at arch_initcall time,
which is about the earliest a normal tty driver can get registered,
and a lot can go wrong between parse_early_param() and
arch_initcall().

	Arnd

Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

[Russell King - ARM Linux]

On Thu, Mar 20, 2014 at 12:45:33PM +0100, Arnd Bergmann wrote:
> On Thursday 20 March 2014, Russell King - ARM Linux wrote:
> > So yes, eventually these will have to be specified manually.  I'm
> > probably going to move them into file(s) in Documentation/arm/
> > so that we retain the information in an easily accessible form.
> 
> Ok, thanks for the clarification. On a semi-related topic, we have
> (once more) discussed adding an early printk implementation during
> the Linaro Connect meeting this month. I think this would all become
> much less painful if we had a way to handle console output in a
> platform independent way before the regular console kicks in, like
> a few other architectures do.
> 
> debug_ll is great for debugging the extremely early code, since
> it works basically from the first instruction on, but a lot of
> problems actually happen much later. The early console code
> runs from parse_early_param(), which comes after setup_arch(),
> so we can't use to debug that part but it means we already
> have an unflattened device tree to work with for finding typical
> 8250 or pl011 ports. At the moment, pl011 doesn't even have a
> console_initcall, so it comes up only at arch_initcall time,
> which is about the earliest a normal tty driver can get registered,
> and a lot can go wrong between parse_early_param() and
> arch_initcall().

It depends how much crap you want to shovel into serial drivers.  Fixing
stuff to work at console_initcall time basically means that the driver
has to have some way to find where the console is independent of the
driver model - because the driver model won't be up and running at that
point.

Yes, we could have a console_initcall() in there parsing the device tree
for a console port, but what console= parameter would that correspond
with?  You wouldn't know which ttyAM* port it should tie up with because
you don't have that information at that point in time - and it's important
to know that so that /dev/console in userspace works.

8250 gets around this by having a static list of ports and a serial port
at well known address.  We are far from having such a luxury on ARM - this
is unfortunately one of the prices we have to pay for the mistakes of the
past where there is no common layout for peripherals.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

[Rob Herring]

On Wed, Mar 19, 2014 at 6:38 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Hi,
>
> When switching between different Multi v7 platform which all select
> DEBUG_UART_8250, whichever platform managed to set
> DEBUG_UART_{PHYS,VIRT} first will end-up forcing its value to the
> others.
>
> What I am seeing at the moment is for instance enabling BCM_MOBILE
> will set DEBUG_UART_PHYS to 0x3e000000, disabling BCM_MOBILE and now
> enabling MVEBU won't force a new DEBUG_UART_PHYS address to be
> re-computed.
>
> Should we add some sort of specific annotation to the Kconfig symbol
> to force a recalculation of the UART PHYS address?

I've hit this before. I believe the problem is some platforms set the
addresses based on their ARCH_FOO rather than a user selected
FOO_DEBUG_UART option. So on multi-platform kernels with one of these
platforms enabled, you have multiple defaults that are true and may
get FOO's addresses instead of what you have selected.

Rob

Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

[Florian Fainelli]

2014-03-20 0:07 GMT-07:00 Arnd Bergmann <arnd@arndb.de>:
> On Wednesday 19 March 2014 16:38:45 Florian Fainelli wrote:
>> Hi,
>>
>> When switching between different Multi v7 platform which all select
>> DEBUG_UART_8250, whichever platform managed to set
>> DEBUG_UART_{PHYS,VIRT} first will end-up forcing its value to the
>> others.
>>
>> What I am seeing at the moment is for instance enabling BCM_MOBILE
>> will set DEBUG_UART_PHYS to 0x3e000000, disabling BCM_MOBILE and now
>> enabling MVEBU won't force a new DEBUG_UART_PHYS address to be
>> re-computed.
>>
>> Should we add some sort of specific annotation to the Kconfig symbol
>> to force a recalculation of the UART PHYS address?
>
> Russell has plans to change the Konfig logic in the future, removing the
> platform specific options. With the current code, you could probably
> do something along the lines of
>
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index f72147f..aa20929 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -1013,7 +1013,7 @@ config DEBUG_UART_8250
>                 ARCH_LPC32XX || ARCH_MV78XX0 || ARCH_ORION5X || ARCH_RPC
>
>  config DEBUG_UART_PHYS
> -       hex "Physical base address of debug UART"
> +       hex
>         default 0x01c20000 if DEBUG_DAVINCI_DMx_UART0
>         default 0x01c28000 if DEBUG_SUNXI_UART0
>         default 0x01c28400 if DEBUG_SUNXI_UART1
> @@ -1074,7 +1074,7 @@ config DEBUG_UART_PHYS
>                 DEBUG_UART_8250 || DEBUG_UART_PL01X
>
>  config DEBUG_UART_VIRT
> -       hex "Virtual base address of debug UART"
> +       hex
>         default 0xe0010fe0 if ARCH_RPC
>         default 0xf0000be0 if ARCH_EBSA110
>         default 0xf0009000 if DEBUG_CNS3XXX
>
> with some extra logic to still allow you to enter the values
> manually if not set by default, but that probably wouldn't work
> any more when the rework is done.

Your change works great, thanks! Should we make that the current
behavior until Russell revamps the Kconfig logic?
-- 
Florian

Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

[Russell King - ARM Linux]

On Thu, Mar 20, 2014 at 11:22:22AM -0700, Florian Fainelli wrote:
> 2014-03-20 0:07 GMT-07:00 Arnd Bergmann <arnd@arndb.de>:
> > On Wednesday 19 March 2014 16:38:45 Florian Fainelli wrote:
> >> Hi,
> >>
> >> When switching between different Multi v7 platform which all select
> >> DEBUG_UART_8250, whichever platform managed to set
> >> DEBUG_UART_{PHYS,VIRT} first will end-up forcing its value to the
> >> others.
> >>
> >> What I am seeing at the moment is for instance enabling BCM_MOBILE
> >> will set DEBUG_UART_PHYS to 0x3e000000, disabling BCM_MOBILE and now
> >> enabling MVEBU won't force a new DEBUG_UART_PHYS address to be
> >> re-computed.
> >>
> >> Should we add some sort of specific annotation to the Kconfig symbol
> >> to force a recalculation of the UART PHYS address?
> >
> > Russell has plans to change the Konfig logic in the future, removing the
> > platform specific options. With the current code, you could probably
> > do something along the lines of
> >
> > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> > index f72147f..aa20929 100644
> > --- a/arch/arm/Kconfig.debug
> > +++ b/arch/arm/Kconfig.debug
> > @@ -1013,7 +1013,7 @@ config DEBUG_UART_8250
> >                 ARCH_LPC32XX || ARCH_MV78XX0 || ARCH_ORION5X || ARCH_RPC
> >
> >  config DEBUG_UART_PHYS
> > -       hex "Physical base address of debug UART"
> > +       hex
> >         default 0x01c20000 if DEBUG_DAVINCI_DMx_UART0
> >         default 0x01c28000 if DEBUG_SUNXI_UART0
> >         default 0x01c28400 if DEBUG_SUNXI_UART1
> > @@ -1074,7 +1074,7 @@ config DEBUG_UART_PHYS
> >                 DEBUG_UART_8250 || DEBUG_UART_PL01X
> >
> >  config DEBUG_UART_VIRT
> > -       hex "Virtual base address of debug UART"
> > +       hex
> >         default 0xe0010fe0 if ARCH_RPC
> >         default 0xf0000be0 if ARCH_EBSA110
> >         default 0xf0009000 if DEBUG_CNS3XXX
> >
> > with some extra logic to still allow you to enter the values
> > manually if not set by default, but that probably wouldn't work
> > any more when the rework is done.
> 
> Your change works great, thanks! Should we make that the current
> behavior until Russell revamps the Kconfig logic?

No, because it takes away the ability to change it manually.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Forcing DEBUG_UART_{PHYS, VIRT} changes when switching between v7 platforms?

[Domenico Andreoli]

On Thu, Mar 20, 2014 at 12:46:48PM +0000, Russell King - ARM Linux wrote:
> On Thu, Mar 20, 2014 at 12:45:33PM +0100, Arnd Bergmann wrote:
> > 
> > Ok, thanks for the clarification. On a semi-related topic, we have
> > (once more) discussed adding an early printk implementation during
> > the Linaro Connect meeting this month. I think this would all become
> > much less painful if we had a way to handle console output in a
> > platform independent way before the regular console kicks in, like
> > a few other architectures do.
 
This reminds me a related investigation I did:

http://thread.gmane.org/gmane.linux.ports.arm.kernel/177186

> > debug_ll is great for debugging the extremely early code, since
> > it works basically from the first instruction on, but a lot of
> > problems actually happen much later. The early console code
> > runs from parse_early_param(), which comes after setup_arch(),
> > so we can't use to debug that part but it means we already
> > have an unflattened device tree to work with for finding typical
> > 8250 or pl011 ports. At the moment, pl011 doesn't even have a
> > console_initcall, so it comes up only at arch_initcall time,
> > which is about the earliest a normal tty driver can get registered,
> > and a lot can go wrong between parse_early_param() and
> > arch_initcall().
> 
> It depends how much crap you want to shovel into serial drivers.  Fixing
> stuff to work at console_initcall time basically means that the driver
> has to have some way to find where the console is independent of the
> driver model - because the driver model won't be up and running at that
> point.

Indeed there was some "early console" driver model and some crap to push
this through the decompressor throat.

> Yes, we could have a console_initcall() in there parsing the device tree
> for a console port, but what console= parameter would that correspond
> with?  You wouldn't know which ttyAM* port it should tie up with because
> you don't have that information at that point in time - and it's important
> to know that so that /dev/console in userspace works.

there was decomp-console-dt.patch saying:

| This is the very minimal support of DT that currently the decompressor
| console provides. That is reading the command line from the DT.
|
| This means that pure DT machines currently need some console data to
| be shipped to the decompressor (tags have their DT table) although they
| already provide all the required info in the DT, where the decompressor
| should ideally take them.
|
| So the current blocker here is what to extract from DT. Or, what to _put_
| into the DT for the decompressor.

I think that the match of console= + DT serial drivers + "early console"
driver model (which establishes the ttyXXXYY <-> DT compatible property,
in the serial driver itself) is enough to get the thing working.

Maybe it would not look so horrible, is it worth a try?

Regards,
Domenico