This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

TPS65381A-Q1: SafeTI Diagnostic Library for Hercules, v2.4.0

Part Number: TPS65381A-Q1
Other Parts Discussed in Thread: TMS320F28335,

Hi.

I'm using TPS65381A-Q1 along with TMS320F28335. I have found a SW driver which is provided for Hercules (including some examples). I took the driver and adopted it to F28335, which was not a big task (I wonder why the driver is not generalized so it could be used with any MCU).

Anyway, I was stuck with the driver usage, I did a lot of debugging and finally I found a mistake in the code:

In Tps_Driver.c, there is a function configuring the Watchdog windows:

boolean TPS_ConfigureWatchdogWindows(TPS_WatchdogWindows watchdog_windows,
        uint8 u8windowtime_scaler)
{
    boolean blRetVal = TRUE;
    uint8 u8tps_device_state = 0U;

    blRetVal = TPS_GetCurrentTPSDeviceState(&u8tps_device_state);
    if ((TRUE == blRetVal) && (TPS_DEVICE_DIAGNOSTIC == u8tps_device_state))
    {
        switch (watchdog_windows)
        {
            case OPEN_WINDOW:
                if (u8windowtime_scaler <= OPEN_WINDOW_SCALER_MAX)
                {
                    blRetVal = TpsIf_SetRegisterBitFieldVerify(TPS_WDT_WIN1_CFG,
                    BF_WD_OPEN_WINDOW_SCALER_START,
                    BF_WD_OPEN_WINDOW_SCALER_LENGTH, u8windowtime_scaler);
                    /*update the init_configuration structure to support register readback*/
                    /*SAFETYMCUSW 9 S MR: 12.2 <APPROVED> "Comment_2"*/
                    BFU8_SET(init_configuration.u8wdt_win1_cfg,
                            u8windowtime_scaler, BF_WD_OPEN_WINDOW_SCALER_START,
                            BF_WD_OPEN_WINDOW_SCALER_LENGTH);
                }
                else
                {
                    blRetVal = FALSE;
                }
                break;
            case CLOSE_WINDOW:
                if (u8windowtime_scaler <= CLOSE_WINDOW_SCALER_MAX)
                {
                    blRetVal = TpsIf_SetRegisterBitFieldVerify(TPS_WDT_WIN2_CFG,
                    BF_WD_CLOSE_WINDOW_SCALER_START,
                    BF_WD_CLOSE_WINDOW_SCALER_LEN, u8windowtime_scaler);
                    /*update the init_configuration structure to support register readback*/
                    /*SAFETYMCUSW 9 S MR: 12.2 <APPROVED> "Comment_2"*/
                    BFU8_SET(init_configuration.u8wdt_win2_cfg,
                            u8windowtime_scaler,
                            BF_WD_CLOSE_WINDOW_SCALER_START,
                            BF_WD_CLOSE_WINDOW_SCALER_LEN);
                }
                else
                {
                    blRetVal = FALSE;
                }
                break;
            default:
                blRetVal = FALSE;
                break;
        }
    }
    else
    {
        blRetVal = FALSE;
    }
    return blRetVal;
}

It seems alright until you find out that the OPEN_WINDOW and CLOSE_WINDOW are swapped (probably accidentally): in case of OPEN_WINDOW, the value is written into TPS_WDT_WIN1_CFG and in case of CLOSE_WINDOW, the value is written into TPS_WDT_WIN2_CFG.

Based on the TPS65381A-Q1 datasheet (available here: https://www.ti.com/lit/ds/symlink/tps65381a-q1.pdf?ts=1635950961406&ref_url=https%253A%252F%252Fwww.google.com%252F - e.g. Fig. 5-6), window 1 (WIN1) is also displayed as CLOSE, while window 2 (WIN2) is displayed as OPEN, the other way around from the code is handling the data.

The faulty behavior comes up when I try to use WIN1 longer than 31 x 0.55 ms = 17.05 ms. The value is sent to the device register WIN2, and it is dropped due to the invalid value, since the WIN2 accepts numbers 31 and lower. Default value is then used, which is 0x1f = 31d = 17.05ms. The same way, WIN2 value is stored to the WIN1 register, but the intended WIN2 may be much shorter than the intended WIN1, thus the entire timing is broken and the system goes to safe state upon leaving the DIAGNOSTIC state.

I believe the issue is described well. I would suggest a bugfix:

boolean TPS_ConfigureWatchdogWindows(TPS_WatchdogWindows watchdog_windows,
        uint8 u8windowtime_scaler)
{
    boolean blRetVal = TRUE;
    uint8 u8tps_device_state = 0U;

    blRetVal = TPS_GetCurrentTPSDeviceState(&u8tps_device_state);
    if ((TRUE == blRetVal) && (TPS_DEVICE_DIAGNOSTIC == u8tps_device_state))
    {
        switch (watchdog_windows)
        {
            case OPEN_WINDOW:
                if (u8windowtime_scaler <= OPEN_WINDOW_SCALER_MAX)
                {
                    blRetVal = TpsIf_SetRegisterBitFieldVerify(TPS_WDT_WIN2_CFG,
                    BF_WD_OPEN_WINDOW_SCALER_START,
                    BF_WD_OPEN_WINDOW_SCALER_LENGTH, u8windowtime_scaler);
                    /*update the init_configuration structure to support register readback*/
                    /*SAFETYMCUSW 9 S MR: 12.2 <APPROVED> "Comment_2"*/
                    BFU8_SET(init_configuration.u8wdt_win2_cfg,
                            u8windowtime_scaler, BF_WD_OPEN_WINDOW_SCALER_START,
                            BF_WD_OPEN_WINDOW_SCALER_LENGTH);
                }
                else
                {
                    blRetVal = FALSE;
                }
                break;
            case CLOSE_WINDOW:
                if (u8windowtime_scaler <= CLOSE_WINDOW_SCALER_MAX)
                {
                    blRetVal = TpsIf_SetRegisterBitFieldVerify(TPS_WDT_WIN1_CFG,
                    BF_WD_CLOSE_WINDOW_SCALER_START,
                    BF_WD_CLOSE_WINDOW_SCALER_LEN, u8windowtime_scaler);
                    /*update the init_configuration structure to support register readback*/
                    /*SAFETYMCUSW 9 S MR: 12.2 <APPROVED> "Comment_2"*/
                    BFU8_SET(init_configuration.u8wdt_win1_cfg,
                            u8windowtime_scaler,
                            BF_WD_CLOSE_WINDOW_SCALER_START,
                            BF_WD_CLOSE_WINDOW_SCALER_LEN);
                }
                else
                {
                    blRetVal = FALSE;
                }
                break;
            default:
                blRetVal = FALSE;
                break;
        }
    }
    else
    {
        blRetVal = FALSE;
    }
    return blRetVal;
}

I hope it helps and I hope it saves some debugging time to others.

Best regards,

Matej

  • ...more confusion in the driver, fixes below:

    Tps_priv.h

    Originally:

    /*Bit field WDT_WIN1_CFG register*/
    #define BF_WD_OPEN_WINDOW_SCALER_START     ((uint8)0U)
    #define BF_WD_OPEN_WINDOW_SCALER_LENGTH    ((uint8)7U)
    #define BF_WD_CLOSE_WINDOW_SCALER_START    ((uint8)0U)
    #define BF_WD_CLOSE_WINDOW_SCALER_LEN      ((uint8)5U)

    Proposal:

    /*Bit field WDT_WIN1_CFG register*/
    #define BF_WD_OPEN_WINDOW_SCALER_START     ((uint8)0U)
    #define BF_WD_OPEN_WINDOW_SCALER_LENGTH    ((uint8)5U)
    #define BF_WD_CLOSE_WINDOW_SCALER_START    ((uint8)0U)
    #define BF_WD_CLOSE_WINDOW_SCALER_LEN      ((uint8)7U)

    Tps_types.h (just a cosmetics/in comments)

    Originally

    typedef enum _TPS_Registers
    {
        TPS_DEV_REV,                /**< 0x00: Alias for device revision register                    */
        TPS_DEV_ID,                 /**< 0x01: Alias for device identifier register                  */
        TPS_DEV_STAT,               /**< 0x02: Alias for device status register                      */
        TPS_DEV_CFG1,               /**< 0x03: Alias for config 1 register                           */
        TPS_DEV_CFG2,               /**< 0x04: Alias for config 2 register                           */
        TPS_VMON_STAT_1,            /**< 0x05: Alias for voltage monitor status 1 register           */
        TPS_VMON_STAT_2,            /**< 0x06: Alias for voltage monitor status 2 register           */
        TPS_SAFETY_STAT_1,          /**< 0x07: Alias for safety status 1 register                    */
        TPS_SAFETY_STAT_2,          /**< 0x08: Alias for safety status 2 register                    */
        TPS_SAFETY_STAT_3,          /**< 0x09: Alias for safety status 3 register                    */
        TPS_SAFETY_STAT_4,          /**< 0x0A: Alias for safety status 4 register                    */
        TPS_SAFETY_STAT_5,          /**< 0x0B: Alias for safety status 5 register                    */
        TPS_SAFETY_ERR_CFG,         /**< 0x0C: Alias for safety error config register                */
        TPS_SAFETY_BIST_CTRL,       /**< 0x0D: Alias for safety BIST control register                */
        TPS_SAFETY_CHECK_CTRL,      /**< 0x0E: Alias for safety check control register               */
        TPS_SAFETY_FUNC_CFG,        /**< 0x0F: Alias for safety function config register             */
        TPS_SAFETY_ERR_STAT,        /**< 0x10: Alias for safety error status register                */
        TPS_SAFETY_ERR_PWM_H,       /**< 0x11: Alias for safety error pwm high register              */
        TPS_SAFETY_ERR_PWM_L,       /**< 0x12: Alias for safety error pwm low register               */
        TPS_SAFETY_PWD_THR_CFG,     /**< 0x13: Alias for safety power down threshold config register */
        TPS_SAFETY_CFG_CRC,         /**< 0x14: Alias for safety config crc register                  */
        TPS_DIAG_CFG_CTRL,          /**< 0x15: Alias for diagnostic config control register          */
        TPS_DIAG_MUX_SEL,           /**< 0x16: Alias for diagnostic mux select register              */
        TPS_WDT_TOKEN_FDBCK,        /**< 0x17: Alias for watchdog token feedback register            */
        TPS_WDT_WIN1_CFG,           /**< 0x18: Alias for watchdog open window config register        */
        TPS_WDT_WIN2_CFG,           /**< 0x19: Alias for watchdog close window config register       */
        TPS_WDT_TOKEN_VALUE,        /**< 0x1A: Alias for watchdog token register                     */
        TPS_WDT_STATUS,             /**< 0x1B: Alias for watchdog status register                    */
        TPS_WDT_ANSWER,             /**< 0x1C: Alias for watchdog answer register                    */
        TPS_SENS_CTRL,              /**< 0x1D: Alias for sense control register                      */
        TPS_SW_LOCK,                /**< 0x1E: Alias for register lock register                      */
        TPS_SW_UNLOCK               /**< 0x1F: Alias for register unlock register                    */
    }TPS_Registers;

    Proposal:

    typedef enum _TPS_Registers
    {
        TPS_DEV_REV,                /**< 0x00: Alias for device revision register                    */
        TPS_DEV_ID,                 /**< 0x01: Alias for device identifier register                  */
        TPS_DEV_STAT,               /**< 0x02: Alias for device status register                      */
        TPS_DEV_CFG1,               /**< 0x03: Alias for config 1 register                           */
        TPS_DEV_CFG2,               /**< 0x04: Alias for config 2 register                           */
        TPS_VMON_STAT_1,            /**< 0x05: Alias for voltage monitor status 1 register           */
        TPS_VMON_STAT_2,            /**< 0x06: Alias for voltage monitor status 2 register           */
        TPS_SAFETY_STAT_1,          /**< 0x07: Alias for safety status 1 register                    */
        TPS_SAFETY_STAT_2,          /**< 0x08: Alias for safety status 2 register                    */
        TPS_SAFETY_STAT_3,          /**< 0x09: Alias for safety status 3 register                    */
        TPS_SAFETY_STAT_4,          /**< 0x0A: Alias for safety status 4 register                    */
        TPS_SAFETY_STAT_5,          /**< 0x0B: Alias for safety status 5 register                    */
        TPS_SAFETY_ERR_CFG,         /**< 0x0C: Alias for safety error config register                */
        TPS_SAFETY_BIST_CTRL,       /**< 0x0D: Alias for safety BIST control register                */
        TPS_SAFETY_CHECK_CTRL,      /**< 0x0E: Alias for safety check control register               */
        TPS_SAFETY_FUNC_CFG,        /**< 0x0F: Alias for safety function config register             */
        TPS_SAFETY_ERR_STAT,        /**< 0x10: Alias for safety error status register                */
        TPS_SAFETY_ERR_PWM_H,       /**< 0x11: Alias for safety error pwm high register              */
        TPS_SAFETY_ERR_PWM_L,       /**< 0x12: Alias for safety error pwm low register               */
        TPS_SAFETY_PWD_THR_CFG,     /**< 0x13: Alias for safety power down threshold config register */
        TPS_SAFETY_CFG_CRC,         /**< 0x14: Alias for safety config crc register                  */
        TPS_DIAG_CFG_CTRL,          /**< 0x15: Alias for diagnostic config control register          */
        TPS_DIAG_MUX_SEL,           /**< 0x16: Alias for diagnostic mux select register              */
        TPS_WDT_TOKEN_FDBCK,        /**< 0x17: Alias for watchdog token feedback register            */
        TPS_WDT_WIN1_CFG,           /**< 0x18: Alias for watchdog close window config register       */
        TPS_WDT_WIN2_CFG,           /**< 0x19: Alias for watchdog open window config register        */
        TPS_WDT_TOKEN_VALUE,        /**< 0x1A: Alias for watchdog token register                     */
        TPS_WDT_STATUS,             /**< 0x1B: Alias for watchdog status register                    */
        TPS_WDT_ANSWER,             /**< 0x1C: Alias for watchdog answer register                    */
        TPS_SENS_CTRL,              /**< 0x1D: Alias for sense control register                      */
        TPS_SW_LOCK,                /**< 0x1E: Alias for register lock register                      */
        TPS_SW_UNLOCK               /**< 0x1F: Alias for register unlock register                    */
    }TPS_Registers;

    Tps_Driver.c - one more edit to align the WatchdogInit code

    Originally:

    boolean TPS_WatchdogInit(const TPS_WatchdoConfiguration *const watchdogconfig)
    {
        uint8 u8wdt_token_fdbck = 0U;
        uint8 u8tps_device_state = 0U;
        boolean blRetVal = TRUE;
    
    	/*Check that pointer values falls in range*/
    	/*SAFETYMCUSW 439 S MR:11.3 <APPROVED> "Comment_3"*/
    	/*SAFETYMCUSW 439 S MR:11.3 <APPROVED> "Comment_3"*/
    	if (!CHECK_RANGE_RAM_PTR(watchdogconfig))
    	{
    		blRetVal = FALSE;
    	}
    	else
    	{
    		blRetVal = TPS_GetCurrentTPSDeviceState(&u8tps_device_state);
    		if ((TRUE == blRetVal)
    				&& (TPS_DEVICE_DIAGNOSTIC == u8tps_device_state)) {
    			blRetVal = TPS_SetWatchdogMode(watchdogconfig->watchdog_mode);
    			/*setting pwm width for error monitoring*/
    			if (QANDA_MODE == watchdogconfig->watchdog_mode) {
    
    				/*sets register wdt_token_fdbck*/
    				/*SAFETYMCUSW 9 S MR: 12.2 <APPROVED> "Comment_2"*/
    				BFU8_SET(u8wdt_token_fdbck, watchdogconfig->u8TokenSeed,
    						BF_TOKEN_SEED_START, BF_TOKEN_SEED_LENGTH);
    				/*SAFETYMCUSW 9 S MR: 12.2 <APPROVED> "Comment_2"*/
    				BFU8_SET(u8wdt_token_fdbck, watchdogconfig->u8TokenFDBK,
    						BF_TOKEN_FDBK_START, BF_TOKEN_FDBK_LENGTH);
    				blRetVal = ((TpsIf_SetRegisterVerify(TPS_WDT_TOKEN_FDBCK,
    						u8wdt_token_fdbck)) && blRetVal);
    
    				/*update the init_configuration structure to support register readback*/
    				init_configuration.u8wdt_token_fbck = u8wdt_token_fdbck;
    			} else {
    				blRetVal = TpsIf_SetRegisterVerify(TPS_WDT_TOKEN_FDBCK, 0U);
    				init_configuration.u8wdt_token_fbck = 0U;
    			}
    
    			blRetVal = ((TPS_ConfigureWatchdogReset(
    					watchdogconfig->blwatchdog_reset)) && blRetVal);
    
    			/*set watchdog window duration(wdt_win1_cfg,wdt_win2_cfg)*/
    			blRetVal = ((TPS_ConfigureWatchdogWindows(OPEN_WINDOW,
    					watchdogconfig->u8open_windowtime_scaler)) && blRetVal);
    			/*update the init_configuration structure to support register readback*/
    			init_configuration.u8wdt_win1_cfg =
    					watchdogconfig->u8open_windowtime_scaler & WDT_WIN1_CFG_MASK;
    
    			blRetVal = ((TPS_ConfigureWatchdogWindows(CLOSE_WINDOW,
    					watchdogconfig->u8close_windowtime_scaler)) && blRetVal);
    			/*update the init_configuration structure to support register readback*/
    			init_configuration.u8wdt_win2_cfg =
    					watchdogconfig->u8close_windowtime_scaler
    							& WDT_WIN2_CFG_MASK;
    		} else {
    			blRetVal = FALSE;
    		}
    	}
    
        return blRetVal;
    
    }
    

    Proposal:

    boolean TPS_WatchdogInit(const TPS_WatchdoConfiguration *const watchdogconfig)
    {
        uint8 u8wdt_token_fdbck = 0U;
        uint8 u8tps_device_state = 0U;
        boolean blRetVal = TRUE;
    
    	/*Check that pointer values falls in range*/
    	/*SAFETYMCUSW 439 S MR:11.3 <APPROVED> "Comment_3"*/
    	/*SAFETYMCUSW 439 S MR:11.3 <APPROVED> "Comment_3"*/
    	if (!CHECK_RANGE_RAM_PTR(watchdogconfig))
    	{
    		blRetVal = FALSE;
    	}
    	else
    	{
    		blRetVal = TPS_GetCurrentTPSDeviceState(&u8tps_device_state);
    		if ((TRUE == blRetVal)
    				&& (TPS_DEVICE_DIAGNOSTIC == u8tps_device_state)) {
    			blRetVal = TPS_SetWatchdogMode(watchdogconfig->watchdog_mode);
    			/*setting pwm width for error monitoring*/
    			if (QANDA_MODE == watchdogconfig->watchdog_mode) {
    
    				/*sets register wdt_token_fdbck*/
    				/*SAFETYMCUSW 9 S MR: 12.2 <APPROVED> "Comment_2"*/
    				BFU8_SET(u8wdt_token_fdbck, watchdogconfig->u8TokenSeed,
    						BF_TOKEN_SEED_START, BF_TOKEN_SEED_LENGTH);
    				/*SAFETYMCUSW 9 S MR: 12.2 <APPROVED> "Comment_2"*/
    				BFU8_SET(u8wdt_token_fdbck, watchdogconfig->u8TokenFDBK,
    						BF_TOKEN_FDBK_START, BF_TOKEN_FDBK_LENGTH);
    				blRetVal = ((TpsIf_SetRegisterVerify(TPS_WDT_TOKEN_FDBCK,
    						u8wdt_token_fdbck)) && blRetVal);
    
    				/*update the init_configuration structure to support register readback*/
    				init_configuration.u8wdt_token_fbck = u8wdt_token_fdbck;
    			} else {
    				blRetVal = TpsIf_SetRegisterVerify(TPS_WDT_TOKEN_FDBCK, 0U);
    				init_configuration.u8wdt_token_fbck = 0U;
    			}
    
    			blRetVal = ((TPS_ConfigureWatchdogReset(
    					watchdogconfig->blwatchdog_reset)) && blRetVal);
    
    			/*set watchdog window duration(wdt_win1_cfg,wdt_win2_cfg)*/
    			blRetVal = ((TPS_ConfigureWatchdogWindows(CLOSE_WINDOW,
    					watchdogconfig->u8close_windowtime_scaler)) && blRetVal);
    			/*update the init_configuration structure to support register readback*/
    			init_configuration.u8wdt_win1_cfg =
    					watchdogconfig->u8close_windowtime_scaler & WDT_WIN1_CFG_MASK;
    
    			blRetVal = ((TPS_ConfigureWatchdogWindows(OPEN_WINDOW,
    					watchdogconfig->u8open_windowtime_scaler)) && blRetVal);
    			/*update the init_configuration structure to support register readback*/
    			init_configuration.u8wdt_win2_cfg =
    					watchdogconfig->u8open_windowtime_scaler
    							& WDT_WIN2_CFG_MASK;
    		} else {
    			blRetVal = FALSE;
    		}
    	}
    
        return blRetVal;
    
    }

  • ...and one more thing: in case you appreciate this effort, would it be possible to appreciate it with an expedited batch of 100pcs TPS65381A-Q1 chips? We are running short of stock... Slight smile

  • Hi Matej,

    Unfortunately we in the TPS65381A-Q1 team are unable to modify the requested code, for any software requests I can reassign this thread to the team responsible for the software library. Let me know if this is something you need.

    As for the sample request, please work with your sales team on this request as I am unable to provide any assistance on sampling requests.

    Best regards,

    Layne J

  • Hi,

    Actually, I don't need the code to be changed (I already did in my code). I was just wondering why such a driver intended to be functionally safe is not tested for almost obvious mistakes and I wanted to let the TI team know about it. If you can reasign this thread to the right guys, I believe it would be appreciated by many.

    And about the sample request - it was a semi-joke: I don't expect anything, but this is a kind of situation when one gets "a beer" for finding such a mistake and I just proposed what kind of "beer" I would appreciate. Currently the situation on the market is very frustrating for small volume players (lead time of 4-6 months).

    All the best,

    Matej

  • To close this self-driven thread, I just noticed that there is a confusion in the datasheet of the TPS65381A-Q1.

    While in the Trigger mode the WIN1=CLOSED and WIN2=OPEN, the Q&A mode uses WIN1=OPEN and WIN2=CLOSED. Now I have to appologize to the SW team. The correct adjustment to the code would be completely different - do not use "open" or "close" in the code, since it is heavily confusing. Use WIN1 and WIN2 instead to enable any kind of mode.

    One more time I find TI drivers single-purpose only (mainly for EVBs)...

    Thanks,

    Matej