What todo with the "document ... programmer" tickets
Hi Anastasia, 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. * 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. 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? 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. 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. 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. * 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? 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
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
On 27.06.22 04:18, Edward O'Callaghan wrote:
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.
What's 'laser'? Peter didn't ask, Anastasia did.
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.
Sounds good, please do.
* 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.
Well, if you don't find the time necessary to look into it deep enough to understand the requirements of flashrom, maybe you can find another developer at Google who can? Maybe somebody with more experience who gets their quicker. Or just get somebody else paid?
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 don't care what role people play during their dayjob. It's not my job to keep track of these things. Also what exactly does it include being an owner? I don't know that. Does it include keeping the code tidy? If so, what's the role of the person who tells owners how to do their job and who is that for Peter? There's a reason why I don't like this "that's the responsible person" pointing. In my experience with coreboot, it turned out that they are never paid to do it anyway, because they are already busy with some- thing else. If you want them to do something after the code is merged, you need to pay them to work proactively on it and make it a top prio- rity. Otherwise, the community might pick up the work, or hate you be- cause they are already too upset.
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!
Why we? You bypassed the process (development guidelines clearly say to discuss things first to avoid frustration). So not we but you should. And we shouldn't have to wait for you (which we already did for years by now). In my experience with you, Edward, you procrastinate no matter what. It's really years already! Please wake up.
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?
Well, if you don't want to fix it now, please go ahead.
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.
No, you talked at length.
Once again, documentation may not even exist at all in the first place.
But you can write it.
This fundamentally assumes the vendor provides such details at all.
I guess you mean the silicon vendor, in that case: Not at all. Details can be figured out. Reverse engineering, testing how the hardware behaves, ...
Agree, we should document better however one can only document what one actually knows for sure.
So you are saying the authors of your drivers knew nothing at all?
Free software support for hardware has a long history of this problem,
Why does it only bother this project now then? I somewhat hate saying it, but it seems you need to hear it: We had much less of such problems before Google came.
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.
So where is the documentation? Or are you saying the best they can do is nothing? I'm confused. Also, why blame silicon vendors for everything? They don't want your code in the flashrom repo, you want it. I feel like there's a deep misunderstanding. Maybe it's this: When I say "vendor", I mean the company that designs products, puts their name and brands on it, produces the software for them and makes money with them, e.g. Google. Maybe I just used the wrong word.
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.
Not similar, it's for the same hardware / IP block IIRC. Did you forget already?
What precisely is against development guidelines? If you can be more concrete and not say "WTF" it could be more helpful.
This is the part that I had in mind: We try to reuse as much code as possible and create new files only if absolutely needed, so if you find a function somewhere in the tree which already does what you want (even if it is for a totally different chip), please use it. See also Command set secrets. The part about discussing first applies too of course. I don't think there is much in our coding guidelines that doesn't disagree. 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
Relax. Update the corresponding bugs with what you see as an actual issue so it can be fixed. :) Cheers Nico, Edward. On Tue, 28 Jun 2022 at 04:19, Nico Huber <nico.h@gmx.de> wrote:
On 27.06.22 04:18, Edward O'Callaghan wrote:
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.
What's 'laser'? Peter didn't ask, Anastasia did.
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.
Sounds good, please do.
* 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.
Well, if you don't find the time necessary to look into it deep enough to understand the requirements of flashrom, maybe you can find another developer at Google who can? Maybe somebody with more experience who gets their quicker. Or just get somebody else paid?
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 don't care what role people play during their dayjob. It's not my job to keep track of these things. Also what exactly does it include being an owner? I don't know that. Does it include keeping the code tidy? If so, what's the role of the person who tells owners how to do their job and who is that for Peter?
There's a reason why I don't like this "that's the responsible person" pointing. In my experience with coreboot, it turned out that they are never paid to do it anyway, because they are already busy with some- thing else. If you want them to do something after the code is merged, you need to pay them to work proactively on it and make it a top prio- rity. Otherwise, the community might pick up the work, or hate you be- cause they are already too upset.
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!
Why we? You bypassed the process (development guidelines clearly say to discuss things first to avoid frustration). So not we but you should. And we shouldn't have to wait for you (which we already did for years by now). In my experience with you, Edward, you procrastinate no matter what. It's really years already! Please wake up.
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?
Well, if you don't want to fix it now, please go ahead.
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.
No, you talked at length.
Once again, documentation may not even exist at all in the first place.
But you can write it.
This fundamentally assumes the vendor provides such details at all.
I guess you mean the silicon vendor, in that case: Not at all. Details can be figured out. Reverse engineering, testing how the hardware behaves, ...
Agree, we should document better however one can only document what one actually knows for sure.
So you are saying the authors of your drivers knew nothing at all?
Free software support for hardware has a long history of this problem,
Why does it only bother this project now then? I somewhat hate saying it, but it seems you need to hear it: We had much less of such problems before Google came.
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.
So where is the documentation? Or are you saying the best they can do is nothing? I'm confused.
Also, why blame silicon vendors for everything? They don't want your code in the flashrom repo, you want it. I feel like there's a deep misunderstanding. Maybe it's this: When I say "vendor", I mean the company that designs products, puts their name and brands on it, produces the software for them and makes money with them, e.g. Google. Maybe I just used the wrong word.
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.
Not similar, it's for the same hardware / IP block IIRC. Did you forget already?
What precisely is against development guidelines? If you can be more concrete and not say "WTF" it could be more helpful.
This is the part that I had in mind:
We try to reuse as much code as possible and create new files only if absolutely needed, so if you find a function somewhere in the tree which already does what you want (even if it is for a totally different chip), please use it. See also Command set secrets.
The part about discussing first applies too of course. I don't think there is much in our coding guidelines that doesn't disagree.
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
Hi Anastasia, On 26.06.22 02:10, Nico Huber wrote:
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? so, I was asked out of band in a weird way to clarify this. That's sarcasm and meant to express my feelings when people merge code with- out acknowledging that it affects the whole flashrom developer commu- nity. I put it there to convey my feelings and not to blame anyone for what happened. That said, I felt a bit ashamed when it was pointed out to me that I did not set it off or mark it as sarcasm in any way. I probably was too tired when writing. Sorry about that.
Nico
Nico: Let me put this very clear and in-band: You are behaving out of line and unprofessional. Not once. Not twice. This has been happening over a long period of time, and it ends now. I have removed your project privileges. There is an opportunity to keep this action limited to one month, if you decide to come to the table and discuss openly with an honest interest in resolving the issues at hand and without backhandedly excusing your behavior with "sarcasm", "expressing feelings" or "tiredness". This action will give you the opportunity to take a step back, relax, and think about paths forward without being burdened by the leadership and involvement in the flashrom (and coreboot) project. Whenever you are ready to have this conversation, please reach out directly to me. Regards, Stefan On Tue, Jun 28, 2022 at 4:48 PM Nico Huber <nico.h@gmx.de> wrote:
Hi Anastasia,
On 26.06.22 02:10, Nico Huber wrote:
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? so, I was asked out of band in a weird way to clarify this. That's sarcasm and meant to express my feelings when people merge code with- out acknowledging that it affects the whole flashrom developer commu- nity. I put it there to convey my feelings and not to blame anyone for what happened. That said, I felt a bit ashamed when it was pointed out to me that I did not set it off or mark it as sarcasm in any way. I probably was too tired when writing. Sorry about that.
Nico _______________________________________________ flashrom mailing list -- flashrom@flashrom.org To unsubscribe send an email to flashrom-leave@flashrom.org
participants (3)
-
Edward O'Callaghan -
Nico Huber -
Stefan Reinauer