Remove wrong patch on atmega4809

[peckpeck]

It seems that the patch has been included to atmega4809 in bulk with other mcu.

The mega0 patch removes 4 pins in PORTC, however the atmega4809 has 8 pins in PORTC that are all documeted in the datasheet, that work, and that are correctly described in vendor/atmega4809.atdf . Moreover, other parts of the patch do not change the atdf.

So it is reasonable to just drop the patch (and it works for me :)

See https://ww1.microchip.com/downloads/en/DeviceDoc/ATmega4808-4809-Data-Sheet-DS40002173A.pdf for datasheet

[Rahix]

Hi, thanks for catching this! Can you check if ATmega4808 is also affected? It seems it also includes the same patch...

It was added in this commit, as part of ATmega3208/ATmega3289 support: a23bdab

Cc: @zacharytomlinson (via #164)

[zacharytomlinson]

I can review the common ports between the two chips but to my memory they are almost identical in layout with the 4809 just being a logical extension of the 4808. The 4809 was not fully fleshed out but common blocks were identified between the two and shared. It could be that certain pins were omitted or re-mapping was not faithfully adhered to, when I am back at a computer I can say this with more certainty.

[peckpeck]

The documentation says that the 4808 has less pins. It matches what is is in the mega0 patch, so it should not be a problem, but the patch is not needed either because the atdf file describes it the same way as the documentation.

I have no 4808 to test, but i'm sure it can be removed too.

[zacharytomlinson]

I have to admit I am a bit confused. The old build succeeded despite not containing an exhaustive implementation for each chip, so I am not sure what is meant by 'incorrect patch' or why deleting them would fix anything. Despite this I did go ahead and completely flesh out the rest of the chip for all of the mega0 that were originally contributed to, the 3208/3209 and 4808/4809. All 4 still build and I flashed a test program to the 4808 and it succeeded. Two things from that, @Rahix this has 1 file deletion and 7 file creates/mods (a common for all mega0 chips, a common for both 32 pin and 48 pin packages, and device specific patches for each chip). I wouldn't necessarily break these up into separate commits as they contain common files between all 4 chips, but if you would like I can break those up. I am waiting to commit until I hear how you want to proceed. For clarity and brevity, again the patch was not wrong. They all built but were not exhaustive definitions in situ for all 4 chips. As such, all I did was complete their patches according to the atdf files obtained from atmel. All four should now be exhaustive implementations as well as build with zero problems. All of this to say I could still be confused as to what the issue is, if that is the case please enlighten me further!

[zacharytomlinson]

As a further sanity check, I grabbed an old arduino nano every (atmega4809) and made a local copy of your example project for the 328p @Rahix . It exercises all of the GPIO to ensure our access and the naming is correct. This flashed and worked no problem. I think the patch wasn't incorrect, but maybe you were using a PORT access that hadn't been defined yet? If so the commit I have should fix that issue for you.

[peckpeck]

The old build succeeds because the patch just ignores some pins, which just prevents the direct usage of some pins with their name (but not with port.set_bit()). As long as you don't use them somewhere, you're fine.

[zacharytomlinson]

Okay then the issue isn't an incorrect patch, it was incomplete; of which you are absolutely correct, fortunately these changes should resolve that. @Rahix as opposed to removing the prior committed definition, I would suggest we review and include the updates I made which to my knowledge now represent an exhaustive implementation for both 32 and 48 pin variants. I only suggest this because I know at least two other people using that atmega4809 definition and that would represent a breaking change for them. It seems a subsequent update to complete the chip definition would be more appropriate.

[peckpeck]

Which changes are you talking about ?

[zacharytomlinson]

The ones referenced here:

Despite this I did go ahead and completely flesh out the rest of the chip for all of the mega0 that were originally contributed to, the 3208/3209 and 4808/4809. All 4 still build and I flashed a test program to the 4808 and it succeeded. Two things from that, @Rahix this has 1 file deletion and 7 file creates/mods (a common for all mega0 chips, a common for both 32 pin and 48 pin packages, and device specific patches for each chip). I wouldn't necessarily break these up into separate commits as they contain common files between all 4 chips, but if you would like I can break those up. I am waiting to commit until I hear how you want to proceed.

I could submit those now but I don't want the author to have to break those commits up if they want it done that way, so I was waiting to hear whether they wanted to proceed as is, or break up by board or pin count etc...

[zacharytomlinson]

I went ahead and pushed these to my fork so you can clone and test if you want. It is here: https://github.com/zacharytomlinson/avr-device/tree/fix/mega0-chip-definitions

[peckpeck]

it works for me

[peckpeck]

Closing in favor of #250