Re: [PATCH v3 3/4] thunderbolt: Require nhi->ops be valid

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 5/15/26 8:30 AM, Mika Westerberg wrote:
> On Wed, May 13, 2026 at 06:23:34PM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> Because of how fundamental ops->init_interrupts() is, it no longer
>> makes sense to consider cases where nhi->ops is NULL.
>>
>> Drop some boilerplate around it and add a single sanity-check in
>> nhi_probe() instead.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---
>>  drivers/thunderbolt/nhi.c    | 32 ++++++++++++++++++--------------
>>  drivers/thunderbolt/switch.c |  6 +++---
>>  2 files changed, 21 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
>> index 740c10ee852b..2a8d1b3716c0 100644
>> --- a/drivers/thunderbolt/nhi.c
>> +++ b/drivers/thunderbolt/nhi.c
>> @@ -559,7 +559,7 @@ static struct tb_ring *tb_ring_alloc(struct tb_nhi *nhi, u32 hop, int size,
>>  	if (!ring->descriptors)
>>  		goto err_free_ring;
>>  
>> -	if (nhi->ops && nhi->ops->request_ring_irq) {
>> +	if (nhi->ops->request_ring_irq) {
> 
> I wonder if it makes this more readable if we wrap these like:
> 
> 	if (nhi_request_ring_irq(nhi)) {

The UFS subsystem does that, and it results in a ton of boilerplate,
i.e. for each op you need to define something like a:

static inline T nhi_foo_bar(struct tb_nhi *nhi, ...)
{
	if (nhi->ops->foo_bar)
		return nhi->ops->foo_bar(...);

	return 0;
}

I can do that, but I don't see real value here

Konrad