On 16-03-23, 22:34, Mukunda,Vijendar wrote:
On 15/03/23 15:36, Vinod Koul wrote:
On 07-03-23, 19:01, Vijendar Mukunda wrote:
+static void amd_sdw_update_slave_status_work(struct work_struct *work) +{
- struct amd_sdw_manager *amd_manager =
container_of(work, struct amd_sdw_manager, amd_sdw_work);- int retry_count = 0;
- if (amd_manager->status[0] == SDW_SLAVE_ATTACHED) {
acp_reg_writel(0, amd_manager->mmio + ACP_SW_STATE_CHANGE_STATUS_MASK_0TO7);acp_reg_writel(0, amd_manager->mmio + ACP_SW_STATE_CHANGE_STATUS_MASK_8TO11);- }
+update_status:
- sdw_handle_slave_status(&amd_manager->bus, amd_manager->status);
- /*
* During the peripheral enumeration sequence, the SoundWire manager interrupts* are masked. Once the device number programming is done for all peripherals,* interrupts will be unmasked. Read the peripheral device status from ping command* and process the response. This sequence will ensure all peripheral devices enumerated* and initialized properly.*/- if (amd_manager->status[0] == SDW_SLAVE_ATTACHED) {
if (retry_count++ < SDW_MAX_DEVICES) {acp_reg_writel(AMD_SDW_IRQ_MASK_0TO7, amd_manager->mmio +ACP_SW_STATE_CHANGE_STATUS_MASK_0TO7);acp_reg_writel(AMD_SDW_IRQ_MASK_8TO11,amd_manager->mmio + ACP_SW_STATE_CHANGE_STATUS_MASK_8TO11);amd_sdw_read_and_process_ping_status(amd_manager);goto update_status;goto are mostly used for error handling, i dont thing case here deserves a goto, can you please change this...
I agree. goto statements will be used mostly for error handling. But this is a different scenario. We have used goto statement to call sdw_handle_slave_status() from if statement to make sure all peripheral devices are enumerated and initialized properly. Please let us know if you are expecting code to be modified as mentioned below.
sdw_handle_slave_status(&amd_manager->bus, amd_manager->status);
if (amd_manager->status[0] == SDW_SLAVE_ATTACHED) { acp_reg_writel(AMD_SDW_IRQ_MASK_0TO7, amd_manager->mmio + ACP_SW_STATE_CHANGE_STATUS_MASK_0TO7); acp_reg_writel(AMD_SDW_IRQ_MASK_8TO11, amd_manager->mmio + ACP_SW_STATE_CHANGE_STATUS_MASK_8TO11); amd_sdw_read_and_process_ping_status(amd_manager); sdw_handle_slave_status(&amd_manager->bus, amd_manager->status); }
We have to check any race conditions occurs or not if we implement code as mentioned above.
what race are you talking about
IMHO, it is still good to go with goto statement implementation.
Since you keep checking, essentially this seems to be a loop?