Re: [PATCH 11/15] RDMA/hbl: add habanalabs RDMA driver

[Omer Shpigelman]

On 7/12/24 16:08, Jason Gunthorpe wrote:
> [You don't often get email from jgg@xxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Fri, Jun 28, 2024 at 10:24:32AM +0000, Omer Shpigelman wrote:
> 
>> We need the core driver to access the IB driver (and to the ETH driver as
>> well). As you wrote, we can't use exported symbols from our IB driver nor
>> rely on function pointers, but what about providing the core driver an ops
>> structure? meaning exporting a register function from the core driver that
>> should be called by the IB driver during auxiliary device probe.
>> Something like:
>>
>> int hbl_cn_register_ib_aux_dev(struct auxiliary_device *adev,
>>                              struct hbl_ib_ops *ops)
>> {
>> ...
>> }
>> EXPORT_SYMBOL(hbl_cn_register_ib_aux_dev);
> 
> Definately do not do some kind of double-register like this.
> 
> The auxiliary_device scheme can already be extended to provide ops for
> each sub device.
> 
> Like
> 
> struct habana_driver {
>    struct auxiliary_driver base;
>    const struct habana_ops *ops;
> };
> 
> If the ops are justified or not is a different question.
> 

Well, I suggested this double-register option because I got a comment that
the design pattern of embedded ops structure shouldn't be used.
So I'm confused now...

>> module reference counter. But we also get the ability to access the IB
>> driver from the core driver (to report a HW error for example).
> 
> Report a HW error seems reasonable to me
> 
> Other driver have used notifier chains for this kind of stuff though.
> 
> Jason

I'll look into the option of using notifier chains in this case, although
as I saw it, the notifier chains are more suitable for broadcast updates
where the updater is not necessarily aware of the identity nor the number
of the subscribers. It looks kind of overkill for our error reporting case
which is simpler.