| Message ID | 20230920023243.2494410-1-utkarsh.h.patel@intel.com |
|---|---|
| Headers | show |
| Series | Displayport Alternate Mode 2.1 Support | expand |
On Mon, Sep 25, 2023 at 03:54:40PM +0000, Patel, Utkarsh H wrote: ... > > > + /** > > > > Are you sure? > > > > > + * Get cable VDO for thunderbolt cables and cables with DPSID but > > does not > > > + * support DPAM2.1. > > > + */ > > > Yes, there are TBT3 cables which advertise DPSID but does not provide any DP > capabilities in the DP discover mode VDO but does support UHBR. In that > case, need to use TBTSID and use capabilities from TBT discover mode VDO. My comment was against the style of the comment, not about content. ... > > You can also make it a bit more readable with (use better names if you think it's > > needed) > > > > u32 signalling = 0; > > u32 cable_type = 0; > > In v2 version of this patch I had them but there was feedback to remove extra > variables and use them inline. OK!
Hi Sergei, Thank you for the review. > -----Original Message----- > From: Sergei Shtylyov <sergei.shtylyov@gmail.com> > Sent: Wednesday, September 20, 2023 2:10 AM > To: Patel, Utkarsh H <utkarsh.h.patel@intel.com>; linux- > kernel@vger.kernel.org; linux-usb@vger.kernel.org > Cc: heikki.krogerus@linux.intel.com; pmalani@chromium.org; chrome- > platform@lists.linux.dev; andriy.shevchenko@linux.intel.com; > bleung@chromium.org > Subject: Re: [PATCH v4 5/5] usb: typec: intel_pmc_mux: Configure Displayport > Alternate mode 2.1 > > On 9/20/23 5:32 AM, Utkarsh Patel wrote: > > > Mux agent driver can configure cable details such as cable type and > > cable speed received as a part of displayport configuration to support > > Displayport Alternate mode 2.1. > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com> > > --- > > Changes in v4: > > - No change. > > > > Changes in v3: > > - No change. > > > > Changes in v2: > > - No change. > > > > drivers/usb/typec/mux/intel_pmc_mux.c | 24 > ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c > > b/drivers/usb/typec/mux/intel_pmc_mux.c > > index 60ed1f809130..233958084b43 100644 > > --- a/drivers/usb/typec/mux/intel_pmc_mux.c > > +++ b/drivers/usb/typec/mux/intel_pmc_mux.c > [...] > > @@ -293,6 +299,24 @@ pmc_usb_mux_dp(struct pmc_usb_port *port, > struct typec_mux_state *state) > > req.mode_data |= (state->mode - TYPEC_STATE_MODAL) << > > PMC_USB_ALTMODE_DP_MODE_SHIFT; > > > > + if (!is_pmc_mux_tbt(port->pmc->iom_adev)) { > > + u8 cable_speed = (data->conf & > DP_CONF_SIGNALLING_MASK) >> > > + DP_CONF_SIGNALLING_SHIFT; > > + > > + u8 cable_type = (data->conf & DP_CONF_CABLE_TYPE_MASK) > >> > > + DP_CONF_CABLE_TYPE_SHIFT; > > + > > + req.mode_data |= > PMC_USB_ALTMODE_CABLE_SPD(cable_speed); > > + > > + if (cable_type == DP_CONF_CABLE_TYPE_OPTICAL) > > + req.mode_data |= PMC_USB_ALTMODE_CABLE_TYPE; > > + else if (cable_type == DP_CONF_CABLE_TYPE_RE_TIMER) > > + req.mode_data |= > PMC_USB_ALTMODE_ACTIVE_CABLE | > > + > PMC_USB_ALTMODE_RETIMER_CABLE; > > + else if (cable_type == DP_CONF_CABLE_TYPE_RE_DRIVER) > > + req.mode_data |= > PMC_USB_ALTMODE_ACTIVE_CABLE; > > Why not *switch* instead of string of *if*s? > Switch can be used here but if there is no added benefit, would stick to if/else. Sincerely, Utkarsh Patel.
Hello: This series was applied to chrome-platform/linux.git (for-next) by Greg Kroah-Hartman <gregkh@linuxfoundation.org>: On Tue, 19 Sep 2023 19:32:38 -0700 you wrote: > This series enabels cable identification flow required for Displayport > Alternate Mode 2.1 support. > > Please add this series to usb-next. > > Changes in v4: > - Added [PATCH 3/5] back in this series with Reviewed-by tag from Heikki. > > [...] Here is the summary with links: - [v4,1/5] usb: typec: Add Displayport Alternate Mode 2.1 Support https://git.kernel.org/chrome-platform/c/a17fae8fc38e - [v4,2/5] usb: typec: Add Active or Passive cable defination to cable discover mode VDO https://git.kernel.org/chrome-platform/c/c365b1e1f404 - [v4,3/5] usb: pd: Add helper macro to get Type C cable speed https://git.kernel.org/chrome-platform/c/f9ee6043283a - [v4,4/5] platform/chrome: cros_ec_typec: Add Displayport Alternatemode 2.1 Support https://git.kernel.org/chrome-platform/c/70ca6c7312c5 - [v4,5/5] usb: typec: intel_pmc_mux: Configure Displayport Alternate mode 2.1 https://git.kernel.org/chrome-platform/c/6c29de68fb29 You are awesome, thank you!
This series enabels cable identification flow required for Displayport Alternate Mode 2.1 support. Please add this series to usb-next. Changes in v4: - Added [PATCH 3/5] back in this series with Reviewed-by tag from Heikki. Changes in v3: - Removed [PATCH 3/5] usb: pd: Add helper macro to get Type C cable speed instead added the helper function in [PATCH 3/4]. - In [PATCH 3/4] Removed use of variable cable_speed. Changes in v2: - Added two new patches [PATCH 2/5] and [PATCH 3/5] in to this series. - Removed [PATCH 2/4] platform/chrome: cros_ec: Add Displayport Alternatemode 2.1 feature flag - In [PATCH 4/5] Removed feature flag and separate function for DP2.1. - In [PATCH 4/5] used cable vdos instead of host commands to get cable details. Utkarsh Patel (5): usb: typec: Add Displayport Alternate Mode 2.1 Support usb: typec: Add Active or Passive cable defination to cable discover mode VDO usb: pd: Add helper macro to get Type C cable speed platform/chrome: cros_ec_typec: Add Displayport Alternatemode 2.1 Support usb: typec: intel_pmc_mux: Configure Displayport Alternate mode 2.1 drivers/platform/chrome/cros_ec_typec.c | 28 ++++++++++++++++++++++++ drivers/usb/typec/altmodes/displayport.c | 5 ++++- drivers/usb/typec/mux/intel_pmc_mux.c | 24 ++++++++++++++++++++ drivers/usb/typec/ucsi/displayport.c | 2 +- drivers/usb/typec/ucsi/ucsi_ccg.c | 4 ++-- include/linux/usb/pd_vdo.h | 1 + include/linux/usb/typec_dp.h | 28 ++++++++++++++++++++---- include/linux/usb/typec_tbt.h | 1 + 8 files changed, 85 insertions(+), 8 deletions(-)