| Message ID | 20240627-ucsi-rework-interface-v4-0-289ddc6874c7@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | usb: typec: ucsi: rework glue driver interface | expand |
On Thu, Jun 27, 2024 at 05:44:39PM +0300, Dmitry Baryshkov wrote: > The interface between UCSI and the glue driver is very low-level. It > allows reading the UCSI data from any offset (but in reality the UCSI > driver reads only VERSION, CCI an MESSAGE_IN data). All event handling > is to be done by the glue driver (which already resulted in several > similar-but-slightly different implementations). It leaves no place to > optimize the write-read-read sequence for the command execution (which > might be beneficial for some of the drivers), etc. > > The patchseries attempts to restructure the UCSI glue driver interface > in order to provide sensible operations instead of a low-level read / > write calls. > > If this approach is found to be acceptable, I plan to further rework the > command interface, moving reading CCI and MESSAGE_IN to the common > control code, which should simplify driver's implementation and remove > necessity to split quirks between sync_control and read_message_in e.g. > as implemented in the ucsi_ccg.c. > > Note, the series was tested only on the ucsi_glink platforms. Further > testing is appreciated. > > Depends: [1], [2] > > [1] https://lore.kernel.org/linux-usb/20240612124656.2305603-1-fabrice.gasnier@foss.st.com/ > > [2] https://lore.kernel.org/linux-usb/20240621-ucsi-yoga-ec-driver-v8-1-e03f3536b8c6@linaro.org/ > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > Changes in v4: > - Rebased on top of Greg's tree to resolve conflicts. Nope, still got conflicts, are you sure you updated properly? Patch 1 applied, but #2 did not. thanks, greg k-h
On Thu, 27 Jun 2024 at 17:54, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Thu, Jun 27, 2024 at 05:44:39PM +0300, Dmitry Baryshkov wrote: > > The interface between UCSI and the glue driver is very low-level. It > > allows reading the UCSI data from any offset (but in reality the UCSI > > driver reads only VERSION, CCI an MESSAGE_IN data). All event handling > > is to be done by the glue driver (which already resulted in several > > similar-but-slightly different implementations). It leaves no place to > > optimize the write-read-read sequence for the command execution (which > > might be beneficial for some of the drivers), etc. > > > > The patchseries attempts to restructure the UCSI glue driver interface > > in order to provide sensible operations instead of a low-level read / > > write calls. > > > > If this approach is found to be acceptable, I plan to further rework the > > command interface, moving reading CCI and MESSAGE_IN to the common > > control code, which should simplify driver's implementation and remove > > necessity to split quirks between sync_control and read_message_in e.g. > > as implemented in the ucsi_ccg.c. > > > > Note, the series was tested only on the ucsi_glink platforms. Further > > testing is appreciated. > > > > Depends: [1], [2] > > > > [1] https://lore.kernel.org/linux-usb/20240612124656.2305603-1-fabrice.gasnier@foss.st.com/ > > > > [2] https://lore.kernel.org/linux-usb/20240621-ucsi-yoga-ec-driver-v8-1-e03f3536b8c6@linaro.org/ > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > Changes in v4: > > - Rebased on top of Greg's tree to resolve conflicts. > > Nope, still got conflicts, are you sure you updated properly? Patch 1 > applied, but #2 did not. I feel stupid enough now. I rebased on top of usb-next instead of usb-testing. Let me spam it once again
On Thu, Jun 27, 2024 at 05:57:30PM +0300, Dmitry Baryshkov wrote: > On Thu, 27 Jun 2024 at 17:54, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Jun 27, 2024 at 05:44:39PM +0300, Dmitry Baryshkov wrote: > > > The interface between UCSI and the glue driver is very low-level. It > > > allows reading the UCSI data from any offset (but in reality the UCSI > > > driver reads only VERSION, CCI an MESSAGE_IN data). All event handling > > > is to be done by the glue driver (which already resulted in several > > > similar-but-slightly different implementations). It leaves no place to > > > optimize the write-read-read sequence for the command execution (which > > > might be beneficial for some of the drivers), etc. > > > > > > The patchseries attempts to restructure the UCSI glue driver interface > > > in order to provide sensible operations instead of a low-level read / > > > write calls. > > > > > > If this approach is found to be acceptable, I plan to further rework the > > > command interface, moving reading CCI and MESSAGE_IN to the common > > > control code, which should simplify driver's implementation and remove > > > necessity to split quirks between sync_control and read_message_in e.g. > > > as implemented in the ucsi_ccg.c. > > > > > > Note, the series was tested only on the ucsi_glink platforms. Further > > > testing is appreciated. > > > > > > Depends: [1], [2] > > > > > > [1] https://lore.kernel.org/linux-usb/20240612124656.2305603-1-fabrice.gasnier@foss.st.com/ > > > > > > [2] https://lore.kernel.org/linux-usb/20240621-ucsi-yoga-ec-driver-v8-1-e03f3536b8c6@linaro.org/ > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > --- > > > Changes in v4: > > > - Rebased on top of Greg's tree to resolve conflicts. > > > > Nope, still got conflicts, are you sure you updated properly? Patch 1 > > applied, but #2 did not. > > I feel stupid enough now. I rebased on top of usb-next instead of > usb-testing. Let me spam it once again Both branches should be identical right now, perhaps you missed when I synced them up.
On Thu, 27 Jun 2024 at 17:57, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Thu, 27 Jun 2024 at 17:54, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Jun 27, 2024 at 05:44:39PM +0300, Dmitry Baryshkov wrote: > > > The interface between UCSI and the glue driver is very low-level. It > > > allows reading the UCSI data from any offset (but in reality the UCSI > > > driver reads only VERSION, CCI an MESSAGE_IN data). All event handling > > > is to be done by the glue driver (which already resulted in several > > > similar-but-slightly different implementations). It leaves no place to > > > optimize the write-read-read sequence for the command execution (which > > > might be beneficial for some of the drivers), etc. > > > > > > The patchseries attempts to restructure the UCSI glue driver interface > > > in order to provide sensible operations instead of a low-level read / > > > write calls. > > > > > > If this approach is found to be acceptable, I plan to further rework the > > > command interface, moving reading CCI and MESSAGE_IN to the common > > > control code, which should simplify driver's implementation and remove > > > necessity to split quirks between sync_control and read_message_in e.g. > > > as implemented in the ucsi_ccg.c. > > > > > > Note, the series was tested only on the ucsi_glink platforms. Further > > > testing is appreciated. > > > > > > Depends: [1], [2] > > > > > > [1] https://lore.kernel.org/linux-usb/20240612124656.2305603-1-fabrice.gasnier@foss.st.com/ > > > > > > [2] https://lore.kernel.org/linux-usb/20240621-ucsi-yoga-ec-driver-v8-1-e03f3536b8c6@linaro.org/ > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > --- > > > Changes in v4: > > > - Rebased on top of Greg's tree to resolve conflicts. > > > > Nope, still got conflicts, are you sure you updated properly? Patch 1 > > applied, but #2 did not. > > I feel stupid enough now. I rebased on top of usb-next instead of > usb-testing. Let me spam it once again Hmm, I see what happened. I had a next+usb-next. Simple usb-next doesn't contain changes from 9e3caa9dd51b ("usb: typec: ucsi_acpi: Add LG Gram quirk") which this patch also modifies. I can rebase it on top of your tree, but then we will have build issues once usb-linus and usb-next get merged together.
On Thu, Jun 27, 2024 at 06:08:07PM +0300, Dmitry Baryshkov wrote: > On Thu, 27 Jun 2024 at 17:57, Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Thu, 27 Jun 2024 at 17:54, Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > > > > > > On Thu, Jun 27, 2024 at 05:44:39PM +0300, Dmitry Baryshkov wrote: > > > > The interface between UCSI and the glue driver is very low-level. It > > > > allows reading the UCSI data from any offset (but in reality the UCSI > > > > driver reads only VERSION, CCI an MESSAGE_IN data). All event handling > > > > is to be done by the glue driver (which already resulted in several > > > > similar-but-slightly different implementations). It leaves no place to > > > > optimize the write-read-read sequence for the command execution (which > > > > might be beneficial for some of the drivers), etc. > > > > > > > > The patchseries attempts to restructure the UCSI glue driver interface > > > > in order to provide sensible operations instead of a low-level read / > > > > write calls. > > > > > > > > If this approach is found to be acceptable, I plan to further rework the > > > > command interface, moving reading CCI and MESSAGE_IN to the common > > > > control code, which should simplify driver's implementation and remove > > > > necessity to split quirks between sync_control and read_message_in e.g. > > > > as implemented in the ucsi_ccg.c. > > > > > > > > Note, the series was tested only on the ucsi_glink platforms. Further > > > > testing is appreciated. > > > > > > > > Depends: [1], [2] > > > > > > > > [1] https://lore.kernel.org/linux-usb/20240612124656.2305603-1-fabrice.gasnier@foss.st.com/ > > > > > > > > [2] https://lore.kernel.org/linux-usb/20240621-ucsi-yoga-ec-driver-v8-1-e03f3536b8c6@linaro.org/ > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > --- > > > > Changes in v4: > > > > - Rebased on top of Greg's tree to resolve conflicts. > > > > > > Nope, still got conflicts, are you sure you updated properly? Patch 1 > > > applied, but #2 did not. > > > > I feel stupid enough now. I rebased on top of usb-next instead of > > usb-testing. Let me spam it once again > > Hmm, I see what happened. I had a next+usb-next. Simple usb-next > doesn't contain changes from 9e3caa9dd51b ("usb: typec: ucsi_acpi: Add > LG Gram quirk") which this patch also modifies. I can rebase it on top > of your tree, but then we will have build issues once usb-linus and > usb-next get merged together. Ah, you need/want stuff from both branches, right? Then just wait until next week when my -linus branch will be in Linus's tree and then I will merge that into the -next branch. thanks, greg k-h
On Fri, 28 Jun 2024 at 17:24, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Thu, Jun 27, 2024 at 06:08:07PM +0300, Dmitry Baryshkov wrote: > > On Thu, 27 Jun 2024 at 17:57, Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > On Thu, 27 Jun 2024 at 17:54, Greg Kroah-Hartman > > > <gregkh@linuxfoundation.org> wrote: > > > > > > > > On Thu, Jun 27, 2024 at 05:44:39PM +0300, Dmitry Baryshkov wrote: > > > > > The interface between UCSI and the glue driver is very low-level. It > > > > > allows reading the UCSI data from any offset (but in reality the UCSI > > > > > driver reads only VERSION, CCI an MESSAGE_IN data). All event handling > > > > > is to be done by the glue driver (which already resulted in several > > > > > similar-but-slightly different implementations). It leaves no place to > > > > > optimize the write-read-read sequence for the command execution (which > > > > > might be beneficial for some of the drivers), etc. > > > > > > > > > > The patchseries attempts to restructure the UCSI glue driver interface > > > > > in order to provide sensible operations instead of a low-level read / > > > > > write calls. > > > > > > > > > > If this approach is found to be acceptable, I plan to further rework the > > > > > command interface, moving reading CCI and MESSAGE_IN to the common > > > > > control code, which should simplify driver's implementation and remove > > > > > necessity to split quirks between sync_control and read_message_in e.g. > > > > > as implemented in the ucsi_ccg.c. > > > > > > > > > > Note, the series was tested only on the ucsi_glink platforms. Further > > > > > testing is appreciated. > > > > > > > > > > Depends: [1], [2] > > > > > > > > > > [1] https://lore.kernel.org/linux-usb/20240612124656.2305603-1-fabrice.gasnier@foss.st.com/ > > > > > > > > > > [2] https://lore.kernel.org/linux-usb/20240621-ucsi-yoga-ec-driver-v8-1-e03f3536b8c6@linaro.org/ > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > --- > > > > > Changes in v4: > > > > > - Rebased on top of Greg's tree to resolve conflicts. > > > > > > > > Nope, still got conflicts, are you sure you updated properly? Patch 1 > > > > applied, but #2 did not. > > > > > > I feel stupid enough now. I rebased on top of usb-next instead of > > > usb-testing. Let me spam it once again > > > > Hmm, I see what happened. I had a next+usb-next. Simple usb-next > > doesn't contain changes from 9e3caa9dd51b ("usb: typec: ucsi_acpi: Add > > LG Gram quirk") which this patch also modifies. I can rebase it on top > > of your tree, but then we will have build issues once usb-linus and > > usb-next get merged together. > > Ah, you need/want stuff from both branches, right? Then just wait until > next week when my -linus branch will be in Linus's tree and then I will > merge that into the -next branch. Ack. Maybe I'll post another iteration based on the discussion on the mailing list.
On Fri, Jun 28, 2024 at 05:25:17PM +0300, Dmitry Baryshkov wrote: > On Fri, 28 Jun 2024 at 17:24, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Jun 27, 2024 at 06:08:07PM +0300, Dmitry Baryshkov wrote: > > > On Thu, 27 Jun 2024 at 17:57, Dmitry Baryshkov > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > On Thu, 27 Jun 2024 at 17:54, Greg Kroah-Hartman > > > > <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > On Thu, Jun 27, 2024 at 05:44:39PM +0300, Dmitry Baryshkov wrote: > > > > > > The interface between UCSI and the glue driver is very low-level. It > > > > > > allows reading the UCSI data from any offset (but in reality the UCSI > > > > > > driver reads only VERSION, CCI an MESSAGE_IN data). All event handling > > > > > > is to be done by the glue driver (which already resulted in several > > > > > > similar-but-slightly different implementations). It leaves no place to > > > > > > optimize the write-read-read sequence for the command execution (which > > > > > > might be beneficial for some of the drivers), etc. > > > > > > > > > > > > The patchseries attempts to restructure the UCSI glue driver interface > > > > > > in order to provide sensible operations instead of a low-level read / > > > > > > write calls. > > > > > > > > > > > > If this approach is found to be acceptable, I plan to further rework the > > > > > > command interface, moving reading CCI and MESSAGE_IN to the common > > > > > > control code, which should simplify driver's implementation and remove > > > > > > necessity to split quirks between sync_control and read_message_in e.g. > > > > > > as implemented in the ucsi_ccg.c. > > > > > > > > > > > > Note, the series was tested only on the ucsi_glink platforms. Further > > > > > > testing is appreciated. > > > > > > > > > > > > Depends: [1], [2] > > > > > > > > > > > > [1] https://lore.kernel.org/linux-usb/20240612124656.2305603-1-fabrice.gasnier@foss.st.com/ > > > > > > > > > > > > [2] https://lore.kernel.org/linux-usb/20240621-ucsi-yoga-ec-driver-v8-1-e03f3536b8c6@linaro.org/ > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > > > --- > > > > > > Changes in v4: > > > > > > - Rebased on top of Greg's tree to resolve conflicts. > > > > > > > > > > Nope, still got conflicts, are you sure you updated properly? Patch 1 > > > > > applied, but #2 did not. > > > > > > > > I feel stupid enough now. I rebased on top of usb-next instead of > > > > usb-testing. Let me spam it once again > > > > > > Hmm, I see what happened. I had a next+usb-next. Simple usb-next > > > doesn't contain changes from 9e3caa9dd51b ("usb: typec: ucsi_acpi: Add > > > LG Gram quirk") which this patch also modifies. I can rebase it on top > > > of your tree, but then we will have build issues once usb-linus and > > > usb-next get merged together. > > > > Ah, you need/want stuff from both branches, right? Then just wait until > > next week when my -linus branch will be in Linus's tree and then I will > > merge that into the -next branch. > > Ack. Maybe I'll post another iteration based on the discussion on the > mailing list. Now queued up, thanks. greg k-h
The interface between UCSI and the glue driver is very low-level. It allows reading the UCSI data from any offset (but in reality the UCSI driver reads only VERSION, CCI an MESSAGE_IN data). All event handling is to be done by the glue driver (which already resulted in several similar-but-slightly different implementations). It leaves no place to optimize the write-read-read sequence for the command execution (which might be beneficial for some of the drivers), etc. The patchseries attempts to restructure the UCSI glue driver interface in order to provide sensible operations instead of a low-level read / write calls. If this approach is found to be acceptable, I plan to further rework the command interface, moving reading CCI and MESSAGE_IN to the common control code, which should simplify driver's implementation and remove necessity to split quirks between sync_control and read_message_in e.g. as implemented in the ucsi_ccg.c. Note, the series was tested only on the ucsi_glink platforms. Further testing is appreciated. Depends: [1], [2] [1] https://lore.kernel.org/linux-usb/20240612124656.2305603-1-fabrice.gasnier@foss.st.com/ [2] https://lore.kernel.org/linux-usb/20240621-ucsi-yoga-ec-driver-v8-1-e03f3536b8c6@linaro.org/ Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Changes in v4: - Rebased on top of Greg's tree to resolve conflicts. - Link to v3: https://lore.kernel.org/r/20240625-ucsi-rework-interface-v3-0-7a6c8e17be3a@linaro.org Changes in v3: - Rebased on top of the next to resolve conflicts with the merge LG Gram patch (Heikki) - Link to v2: https://lore.kernel.org/r/20240621-ucsi-rework-interface-v2-0-a399ff96bf88@linaro.org Changes in v2: - Dropped the RFC prefix - Rebased on top of the fixed STM32 patch - Included the pending Yoga C630 driver into the cleanup. - Link to v1: https://lore.kernel.org/r/20240603-ucsi-rework-interface-v1-0-99a6d544cec8@linaro.org --- Dmitry Baryshkov (7): usb: typec: ucsi: move ucsi_acknowledge() from ucsi_read_error() usb: typec: ucsi: simplify command sending API usb: typec: ucsi: split read operation usb: typec: ucsi: rework command execution functions usb: typec: ucsi: inline ucsi_read_message_in usb: typec: ucsi: extract common code for command handling usb: typec: ucsi: reorder operations in ucsi_run_command() drivers/usb/typec/ucsi/ucsi.c | 221 +++++++++++++++++--------------- drivers/usb/typec/ucsi/ucsi.h | 26 ++-- drivers/usb/typec/ucsi/ucsi_acpi.c | 127 +++++++++--------- drivers/usb/typec/ucsi/ucsi_ccg.c | 103 +++++++-------- drivers/usb/typec/ucsi/ucsi_glink.c | 74 ++++------- drivers/usb/typec/ucsi/ucsi_stm32g0.c | 79 ++++-------- drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 104 +++++---------- 7 files changed, 325 insertions(+), 409 deletions(-) --- base-commit: 878bae2ba39bdd581cbe77d8210b9a03ae172adf change-id: 20240525-ucsi-rework-interface-5ff2264f6aec Best regards,