On Tue, Jun 7, 2011 at 2:37 PM, Laurent Gauch <laurent.ga...@amontec.com>wrote:

> Hi Andreas,
>
> Thank you for your comment.
>
>
>  It doesn't build. Leftover variable no longer used.
>>
> minor issue
>
>  Apart from that it seems OK from my limited review. I could agree with the
>> others that it looks like a needlessly complex fix for a simple missing
>> free. Maybe there's a bigger picture here but I don't get the open/init
>> deinit/close separation or why it's needed (Why would you want to open
>> without initializing?). Haven't given it much thought though. Is the
>> mentioned followup patch already posted?
>>
>
> The mentioned patches are not posted, but ready to.
>
> As asked by maintainer's, we want to have small and precise patches. We
> only posted the patch 1/5 .
> https://lists.berlios.de/pipermail/openocd-development/2011-May/019176.htmlWe 
> want to have it merged before to give the 2..5/5.
> The 2..5/5 will be depending of the acceptation of 1/5 ...
>
>
That is the problem with this patch. It adresses the problem of a memory
leak, while at the same time it tries to sneak in an unrelated refactoring
in preparation for some future patches we still haven't seen or discussed. I
guess that's why this patch has received so little attention and acceptance.
That, plus your threatening attitude towards the community and the
maintainers when criticism is raised.

I suggest to drop this patch and merge a simpler fix for the problem
adressed by this patch. Only a few lines of code would need to change.

The refactoring part of this patch should be made part of the upcoming
patchset for the apparent open/init/deinit/close-problem. Although I can't
see why that separation would be necessary or even desired to fix the
problem with leaving the ft2232 in a known state after shutdown. Preparing
the ft2232 driver for the SWD work is futile since it's no such work is in
place and we can't be sure what will be required in terms of API changes.


---
>
> We do not want to open without init, but we want to init or re-init an
> already opened device handle :-)
> In the same way, we want to deinit without having to close the device
> handle !
>
> In an other way, we want to be able to force deinit and close or close only
> without having to have a interface quit() call from the upper layer, in case
> something wrong during open init or others.
>
> We want to make sure any USB JTAG SWD adapters based ft2232 (as jtagkey)
> are correctly deinitialized at any shutdown of openocd (as via
> jtagkey_deinit).
> (Actually nothing is done during the quit except close the handle ... but
> we have to make sure deassert JTAG PORT AND TRST SRST IO (and to deassert
> SWD mode), to reset the MPSSE, to go away the bit-bang mode, and only then
> close the handle)
>
> If we split this to a deinit() close() we will produce a much better code.
>
> Having open init deinit close well separated IN the ft2232.c will help us
> to make the ft2232 interface more robust and more clean.
>

I beg to differ. A layout->deinit() could be added and called from
ft22332_quit with minimal changes to the driver.


> But remember, the open init deinit close ARE NOT ON THE INTERFACE API,
>
>  That's why they're unnecessary before the API is changed accordingly.

/Andreas
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to