| Message ID | 20241203102318.3386345-1-ukaszb@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] usb: typec: ucsi: Fix completion notifications | expand |
On Tue, 3 Dec 2024 at 12:23, Łukasz Bartosik <ukaszb@chromium.org> wrote: > > OPM PPM LPM > | 1.send cmd | | > |-------------------------->| | > | |-- | > | | | 2.set busy bit in CCI | > | |<- | > | 3.notify the OPM | | > |<--------------------------| | > | | 4.send cmd to be executed | > | |-------------------------->| > | | | > | | 5.cmd completed | > | |<--------------------------| > | | | > | |-- | > | | | 6.set cmd completed | > | |<- bit in CCI | > | | | > | 7.notify the OPM | | > |<--------------------------| | > | | | > | 8.handle notification | | > | from point 3, read CCI | | > |<--------------------------| | > | | | > > When the PPM receives command from the OPM (p.1) it sets the busy bit > in the CCI (p.2), sends notification to the OPM (p.3) and forwards the > command to be executed by the LPM (p.4). When the PPM receives command > completion from the LPM (p.5) it sets command completion bit in the CCI > (p.6) and sends notification to the OPM (p.7). If command execution by > the LPM is fast enough then when the OPM starts handling the notification > from p.3 in p.8 and reads the CCI value it will see command completion bit > set and will call complete(). Then complete() might be called again when > the OPM handles notification from p.7. > > This fix replaces test_bit() with test_and_clear_bit() > in ucsi_notify_common() in order to call complete() only > once per request. > > This fix also reinitializes completion variable in > ucsi_sync_control_common() before a command is sent. Thank you! > > Fixes: 584e8df58942 ("usb: typec: ucsi: extract common code for command handling") > Cc: stable@vger.kernel.org > Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org> > --- Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Tue, Dec 03, 2024 at 10:23:18AM +0000, Łukasz Bartosik wrote: > OPM PPM LPM > | 1.send cmd | | > |-------------------------->| | > | |-- | > | | | 2.set busy bit in CCI | > | |<- | > | 3.notify the OPM | | > |<--------------------------| | > | | 4.send cmd to be executed | > | |-------------------------->| > | | | > | | 5.cmd completed | > | |<--------------------------| > | | | > | |-- | > | | | 6.set cmd completed | > | |<- bit in CCI | > | | | > | 7.notify the OPM | | > |<--------------------------| | > | | | > | 8.handle notification | | > | from point 3, read CCI | | > |<--------------------------| | > | | | > > When the PPM receives command from the OPM (p.1) it sets the busy bit > in the CCI (p.2), sends notification to the OPM (p.3) and forwards the > command to be executed by the LPM (p.4). When the PPM receives command > completion from the LPM (p.5) it sets command completion bit in the CCI > (p.6) and sends notification to the OPM (p.7). If command execution by > the LPM is fast enough then when the OPM starts handling the notification > from p.3 in p.8 and reads the CCI value it will see command completion bit > set and will call complete(). Then complete() might be called again when > the OPM handles notification from p.7. > > This fix replaces test_bit() with test_and_clear_bit() > in ucsi_notify_common() in order to call complete() only > once per request. > > This fix also reinitializes completion variable in > ucsi_sync_control_common() before a command is sent. > > Fixes: 584e8df58942 ("usb: typec: ucsi: extract common code for command handling") > Cc: stable@vger.kernel.org > Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com > --- > > Changes in v2: > - Swapped points 7 and 8 in the commit description > in order to make diagram more clear. > - Added reinitialization of completion variable > in the ucsi_sync_control_common(). > --- > > drivers/usb/typec/ucsi/ucsi.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index c435c0835744..7a65a7672e18 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -46,11 +46,11 @@ void ucsi_notify_common(struct ucsi *ucsi, u32 cci) > ucsi_connector_change(ucsi, UCSI_CCI_CONNECTOR(cci)); > > if (cci & UCSI_CCI_ACK_COMPLETE && > - test_bit(ACK_PENDING, &ucsi->flags)) > + test_and_clear_bit(ACK_PENDING, &ucsi->flags)) > complete(&ucsi->complete); > > if (cci & UCSI_CCI_COMMAND_COMPLETE && > - test_bit(COMMAND_PENDING, &ucsi->flags)) > + test_and_clear_bit(COMMAND_PENDING, &ucsi->flags)) > complete(&ucsi->complete); > } > EXPORT_SYMBOL_GPL(ucsi_notify_common); > @@ -65,6 +65,8 @@ int ucsi_sync_control_common(struct ucsi *ucsi, u64 command) > else > set_bit(COMMAND_PENDING, &ucsi->flags); > > + reinit_completion(&ucsi->complete); > + > ret = ucsi->ops->async_control(ucsi, command); > if (ret) > goto out_clear_bit; > -- > 2.47.0.338.g60cca15819-goog
On Tue, Dec 03, 2024 at 02:48:59PM +0200, Heikki Krogerus wrote: > On Tue, Dec 03, 2024 at 10:23:18AM +0000, Łukasz Bartosik wrote: > > OPM PPM LPM > > | 1.send cmd | | > > |-------------------------->| | > > | |-- | > > | | | 2.set busy bit in CCI | > > | |<- | > > | 3.notify the OPM | | > > |<--------------------------| | > > | | 4.send cmd to be executed | > > | |-------------------------->| > > | | | > > | | 5.cmd completed | > > | |<--------------------------| > > | | | > > | |-- | > > | | | 6.set cmd completed | > > | |<- bit in CCI | > > | | | > > | 7.notify the OPM | | > > |<--------------------------| | > > | | | > > | 8.handle notification | | > > | from point 3, read CCI | | > > |<--------------------------| | > > | | | > > > > When the PPM receives command from the OPM (p.1) it sets the busy bit > > in the CCI (p.2), sends notification to the OPM (p.3) and forwards the > > command to be executed by the LPM (p.4). When the PPM receives command > > completion from the LPM (p.5) it sets command completion bit in the CCI > > (p.6) and sends notification to the OPM (p.7). If command execution by > > the LPM is fast enough then when the OPM starts handling the notification > > from p.3 in p.8 and reads the CCI value it will see command completion bit > > set and will call complete(). Then complete() might be called again when > > the OPM handles notification from p.7. > > > > This fix replaces test_bit() with test_and_clear_bit() > > in ucsi_notify_common() in order to call complete() only > > once per request. > > > > This fix also reinitializes completion variable in > > ucsi_sync_control_common() before a command is sent. > > > > Fixes: 584e8df58942 ("usb: typec: ucsi: extract common code for command handling") > > Cc: stable@vger.kernel.org > > Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org> > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Sorry about that typo.
On Tue, Dec 3, 2024 at 1:51 PM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > On Tue, Dec 03, 2024 at 02:48:59PM +0200, Heikki Krogerus wrote: > > On Tue, Dec 03, 2024 at 10:23:18AM +0000, Łukasz Bartosik wrote: > > > OPM PPM LPM > > > | 1.send cmd | | > > > |-------------------------->| | > > > | |-- | > > > | | | 2.set busy bit in CCI | > > > | |<- | > > > | 3.notify the OPM | | > > > |<--------------------------| | > > > | | 4.send cmd to be executed | > > > | |-------------------------->| > > > | | | > > > | | 5.cmd completed | > > > | |<--------------------------| > > > | | | > > > | |-- | > > > | | | 6.set cmd completed | > > > | |<- bit in CCI | > > > | | | > > > | 7.notify the OPM | | > > > |<--------------------------| | > > > | | | > > > | 8.handle notification | | > > > | from point 3, read CCI | | > > > |<--------------------------| | > > > | | | > > > > > > When the PPM receives command from the OPM (p.1) it sets the busy bit > > > in the CCI (p.2), sends notification to the OPM (p.3) and forwards the > > > command to be executed by the LPM (p.4). When the PPM receives command > > > completion from the LPM (p.5) it sets command completion bit in the CCI > > > (p.6) and sends notification to the OPM (p.7). If command execution by > > > the LPM is fast enough then when the OPM starts handling the notification > > > from p.3 in p.8 and reads the CCI value it will see command completion bit > > > set and will call complete(). Then complete() might be called again when > > > the OPM handles notification from p.7. > > > > > > This fix replaces test_bit() with test_and_clear_bit() > > > in ucsi_notify_common() in order to call complete() only > > > once per request. > > > > > > This fix also reinitializes completion variable in > > > ucsi_sync_control_common() before a command is sent. > > > > > > Fixes: 584e8df58942 ("usb: typec: ucsi: extract common code for command handling") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org> > > > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Sorry about that typo. > > -- > heikki Heikki, Dmitry, Thank you for the review. Regards, Łukasz
On Tue, Dec 03, 2024 at 10:23:18AM +0000, Łukasz Bartosik wrote: > OPM PPM LPM > | 1.send cmd | | > |-------------------------->| | > | |-- | > | | | 2.set busy bit in CCI | > | |<- | > | 3.notify the OPM | | > |<--------------------------| | > | | 4.send cmd to be executed | > | |-------------------------->| > | | | > | | 5.cmd completed | > | |<--------------------------| > | | | > | |-- | > | | | 6.set cmd completed | > | |<- bit in CCI | > | | | > | 7.notify the OPM | | > |<--------------------------| | > | | | > | 8.handle notification | | > | from point 3, read CCI | | > |<--------------------------| | > | | | > > When the PPM receives command from the OPM (p.1) it sets the busy bit > in the CCI (p.2), sends notification to the OPM (p.3) and forwards the > command to be executed by the LPM (p.4). When the PPM receives command > completion from the LPM (p.5) it sets command completion bit in the CCI > (p.6) and sends notification to the OPM (p.7). If command execution by > the LPM is fast enough then when the OPM starts handling the notification > from p.3 in p.8 and reads the CCI value it will see command completion bit > set and will call complete(). Then complete() might be called again when > the OPM handles notification from p.7. > > This fix replaces test_bit() with test_and_clear_bit() > in ucsi_notify_common() in order to call complete() only > once per request. > > This fix also reinitializes completion variable in > ucsi_sync_control_common() before a command is sent. > > Fixes: 584e8df58942 ("usb: typec: ucsi: extract common code for command handling") > Cc: stable@vger.kernel.org > Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org> Reviewed-by: Benson Leung <bleung@chromium.org> > --- > > Changes in v2: > - Swapped points 7 and 8 in the commit description > in order to make diagram more clear. > - Added reinitialization of completion variable > in the ucsi_sync_control_common(). > --- > > drivers/usb/typec/ucsi/ucsi.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index c435c0835744..7a65a7672e18 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -46,11 +46,11 @@ void ucsi_notify_common(struct ucsi *ucsi, u32 cci) > ucsi_connector_change(ucsi, UCSI_CCI_CONNECTOR(cci)); > > if (cci & UCSI_CCI_ACK_COMPLETE && > - test_bit(ACK_PENDING, &ucsi->flags)) > + test_and_clear_bit(ACK_PENDING, &ucsi->flags)) > complete(&ucsi->complete); > > if (cci & UCSI_CCI_COMMAND_COMPLETE && > - test_bit(COMMAND_PENDING, &ucsi->flags)) > + test_and_clear_bit(COMMAND_PENDING, &ucsi->flags)) > complete(&ucsi->complete); > } > EXPORT_SYMBOL_GPL(ucsi_notify_common); > @@ -65,6 +65,8 @@ int ucsi_sync_control_common(struct ucsi *ucsi, u64 command) > else > set_bit(COMMAND_PENDING, &ucsi->flags); > > + reinit_completion(&ucsi->complete); > + > ret = ucsi->ops->async_control(ucsi, command); > if (ret) > goto out_clear_bit; > -- > 2.47.0.338.g60cca15819-goog >
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index c435c0835744..7a65a7672e18 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -46,11 +46,11 @@ void ucsi_notify_common(struct ucsi *ucsi, u32 cci) ucsi_connector_change(ucsi, UCSI_CCI_CONNECTOR(cci)); if (cci & UCSI_CCI_ACK_COMPLETE && - test_bit(ACK_PENDING, &ucsi->flags)) + test_and_clear_bit(ACK_PENDING, &ucsi->flags)) complete(&ucsi->complete); if (cci & UCSI_CCI_COMMAND_COMPLETE && - test_bit(COMMAND_PENDING, &ucsi->flags)) + test_and_clear_bit(COMMAND_PENDING, &ucsi->flags)) complete(&ucsi->complete); } EXPORT_SYMBOL_GPL(ucsi_notify_common); @@ -65,6 +65,8 @@ int ucsi_sync_control_common(struct ucsi *ucsi, u64 command) else set_bit(COMMAND_PENDING, &ucsi->flags); + reinit_completion(&ucsi->complete); + ret = ucsi->ops->async_control(ucsi, command); if (ret) goto out_clear_bit;
OPM PPM LPM | 1.send cmd | | |-------------------------->| | | |-- | | | | 2.set busy bit in CCI | | |<- | | 3.notify the OPM | | |<--------------------------| | | | 4.send cmd to be executed | | |-------------------------->| | | | | | 5.cmd completed | | |<--------------------------| | | | | |-- | | | | 6.set cmd completed | | |<- bit in CCI | | | | | 7.notify the OPM | | |<--------------------------| | | | | | 8.handle notification | | | from point 3, read CCI | | |<--------------------------| | | | | When the PPM receives command from the OPM (p.1) it sets the busy bit in the CCI (p.2), sends notification to the OPM (p.3) and forwards the command to be executed by the LPM (p.4). When the PPM receives command completion from the LPM (p.5) it sets command completion bit in the CCI (p.6) and sends notification to the OPM (p.7). If command execution by the LPM is fast enough then when the OPM starts handling the notification from p.3 in p.8 and reads the CCI value it will see command completion bit set and will call complete(). Then complete() might be called again when the OPM handles notification from p.7. This fix replaces test_bit() with test_and_clear_bit() in ucsi_notify_common() in order to call complete() only once per request. This fix also reinitializes completion variable in ucsi_sync_control_common() before a command is sent. Fixes: 584e8df58942 ("usb: typec: ucsi: extract common code for command handling") Cc: stable@vger.kernel.org Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org> --- Changes in v2: - Swapped points 7 and 8 in the commit description in order to make diagram more clear. - Added reinitialization of completion variable in the ucsi_sync_control_common(). --- drivers/usb/typec/ucsi/ucsi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)