Skip to content

Add freebsd_spi programmer #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

valpackett
Copy link

@valpackett valpackett commented Jul 25, 2018

Based on linux_spi, using FreeBSD's spigen(4) interface

@emaste
Copy link

emaste commented Nov 26, 2018

Is there anything holding up this change?

Copy link
Contributor

@dhendrix dhendrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks pretty good overall. My only issue is the use of alloca(), and that's easy to fix.

I also went ahead and posted this on the upstream review server: https://review.coreboot.org/c/flashrom/+/30764.


/* FreeBSD uses a single buffer for rx and tx. Allocate a temporary one to avoid overwriting anything. */
size_t tmpcnt = readcnt + writecnt;
unsigned char *tmpbuf = alloca(tmpcnt);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use malloc() instead (see the BUGS section of https://www.freebsd.org/cgi/man.cgi?alloca).

@felixsinger
Copy link
Contributor

@unrelentingtech Hi there. Thank you for your patch! Very cool.

Could you rebase this and push it to review.coreboot.org? That would be great!

@valpackett
Copy link
Author

No, sorry, I'm not interested anymore.

@valpackett valpackett closed this Aug 9, 2022
@flashrom flashrom locked and limited conversation to collaborators Aug 9, 2022
@flashrom flashrom unlocked this conversation Aug 9, 2022
@felixsinger felixsinger reopened this Aug 9, 2022
@n-huber
Copy link
Contributor

n-huber commented Aug 9, 2022

Hi Greg, sorry that this was lost in 2019. Not sure if you received emails from our Gerrit instance. There was some concern about your Signed-off-by. Is "Greg V" your full name? If not, I think either " " or "Greg V" with a complete email address would suffice. This would help us to get the code merged without taking any risk.
Also, if possible GPLv2+, i.e. "version 2 or later", would be preferred. If you take the time to push an updated commit, that would be very nice. We could take it from there and update and merge the code.

@valpackett
Copy link
Author

valpackett commented Aug 9, 2022

  • I'm in the process of deprecating that name and I do not publicly have a "real" or "full" sounding name right now;
  • I hate harmful "real name" policies, anything resembling those makes me disinterested very quickly;
  • As I said, I'm not interested in this actual work anymore, I don't currently have the means to update and test the patch. I closed this for a reason;
  • Anyone is free to take the patch, assign copyright to themselves, relicense to whatever license, rebase/update, etc. Basically all my contributions like that are intended to be available under maximally permissive terms like 0BSD.

Based on linux_spi, using FreeBSD's spigen(4) interface.

Change-Id: I4e1689416fbb309df94807f51635bc1f4b53e0c8
@n-huber
Copy link
Contributor

n-huber commented Aug 9, 2022

Thanks! your written permission here is good enough for me. And I'm very sorry about one point of confusion: I didn't realize that technology is a TLD by now ._.

@emaste
Copy link

emaste commented Aug 9, 2022

@n-huber will you make the malloc change and merge then?

@felixsinger
Copy link
Contributor

felixsinger commented Aug 9, 2022

The patch needs to be rebased first. In the meantime things might have changed a little.

And also, we had a discussion about it in IRC. We are thinking if we could add an abstraction layer to minimize the code for freebsd_spi, linux_spi and maybe others.

But in general, we want to have this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants