Re: [Xen-devel] [RFC v2][PATCH] arm-acpi: Add ITS Support for Dom0

[Julien Grall <julien.grall@xxxxxxx>]

On 13/06/17 12:44, Manish Jaggi wrote:
> On 6/13/2017 4:58 PM, Julien Grall wrote:
> > On 13/06/17 12:02, Manish Jaggi wrote:
> > > > > Will the below code be ok?
> > > > If you noticed, I didn't say this code is wrong. Instead I asked why
> > > > you use the same ID. Meaning, is there anything in the DSDT requiring
> > > > this value?
> > > >
> > > > > + int tras_id = 0;
> > > > unsigned.
> > > >
> > > > > + list_for_each_entry(its_data, &host_its_list, entry)
> > > > > + {
> > > > > +    gic_its->translation_id = ++trans_id;
> > > > You start the translation ID at 1. Why?
> > > as per the ACPI spec the value should be unique to each GIC ITS unit.
> > > Does starting with 1 break anything? Or should I start with a magic
> > > number?
> > Rather than arguing on the start value here, you should have first
> > answer to the question regarding the usage of translation_id.
> in v1 I assumed that it would be the same as read from host its tables,
> so it would have a unique value as programmed by host firmware.
> > I understand that nobody is using it today. However, when I asked
> > around me nobody ruled out to any future usage of GIC ITS ID and
> > request this to be kept as it is.
> >
> > This means that you can simply copy over the ACPI tables. Rather
> > regenerating them.
> I dont follow your comment, a bit confused
> In v1 you mentioned that "Please explain why you need to have the same
> ID as the host."
> now when you say we copy over the translation_id would be same as that
> of host?
Usually when I say: "Please explain..." it means I want more 
documentation in the code because I am not sure to follow why it is 
necessary. It does not mean "The code is wrong". If it was, I would have 
clearly wrote it and give justification on it.
Furthermore, this kind of documentation will help a reader to understand 
your code and avoid spending hours to find a justification.
The contributor should be able to justify any code he wrote and help the 
reviewers to understand the patch quickly.
Cheers,

--
Julien Grall

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