search

simplefoc / Arduino-FOC

Arduino FOC for BLDC and Stepper motors - Arduino Based Field Oriented Control Algorithm Library
https://docs.simplefoc.com
MIT License
2.1k stars 537 forks source link

TCC0 pin config ATSAME51 #122

Closed Juanduino closed 2 years ago

Juanduino commented 3 years ago

Hi

There is some bug in the /drivers/hardware_specific/samd51_mcu.cpp

See this log: https://community.simplefoc.com/t/same51-pin-configuration/1280/3

Attached variant.cpp: https://github.com/adafruit/ArduinoCore-samd/blob/master/variants/feather_m4_can/variant.cpp

According to the variant file pin 13, 12, 11, 10 are PIN_ATTR_PWM_G

There seems to be some confusion in the appropriate function when fetchin pin configuration.

I would like to setup pins 13, 12, 11 and 10 for TCC0. Plz advice

Is the Peripheral letter used for configuring pin?

Here the samd_mcu.cpp is supposed to fin a "G" ?


#if defined(_SAMD51_)||defined(_SAME51_)
    else if (peripheral==PIO_TCC_PDEC) {
        result.tcc.chaninfo = association.tccG;
        result.wo = association.woG
```;
Juanduino commented 3 years ago

Wright, I haven't been up to date with the issues and bugs. I see this is a bug you are aware of: https://github.com/simplefoc/Arduino-FOC/pull/93#issue-915629852

As seen in the log above, I am able to select TCC0 for all pins, if I crude-hack the source. Im just not sure about the Peripheral letter. Does it have any function other then info, in regards to the driver.

image

Juanduino commented 3 years ago

Source: https://github.com/adafruit/ArduinoCore-samd/blob/master/cores/arduino/wiring_analog.c

if(attr & (PIN_ATTR_PWM_E|PIN_ATTR_PWM_F|PIN_ATTR_PWM_G)){

    uint32_t tcNum = GetTCNumber(pinDesc.ulPWMChannel);
    uint8_t tcChannel = GetTCChannelNumber(pinDesc.ulPWMChannel);
    static bool tcEnabled[TCC_INST_NUM+TC_INST_NUM];

    if(attr & PIN_ATTR_PWM_E)
        pinPeripheral(pin, PIO_TIMER);
    else if(attr & PIN_ATTR_PWM_F)
        pinPeripheral(pin, PIO_TIMER_ALT);
    else if(attr & PIN_ATTR_PWM_G)
        pinPeripheral(pin, PIO_TCC_PDEC);

PinDescription pinDesc = g_APinDescription[pin]; uint32_t attr = pinDesc.ulPinAttribute;

Juanduino commented 3 years ago

Solved (void printTCCConfiguration is still buggy, so letter is printed wrong) The appropriate TCC channel is selected, without other modifications to src.

tccConfiguration getTCCChannelNr(int pin, EPioType peripheral) {
    tccConfiguration result;
    result.pin = pin;
    result.peripheral  = peripheral;
    result.tcc.tccn = -2;
    result.tcc.chan = -2;
    const PinDescription& pinDesc = g_APinDescription[pin];
    uint32_t attr = pinDesc.ulPinAttribute;
    struct wo_association& association = getWOAssociation(pinDesc.ulPort, pinDesc.ulPin);

    if(attr == PIN_ATTR_PWM_E)
        peripheral =  PIO_TIMER;
    else if(attr == PIN_ATTR_PWM_F)
        peripheral = PIO_TIMER_ALT;
    else if(attr == PIN_ATTR_PWM_G)
        peripheral = PIO_TCC_PDEC;

    if (association.port==NOT_A_PORT)
        return result; // could not find the port/pin
    if (peripheral==PIO_TIMER) {
        result.tcc.chaninfo = association.tccE;
        result.wo = association.woE;
    }
    else if (peripheral==PIO_TIMER_ALT) {
        result.tcc.chaninfo = association.tccF;
        result.wo = association.woF;
    }
//#if defined(__SAMD51__) || defined(__SAME51__) 
    else if (peripheral==PIO_TCC_PDEC) {
        result.tcc.chaninfo = association.tccG;
        result.wo = association.woG;
    }
//#endif
    return result;
}
runger1101001 commented 3 years ago

Thank you very much for looking into this!

As you have discovered the SAMD/E51 support is still a bit 'untested' - I've been mostly focused on the SAMD21 so far.

The problems you've reported have also been experienced by others, but so far I have not been able to reproduce them.

I recently bought a Metro M4 (mouser had them on special cheap!) and I have a couple of other SAMD/E51 boards as well now. I'm on holiday now though, back home in 2 weeks... I'll try to do a full testing on all the boards then. In the meantime I'll try to follow your report 'theoretically'...

runger1101001 commented 3 years ago

Ok, looking at the theoretical side:

I think you cannot trust the variant.cpp files, unfortunately. Both for SAMD21 and 51, both for the Arduino boards and the Adafruit boards, the variant.cpp files are either just wrong, or incomplete regarding the timers.

This is why the implementation has its own table for the pin to TCC mappings. I would have given a lot to avoid having to do it this way, but I could not see a way to make it work based on the variant.cpp files.

An example:

In your variant.cpp files attached (for the Adafruit board) pin 10 is defined like this:

{ PORTA, 20, PIO_TIMER_ALT, PIN_ATTR_PWM_G, No_ADC_Channel, TCC0_CH0, NOT_ON_TIMER, EXTERNAL_INT_4 },

So it's the SAME51 pin PA20, and according to the variant.cpp file supports PWM on peripheral G, TCC0_CH0. There is no information which WO it is (It's WO0).

But if you check the data sheet for the SAME51, Table 6-1, page 35 (https://www.mouser.com/datasheet/2/268/60001507A-1130176.pdf) you will find that PA20 also supports TCC1_CH0 (WO4), as well as TC7 (which we don't use).

So for this reason the simplefoc driver code includes the table in samd51_mcu.cpp: https://github.com/simplefoc/Arduino-FOC/blob/master/src/drivers/hardware_specific/samd51_mcu.cpp

This is (supposed to be) a copy of the TCC and WO information from table 6-1 from the datasheet... to compensate the deficits of the variant.cpp files...

Juanduino commented 3 years ago

Hi!

Tjek out the github issue I filed.

There is a better Way to index:)

tir. 26. okt. 2021 kl. 20.24 skrev runger1101001 @.***>:

Ok, looking at the theoretical side:

I think you cannot trust the variant.cpp files, unfortunately. Both for SAMD21 and 51, both for the Arduino boards and the Adafruit boards, the variant.cpp files are either just wrong, or incomplete regarding the timers.

This is why the implementation has its own table for the pin to TCC mappings. I would have given a lot to avoid having to do it this way, but I could not see a way to make it work based on the variant.cpp files.

An example:

In your variant.cpp files attached (for the Adafruit board) pin 10 is defined like this:

{ PORTA, 20, PIO_TIMER_ALT, PIN_ATTR_PWM_G, No_ADC_Channel, TCC0_CH0, NOT_ON_TIMER, EXTERNAL_INT_4 },

So it's the SAME51 pin PA20, and according to the variant.cpp file supports PWM on peripheral G, TCC0_CH0. There is no information which WO it is (It's WO0).

But if you check the data sheet for the SAME51, Table 6-1, page 35 ( https://www.mouser.com/datasheet/2/268/60001507A-1130176.pdf) you will find that PA20 also supports TCC1_CH0 (WO4), as well as TC7 (which we don't use).

So for this reason the simplefoc driver code includes the table in samd51_mcu.cpp: https://github.com/simplefoc/Arduino-FOC/blob/master/src/drivers/hardware_specific/samd51_mcu.cpp

This is (supposed to be) a copy of the TCC and WO information from table 6-1 from the datasheet... to compensate the deficits of the variant.cpp files...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/simplefoc/Arduino-FOC/issues/122#issuecomment-952198013, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBKYHTLJZP4EUZZY4D2E7LUI3W6XANCNFSM5GWTRFBA .

runger1101001 commented 3 years ago

Hi @Juanduino,

Which GitHub issue are you referring to? Is it not this one? I'm sorry, I don't really understand your response...

Are you referring to your code above? This is not a correct solution. The getTCCChannelNr() is supposed to return information about the pin, but not actually set anything, so you should not call pinPeripheral() from in this function. And relying on the pinDesc.ulPinAttribute from variant.cpp will not work. Maybe it worked for you, because you wanted TCC0, but it would fail for someone trying to use peripheral F (TCC1) on that same pin.

Juanduino commented 3 years ago

Sorry, I thought we where in SimpleFoc community.

You are right, but this is because the attribute is set in the variant file. There is a conflict between the attribute (PIN_ATTR_PWM_G) and the PIO_DIGITAL/ PIO_TIMER_ALT. Calling pinPeripheral() I have changed in above snip.

The only way to do it right, is to change the variant file. Since the pio_tcc_pdec (peripheral G) variable is not in the variant file. This in turn means, that TCC0 on PA20 can´t be chosen in the current release. The weird thing is, even changing the variant file to this, { PORTA, 20, PIO_TCC_PDEC, PIN_ATTR_PWM_G, No_ADC_Channel, TCC0_CH0, NOT_ON_TIMER, EXTERNAL_INT_4 }, This is from the variant file: { PORTA, 20, PIO_TIMER_ALT, PIN_ATTR_PWM_G, No_ADC_Channel, TCC0_CH0, NOT_ON_TIMER, EXTERNAL_INT_4 },

,will not make it choose TCC0.

I know your goal with this implementation is to make it simple to setup, but in my opinion it beats the purpose, when a TC channel is selected, when it should be TCC. By the way, great work on the code. Overall it is pretty awesome.

ons. 27. okt. 2021 kl. 00.35 skrev runger1101001 @.***>:

Hi @Juanduino https://github.com/Juanduino,

Which GitHub issue are you referring to? Is it not this one? I'm sorry, I don't really understand your response...

Are you referring to your code above? This is not a correct solution. The getTCCChannelNr() is supposed to return information about the pin, but not actually set anything, so you should not call pinPeripheral() from in this function. And relying on the pinDesc.ulPinAttribute from variant.cpp will not work. Maybe it worked for you, because you wanted TCC0, but it would fail for someone trying to use peripheral F (TCC1) on that same pin.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/simplefoc/Arduino-FOC/issues/122#issuecomment-952380272, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBKYHQN4X4QYPZOIDA6G6LUI43NHANCNFSM5GWTRFBA .

runger1101001 commented 3 years ago

Yeah, there clearly is still something wrong, both you and Antun are getting the wrong pin assignments...

The code is complicated because the chip is a bit complicated :-) Each pin can have 0 to 3 timer peripherals associated, but what's worse when doing motor control, the different timer pins are associated to different "channels" (each with its own compare registers) and WO (waveform output #) which makes a difference when using dead-time insertion...

Compare that to the RP2040, where basically any pin can do PWM in the same way, or ESP32 where any pin can be used with any peripheral, and it explains why the code for SAME51 gets so complex.

So when specifying 3 pins using Arduino PIN numbers, the semantics of automatically choosing which TCCs to use become quite complex. The code attempts to solve this by trying all the permutations of TCC units possible on the pins given (its only a few hundred combinations for the SAME51), scoring them, and selecting the best one. Thus, if you pick 3 pins which have TCC0 on either peripheral F or G, it should find that combination as the lowest scoring and use it.

In my tests it was actually working quite well, but clearly there's still a bug somewhere. I will solve it though, with your guys help!

Juanduino commented 3 years ago

I can confirm, that changing the variant file from PIN_ATTR_PWM_G to PIN_ATTR_PWM_F will make it choose TCC1.

So it is on the board maker to make sure the variant file is correct.

If on a dev. board like the feather, you need to change the variant file.

Juanduino commented 3 years ago

I ended up just putting "NOT_ON_TIMER" to make it skip the wrong TC association, and use TCC1. Until there is a proper fix, this will work fine. Not on timer

Juanduino commented 3 years ago

I did a bit of re-arranging on the wiring_analog file 

#if defined(__SAMD51__) || defined(_SAME51_) 
  Adc *adc;
  if(g_APinDescription[pin].ulPinAttribute & PIN_ATTR_ANALOG) adc = ADC0;

  else if(g_APinDescription[pin].ulPinAttribute & PIN_ATTR_ANALOG_ALT) adc = ADC1;

  else return 0;

  if (adc == ADC0) {
      SUPC->VREF.reg |= SUPC_VREF_TSEN | SUPC_VREF_ONDEMAND;
  GCLK->PCHCTRL[ADC0_GCLK_ID].reg = GCLK_PCHCTRL_GEN_GCLK1_Val | (1 << GCLK_PCHCTRL_CHEN_Pos);
  }
  if (adc == ADC1) {
      SUPC->VREF.reg |= SUPC_VREF_TSEN | SUPC_VREF_ONDEMAND;
  GCLK->PCHCTRL[ADC1_GCLK_ID].reg = GCLK_PCHCTRL_GEN_GCLK1_Val | (1 << GCLK_PCHCTRL_CHEN_Pos);
  }

  adc->CTRLA.bit.PRESCALER = ADC_CTRLA_PRESCALER_DIV4_Val; 
  while( adc->SYNCBUSY.reg & ADC_SYNCBUSY_CTRLB );
  adc->CTRLB.bit.RESSEL = ADC_CTRLB_RESSEL_12BIT_Val;

  while (adc->SYNCBUSY.reg & ADC_SYNCBUSY_CTRLB); //wait for sync
  adc->SAMPCTRL.reg = 0;   
  while( adc->SYNCBUSY.reg & ADC_SYNCBUSY_SAMPCTRL ); //wait for sync
// Averaging (see datasheet table in AVGCTRL register description)
        adc->AVGCTRL.reg = ADC_AVGCTRL_SAMPLENUM_1 |    // 1 sample only (no oversampling nor averaging)
                            ADC_AVGCTRL_ADJRES(0x0ul);   // Adjusting result by 0

        while( adc->SYNCBUSY.reg & ADC_SYNCBUSY_AVGCTRL );  //wait for sync

  while( adc->SYNCBUSY.reg & ADC_SYNCBUSY_INPUTCTRL ); //wait for sync
  adc->INPUTCTRL.bit.MUXPOS = g_APinDescription[pin].ulADCChannelNumber; // Selection for the positive ADC input
runger1101001 commented 3 years ago

Ok, I have fixed the SAMD51 pin assignment issue, I believe. But I think you may still not get what you want as described above.

The issue had to do with the peripheral E/F/G selection and assignment. Basically it was using E when it should have been using G, and in SAMD51 the TC units (on E) don't even get configured by the code, so it wasn't working at all.

But now this is fixed, and in my tests it correctly selects pins on peripheral F or G (or a mix thereof) according to its pin-selection algorithm.

However, this still does not mean you will get peripheral G for pins 10,11,12... this is because they also have a valid (and equal or better) configuration on TCC1, peripheral F. And since all you can specify via the API is the pin numbers, there is no way for the driver code to know you might prefer G. The auto-selection algorithm will pick the "best" available configuration though - preferring to keep pins on the same TCC unit for example...

The fixed code is merged to the dev branch of SimpleFOC, you could test it if you like.

runger1101001 commented 3 years ago

Fixed in https://github.com/simplefoc/Arduino-FOC/pull/134