UART DMA Rx STM32H745 with LL

[veera1]

Hello,

I have tried implement UART Driver only with LL Library. I'm using Nucleo-H745ZIQ. Reception through DMA is not happening. Data is not getting copied to the buffer. MPU Also Configured. I'm attaching the code. Hope DMA Configuration for Rx is ok. Please have a look and correct me if I'm wrong. Please let me know if any modifications needed

/* Includes */
#include "main.h"
#include <string.h>
 
/* Private function prototypes */
void SystemClock_Config(void);
static void MPU_Config(void);
 
/* USART related functions */
void usart_init(void);
void usart_rx_check(void);
void usart_process_data(const void* data, size_t len);
void usart_send_string(const char* str);
 
#define ARRAY_LEN(x)            (sizeof(x) / sizeof((x)[0]))
 
char usart_rx_dma_buffer[10];
 
int main(void) 
{
	
    	/* MCU Configuration */
	MPU_Config();
	
    	/* Reset of all peripherals, Initializes the Flash interface and the Systick. */
    	LL_APB4_GRP1_EnableClock(LL_APB4_GRP1_PERIPH_SYSCFG);
	
    	NVIC_SetPriorityGrouping(NVIC_PRIORITYGROUP_4);
	
    	/* Configure the system clock */
    	SystemClock_Config();
	
    	/* Initialize all configured peripherals */
    	usart_init();
    	usart_send_string("DMA HT & TC + USART IDLE LINE interrupts\r\n");
    	//usart_send_string("Start sending data to STM32\r\n");
	
    	/* Infinite loop */
    	while (1)
	{
			
   	}
}
 
 
void usart_init(void) 
{
    	LL_USART_InitTypeDef USART_InitStruct = {0};
    	LL_GPIO_InitTypeDef GPIO_InitStruct = {0};
	
	LL_RCC_SetUSARTClockSource(LL_RCC_USART16_CLKSOURCE_PCLK2);
	
    	/* Peripheral clock enable */
    	LL_APB2_GRP1_EnableClock(LL_APB2_GRP1_PERIPH_USART1);
	LL_AHB4_GRP1_EnableClock(LL_AHB4_GRP1_PERIPH_GPIOB);
	
	/**USART1 GPIO Configuration
	PB14   ------> USART1_TX
	PB15   ------> USART1_RX
	*/
	
    	GPIO_InitStruct.Pin = LL_GPIO_PIN_14|LL_GPIO_PIN_15;
	GPIO_InitStruct.Mode = LL_GPIO_MODE_ALTERNATE;
	GPIO_InitStruct.Speed = LL_GPIO_SPEED_FREQ_HIGH;
	GPIO_InitStruct.OutputType = LL_GPIO_OUTPUT_PUSHPULL;
	GPIO_InitStruct.Pull = LL_GPIO_PULL_NO;
	GPIO_InitStruct.Alternate = LL_GPIO_AF_4;
	LL_GPIO_Init(GPIOB, &GPIO_InitStruct);
	
    	/* USART1 DMA Init for RX */
	
    	LL_DMA_SetPeriphRequest(DMA1, LL_DMA_STREAM_0, LL_DMAMUX1_REQ_USART1_RX);
	
	LL_DMA_SetDataTransferDirection(DMA1, LL_DMA_STREAM_0, LL_DMA_DIRECTION_PERIPH_TO_MEMORY);
	LL_DMA_SetStreamPriorityLevel(DMA1, LL_DMA_STREAM_0, LL_DMA_PRIORITY_HIGH);
	LL_DMA_SetMode(DMA1, LL_DMA_STREAM_0, LL_DMA_MODE_CIRCULAR);
	LL_DMA_SetPeriphIncMode(DMA1, LL_DMA_STREAM_0, LL_DMA_PERIPH_NOINCREMENT);
	LL_DMA_SetMemoryIncMode(DMA1, LL_DMA_STREAM_0, LL_DMA_MEMORY_INCREMENT);
	LL_DMA_SetPeriphSize(DMA1, LL_DMA_STREAM_0, LL_DMA_PDATAALIGN_BYTE);
	LL_DMA_SetMemorySize(DMA1, LL_DMA_STREAM_0, LL_DMA_MDATAALIGN_BYTE);
	LL_DMA_DisableFifoMode(DMA1, LL_DMA_STREAM_0);
	
    	LL_DMA_SetPeriphAddress(DMA1, LL_DMA_STREAM_0, LL_USART_DMA_GetRegAddr(USART1, LL_USART_DMA_REG_DATA_RECEIVE ));
    	LL_DMA_SetMemoryAddress(DMA1, LL_DMA_STREAM_0, (uint32_t)usart_rx_dma_buffer);
    	LL_DMA_SetDataLength(DMA1, LL_DMA_STREAM_0, ARRAY_LEN(usart_rx_dma_buffer));
	
	/* DMA interrupt init */
	NVIC_SetPriority(DMA1_Stream0_IRQn, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 0, 0));
	NVIC_EnableIRQ(DMA1_Stream0_IRQn);
	    
	/* USART configuration */
	USART_InitStruct.BaudRate = 115200;
	USART_InitStruct.DataWidth = LL_USART_DATAWIDTH_8B;
	USART_InitStruct.StopBits = LL_USART_STOPBITS_1;
	USART_InitStruct.Parity = LL_USART_PARITY_NONE;
	USART_InitStruct.TransferDirection = LL_USART_DIRECTION_TX_RX;
	USART_InitStruct.HardwareFlowControl = LL_USART_HWCONTROL_NONE;
	USART_InitStruct.OverSampling = LL_USART_OVERSAMPLING_16;
	LL_USART_Init(USART1, &USART_InitStruct);
	    
	LL_USART_ConfigAsyncMode(USART1);
	LL_USART_EnableDMAReq_RX(USART1);
	LL_USART_EnableIT_IDLE(USART1);
	    
	/* USART interrupt */
	NVIC_SetPriority(USART1_IRQn, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 0, 0));
	NVIC_EnableIRQ(USART1_IRQn);
	    
	/* Enable USART and DMA */
	LL_USART_Enable(USART1);
	LL_DMA_EnableStream(DMA1, LL_DMA_STREAM_0);
	
	/* Enable HT & TC interrupts */
	//LL_DMA_EnableIT_HT(DMA1, LL_DMA_STREAM_0);
	LL_DMA_EnableIT_TC(DMA1, LL_DMA_STREAM_0);
	LL_DMA_EnableIT_TE(DMA1, LL_DMA_STREAM_0);
}
 
