[coreboot] [patch][v2] AMD Fam10 HT cleanup

Peter Stuge peter at stuge.se
Thu Jul 17 01:15:27 CEST 2008


On Wed, Jul 16, 2008 at 04:06:52PM -0600, Marc Jones wrote:
> Clean up AMD FAM10 HT variable initialization. The structure init
> is cleaner, avoid compiler warnings, and matches the AMD example
> code more closely.
> 
> Signed-off-by: Marc Jones <marc.jones at amd.com>

It bothers me a bit that there are so many identical lines.
Maybe those event types could use some *_INIT_* macros. Oh well. :)

Acked-by: Peter Stuge <peter at stuge.se>


> Index: coreboot-v2/src/northbridge/amd/amdht/h3finit.c
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdht/h3finit.c	2008-07-16 15:35:11.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdht/h3finit.c	2008-07-15 15:48:21.000000000 -0600
> @@ -112,7 +112,7 @@
>   */
>  
>  /*----------------------------------------------------------------------------------------
> - * int
> + * u8
>   * graphHowManyNodes(u8 *graph)
>   *
>   *  Description:
> @@ -123,7 +123,7 @@
>   *	@param[out] u8 results = the number of nodes in the graph
>   * ---------------------------------------------------------------------------------------
>   */
> -int graphHowManyNodes(u8 *graph)
> +u8 graphHowManyNodes(u8 *graph)
>  {
>  	return graph[0];
>  }
> @@ -198,7 +198,7 @@
>   */
>  u8 graphGetReq(u8 *graph, u8 nodeA, u8 nodeB)
>  {
> -	int size = graph[0];
> +	u8 size = graph[0];
>  	ASSERT(size <= MAX_NODES);
>  	ASSERT((nodeA < size) && (nodeB < size));
>  	return (graph[1+(nodeA*size+nodeB)*2+1] & 0x0F);
> @@ -206,7 +206,7 @@
>  
>  /*----------------------------------------------------------------------------------------
>   * u8
> - * graphGetBc(unsigned char *graph, int nodeA, int nodeB)
> + * graphGetBc(u8 *graph, u8 nodeA, u8 nodeB)
>   *
>   *  Description:
>   *	 Returns a bit vector of nodes that nodeA should forward a broadcast from
> @@ -219,9 +219,9 @@
>   *	OU    u8    results = the broadcast routes for nodeA from nodeB
>   * ---------------------------------------------------------------------------------------
>   */
> -u8 graphGetBc(unsigned char *graph, int nodeA, int nodeB)
> +u8 graphGetBc(u8 *graph, u8 nodeA, u8 nodeB)
>  {
> -	int size = graph[0];
> +	u8 size = graph[0];
>  	ASSERT(size <= MAX_NODES);
>  	ASSERT((nodeA < size) && (nodeB < size));
>  	return graph[1+(nodeA*size+nodeB)*2];
> @@ -415,10 +415,11 @@
>  				/* Notify BIOS of event (while variables are still the same) */
>  				if (pDat->HtBlock->AMD_CB_EventNotify)
>  				{
> -					sHtEventCohFamilyFeud evt = {sizeof(sHtEventCohFamilyFeud),
> -								currentNode,
> -								currentLink,
> -								pDat->NodesDiscovered};
> +					sHtEventCohFamilyFeud evt;
> +					evt.eSize = sizeof(sHtEventCohFamilyFeud);
> +					evt.node = currentNode;
> +					evt.link = currentLink;
> +					evt.totalNodes = pDat->NodesDiscovered;
>  
>  					pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_ERROR,
>  									HT_EVENT_COH_FAMILY_FEUD,
> @@ -470,11 +471,12 @@
>  					/* Notify BIOS of event  */
>  					if (pDat->HtBlock->AMD_CB_EventNotify)
>  					{
> -						sHtEventCohMpCapMismatch evt = {sizeof(sHtEventCohMpCapMismatch),
> -									currentNode,
> -									currentLink,
> -									pDat->sysMpCap,
> -									pDat->NodesDiscovered};
> +						sHtEventCohMpCapMismatch evt;
> +						evt.eSize = sizeof(sHtEventCohMpCapMismatch);
> +						evt.node = currentNode;
> +						evt.link = currentLink;
> +						evt.sysMpCap = pDat->sysMpCap;
> +						evt.totalNodes = pDat->NodesDiscovered;
>  
>  						pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_ERROR,
>  									HT_EVENT_COH_MPCAP_MISMATCH,
> @@ -502,10 +504,11 @@
>  				 */
>  				if (pDat->HtBlock->AMD_CB_EventNotify)
>  				{
> -					sHtEventCohNodeDiscovered evt = {sizeof(sHtEventCohNodeDiscovered),
> -								currentNode,
> -								currentLink,
> -								token};
> +					sHtEventCohNodeDiscovered evt;
> +					evt.eSize = sizeof(sHtEventCohNodeDiscovered);
> +					evt.node = currentNode;
> +					evt.link = currentLink;
> +					evt.newNode = token;
>  
>  					pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_INFO,
>  								HT_EVENT_COH_NODE_DISCOVERED,
> @@ -527,12 +530,13 @@
>  				 */
>  				if (pDat->HtBlock->AMD_CB_EventNotify)
>  				{
> -					sHtEventCohLinkExceed evt = {sizeof(sHtEventCohLinkExceed),
> -								currentNode,
> -								currentLink,
> -								token,
> -								pDat->NodesDiscovered,
> -								pDat->nb->maxLinks};
> +					sHtEventCohLinkExceed evt;
> +					evt.eSize = sizeof(sHtEventCohLinkExceed);
> +					evt.node = currentNode;
> +					evt.link = currentLink;
> +					evt.targetNode = token;
> +					evt.totalNodes = pDat->NodesDiscovered;
> +					evt.maxLinks = pDat->nb->maxLinks;
>  
>  					pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_ERROR,
>  									HT_EVENT_COH_LINK_EXCEED,
> @@ -776,8 +780,9 @@
>  		 */
>  		if (pDat->HtBlock->AMD_CB_EventNotify)
>  		{
> -			sHtEventCohNoTopology evt = {sizeof(sHtEventCohNoTopology),
> -							pDat->NodesDiscovered};
> +			sHtEventCohNoTopology evt;
> +			evt.eSize = sizeof(sHtEventCohNoTopology);
> +			evt.totalNodes = pDat->NodesDiscovered;
>  
>  			pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_ERROR,
>  						HT_EVENT_COH_NO_TOPOLOGY,
> @@ -840,7 +845,7 @@
>   */
>  void coherentInit(sMainData *pDat)
>  {
> -	int i, j;
> +	u8 i, j;
>  
>  #ifdef HT_BUILD_NC_ONLY
>  	/* Replace discovery process with:
> @@ -912,7 +917,11 @@
>  			 */
>  			if (pDat->HtBlock->AMD_CB_EventNotify)
>  			{
> -				sHTEventNcohBusMaxExceed evt = {sizeof(sHTEventNcohBusMaxExceed), node, link, pDat->AutoBusCurrent};
> +				sHTEventNcohBusMaxExceed evt;
> +				evt.eSize = sizeof(sHTEventNcohBusMaxExceed);
> +				evt.node = node;
> +				evt.link = link;
> +				evt.bus = pDat->AutoBusCurrent;
>  
>  				pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_ERROR,HT_EVENT_NCOH_BUS_MAX_EXCEED,(u8 *)&evt);
>  			}
> @@ -927,7 +936,10 @@
>  			 */
>  			if (pDat->HtBlock->AMD_CB_EventNotify)
>  			{
> -				sHtEventNcohCfgMapExceed evt = {sizeof(sHtEventNcohCfgMapExceed), node, link};
> +				sHtEventNcohCfgMapExceed evt;
> +				evt.eSize = sizeof(sHtEventNcohCfgMapExceed);
> +				evt.node = node;
> +				evt.link = link;
>  
>  				pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_ERROR,
>  							HT_EVENT_NCOH_CFG_MAP_EXCEED,
> @@ -1049,11 +1061,12 @@
>  				 */
>  				if (pDat->HtBlock->AMD_CB_EventNotify)
>  				{
> -					sHtEventNcohLinkExceed evt = {sizeof(sHtEventNcohLinkExceed),
> -							node,
> -							link,
> -							depth,
> -							pDat->nb->maxLinks};
> +					sHtEventNcohLinkExceed evt;
> +					evt.eSize = sizeof(sHtEventNcohLinkExceed);
> +					evt.node = node;
> +					evt.link = link;
> +					evt.depth = depth;
> +					evt.maxLinks = pDat->nb->maxLinks;
>  
>  					pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_ERROR,
>  								HT_EVENT_NCOH_LINK_EXCEED,
> @@ -1097,8 +1110,13 @@
>  				/* An error handler for the case where we run out of BUID's on a chain */
>  				if (pDat->HtBlock->AMD_CB_EventNotify)
>  				{
> -					sHtEventNcohBuidExceed evt = {sizeof(sHtEventNcohBuidExceed),
> -								node, link, depth, (u8)currentBUID, (u8)unitIDcnt};
> +					sHtEventNcohBuidExceed evt;
> +					evt.eSize = sizeof(sHtEventNcohBuidExceed);
> +					evt.node = node;
> +					evt.link = link;
> +					evt.depth = depth;
> +					evt.currentBUID = (uint8)currentBUID;
> +					evt.unitCount = (uint8)unitIDcnt;
>  
>  					pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_ERROR,HT_EVENT_NCOH_BUID_EXCEED,(u8 *)&evt);
>  				}
> @@ -1115,8 +1133,12 @@
>  				/* An error handler for this critical error */
>  				if (pDat->HtBlock->AMD_CB_EventNotify)
>  				{
> -					sHtEventNcohDeviceFailed evt = {sizeof(sHtEventNcohDeviceFailed),
> -							node, link, depth, (u8)currentBUID};
> +					sHtEventNcohDeviceFailed evt;
> +					evt.eSize = sizeof(sHtEventNcohDeviceFailed);
> +					evt.node = node;
> +					evt.link = link;
> +					evt.depth = depth;
> +					evt.attemptedBUID = (uint8)currentBUID;
>  
>  					pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_ERROR,HT_EVENT_NCOH_DEVICE_FAILED,(u8 *)&evt);
>  				}
> @@ -1138,7 +1160,11 @@
>  		if (pDat->HtBlock->AMD_CB_EventNotify)
>  		{
>  			/* Provide information on automatic device results */
> -			sHtEventNcohAutoDepth evt = {sizeof(sHtEventNcohAutoDepth), node, link, (depth - 1)};
> +			sHtEventNcohAutoDepth evt;
> +			evt.eSize = sizeof(sHtEventNcohAutoDepth);
> +			evt.node = node;
> +			evt.link = link;
> +			evt.depth = (depth - 1);
>  
>  			pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_INFO,HT_EVENT_NCOH_AUTO_DEPTH,(u8 *)&evt);
>  		}
> Index: coreboot-v2/src/northbridge/amd/amdht/h3ncmn.c
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdht/h3ncmn.c	2008-07-16 15:35:46.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdht/h3ncmn.c	2008-07-15 15:48:21.000000000 -0600
> @@ -401,7 +401,11 @@
>  				if (pDat->HtBlock->AMD_CB_EventNotify)
>  				{
>  					/* Pass the node and link on which the generic synch flood event occurred. */
> -					sHtEventHWHtCrc evt = {sizeof(sHtEventHWHtCrc), node, link, (u8)crc};
> +					sHtEventHWHtCrc evt;
> +					evt.eSize = sizeof(sHtEventHWHtCrc);
> +					evt.node = node;
> +					evt.link = link;
> +					evt.laneMask = (uint8)crc;
>  
>  					pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_HW_FAULT,
>  									HT_EVENT_HW_HTCRC,
> @@ -414,7 +418,10 @@
>  				if (pDat->HtBlock->AMD_CB_EventNotify)
>  				{
>  					/* Pass the node and link on which the generic synch flood event occurred. */
> -					sHtEventHWSynchFlood evt = {sizeof(sHtEventHWSynchFlood), node, link};
> +					sHtEventHWSynchFlood evt;
> +					evt.eSize = sizeof(sHtEventHWSynchFlood);
> +					evt.node = node;
> +					evt.link = link;
>  
>  					pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_HW_FAULT,
>  									HT_EVENT_HW_SYNCHFLOOD,
> @@ -1112,8 +1119,6 @@
>  		return 2;
>  	}
>  	STOP_HERE; /*  This is an error internal condition */
> -
> -	return 0xFF;	// make the compiler happy.
>  }
>  
>  /**----------------------------------------------------------------------------------------
> @@ -1143,8 +1148,6 @@
>  		return 4;
>  	}
>  	STOP_HERE; /*  This is an internal error condition */
> -
> -	return 0xFF;	// make the compiler happy.
>  }
>  
>  /**----------------------------------------------------------------------------------------
> @@ -1468,10 +1471,11 @@
>  					{
>  						if (pDat->HtBlock->AMD_CB_EventNotify)
>  						{
> -							sHtEventOptRequiredCap evt ={sizeof(sHtEventOptRequiredCap),
> -										pDat->PortList[i].NodeID,
> -										pDat->PortList[i].HostLink,
> -										pDat->PortList[i].HostDepth};
> +							sHtEventOptRequiredCap evt;
> +							evt.eSize = sizeof(sHtEventOptRequiredCap);
> +							evt.node = pDat->PortList[i].NodeID;
> +							evt.link = pDat->PortList[i].HostLink;
> +							evt.depth = pDat->PortList[i].HostDepth;
>  
>  							pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_WARNING,
>  										HT_EVENT_OPT_REQUIRED_CAP_RETRY,
> @@ -1513,10 +1517,11 @@
>  					{
>  						if (pDat->HtBlock->AMD_CB_EventNotify)
>  						{
> -							sHtEventOptRequiredCap evt ={sizeof(sHtEventOptRequiredCap),
> -									pDat->PortList[i].NodeID,
> -									pDat->PortList[i].HostLink,
> -									pDat->PortList[i].HostDepth};
> +							sHtEventOptRequiredCap evt;
> +							evt.eSize = sizeof(sHtEventOptRequiredCap);
> +							evt.node = pDat->PortList[i].NodeID;
> +							evt.link = pDat->PortList[i].HostLink;
> +							evt.depth = pDat->PortList[i].HostDepth;
>  
>  							pDat->HtBlock->AMD_CB_EventNotify(HT_EVENT_CLASS_WARNING,
>  										HT_EVENT_OPT_REQUIRED_CAP_GEN3,




More information about the coreboot mailing list