| Message ID | 20220928131615.3286936-1-waynec@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] usb: typec: ucsi_ccg: Disable UCSI ALT support on Tegra | expand |
On Wed, Sep 28, 2022 at 09:16:15PM +0800, Wayne Chang wrote: > From: Sing-Han Chen <singhanc@nvidia.com> > > Firmware built for Tegra doesn't support UCSI ALT > command and has known issue of reporting wrong > capability info. > > This commit disables UCSI ALT support when reading > the capability on Tegra. You have a full 72 columns to use, no need to make it shorter :) > > Signed-off-by: Sing-Han Chen <singhanc@nvidia.com> > Signed-off-by: Wayne Chang <waynec@nvidia.com> > --- > drivers/usb/typec/ucsi/ucsi_ccg.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c > index 5c0bf48be766..fde3da0605f5 100644 > --- a/drivers/usb/typec/ucsi/ucsi_ccg.c > +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c > @@ -122,9 +122,14 @@ struct version_format { > * Firmware version 3.1.10 or earlier, built for NVIDIA has known issue > * of missing interrupt when a device is connected for runtime resume > */ > -#define CCG_FW_BUILD_NVIDIA (('n' << 8) | 'v') > +#define CCG_FW_BUILD_NVIDIA_RTX (('n' << 8) | 'v') Why change this here? It's not needed, just add the new command instead. And what commit id does this fix? Is it needed for stable kernels? If so, how far back? thanks, greg k-h
Hi Greg, Thanks for the review. On 9/28/22 21:45, Greg KH wrote: > External email: Use caution opening links or attachments > > > On Wed, Sep 28, 2022 at 09:16:15PM +0800, Wayne Chang wrote: >> From: Sing-Han Chen <singhanc@nvidia.com> >> >> Firmware built for Tegra doesn't support UCSI ALT >> command and has known issue of reporting wrong >> capability info. >> >> This commit disables UCSI ALT support when reading >> the capability on Tegra. > > You have a full 72 columns to use, no need to make it shorter :) Thanks. I'll update in the next patchset. > >> >> Signed-off-by: Sing-Han Chen <singhanc@nvidia.com> >> Signed-off-by: Wayne Chang <waynec@nvidia.com> >> --- >> drivers/usb/typec/ucsi/ucsi_ccg.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c >> index 5c0bf48be766..fde3da0605f5 100644 >> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c >> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c >> @@ -122,9 +122,14 @@ struct version_format { >> * Firmware version 3.1.10 or earlier, built for NVIDIA has known issue >> * of missing interrupt when a device is connected for runtime resume >> */ >> -#define CCG_FW_BUILD_NVIDIA (('n' << 8) | 'v') >> +#define CCG_FW_BUILD_NVIDIA_RTX (('n' << 8) | 'v') > > Why change this here? It's not needed, just add the new command > instead. The change here is to distinguish the FW built for NVIDIA RTX products and NVIDIA Tegra products. #define CCG_FW_BUILD_NVIDIA_RTX (('n' << 8) | 'v') #define CCG_FW_BUILD_NVIDIA_TEGRA (('g' << 8) | 'n') I'll update the change if it is not needed to do so. > > And what commit id does this fix? Is it needed for stable kernels? If > so, how far back? We are now enabling the NVIDIA Tegra products on upstream kernel. The change is to add the Cypress cypd 4226 support for NVIDA Tegra products including Xavier AGX, Xavier Orin and the upcoming products. The Cypress cypd4226 is not enabled in current kernel codebase. And thus, we shall not need it for stable kernels and backporting, do we? > > thanks, > > greg k-h > thanks, Wayne.
On Wed, Sep 28, 2022 at 02:11:36PM +0000, Wayne Chang wrote: > Hi Greg, > > Thanks for the review. > > On 9/28/22 21:45, Greg KH wrote: > > External email: Use caution opening links or attachments > > > > > > On Wed, Sep 28, 2022 at 09:16:15PM +0800, Wayne Chang wrote: > >> From: Sing-Han Chen <singhanc@nvidia.com> > >> > >> Firmware built for Tegra doesn't support UCSI ALT > >> command and has known issue of reporting wrong > >> capability info. > >> > >> This commit disables UCSI ALT support when reading > >> the capability on Tegra. > > > > You have a full 72 columns to use, no need to make it shorter :) > Thanks. I'll update in the next patchset. > > > > >> > >> Signed-off-by: Sing-Han Chen <singhanc@nvidia.com> > >> Signed-off-by: Wayne Chang <waynec@nvidia.com> > >> --- > >> drivers/usb/typec/ucsi/ucsi_ccg.c | 16 ++++++++++++++-- > >> 1 file changed, 14 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c > >> index 5c0bf48be766..fde3da0605f5 100644 > >> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c > >> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c > >> @@ -122,9 +122,14 @@ struct version_format { > >> * Firmware version 3.1.10 or earlier, built for NVIDIA has known issue > >> * of missing interrupt when a device is connected for runtime resume > >> */ > >> -#define CCG_FW_BUILD_NVIDIA (('n' << 8) | 'v') > >> +#define CCG_FW_BUILD_NVIDIA_RTX (('n' << 8) | 'v') > > > > Why change this here? It's not needed, just add the new command > > instead. > > The change here is to distinguish the FW built for NVIDIA RTX products > and NVIDIA Tegra products. > > #define CCG_FW_BUILD_NVIDIA_RTX (('n' << 8) | 'v') > #define CCG_FW_BUILD_NVIDIA_TEGRA (('g' << 8) | 'n') > > I'll update the change if it is not needed to do so. No need to make this change here, right? Do so in a later commit if you really need to. > > And what commit id does this fix? Is it needed for stable kernels? If > > so, how far back? > We are now enabling the NVIDIA Tegra products on upstream kernel. > The change is to add the Cypress cypd 4226 support for NVIDA Tegra > products including Xavier AGX, Xavier Orin and the upcoming products. > The Cypress cypd4226 is not enabled in current kernel codebase. > And thus, we shall not need it for stable kernels and backporting, do we? Ok, that's fine, it was not obvious so you might want to say that in the changelog text. thanks, greg k-h
On 9/28/22 22:29, Greg KH wrote: > External email: Use caution opening links or attachments > > > On Wed, Sep 28, 2022 at 02:11:36PM +0000, Wayne Chang wrote: >> Hi Greg, >> >> Thanks for the review. >> >> On 9/28/22 21:45, Greg KH wrote: >>> External email: Use caution opening links or attachments >>> >>> >>> On Wed, Sep 28, 2022 at 09:16:15PM +0800, Wayne Chang wrote: >>>> From: Sing-Han Chen <singhanc@nvidia.com> >>>> >>>> Firmware built for Tegra doesn't support UCSI ALT >>>> command and has known issue of reporting wrong >>>> capability info. >>>> >>>> This commit disables UCSI ALT support when reading >>>> the capability on Tegra. >>> >>> You have a full 72 columns to use, no need to make it shorter :) >> Thanks. I'll update in the next patchset. >> >>> >>>> >>>> Signed-off-by: Sing-Han Chen <singhanc@nvidia.com> >>>> Signed-off-by: Wayne Chang <waynec@nvidia.com> >>>> --- >>>> drivers/usb/typec/ucsi/ucsi_ccg.c | 16 ++++++++++++++-- >>>> 1 file changed, 14 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c >>>> index 5c0bf48be766..fde3da0605f5 100644 >>>> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c >>>> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c >>>> @@ -122,9 +122,14 @@ struct version_format { >>>> * Firmware version 3.1.10 or earlier, built for NVIDIA has known issue >>>> * of missing interrupt when a device is connected for runtime resume >>>> */ >>>> -#define CCG_FW_BUILD_NVIDIA (('n' << 8) | 'v') >>>> +#define CCG_FW_BUILD_NVIDIA_RTX (('n' << 8) | 'v') >>> >>> Why change this here? It's not needed, just add the new command >>> instead. >> >> The change here is to distinguish the FW built for NVIDIA RTX products >> and NVIDIA Tegra products. >> >> #define CCG_FW_BUILD_NVIDIA_RTX (('n' << 8) | 'v') >> #define CCG_FW_BUILD_NVIDIA_TEGRA (('g' << 8) | 'n') >> >> I'll update the change if it is not needed to do so. > > No need to make this change here, right? Do so in a later commit if you > really need to. Yes, agree. I'll update it in the next patchset. Thanks. > >>> And what commit id does this fix? Is it needed for stable kernels? If >>> so, how far back? >> We are now enabling the NVIDIA Tegra products on upstream kernel. >> The change is to add the Cypress cypd 4226 support for NVIDA Tegra >> products including Xavier AGX, Xavier Orin and the upcoming products. >> The Cypress cypd4226 is not enabled in current kernel codebase. >> And thus, we shall not need it for stable kernels and backporting, do we? > > Ok, that's fine, it was not obvious so you might want to say that in the > changelog text. Sure. Will do in the next patchset. Thanks. > > thanks, > > greg k-h > thanks, Wayne.
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c index 5c0bf48be766..fde3da0605f5 100644 --- a/drivers/usb/typec/ucsi/ucsi_ccg.c +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c @@ -122,9 +122,14 @@ struct version_format { * Firmware version 3.1.10 or earlier, built for NVIDIA has known issue * of missing interrupt when a device is connected for runtime resume */ -#define CCG_FW_BUILD_NVIDIA (('n' << 8) | 'v') +#define CCG_FW_BUILD_NVIDIA_RTX (('n' << 8) | 'v') #define CCG_OLD_FW_VERSION (CCG_VERSION(0x31) | CCG_VERSION_PATCH(10)) +/* Firmware for Tegra doesn't support UCSI ALT command, built + * for NVIDIA has known issue of reporting wrong capability info + */ +#define CCG_FW_BUILD_NVIDIA_TEGRA (('g' << 8) | 'n') + /* Altmode offset for NVIDIA Function Test Board (FTB) */ #define NVIDIA_FTB_DP_OFFSET (2) #define NVIDIA_FTB_DBG_OFFSET (3) @@ -513,6 +518,7 @@ static int ucsi_ccg_read(struct ucsi *ucsi, unsigned int offset, { struct ucsi_ccg *uc = ucsi_get_drvdata(ucsi); u16 reg = CCGX_RAB_UCSI_DATA_BLOCK(offset); + struct ucsi_capability *cap; struct ucsi_altmode *alt; int ret; @@ -536,6 +542,12 @@ static int ucsi_ccg_read(struct ucsi *ucsi, unsigned int offset, ucsi_ccg_nvidia_altmode(uc, alt); } break; + case UCSI_GET_CAPABILITY: + if (uc->fw_build == CCG_FW_BUILD_NVIDIA_TEGRA) { + cap = val; + cap->features &= ~UCSI_CAP_ALT_MODE_DETAILS; + } + break; default: break; } @@ -1452,7 +1464,7 @@ static int ucsi_ccg_runtime_resume(struct device *dev) * of missing interrupt when a device is connected for runtime resume. * Schedule a work to call ISR as a workaround. */ - if (uc->fw_build == CCG_FW_BUILD_NVIDIA && + if (uc->fw_build == CCG_FW_BUILD_NVIDIA_RTX && uc->fw_version <= CCG_OLD_FW_VERSION) schedule_work(&uc->pm_work);