Re: [PATCH 2/3] ARM: OMAP2+: HSI: Introduce OMAP SSI driver

[Sebastian Reichel]

Hi,

On Fri, Aug 23, 2013 at 03:57:05PM +0200, Linus Walleij wrote:
> The HSI subsystem is lacking an active maintainer, interested?
> Given that you can apparently test the OMAP HSI driver you're
> one of the few applicable candidates.

I don't think I'm a good candidate for that. At least not yet. This
is my first patch sent to the kernel and I merly took the code from
Carlos Chinea and ported it to some new kernel frameworks. Apart
from that I'm lacking any documentation for the interface (SSI is
not part of TI's public OMAP3 TRM).

> Overall there is one big problem with this patch in that it implements
> a lot of stuff that should not be implemented in the driver at all,
> but in the HSI core.

The actual implementation has not (yet?) been changed by me. It's
still the code as written by Carlos.

> For example compare commit
> ffbbdd21329f3e15eeca6df2d4bc11c04d9d91c0
> "spi: create a message queueing infrastructure"
> 
> This patch basically seems to redo the mistake we did in
> SPI and not create a central message queue from day one,
> instead re-implementing the same code in each and every
> driver.
> 
> Please attempt to draw the message queueuing into the
> driver core atlease.

I will have a look at it after conversion to DT. I will sent another
RFC before this change to speed up the DT review, though.

> Further the allocation of hosts seem pretty generic as well
> but I'm unsure about this. I'd prefer if you take a second
> look at the generalizeable parts.

OK.

> [...] (Code Comments)

Thanks for the review. The code has been written by Carlos, who
apparently had documentation for the SSI IP used in the OMAP3. I
cannot fix the magic numbers without either reverse engineering the
IP (which I do not have the time for) or access to the documentation.

I will fix the other comments in the next RFC patch.

-- Sebastian

Attachment: signature.asc
Description: Digital signature