From: Victor Kamensky <[email protected]>
Assembler functions defined in sleep44xx.S need to byteswap values
after read / before write from h/w register if code compiled in big
endian mode. Simple change to do 'rev x, x' before str instruction
and after ldr instruction that deals with h/w registers.
Signed-off-by: Victor Kamensky <[email protected]>
Signed-off-by: Taras Kondratiuk <[email protected]>
---
This is a part of RFC series [1].
Based on v3.13-rc8.
[1] http://www.spinics.net/lists/linux-omap/msg99927.html
arch/arm/mach-omap2/sleep44xx.S | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm/mach-omap2/sleep44xx.S b/arch/arm/mach-omap2/sleep44xx.S
index 9086ce0..8017016 100644
--- a/arch/arm/mach-omap2/sleep44xx.S
+++ b/arch/arm/mach-omap2/sleep44xx.S
@@ -12,6 +12,7 @@
#include <linux/linkage.h>
#include <asm/smp_scu.h>
#include <asm/memory.h>
+#include <asm/assembler.h>
#include <asm/hardware/cache-l2x0.h>
#include "omap-secure.h"
@@ -74,6 +75,7 @@ ENTRY(omap4_finish_suspend)
*/
bl omap4_get_sar_ram_base
ldr r9, [r0, #OMAP_TYPE_OFFSET]
+ARM_BE8(rev r9, r9)
cmp r9, #0x1 @ Check for HS device
bne skip_secure_l1_clean
mov r0, #SCU_PM_NORMAL
@@ -113,12 +115,14 @@ skip_secure_l1_clean:
bl omap4_get_sar_ram_base
mov r8, r0
ldr r9, [r8, #OMAP_TYPE_OFFSET]
+ARM_BE8(rev r9, r9)
cmp r9, #0x1 @ Check for HS device
bne scu_gp_set
mrc p15, 0, r0, c0, c0, 5 @ Read MPIDR
ands r0, r0, #0x0f
ldreq r0, [r8, #SCU_OFFSET0]
ldrne r0, [r8, #SCU_OFFSET1]
+ARM_BE8(rev r0, r0)
mov r1, #0x00
stmfd r13!, {r4-r12, r14}
ldr r12, =OMAP4_MON_SCU_PWR_INDEX
@@ -130,6 +134,7 @@ scu_gp_set:
ands r0, r0, #0x0f
ldreq r1, [r8, #SCU_OFFSET0]
ldrne r1, [r8, #SCU_OFFSET1]
+ARM_BE8(rev r1, r1)
bl omap4_get_scu_base
bl scu_power_mode
skip_scu_gp_set:
@@ -157,6 +162,7 @@ skip_scu_gp_set:
ands r5, r5, #0x0f
ldreq r0, [r8, #L2X0_SAVE_OFFSET0] @ Retrieve L2 state from SAR
ldrne r0, [r8, #L2X0_SAVE_OFFSET1] @ memory.
+ARM_BE8(rev r0, r0)
cmp r0, #3
bne do_WFI
#ifdef CONFIG_PL310_ERRATA_727915
@@ -167,9 +173,11 @@ skip_scu_gp_set:
bl omap4_get_l2cache_base
mov r2, r0
ldr r0, =0xffff
+ARM_BE8(rev r0, r0)
str r0, [r2, #L2X0_CLEAN_INV_WAY]
wait:
ldr r0, [r2, #L2X0_CLEAN_INV_WAY]
+ARM_BE8(rev r0, r0)
ldr r1, =0xffff
ands r0, r0, r1
bne wait
@@ -182,9 +190,11 @@ l2x_sync:
bl omap4_get_l2cache_base
mov r2, r0
mov r0, #0x0
+ARM_BE8(rev r0, r0)
str r0, [r2, #L2X0_CACHE_SYNC]
sync:
ldr r0, [r2, #L2X0_CACHE_SYNC]
+ARM_BE8(rev r0, r0)
ands r0, r0, #0x1
bne sync
#endif
@@ -216,6 +226,7 @@ do_WFI:
bl omap4_get_sar_ram_base
mov r8, r0
ldr r9, [r8, #OMAP_TYPE_OFFSET]
+ARM_BE8(rev r9, r9)
cmp r9, #0x1 @ Check for HS device
bne scu_gp_clear
mov r0, #SCU_PM_NORMAL
@@ -258,6 +269,7 @@ ENTRY(omap4_cpu_resume)
*/
ldr r8, =OMAP44XX_SAR_RAM_BASE
ldr r9, [r8, #OMAP_TYPE_OFFSET]
+ARM_BE8(rev r9, r9)
cmp r9, #0x1 @ Skip if GP device
bne skip_ns_smp_enable
mrc p15, 0, r0, c0, c0, 5
@@ -292,16 +304,19 @@ skip_ns_smp_enable:
*/
ldr r2, =OMAP44XX_L2CACHE_BASE
ldr r0, [r2, #L2X0_CTRL]
+ARM_BE8(rev r0, r0)
and r0, #0x0f
cmp r0, #1
beq skip_l2en @ Skip if already enabled
ldr r3, =OMAP44XX_SAR_RAM_BASE
ldr r1, [r3, #OMAP_TYPE_OFFSET]
+ARM_BE8(rev r1, r1)
cmp r1, #0x1 @ Check for HS device
bne set_gp_por
ldr r0, =OMAP4_PPA_L2_POR_INDEX
ldr r1, =OMAP44XX_SAR_RAM_BASE
ldr r4, [r1, #L2X0_PREFETCH_CTRL_OFFSET]
+ARM_BE8(rev r4, r4)
adr r3, ppa_por_params
str r4, [r3, #0x04]
mov r1, #0x0 @ Process ID
@@ -313,11 +328,13 @@ skip_ns_smp_enable:
set_gp_por:
ldr r1, =OMAP44XX_SAR_RAM_BASE
ldr r0, [r1, #L2X0_PREFETCH_CTRL_OFFSET]
+ARM_BE8(rev r0, r0)
ldr r12, =OMAP4_MON_L2X0_PREFETCH_INDEX @ Setup L2 PREFETCH
DO_SMC
set_aux_ctrl:
ldr r1, =OMAP44XX_SAR_RAM_BASE
ldr r0, [r1, #L2X0_AUXCTRL_OFFSET]
+ARM_BE8(rev r0, r0)
ldr r12, =OMAP4_MON_L2X0_AUXCTRL_INDEX @ Setup L2 AUXCTRL
DO_SMC
mov r0, #0x1
--
1.7.9.5
On 01/13/2014 09:03 AM, Taras Kondratiuk wrote:
> From: Victor Kamensky <[email protected]>
>
> Assembler functions defined in sleep44xx.S need to byteswap values
> after read / before write from h/w register if code compiled in big
> endian mode. Simple change to do 'rev x, x' before str instruction
> and after ldr instruction that deals with h/w registers.
>
> Signed-off-by: Victor Kamensky <[email protected]>
> Signed-off-by: Taras Kondratiuk <[email protected]>
> ---
> This is a part of RFC series [1].
> Based on v3.13-rc8.
>
> [1] http://www.spinics.net/lists/linux-omap/msg99927.html
>
> arch/arm/mach-omap2/sleep44xx.S | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
OMAP4 is LE, and if there is a gcc flag for the same, is'nt it cleaner
to deal with it in Makefile rather than trying to make an assembly
meant only for LE by force building it for BE?
> diff --git a/arch/arm/mach-omap2/sleep44xx.S b/arch/arm/mach-omap2/sleep44xx.S
> index 9086ce0..8017016 100644
> --- a/arch/arm/mach-omap2/sleep44xx.S
> +++ b/arch/arm/mach-omap2/sleep44xx.S
> @@ -12,6 +12,7 @@
> #include <linux/linkage.h>
> #include <asm/smp_scu.h>
> #include <asm/memory.h>
> +#include <asm/assembler.h>
> #include <asm/hardware/cache-l2x0.h>
>
> #include "omap-secure.h"
> @@ -74,6 +75,7 @@ ENTRY(omap4_finish_suspend)
> */
> bl omap4_get_sar_ram_base
> ldr r9, [r0, #OMAP_TYPE_OFFSET]
> +ARM_BE8(rev r9, r9)
> cmp r9, #0x1 @ Check for HS device
> bne skip_secure_l1_clean
> mov r0, #SCU_PM_NORMAL
> @@ -113,12 +115,14 @@ skip_secure_l1_clean:
> bl omap4_get_sar_ram_base
> mov r8, r0
> ldr r9, [r8, #OMAP_TYPE_OFFSET]
> +ARM_BE8(rev r9, r9)
> cmp r9, #0x1 @ Check for HS device
> bne scu_gp_set
> mrc p15, 0, r0, c0, c0, 5 @ Read MPIDR
> ands r0, r0, #0x0f
> ldreq r0, [r8, #SCU_OFFSET0]
> ldrne r0, [r8, #SCU_OFFSET1]
> +ARM_BE8(rev r0, r0)
> mov r1, #0x00
> stmfd r13!, {r4-r12, r14}
> ldr r12, =OMAP4_MON_SCU_PWR_INDEX
> @@ -130,6 +134,7 @@ scu_gp_set:
> ands r0, r0, #0x0f
> ldreq r1, [r8, #SCU_OFFSET0]
> ldrne r1, [r8, #SCU_OFFSET1]
> +ARM_BE8(rev r1, r1)
> bl omap4_get_scu_base
> bl scu_power_mode
> skip_scu_gp_set:
> @@ -157,6 +162,7 @@ skip_scu_gp_set:
> ands r5, r5, #0x0f
> ldreq r0, [r8, #L2X0_SAVE_OFFSET0] @ Retrieve L2 state from SAR
> ldrne r0, [r8, #L2X0_SAVE_OFFSET1] @ memory.
> +ARM_BE8(rev r0, r0)
> cmp r0, #3
> bne do_WFI
> #ifdef CONFIG_PL310_ERRATA_727915
> @@ -167,9 +173,11 @@ skip_scu_gp_set:
> bl omap4_get_l2cache_base
> mov r2, r0
> ldr r0, =0xffff
> +ARM_BE8(rev r0, r0)
> str r0, [r2, #L2X0_CLEAN_INV_WAY]
> wait:
> ldr r0, [r2, #L2X0_CLEAN_INV_WAY]
> +ARM_BE8(rev r0, r0)
> ldr r1, =0xffff
> ands r0, r0, r1
> bne wait
> @@ -182,9 +190,11 @@ l2x_sync:
> bl omap4_get_l2cache_base
> mov r2, r0
> mov r0, #0x0
> +ARM_BE8(rev r0, r0)
> str r0, [r2, #L2X0_CACHE_SYNC]
> sync:
> ldr r0, [r2, #L2X0_CACHE_SYNC]
> +ARM_BE8(rev r0, r0)
> ands r0, r0, #0x1
> bne sync
> #endif
> @@ -216,6 +226,7 @@ do_WFI:
> bl omap4_get_sar_ram_base
> mov r8, r0
> ldr r9, [r8, #OMAP_TYPE_OFFSET]
> +ARM_BE8(rev r9, r9)
> cmp r9, #0x1 @ Check for HS device
> bne scu_gp_clear
> mov r0, #SCU_PM_NORMAL
> @@ -258,6 +269,7 @@ ENTRY(omap4_cpu_resume)
> */
> ldr r8, =OMAP44XX_SAR_RAM_BASE
> ldr r9, [r8, #OMAP_TYPE_OFFSET]
> +ARM_BE8(rev r9, r9)
> cmp r9, #0x1 @ Skip if GP device
> bne skip_ns_smp_enable
> mrc p15, 0, r0, c0, c0, 5
> @@ -292,16 +304,19 @@ skip_ns_smp_enable:
> */
> ldr r2, =OMAP44XX_L2CACHE_BASE
> ldr r0, [r2, #L2X0_CTRL]
> +ARM_BE8(rev r0, r0)
> and r0, #0x0f
> cmp r0, #1
> beq skip_l2en @ Skip if already enabled
> ldr r3, =OMAP44XX_SAR_RAM_BASE
> ldr r1, [r3, #OMAP_TYPE_OFFSET]
> +ARM_BE8(rev r1, r1)
> cmp r1, #0x1 @ Check for HS device
> bne set_gp_por
> ldr r0, =OMAP4_PPA_L2_POR_INDEX
> ldr r1, =OMAP44XX_SAR_RAM_BASE
> ldr r4, [r1, #L2X0_PREFETCH_CTRL_OFFSET]
> +ARM_BE8(rev r4, r4)
> adr r3, ppa_por_params
> str r4, [r3, #0x04]
> mov r1, #0x0 @ Process ID
> @@ -313,11 +328,13 @@ skip_ns_smp_enable:
> set_gp_por:
> ldr r1, =OMAP44XX_SAR_RAM_BASE
> ldr r0, [r1, #L2X0_PREFETCH_CTRL_OFFSET]
> +ARM_BE8(rev r0, r0)
> ldr r12, =OMAP4_MON_L2X0_PREFETCH_INDEX @ Setup L2 PREFETCH
> DO_SMC
> set_aux_ctrl:
> ldr r1, =OMAP44XX_SAR_RAM_BASE
> ldr r0, [r1, #L2X0_AUXCTRL_OFFSET]
> +ARM_BE8(rev r0, r0)
> ldr r12, =OMAP4_MON_L2X0_AUXCTRL_INDEX @ Setup L2 AUXCTRL
> DO_SMC
> mov r0, #0x1
>
--
Regards,
Nishanth Menon
On 13 January 2014 17:23, Nishanth Menon <[email protected]> wrote:
> On 01/13/2014 09:03 AM, Taras Kondratiuk wrote:
>> From: Victor Kamensky <[email protected]>
>>
>> Assembler functions defined in sleep44xx.S need to byteswap values
>> after read / before write from h/w register if code compiled in big
>> endian mode. Simple change to do 'rev x, x' before str instruction
>> and after ldr instruction that deals with h/w registers.
>>
>> Signed-off-by: Victor Kamensky <[email protected]>
>> Signed-off-by: Taras Kondratiuk <[email protected]>
>> ---
>> This is a part of RFC series [1].
>> Based on v3.13-rc8.
>>
>> [1] http://www.spinics.net/lists/linux-omap/msg99927.html
>>
>> arch/arm/mach-omap2/sleep44xx.S | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>
> OMAP4 is LE, and if there is a gcc flag for the same, is'nt it cleaner
> to deal with it in Makefile rather than trying to make an assembly
> meant only for LE by force building it for BE?
Hi Nishanth
I'm not sure I got your point.
Do you propose to build this file as LE while the rest of kernel is BE?
--
Regards,
Taras Kondratiuk
On 01/14/2014 05:14 AM, Taras Kondratiuk wrote:
> On 13 January 2014 17:23, Nishanth Menon <[email protected]> wrote:
>> On 01/13/2014 09:03 AM, Taras Kondratiuk wrote:
>>> From: Victor Kamensky <[email protected]>
>>>
>>> Assembler functions defined in sleep44xx.S need to byteswap values
>>> after read / before write from h/w register if code compiled in big
>>> endian mode. Simple change to do 'rev x, x' before str instruction
>>> and after ldr instruction that deals with h/w registers.
>>>
>>> Signed-off-by: Victor Kamensky <[email protected]>
>>> Signed-off-by: Taras Kondratiuk <[email protected]>
>>> ---
>>> This is a part of RFC series [1].
>>> Based on v3.13-rc8.
>>>
>>> [1] http://www.spinics.net/lists/linux-omap/msg99927.html
>>>
>>> arch/arm/mach-omap2/sleep44xx.S | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>
>> OMAP4 is LE, and if there is a gcc flag for the same, is'nt it cleaner
>> to deal with it in Makefile rather than trying to make an assembly
>> meant only for LE by force building it for BE?
>
> Hi Nishanth
> I'm not sure I got your point.
> Do you propose to build this file as LE while the rest of kernel is BE?
>
I dont see why I should deal with the BE macro for every code change
we have in omap4,am335x assembly. The hardware is LE and wont change
just coz you are building it for BE. So I dont get the rationale for
changing the assembly here - yes, if the assembly can be maintained as
LE only mode and the build handling be adequately handled in Makefile
(similar to SMC handling), that would be the best.
is the idea of BE build meant to deal with having a single BE kernel
build work for all platforms (including LE ones)?
--
Regards,
Nishanth Menon
Hi Nishanth,
On 14 January 2014 07:51, Nishanth Menon <[email protected]> wrote:
> On 01/14/2014 05:14 AM, Taras Kondratiuk wrote:
>> On 13 January 2014 17:23, Nishanth Menon <[email protected]> wrote:
>>> On 01/13/2014 09:03 AM, Taras Kondratiuk wrote:
>>>> From: Victor Kamensky <[email protected]>
>>>>
>>>> Assembler functions defined in sleep44xx.S need to byteswap values
>>>> after read / before write from h/w register if code compiled in big
>>>> endian mode. Simple change to do 'rev x, x' before str instruction
>>>> and after ldr instruction that deals with h/w registers.
>>>>
>>>> Signed-off-by: Victor Kamensky <[email protected]>
>>>> Signed-off-by: Taras Kondratiuk <[email protected]>
>>>> ---
>>>> This is a part of RFC series [1].
>>>> Based on v3.13-rc8.
>>>>
>>>> [1] http://www.spinics.net/lists/linux-omap/msg99927.html
>>>>
>>>> arch/arm/mach-omap2/sleep44xx.S | 17 +++++++++++++++++
>>>> 1 file changed, 17 insertions(+)
>>>>
>>>
>>> OMAP4 is LE, and if there is a gcc flag for the same, is'nt it cleaner
>>> to deal with it in Makefile rather than trying to make an assembly
>>> meant only for LE by force building it for BE?
>>
>> Hi Nishanth
>> I'm not sure I got your point.
>> Do you propose to build this file as LE while the rest of kernel is BE?
>>
> I dont see why I should deal with the BE macro for every code change
> we have in omap4,am335x assembly. The hardware is LE and wont change
> just coz you are building it for BE. So I dont get the rationale for
> changing the assembly here - yes, if the assembly can be maintained as
> LE only mode and the build handling be adequately handled in Makefile
> (similar to SMC handling), that would be the best.
ARM core is capable of running in LE or BE modes. Yes, OMAP
memory mapped periphery gives data in LE form. When core runs
in BE mode after reading h/w register it will byteswap it, also before
writing h/w register it byteswap it. In such way BE kernel can work
with LE periphery.
When it comes to C code that works with LE periphery, if correct
access functions are used like readl_relaxed and writel_relaxed
(vs __raw_readl and __raw_writel), the functions will take care of
the swaps. In case of asm files there is no other way than to insert
those byteswaps manually and conditionally - they will be enabled only
if kernel compiled in BE mode. The reason why it could be done only
manually is that load/store opcodes behavior changes when core
runs in BE mode (E bit set) in these case ldr/str treat memory as
big endian. So when LE periphery register is read/stored additional
byteswaps that compensate for it should be inserted.
When BE kernel is built Makefile does take of compiling code in BE
mode. I.e all proper flags like -mbig-endian and -Wl,--be8 will be set.
> is the idea of BE build meant to deal with having a single BE kernel
> build work for all platforms (including LE ones)?
Sort of. The idea here to run BE image on OMAP4 chip, with
kernel that would deals with LE periphery correctly, but ARM
core run in BE with special kernel that compiled for BE case (i.e
CONFIG_CPU_BIG_ENDIAN is set).
Thanks,
Victor
> --
> Regards,
> Nishanth Menon
On Tue, Jan 14, 2014 at 11:35 AM, Victor Kamensky
<[email protected]> wrote:
>
> When BE kernel is built Makefile does take of compiling code in BE
> mode. I.e all proper flags like -mbig-endian and -Wl,--be8 will be set.
Agreed, and I assume you cannot instead switch to LE mode when
entering assembly assuming LE?
The reason I ask this is - most of our development is NOT in BE mode.
we will continue to manipulate and add assembly - AM335x, DRA7/OMAP5
etc.. and obviously not every code change will indulge in ensuring
right markers will be in place.
by ensuring readl_relaxed handles the variations, you have ensured
that I dont need to care about drivers other than to ensure they use
_relaxed variants. in the case of assembly, this does not seem long
term manageable.
>
>> is the idea of BE build meant to deal with having a single BE kernel
>> build work for all platforms (including LE ones)?
>
> Sort of. The idea here to run BE image on OMAP4 chip, with
> kernel that would deals with LE periphery correctly, but ARM
> core run in BE with special kernel that compiled for BE case (i.e
> CONFIG_CPU_BIG_ENDIAN is set).
I still dont get the usecase - other than "hey, we do this coz we can
do it!".. I mean, yep, it sounds great and all.. but 4 years down the
line, is this still going to work? is this going to be interesting
careabout? or we are just maintaining additional code for a passing
fancy or proof-of-concept?
Regards,
Nishanth Menon
On 14 January 2014 09:56, Nishanth Menon <[email protected]> wrote:
> On Tue, Jan 14, 2014 at 11:35 AM, Victor Kamensky
> <[email protected]> wrote:
>>
>> When BE kernel is built Makefile does take of compiling code in BE
>> mode. I.e all proper flags like -mbig-endian and -Wl,--be8 will be set.
>
> Agreed, and I assume you cannot instead switch to LE mode when
> entering assembly assuming LE?
Yes. Note that this asm interacts with other data in kernel that would
be in BE form. And after all linker will not allow to put together files
that were compiled in different endianity.
> The reason I ask this is - most of our development is NOT in BE mode.
> we will continue to manipulate and add assembly - AM335x, DRA7/OMAP5
> etc.. and obviously not every code change will indulge in ensuring
> right markers will be in place.
>
> by ensuring readl_relaxed handles the variations, you have ensured
> that I dont need to care about drivers other than to ensure they use
> _relaxed variants. in the case of assembly, this does not seem long
> term manageable.
Yes, I agree if it is outside of main use case it will get broken if not
attended to. Definitely universe entropy increases :) - if left without
attention things will decay. Note we admit that even with ARM CPU
core BE changes similar decay can happen eventually ...that is
what LNG BE group trying to prevent. I think
the deal here is that next user of it can/need to fix things if they
decayed.
>>
>>> is the idea of BE build meant to deal with having a single BE kernel
>>> build work for all platforms (including LE ones)?
>>
>> Sort of. The idea here to run BE image on OMAP4 chip, with
>> kernel that would deals with LE periphery correctly, but ARM
>> core run in BE with special kernel that compiled for BE case (i.e
>> CONFIG_CPU_BIG_ENDIAN is set).
>
> I still dont get the usecase - other than "hey, we do this coz we can
> do it!".. I mean, yep, it sounds great and all.. but 4 years down the
> line, is this still going to work? is this going to be interesting
> careabout? or we are just maintaining additional code for a passing
> fancy or proof-of-concept?
Valid concern. From LNG BE group point of view it is not "we can do
it". It is more like we've done it. We have Pandaboard ES running BE
kernel for a while. It is in LNG BE tree. We used it as development
and testing vehicle for BE work for a while. We are very grateful to
the platform for that - it is affordable and easily available! Given,
beyond ongoing BE testing on Pandaboard in LNG there may not be valid
use case for further things on OMAP4 BE. We had discussion
with Santosh Shilimkar from TI during last Linaro connect what to
do with LNG BE Pandaboard series. Santosh suggested and we
agreed that we would try to contribute them back to community. And
that is what Taras is doing. IMHO even though there may not be real
product use case for BE OMAP4, it could serve in kernel source as good
example that shows how to work with LE periphery from BE kernel. After
all, these changes are noop for LE case and they don't introduce
a lot of clutter: all BE asm rev and rev16 instructions wrapped around
with ARM_BE8 macro.
Thanks,
Victor
> Regards,
> Nishanth Menon
On Tue, Jan 14, 2014 at 2:20 PM, Victor Kamensky
<[email protected]> wrote:
> On 14 January 2014 09:56, Nishanth Menon <[email protected]> wrote:
>> On Tue, Jan 14, 2014 at 11:35 AM, Victor Kamensky
>> <[email protected]> wrote:
>>>
>>> When BE kernel is built Makefile does take of compiling code in BE
>>> mode. I.e all proper flags like -mbig-endian and -Wl,--be8 will be set.
>>
>> Agreed, and I assume you cannot instead switch to LE mode when
>> entering assembly assuming LE?
>
> Yes. Note that this asm interacts with other data in kernel that would
> be in BE form. And after all linker will not allow to put together files
> that were compiled in different endianity.
>
>> The reason I ask this is - most of our development is NOT in BE mode.
>> we will continue to manipulate and add assembly - AM335x, DRA7/OMAP5
>> etc.. and obviously not every code change will indulge in ensuring
>> right markers will be in place.
>>
>> by ensuring readl_relaxed handles the variations, you have ensured
>> that I dont need to care about drivers other than to ensure they use
>> _relaxed variants. in the case of assembly, this does not seem long
>> term manageable.
>
> Yes, I agree if it is outside of main use case it will get broken if not
> attended to. Definitely universe entropy increases :) - if left without
> attention things will decay. Note we admit that even with ARM CPU
> core BE changes similar decay can happen eventually ...that is
> what LNG BE group trying to prevent. I think
> the deal here is that next user of it can/need to fix things if they
> decayed.
>
Personally, I have no idea what "LNG BE" stands for.. I see the
approach we are attempting here is to do any interaction external to
ARM boundary to go through the ARM_BE8 macro.
If we want to make this something we can live with, then the platforms
will have to ensure memory operations external to ARM must be operated
upon by these macros. Instead, an approach that may be used is to
introduce accessors macros that will provide right instruction based
on BE/LE build and BE/LE SoC peripheral behavior.
>>>
>>>> is the idea of BE build meant to deal with having a single BE kernel
>>>> build work for all platforms (including LE ones)?
>>>
>>> Sort of. The idea here to run BE image on OMAP4 chip, with
>>> kernel that would deals with LE periphery correctly, but ARM
>>> core run in BE with special kernel that compiled for BE case (i.e
>>> CONFIG_CPU_BIG_ENDIAN is set).
>>
>> I still dont get the usecase - other than "hey, we do this coz we can
>> do it!".. I mean, yep, it sounds great and all.. but 4 years down the
>> line, is this still going to work? is this going to be interesting
>> careabout? or we are just maintaining additional code for a passing
>> fancy or proof-of-concept?
>
> Valid concern. From LNG BE group point of view it is not "we can do
> it". It is more like we've done it. We have Pandaboard ES running BE
> kernel for a while. It is in LNG BE tree. We used it as development
> and testing vehicle for BE work for a while. We are very grateful to
> the platform for that - it is affordable and easily available! Given,
> beyond ongoing BE testing on Pandaboard in LNG there may not be valid
> use case for further things on OMAP4 BE. We had discussion
> with Santosh Shilimkar from TI during last Linaro connect what to
> do with LNG BE Pandaboard series. Santosh suggested and we
> agreed that we would try to contribute them back to community. And
> that is what Taras is doing. IMHO even though there may not be real
ok.. some sort of Linaro thing about which I have no background about
- but dont really care in this context.
> product use case for BE OMAP4, it could serve in kernel source as good
> example that shows how to work with LE periphery from BE kernel. After
> all, these changes are noop for LE case and they don't introduce
> a lot of clutter: all BE asm rev and rev16 instructions wrapped around
> with ARM_BE8 macro.
>
So, why are we not doing this for *all* OMAP platforms? we try very
hard not to regress on features introduced and wish to ensure all
platforms are equivalent in terms of usage. it makes code sharing a
little harder if we have to ensure that ARM_BE8 is supported in a mix
of reusable codebases.
Personally, here is what I might expect to see:
a) if we are going to force usage of BE and LE build options - then we
*must* be able to reuse code across platforms without having to worry
about breaking platform x etc..
b) this seems like - "take this code in" if something breaks, the guy
who needs it later will come and fix - Shrug, I personally dont buy
that. if something benefits many folks, lets all use it in upstream,
if there is just an exception usage for a short duration which may
impact code scalability and hard to maintain, keep it forked.
c) considering this is non standard usage, some sort of regular test
procedure and owner to ensure this feature is kept sane
With out the above, we are just adding dead code IMHO.
Regards,
Nishanth Menon
On Tuesday 14 January 2014 03:56 PM, Nishanth Menon wrote:
> On Tue, Jan 14, 2014 at 2:20 PM, Victor Kamensky
> <[email protected]> wrote:
>> On 14 January 2014 09:56, Nishanth Menon <[email protected]> wrote:
>>> On Tue, Jan 14, 2014 at 11:35 AM, Victor Kamensky
>>> <[email protected]> wrote:
>>>>
>>>> When BE kernel is built Makefile does take of compiling code in BE
>>>> mode. I.e all proper flags like -mbig-endian and -Wl,--be8 will be set.
>>>
>>> Agreed, and I assume you cannot instead switch to LE mode when
>>> entering assembly assuming LE?
>>
>> Yes. Note that this asm interacts with other data in kernel that would
>> be in BE form. And after all linker will not allow to put together files
>> that were compiled in different endianity.
>>
>>> The reason I ask this is - most of our development is NOT in BE mode.
>>> we will continue to manipulate and add assembly - AM335x, DRA7/OMAP5
>>> etc.. and obviously not every code change will indulge in ensuring
>>> right markers will be in place.
>>>
>>> by ensuring readl_relaxed handles the variations, you have ensured
>>> that I dont need to care about drivers other than to ensure they use
>>> _relaxed variants. in the case of assembly, this does not seem long
>>> term manageable.
>>
>> Yes, I agree if it is outside of main use case it will get broken if not
>> attended to. Definitely universe entropy increases :) - if left without
>> attention things will decay. Note we admit that even with ARM CPU
>> core BE changes similar decay can happen eventually ...that is
>> what LNG BE group trying to prevent. I think
>> the deal here is that next user of it can/need to fix things if they
>> decayed.
>>
>
> Personally, I have no idea what "LNG BE" stands for.. I see the
> approach we are attempting here is to do any interaction external to
> ARM boundary to go through the ARM_BE8 macro.
>
> If we want to make this something we can live with, then the platforms
> will have to ensure memory operations external to ARM must be operated
> upon by these macros. Instead, an approach that may be used is to
> introduce accessors macros that will provide right instruction based
> on BE/LE build and BE/LE SoC peripheral behavior.
>
>>>>
>>>>> is the idea of BE build meant to deal with having a single BE kernel
>>>>> build work for all platforms (including LE ones)?
>>>>
>>>> Sort of. The idea here to run BE image on OMAP4 chip, with
>>>> kernel that would deals with LE periphery correctly, but ARM
>>>> core run in BE with special kernel that compiled for BE case (i.e
>>>> CONFIG_CPU_BIG_ENDIAN is set).
>>>
>>> I still dont get the usecase - other than "hey, we do this coz we can
>>> do it!".. I mean, yep, it sounds great and all.. but 4 years down the
>>> line, is this still going to work? is this going to be interesting
>>> careabout? or we are just maintaining additional code for a passing
>>> fancy or proof-of-concept?
>>
>> Valid concern. From LNG BE group point of view it is not "we can do
>> it". It is more like we've done it. We have Pandaboard ES running BE
>> kernel for a while. It is in LNG BE tree. We used it as development
>> and testing vehicle for BE work for a while. We are very grateful to
>> the platform for that - it is affordable and easily available! Given,
>> beyond ongoing BE testing on Pandaboard in LNG there may not be valid
>> use case for further things on OMAP4 BE. We had discussion
>> with Santosh Shilimkar from TI during last Linaro connect what to
>> do with LNG BE Pandaboard series. Santosh suggested and we
>> agreed that we would try to contribute them back to community. And
>> that is what Taras is doing. IMHO even though there may not be real
>
> ok.. some sort of Linaro thing about which I have no background about
> - but dont really care in this context.
>
Nothing related Linaro. Its just that platforms are supporting ARM BE
mode and Linaro folks had working patches for Panda. So I suggested
to get them on the lists.
Regards,
Santosh
On Tue, Jan 14, 2014 at 3:03 PM, Santosh Shilimkar
<[email protected]> wrote:
>
>> ok.. some sort of Linaro thing about which I have no background about
>> - but dont really care in this context.
>>
> Nothing related Linaro. Its just that platforms are supporting ARM BE
> mode and Linaro folks had working patches for Panda. So I suggested
> to get them on the lists.
I tend to think -> is this with OFF mode and CPUidle completely
working? All context save and restore works with this? on HS and GP
devices with BE mode builds? works on SDP4430,60 variations,
considered reuse with AM43xx which could use parts of that logic?
I mean to indicate that terms like "works on panda" tends always to be relative.
It is nice to see it as a proof of concept, but I'd hate to see some
dead code lying around in kernel and folks blindly following suit and
introducing macros for new assembly for a feature that in practice
just one group of folks care about and creates additional burden for
rest of folks trying to keep that functionality going as we jump from
one "device tree" style churn to another "framework"? Not to mean that
good features should be kept away.. but personally, I could not find
convincing arguments in this case..
Regards,
Nishanth Menon
On 14 January 2014 23:13, Nishanth Menon <[email protected]> wrote:
> On Tue, Jan 14, 2014 at 3:03 PM, Santosh Shilimkar
> <[email protected]> wrote:
>>
>>> ok.. some sort of Linaro thing about which I have no background about
>>> - but dont really care in this context.
>>>
>> Nothing related Linaro. Its just that platforms are supporting ARM BE
>> mode and Linaro folks had working patches for Panda. So I suggested
>> to get them on the lists.
>
> I tend to think -> is this with OFF mode and CPUidle completely
> working? All context save and restore works with this? on HS and GP
> devices with BE mode builds? works on SDP4430,60 variations,
> considered reuse with AM43xx which could use parts of that logic?
>
> I mean to indicate that terms like "works on panda" tends always to be relative.
Nishanth, let's be objective here.
CPUidle on 4460 does *not* work in mainline for at least two kernel releases
even for LE [1]. That fix exists because that "one group of folks"
faced the issue
during BE testing. Looks like not much people care about CPUIdle on OMAP4.
> It is nice to see it as a proof of concept, but I'd hate to see some
> dead code lying around in kernel and folks blindly following suit and
> introducing macros for new assembly for a feature that in practice
> just one group of folks care about and creates additional burden for
> rest of folks trying to keep that functionality going as we jump from
> one "device tree" style churn to another "framework"? Not to mean that
> good features should be kept away.. but personally, I could not find
> convincing arguments in this case..
In general I understand your concerns from previous e-mails, but I don't
see a point to spend time for a generic solution for a single user.
If there will be other platforms which need similar changes, then we can
think of some generic solution. Let's drop this patch for now.
We can just disable CPUidle for BE tasks or keep this patch forked.
[1] https://lkml.org/lkml/2013/10/22/401
--
Regards,
Taras Kondratiuk
On Tuesday 14 January 2014 04:13 PM, Nishanth Menon wrote:
> On Tue, Jan 14, 2014 at 3:03 PM, Santosh Shilimkar
> <[email protected]> wrote:
>>
>>> ok.. some sort of Linaro thing about which I have no background about
>>> - but dont really care in this context.
>>>
>> Nothing related Linaro. Its just that platforms are supporting ARM BE
>> mode and Linaro folks had working patches for Panda. So I suggested
>> to get them on the lists.
>
> I tend to think -> is this with OFF mode and CPUidle completely
> working? All context save and restore works with this? on HS and GP
> devices with BE mode builds? works on SDP4430,60 variations,
> considered reuse with AM43xx which could use parts of that logic?
>
> I mean to indicate that terms like "works on panda" tends always to be relative.
>
Fair enough.
> It is nice to see it as a proof of concept, but I'd hate to see some
> dead code lying around in kernel and folks blindly following suit and
> introducing macros for new assembly for a feature that in practice
> just one group of folks care about and creates additional burden for
> rest of folks trying to keep that functionality going as we jump from
> one "device tree" style churn to another "framework"? Not to mean that
> good features should be kept away.. but personally, I could not find
> convincing arguments in this case..
>
I haven't looked at patch myself but as you pointed out if it adds
dead code and makes the code un-readable then probably that something
we shouldn't merge.
Regards,
Santosh
* Santosh Shilimkar <[email protected]> [140114 15:46]:
> On Tuesday 14 January 2014 04:13 PM, Nishanth Menon wrote:
>
> I haven't looked at patch myself but as you pointed out if it adds
> dead code and makes the code un-readable then probably that something
> we shouldn't merge.
Yeah it seems the assembly parts should be done in more generic
way using macros so the same setup can then be used for other
SoCs. For the other trivial changes, let's try to get them merged
to shrink down the patchset.
Regards,
Tony