Re: [PATCH v5 05/11] ARM: dts: add imx7d soc dtsi file

[Zhi Li <lznuaa@xxxxxxxxx>]

On Tue, Apr 21, 2015 at 12:28 PM, Mark Rutland <mark.rutland@xxxxxxx> wrote:
>> +/*
>> +               cpu1: cpu@1 {
>> +                       compatible = "arm,cortex-a7";
>> +                       device_type = "cpu";
>> +                       reg = <1>;
>> +               };
>> +*/
>
> Why have it commented out? If it's not available, leave it out entirely.

This patches just bring up mx7.
Have not enabled SMP.
If you don't like, I can remove it.

>
>> +       intc: interrupt-controller@31001000 {
>> +               compatible = "arm,cortex-a7-gic";
>> +               #interrupt-cells = <3>;
>> +               interrupt-controller;
>> +               reg = <0x31001000 0x1000>,
>> +                     <0x31002000 0x100>;
>> +       };
>
> GICC should be larger than 0x100 bytes.
>
> What about GICH, GICV?

Seem we have not used it.
I can added.

>
> What state do CPUs enter the kernel?

Security mode.

>
>> +
>> +       clocks {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               ckil: clock@0 {
>> +                       compatible = "fixed-clock";
>> +                       reg = <0>;
>> +                       #clock-cells = <0>;
>> +                       clock-frequency = <32768>;
>> +                       clock-output-names = "ckil";
>> +               };
>> +
>> +               osc: clock@1 {
>> +                       compatible = "fixed-clock";
>> +                       reg = <1>;
>> +                       #clock-cells = <0>;
>> +                       clock-frequency = <24000000>;
>> +                       clock-output-names = "osc";
>> +               };
>> +
>> +       };
>
> Get rid of the clocks container node, and put these under the root.

Okay.

>
>> +
>> +       timer {
>> +               compatible = "arm,armv7-timer";
>> +               arm,cpu-registers-not-fw-configured;
>
> This was bad enough the first time...

What's means. Our platform need it to access physical counter.
virtual count offset is random data.

>
>> +               interrupts = <GIC_PPI 13
>> +                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +                       <GIC_PPI 14
>> +                       (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>
> Two interrupts are missing.
>
>> +               interrupt-parent = <&intc>;
>> +               clock-frequency = <8000000>;
>
> Why isn't this programmed?
>
>> +       };
>> +
>> +       soc {
>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               compatible = "simple-bus";
>> +               interrupt-parent = <&intc>;
>> +               ranges;
>> +
>> +               pmu {
>> +                       compatible = "arm,cortex-a7-pmu";
>> +                       interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
>> +                       status = "disabled";
>> +               };
>
> Move this under the root. It doesn't live on an SoC-specific bus.
>
> Why disabled?

This patch just is just for bring up imx7.
Only basic function enabled.
If you like, I can remove this part.

>
> Thanks,
> Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html