On 29/03/17 11:37, Leo Yan wrote:
On Wed, Mar 29, 2017 at 11:31:03AM +0100, Suzuki K Poulose wrote:
On 29/03/17 11:27, Leo Yan wrote:
On Wed, Mar 29, 2017 at 10:07:07AM +0100, Suzuki K Poulose wrote:
[...]
+ if (mode == EDDEVID_IMPL_NONE) {
+ drvdata->edpcsr_present = false;
+ drvdata->edcidsr_present = false;
+ drvdata->edvidsr_present = false;
+ } else if (mode == EDDEVID_IMPL_EDPCSR) {
+ drvdata->edpcsr_present = true;
+ drvdata->edcidsr_present = false;
+ drvdata->edvidsr_present = false;
+ } else if (mode == EDDEVID_IMPL_EDPCSR_EDCIDSR) {
+ if (!IS_ENABLED(CONFIG_64BIT) &&
+ (pcsr_offset == EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32))
+ drvdata->edpcsr_present = false;
+ else
+ drvdata->edpcsr_present = true;
Sorry, I forgot why we do this check only in this mode. Shouldn't this be
common to all modes (of course which implies PCSR is present) ?
No. PCSROffset is defined differently in ARMv7 and ARMv8; So finally we
simplize PCSROffset value :
0000 - Sample offset applies based on the instruction state (indicated by PCSR[0])
0001 - No offset applies.
0010 - No offset applies, but do not use in AArch32 mode!
So we need handle the corner case is when CPU runs AArch32 mode and
PCSRoffset = 'b0010. Other cases the pcsr should be present.
I understand that reasoning. But my question is, why do we check for PCSROffset
only when mode == EDDEVID_IMPL_EDPCSR_EDCIDSR and not for say mode == EDDEVID_IMPL_EDPCSR or
any other mode where PCSR is present.
Sorry I misunderstood your question.
I made mistake when I analyzed the possbile combination for mode and
PCSROffset so I thought it's the only case should handle:
{ EDDEVID_IMPL_EDPCSR_EDCIDSR, EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32 }
Below three combinations are possible to exist; so you are right, I
should move this out for the checking:
{ EDDEVID_IMPL_NONE, EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32 }
That need not be covered, as IMPL_NONE says PCSR is not implemented hence you
don't worry about anything as the functionality is missing. This should rather be:
EDDEVID_IMPL_EDPCSR, where only PCSR is implemented.
I think below combination doesn't really exist:
{ EDDEVID_IMPL_EDPCSR, EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32 };
EDDEVID_IMPL_EDPCSR is only defined in ARMv7 ARM, and
EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32 is only defined in ARMv8 ARM.
It is not wrong to check the PCSROffset in all cases where PCSR is available, as if
we hit PCSR on ARMv7 then PCSROffset shouldn't be DIS_AARCH32. And in fact that
would make the code a bit more cleaner. Anyways, I am not particular about this.
Suzuki
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