void DMA1_Stream0_IRQHandler(void) 
{
	/* Check transfer-complete interrupt */
	if(LL_DMA_IsActiveFlag_TC0(DMA1)) {
	    LL_DMA_ClearFlag_TC0(DMA1);             /* Clear transfer complete flag */
	    usart_rx_check();                       /* Check for data to process */
	}
	/* Check transfer-error interrupt */
	if (LL_DMA_IsActiveFlag_TE0(DMA1)) {
	    LL_DMA_ClearFlag_TC0(DMA1);             /* Clear transfer complete flag */
	    usart_rx_check();                       /* Check for data to process */
	}
	  
    /* Implement other events when needed */
}
 
void USART1_IRQHandler(void) 
{
	/* Check for IDLE line interrupt */
	if (LL_USART_IsEnabledIT_IDLE(USART1) && LL_USART_IsActiveFlag_IDLE(USART1)) {
		LL_USART_ClearFlag_IDLE(USART1);        /* Clear IDLE line flag */
		usart_rx_check();                       /* Check for data to process */
	}
}
 
void SystemClock_Config(void) 
{
	
    	/* Configure flash latency */
    	LL_FLASH_SetLatency(LL_FLASH_LATENCY_4);
	while(LL_FLASH_GetLatency()!= LL_FLASH_LATENCY_4)
	{
	}
	LL_PWR_ConfigSupply(LL_PWR_DIRECT_SMPS_SUPPLY);
	LL_PWR_SetRegulVoltageScaling(LL_PWR_REGU_VOLTAGE_SCALE0);
	LL_RCC_HSE_Enable();
	
	/* Wait till HSE is ready */
	while(LL_RCC_HSE_IsReady() != 1)
	{
		
	}
	LL_RCC_PLL_SetSource(LL_RCC_PLLSOURCE_HSE);
	LL_RCC_PLL1P_Enable();
	LL_RCC_PLL1_SetVCOInputRange(LL_RCC_PLLINPUTRANGE_8_16);
	LL_RCC_PLL1_SetVCOOutputRange(LL_RCC_PLLVCORANGE_WIDE);
	LL_RCC_PLL1_SetM(1);
	LL_RCC_PLL1_SetN(120);
	LL_RCC_PLL1_SetP(2);
	LL_RCC_PLL1_SetQ(2);
	LL_RCC_PLL1_SetR(2);
	LL_RCC_PLL1_Enable();
	
	/* Wait till PLL is ready */
	while(LL_RCC_PLL1_IsReady() != 1)
	{
	}
	
	/* Intermediate AHB prescaler 2 when target frequency clock is higher than 80 MHz */
	LL_RCC_SetAHBPrescaler(LL_RCC_AHB_DIV_2);
	
	LL_RCC_SetSysClkSource(LL_RCC_SYS_CLKSOURCE_PLL1);
	
	/* Wait till System clock is ready */
	while(LL_RCC_GetSysClkSource() != LL_RCC_SYS_CLKSOURCE_STATUS_PLL1)
	{
		
	}
	LL_RCC_SetSysPrescaler(LL_RCC_SYSCLK_DIV_1);
	LL_RCC_SetAHBPrescaler(LL_RCC_AHB_DIV_2);
	LL_RCC_SetAPB1Prescaler(LL_RCC_APB1_DIV_2);
	LL_RCC_SetAPB2Prescaler(LL_RCC_APB2_DIV_2);
	LL_RCC_SetAPB3Prescaler(LL_RCC_APB3_DIV_2);
	LL_RCC_SetAPB4Prescaler(LL_RCC_APB4_DIV_2);
	
	LL_Init1msTick(480000000);
	
	LL_SetSystemCoreClock(480000000);
}
 
/** MPU Configuration **/
void MPU_Config(void)
{
	/* Disables the MPU */
	LL_MPU_Disable();
	
	// Initializes and configures the RAM Region and the memory to be protected 
  	LL_MPU_ConfigRegion(LL_MPU_REGION_NUMBER0, 0x0, 0x24000000, LL_MPU_TEX_LEVEL0|LL_MPU_REGION_SIZE_8KB|LL_MPU_REGION_FULL_ACCESS|LL_MPU_INSTRUCTION_ACCESS_ENABLE|LL_MPU_ACCESS_SHAREABLE|LL_MPU_ACCESS_CACHEABLE|LL_MPU_ACCESS_NOT_BUFFERABLE);
  	LL_MPU_EnableRegion(LL_MPU_REGION_NUMBER0);
	
  	// Initializes and configures the Flash Region and the memory to be protected
  	LL_MPU_ConfigRegion(LL_MPU_REGION_NUMBER1, 0x0, 0x08000000, LL_MPU_TEX_LEVEL0|LL_MPU_REGION_SIZE_1MB|LL_MPU_REGION_FULL_ACCESS|LL_MPU_INSTRUCTION_ACCESS_ENABLE|LL_MPU_ACCESS_NOT_SHAREABLE|LL_MPU_ACCESS_CACHEABLE|LL_MPU_ACCESS_NOT_BUFFERABLE);
  	LL_MPU_EnableRegion(LL_MPU_REGION_NUMBER1);
	
  	//Initializes and configures the FMC Region and the memory to be protected
  	LL_MPU_ConfigRegion(LL_MPU_REGION_NUMBER2, 0x0, 0x60000000, LL_MPU_TEX_LEVEL0|LL_MPU_REGION_SIZE_512MB|LL_MPU_REGION_FULL_ACCESS|LL_MPU_INSTRUCTION_ACCESS_ENABLE|LL_MPU_ACCESS_SHAREABLE|LL_MPU_ACCESS_NOT_CACHEABLE|LL_MPU_ACCESS_BUFFERABLE);
  	LL_MPU_EnableRegion(LL_MPU_REGION_NUMBER2);
	
	/* Enables the MPU */
	LL_MPU_Enable(LL_MPU_CTRL_PRIVILEGED_DEFAULT);
	
}

[Imen.D]

Hello @veera​ and welcome to the Community :)

Maybe it's cache issues: DMA is not working on STM32H7 devices

Imen

[Tesla DeLorean]

Pay attention to where the buffer memory is situated, and that you're actively manager for cache-coherency where appropriate.

Remember also that the cache is managed on 32-byte lines, and you really want to avoid spanning lines front and back of the buffer, and other data you store/park in those regions. Those can suffer collateral damage if you're inattentive. 32-line alignment, and size multiples, are strongly suggested.

[veera1]

Thank you for answering.. I have made modifications as per the suggestions provided by you. I have placed the buffer at RAM3 which is accessible by DMA1 and I have taken care of cache management but still I'm receiving null data through the DMA. I have attached my code please have a look and suggest me what are the things I need to take care of and what else I have to do.

/* Includes */
#include "main.h"
#include <string.h>
 
/* Buffer used for reception */
/* Buffer location and size should aligned to cache line size (32 bytes) */
/***************
#if defined ( __ICCARM__ )
#pragma location = 0x38000000
#elif defined ( __CC_ARM )
uint8_t __attribute__((section (".RAM_D3"))) usart_rx_dma_buffer[BUFFER_SIZE]);
#elif defined ( __GNUC__ )
uint8_t __attribute__((section (".RAM_D3"))) usart_rx_dma_buffer[BUFFER_SIZE]);
#endif 
****************/
/* Macro to get variable aligned on 32-bytes,needed for cache maintenance purpose */
 #if defined   (__GNUC__)        /* GNU Compiler */
 #define ALIGN_32BYTES(buf)  buf __attribute__ ((aligned (32)))
 #elif defined (__ICCARM__)    /* IAR Compiler */
 #define ALIGN_32BYTES(buf) _Pragma("data_alignment=32") buf
 #elif defined   (__CC_ARM)      /* ARM Compiler */
 #define ALIGN_32BYTES(buf) __align(32) buf
 #endif
 
//length of statically allocated array
#define ARRAY_LEN(x)            (sizeof(x) / sizeof((x)[0]))
 
#define BUFFER_SIZE 			10
 
ALIGN_32BYTES(static char usart_rx_dma_buffer[BUFFER_SIZE]);
 
 
/* Private function prototypes */
void SystemClock_Config(void);
void CPU_CACHE_Enable(void);
static void MPU_Config(void);
 
/* USART related functions */
void usart_init(void);
void dma_init(void);
void usart_rx_check(void);
void usart_process_data(const void* data, size_t len);
void usart_send_string(const char* str);
 
 
int main(void) 
{
	/* Enable the CPU Cache */ 
  	CPU_CACHE_Enable();
	
    	/* MCU Configuration */
	MPU_Config();
	
    	/* Reset of all peripherals, Initializes the Flash interface and the Systick. */
    	LL_APB4_GRP1_EnableClock(LL_APB4_GRP1_PERIPH_SYSCFG);
	
    	NVIC_SetPriorityGrouping(NVIC_PRIORITYGROUP_4);
	
    	/* Configure the system clock */
    	SystemClock_Config();
	
    	/* Initialize all configured peripherals */
    	usart_init();
	dma_init();
	
    	usart_send_string("DMA HT TC  USART IDLE LINE interrupts\r\n");
    	//usart_send_string("Start sending data to STM32\r\n");
	
    	/* Infinite loop */
    	while (1)
	{
		LL_mDelay(2000);
   	}
}
 
 
void usart_init(void) 
{
    	LL_USART_InitTypeDef USART_InitStruct = {0};
    	LL_GPIO_InitTypeDef GPIO_InitStruct = {0};
	
	LL_RCC_SetUSARTClockSource(LL_RCC_USART16_CLKSOURCE_PCLK2);
	
    	/* Peripheral clock enable */
    	LL_APB2_GRP1_EnableClock(LL_APB2_GRP1_PERIPH_USART1);
	LL_AHB4_GRP1_EnableClock(LL_AHB4_GRP1_PERIPH_GPIOB);
	
	/**USART1 GPIO Configuration
	PB14   ------> USART1_TX
	PB15   ------> USART1_RX
	*/
	
    	GPIO_InitStruct.Pin			= LL_GPIO_PIN_14|LL_GPIO_PIN_15;
	GPIO_InitStruct.Mode			= LL_GPIO_MODE_ALTERNATE;
	GPIO_InitStruct.Speed 			= LL_GPIO_SPEED_FREQ_HIGH;
	GPIO_InitStruct.OutputType 		= LL_GPIO_OUTPUT_PUSHPULL;
	GPIO_InitStruct.Pull 			= LL_GPIO_PULL_NO;
	GPIO_InitStruct.Alternate 		= LL_GPIO_AF_4;
	LL_GPIO_Init(GPIOB, &GPIO_InitStruct);
	
	/* USART configuration */
	USART_InitStruct.BaudRate 		= 115200;
	USART_InitStruct.DataWidth 		= LL_USART_DATAWIDTH_8B;
	USART_InitStruct.StopBits 		= LL_USART_STOPBITS_1;
	USART_InitStruct.Parity 		= LL_USART_PARITY_NONE;
	USART_InitStruct.TransferDirection 	= LL_USART_DIRECTION_TX_RX;
	USART_InitStruct.HardwareFlowControl 	= LL_USART_HWCONTROL_NONE;
	USART_InitStruct.OverSampling 		= LL_USART_OVERSAMPLING_16;
	LL_USART_Init(USART1, &USART_InitStruct);
	
	LL_USART_ConfigAsyncMode(USART1);
	
	/* Configure NVIC for UART and interrupts */
	NVIC_SetPriority(USART1_IRQn, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 0, 0));
	NVIC_EnableIRQ(USART1_IRQn);
	
	// Enable Idle interrupt
	LL_USART_EnableIT_IDLE(USART1);
	
	/* Enable USART */
	LL_USART_Enable(USART1);
	
	// Enable DMA Rx interrupt
	LL_USART_EnableDMAReq_RX(USART1);
	
	/* Polling USART1 initialization */
    	while (!LL_USART_IsActiveFlag_TEACK(USART1) || !LL_USART_IsActiveFlag_REACK(USART1)) {}
}
 
void dma_init(void)
{
	LL_DMA_InitTypeDef dma_initstruct = {0};
	
	/* Enable the clock of DMA1 */
  	LL_AHB1_GRP1_EnableClock(LL_AHB1_GRP1_PERIPH_DMA1);
	
	/* Configure the DMA functional parameters for Rx*/
	//Set fields of initialization structure:
	dma_initstruct.PeriphOrM2MSrcAddress  = LL_USART_DMA_GetRegAddr(USART1, LL_USART_DMA_REG_DATA_RECEIVE);
	dma_initstruct.MemoryOrM2MDstAddress  = ((uint32_t)usart_rx_dma_buffer);
	dma_initstruct.Direction              = LL_DMA_DIRECTION_PERIPH_TO_MEMORY;
	dma_initstruct.Mode                   = LL_DMA_MODE_CIRCULAR;
	dma_initstruct.PeriphOrM2MSrcIncMode  = LL_DMA_PERIPH_INCREMENT;
	dma_initstruct.MemoryOrM2MDstIncMode  = LL_DMA_MEMORY_INCREMENT;
	dma_initstruct.PeriphOrM2MSrcDataSize = LL_DMA_PDATAALIGN_BYTE;
	dma_initstruct.MemoryOrM2MDstDataSize = LL_DMA_MDATAALIGN_BYTE;
	dma_initstruct.NbData                 = BUFFER_SIZE;
	dma_initstruct.PeriphRequest          = LL_DMAMUX1_REQ_USART1_RX;
	dma_initstruct.Priority               = LL_DMA_PRIORITY_HIGH;
	dma_initstruct.FIFOMode               = LL_DMA_FIFOMODE_DISABLE;
	dma_initstruct.FIFOThreshold          = LL_DMA_FIFOTHRESHOLD_FULL;
	dma_initstruct.MemBurst               = LL_DMA_MBURST_INC4;
	dma_initstruct.PeriphBurst            = LL_DMA_PBURST_INC4;
	
	/* Initialize DMA instance according to parameters defined in initialization structure. */
	LL_DMA_Init(DMA1, LL_DMA_STREAM_0, &dma_initstruct);
	
	/* Configure NVIC for DMA transfer complete/error interrupts */
	NVIC_SetPriority(DMA1_Stream0_IRQn, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 0, 0));
	NVIC_EnableIRQ(DMA1_Stream0_IRQn);
	
	/* Enable HT, TC and TE interrupts */
	LL_DMA_EnableIT_HT(DMA1, LL_DMA_STREAM_0);
  	LL_DMA_EnableIT_TC(DMA1, LL_DMA_STREAM_0);
  	LL_DMA_EnableIT_TE(DMA1, LL_DMA_STREAM_0);
	
	/*CPU Data Cache maintenance :
      	It is recommended to invalidate the CPU Data cache after the DMA transfer.
      	As the destination buffer may be used by the CPU, this guarantees Up-to-date data when CPU accesses
      	to the destination buffer located in the AXI-SRAM (which is cacheable).
     	*/
	
     	//SCB_InvalidateDCache_by_Addr((uint32_t *) usart_rx_dma_buffer, sizeof(usart_rx_dma_buffer) );
	
	/* Start the DMA transfer */
  	LL_DMA_EnableStream(DMA1, LL_DMA_STREAM_0);
}
 
