On Thu, 22 Apr 2010, Magnus Damm wrote:
> On Thu, Apr 22, 2010 at 3:28 PM, Guennadi Liakhovetski
> <g.liakhovetski@xxxxxx> wrote:
> > On Wed, 21 Apr 2010, Magnus Damm wrote:
> >> On Wed, Apr 21, 2010 at 11:36 AM, Guennadi Liakhovetski
> >> <g.liakhovetski@xxxxxx> wrote:
> >> > SuperH SDHI controllers can use DMA, add slave IDs to the header and slave
> >> > definitions to sh7722.
> >> >
> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> >> > ---
> >> > arch/sh/include/asm/dmaengine.h | 4 ++++
> >> > arch/sh/kernel/cpu/sh4a/setup-sh7722.c | 10 ++++++++++
> >> > 2 files changed, 14 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/sh/include/asm/dmaengine.h b/arch/sh/include/asm/dmaengine.h
> >> > index 2a02b61..876e601 100644
> >> > --- a/arch/sh/include/asm/dmaengine.h
> >> > +++ b/arch/sh/include/asm/dmaengine.h
> >> > @@ -29,6 +29,10 @@ enum {
> >> > SHDMA_SLAVE_SIUA_RX,
> >> > SHDMA_SLAVE_SIUB_TX,
> >> > SHDMA_SLAVE_SIUB_RX,
> >> > + SHDMA_SLAVE_SDHI0_RX,
> >> > + SHDMA_SLAVE_SDHI0_TX,
> >> > + SHDMA_SLAVE_SDHI1_RX,
> >> > + SHDMA_SLAVE_SDHI1_TX,
> >> > };
> >> >
> >> > #endif
> >>
> >> Thanks for your work on this. Two SDHIs may be enough for most
> >> processor types, but I think the G3 processor actually has three. With
> >> that in mind, I think it makes more sense to keep the slave ids
> >> together with the processor specific bits in for instance
> >> arch/sh/include/cpu-sh4/cpu/sh7722.h.
> >>
> >> If you have time, can you please make a patch that moves the slave ids
> >> into the processor specific header files? This on top of the other
> >> code in the sh/dmaengine topic branch.
> >
> > Ok, yes, first, I forgot to mention - this patch series is based on your
> > earlier SH dmaengine patches. Secondly - sure, we can move these defines
> > in cpu-specific headers, otoh - why? This is just an enum, it doesn't take
> > space in binaries, and having them all together helps uniformity and
> > avoids duplication, I think. So, I would just add those SDHI2 macros to
> > this list. The important thing is, the actual slave definition array are
> > per-CPU. Which is now the case after your patch series.
>
> Like you say, the actual slave definition array is per processor type.
> But the available hardware blocks varies with processor type, and we
> already have other processor specific information in the cpu-specific
> headers. It does not add any code size, true.
>
> The reasons why I prefer them to be split out are:
>
> 1) When someone adds support for a new processor it's easy to
> implement the same type of information in the new cpu-specific header
> as other processors. Adding and removing things from a global list of
> processor blocks in a driver/framework specific header file hidden
> somewhere is not going to happen. Also, people usually only focus on
> one processor at a time. A long global list is just going to make
> people confused. "Why is there a SDHI2 there when I only have SDHI on
> my processor?"
>
> 2) Having them in a global list adds an unneccesary point of patch
> serialization.
>
> 3) We keep other things like GPIOs and HWBLK in cpu-specific headers
> already, why be special?
ok... you mean headers like arch/sh/include/cpu-sh4/cpu/sh7722.h, yeah,
that makes sense, agree. Will redo.
> 4) Having it in a global list may encourage people to include this
> header from drivers. Isn't it better to force the drivers to use this
> value as a cookie instead? This prevents people from adding hardware
> block specific workarounds to the driver that may or may not work on
> all processors...
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
