[PATCH] arm: omap: Fix 'get_device_type()' for OMAP34XX

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] arm: omap: Fix 'get_device_type()' for OMAP34XX

Derald D. Woods
Fixes 00bbe96ebabb (arm: omap: Unify get_device_type() function) where
the control status register value is embedded in a structure somewhere
in SRAM. This patch allows OMAP3 EVM (TMDSEVM3530) to boot again using
a known global constant and offset for 'readl'.
---
 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;
 }
--
2.13.3

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Derald D. Woods
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 <[hidden email]>

---
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;
 }
--
2.13.3

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] arm: omap: Fix 'get_device_type()' for OMAP34XX

Derald D. Woods
On Mon, Jul 31, 2017 at 7:41 AM, Derald D. Woods <[hidden email]>
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 <[hidden email]>
>
> ---
> 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;
>  }
> --
> 2.13.3
>
>

​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?


Derald
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] arm: omap: Fix 'get_device_type()' for OMAP34XX

Tom Rini-4
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 <[hidden email]>
> 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 <[hidden email]>
> >
> > ---
> > 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

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] arm: omap: Fix 'get_device_type()' for OMAP34XX

Derald D. Woods
On Mon, Aug 7, 2017 at 7:41 AM, Tom Rini <[hidden email]> 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 <
> [hidden email]>
> > 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 <[hidden email]>
> > >
> > > ---
> > > 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​
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] arm: omap: Fix 'get_device_type()' for OMAP34XX

Derald D. Woods
On Mon, Aug 7, 2017 at 5:19 PM, Derald Woods <[hidden email]>
wrote:

> On Mon, Aug 7, 2017 at 7:41 AM, Tom Rini <[hidden email]> 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 <
>> [hidden email]>
>> > 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 <[hidden email]>
>> > >
>> > > ---
>> > > 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
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] arm: omap: Fix 'get_device_type()' for OMAP34XX

Derald D. Woods
On Thu, Aug 10, 2017 at 7:25 AM, Derald Woods <[hidden email]>
wrote:

> On Mon, Aug 7, 2017 at 5:19 PM, Derald Woods <[hidden email]>
> wrote:
>
>> On Mon, Aug 7, 2017 at 7:41 AM, Tom Rini <[hidden email]> 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 <
>>> [hidden email]>
>>> > 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 <[hidden email]>
>>> > >
>>> > > ---
>>> > > 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
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] arm: omap: Fix 'get_device_type()' for OMAP34XX

Derald D. Woods
On Thu, Aug 10, 2017 at 7:50 AM, Derald Woods <[hidden email]>
wrote:

> On Thu, Aug 10, 2017 at 7:25 AM, Derald Woods <[hidden email]>
> wrote:
>
>> On Mon, Aug 7, 2017 at 5:19 PM, Derald Woods <[hidden email]>
>> wrote:
>>
>>> On Mon, Aug 7, 2017 at 7:41 AM, Tom Rini <[hidden email]> 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 <
>>>> [hidden email]>
>>>> > 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 <[hidden email]>
>>>> > >
>>>> > > ---
>>>> > > 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 ​
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] arm: omap: Fix 'get_device_type()' for OMAP34XX

Tom Rini-4
In reply to this post by Derald D. Woods
On Thu, Aug 10, 2017 at 07:25:29AM -0500, Derald Woods wrote:

> On Mon, Aug 7, 2017 at 5:19 PM, Derald Woods <[hidden email]>
> wrote:
>
> > On Mon, Aug 7, 2017 at 7:41 AM, Tom Rini <[hidden email]> 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 <
> >> [hidden email]>
> >> > 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 <[hidden email]>
> >> > >
> >> > > ---
> >> > > 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.
OK, thanks for the analysis.  It is indeed a bit too much overhead atm
to see if we can further reconcile all of the code and should grab your
patch, thanks!

--
Tom

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Tom Rini-4
In reply to this post by Derald D. Woods
On Mon, Jul 31, 2017 at 07:41:40AM -0500, Derald D. Woods 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 <[hidden email]>

Applied to u-boot/master, thanks!

--
Tom

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot

signature.asc (836 bytes) Download Attachment
Loading...