void usart_process_data(const void* data, size_t len) 
{
	const uint8_t* d = data;
	for (; len > 0; --len, ++d) 
	{
		LL_USART_TransmitData8(USART1, *d);
		
		// wait until TXE flag is raised
		while (!LL_USART_IsActiveFlag_TXE(USART1)) {}
	}
	while (!LL_USART_IsActiveFlag_TC(USART1)) {}
}
 
 
void usart_send_string(const char* str) 
{
    	usart_process_data(str, strlen(str));
}
 
 
void DMA1_Stream0_IRQHandler(void) 
{
	/* Check half-transfer complete interrupt */
    	if (LL_DMA_IsEnabledIT_HT(DMA1, LL_DMA_STREAM_0) && LL_DMA_IsActiveFlag_HT0(DMA1)) {
        	LL_DMA_ClearFlag_HT0(DMA1);             /* Clear half-transfer complete flag */
        	usart_rx_check();                       /* Check for data to process */
    	}
	/* Check transfer-complete interrupt */
	if(LL_DMA_IsActiveFlag_TC0(DMA1)) {
		LL_DMA_ClearFlag_TC0(DMA1);             /* Clear transfer complete flag */
	    	usart_rx_check();                       /* Check for data to process */
	}
	/* Check transfer-error interrupt */
	if (LL_DMA_IsActiveFlag_TE0(DMA1)) {
	    	LL_DMA_ClearFlag_TE0(DMA1);             /* Clear transfer complete flag */
	    	usart_rx_check();                       /* Check for data to process */
	}
}
 
void USART1_IRQHandler(void) 
{
	/* Check for IDLE line interrupt */
	if (LL_USART_IsEnabledIT_IDLE(USART1) && LL_USART_IsActiveFlag_IDLE(USART1)) {
		LL_USART_ClearFlag_IDLE(USART1);        /* Clear IDLE line flag */
		usart_rx_check();                       /* Check for data to process */
	}
}
 
/* CPU L1-Cache enable */
void CPU_CACHE_Enable(void)
{
	/* Enable I-Cache */
	SCB_EnableICache();
	
	/* Enable D-Cache */
      	SCB_EnableDCache();
}
 
void usart_rx_check(void) 
{	
	static size_t old_pos;
	size_t pos;
	    
	/* Calculate current position in buffer and check for new data available */
	pos = ARRAY_LEN(usart_rx_dma_buffer) - LL_DMA_GetDataLength(DMA1, LL_DMA_STREAM_0);
	if (pos != old_pos) {                       /* Check change in received data */
	    if (pos > old_pos) {                    /* Current position is over previous one */
		usart_process_data(&usart_rx_dma_buffer[old_pos], pos - old_pos);
	    } else {
		usart_process_data(&usart_rx_dma_buffer[old_pos], 
                ARRAY_LEN(usart_rx_dma_buffer) - old_pos);
		if (pos > 0) {
		    usart_process_data(&usart_rx_dma_buffer[0], pos);
		}
	    }
	    old_pos = pos;                          /* Save current position as old for next transfers */
	}
 }

[veera1]

Thank you for answering..

I have made modifications as per the suggestions provided by you. I have placed the buffer at RAM3 which is accessible by DMA1 and I have taken care of cache management but still I'm receiving null data through the DMA. I have attached my code please have a look and suggest me what are the things I need to take care of and what else I have to do. Code posted below as a answer please have a look