| Message ID | 1512122177-2889-10-git-send-email-vinod.koul@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/1/17 3:56 AM, Vinod Koul wrote: > Add status handling API sdw_handle_slave_status() to handle > Slave status changes. > > Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com> > Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com> > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > --- > drivers/soundwire/bus.c | 351 ++++++++++++++++++++++++++++++++++++++++++ > drivers/soundwire/bus.h | 2 + > include/linux/soundwire/sdw.h | 20 +++ > 3 files changed, 373 insertions(+) > > diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c > index 09126ddd3cdd..c6a59a7a1306 100644 > --- a/drivers/soundwire/bus.c > +++ b/drivers/soundwire/bus.c > @@ -602,3 +602,354 @@ static int sdw_initialize_slave(struct sdw_slave *slave) > > return 0; > } > + > +static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status) > +{ > + u8 clear = 0, impl_int_mask; > + int status, ret, count = 0; > + > + status = sdw_read(slave, SDW_DP0_INT); > + if (status < 0) { > + dev_err(slave->bus->dev, > + "SDW_DP0_INT read failed:%d", status); > + return status; > + } > + > + do { > + > + if (status & SDW_DP0_INT_TEST_FAIL) { > + dev_err(&slave->dev, "Test fail for port 0"); > + clear |= SDW_DP0_INT_TEST_FAIL; > + } > + > + /* > + * Assumption: PORT_READY interrupt will be received only for > + * ports implementing Channel Prepare state machine (CP_SM) > + */ > + > + if (status & SDW_DP0_INT_PORT_READY) { > + complete(&slave->port_ready[0]); > + clear |= SDW_DP0_INT_PORT_READY; > + } > + > + if (status & SDW_DP0_INT_BRA_FAILURE) { > + dev_err(&slave->dev, "BRA failed"); > + clear |= SDW_DP0_INT_BRA_FAILURE; > + } > + > + impl_int_mask = SDW_DP0_INT_IMPDEF1 | > + SDW_DP0_INT_IMPDEF2 | SDW_DP0_INT_IMPDEF3; > + > + if (status & impl_int_mask) { > + clear |= impl_int_mask; > + *slave_status = clear; > + } > + > + /* clear the interrupt */ > + ret = sdw_write(slave, SDW_DP0_INT, clear); > + if (ret < 0) { > + dev_err(slave->bus->dev, > + "SDW_DP0_INT write failed:%d", ret); > + return ret; > + } > + > + /* Read DP0 interrupt again */ > + status = sdw_read(slave, SDW_DP0_INT); > + if (status < 0) { > + dev_err(slave->bus->dev, > + "SDW_DP0_INT read failed:%d", status); > + return status; > + } > + > + count++; > + > + /* we can get alerts while processing so keep retrying */ This is not incorrect, but this goes beyond what the spec requires. The additional read is to make sure some interrupts are not lost due to a known race condition. It would be enough to mask the status read the second time to only check if the interrupts sources which were cleared are still signaling something. With the code as it is, you may catch *new* interrupt sources, which could impact the arbitration/priority/policy in handling interrupts. It's not necessarily bad, but you'd need to document whether you want to deal with the race condition described in the MIPI spec or try to be smarter. > + } while (status != 0 && count < SDW_READ_INTR_CLEAR_RETRY); > + > + if (count == SDW_READ_INTR_CLEAR_RETRY) > + dev_warn(slave->bus->dev, "Reached MAX_RETRY on DP0 read"); > + > + return ret; > +} > + > +static int sdw_handle_port_interrupt(struct sdw_slave *slave, > + int port, u8 *slave_status) > +{ > + u8 clear = 0, impl_int_mask; > + int status, ret, count = 0; > + u32 addr; > + > + if (port == 0) > + return sdw_handle_dp0_interrupt(slave, slave_status); > + > + addr = SDW_DPN_INT(port); > + status = sdw_read(slave, addr); > + if (status < 0) { > + dev_err(slave->bus->dev, > + "SDW_DPN_INT read failed:%d", status); > + > + return status; > + } > + > + do { > + > + if (status & SDW_DPN_INT_TEST_FAIL) { > + dev_err(&slave->dev, "Test fail for port:%d", port); > + clear |= SDW_DPN_INT_TEST_FAIL; > + } > + > + /* > + * Assumption: PORT_READY interrupt will be received only > + * for ports implementing CP_SM. > + */ > + if (status & SDW_DPN_INT_PORT_READY) { > + complete(&slave->port_ready[port]); > + clear |= SDW_DPN_INT_PORT_READY; > + } > + > + impl_int_mask = SDW_DPN_INT_IMPDEF1 | > + SDW_DPN_INT_IMPDEF2 | SDW_DPN_INT_IMPDEF3; > + > + > + if (status & impl_int_mask) { > + clear |= impl_int_mask; > + *slave_status = clear; > + } > + > + /* clear the interrupt */ > + ret = sdw_write(slave, addr, clear); > + if (ret < 0) { > + dev_err(slave->bus->dev, > + "SDW_DPN_INT write failed:%d", ret); > + return ret; > + } > + > + /* Read DPN interrupt again */ > + status = sdw_read(slave, addr); > + if (status < 0) { > + dev_err(slave->bus->dev, > + "SDW_DPN_INT read failed:%d", status); > + return status; > + } > + > + count++; > + > + /* we can get alerts while processing so keep retrying */ > + } while (status != 0 && count < SDW_READ_INTR_CLEAR_RETRY); > + > + if (count == SDW_READ_INTR_CLEAR_RETRY) > + dev_warn(slave->bus->dev, "Reached MAX_RETRY on port read"); > + > + return ret; > +} > + > +static int sdw_handle_slave_alerts(struct sdw_slave *slave) > +{ > + u8 clear = 0, bit, port_status[15]; > + int port_num, stat, ret, count = 0; > + unsigned long port; > + bool slave_notify = false; > + u8 buf[3]; > + > + sdw_modify_slave_status(slave, SDW_SLAVE_ALERT); > + > + /* Read Instat 1, Instat 2 and Instat 3 registers */ > + ret = sdw_nread(slave, SDW_SCP_INT1, 3, buf); > + if (ret < 0) { > + dev_err(slave->bus->dev, > + "SDW_SCP_INT1 read failed:%d", ret); > + return ret; > + } > + > + do { > + /* > + * Check parity, bus clash and Slave (impl defined) > + * interrupt > + */ > + if (buf[0] & SDW_SCP_INT1_PARITY) { > + dev_err(&slave->dev, "Parity error detected"); > + clear |= SDW_SCP_INT1_PARITY; > + } > + > + if (buf[0] & SDW_SCP_INT1_BUS_CLASH) { > + dev_err(&slave->dev, "Bus clash error detected"); > + clear |= SDW_SCP_INT1_BUS_CLASH; > + } > + > + /* > + * When bus clash or parity errors are detected, such errors > + * are unlikely to be recoverable errors. > + * TODO: In such scenario, reset bus. Make this configurable > + * via sysfs property with bus reset being the default. > + */ > + > + if (buf[0] & SDW_SCP_INT1_IMPL_DEF) { > + dev_dbg(&slave->dev, "Slave impl defined interrupt\n"); > + clear |= SDW_SCP_INT1_IMPL_DEF; > + slave_notify = true; > + } > + > + /* Check port 0 - 4 interrupts */ > + port = buf[0] & SDW_SCP_INT1_PORT0_3; > + > + /* To get port number corresponding to bits, shift it */ > + port = port >> SDW_REG_SHIFT(SDW_SCP_INT1_PORT0_3); > + for_each_set_bit(bit, &port, 8) { > + sdw_handle_port_interrupt(slave, bit, > + &port_status[bit]); > + > + } > + > + /* Check if cascade 2 interrupt is present */ > + if (buf[0] & SDW_SCP_INT1_SCP2_CASCADE) { > + port = buf[1] & SDW_SCP_INTSTAT2_PORT4_10; > + for_each_set_bit(bit, &port, 8) { > + /* scp2 ports start from 4 */ > + port_num = bit + 3; > + sdw_handle_port_interrupt(slave, > + port_num, > + &port_status[port_num]); > + } > + } > + > + /* now check last cascade */ > + if (buf[1] & SDW_SCP_INTSTAT2_SCP3_CASCADE) { > + port = buf[2] & SDW_SCP_INTSTAT3_PORT11_14; > + for_each_set_bit(bit, &port, 8) { > + /* scp3 ports start from 11 */ > + port_num = bit + 10; > + sdw_handle_port_interrupt(slave, > + port_num, > + &port_status[port_num]); > + } > + } > + > + /* Update the Slave driver */ > + if (slave_notify && (slave->ops) && > + (slave->ops->interrupt_callback)) { > + struct sdw_slave_intr_status slave_intr; > + > + slave_intr.control_port = clear; > + memcpy(slave_intr.port, &port_status, > + sizeof(slave_intr.port)); > + > + slave->ops->interrupt_callback(slave, &slave_intr); > + } > + > + /* Ack interrupt */ > + ret = sdw_write(slave, SDW_SCP_INT1, clear); > + if (ret < 0) { > + dev_err(slave->bus->dev, > + "SDW_SCP_INT1 write failed:%d", ret); > + return ret; > + } > + > + /* > + * Read status again to ensure no new interrupts arrived > + * while servicing interrupts > + */ > + ret = sdw_nread(slave, SDW_SCP_INT1, 3, buf); > + if (ret < 0) { > + dev_err(slave->bus->dev, > + "SDW_SCP_INT1 read failed:%d", ret); > + return ret; > + } > + > + /* Make sure no interrupts are pending */ > + stat = buf[0] || buf[1] || buf[2]; > + > + /* > + * Exit loop if Slave is continuously in ALERT state even > + * after servicing the interrupt multiple times. > + */ > + count++; > + > + /* we can get alerts while processing so keep retrying */ > + } while (stat != 0 && count < SDW_READ_INTR_CLEAR_RETRY); so that's a different case from what is done above, here you are making a conscious decision to re-read the interrupt status, it's got nothing to do with the spec requirements and you'd probably want a different RETRY value. > + > + if (count == SDW_READ_INTR_CLEAR_RETRY) > + dev_warn(slave->bus->dev, "Reached MAX_RETRY on alert read"); > + > + return ret; > +} > + > +static int sdw_update_slave_status(struct sdw_slave *slave, > + enum sdw_slave_status status) > +{ > + if ((slave->ops) && (slave->ops->update_status)) > + return slave->ops->update_status(slave, status); > + > + return 0; > +} > + > +/** > + * sdw_handle_slave_status() - Handle Slave status > + * @bus: SDW bus instance > + * @status: Status for all Slave(s) > + */ > +int sdw_handle_slave_status(struct sdw_bus *bus, > + enum sdw_slave_status status[]) > +{ > + struct sdw_slave *slave; > + int i, ret = 0; > + > + if (status[0] == SDW_SLAVE_ATTACHED) { > + ret = sdw_program_device_num(bus); > + if (ret) > + dev_err(bus->dev, "Slave attach failed: %d", ret); > + } > + > + /* Continue to check other slave statuses */ > + for (i = 1; i <= SDW_MAX_DEVICES; i++) { > + mutex_lock(&bus->bus_lock); > + if (test_bit(i, bus->assigned) == false) { > + mutex_unlock(&bus->bus_lock); > + continue; > + } > + mutex_unlock(&bus->bus_lock); > + > + slave = sdw_get_slave(bus, i); > + if (!slave) > + continue; > + > + switch (status[i]) { > + case SDW_SLAVE_UNATTACHED: > + if (slave->status == SDW_SLAVE_UNATTACHED) > + break; > + > + sdw_modify_slave_status(slave, SDW_SLAVE_UNATTACHED); > + break; > + > + case SDW_SLAVE_ALERT: > + ret = sdw_handle_slave_alerts(slave); > + if (ret) > + dev_err(bus->dev, > + "Slave %d alert handling failed: %d", > + i, ret); > + break; > + > + case SDW_SLAVE_ATTACHED: > + if (slave->status == SDW_SLAVE_ATTACHED) > + break; > + > + sdw_initialize_slave(slave); > + sdw_modify_slave_status(slave, SDW_SLAVE_ATTACHED); > + > + break; > + > + default: > + dev_err(bus->dev, "Invalid slave %d status:%d", > + i, status[i]); > + break; > + } > + > + ret = sdw_update_slave_status(slave, status[i]); > + if (ret) > + dev_err(slave->bus->dev, > + "Update Slave status failed:%d", ret); > + > + } > + > + return ret; > +} > +EXPORT_SYMBOL(sdw_handle_slave_status); > diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h > index b491e86f2c4c..463fecb02563 100644 > --- a/drivers/soundwire/bus.h > +++ b/drivers/soundwire/bus.h > @@ -45,6 +45,8 @@ struct sdw_msg { > bool page; > }; > > +#define SDW_READ_INTR_CLEAR_RETRY 10 > + > int sdw_transfer(struct sdw_bus *bus, struct sdw_slave *slave, > struct sdw_msg *msg); > int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_slave *slave, > diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h > index 09619d042909..861a910911b6 100644 > --- a/include/linux/soundwire/sdw.h > +++ b/include/linux/soundwire/sdw.h > @@ -323,11 +323,28 @@ struct sdw_slave_id { > }; > > /** > + * struct sdw_slave_intr_status - Slave interrupt status > + * @control_port: control port status > + * @port: data port status > + */ > +struct sdw_slave_intr_status { > + u8 control_port; > + u8 port[14]; > +}; > + > +/** > * struct sdw_slave_ops - Slave driver callback ops > * @read_prop: Read Slave properties > + * @interrupt_callback: Device interrupt notification (invoked in thread > + * context) > + * @update_status: Update Slave status > */ > struct sdw_slave_ops { > int (*read_prop)(struct sdw_slave *sdw); > + int (*interrupt_callback)(struct sdw_slave *slave, > + struct sdw_slave_intr_status *status); > + int (*update_status)(struct sdw_slave *slave, > + enum sdw_slave_status status); > }; > > /** > @@ -377,6 +394,9 @@ struct sdw_driver { > int sdw_handle_slave_status(struct sdw_bus *bus, > enum sdw_slave_status status[]); > > +int sdw_handle_slave_status(struct sdw_bus *bus, > + enum sdw_slave_status status[]); > + > /* > * SDW master structures and APIs > */ >
On Fri, Dec 01, 2017 at 05:52:03PM -0600, Pierre-Louis Bossart wrote: > >+ status = sdw_read(slave, SDW_DP0_INT); > >+ if (status < 0) { > >+ dev_err(slave->bus->dev, > >+ "SDW_DP0_INT read failed:%d", status); > >+ return status; > >+ } > >+ > >+ count++; > >+ > >+ /* we can get alerts while processing so keep retrying */ > > This is not incorrect, but this goes beyond what the spec requires. > > The additional read is to make sure some interrupts are not lost due to a > known race condition. It would be enough to mask the status read the second > time to only check if the interrupts sources which were cleared are still > signaling something. > > With the code as it is, you may catch *new* interrupt sources, which could > impact the arbitration/priority/policy in handling interrupts. It's not > necessarily bad, but you'd need to document whether you want to deal with > the race condition described in the MIPI spec or try to be smarter. This was based on your last comment, lets discuss more offline on this to see what else is required here.
On 12/3/17 11:11 AM, Vinod Koul wrote: > On Fri, Dec 01, 2017 at 05:52:03PM -0600, Pierre-Louis Bossart wrote: > >>> + status = sdw_read(slave, SDW_DP0_INT); >>> + if (status < 0) { >>> + dev_err(slave->bus->dev, >>> + "SDW_DP0_INT read failed:%d", status); >>> + return status; >>> + } >>> + >>> + count++; >>> + >>> + /* we can get alerts while processing so keep retrying */ >> >> This is not incorrect, but this goes beyond what the spec requires. >> >> The additional read is to make sure some interrupts are not lost due to a >> known race condition. It would be enough to mask the status read the second >> time to only check if the interrupts sources which were cleared are still >> signaling something. >> >> With the code as it is, you may catch *new* interrupt sources, which could >> impact the arbitration/priority/policy in handling interrupts. It's not >> necessarily bad, but you'd need to document whether you want to deal with >> the race condition described in the MIPI spec or try to be smarter. > > This was based on your last comment, lets discuss more offline on this to > see what else is required here. > I am fine if you leave the code as is for now, it's not bad but can be optimized.
On Sun, Dec 03, 2017 at 09:11:39PM -0600, Pierre-Louis Bossart wrote: > On 12/3/17 11:11 AM, Vinod Koul wrote: > >On Fri, Dec 01, 2017 at 05:52:03PM -0600, Pierre-Louis Bossart wrote: > > > >>>+ status = sdw_read(slave, SDW_DP0_INT); > >>>+ if (status < 0) { > >>>+ dev_err(slave->bus->dev, > >>>+ "SDW_DP0_INT read failed:%d", status); > >>>+ return status; > >>>+ } > >>>+ > >>>+ count++; > >>>+ > >>>+ /* we can get alerts while processing so keep retrying */ > >> > >>This is not incorrect, but this goes beyond what the spec requires. > >> > >>The additional read is to make sure some interrupts are not lost due to a > >>known race condition. It would be enough to mask the status read the second > >>time to only check if the interrupts sources which were cleared are still > >>signaling something. > >> > >>With the code as it is, you may catch *new* interrupt sources, which could > >>impact the arbitration/priority/policy in handling interrupts. It's not > >>necessarily bad, but you'd need to document whether you want to deal with > >>the race condition described in the MIPI spec or try to be smarter. > > > >This was based on your last comment, lets discuss more offline on this to > >see what else is required here. > > I am fine if you leave the code as is for now, it's not bad but can be > optimized. Not bad is not good here :) Okay I still havent grabbed my coffee, so help me out here. I am not sure I understand here, can you point me to the part of spec handling you were referring and what should be *ideally* done
On 12/3/17 9:21 PM, Vinod Koul wrote: > On Sun, Dec 03, 2017 at 09:11:39PM -0600, Pierre-Louis Bossart wrote: >> On 12/3/17 11:11 AM, Vinod Koul wrote: >>> On Fri, Dec 01, 2017 at 05:52:03PM -0600, Pierre-Louis Bossart wrote: >>> >>>>> + status = sdw_read(slave, SDW_DP0_INT); >>>>> + if (status < 0) { >>>>> + dev_err(slave->bus->dev, >>>>> + "SDW_DP0_INT read failed:%d", status); >>>>> + return status; >>>>> + } >>>>> + >>>>> + count++; >>>>> + >>>>> + /* we can get alerts while processing so keep retrying */ >>>> >>>> This is not incorrect, but this goes beyond what the spec requires. >>>> >>>> The additional read is to make sure some interrupts are not lost due to a >>>> known race condition. It would be enough to mask the status read the second >>>> time to only check if the interrupts sources which were cleared are still >>>> signaling something. >>>> >>>> With the code as it is, you may catch *new* interrupt sources, which could >>>> impact the arbitration/priority/policy in handling interrupts. It's not >>>> necessarily bad, but you'd need to document whether you want to deal with >>>> the race condition described in the MIPI spec or try to be smarter. >>> >>> This was based on your last comment, lets discuss more offline on this to >>> see what else is required here. >> >> I am fine if you leave the code as is for now, it's not bad but can be >> optimized. > > Not bad is not good here :) > > Okay I still havent grabbed my coffee, so help me out here. I am not sure I > understand here, can you point me to the part of spec handling you were > referring and what should be *ideally* done You first read the status, then clear the interrupts then re-read the status. I'd be good enough in the second read to mask with the settings of the first read. This is intended to detect alert sources that fired between the last successful read and the write to clear interrupts (see Figure 92 in the 1.1 spec) e.g. do { status1= sdw_read() deal with interrupts status2 = sdw_read() status2 &= status1; /* filter initial sources */ /* we can get alerts while processing so keep retrying */ } while (status2 != 0 && count < SDW_READ_INTR_CLEAR_RETRY);
On Sun, Dec 03, 2017 at 09:52:48PM -0600, Pierre-Louis Bossart wrote: > On 12/3/17 9:21 PM, Vinod Koul wrote: > >On Sun, Dec 03, 2017 at 09:11:39PM -0600, Pierre-Louis Bossart wrote: > >>On 12/3/17 11:11 AM, Vinod Koul wrote: > >>>On Fri, Dec 01, 2017 at 05:52:03PM -0600, Pierre-Louis Bossart wrote: > >>> > >>>>>+ status = sdw_read(slave, SDW_DP0_INT); > >>>>>+ if (status < 0) { > >>>>>+ dev_err(slave->bus->dev, > >>>>>+ "SDW_DP0_INT read failed:%d", status); > >>>>>+ return status; > >>>>>+ } > >>>>>+ > >>>>>+ count++; > >>>>>+ > >>>>>+ /* we can get alerts while processing so keep retrying */ > >>>> > >>>>This is not incorrect, but this goes beyond what the spec requires. > >>>> > >>>>The additional read is to make sure some interrupts are not lost due to a > >>>>known race condition. It would be enough to mask the status read the second > >>>>time to only check if the interrupts sources which were cleared are still > >>>>signaling something. > >>>> > >>>>With the code as it is, you may catch *new* interrupt sources, which could > >>>>impact the arbitration/priority/policy in handling interrupts. It's not > >>>>necessarily bad, but you'd need to document whether you want to deal with > >>>>the race condition described in the MIPI spec or try to be smarter. > >>> > >>>This was based on your last comment, lets discuss more offline on this to > >>>see what else is required here. > >> > >>I am fine if you leave the code as is for now, it's not bad but can be > >>optimized. > > > >Not bad is not good here :) > > > >Okay I still havent grabbed my coffee, so help me out here. I am not sure I > >understand here, can you point me to the part of spec handling you were > >referring and what should be *ideally* done > > You first read the status, then clear the interrupts then re-read the > status. I'd be good enough in the second read to mask with the settings of > the first read. This is intended to detect alert sources that fired between > the last successful read and the write to clear interrupts (see Figure 92 in > the 1.1 spec) > > e.g. > > do { > status1= sdw_read() > deal with interrupts > status2 = sdw_read() > status2 &= status1; /* filter initial sources */ Sounds better, updated now, thanks
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 09126ddd3cdd..c6a59a7a1306 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -602,3 +602,354 @@ static int sdw_initialize_slave(struct sdw_slave *slave) return 0; } + +static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status) +{ + u8 clear = 0, impl_int_mask; + int status, ret, count = 0; + + status = sdw_read(slave, SDW_DP0_INT); + if (status < 0) { + dev_err(slave->bus->dev, + "SDW_DP0_INT read failed:%d", status); + return status; + } + + do { + + if (status & SDW_DP0_INT_TEST_FAIL) { + dev_err(&slave->dev, "Test fail for port 0"); + clear |= SDW_DP0_INT_TEST_FAIL; + } + + /* + * Assumption: PORT_READY interrupt will be received only for + * ports implementing Channel Prepare state machine (CP_SM) + */ + + if (status & SDW_DP0_INT_PORT_READY) { + complete(&slave->port_ready[0]); + clear |= SDW_DP0_INT_PORT_READY; + } + + if (status & SDW_DP0_INT_BRA_FAILURE) { + dev_err(&slave->dev, "BRA failed"); + clear |= SDW_DP0_INT_BRA_FAILURE; + } + + impl_int_mask = SDW_DP0_INT_IMPDEF1 | + SDW_DP0_INT_IMPDEF2 | SDW_DP0_INT_IMPDEF3; + + if (status & impl_int_mask) { + clear |= impl_int_mask; + *slave_status = clear; + } + + /* clear the interrupt */ + ret = sdw_write(slave, SDW_DP0_INT, clear); + if (ret < 0) { + dev_err(slave->bus->dev, + "SDW_DP0_INT write failed:%d", ret); + return ret; + } + + /* Read DP0 interrupt again */ + status = sdw_read(slave, SDW_DP0_INT); + if (status < 0) { + dev_err(slave->bus->dev, + "SDW_DP0_INT read failed:%d", status); + return status; + } + + count++; + + /* we can get alerts while processing so keep retrying */ + } while (status != 0 && count < SDW_READ_INTR_CLEAR_RETRY); + + if (count == SDW_READ_INTR_CLEAR_RETRY) + dev_warn(slave->bus->dev, "Reached MAX_RETRY on DP0 read"); + + return ret; +} + +static int sdw_handle_port_interrupt(struct sdw_slave *slave, + int port, u8 *slave_status) +{ + u8 clear = 0, impl_int_mask; + int status, ret, count = 0; + u32 addr; + + if (port == 0) + return sdw_handle_dp0_interrupt(slave, slave_status); + + addr = SDW_DPN_INT(port); + status = sdw_read(slave, addr); + if (status < 0) { + dev_err(slave->bus->dev, + "SDW_DPN_INT read failed:%d", status); + + return status; + } + + do { + + if (status & SDW_DPN_INT_TEST_FAIL) { + dev_err(&slave->dev, "Test fail for port:%d", port); + clear |= SDW_DPN_INT_TEST_FAIL; + } + + /* + * Assumption: PORT_READY interrupt will be received only + * for ports implementing CP_SM. + */ + if (status & SDW_DPN_INT_PORT_READY) { + complete(&slave->port_ready[port]); + clear |= SDW_DPN_INT_PORT_READY; + } + + impl_int_mask = SDW_DPN_INT_IMPDEF1 | + SDW_DPN_INT_IMPDEF2 | SDW_DPN_INT_IMPDEF3; + + + if (status & impl_int_mask) { + clear |= impl_int_mask; + *slave_status = clear; + } + + /* clear the interrupt */ + ret = sdw_write(slave, addr, clear); + if (ret < 0) { + dev_err(slave->bus->dev, + "SDW_DPN_INT write failed:%d", ret); + return ret; + } + + /* Read DPN interrupt again */ + status = sdw_read(slave, addr); + if (status < 0) { + dev_err(slave->bus->dev, + "SDW_DPN_INT read failed:%d", status); + return status; + } + + count++; + + /* we can get alerts while processing so keep retrying */ + } while (status != 0 && count < SDW_READ_INTR_CLEAR_RETRY); + + if (count == SDW_READ_INTR_CLEAR_RETRY) + dev_warn(slave->bus->dev, "Reached MAX_RETRY on port read"); + + return ret; +} + +static int sdw_handle_slave_alerts(struct sdw_slave *slave) +{ + u8 clear = 0, bit, port_status[15]; + int port_num, stat, ret, count = 0; + unsigned long port; + bool slave_notify = false; + u8 buf[3]; + + sdw_modify_slave_status(slave, SDW_SLAVE_ALERT); + + /* Read Instat 1, Instat 2 and Instat 3 registers */ + ret = sdw_nread(slave, SDW_SCP_INT1, 3, buf); + if (ret < 0) { + dev_err(slave->bus->dev, + "SDW_SCP_INT1 read failed:%d", ret); + return ret; + } + + do { + /* + * Check parity, bus clash and Slave (impl defined) + * interrupt + */ + if (buf[0] & SDW_SCP_INT1_PARITY) { + dev_err(&slave->dev, "Parity error detected"); + clear |= SDW_SCP_INT1_PARITY; + } + + if (buf[0] & SDW_SCP_INT1_BUS_CLASH) { + dev_err(&slave->dev, "Bus clash error detected"); + clear |= SDW_SCP_INT1_BUS_CLASH; + } + + /* + * When bus clash or parity errors are detected, such errors + * are unlikely to be recoverable errors. + * TODO: In such scenario, reset bus. Make this configurable + * via sysfs property with bus reset being the default. + */ + + if (buf[0] & SDW_SCP_INT1_IMPL_DEF) { + dev_dbg(&slave->dev, "Slave impl defined interrupt\n"); + clear |= SDW_SCP_INT1_IMPL_DEF; + slave_notify = true; + } + + /* Check port 0 - 4 interrupts */ + port = buf[0] & SDW_SCP_INT1_PORT0_3; + + /* To get port number corresponding to bits, shift it */ + port = port >> SDW_REG_SHIFT(SDW_SCP_INT1_PORT0_3); + for_each_set_bit(bit, &port, 8) { + sdw_handle_port_interrupt(slave, bit, + &port_status[bit]); + + } + + /* Check if cascade 2 interrupt is present */ + if (buf[0] & SDW_SCP_INT1_SCP2_CASCADE) { + port = buf[1] & SDW_SCP_INTSTAT2_PORT4_10; + for_each_set_bit(bit, &port, 8) { + /* scp2 ports start from 4 */ + port_num = bit + 3; + sdw_handle_port_interrupt(slave, + port_num, + &port_status[port_num]); + } + } + + /* now check last cascade */ + if (buf[1] & SDW_SCP_INTSTAT2_SCP3_CASCADE) { + port = buf[2] & SDW_SCP_INTSTAT3_PORT11_14; + for_each_set_bit(bit, &port, 8) { + /* scp3 ports start from 11 */ + port_num = bit + 10; + sdw_handle_port_interrupt(slave, + port_num, + &port_status[port_num]); + } + } + + /* Update the Slave driver */ + if (slave_notify && (slave->ops) && + (slave->ops->interrupt_callback)) { + struct sdw_slave_intr_status slave_intr; + + slave_intr.control_port = clear; + memcpy(slave_intr.port, &port_status, + sizeof(slave_intr.port)); + + slave->ops->interrupt_callback(slave, &slave_intr); + } + + /* Ack interrupt */ + ret = sdw_write(slave, SDW_SCP_INT1, clear); + if (ret < 0) { + dev_err(slave->bus->dev, + "SDW_SCP_INT1 write failed:%d", ret); + return ret; + } + + /* + * Read status again to ensure no new interrupts arrived + * while servicing interrupts + */ + ret = sdw_nread(slave, SDW_SCP_INT1, 3, buf); + if (ret < 0) { + dev_err(slave->bus->dev, + "SDW_SCP_INT1 read failed:%d", ret); + return ret; + } + + /* Make sure no interrupts are pending */ + stat = buf[0] || buf[1] || buf[2]; + + /* + * Exit loop if Slave is continuously in ALERT state even + * after servicing the interrupt multiple times. + */ + count++; + + /* we can get alerts while processing so keep retrying */ + } while (stat != 0 && count < SDW_READ_INTR_CLEAR_RETRY); + + if (count == SDW_READ_INTR_CLEAR_RETRY) + dev_warn(slave->bus->dev, "Reached MAX_RETRY on alert read"); + + return ret; +} + +static int sdw_update_slave_status(struct sdw_slave *slave, + enum sdw_slave_status status) +{ + if ((slave->ops) && (slave->ops->update_status)) + return slave->ops->update_status(slave, status); + + return 0; +} + +/** + * sdw_handle_slave_status() - Handle Slave status + * @bus: SDW bus instance + * @status: Status for all Slave(s) + */ +int sdw_handle_slave_status(struct sdw_bus *bus, + enum sdw_slave_status status[]) +{ + struct sdw_slave *slave; + int i, ret = 0; + + if (status[0] == SDW_SLAVE_ATTACHED) { + ret = sdw_program_device_num(bus); + if (ret) + dev_err(bus->dev, "Slave attach failed: %d", ret); + } + + /* Continue to check other slave statuses */ + for (i = 1; i <= SDW_MAX_DEVICES; i++) { + mutex_lock(&bus->bus_lock); + if (test_bit(i, bus->assigned) == false) { + mutex_unlock(&bus->bus_lock); + continue; + } + mutex_unlock(&bus->bus_lock); + + slave = sdw_get_slave(bus, i); + if (!slave) + continue; + + switch (status[i]) { + case SDW_SLAVE_UNATTACHED: + if (slave->status == SDW_SLAVE_UNATTACHED) + break; + + sdw_modify_slave_status(slave, SDW_SLAVE_UNATTACHED); + break; + + case SDW_SLAVE_ALERT: + ret = sdw_handle_slave_alerts(slave); + if (ret) + dev_err(bus->dev, + "Slave %d alert handling failed: %d", + i, ret); + break; + + case SDW_SLAVE_ATTACHED: + if (slave->status == SDW_SLAVE_ATTACHED) + break; + + sdw_initialize_slave(slave); + sdw_modify_slave_status(slave, SDW_SLAVE_ATTACHED); + + break; + + default: + dev_err(bus->dev, "Invalid slave %d status:%d", + i, status[i]); + break; + } + + ret = sdw_update_slave_status(slave, status[i]); + if (ret) + dev_err(slave->bus->dev, + "Update Slave status failed:%d", ret); + + } + + return ret; +} +EXPORT_SYMBOL(sdw_handle_slave_status); diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index b491e86f2c4c..463fecb02563 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -45,6 +45,8 @@ struct sdw_msg { bool page; }; +#define SDW_READ_INTR_CLEAR_RETRY 10 + int sdw_transfer(struct sdw_bus *bus, struct sdw_slave *slave, struct sdw_msg *msg); int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_slave *slave, diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 09619d042909..861a910911b6 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -323,11 +323,28 @@ struct sdw_slave_id { }; /** + * struct sdw_slave_intr_status - Slave interrupt status + * @control_port: control port status + * @port: data port status + */ +struct sdw_slave_intr_status { + u8 control_port; + u8 port[14]; +}; + +/** * struct sdw_slave_ops - Slave driver callback ops * @read_prop: Read Slave properties + * @interrupt_callback: Device interrupt notification (invoked in thread + * context) + * @update_status: Update Slave status */ struct sdw_slave_ops { int (*read_prop)(struct sdw_slave *sdw); + int (*interrupt_callback)(struct sdw_slave *slave, + struct sdw_slave_intr_status *status); + int (*update_status)(struct sdw_slave *slave, + enum sdw_slave_status status); }; /** @@ -377,6 +394,9 @@ struct sdw_driver { int sdw_handle_slave_status(struct sdw_bus *bus, enum sdw_slave_status status[]); +int sdw_handle_slave_status(struct sdw_bus *bus, + enum sdw_slave_status status[]); + /* * SDW master structures and APIs */