Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Edward O'Callaghan, Anastasia Klimchuk.
1 comment:
Patchset:
Sorry that was Patchset 1, where I got asked to switch it to the physmap area... is there some way to get this implemented? I get told physmap isn't the right place and also flashrom.c is not the right place and also it is... I'm confused 😕
Well, you shouldn't rush things. IMHO, PS1 was closer to a correct solution
(closer but not close, let's give PS1 20%, and PS2 15%). You should start by
reading all the code involved. It's not an easy task. .map_flash_region
seems to be abused as a general hook into programmer code during flash
probing. I guess this would need to be fixed first.
If this was an easy task, we'd have it done years ago. Alas, things have
moved backwards during the last two years. Hacks like the one in sb600spi
were added without proper review. Now they block any progress. I can see
at least three things that are wrong with this call (calling something
that is not supposed to be an API, mixing that with another API, no return
value checked, overlapping calls of map_flash()/unmap_flash(), no support
for >16MiB; oh that's 5 already?). I think this needs to be addressed
before we should even start thinking about changing anything map_flash()
related. Btw. I have a feeling that the code would work without the
call ;) Still there is the problem that the code uses the memory mapping
for no documented reason. So I'd start by requesting public documentation
from AMD.
I know you just wanted to fix a seemingly small issue. So I can fully
understand if you don't want to dig deeper into it. But that's just
how things are now for flashrom: somebody needs to clean up after your
colleagues. The project is pretty much congested by now. To have another
release from the master branch somebody needs to look into all the
changes since the last release again, if there are more hacks like this
lurking.
To view, visit change 56721. To unsubscribe, or for help writing mail filters, visit settings.