Hello Thomas,
I went ahead and started looking into shutdown functions. Almost of
them use global variables, but I already have ideas on how to rewrite
it. Now I start coding a prototype and want to implement struct
example_data. But I have run into a problem with its initialization:
1) In theory, we can declare a static variable within each programmer's
file. It would be convenient, but this method has a big disadvantage.
We allocate private_data for every programmers but use only one.
static struct example_data {
...
} private_data;
2) It's not possible to add a variable to struct programmer_entry
because the structure is read only (structures in programmer.h are
declared as const).
3) Use a static global variable in flashrom.c. Lesser of two evils
principle as they say
static const struct programmer_entry *programmer = NULL;
static const char *programmer_param = NULL;
static void *programmer_data = NULL;
The next issue is the initialization of programmer_data:
a) Do it inside programmer->init(). The problem here is the duplication
of programmer_data init code in each function.
b) Do it outside programmer->init(). The problem here is that we can't
find out the size of example_data (it can be drkaised_data,
it85spi_data and etc)
programmer_data = calloc(1, sizeof(EXAMPLE_DATA));
if (!data) {
...
}
...
ret = programmer->init(&programmer_data);
Perhaps there is some simpler solution, but I don't notice it.
--
Joursoir
On Sat, 19 Mar 2022 21:49:41 +0000
Thomas Heijligen
src@posteo.de wrote:
> Hi Joursoir,
>
> Beside the internal programmer, most programmer manipulate only a few
> things, so it is not that much. A data struct for each programmer
> should be used to store the context of the programmer instead of
> setting global variables. This gives the programmer a defined lifecycle
> . The shutdown() function should take care of all initialized items
> from the init() function. That can also be a pointer to memory mapped
> I/O, a pci_dev or libusb context.
>
> When we take a simple programmer like the drkaiser pci programmer,
> there is a pci device, a pointer to memory mapped I/O and one register.
> This can easily be handled in a few lines.
>
> * The internal programmer is some kind of special, since it was part of
> the flashrom core before other programmers exits.
> * How to implement certain aspects is then part of the project.
> * We try to reduce the amount of global variables. This project is part
> of it.
>
> -- Thomas
>
> On Sat, 2022-03-19 at 15:59 +0300, Joursoir wrote:
> > Hello Thomas and Anastasia,
> >
> > Thank you for the quick answer. Now I understand the idea of the
> > project more correctly. It really will be great if programmers
> > have their own shutdown functions.
> >
> > In your example you have used pci_write_*(), but the real flashrom
> > code uses only rpci_write_* (it registers an undo-callback). Saving
> > the data to some structure on every pci_write_*() is unnecessary
> > complexity. So, we can continue to use rpci_write_*(), which will do
> > work for us, but in a different way. The main problem is that
> > register_shutdown() often occurs not in example_init(), but in its
> > called funcs. Let's see on the example of internal programmer:
> >
> > internal_init() -> chipset_flash_enable() ->
> > -> chipset_enables[i].doit() -> enable_flash_vt823x()
> >
> > The last one can contain 1 or more rpci_write_*(). I suggest not to
> > register shutdown in rcpi_write_*(), but to fill a linked list with
> > value that should be restored. It will be restored in
> > example_shutdown().
> >
> > Should global variables be used? In the case of an internal
> > programmer, struct example_data will go a long way before data is
> > written to it.
> > _______________________________________________
> > flashrom mailing list -- flashrom@flashrom.org
> > To unsubscribe send an email to flashrom-leave@flashrom.org
>
