Re: [RFC PATCHv5 1/7] HSI: hsi: Introducing HSI framework

[Carlos Chinea]

On Fri, 2011-07-22 at 15:05 +0300, ext Felipe Balbi wrote:
> Hi,
> 
> On Fri, Jul 22, 2011 at 02:51:12PM +0300, Carlos Chinea wrote:
> > Hi Felipe, 
> 
> hello there :-)
> 
> > > are you already using pm_runtime ? You could put that logic grouped in
> > > runtime_suspend()/runtime_resume() calls and from the driver, just call
> > > pm_runtime_put()/pm_runtime_get_sync() whenever you want to
> > > suspend/resume.
> > 
> > In the case of the omap_ssi driver not yet, but hopefully the
> > implementation will come soon ;) Anyway, the DATA/FLAG workaround is not
> > something I'm going to add to the omap_ssi, at least not now. This seems
> > to be needed by ST-Ericcson in their HSI controller.
> 
> aha, I see. Thanks for the clarification.
> 
> > > true, but I'm not sure a usecount is a good way to go. Why don't you
> > > just remove the sync API altogether and use only async, then the OMAP
> > > HSI controller driver is supposed to know when it can go to sleep. If
> > > you receive some data before a client queues a request, you just defer
> > > processing of that data until a new request is queued, or something...
> > 
> > Hmmm, Do you mean I remove the hsi_start_tx() and hsi_stop_tx()
> > completely ? Or Do I just create an async version of them ?
> 
> I would say remove completely and add async-only version.
> 
> > The truth is that they should not even exist in the first place, and the
> > wakelines should have been handled silently by the hsi controllers.  
> > But they are need because the hsi_clients/protocols, like CAIF or the
> > ssi_protocol in N900, need access to the wakelines to implement some
> > workarounds due to some races in the HSI/SSI HW spec.
> 
> that's quite nasty situation :-)
> 
> I still think, though, that if the clients don't really start a transfer
> straight away (meaning it's all ASYNC), you might be able to handle the
> workarounds in the HSI framework itself by having some extra flags on
> your hsi request structure (?).
> 
> See for example the use of short_not_ok flags on the gadget framework.
> The gadget drivers which can't handle short transfers (as of today only
> the Mass Storage gadget) will set that flag to tell the controller that
> we're not expecting a short packet and if we do, treat it as error. In
> case of such error, gadget driver is required to dequeue any queued
> transfer, flush the FIFO and restart ;-)
> 
> Maybe you could have something similar ? Depending on the workaround
> you're talking about, this might be feasible with few lines of code on
> the async API.
> 

The problem is that there is no standard way to handle this situation :(

For example ssi_protocol tries to deal with the wakelines race problem
by raising the wakeline and then waiting for the other end to send a
"READY to receive" data frame, before start sending. But as you can see
in the case of CAIF, they went for a solution where they raise their
wakeline and wait for the other end to raise its wakeline to signal that
they are "READY to received". Both solutions have their pros and cons.

Br,
-- 
Carlos Chinea <carlos.chinea@xxxxxxxxx>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/