[flashrom] Re: [GSoC 2022] Restructure Shutdown Function

[Thomas Heijligen]

Hi Joursoir,

The idea for the "Restructure Shutdown Functions" project is to extend
the struct programmer_entry with a function to handle programmer
related shutdowns. Currently, the init function of each programmer
calls, mostly indirect, register_shutdown on each item  separately to
handle its shutdown later. In this project, the state tracking and
shutdown of the programmer own data should be moved to each programmer.

E.g.:
const struct programmer_entry example_programmer = {
    ...
    .init = example_init,
    .shutdown = example_shutdown,
};

struct example_data {
    int reg_a;
};

static int example_init(struct example_data* data)
{
    ...
    // keep track of the original value
    data->reg_a = pci_read_word(dev, PCI_MAGIC_ADDR, PCI_MAGIC_VALUE);
    // write new value
    pci_write_word(dev, PCI_MAGIC_ADDR, 0x42);
}

static int example_shutdown(struct example_data* data)
{
    // restore the original value
    pci_write_word(dev, PCI_MAGIC_ADDR, data->reg_a);
}

Feel free to contact me if you have further questions.

-- Thomas

On Fri, 2022-03-18 at 17:46 +0300, Joursoir wrote:
> Good day,
>
> I am a student and very interested in flashrom. I fit the description
> of experienced C programmer. Open-source and low-level programming
> are
> my interests. I already have real experience with flashrom (CH341A
> programmer + chip "MX25L6436E") and want to become a GSoC 2022
> contributor in this project.
>
> I find the "Restructure Shutdown Function" project interestring to
> me.
> But right now it has no mentors. Maybe someone from the mailing can
> help me.
>
> I have gone through sources and understand how things are now. The
> project description says "have a defined shutdown function in the
> programmer API". But we already have it. It's programmer_shutdown().
> Or no?
>
> The first and easiest thing that comes to my mind is to refuse using
> callbacks and use enums, thusly:
>
> enum shutdown_func {
>     SERPROG = 1 << 0,
>     SPI = 1 << 1,
>     ...
> };
>
> int programmer_shutdown(enum shutdown_func execfunc)
> {
>     ...
>
> if(execfunc & SERPROG)
>         ret |= serprog_shutdown(...);
>
> if(execfunc & SPI) {
>         /* the problem is that SPI programmers have own shutdown
> funcs,
>            like dlc5_shutdown, byteblaster_shutdown,
> stlinkv3_spi_shutdown,
>            so use struct spi_master, that contains shutdown callback
> */
>         ret |= spi_master_shutdown(...);
>     }
>
> ...
> }
>
> This system has disadvantages, but it'll make easier to track the
> code
> flow. What are the potential problems in this implementation?
>
> What's with struct flashrom_programmer? Is not implemented yet?
>
> I would appreciate any feedback.