Skip to content
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

Revert "Ensure page_size is always at least 1" #984

Closed
wants to merge 1 commit into from

Conversation

umbynos
Copy link
Contributor

@umbynos umbynos commented May 31, 2022

This reverts commit da0e437.

Reason for this revert ElTangas/jtag2updi#16 (comment)
We use the jtag2udpi fw on the SAMD11 to be able to program the ATMega4809. That commit was breaking the upload with AVRdude

Related conversation with @MCUdude umbynos@9ae854d#commitcomment-73386063

Co-authored-by: Martino Facchin [email protected]

This reverts commit da0e437.

Reason for this revert ElTangas/jtag2updi#16 (comment)
We use the jtag2udpi fw on the SAMD11 to be able to program the ATMega4809. That commit was breaking the upload with AVRdude


Co-authored-by: Martino Facchin <[email protected]>
@MCUdude
Copy link
Collaborator

MCUdude commented Jun 15, 2022

By looking at this PR, it's not obvious to me why the config_gram.y has to be modified.
With this PR it just blindly accepts whatever $3->value.number holds, instead of checking for <= 0 and printing a warning.

As for the removed line in jtagmkii.c I don't know if this will have any consequences for other hardware that uses this protocol. @dl8dtl do you have any thoughts on this one? AFAIK, page_size should be defined for all memory types for all targets anyways.

@dl8dtl
Copy link
Contributor

dl8dtl commented Jun 15, 2022

I'm against reverting that change – after all, it has been made for a reason (fixing a bug or crash).
There must be a better solution, but I didn't look into the details so far.

@mcuee mcuee added the invalid This doesn't seem right label Jun 20, 2022
@mcuee
Copy link
Collaborator

mcuee commented Jun 20, 2022

In this case, we can close this one.

@mcuee mcuee closed this Jun 20, 2022
@umbynos
Copy link
Contributor Author

umbynos commented Jun 20, 2022

Unfortunately, I don't know the reasons behind da0e437. I understand this is not a fix but only a quick way to solve ElTangas/jtag2updi#16 (comment). For sure, there is a better way to fix this, but I don't know sufficiently about avrdude to do it.

@mcuee
Copy link
Collaborator

mcuee commented Jun 20, 2022

@umbynos Can you ask the affected user to try out latest avrdude 7.0 release or even git master to see if the issue has been fixed? I think there are a few improbements done on this area.

@MCUdude
Copy link
Collaborator

MCUdude commented Jun 20, 2022

I can try this with my Nano Every later today!

@mcuee
Copy link
Collaborator

mcuee commented Aug 1, 2022

Re-open this topic, just want to make sure if the has already been fixed in avrdude 7.0 release and git.

@mcuee mcuee reopened this Aug 1, 2022
@mcuee
Copy link
Collaborator

mcuee commented Aug 1, 2022

I do not have issues using avrdude-7.0 and git with Arduino Nano Every (with SAM D11 as the JTAG2UPDI controller). Another user also reported the susccess story in another issue.
Ref:

@mcuee
Copy link
Collaborator

mcuee commented Aug 1, 2022

@umbynos

In this case, I think this issue can really be closed. Any objections?

@stefanrueger
Copy link
Collaborator

Looks like AVRDUDE 7.0 does not expose the issue that this PR tries to solve, so not reverting.

@umbynos umbynos deleted the revert_page_size branch August 3, 2022 07:45
umbynos added a commit to arduino/avrdude-packing that referenced this pull request Nov 2, 2022
umbynos added a commit to arduino/avrdude-packing that referenced this pull request Nov 2, 2022
umbynos added a commit to arduino/avrdude-packing that referenced this pull request Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants