On Sun, 26 Jun 2022 at 10:10, Nico Huber <nico.h@gmx.de> wrote:
Hi Anastasia,

Probably not suitable to laser Anastasia in particular. Peter owns the i2c set of drivers.
 

I've seen you put some questions about the programmer tickets[1-4] on
the meeting agenda. I hope I can provide some answers already. Looking
at the tickets, I see there are already lists of things to do. But you
ask what is left to do. So somewhere seems to be a misunderstanding.

Maybe a good idea to clarify within the ticket the specific in order to keep the owner of the ticket engaged with the issue and not have the dialogue fragmented across tracker and mailing list. The issues here do not seem to be fundamentally design discussion, rather just nits and clarifications.
 

* raiden_debug_spi: Seems ok. I think this one was really only missing
  a manpage entry. Now that it has some context, I guess an independent
  developer could actually review it now. ^^

* lspcon_i2c_spi & realtek_mst_i2c_spi: There's a misunderstanding I
  guess. The tickets say "Document ... programmer". But they list actual
  problems. My idea when I wrote the long lists of issues was not to
  document issues. But, hopefully, to get them addressed, in code.

Comment on the bug to re-focus it to what you want? I believe Anastasia opened these bugs to capture your initial thoughts in the new issue tracker and not completely understand the full scope from the get go from a bullet point list.
 

  We could also document the issues, e.g. by cluttering the code with
  FIXME comments. This might at least slow the spreading of such code
  down. But I would have to ask, why have these drivers on the master
  branch? In one of the tickets it even says the driver is not used
  in production. Does that mean it's abandoned?

No they are not abandoned, this has been answered a number of times now so I am unclear why the looping of this idea keeps happening? Drivers have an owner, Peter, these are used for debug and are of interest to the wider community for people who want to access this hardware.

I think the OWNERS file discussed could help remind who to contact and make the bug owner for a specific issue. 
 

  Like I mentioned elsewhere, many of the issues could be mitigated by
  going through the board_enabled infrastructure. This was also men-
  tioned in the review of the mediatek_i2c_spi driver. All these I2C
  drivers could share a similar design, AFAICT.

I think this was already answered before. board_enable was designed around PCIe and similar, right now in its current API form it is insufficiently extensible to trivially allow for i2c<->board pairing. Agree this definitely needs core flashrom development work however does not seem like a driver author burden but rather a collective make flashrom have more extensible and robust core API missing.

We should open some bugs here to try and drive towards a better board_enable future certainly!


  One of the manpage entries also revealed a new problem: I think it
  implies that after running `flashrom -p realtek_mst_i2c_spi:bus=0 -E`
  the device would be broken after a reset. That's unacceptable for
  flashrom. Another sign that it should go through the infrastructure
  of the internal programmer.

Seems like a bug for the bug tracker and assign the driver owner to take care of it?
 

  Documentation for the hardware interface would also be very welcome.
  Basically, everything that one needs to understand the code (and can't
  be derived from flashrom sources) should be publicly documented.
  I think we should make this a rule for vendor additions.

This was discussed at length. Once again, documentation may not even exist at all in the first place. This fundamentally assumes the vendor provides such details at all. Agree, we should document better however one can only document what one actually knows for sure. Free software support for hardware has a long history of this problem, it is unreasonable to point the finger to have this paradise world where silicon vendors are so forthcoming, in practice we live in a very mixed world where the developer works things out and does their best to document what they know at the time.

Although, a bug to document how the driver seems to operate in a documentation/ directory has been discussed so I think we could get that moving.
 

* mediatek_i2c_spi: What I have to say about this, I did on Gerrit.
  What happened is clearly against our development guidelines. It's
  a copy of another driver, but we don't have the time to look into
  it so let the community take care of it? WTF?

Seldom have I seen someone developing hardware support for the first time not copy something similar and use that as a starting point. What precisely is against development guidelines? If you can be more concrete and not say "WTF" it could be more helpful.


Nico

[1] https://ticket.coreboot.org/issues/358
[2] https://ticket.coreboot.org/issues/360
[3] https://ticket.coreboot.org/issues/361
[4] https://ticket.coreboot.org/issues/376
_______________________________________________
flashrom mailing list -- flashrom@flashrom.org
To unsubscribe send an email to flashrom-leave@flashrom.org