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

[Mika Westerberg]

On Fri, May 15, 2026 at 11:34:41AM +0200, Konrad Dybcio wrote:
> 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@xxxxxxxxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxxxxxxxx>
> >> ---
> >>  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

Yeah, I was thinking if that would make the calls more readable but I guess
we can do this later if at all.