Re: [Openocd-development] NAND: add erase_page callback
Dne Po 14. prosince 2009 21:55:25 David Brownell napsal(a): On Monday 14 December 2009, Marek Vasut wrote: Dne Po 14. prosince 2009 02:46:26 David Brownell napsal(a): On Sunday 13 December 2009, Marek Vasut wrote: I'd send followup patch that'd clean that mess up altogether ... it's cleaner and much easier to track back in git log. Go for it then. :) Merge the erase patch then. :) Not till I see that followup patch ... ;) Actually btw. what's your idea here? I kind-of lost track ... ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
Dne Po 14. prosince 2009 02:46:26 David Brownell napsal(a): On Sunday 13 December 2009, Marek Vasut wrote: I'd send followup patch that'd clean that mess up altogether ... it's cleaner and much easier to track back in git log. Go for it then. :) Merge the erase patch then. :) ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Monday 14 December 2009, Marek Vasut wrote: Dne Po 14. prosince 2009 02:46:26 David Brownell napsal(a): On Sunday 13 December 2009, Marek Vasut wrote: I'd send followup patch that'd clean that mess up altogether ... it's cleaner and much easier to track back in git log. Go for it then. :) Merge the erase patch then. :) Not till I see that followup patch ... ;) ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
Dne Po 14. prosince 2009 21:55:25 David Brownell napsal(a): On Monday 14 December 2009, Marek Vasut wrote: Dne Po 14. prosince 2009 02:46:26 David Brownell napsal(a): On Sunday 13 December 2009, Marek Vasut wrote: I'd send followup patch that'd clean that mess up altogether ... it's cleaner and much easier to track back in git log. Go for it then. :) Merge the erase patch then. :) Not till I see that followup patch ... ;) You won't till I see this one in git ... ;) ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Mon, Dec 14, 2009 at 6:42 PM, Marek Vasut marek.va...@gmail.com wrote: Dne Po 14. prosince 2009 21:55:25 David Brownell napsal(a): On Monday 14 December 2009, Marek Vasut wrote: Dne Po 14. prosince 2009 02:46:26 David Brownell napsal(a): On Sunday 13 December 2009, Marek Vasut wrote: I'd send followup patch that'd clean that mess up altogether ... it's cleaner and much easier to track back in git log. Go for it then. :) Merge the erase patch then. :) Not till I see that followup patch ... ;) You won't till I see this one in git ... ;) Then make your own repository on http://repo.or.cz, push the OpenOCD source there with your patch and get on with it! Man you guys are annoying. :) // Dean Glazeski ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Tue, Dec 15, 2009 at 1:42 AM, Marek Vasut marek.va...@gmail.com wrote: Dne Po 14. prosince 2009 21:55:25 David Brownell napsal(a): On Monday 14 December 2009, Marek Vasut wrote: Dne Po 14. prosince 2009 02:46:26 David Brownell napsal(a): On Sunday 13 December 2009, Marek Vasut wrote: I'd send followup patch that'd clean that mess up altogether ... it's cleaner and much easier to track back in git log. Go for it then. :) Merge the erase patch then. :) Not till I see that followup patch ... ;) You won't till I see this one in git ... ;) David is one of the official maintainers and he is responsible for adhering to the standard that the OpenOCD community has agreed on. He can't just commit something because he wants to. If you feel that David is in error in interpreting the coding rules, reviewing code, making unreasonable requests, etc. that the community has agreed on, then feel free to raise that issue. He's got an excellent track record btw. One of the reasons we ask that contributors do more work is that if we push something to the tree that still needs work, then somebody has to do that work and it would be wrong to ask the maintainers to do so. Maintainers sometime make tiny tweaks(formatting, adjusting release comments, etc.), but really they should leave all changes to contributors. Dean made a suggestion that can work for you, create your own branch of openocd at http://repo.or.cz/w/openocd.git. That way you can commit push your entire series(including patches that are not your own). You may be familiar with git rebase? (I wasn't a month ago, it's great!). It will automatically move your changes to be on top of the latest version of OpenOCD. -- Øyvind Harboe US toll free 1-866-980-3434 / International +47 51 63 25 00 http://www.zylin.com/zy1000.html ARM7 ARM9 ARM11 XScale Cortex JTAG debugger and flash programmer ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
Dne Ne 13. prosince 2009 06:36:47 David Brownell napsal(a):
On Saturday 12 December 2009, Marek Vasut wrote:
Also, can you restructure it so that you don't just add a bit
if (there's a custom erase_page) { ... }? That's the sort
of thing which is easier to just require.
Require? Why? No other driver probably needs it/will find it useful.
To streamline and simplify the primary code paths. It's better to
ptr-op(...)
than to
if (ptr-op)
ptr-op(...);
else
inlined default version ptr-op();
Admittedly this is one of the ways the current source is less
clean than it should be. But that's not a reason to perpetuate
such practices.
- Dave
Either provide the
current logic in a separate routine that gets patched into the
ops vector of any driver that doesn't have it, or ...
We can do so in one go in a separate patch if you want to do this kind of
cleanup. That'd be cleaner solution.
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Sunday 13 December 2009, Marek Vasut wrote: Either provide the current logic in a separate routine that gets patched into the ops vector of any driver that doesn't have it, or ... We can do so in one go in a separate patch if you want to do this kind of cleanup. That'd be cleaner solution. I won't be picky about how many patches it takes, but I'm not keen on merging this new feature *and* worsening that if (ptr-op) ... mess. However, if you were to submit a followup to $PATCH that just addressed that point for this feature, I'd likely combine the two in one commit. :) ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
Dne Ne 13. prosince 2009 21:06:32 David Brownell napsal(a): On Sunday 13 December 2009, Marek Vasut wrote: Either provide the current logic in a separate routine that gets patched into the ops vector of any driver that doesn't have it, or ... We can do so in one go in a separate patch if you want to do this kind of cleanup. That'd be cleaner solution. I won't be picky about how many patches it takes, but I'm not keen on merging this new feature *and* worsening that if (ptr-op) ... mess. Not my loss ... However, if you were to submit a followup to $PATCH that just addressed that point for this feature, I'd likely combine the two in one commit. :) I'd send followup patch that'd clean that mess up altogether ... it's cleaner and much easier to track back in git log. ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Sunday 13 December 2009, Marek Vasut wrote: I'd send followup patch that'd clean that mess up altogether ... it's cleaner and much easier to track back in git log. Go for it then. :) ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
Dne So 12. prosince 2009 00:41:48 Marek Vasut napsal(a): Hi, included is a patch that adds possibility to supply erase_page function in a driver, overriding the default behaviour of core.c Hi, any updates ? Thanks ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Saturday 12 December 2009, Marek Vasut wrote: Dne So 12. prosince 2009 00:41:48 Marek Vasut napsal(a): included is a patch that adds possibility to supply erase_page function in a driver, overriding the default behaviour of core.c Hi, any updates ? Thanks I was wondering why to merge something that doesn't have any users ... :) And was also curious what's wrong with the standard erase_page. Is this something that would be needed to support OneNAND, for example? - Dave ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
Dne So 12. prosince 2009 22:39:43 David Brownell napsal(a): On Saturday 12 December 2009, Marek Vasut wrote: Dne So 12. prosince 2009 00:41:48 Marek Vasut napsal(a): included is a patch that adds possibility to supply erase_page function in a driver, overriding the default behaviour of core.c Hi, any updates ? Thanks I was wondering why to merge something that doesn't have any users ... :) I have pxa3xx nand driver ready that will need this, preparing ground to deploy it. And was also curious what's wrong with the standard erase_page. Is this something that would be needed to support OneNAND, for example? I need the whole page address when writing the command, not supplied in 8bit/16bit values. - Dave ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Saturday 12 December 2009, Marek Vasut wrote:
I was wondering why to merge something that doesn't have any
users ... :)
I have pxa3xx nand driver ready that will need this, preparing ground to
deploy
it.
Mmkay...
And was also curious what's wrong with the standard erase_page.
Is this something that would be needed to support OneNAND, for
example?
I need the whole page address when writing the command, not supplied in
8bit/16bit values.
That information should be in the patch comment.
Also, can you restructure it so that you don't just add a bit
if (there's a custom erase_page) { ... }? That's the sort
of thing which is easier to just require. Either provide the
current logic in a separate routine that gets patched into the
ops vector of any driver that doesn't have it, or (probably
better) just export that routine and update all the NAND drivers
to provide it. (Better because we actually want to see all the
driver structs be static const someday: read-only.)
That'll be easier to review too.
- Dave
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
Dne Ne 13. prosince 2009 03:25:07 David Brownell napsal(a):
On Saturday 12 December 2009, Marek Vasut wrote:
I was wondering why to merge something that doesn't have any
users ... :)
I have pxa3xx nand driver ready that will need this, preparing ground to
deploy it.
Mmkay...
And was also curious what's wrong with the standard erase_page.
Is this something that would be needed to support OneNAND, for
example?
I need the whole page address when writing the command, not supplied in
8bit/16bit values.
That information should be in the patch comment.
Also, can you restructure it so that you don't just add a bit
if (there's a custom erase_page) { ... }? That's the sort
of thing which is easier to just require.
Require? Why? No other driver probably needs it/will find it useful.
Either provide the
current logic in a separate routine that gets patched into the
ops vector of any driver that doesn't have it, or (probably
better) just export that routine and update all the NAND drivers
to provide it. (Better because we actually want to see all the
driver structs be static const someday: read-only.)
That'll be easier to review too.
- Dave
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
Also, can you restructure it so that you don't just add a bit
if (there's a custom erase_page) { ... }? That's the sort
of thing which is easier to just require.
So, does this mean we restructure the entire NAND core to get rid of if
(nand-function) {nand-function()} blocks for just about every read and
write command? Look at nand_read_page and nand_write_page. They have that
sort of logic.
// Dean Glazeski
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
Dne Ne 13. prosince 2009 05:21:36 Dean Glazeski napsal(a):
Also, can you restructure it so that you don't just add a bit
if (there's a custom erase_page) { ... }? That's the sort
of thing which is easier to just require.
So, does this mean we restructure the entire NAND core to get rid of if
(nand-function) {nand-function()} blocks for just about every read and
write command? Look at nand_read_page and nand_write_page. They have that
sort of logic.
Not only those ...
// Dean Glazeski
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Saturday 12 December 2009, Marek Vasut wrote:
Also, can you restructure it so that you don't just add a bit
if (there's a custom erase_page) { ... }? That's the sort
of thing which is easier to just require.
Require? Why? No other driver probably needs it/will find it useful.
To streamline and simplify the primary code paths. It's better to
ptr-op(...)
than to
if (ptr-op)
ptr-op(...);
else
inlined default version ptr-op();
Admittedly this is one of the ways the current source is less
clean than it should be. But that's not a reason to perpetuate
such practices.
- Dave
Either provide the
current logic in a separate routine that gets patched into the
ops vector of any driver that doesn't have it, or ...
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Saturday 12 December 2009, Dean Glazeski wrote:
Also, can you restructure it so that you don't just add a bit
if (there's a custom erase_page) { ... }? That's the sort
of thing which is easier to just require.
So, does this mean we restructure the entire NAND core to get rid of if
(nand-function) {nand-function()} blocks for just about every read and
write command? Look at nand_read_page and nand_write_page. They have that
sort of logic.
Those are the worst offenders. Yes, I think that sort of
cleanup would be a good thing.
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] NAND: add erase_page callback
On Sat, Dec 12, 2009 at 11:36 PM, David Brownell davi...@pacbell.netwrote:
On Saturday 12 December 2009, Marek Vasut wrote:
Also, can you restructure it so that you don't just add a bit
if (there's a custom erase_page) { ... }? That's the sort
of thing which is easier to just require.
Require? Why? No other driver probably needs it/will find it useful.
To streamline and simplify the primary code paths. It's better to
ptr-op(...)
than to
if (ptr-op)
ptr-op(...);
else
inlined default version ptr-op();
Admittedly this is one of the ways the current source is less
clean than it should be. But that's not a reason to perpetuate
such practices.
I can't say I disagree with this position. Maybe as one of my tasks for the
next week, I'll start doing this kind of cleanup in my nand-refactor branch
at http://repo.or.cz/w/openocd/dnglaze.git. I still have some NAND tweaks
up there that aren't in mainline, but something like this might be worth
including in that work. It's sort of a NAND cleanup branch anyway.
// Dean Glazeski
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
