drivers/fsi: Flag FSI bus errors with dev_err( )
PATCH 1/1 drivers/fsi: Flag FSI bus errors with dev_err( )
Currently not seeing details of any FSI bus errors that may occur. Send info via dev_err so that the type of FSI bus fail can be determined.
--- drivers/fsi/fsi-master-gpio.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/fsi/fsi-master-gpio.c +6 -6
@@ -218,8 +218,8 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,break ; }if (i >=FSI_GPIO_MTOE_COUNT ) { -dev_dbg (master->master.dev, -"Master time out waiting for response\n" ); +dev_err (master->master.dev, +"MTOE: Master time out waiting for response\n" );drain_response (master);fsi_master_gpio_error (master,FSI_GPIO_MTOE );return -EIO ;
@@ -257,8 +257,8 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,case FSI_GPIO_RESP_ERRA :case FSI_GPIO_RESP_ERRC : -dev_dbg (master->master.dev,"ERR received: %d\n" , - (int )response.msg); +dev_err (master->master.dev,"ERRA/C received: %d\n" , + response_id);/* * todo: Verify crc from slave and in general * only act on any response if crc is correct
@@ -283,7 +283,7 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,/* Read in the crc and check it */ serial_in (master, &response,FSI_GPIO_CRC_SIZE );if (crc_in != response.msg) { -dev_dbg (master->master.dev,"ERR response CRC\n" ); +dev_err (master->master.dev,"ERR response CRC\n" );fsi_master_gpio_error (master,FSI_GPIO_CRC_INVAL );return -EIO ; }
@@ -293,7 +293,7 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected, }while (busy_count++ <FSI_GPIO_MAX_BUSY ); -dev_dbg (master->master.dev,"ERR slave is stuck in busy state\n" ); +dev_err (master->master.dev,"ERR slave is stuck in busy state\n" );fsi_master_gpio_error (master,FSI_GPIO_ERR_BUSY );return -EIO ;
Signature · 1 line
1.8.2.2
Quoted 3 lines · Christopher Bostic
Currently not seeing details of any FSI bus errors that may occur. Send info via
dev_errso that the type of FSI bus fail can be determined.
Are these really errors for normal users of the system?
I think these will only be useful for FSI developers. Use the dynamic debug system for having developer errors.
Quoted 53 lines
Signed-off-by Christopher Bostic <[email protected]>--- drivers/fsi/fsi-master-gpio.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)drivers/fsi/fsi-master-gpio.c +6
-7@@ -218,8 +218,8 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,break ; }if (i >=FSI_GPIO_MTOE_COUNT ) { -dev_dbg (master->master.dev, -"Master time out waiting for response\n" ); +dev_err (master->master.dev, +"MTOE: Master time out waiting for response\n" );drain_response (master);fsi_master_gpio_error (master,FSI_GPIO_MTOE );return -EIO ;@@ -257,8 +257,8 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,case FSI_GPIO_RESP_ERRA :case FSI_GPIO_RESP_ERRC : -dev_dbg (master->master.dev,"ERR received: %d\n" , - (int )response.msg); +dev_err (master->master.dev,"ERRA/C received: %d\n" , + response_id);/* * todo: Verify crc from slave and in general * only act on any response if crc is correct @@ -283,7 +283,7 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,/* Read in the crc and check it */ serial_in (master, &response,FSI_GPIO_CRC_SIZE );if (crc_in != response.msg) { -dev_dbg (master->master.dev,"ERR response CRC\n" ); +dev_err (master->master.dev,"ERR response CRC\n" );fsi_master_gpio_error (master,FSI_GPIO_CRC_INVAL );return -EIO ; }@@ -293,7 +293,7 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected, }while (busy_count++ <FSI_GPIO_MAX_BUSY ); -dev_dbg (master->master.dev,"ERR slave is stuck in busy state\n" ); +dev_err (master->master.dev,"ERR slave is stuck in busy state\n" );fsi_master_gpio_error (master,FSI_GPIO_ERR_BUSY );return -EIO ; --1.8.2.2
Quoted 9 lines · Joel Stanley
Quoted 3 lines · Christopher Bostic
Currently not seeing details of any FSI bus errors that may occur. Send info via
dev_errso that the type of FSI bus fail can be determined.Are these really errors for normal users of the system?
I think these will only be useful for FSI developers. Use the dynamic debug system for having developer errors.
Hi Joel,
I suppose there are two different situations where this error info would be useful. First during developer debug and bringup which would sound more like a candidate for the dynamic debug system you mention. Second case is after all firmware bugs have been ironed out and we have general system errors related to hardware fails. Is that a developer error or info for users. To me that would be more for normal users of the system.
Thanks
-Chris.
Quoted 52 lines
Quoted 52 lines
Signed-off-by Christopher Bostic <[email protected]>--- drivers/fsi/fsi-master-gpio.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)drivers/fsi/fsi-master-gpio.c +6
-7@@ -218,8 +218,8 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,break ; }if (i >=FSI_GPIO_MTOE_COUNT ) { -dev_dbg (master->master.dev, -"Master time out waiting for response\n" ); +dev_err (master->master.dev, +"MTOE: Master time out waiting for response\n" );drain_response (master);fsi_master_gpio_error (master,FSI_GPIO_MTOE );return -EIO ;@@ -257,8 +257,8 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,case FSI_GPIO_RESP_ERRA :case FSI_GPIO_RESP_ERRC : -dev_dbg (master->master.dev,"ERR received: %d\n" , - (int )response.msg); +dev_err (master->master.dev,"ERRA/C received: %d\n" , + response_id);/* * todo: Verify crc from slave and in general * only act on any response if crc is correct @@ -283,7 +283,7 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,/* Read in the crc and check it */ serial_in (master, &response,FSI_GPIO_CRC_SIZE );if (crc_in != response.msg) { -dev_dbg (master->master.dev,"ERR response CRC\n" ); +dev_err (master->master.dev,"ERR response CRC\n" );fsi_master_gpio_error (master,FSI_GPIO_CRC_INVAL );return -EIO ; }@@ -293,7 +293,7 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected, }while (busy_count++ <FSI_GPIO_MAX_BUSY ); -dev_dbg (master->master.dev,"ERR slave is stuck in busy state\n" ); +dev_err (master->master.dev,"ERR slave is stuck in busy state\n" );fsi_master_gpio_error (master,FSI_GPIO_ERR_BUSY );return -EIO ; --1.8.2.2
Quoted 9 lines · Joel Stanley
Quoted 3 lines · Christopher Bostic
Currently not seeing details of any FSI bus errors that may occur. Send info via
dev_errso that the type of FSI bus fail can be determined.Are these really errors for normal users of the system?
I think these will only be useful for FSI developers. Use the dynamic debug system for having developer errors.
Hi Joel,
On second thought I think the dynamic debug facilities will be enough. This change will generate more spam to console for benign bus errors during the normal scan process. I'll cancel submission.
Thanks
-Chris
Quoted 52 lines
Quoted 52 lines
Signed-off-by Christopher Bostic <[email protected]>--- drivers/fsi/fsi-master-gpio.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)drivers/fsi/fsi-master-gpio.c +6
-7@@ -218,8 +218,8 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,break ; }if (i >=FSI_GPIO_MTOE_COUNT ) { -dev_dbg (master->master.dev, -"Master time out waiting for response\n" ); +dev_err (master->master.dev, +"MTOE: Master time out waiting for response\n" );drain_response (master);fsi_master_gpio_error (master,FSI_GPIO_MTOE );return -EIO ;@@ -257,8 +257,8 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,case FSI_GPIO_RESP_ERRA :case FSI_GPIO_RESP_ERRC : -dev_dbg (master->master.dev,"ERR received: %d\n" , - (int )response.msg); +dev_err (master->master.dev,"ERRA/C received: %d\n" , + response_id);/* * todo: Verify crc from slave and in general * only act on any response if crc is correct @@ -283,7 +283,7 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,/* Read in the crc and check it */ serial_in (master, &response,FSI_GPIO_CRC_SIZE );if (crc_in != response.msg) { -dev_dbg (master->master.dev,"ERR response CRC\n" ); +dev_err (master->master.dev,"ERR response CRC\n" );fsi_master_gpio_error (master,FSI_GPIO_CRC_INVAL );return -EIO ; }@@ -293,7 +293,7 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected, }while (busy_count++ <FSI_GPIO_MAX_BUSY ); -dev_dbg (master->master.dev,"ERR slave is stuck in busy state\n" ); +dev_err (master->master.dev,"ERR slave is stuck in busy state\n" );fsi_master_gpio_error (master,FSI_GPIO_ERR_BUSY );return -EIO ; --1.8.2.2
Quoted 13 lines · Christopher Bostic
Quoted 10 lines · Joel Stanley
Quoted 4 lines · Christopher Bostic
Currently not seeing details of any FSI bus errors that may occur. Send info via
dev_errso that the type of FSI bus fail can be determined.Are these really errors for normal users of the system?
Quoted 8 lines
Quoted 2 lines
I think these will only be useful for FSI developers. Use the dynamic debug system for having developer errors.
Hi Joel,
On second thought I think the dynamic debug facilities will be enough. This change will generate more spam to console for benign bus errors during
the normal scan process. I'll cancel submission.
Thanks for taking the time to consider. I agree that for now it would be too much noise.
As the driver and userspace matures, we should make sure the userspace performs appropriate error handing and logging.
Cheers,
Joel