On Thu, Jan 19, 2023 at 1:26 AM Heikki Krogerus heikki.krogerus@linux.intel.com wrote:
Hi Prashant,
On Wed, Jan 18, 2023 at 10:26:21AM -0800, Prashant Malani wrote:
Hi Heikki,
Thanks for reviewing the patch.
On Wed, Jan 18, 2023 at 1:39 AM Heikki Krogerus heikki.krogerus@linux.intel.com wrote:
On Wed, Jan 18, 2023 at 03:15:15AM +0000, Prashant Malani wrote:
FWIW, I think we can make the typec_altmode_update_active() calls from our (cros-ec-typec) port driver too, but displayport.c is parsing the header anyway, so it seemed repetitive. Just wanted to clarify the intention here.
The alt modes may have been entered even if there are no drivers for them, if for example the PD controller handles the mode entry. In those cases the port driver needs to update the active state of the partner alt mode.
Ack. Thanks for explaining the rationale here.
Since the port drivers have to handle that in some cases, for the sake of consistency I thought that they might as well take care of it in every case.
On the other hand, it should be safe to do it in both the port driver and the altmode driver.
If you prefer that the altmode drivers always do this, I'm not against it. But in that case could you patch tcpm.c while at it - in the same series:
Sure, I will send out a v2 with the below diff as Patch 2/2 (I will mark you as "Suggested-by" but as always LMK if you prefer another way to denote attribution).
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 904c7b4ce2f0c..0f5a9d4db105a 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -1693,14 +1693,11 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev, } break; case CMD_ENTER_MODE:
if (adev && pdev) {typec_altmode_update_active(pdev, true);
if (adev && pdev) *adev_action = ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL;
} return 0; case CMD_EXIT_MODE: if (adev && pdev) {typec_altmode_update_active(pdev, false); /* Back to USB Operation */ *adev_action = ADEV_NOTIFY_USB_AND_QUEUE_VDM; return 0;That's the only driver that will definitely always requires the altmode drivers, so perhaps it would be good to drop the calls from it at the same time.
thanks,
-- heikki