Skip to content

Minor warning fixes #589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jan 4, 2023
Merged

Minor warning fixes #589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6cfc758
Eliminate compiler unused parameter warning
ChristosZosi Nov 24, 2022
43ab28b
Eliminate compiler unused variable warnings
ChristosZosi Nov 24, 2022
5665b9d
Eliminate compiler unused function warning
ChristosZosi Nov 24, 2022
9829705
Rework callback setups in the EMAC-driver of the Xilinx UltraScale port
ChristosZosi Nov 24, 2022
bf1b30b
Uncrustify: triggered by comment.
actions-user Nov 24, 2022
24d6a76
Update source/portable/NetworkInterface/xilinx_ultrascale/x_emacpsif_…
ChristosZosi Nov 28, 2022
ba1f887
Apply suggestions from code review
ChristosZosi Nov 28, 2022
2a01c42
Merge branch 'FreeRTOS:main' into warnings
ChristosZosi Nov 28, 2022
398ff8c
Merge branch 'main' into warnings
ChristosZosi Dec 6, 2022
773596f
Merge branch 'main' into warnings
ChristosZosi Dec 29, 2022
bd4c4e9
Address comments from reviews
ChristosZosi Dec 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions source/FreeRTOS_ICMP.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@
}
#else
{
/* Just to prevent compiler warnings about unused parameters. */
( void ) pxNetworkBuffer;

/* Many EMAC peripherals will only calculate the ICMP checksum
* correctly if the field is nulled beforehand. */
pxICMPHeader->usChecksum = 0U;
Expand Down
3 changes: 3 additions & 0 deletions source/portable/NetworkInterface/xilinx_ultrascale/x_emacpsif_dma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ int is_tx_space_available( xemacpsif_s * xemacpsif )
{
size_t uxCount;

/* Just to prevent compiler warnings about unused parameters. */
( void ) xemacpsif;

if( xTXDescriptorSemaphore != NULL )
{
uxCount = ( ( UBaseType_t ) ipconfigNIC_N_TX_DESC ) - uxSemaphoreGetCount( xTXDescriptorSemaphore );
Expand Down
17 changes: 8 additions & 9 deletions source/portable/NetworkInterface/xilinx_ultrascale/x_emacpsif_hw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,16 @@ void setup_isr( xemacpsif_s * xemacpsif )
/*
* Setup callbacks
*/
XEmacPs_SetHandler( &xemacpsif->emacps, XEMACPS_HANDLER_DMASEND,
( void * ) emacps_send_handler,
( void * ) xemacpsif );
Copy link
Contributor

@htibosch htibosch Dec 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this change: why would you remove the function calls and replace them with direct assignments?
Was there a problem with the functions? Or did it introduce a dependency on sources?

Copy link
Member

@AniruddhaKanhere AniruddhaKanhere Dec 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @ChristosZosi,
I agree with Hein here. I am also not sure why was this removed?

Copy link
Contributor Author

@ChristosZosi ChristosZosi Dec 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the commit message (9829705) I did add an explanation why the calls to the XEmacPs_SetHandler is problematic. In short, the compiler would output the pedantic warning ISO C forbids conversion of object pointer to function pointer type, when compiled with -Wpedantic.

The reason for it has to do with the function prototype of XEmacPs_SetHandler

LONG XEmacPs_SetHandler(XEmacPs *InstancePtr, u32 HandlerType,
			void *FuncPointer, void *CallBackRef);

Here you can see that the third parameter of the function is a void-pointer. However we pass to the function a function pointer. And here lies the issue, that function pointers (text) are not the same as void pointers (data). That is the reason for the compiler warning.

Here is also a nice discussion on stack overflow, which addresses this exact issue https://stackoverflow.com/questions/36645660/why-cant-i-cast-a-function-pointer-to-void

I hope this explains it 😄.

Copy link
Contributor

@htibosch htibosch Dec 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the URL and for the insights.

Actually I had the same problem when using setsockopt() which has a parameter void *optval. It was not allowed to pass a function pointer.

As a solution, I introduced a struct called F_TCP_UDP_Handler_t that holds function pointers.
So let's go for your solution, assigning pointers directly.

XEmacPs * xInstancePtr = &( xemacpsif->emacps );

XEmacPs_SetHandler( &xemacpsif->emacps, XEMACPS_HANDLER_DMARECV,
( void * ) emacps_recv_handler,
( void * ) xemacpsif );
xInstancePtr->SendHandler = emacps_send_handler;
xInstancePtr->SendRef = ( void * ) xemacpsif;

XEmacPs_SetHandler( &xemacpsif->emacps, XEMACPS_HANDLER_ERROR,
( void * ) emacps_error_handler,
( void * ) xemacpsif );
xInstancePtr->RecvHandler = emacps_recv_handler;
xInstancePtr->RecvRef = ( void * ) xemacpsif;

xInstancePtr->ErrorHandler = emacps_error_handler;
xInstancePtr->ErrorRef = ( void * ) xemacpsif;
}

void start_emacps( xemacpsif_s * xemacps )
Expand Down
61 changes: 8 additions & 53 deletions source/portable/NetworkInterface/xilinx_ultrascale/x_emacpsif_physpeed.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,18 +245,10 @@ void my_sleep( uint32_t uxTicks )
u32 phymapemac0[ 32 ];
u32 phymapemac1[ 32 ];

static uint16_t prvAR803x_debug_reg_read( XEmacPs * xemacpsp,
uint32_t phy_addr,
u16 reg );
static uint16_t prvAR803x_debug_reg_write( XEmacPs * xemacpsp,
uint32_t phy_addr,
u16 reg,
u16 value );
static int prvAR803x_debug_reg_mask( XEmacPs * xemacpsp,
uint32_t phy_addr,
u16 reg,
u16 clear,
u16 set );
static void prvSET_AR803x_TX_Timing( XEmacPs * xemacpsp,
uint32_t phy_addr );

Expand Down Expand Up @@ -832,29 +824,8 @@ static uint32_t get_AR8035_phy_speed( XEmacPs * xemacpsp,
}
}

static void ar8035Tick( XEmacPs * xemacpsp,
uint32_t phy_addr )
{
uint16_t value;
BaseType_t linkState;

/*Read basic status register */
value = XEmacPs_PhyRead2( xemacpsp, phy_addr, IEEE_STATUS_REG_OFFSET );
/*Retrieve current link state */
linkState = ( value & IEEE_STAT_LINK_STATUS ) ? TRUE : FALSE;
}

#define AR803X_DEBUG_ADDR 0x1D
#define AR803X_DEBUG_DATA 0x1E
static uint16_t prvAR803x_debug_reg_read( XEmacPs * xemacpsp,
uint32_t phy_addr,
u16 reg )
{
XEmacPs_PhyWrite( xemacpsp, phy_addr, AR803X_DEBUG_ADDR, reg );

return XEmacPs_PhyRead2( xemacpsp, phy_addr, AR803X_DEBUG_DATA );
}

static uint16_t prvAR803x_debug_reg_write( XEmacPs * xemacpsp,
uint32_t phy_addr,
u16 reg,
Expand All @@ -865,29 +836,6 @@ static uint16_t prvAR803x_debug_reg_write( XEmacPs * xemacpsp,
return XEmacPs_PhyWrite( xemacpsp, phy_addr, AR803X_DEBUG_DATA, value );
}

static int prvAR803x_debug_reg_mask( XEmacPs * xemacpsp,
uint32_t phy_addr,
u16 reg,
u16 clear,
u16 set )
{
u16 val;
int ret;

ret = prvAR803x_debug_reg_read( xemacpsp, phy_addr, reg );

if( ret < 0 )
{
return ret;
}

val = ret & 0xffff;
val &= ~clear;
val |= set;

return XEmacPs_PhyWrite( xemacpsp, phy_addr, AR803X_DEBUG_DATA, val );
}

static uint32_t ar8035CheckStatus( XEmacPs * xemacpsp,
uint32_t phy_addr )
{
Expand Down Expand Up @@ -970,6 +918,9 @@ static uint32_t ar8035CheckStatus( XEmacPs * xemacpsp,
/* nicNotifyLinkChange(interface); */
}

/* Just to prevent compiler warnings about unused variable. */
( void ) linkState;

return linkSpeed;
}

Expand Down Expand Up @@ -1015,7 +966,11 @@ static uint32_t get_IEEE_phy_speed_US( XEmacPs * xemacpsp,
XEmacPs_PhyRead( xemacpsp, phy_addr, PHY_IDENTIFIER_1_REG,
&phy_identity );

FreeRTOS_printf( ( "Start %s PHY autonegotiation. ID = 0x%04X\n", pcGetPHIName( phy_identity ), phy_identity ) );
const char * pcPHYName = pcGetPHIName( phy_identity );
FreeRTOS_printf( ( "Start %s PHY autonegotiation. ID = 0x%04X\n", pcPHYName, phy_identity ) );

/* Just to prevent compiler warnings about unused variablies. */
( void ) pcPHYName;

switch( phy_identity )
{
Expand Down