lkml.org 
[lkml]   [2012]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC 21/23] ARM: keystone: introducing TI Keystone platform
Arnd,

Thanks for taking the time to dig through this.

On 7/24/2012 10:46 AM, Arnd Bergmann wrote:
> On Tuesday 24 July 2012, Cyril Chemparathy wrote:
>
>> diff --git a/arch/arm/boot/dts/keystone-sim.dts b/arch/arm/boot/dts/keystone-sim.dts
>> new file mode 100644
>> index 0000000..118d631
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/keystone-sim.dts
>> @@ -0,0 +1,77 @@
>> +/dts-v1/;
>> +/include/ "skeleton.dtsi"
>> +
>> +/ {
>> + model = "Texas Instruments Keystone 2 SoC";
>> + compatible = "ti,keystone-evm";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>
> I would assume that you need at least #address-cells=<2>, possibly
> also #size-cells=<2>, in order to express large memory ranges.
>

Thanks, will fix.

>> diff --git a/arch/arm/mach-keystone/include/mach/entry-macro.S b/arch/arm/mach-keystone/include/mach/entry-macro.S
>> new file mode 100644
>> index 0000000..7f486f3
>> --- /dev/null
>> +++ b/arch/arm/mach-keystone/include/mach/entry-macro.S
>>
>> + .macro disable_fiq
>> + .endm
>> +
>> + .macro arch_ret_to_user, tmp1, tmp2
>> + .endm
>
> I think it would be better to enable MULTI_IRQ_HANDLER and remove
> this file.
>

Agreed.

>> diff --git a/arch/arm/mach-keystone/include/mach/io.h b/arch/arm/mach-keystone/include/mach/io.h
>> new file mode 100644
>> index 0000000..844d659
>> --- /dev/null
>> +++ b/arch/arm/mach-keystone/include/mach/io.h
>
>> +#ifndef __MACH_IO_H
>> +#define __MACH_IO_H
>> +
>> +#define __io(a) ({ (void)(a); __typesafe_io(0); })
>> +#define __mem_pci(a) (a)
>> +
>> +#endif
>
> This should also be removed.
>

Agreed.

>> diff --git a/arch/arm/mach-keystone/include/mach/memory.h b/arch/arm/mach-keystone/include/mach/memory.h
>> new file mode 100644
>> index 0000000..7c78b1e
>> --- /dev/null
>> +++ b/arch/arm/mach-keystone/include/mach/memory.h
>
>> +#ifndef __ASM_MACH_MEMORY_H
>> +#define __ASM_MACH_MEMORY_H
>> +
>> +#define MAX_PHYSMEM_BITS 36
>> +#define SECTION_SIZE_BITS 34
>> +
>> +#endif /* __ASM_MACH_MEMORY_H */
>
> I wonder if there is anything we can do to make these generic. What you
> have here is ok for now, but we will need to do this differently once
> we are building multiplatform kernels with keystone and sparse memory.
>

Understood. Any ideas on the general direction towards solving this?

>> diff --git a/arch/arm/mach-keystone/include/mach/system.h b/arch/arm/mach-keystone/include/mach/system.h
>> new file mode 100644
>> index 0000000..4887b4c
>> --- /dev/null
>> +++ b/arch/arm/mach-keystone/include/mach/system.h
>
>> +#ifndef __MACH_SYSTEM_H
>> +#define __MACH_SYSTEM_H
>> +
>> +static inline void arch_idle(void)
>> +{
>> + cpu_do_idle();
>> +}
>> +
>> +static inline void arch_reset(char mode, const char *cmd)
>> +{
>> + while (1)
>> + ;
>> +}
>> +
>> +#endif
>
> These are no longer used, please remove the file.
>

Sure.

>
>> diff --git a/arch/arm/mach-keystone/include/mach/vmalloc.h b/arch/arm/mach-keystone/include/mach/vmalloc.h
>> new file mode 100644
>> index 0000000..9d34c09
>> --- /dev/null
>> +++ b/arch/arm/mach-keystone/include/mach/vmalloc.h
>
>> +
>> +#define VMALLOC_END 0xFE800000UL
>> +
>> +#endif
>
> same here.
>

Sure.

>> +DT_MACHINE_START(KEYSTONE, "Keystone")
>> + .map_io = keystone_map_io,
>> + .init_irq = keystone_init_irq,
>> + .timer = &keystone_timer,
>> + .handle_irq = gic_handle_irq,
>> + .init_machine = keystone_init,
>> + .dt_compat = keystone_match,
>> + .nr_irqs = 480,
>> +MACHINE_END
>
> IIRC, you don't need to set the nr_irqs this high in advance,
> they will be allocated automatically since you have enabled
> sparse IRQs.
>

We were seeing a complaint without nr_irqs set, but I'll dig into that
further.

--
Thanks
- Cyril


\
 
 \ /
  Last update: 2012-07-24 20:41    [W:0.080 / U:0.960 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site