linux-kernel - Re: [PATCH] DMA: TXx9 Soc DMA Controller driver (v2)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49ECF040.2000508@intel.com>
Date:	Mon, 20 Apr 2009 14:59:28 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	Atsushi Nemoto <anemo@....ocn.ne.jp>
CC:	"linux-mips@...ux-mips.org" <linux-mips@...ux-mips.org>,
	"ralf@...ux-mips.org" <ralf@...ux-mips.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] DMA: TXx9 Soc DMA Controller driver (v2)

Atsushi Nemoto wrote:
> On Sat, 18 Apr 2009 13:05:15 -0700, Dan Williams <dan.j.williams@...el.com> wrote:
>> Not quite "ackable" yet...
> 
> Thank you for review!
> 
>>> +#ifdef CONFIG_MACH_TX49XX
>>> +#define TXX9_DMA_MAY_HAVE_64BIT_REGS
>>> +#define TXX9_DMA_HAVE_CCR_LE
>>> +#define TXX9_DMA_HAVE_SMPCHN
>>> +#define TXX9_DMA_HAVE_IRQ_PER_CHAN
>>> +#endif
>>> +
>>> +#ifdef TXX9_DMA_HAVE_SMPCHN
>>> +#define TXX9_DMA_USE_SIMPLE_CHAIN
>>> +#endif
>>> +
>> There seems to be a lot of ifdef magic in the code based on these
>> defines.  Can we move this magic and some of the pure definitions to
>> drivers/dma/txx9dmac.h?  (See the "#ifdefs are ugly" section of
>> Documentation/SubmittingPatches)
> 
> OK, I will try to clean them up.  But since I don't want to export
> internal implementation details, some of the magics will be left in
> txx9dmac.c, perhaps.

You only need to hide txx9dmac magic if the header was in 
include/linux/, but since it will be in drivers/dma/ you can assume it 
is private.
                }
>> Is there a reason to keep f'irst' off of the tx_list?  It seems like
>> you could simplify this logic and get rid of the scary looking
>> list_splice followed by list_add in txx9dmac_desc_put.  It also seems
>> odd that the descriptors on tx_list are not reachable from the
>> dc->queue list after a submit... but maybe I am missing a subtle
>> detail?
> 
> Well, I'm not sure what do you mean...
> 
> The completion callback handler of the first descriptor should be
> called _after_ the completion of the _last_ child of the descriptor.
> Also I use desc_node for both dc->queue, dc->active_list and
> txd.tx_list.  So if I putted all children to dc->queue or
> dc->active_list, txx9dmac_descriptor_complete() (or its caller) will
> be more complex.
> 
> Or do you mean adding another list_head to maintain txd.tx_list?  Or
> something another at all?

The piece I was missing was that it would make 
txx9dmac_descriptor_complete() more complex.  So, I am fine with the 
leaving the current implementation.

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