On 11/27/09 12:24 AM, Carl-Daniel Hailfinger wrote:
Well, with some careful changes, I think flashrom can get by with maybe 2 kB of RAM or CAR. I know what to change, but I can't make 100% correct RAM requirement estimates. And we'll need boatloads of const keywords. Patches welcome, especially for the const stuff. The biggest writable object is the flashchip array. We can make it const, but then we have to copy each chip to a temporary variable during probe. Not really ideal. Hm.
Why the copy?
Can't we instead have a writable struct flashchip_instance and let that contain a pointer to the current readonly struct flashchip member?
Stefan
On 27.11.2009 00:30, Stefan Reinauer wrote:
On 11/27/09 12:24 AM, Carl-Daniel Hailfinger wrote:
Well, with some careful changes, I think flashrom can get by with maybe 2 kB of RAM or CAR. I know what to change, but I can't make 100% correct RAM requirement estimates. And we'll need boatloads of const keywords. Patches welcome, especially for the const stuff. The biggest writable object is the flashchip array. We can make it const, but then we have to copy each chip to a temporary variable during probe. Not really ideal. Hm.
Why the copy?
Can't we instead have a writable struct flashchip_instance and let that contain a pointer to the current readonly struct flashchip member?
- You'd have to split struct flashchip into const-by-nature and variable-by-nature fields. Can do, but it needs a replacement of struct flashchip to struct flashchip_instance and associated glue code in every function in flashrom. - Handling chips with varying sizes (based on some config byte) is broken by this change. - Since such a change will break every other unapplied patch out there, the only chance this has in getting in is when the patch queue size is near zero. Won't happen anytime soon if the current review rate (more patches sent than reviewed) is any indication.
To summarize, the copy approach is the only thing that has a chance of going in in the next few weeks unless a review wonder happens and we can agree on dropping support for a few variable-size chips.
Plus, we have to convert all of printf(), printf_debug(), fprintf(stderr,) to wrapper functions which can be compiled out or be adapted to serialice. And we need a rewritten main(). Doable, but post 0.9.2 material (unless someone sends easy-to-review patches).
I don't want to discourage you, but it won't be a walk in the park.
Regards, Carl-Daniel
On 11/27/09 12:54 AM, Carl-Daniel Hailfinger wrote:
- Handling chips with varying sizes (based on some config byte) is
broken by this change.
Which chips are that?
- Since such a change will break every other unapplied patch out there,
the only chance this has in getting in is when the patch queue size is near zero. Won't happen anytime soon if the current review rate (more patches sent than reviewed) is any indication.
Makes sense.
Plus, we have to convert all of printf(), printf_debug(), fprintf(stderr,) to wrapper functions which can be compiled out or be adapted to serialice.
that should happen anyways..
And we need a rewritten main(). Doable, but post 0.9.2 material (unless someone sends easy-to-review patches).
I'd refrain from making main() part of this.. All the other stuff is libpayload material... so main() can be done by whoever uses libpayload then...
I don't want to discourage you, but it won't be a walk in the park.
Ok, let's delay this until after 0.9.2
Stefan
On 27.11.2009 01:05, Stefan Reinauer wrote:
On 11/27/09 12:54 AM, Carl-Daniel Hailfinger wrote:
- Handling chips with varying sizes (based on some config byte) is
broken by this change.
Which chips are that?
They have a comment in flashchips.c: /* Size can only be determined from status register */
Plus, we have to convert all of printf(), printf_debug(), fprintf(stderr,) to wrapper functions which can be compiled out or be adapted to serialice.
that should happen anyways..
Agreed. Patches welcome.
And we need a rewritten main(). Doable, but post 0.9.2 material (unless someone sends easy-to-review patches).
I'd refrain from making main() part of this.. All the other stuff is libpayload material... so main() can be done by whoever uses libpayload then...
Yes. With libpayload, we can use whatever main() we want. But the libpayload conversion has to happen.
I don't want to discourage you, but it won't be a walk in the park.
Ok, let's delay this until after 0.9.2
The printf stuff is welcome even before 0.9.2.
The real problem right is the lack of reviewers (and I don't believe in self-ack nor in commit-for-another-developer-who-has-commit-access).
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Stefan Reinauer