Re: [PATCH v3 fwctl 2/5] fwctl/bnxt_en: Refactor aux bus functions to be more generic

[Pavan Chebbi]

> >-void bnxt_rdma_aux_device_del(struct bnxt *bp)
> >+void bnxt_aux_devices_del(struct bnxt *bp)
> > {
> >-      if (!bp->edev)
> >-              return;
> >+      int idx;
> >
> >-      auxiliary_device_delete(&bp->aux_priv->aux_dev);
> >+      mutex_lock(&bp->auxdev_lock);
> >+      for (idx = 0; idx < __BNXT_AUXDEV_MAX; idx++) {
> >+              if (bnxt_auxdev_is_active(bp, idx)) {
> >+                      auxiliary_device_delete(&bp->aux_priv[idx]->aux_dev);
> >+                      bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_INIT);
>
> I can't really follow why you need to maintain a dangling device state !

This is so because the init-add and del-uninit function pair in bnxt
are not tightly coupled, possibly for some historic reason.
I see it produces regressions trying to club them together. As of now,
these states are going to be in use only during probe/remove.

>
> Why not completely remove and free an aux dev and then on probe/add init
> and add from scratch, simplified patch #3 which prompted me to come and
> look back here.

I see what you are saying. This is a design carried over from
pre-auxbus time between bnxt_en and RoCE drivers and I need a buy-in
from our RoCE driver folks to make this change.
I am very certain I will endup touching RoCE driver code and it will
increase the scope of the changes in this series. If not anything
else, the regression tests will require a lot of time and effort.
That said, it's a good change to make. Let me check with our RoCE team
internally and get back.

>
> >+              }
> >+      }
> >+      mutex_unlock(&bp->auxdev_lock);
> > }
> >

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature