Add AT90USB support for serial.c

[drashna]

Description

Adds support for the AT90USB chips in the serial driver for AVR.

Types of Changes

• Core
• Enhancement/optimization

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[mtei]

I reexamined the datasheets.

It seems that serial.c cannot support ATmega32A because the control registers are different.

datasheet summary

• ATmega16U4/ATmega32U4

  • http://ww1.microchip.com/downloads/en/devicedoc/atmel-7766-8-bit-avr-atmega16u4-32u4_datasheet.pdf

  Reg name	bit 7	bit 6	bit 5	bit 4	bit 3	bit 2	bit 1	bit 0
  EIMSK	-	INT6	-	-	INT3	INT2	INT1	INT0
  EICRA	ISC31	ISC30	ISC21	ISC20	ISC11	ISC10	ISC01	ISC00
  EICRB	-	-	ISC61	ISC60	-	-	-	-

  ISCn1	ISCn0	Description
  0	0	The low level of INTn generates an interrupt request.
  0	1	Any edge of INTn generates asynchronously an interrupt request.
  1	0	The falling edge of INTn generates asynchronously an interrupt request.
  1	1	The rising edge of INTn generates asynchronously an interrupt request.

  pin	interrupt	EICRx
  PD0	INT0	EICRA
  PD1	INT1	EICRA
  PD2	INT2	EICRA
  PD3	INT3	EICRA
  -	-
  PE6	INT6 (only level interrupt)	EICRB

• ATmega16U2/ATmega32U2

  • http://ww1.microchip.com/downloads/en/DeviceDoc/doc7799.pdf

  Reg name	bit 7	bit 6	bit 5	bit 4	bit 3	bit 2	bit 1	bit 0
  EIMSK	INT7	INT6	INT5	INT4	INT3	INT2	INT1	INT0
  EICRA	ISC31	ISC30	ISC21	ISC20	ISC11	ISC10	ISC01	ISC00
  EICRB	ISC71	ISC70	ISC61	ISC60	ISC51	ISC50	ISC41	ISC40

  pin	interrupt	EICRx
  PD0	INT0	EICRA
  PD1	INT1	EICRA
  PD2	INT2	EICRA
  PD3	INT3	EICRA
  -	-
  PD4	INT5	EICRB
  PD6	INT6	EICRB
  PD7	INT7	EICRB
  -	-
  PC7	INT4	EICRB

• AT90USB646/AT90USB647/AT90USB1286/AT90USB1287

  • https://www.mouser.jp/datasheet/2/268/doc7593-1369068.pdf

  Reg name	bit 7	bit 6	bit 5	bit 4	bit 3	bit 2	bit 1	bit 0
  EIMSK	INT7	INT6	INT5	INT4	INT3	INT2	INT1	INT0
  EICRA	ISC31	ISC30	ISC21	ISC20	ISC11	ISC10	ISC01	ISC00
  EICRB	ISC71	ISC70	ISC61	ISC60	ISC51	ISC50	ISC41	ISC40

  pin	interrupt	EICRx
  PD0	INT0	EICRA
  PD1	INT1	EICRA
  PD2	INT2	EICRA
  PD3	INT3	EICRA
  -	-
  PE4	INT4	EICRB
  PE5	INT5	EICRB
  PE6	INT6	EICRB
  PE7	INT7	EICRB

• ATmega32A

  • http://ww1.microchip.com/downloads/en/DeviceDoc/ATmega32A-DataSheet-Complete-DS40002072A.pdf

  The control register configuration is different. Requires different additional code to support in serial.c.

  Reg name	bit 7	bit 6	bit 5	bit 4	bit 3	bit 2	bit 1	bit 0
  GICR	INT1	INT0	INT2	-	-	-	IVSEL	IVCE
  MCUCR	SE	SM2	SM1	SM0	ISC11	ISC10	ISC01	ISC00

  pin	interrupt
  PD2	INT0
  PD3	INT1
  PB2	INT2 (edge only)

[mtei]

As a reference, I tryed make a patch.

https://github.com/mtei/qmk_firmware/blob/PR_10706_suggestion_patch/drivers/avr/serial.c

mtei@e188230 in https://github.com/mtei/qmk_firmware/commits/PR_10706_suggestion_patch

[drashna]

Thanks for the clarification. I'm a bit lost at reading the datasheets, so the review and feedback is very appreciated!

I'm also wondering if it's worth abstracting the interrupt vectors for the pins, as there are other places that they would be helpful at.

[mtei]

The following parts also need to be fixed.

@@ -258,13 +275,11 @@ void soft_serial_target_init(SSTD_t *sstd_table, int sstd_table_size) {
     Transaction_table_size = (uint8_t)sstd_table_size;
     serial_input_with_pullup();
 
-    // Enable INT0-INT3,INT6
+    // Enable INT0-INT7
     EIMSK |= EIMSK_BIT;
-#    if SOFT_SERIAL_PIN == E6
-    // Trigger on falling edge of INT6
+#    if EIMSK_BIT >= _BV(INT4)
     EICRB &= EICRx_BIT;
 #    else
-    // Trigger on falling edge of INT0-INT3
     EICRA &= EICRx_BIT;
 #    endif
 }

[drashna]

I was actually wondering about that, but hadn't had a chance to look into it yet!

[mtei]

As you can see from the table above, the interrupt conditions for INT0, INT1, INT2, and INT3 are set in EICRA, and the interrupt conditions for INT4, INT5, INT6, and INT7 are set in EICRB.

[drashna]

Yup, I did see that, and verified that it was the case, just to make sure.

Though, it may be worth having it configurable/defined if not all AVR chips support that schema. But ... for now, I think this should be fine.

[mtei]

I did a build test on 1f1f24f and 2d468c4.
I did each build of PD0, PD1, PD2, PD3, PE6 with a keyboard using ATmega32u4 and confirmed that the binaries match in the two versions.

[mtei]

As a reference, I tryed make a patch again.

https://github.com/mtei/qmk_firmware/blob/PR_10706_suggestion_patch2/drivers/avr/serial.c

mtei@10fe308 in https://github.com/mtei/qmk_firmware/commits/PR_10706_suggestion_patch2

It has been confirmed that the build binary does not change.

[drashna]

Finally got my keyboard assembled, and can verify that this works on teensy++ 2.0s (at90usb1286).

[drashna]

@mtei for the same of ... simplicity, any issues with just doing this?
4426ab7 (#10706)

[mtei]

@mtei for the same of ... simplicity, any issues with just doing this?
4426ab7 (#10706)

I have confirmed that the build results for ATmega32u4 have not changed. That change is fine.

[mtei]

However, for me, I like the following compactness. :-)
Of course the decision is yours.

--- a/drivers/avr/serial.c
+++ b/drivers/avr/serial.c
@@ -29,52 +29,43 @@
 #            error Using I2C, so can not use PD0, PD1
 #        endif
 #    endif
+
+#    define _GET_PARAM_1(a, b, c, d) a
+#    define GET_PARAM_1(param4) _GET_PARAM_1(param4)
+#    define _GET_PARAM_2(a, b, c, d) b
+#    define GET_PARAM_2(param4) _GET_PARAM_2(param4)
+#    define _GET_PARAM_3(a, b, c, d) c
+#    define GET_PARAM_3(param4) _GET_PARAM_3(param4)
+#    define _GET_PARAM_4(a, b, c, d) d
+#    define GET_PARAM_4(param4) _GET_PARAM_4(param4)
+
+#    define SERIAL_PIN_INTERRUPT GET_PARAM_1(REG_INFOS)
+#    define EIMSK_BIT            GET_PARAM_2(REG_INFOS)
+#    define EICRx_BIT            GET_PARAM_3(REG_INFOS)
+#    define EICRx                GET_PARAM_4(REG_INFOS)
+
 // PD0..PD3, common config
 #    if SOFT_SERIAL_PIN == D0
-#        define EIMSK_BIT _BV(INT0)
-#        define EICRx_BIT (~(_BV(ISC00) | _BV(ISC01)))
-#        define SERIAL_PIN_INTERRUPT INT0_vect
-#        define EICRx EICRA
+#        define REG_INFOS INT0_vect, _BV(INT0), (~(_BV(ISC00) | _BV(ISC01))), EICRA
 #    elif SOFT_SERIAL_PIN == D1
-#        define EIMSK_BIT _BV(INT1)
-#        define EICRx_BIT (~(_BV(ISC10) | _BV(ISC11)))
-#        define SERIAL_PIN_INTERRUPT INT1_vect
-#        define EICRx EICRA
+#        define REG_INFOS INT1_vect, _BV(INT1), (~(_BV(ISC10) | _BV(ISC11))), EICRA
 #    elif SOFT_SERIAL_PIN == D2
-#        define EIMSK_BIT _BV(INT2)
-#        define EICRx_BIT (~(_BV(ISC20) | _BV(ISC21)))
-#        define SERIAL_PIN_INTERRUPT INT2_vect
-#        define EICRx EICRA
+#        define REG_INFOS INT2_vect, _BV(INT2), (~(_BV(ISC20) | _BV(ISC21))), EICRA
<<<<Omitted on the way>>>>

and

-#    ifndef SERIAL_PIN_INTERRUPT
+#    ifndef REG_INFOS
 #        error invalid SOFT_SERIAL_PIN value
 #    endif

[drashna]

Awesome. I feel more comfortable with "my way". It's less complex, and easier for me to read.

[takai]

I've just tested this PR on my ATmega32U2 split keyboard and it seems to work fine.
Thanks for the ping, @mtei.