RE: [PATCH] cdx: add MSI support for CDX bus

[Thomas Gleixner <tglx@linutronix.de>]

Nipun!

On Tue, May 09 2023 at 11:06, Nipun Gupta wrote:
>> -----Original Message-----
>> From: Thomas Gleixner <tglx@linutronix.de>
>> Sent: Tuesday, May 9, 2023 1:32 PM
>> To: Gupta, Nipun <Nipun.Gupta@amd.com>; gregkh@linuxfoundation.org;
>> maz@kernel.org; jgg@ziepe.ca; linux-kernel@vger.kernel.org

Can you please fix your mail client to not copy half of the mail header
into your reply?

>> Caution: This message originated from an External Source. Use proper
>> caution when opening attachments, clicking links, or responding.

That's also relevant information for me, right?
 
>> The only real CDX specific functionality here is a CDX specific
>> irq_write_msi_msg() callback, right?
>> 
>> And I gave you a pointer how this should be handled, but instead of
>> helping this effort along you go off and implement it differently just
>> because. Sigh!
>
> As you rightly mentioned the irq_chip has only irq_write_msi_msg() as
> callback, but there is also cdx_msi_prepare() in msi_domain_ops which
> needs to fetch device ID from CDX device, due to which we are currently
> using separate CDX domain.

Sure. But where is that information in the changelog?

> IIUC, as per your suggestion we should have CDX bus token added into 
> its_init_dev_msi_info() of
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git/tree/drivers/irqchip/irq-gic-v3-its-msi-parent.c?h=devmsi-arm,
> and register CDX specific 'msi_prepare' here; so that we can use
> msi_create_device_irq_domain() to create a per device domain?

Correct.

I'm not insisting on that, but you could at least have had the courtesy
of responding to my review reply and explain to me why you want to solve
it differently and why my suggestion is not the right solution.

Alternatively you could have added that information in the changelog or
cover letter.

So in summary you ignored _all_ review comments I made, went off and did
something different and provided a slightly different useless changelog
with the extra add on of a broken Signed-off-by chain.

Feel free to ignore my reviews and the documentation which we put out
there to make collaboration feasible for both sides, but please don't be
upset when I ignore you and your patches in return.

Thanks,

        tglx