| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2262853.Mh6RI2rZIc@senjougahara>
Date: Tue, 03 Feb 2026 13:32:29 +0900
From: Mikko Perttunen <mperttunen@...dia.com>
To: Svyatoslav Ryhel <clamor95@...il.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Thierry Reding <thierry.reding@...il.com>,
Thierry Reding <treding@...dia.com>, Jonathan Hunter <jonathanh@...dia.com>,
Dmitry Osipenko <digetx@...il.com>, linux-usb@...r.kernel.org,
linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] usb: phy: tegra: add HSIC support
On Monday, February 2, 2026 5:24 PM Svyatoslav Ryhel wrote:
> пн, 2 лют. 2026 р. о 10:06 Mikko Perttunen <mperttunen@...dia.com> пише:
> >
> > On Monday, February 2, 2026 3:37 PM Svyatoslav Ryhel wrote:
> > > пн, 2 лют. 2026 р. о 06:07 Mikko Perttunen <mperttunen@...dia.com> пише:
> > > >
> > > > On Friday, January 23, 2026 12:11 AM Svyatoslav Ryhel wrote:
> > > > > Add support for HSIC USB mode, which can be set for second USB controller
> > > > > and PHY on Tegra SoC along with already supported UTMI or ULPI.
> > > > >
> > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@...il.com>
> > > > > ---
> > > > > drivers/usb/phy/phy-tegra-usb.c | 249 ++++++++++++++++++++++++++++--
> > > > > include/linux/usb/tegra_usb_phy.h | 5 +
> > > > > 2 files changed, 243 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> > > > > index afa5b5535f92..4f0fde33647e 100644
> > > > > --- a/drivers/usb/phy/phy-tegra-usb.c
> > > > > +++ b/drivers/usb/phy/phy-tegra-usb.c
> > > > > @@ -29,17 +29,26 @@
> > > > > #include <linux/usb/tegra_usb_phy.h>
> > > > > #include <linux/usb/ulpi.h>
> > > > >
> > > > > +#define USB_TXFILLTUNING 0x154
> > > > > +#define USB_FIFO_TXFILL_THRES(x) (((x) & 0x1f) << 16)
> > > > > +#define USB_FIFO_TXFILL_MASK 0x1f0000
> > > > > +
> > > > > #define ULPI_VIEWPORT 0x170
> > > > >
> > > > > /* PORTSC PTS/PHCD bits, Tegra20 only */
> > > > > #define TEGRA_USB_PORTSC1 0x184
> > > > > -#define TEGRA_USB_PORTSC1_PTS(x) (((x) & 0x3) << 30)
> > > > > -#define TEGRA_USB_PORTSC1_PHCD BIT(23)
> > > > > +#define TEGRA_USB_PORTSC1_PTS(x) (((x) & 0x3) << 30)
> > > > > +#define TEGRA_USB_PORTSC1_PHCD BIT(23)
> > > > > +#define TEGRA_USB_PORTSC1_WKOC BIT(22)
> > > > > +#define TEGRA_USB_PORTSC1_WKDS BIT(21)
> > > > > +#define TEGRA_USB_PORTSC1_WKCN BIT(20)
> > > > >
> > > > > /* HOSTPC1 PTS/PHCD bits, Tegra30 and above */
> > > > > +#define TEGRA30_USB_PORTSC1 0x174
> > > > > #define TEGRA_USB_HOSTPC1_DEVLC 0x1b4
> > > > > -#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x) (((x) & 0x7) << 29)
> > > > > -#define TEGRA_USB_HOSTPC1_DEVLC_PHCD BIT(22)
> > > > > +#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x) (((x) & 0x7) << 29)
> > > > > +#define TEGRA_USB_HOSTPC1_DEVLC_PHCD BIT(22)
> > > > > +#define TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC BIT(2)
> > > >
> > > > PTS is an enumeration, not a bitfield, so I would say '4' instead of 'BIT(2)'
> > > >
> > >
> > > Noted
> > >
> > > > >
> > > > > /* Bits of PORTSC1, which will get cleared by writing 1 into them */
> > > > > #define TEGRA_PORTSC1_RWC_BITS (PORT_CSC | PORT_PEC | PORT_OCC)
> > > > > @@ -51,11 +60,12 @@
> > > > > #define USB_SUSP_CLR BIT(5)
> > > > > #define USB_PHY_CLK_VALID BIT(7)
> > > > > #define UTMIP_RESET BIT(11)
> > > > > -#define UHSIC_RESET BIT(11)
> > > > > #define UTMIP_PHY_ENABLE BIT(12)
> > > > > #define ULPI_PHY_ENABLE BIT(13)
> > > > > #define USB_SUSP_SET BIT(14)
> > > > > +#define UHSIC_RESET BIT(14)
> > > > > #define USB_WAKEUP_DEBOUNCE_COUNT(x) (((x) & 0x7) << 16)
> > > > > +#define UHSIC_PHY_ENABLE BIT(19)
> > > > >
> > > > > #define USB_PHY_VBUS_SENSORS 0x404
> > > > > #define B_SESS_VLD_WAKEUP_EN BIT(14)
> > > > > @@ -156,6 +166,58 @@
> > > > > #define UTMIP_BIAS_CFG1 0x83c
> > > > > #define UTMIP_BIAS_PDTRK_COUNT(x) (((x) & 0x1f) << 3)
> > > > >
> > > > > +/*
> > > > > + * Tegra20 has no UTMIP registers on PHY2 and UHSIC registers start from 0x800
> > > > > + * just where UTMIP registers should have been. This is the case only with Tegra20
> > > > > + * Tegra30+ have UTMIP registers at 0x800 and UHSIC registers shifter by 0x400
> > > >
> > > > 'shift', or 'are shifted'
> > > >
> > >
> > > Noted
> > >
> > > > > + * to 0xc00, but register layout is preserved.
> > > > > + */
> > > > > +#define UHSIC_PLL_CFG1 0x804
> > > > > +#define UHSIC_XTAL_FREQ_COUNT(x) (((x) & 0xfff) << 0)
> > > > > +#define UHSIC_PLLU_ENABLE_DLY_COUNT(x) (((x) & 0x1f) << 14)
> > > > > +
> > > > > +#define UHSIC_HSRX_CFG0 0x808
> > > > > +#define UHSIC_ELASTIC_UNDERRUN_LIMIT(x) (((x) & 0x1f) << 2)
> > > > > +#define UHSIC_ELASTIC_OVERRUN_LIMIT(x) (((x) & 0x1f) << 8)
> > > > > +#define UHSIC_IDLE_WAIT(x) (((x) & 0x1f) << 13)
> > > > > +
> > > > > +#define UHSIC_HSRX_CFG1 0x80c
> > > > > +#define UHSIC_HS_SYNC_START_DLY(x) (((x) & 0x1f) << 1)
> > > > > +
> > > > > +#define UHSIC_TX_CFG0 0x810
> > > > > +#define UHSIC_HS_READY_WAIT_FOR_VALID BIT(9)
> > > > > +
> > > > > +#define UHSIC_MISC_CFG0 0x814
> > > > > +#define UHSIC_SUSPEND_EXIT_ON_EDGE BIT(7)
> > > > > +#define UHSIC_DETECT_SHORT_CONNECT BIT(8)
> > > > > +#define UHSIC_FORCE_XCVR_MODE BIT(15)
> > > > > +#define UHSIC_DISABLE_BUSRESET BIT(20)
> > > > > +
> > > > > +#define UHSIC_MISC_CFG1 0x818
> > > > > +#define UHSIC_PLLU_STABLE_COUNT(x) (((x) & 0xfff) << 2)
> > > > > +
> > > > > +#define UHSIC_PADS_CFG0 0x81c
> > > > > +#define UHSIC_TX_RTUNEN 0xf000
> > > > > +#define UHSIC_TX_RTUNE(x) (((x) & 0xf) << 12)
> > > > > +
> > > > > +#define UHSIC_PADS_CFG1 0x820
> > > > > +#define UHSIC_PD_BG BIT(2)
> > > > > +#define UHSIC_PD_TX BIT(3)
> > > > > +#define UHSIC_PD_TRK BIT(4)
> > > > > +#define UHSIC_PD_RX BIT(5)
> > > > > +#define UHSIC_PD_ZI BIT(6)
> > > > > +#define UHSIC_RX_SEL BIT(7)
> > > > > +#define UHSIC_RPD_DATA BIT(9)
> > > > > +#define UHSIC_RPD_STROBE BIT(10)
> > > > > +#define UHSIC_RPU_DATA BIT(11)
> > > > > +#define UHSIC_RPU_STROBE BIT(12)
> > > > > +
> > > > > +#define UHSIC_CMD_CFG0 0x824
> > > > > +#define UHSIC_PRETEND_CONNECT_DETECT BIT(5)
> > > > > +
> > > > > +#define UHSIC_STAT_CFG0 0x828
> > > > > +#define UHSIC_CONNECT_DETECT BIT(0)
> > > > > +
> > > > > /* For Tegra30 and above only, the address is different in Tegra20 */
> > > > > #define USB_USBMODE 0x1f8
> > > > > #define USB_USBMODE_MASK (3 << 0)
> > > > > @@ -174,7 +236,8 @@ struct tegra_xtal_freq {
> > > > > u8 enable_delay;
> > > > > u8 stable_count;
> > > > > u8 active_delay;
> > > > > - u8 xtal_freq_count;
> > > > > + u8 utmi_xtal_freq_count;
> > > > > + u16 hsic_xtal_freq_count;
> > > > > u16 debounce;
> > > > > };
> > > > >
> > > > > @@ -184,7 +247,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > > > .enable_delay = 0x02,
> > > > > .stable_count = 0x2F,
> > > > > .active_delay = 0x04,
> > > > > - .xtal_freq_count = 0x76,
> > > > > + .utmi_xtal_freq_count = 0x76,
> > > > > + .hsic_xtal_freq_count = 0x1CA,
> > > > > .debounce = 0x7530,
> > > > > },
> > > > > {
> > > > > @@ -192,7 +256,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > > > .enable_delay = 0x02,
> > > > > .stable_count = 0x33,
> > > > > .active_delay = 0x05,
> > > > > - .xtal_freq_count = 0x7F,
> > > > > + .utmi_xtal_freq_count = 0x7F,
> > > > > + .hsic_xtal_freq_count = 0x1F0,
> > > > > .debounce = 0x7EF4,
> > > > > },
> > > > > {
> > > > > @@ -200,7 +265,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > > > .enable_delay = 0x03,
> > > > > .stable_count = 0x4B,
> > > > > .active_delay = 0x06,
> > > > > - .xtal_freq_count = 0xBB,
> > > > > + .utmi_xtal_freq_count = 0xBB,
> > > > > + .hsic_xtal_freq_count = 0x2DD,
> > > > > .debounce = 0xBB80,
> > > > > },
> > > > > {
> > > > > @@ -208,7 +274,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > > > .enable_delay = 0x04,
> > > > > .stable_count = 0x66,
> > > > > .active_delay = 0x09,
> > > > > - .xtal_freq_count = 0xFE,
> > > > > + .utmi_xtal_freq_count = 0xFE,
> > > > > + .hsic_xtal_freq_count = 0x3E0,
> > > > > .debounce = 0xFDE8,
> > > > > },
> > > > > };
> > > > > @@ -541,7 +608,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
> > > > > val = readl_relaxed(base + UTMIP_PLL_CFG1);
> > > > > val &= ~(UTMIP_XTAL_FREQ_COUNT(~0) |
> > > > > UTMIP_PLLU_ENABLE_DLY_COUNT(~0));
> > > > > - val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->xtal_freq_count) |
> > > > > + val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->utmi_xtal_freq_count) |
> > > > > UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> > > > > writel_relaxed(val, base + UTMIP_PLL_CFG1);
> > > > > }
> > > > > @@ -812,6 +879,153 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static u32 tegra_hsic_readl(struct tegra_usb_phy *phy, u32 reg)
> > > > > +{
> > > > > + void __iomem *base = phy->regs;
> > > > > + u32 shift = phy->soc_config->uhsic_registers_shift;
> > > > > +
> > > > > + return readl_relaxed(base + shift + reg);
> > > > > +}
> > > > > +
> > > > > +static void tegra_hsic_writel(struct tegra_usb_phy *phy, u32 reg, u32 value)
> > > > > +{
> > > > > + void __iomem *base = phy->regs;
> > > > > + u32 shift = phy->soc_config->uhsic_registers_shift;
> > > > > +
> > > > > + writel_relaxed(value, base + shift + reg);
> > > > > +}
> > > > > +
> > > > > +static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
> > > > > +{
> > > > > + struct tegra_utmip_config *config = phy->config;
> > > > > + void __iomem *base = phy->regs;
> > > > > + u32 val;
> > > > > +
> > > > > + val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > > > > + val &= ~(UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > > > > + UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > > > > + val |= UHSIC_RX_SEL;
> > > >
> > > > L4T r16 (Tegra30) keeps UHSIC_PD_TX set until after UHSIC_RESET has been cleared. Commit message says this avoids a signal glitch.
> > > >
> > >
> > > I did not notice any difference with this change and without. Since it
> > > does not affect detection of modem by my device I can apply it (is it
> > > worth applying at all?). Should it be applied globally or for Tegra30+
> > > only?
> >
> > Downstream only has it for Tegra30, but that's probably just because it was tested in the Tegra30 timeframe. If it's not causing any issue on Tegra20 I would apply it globally. Considering it's a signal glitch it might only have an effect in marginal signal situations.
> >
> > >
> > > > > + tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > > > > +
> > > > > + udelay(2);
> > > > > +
> > > > > + val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > + val |= UHSIC_RESET;
> > > > > + writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > + udelay(30);
> > > > > +
> > > > > + val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > + val |= UHSIC_PHY_ENABLE;
> > > > > + writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > + val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG0);
> > > > > + val &= ~(UHSIC_IDLE_WAIT(~0) |
> > > > > + UHSIC_ELASTIC_UNDERRUN_LIMIT(~0) |
> > > > > + UHSIC_ELASTIC_OVERRUN_LIMIT(~0));
> > > > > + val |= UHSIC_IDLE_WAIT(config->idle_wait_delay) |
> > > > > + UHSIC_ELASTIC_UNDERRUN_LIMIT(config->elastic_limit) |
> > > > > + UHSIC_ELASTIC_OVERRUN_LIMIT(config->elastic_limit);
> > > > > + tegra_hsic_writel(phy, UHSIC_HSRX_CFG0, val);
> > > > > +
> > > > > + val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG1);
> > > > > + val &= ~UHSIC_HS_SYNC_START_DLY(~0);
> > > > > + val |= UHSIC_HS_SYNC_START_DLY(config->hssync_start_delay);
> > > > > + tegra_hsic_writel(phy, UHSIC_HSRX_CFG1, val);
> > > > > +
> > > >
> > > > L4T r16 (Tegra30) clears UHSIC_TX_CFG0.UHSIC_HS_READY_WAIT_FOR_VALID here. According to commit message this fixes an intermittent failure to connect to HSIC modem. It also sets UHSIC_MISC_CFG0.UHSIC_DISABLE_BUSRESET below due to same issue.
> > > >
> > >
> > > And this change causes my modem to fail entirely.
> > >
> > > [ 10.145976] usb 1-1: new high-speed USB device number 2 using ci_hdrc
> > > [ 10.295970] usb 1-1: device descriptor read/64, error -71
> > > [ 10.545975] usb 1-1: device descriptor read/64, error -71
> > > [ 10.795990] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> > > [ 10.935970] usb 1-1: device descriptor read/64, error -71
> > > [ 11.185977] usb 1-1: device descriptor read/64, error -71
> > >
> > > I assume UHSIC_DISABLE_BUSRESET, UHSIC_HS_READY_WAIT_FOR_VALID and
> > > UHSIC_PD_TX workarounds are all consequences of how modem in
> > > downstream kernel is called. Instead of relaying on modem to lead the
> > > controller probe, downstream just brutally removes and reinits
> > > controller until modem probes. I have never observed modem not
> > > appearing without any of discussed patches.
> >
> > The downstream commit says that without this workaround, the modem would sometimes not come up in stress tests. So I think it's possible there is still a rare hardware bug that this is working around, but perhaps we're missing some other part and that's why it's not working. I think it's fine to drop these changes.
> >
>
> I propose not adding any of the three workarounds for now. I don't
> have a Tegra20 with an HSIC modem, as it seems that most Tegra20
> devices use ULPI mode. Among my personal devices, only the LG Optimus
> Vu (P895) is a GSM device with an XMM HSIC modem, and it fails if I
> add UHSIC_DISABLE_BUSRESET and UHSIC_HS_READY_WAIT_FOR_VALID. Since I
> may be the only one at the moment actively working in this area of
> legacy Tegra SoCs — and this discussion is public — anyone who has
> issues can contact me to resolve them. Additionally, I will keep in
> mind that there may be a need for these workarounds; if I find any
> need for them during modem bring-ups in the legacy Tegra community, I
> will submit dedicated patches with explanations.
Sure, that sounds fine.
Mikko
>
> > >
> > >
> > > > > + val = tegra_hsic_readl(phy, UHSIC_MISC_CFG0);
> > > > > + val |= UHSIC_SUSPEND_EXIT_ON_EDGE;
> > > > > + tegra_hsic_writel(phy, UHSIC_MISC_CFG0, val);
> > > > > +
> > > > > + val = tegra_hsic_readl(phy, UHSIC_MISC_CFG1);
> > > > > + val &= ~UHSIC_PLLU_STABLE_COUNT(~0);
> > > > > + val |= UHSIC_PLLU_STABLE_COUNT(phy->freq->stable_count);
> > > > > + tegra_hsic_writel(phy, UHSIC_MISC_CFG1, val);
> > > > > +
> > > > > + val = tegra_hsic_readl(phy, UHSIC_PLL_CFG1);
> > > > > + val &= ~(UHSIC_XTAL_FREQ_COUNT(~0) |
> > > > > + UHSIC_PLLU_ENABLE_DLY_COUNT(~0));
> > > > > + val |= UHSIC_XTAL_FREQ_COUNT(phy->freq->hsic_xtal_freq_count) |
> > > > > + UHSIC_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> > > > > + tegra_hsic_writel(phy, UHSIC_PLL_CFG1, val);
> > > > > +
> > > > > + val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > + val &= ~UHSIC_RESET;
> > > > > + writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > + udelay(2);
> > > > > +
> > > > > + if (phy->soc_config->requires_usbmode_setup) {
> > > > > + val = readl_relaxed(base + USB_USBMODE);
> > > > > + val &= ~USB_USBMODE_MASK;
> > > > > + if (phy->mode == USB_DR_MODE_HOST)
> > > > > + val |= USB_USBMODE_HOST;
> > > > > + else
> > > > > + val |= USB_USBMODE_DEVICE;
> > > > > + writel_relaxed(val, base + USB_USBMODE);
> > > > > + }
> > > > > +
> > > > > + if (phy->soc_config->has_hostpc)
> > > > > + set_pts(phy, TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC);
> > > > > + else
> > > > > + set_pts(phy, 0);
> > > >
> > > > This (and below) are abusing has_hostpc to detect Tegra30 vs Tegra20. We should instead add soc_config fields hsic_pts_value and portsc1_offset.
> > > >
> > > > > +
> > > > > + val = readl_relaxed(base + USB_TXFILLTUNING);
> > > > > + if ((val & USB_FIFO_TXFILL_MASK) != USB_FIFO_TXFILL_THRES(0x10)) {
> > > > > + val = USB_FIFO_TXFILL_THRES(0x10);
> > > > > + writel_relaxed(val, base + USB_TXFILLTUNING);
> > > > > + }
> > > > > +
> > > > > + if (phy->soc_config->has_hostpc) {
> > > > > + val = readl_relaxed(base + TEGRA30_USB_PORTSC1);
> > > > > + val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > > > > + TEGRA_USB_PORTSC1_WKCN);
> > > > > + writel_relaxed(val, base + TEGRA30_USB_PORTSC1);
> > > > > + } else {
> > > > > + val = readl_relaxed(base + TEGRA_USB_PORTSC1);
> > > > > + val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > > > > + TEGRA_USB_PORTSC1_WKCN);
> > > > > + writel_relaxed(val, base + TEGRA_USB_PORTSC1);
> > > > > + }
> > > > > +
> > > > > + val = tegra_hsic_readl(phy, UHSIC_PADS_CFG0);
> > > > > + val &= ~UHSIC_TX_RTUNEN;
> > > > > + val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
> > > > > + tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
> > > > > +
> > > > > + if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> > > > > + USB_PHY_CLK_VALID))
> > > > > + dev_err(phy->u_phy.dev,
> > > > > + "Timeout waiting for PHY to stabilize on enable (HSIC)\n");
> > > >
> > > > Should return error (return value of utmi_wait_register) here?
> > > >
> > >
> > > Noted.
> > >
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int uhsic_phy_power_off(struct tegra_usb_phy *phy)
> > > > > +{
> > > > > + void __iomem *base = phy->regs;
> > > > > + u32 val;
> > > > > +
> > > > > + set_phcd(phy, true);
> > > > > +
> > > > > + val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > > > > + val |= (UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > > > > + UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > > > > + tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > > > > +
> > > > > + val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > + val |= UHSIC_RESET;
> > > > > + writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > + udelay(30);
> > > > > +
> > > > > + val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > + val &= ~UHSIC_PHY_ENABLE;
> > > > > + writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> > > > > {
> > > > > int err = 0;
> > > > > @@ -828,6 +1042,10 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> > > > > err = ulpi_phy_power_on(phy);
> > > > > break;
> > > > >
> > > > > + case USBPHY_INTERFACE_MODE_HSIC:
> > > > > + err = uhsic_phy_power_on(phy);
> > > > > + break;
> > > > > +
> > > > > default:
> > > > > break;
> > > > > }
> > > > > @@ -859,6 +1077,10 @@ static int tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
> > > > > err = ulpi_phy_power_off(phy);
> > > > > break;
> > > > >
> > > > > + case USBPHY_INTERFACE_MODE_HSIC:
> > > > > + err = uhsic_phy_power_off(phy);
> > > > > + break;
> > > > > +
> > > > > default:
> > > > > break;
> > > > > }
> > > > > @@ -1256,6 +1478,8 @@ static const struct tegra_phy_soc_config tegra20_soc_config = {
> > > > > .requires_usbmode_setup = false,
> > > > > .requires_extra_tuning_parameters = false,
> > > > > .requires_pmc_ao_power_up = false,
> > > > > + .uhsic_registers_shift = 0,
> > > > > + .uhsic_tx_rtune = 0, /* 40 ohm */
> > > > > };
> > > > >
> > > > > static const struct tegra_phy_soc_config tegra30_soc_config = {
> > > > > @@ -1264,6 +1488,8 @@ static const struct tegra_phy_soc_config tegra30_soc_config = {
> > > > > .requires_usbmode_setup = true,
> > > > > .requires_extra_tuning_parameters = true,
> > > > > .requires_pmc_ao_power_up = true,
> > > > > + .uhsic_registers_shift = 0x400,
> > > > > + .uhsic_tx_rtune = 8, /* 50 ohm */
> > > > > };
> > > > >
> > > > > static const struct of_device_id tegra_usb_phy_id_table[] = {
> > > > > @@ -1360,6 +1586,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
> > > > > tegra_phy->phy_type = of_usb_get_phy_mode(np);
> > > > > switch (tegra_phy->phy_type) {
> > > > > case USBPHY_INTERFACE_MODE_UTMI:
> > > > > + case USBPHY_INTERFACE_MODE_HSIC:
> > > > > err = utmi_phy_probe(tegra_phy, pdev);
> > > > > if (err)
> > > > > return err;
> > > > > diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
> > > > > index e394f4880b7e..4e79f1c2173a 100644
> > > > > --- a/include/linux/usb/tegra_usb_phy.h
> > > > > +++ b/include/linux/usb/tegra_usb_phy.h
> > > > > @@ -24,6 +24,9 @@ struct gpio_desc;
> > > > > * requires_extra_tuning_parameters: true if xcvr_hsslew, hssquelch_level
> > > > > * and hsdiscon_level should be set for adequate signal quality
> > > > > * requires_pmc_ao_power_up: true if USB AO is powered down by default
> > > > > + * uhsic_registers_shift: for Tegra30+ where HSIC registers were shifted
> > > > > + * comparing to Tegra20 by 0x400, since Tegra20 has no UTMIP on PHY2
> > > > > + * uhsic_tx_rtune: fine tuned 50 Ohm termination resistor for NMOS/PMOS driver
> > > > > */
> > > > >
> > > > > struct tegra_phy_soc_config {
> > > > > @@ -32,6 +35,8 @@ struct tegra_phy_soc_config {
> > > > > bool requires_usbmode_setup;
> > > > > bool requires_extra_tuning_parameters;
> > > > > bool requires_pmc_ao_power_up;
> > > > > + u32 uhsic_registers_shift;
> > > >
> > > > I would rather call this 'uhsic_registers_offset'. Shift first brings to mind bitshifts, and we often have fields in these config structs for bit shifts called '*_shift'.
> > > >
> > >
> > > Yes, offset seems more appropriate.
> > >
> > > > > + u32 uhsic_tx_rtune;
> > > > > };
> > > > >
> > > > > struct tegra_utmip_config {
> > > > >
> > >
> > > Mikko, thank you for your review, since HSIC patches already were
> > > picked into linux/next I will prepare a few followup patches to
> > > address some of uncertainties.
> >
> > Thank you!
> >
> >
> >
Powered by blists - more mailing lists