On Thu, Sep 30, 2010 at 12:08 PM, Wolfgang Denk <w...@denx.de> wrote: [snip other valid review comments] > > This is a header file, yet you add here literally thousands of lines of > code.
Yes, these functions are entirely too large to be inlined. It looks like you are trying to borrow too heavily from the iop-adma model. The differences between iop13xx and iop33x from a adma perspective are just in descriptor format and channel capabilities. If you look at the routines implemented in: arch/arm/include/asm/hardware/iop3xx-adma.h arch/arm/mach-iop13xx/include/mach/adma.h ...they are just simple helpers that abstract the descriptor details. For example: iop_adma_prep_dma_xor() { [snip] spin_lock_bh(&iop_chan->lock); slot_cnt = iop_chan_xor_slot_count(len, src_cnt, &slots_per_op); sw_desc = iop_adma_alloc_slots(iop_chan, slot_cnt, slots_per_op); if (sw_desc) { grp_start = sw_desc->group_head; iop_desc_init_xor(grp_start, src_cnt, flags); iop_desc_set_byte_count(grp_start, iop_chan, len); iop_desc_set_dest_addr(grp_start, iop_chan, dma_dest); sw_desc->unmap_src_cnt = src_cnt; sw_desc->unmap_len = len; sw_desc->async_tx.flags = flags; while (src_cnt--) iop_desc_set_xor_src_addr(grp_start, src_cnt, dma_src[src_cnt]); } spin_unlock_bh(&iop_chan->lock); return sw_desc ? &sw_desc->async_tx : NULL; } Where iop_adma_alloc_slots() is implemented differently between iop13xx and iop3xx. In this case why does ppc440spe-adma.h exist? If it has code specific to ppe440spe it should just live in the ppe440spe C file. If it is truly generic it should move to the base adma.c implementation. If you want to reuse a ppe440spe routine just link to it. > Selecting the architecture at build time is bad as it prevents using a > sinlge kernel image across a wide range of boards. You only replied > "We select the architecture at build time." without any explanation if > there is a pressing technical reason to do it this way, or if this was > just a arbitrary decision. I agree I have yet to see any indication that this driver needs to have an architecture selected at build time. A potential compromise a first step would be to have a common C file that is shared between two driver modules until such point that they can be unified into a common module. -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html