[U-Boot] [PATCH v2] arm: omap: Fix 'get_device_type()' for OMAP34XX

[Derald Woods]

On Thu, Aug 10, 2017 at 7:50 AM, Derald Woods <woods.technical at gmail.com>
wrote:

> On Thu, Aug 10, 2017 at 7:25 AM, Derald Woods <woods.technical at gmail.com>
> wrote:
>
>> On Mon, Aug 7, 2017 at 5:19 PM, Derald Woods <woods.technical at gmail.com>
>> wrote:
>>
>>> On Mon, Aug 7, 2017 at 7:41 AM, Tom Rini <trini at konsulko.com> wrote:
>>>
>>>> On Mon, Aug 07, 2017 at 07:35:30AM -0500, Derald Woods wrote:
>>>> > On Mon, Jul 31, 2017 at 7:41 AM, Derald D. Woods <
>>>> woods.technical at gmail.com>
>>>> > wrote:
>>>> >
>>>> > > Fixes: 00bbe96ebabb ("arm: omap: Unify get_device_type() function")
>>>> > >
>>>> > > The control status register value is embedded in a structure
>>>> somewhere
>>>> > > in SRAM, with the last refactoring effort. This patch allows OMAP3
>>>> EVM
>>>> > > (TMDSEVM3530) to boot again using the known control register base
>>>> and
>>>> > > offset for 'readl', for the OMAP34XX case.
>>>> > >
>>>> > > Signed-off-by: Derald D. Woods <woods.technical at gmail.com>
>>>> > >
>>>> > > ---
>>>> > > Changes in v2:
>>>> > > - Added 'signed-off-by'
>>>> > > - Updated description
>>>> > >
>>>> > > ---
>>>> > >  arch/arm/mach-omap2/sysinfo-common.c | 4 ++++
>>>> > >  1 file changed, 4 insertions(+)
>>>> > >
>>>> > > diff --git a/arch/arm/mach-omap2/sysinfo-common.c
>>>> > > b/arch/arm/mach-omap2/sysinfo-common.c
>>>> > > index 1dc7051ab3..3955e803ad 100644
>>>> > > --- a/arch/arm/mach-omap2/sysinfo-common.c
>>>> > > +++ b/arch/arm/mach-omap2/sysinfo-common.c
>>>> > > @@ -16,6 +16,10 @@
>>>> > >   */
>>>> > >  u32 get_device_type(void)
>>>> > >  {
>>>> > > +#if defined(CONFIG_OMAP34XX)
>>>> > > +       return (readl(OMAP34XX_CTRL_BASE + 0x2f0) &
>>>> DEVICE_TYPE_MASK) >>
>>>> > > +               DEVICE_TYPE_SHIFT;
>>>> > > +#endif
>>>> > >         return (readl((*ctrl)->control_status) & DEVICE_TYPE_MASK)
>>>> >>
>>>> > >                 DEVICE_TYPE_SHIFT;
>>>> > >  }
>>>> >
>>>> > ​Is there any​ comment or concern with this patch? It was the simplest
>>>> > means to get OMAP35XX booting again and still keep the original patch.
>>>> > Basically it avoids the pointer to the OMAP_SRAM_SCRATCH_SYS_CTRL
>>>> located
>>>> > structure as now defined in "arch/arm/mach-omap2/omap3/hw_data.c".
>>>> OMAP3
>>>> > has a larger active SOC base than just OMAP36XX and greater. Was there
>>>> > anything really broken with 'get_device_type()' previously?
>>>>
>>>> Is the pointer value wrong for 35xx?  The problem, such as it was, was
>>>> having the same function duplicated in a number of places, and needing
>>>> to make it more easily / readily available (rather than duplicated
>>>> again) for some additional patches.  I'd really rather not introduce
>>>> an #if here unless we really have no other choice.  Thanks!
>>>>
>>>> --
>>>> Tom
>>>>
>>>
>>> ​I will examine/debug the new 'get_device_type()' source code again
>>> tonight. ‎Without the patch, I have two OMAP3530 boards that do not boot at
>>> all.
>>>
>> ​​
>>
>> Derald​
>> ​
>>
>> In "arch/arm/mach-omap2/omap3/board.c", the following routines call
>> 'get_device_type':
>> - v7_arch_cp15_set_l2aux_ctrl      (called from
>> "arch/arm/cpu/armv7/start.S")
>> - v7_arch_cp15_set_acr             (called from
>> "arch/arm/cpu/armv7/start.S")
>> - omap3_invalidate_l2_cache_secure (called from "board.c" 's_init')
>> - try_unlock_memory                (called from "board.c" 's_init')
>>
>> Each one of these calls to 'get_device_type' seems to have a purpose
>> (i.e. ERRATA or setup). What it highlights is the difference in usage from
>> OMAP36XX, AM33XX, and newer. The SRAM usage, L2 cache and call ordering are
>> likely factors here. ​Figuring out the implications for OMAP34XX will take
>> time. But you need to have boards that boot to address those issues. So
>> 'get_device_type' usage, for OMAP34XX, is not strictly a "Apples to Apples"
>> type comparison. For now I can proceed with an "out-of-tree" patch, since
>> it is small. Let me know if there is something that I am missing here.
>>
>> Derald
>>
>
> ​
> Based on all of the above, I think the "#if" is still the simplest path to
> booting.
>
> Derald
>
>
>
​Are there any further thoughts about OMAP34XX calling 'get_device_type()'
from "start.S"?

Derald ​