Hi,

On 18/04/2020 00:12, Stefano Stabellini wrote:
On Fri, 17 Apr 2020, Julien Grall wrote:
Hi,

The title claim this is a resend, but this is actually not the latest
version of this patch. Can you explain why you decided to use v1
rather than v2?

Because v2 added a printk for every read, and I thought you only wanted
the range fix. Also, the printk is not a "warn once" so after the
discussion where it became apparent that the register can be read many
times, it sounded undesirable.

You previously mentionned that you will use Peng's patch. From my perspective, this meant you are using the latest version of a patch not a previous one.

So this was a bit of a surprised to me.


Nonetheless I don't have a strong preference between the two. If you
prefer v2 it is here:

https://marc.info/?l=xen-devel&m=157440872522065
I understand the "warn" issue but we did agree with it back then. I am ok to drop it. However, may I request to be more forthcoming in your patch in the future?

For instance in this case, this a link to the original patch and an explanation why you selected v1 would have been really useful.


Do you need me to resent it? If it is OK for you as-is, you can give
your ack here, and I'll go ahead and commit it.


On Fri, 17 Apr 2020 at 23:16, Stefano Stabellini <[email protected]> wrote:

From: Peng Fan <[email protected]>

The end should be GICD_ISACTIVERN not GICD_ISACTIVER.

(See https://marc.info/?l=xen-devel&m=158527653730795 for a discussion on
what it would take to implement GICD_ISACTIVER/GICD_ICACTIVER properly.)

Signed-off-by: Peng Fan <[email protected]>
Signed-off-by: Stefano Stabellini <[email protected]>
Reviewed-by: Stefano Stabellini <[email protected]>

I don't think you can be at the same time an author of the patch and a
reviewer. Otherwise, I could review my own patch ;).

Yeah ... that was not the intention. I changed the commit message so I
had to add my signed-off-by for copyright reasons.
At the same time I
reviewed it even before changing the commit message so I added the
reviewed-by. I agree it looks kind of weird.

I don't feel this should go as-is. It is not clear in the commit message the changes you did and could lead confusion once merged. One may think you reviewed your own patch.

In general when I tweak a commit message, I will not add my signed-off-by but just add [julieng: Tweak commit message] above my reviewed-by/acked-by tag.

Alternatively, you can remove your reviewed-by and let another maintainer reviewing it.

I will let you decide your preference and resend the patch appropriately.

Cheers,

--
Julien Grall

Reply via email to