Re: [PATCH 1/3] powerpc/mpic: finish supporting timer group B on Freescale chips

[Scott Wood <scottwood@freescale.com>]

On 07/09/2012 12:36 PM, Kumar Gala wrote:
> 
> On Jul 9, 2012, at 11:43 AM, Scott Wood wrote:
> 
>> On 07/09/2012 09:12 AM, Kumar Gala wrote:
>>>
>>> On Jul 9, 2012, at 3:45 AM, Varun Sethi wrote:
>>>
>>>> Previously, these interrupts would be mapped, but the offset
>>>> calculation was broken, and only the first group was initialized.
>>>>
>>>> Signed-off-by: Scott Wood <scottwood@freescale.com>
>>>> ---
>>>> arch/powerpc/include/asm/mpic.h |    5 +++
>>>> arch/powerpc/sysdev/mpic.c      |   58 ++++++++++++++++++++++++++++-----------
>>>> 2 files changed, 47 insertions(+), 16 deletions(-)
>>
>> Varun, where's your signoff?
>>
>>>> +	if (mpic->flags & MPIC_FSL) {
>>>> +		u32 brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs,
>>>> +				      MPIC_FSL_BRR1);
>>>> +		u32 version = brr1 & MPIC_FSL_BRR1_VER;
>>>> +
>>>> +		/*
>>>> +		 * Timer group B is present at the latest in MPIC 3.1 (e.g.
>>>> +		 * mpc8536).  It is not present in MPIC 2.0 (e.g. mpc8544).
>>>> +		 * I don't know about the status of intermediate versions (or
>>>> +		 * whether they even exist).
>>>> +		 */
>>>> +		if (version >= 0x0301)
>>>> +			num_timers = 8;
>>>> +	}
>>>> +
>>>
>>> Why don't we do this just via the device tree?
>>
>> Then we'd have to change existing device trees (again), and in general
>> there's no reason to put it in the device tree if it's discoverable via
>> hardware version registers.
> 
> Except for the whole AMP issue ;).  One reason we did add the 2nd bank of timers was for AMP.

What's the AMP issue?  This is just for determining whether the timer
IVPRs exist and can be requested.  The interrupt should not be requested
if the timer node isn't in the tree, unless the caller has other
knowledge that it is valid (such as a KVM directly assigned interrupt).

> Also, we have this in the .dts already:
> 
> $ git grep pq3-mpic-timer-B.dtsi arch/powerpc/boot/dts/
> arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi"
> arch/powerpc/boot/dts/fsl/mpc8572si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi"
> arch/powerpc/boot/dts/fsl/p1010si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi"
> arch/powerpc/boot/dts/fsl/p1020si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi"
> arch/powerpc/boot/dts/fsl/p1021si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi"
> arch/powerpc/boot/dts/fsl/p1022si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi"
> arch/powerpc/boot/dts/fsl/p1023si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi"
> arch/powerpc/boot/dts/fsl/p2020si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi"

Right, but what if we remove that node to keep the host from accessing
those timers, and still want to assign it to a KVM guest?  I thought you
were asking for a property on the MPIC node.

-Scott