Re: [Xen-devel] [PATCH for-4.5 v11 2/7] xen/arm: Add vgic callback to read irq priority

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Tue, Sep 16, 2014 at 12:35 AM, Stefano Stabellini
<stefano.stabellini@xxxxxxxxxxxxx> wrote:
> On Mon, 15 Sep 2014, Vijay Kilari wrote:
>> On Fri, Sep 12, 2014 at 11:38 PM, Stefano Stabellini
>> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> > On Fri, 12 Sep 2014, vijay.kilari@xxxxxxxxx wrote:
>> >> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
>> >>
>> >> Use callback in vgic driver to read priority for
>> >> a given irq
>> >>
>> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
>> >> ---
>> >> v11: use vgic_irq_rank
>> >> ---
>> >>  xen/arch/arm/vgic-v2.c     |   12 ++++++++++++
>> >>  xen/arch/arm/vgic.c        |    2 +-
>> >>  xen/include/asm-arm/vgic.h |    2 ++
>> >>  3 files changed, 15 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
>> >> index 54751b6..e674192 100644
>> >> --- a/xen/arch/arm/vgic-v2.c
>> >> +++ b/xen/arch/arm/vgic-v2.c
>> >> @@ -521,6 +521,17 @@ static struct vcpu *vgic_v2_get_target_vcpu(struct 
>> >> vcpu *v, unsigned int irq)
>> >>      return v_target;
>> >>  }
>> >>
>> >> +static int vgic_v2_get_irq_priority(struct vcpu *v, unsigned int irq)
>> >> +{
>> >> +    int priority;
>> >> +    struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
>> >
>> > this is good
>> >
>> >
>> >> +    ASSERT(spin_is_locked(&rank->lock));
>> >> +    priority = vgic_byte_read(rank->ipriority[(irq%32)/4], 0, irq % 4);
>> >
>> > this has been changed without mention in the change log for the patch, it 
>> > was
>> >
>> > priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8, irq,
>> >             DABT_WORD)], 0, irq & 0x3);
>> >
>> > It is best to use the previous version, as it uses the common conversion
>> > functions.
>>
>>  I changed it because we don't need do clumsy things
>> to get ipriority index with irq number in hand. If you don't
>> have any further comments, I will send final version with this change
>> for your ack.
>
> TBH I think that the open coded version is more readable than the macro,
> but we need to be consistent (so that we can easily change the macro and
> fix all the call sites in one change for example).
>
> In any case I don't think that you need to resend just for this small
> change. We can fix in a follow up patch.

 I see that you have made similar changes using in vgic-v2.c in

 static struct vcpu *vgic_v2_get_target_vcpu(struct vcpu *v, unsigned int irq)
 {
....
     target = vgic_byte_read(rank->itargets[(irq%32)/4], 0, irq % 4);

shall I revert back to use REG_RANK_INDEX here as well in my next
commit?

Regards
Vijay

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel