Messages in this thread | | | Date | Thu, 30 Apr 2020 10:07:33 +0200 | From | Boris Brezillon <> | Subject | Re: [PATCH v6 3/8] i3c: master: i3c mastership request and handover |
| |
On Fri, 17 Apr 2020 18:21:02 +0200 Parshuram Thombare <pthombar@cadence.com> wrote:
> + > +/* This function is expected to be called with normaluse_lock */ > +int i3c_master_acquire_bus(struct i3c_master_controller *master) > +{ > + int ret = 0; > + > + if (!master->this || master->this != master->bus.cur_master) {
Let's limit the number of indentations by doing
if (master->this == master->bus.cur_master) return 0;
> + if (master->mr_state == I3C_MR_IDLE) { > + master->mr_state = I3C_MR_WAIT_DA; > + init_completion(&master->mr_comp); > + queue_work(master->wq, &master->sec_mst_work); > + /* > + * Bus acquire procedure may need write lock > + * so release read lock before yielding > + * to bus acquire state machine > + */ > + i3c_bus_normaluse_unlock(&master->bus); > + wait_for_completion(&master->mr_comp); > + i3c_bus_normaluse_lock(&master->bus);
Is that really enough? I remember we had something a bit more complex to handle the case where bus is acquired to send a message to a device, and another master on the bus re-acquires it before we have a chance to send this message message. i3c_master_acquire_bus_ownership() was dealing with that in Przemek series. It seems you've rewritten a lot of these things. Would you mind explaining why, and how that works?
> + if (master->mr_state != I3C_MR_DONE) > + ret = -EAGAIN; > + master->mr_state = I3C_MR_IDLE; > + } else { > + /* > + * MR request is already in process for > + * this master > + */ > + ret = -EAGAIN; > + } > + } > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(i3c_master_acquire_bus);
| |