Re: [PATCH v4 4/6] ARM: rockchip: add basic smp support for rk3288

From: Kever Yang
Date: Wed Oct 15 2014 - 02:49:22 EST


Russell,

On 10/14/2014 04:37 PM, Russell King - ARM Linux wrote:
On Tue, Oct 14, 2014 at 02:50:07PM -0700, Kever Yang wrote:
Heiko,

On 10/14/2014 02:23 PM, Heiko Stübner wrote:
Am Dienstag, 14. Oktober 2014, 13:24:03 schrieb Doug Anderson:
Kever,

On Mon, Oct 13, 2014 at 1:12 PM, Kever Yang <kever.yang@xxxxxxxxxxxxxx>
wrote:
+ /*
+ * We need to soft reset the cpu when we turn off the cpu power
domain, + * or else the active processors might be stalled when
the individual + * processor is powered down.
+ */
+ if (read_cpuid_part_number() != ARM_CPU_PART_CORTEX_A9) {
I haven't done a full review of this patch, but it seems unlikely that
your uses of read_cpuid_part_number() and read_cpuid_part() in this
patch are correct. You use both functions and in both cases compare
the results to ARM_CPU_PART_CORTEX_A9.
I think read_cpuid_part() would be the correct one, as it does
You are right, read_cpuid_part() is correct one on kernel next,
I mix up 3.14 kernel and next tree, only read_cpuid_part_number() is
available
in 3.14 kernel.

I will correct it in my next version, any other changes needed for new
version?
You need to at the _very_ _minimum_ build test your code against a
recent kernel, and preferably test it to make sure that it works as
you intend.
Thanks for you advice, I'll be more careful next time.

I'm sorry for not test this patch with rk3188(A9 based) board,
I do have test it on top of kernel next with my rk3288 evb(not A9 based),
and I didn't get any warning for read_cpuid_part_number()
during building the image.

- Kever

Developing on such an old kernel version, and hoping that the code is
going to be correct for later kernels isn't a nice idea.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/