| 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: <a231fef1-9b50-7ac0-add4-92113a8f7dc6@quicinc.com>
Date: Sat, 24 Jun 2023 12:45:56 +0530
From: Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Felipe Balbi <balbi@...nel.org>,
"Wesley Cheng" <quic_wcheng@...cinc.com>,
Johan Hovold <johan@...nel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"quic_pkondeti@...cinc.com" <quic_pkondeti@...cinc.com>,
"quic_ppratap@...cinc.com" <quic_ppratap@...cinc.com>,
"quic_jackp@...cinc.com" <quic_jackp@...cinc.com>,
"quic_harshq@...cinc.com" <quic_harshq@...cinc.com>,
"ahalaney@...hat.com" <ahalaney@...hat.com>,
"quic_shazhuss@...cinc.com" <quic_shazhuss@...cinc.com>
Subject: Re: [PATCH v9 05/10] usb: dwc3: core: Refactor PHY logic to support
Multiport Controller
On 6/24/2023 4:25 AM, Thinh Nguyen wrote:
> On Wed, Jun 21, 2023, Krishna Kurapati wrote:
>> Currently the DWC3 driver supports only single port controller
>> which requires at most one HS and one SS PHY.
>>
>> But the DWC3 USB controller can be connected to multiple ports and
>> each port can have their own PHYs. Each port of the multiport
>> controller can either be HS+SS capable or HS only capable
>> Proper quantification of them is required to modify GUSB2PHYCFG
>> and GUSB3PIPECTL registers appropriately.
>>
>> Add support for detecting, obtaining and configuring phy's supported
>> by a multiport controller and limit the max number of ports
>> supported to 4.
>>
>> Signed-off-by: Harsh Agarwal <quic_harshq@...cinc.com>
>> [Krishna: Modifed logic for generic phy and rebased the patch]
>> Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
>> ---
>> drivers/usb/dwc3/core.c | 252 ++++++++++++++++++++++++++++------------
>> drivers/usb/dwc3/core.h | 11 +-
>> drivers/usb/dwc3/drd.c | 15 ++-
>> 3 files changed, 192 insertions(+), 86 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index e1ebae54454f..2ac72525de7d 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -120,10 +120,11 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
>> static void __dwc3_set_mode(struct work_struct *work)
>> {
>> struct dwc3 *dwc = work_to_dwc(work);
>> + u32 desired_dr_role;
>> unsigned long flags;
>> int ret;
>> u32 reg;
>> - u32 desired_dr_role;
>> + int i;
>>
>> mutex_lock(&dwc->mutex);
>> spin_lock_irqsave(&dwc->lock, flags);
>> @@ -201,8 +202,10 @@ static void __dwc3_set_mode(struct work_struct *work)
>> } else {
>> if (dwc->usb2_phy)
>> otg_set_vbus(dwc->usb2_phy->otg, true);
>> - phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>> - phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
>> + phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
>> + }
>> if (dwc->dis_split_quirk) {
>> reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
>> reg |= DWC3_GUCTL3_SPLITDISABLE;
>> @@ -217,8 +220,8 @@ static void __dwc3_set_mode(struct work_struct *work)
>>
>> if (dwc->usb2_phy)
>> otg_set_vbus(dwc->usb2_phy->otg, false);
>> - phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
>> - phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
>> + phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
>> + phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>>
>> ret = dwc3_gadget_init(dwc);
>> if (ret)
>> @@ -587,22 +590,14 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
>> return ret;
>> }
>>
>> -/**
>> - * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
>> - * @dwc: Pointer to our controller context structure
>> - *
>> - * Returns 0 on success. The USB PHY interfaces are configured but not
>> - * initialized. The PHY interfaces and the PHYs get initialized together with
>> - * the core in dwc3_core_init.
>> - */
>> -static int dwc3_phy_setup(struct dwc3 *dwc)
>> +static int dwc3_ss_phy_setup(struct dwc3 *dwc, int index)
>> {
>> unsigned int hw_mode;
>> u32 reg;
>>
>> hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>>
>> - reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
>> + reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(index));
>>
>> /*
>> * Make sure UX_EXIT_PX is cleared as that causes issues with some
>> @@ -657,9 +652,19 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>> if (dwc->dis_del_phy_power_chg_quirk)
>> reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
>>
>> - dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
>> + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(index), reg);
>> +
>> + return 0;
>> +}
>> +
>> +static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index)
>> +{
>> + unsigned int hw_mode;
>> + u32 reg;
>>
>> - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>> +
>> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
>>
>> /* Select the HS PHY interface */
>> switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
>> @@ -671,7 +676,7 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>> } else if (dwc->hsphy_interface &&
>> !strncmp(dwc->hsphy_interface, "ulpi", 4)) {
>> reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
>> - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
>> } else {
>> /* Relying on default value. */
>> if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
>> @@ -738,7 +743,35 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>> if (dwc->ulpi_ext_vbus_drv)
>> reg |= DWC3_GUSB2PHYCFG_ULPIEXTVBUSDRV;
>>
>> - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
>> + * @dwc: Pointer to our controller context structure
>> + *
>> + * Returns 0 on success. The USB PHY interfaces are configured but not
>> + * initialized. The PHY interfaces and the PHYs get initialized together with
>> + * the core in dwc3_core_init.
>> + */
>> +static int dwc3_phy_setup(struct dwc3 *dwc)
>> +{
>> + int i;
>> + int ret;
>> +
>> + for (i = 0; i < dwc->num_usb3_ports; i++) {
>> + ret = dwc3_ss_phy_setup(dwc, i);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + ret = dwc3_hs_phy_setup(dwc, i);
>> + if (ret)
>> + return ret;
>> + }
>>
>> return 0;
>> }
>> @@ -746,23 +779,34 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>> static int dwc3_phy_init(struct dwc3 *dwc)
>> {
>> int ret;
>> + int i;
>> + int j;
>>
>> usb_phy_init(dwc->usb2_phy);
>> usb_phy_init(dwc->usb3_phy);
>>
>> - ret = phy_init(dwc->usb2_generic_phy);
>> - if (ret < 0)
>> - goto err_shutdown_usb3_phy;
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + ret = phy_init(dwc->usb2_generic_phy[i]);
>> + if (ret < 0)
>> + goto err_exit_usb2_phy;
>> + }
>>
>> - ret = phy_init(dwc->usb3_generic_phy);
>> - if (ret < 0)
>> - goto err_exit_usb2_phy;
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + ret = phy_init(dwc->usb3_generic_phy[i]);
>> + if (ret < 0)
>> + goto err_exit_usb3_phy;
>> + }
>>
>> return 0;
>>
>> +err_exit_usb3_phy:
>> + for (j = i-1; j >= 0; j--)
>
> Minor nits, can we add spacing around the '-' for here and other places.
> Checkpatch should be able to catch this.
>
Hi Thinh,
Checkpatch actually caught this, but the only reason I didn't fix this
single nit is it looked like too much spaces in a single for loop, (I
know it doesn't sound convincing). I will fix this nit in next version.
Regards,
Krishna,
>> + phy_exit(dwc->usb3_generic_phy[j]);
>> + i = dwc->num_usb2_ports;
>> err_exit_usb2_phy:
>> - phy_exit(dwc->usb2_generic_phy);
>> -err_shutdown_usb3_phy:
>> + for (j = i-1; j >= 0; j--)
>> + phy_exit(dwc->usb2_generic_phy[j]);
>> +
>> usb_phy_shutdown(dwc->usb3_phy);
>> usb_phy_shutdown(dwc->usb2_phy);
>>
>> @@ -771,8 +815,12 @@ static int dwc3_phy_init(struct dwc3 *dwc)
>>
>> static void dwc3_phy_exit(struct dwc3 *dwc)
>> {
>> - phy_exit(dwc->usb3_generic_phy);
>> - phy_exit(dwc->usb2_generic_phy);
>> + int i;
>> +
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + phy_exit(dwc->usb3_generic_phy[i]);
>> + phy_exit(dwc->usb2_generic_phy[i]);
>> + }
>>
>> usb_phy_shutdown(dwc->usb3_phy);
>> usb_phy_shutdown(dwc->usb2_phy);
>> @@ -781,23 +829,34 @@ static void dwc3_phy_exit(struct dwc3 *dwc)
>> static int dwc3_phy_power_on(struct dwc3 *dwc)
>> {
>> int ret;
>> + int i;
>> + int j;
>>
>> usb_phy_set_suspend(dwc->usb2_phy, 0);
>> usb_phy_set_suspend(dwc->usb3_phy, 0);
>>
>> - ret = phy_power_on(dwc->usb2_generic_phy);
>> - if (ret < 0)
>> - goto err_suspend_usb3_phy;
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + ret = phy_power_on(dwc->usb2_generic_phy[i]);
>> + if (ret < 0)
>> + goto err_power_off_usb2_phy;
>> + }
>>
>> - ret = phy_power_on(dwc->usb3_generic_phy);
>> - if (ret < 0)
>> - goto err_power_off_usb2_phy;
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + ret = phy_power_on(dwc->usb3_generic_phy[i]);
>> + if (ret < 0)
>> + goto err_power_off_usb3_phy;
>> + }
>>
>> return 0;
>>
>> +err_power_off_usb3_phy:
>> + for (j = i-1; j >= 0; j--)
>> + phy_power_off(dwc->usb3_generic_phy[i]);
>> + i = dwc->num_usb2_ports;
>> err_power_off_usb2_phy:
>> - phy_power_off(dwc->usb2_generic_phy);
>> -err_suspend_usb3_phy:
>> + for (j = i-1; j >= 0; j--)
>> + phy_power_off(dwc->usb2_generic_phy[i]);
>> +
>> usb_phy_set_suspend(dwc->usb3_phy, 1);
>> usb_phy_set_suspend(dwc->usb2_phy, 1);
>>
>> @@ -806,8 +865,12 @@ static int dwc3_phy_power_on(struct dwc3 *dwc)
>>
>> static void dwc3_phy_power_off(struct dwc3 *dwc)
>> {
>> - phy_power_off(dwc->usb3_generic_phy);
>> - phy_power_off(dwc->usb2_generic_phy);
>> + int i;
>> +
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + phy_power_off(dwc->usb3_generic_phy[i]);
>> + phy_power_off(dwc->usb2_generic_phy[i]);
>> + }
>>
>> usb_phy_set_suspend(dwc->usb3_phy, 1);
>> usb_phy_set_suspend(dwc->usb2_phy, 1);
>> @@ -1080,6 +1143,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>> unsigned int hw_mode;
>> u32 reg;
>> int ret;
>> + int i;
>>
>> hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>>
>> @@ -1123,15 +1187,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
>> if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
>> !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
>> if (!dwc->dis_u3_susphy_quirk) {
>> - reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
>> - reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>> - dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
>> + for (i = 0; i < dwc->num_usb3_ports; i++) {
>> + reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
>> + reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>> + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
>> + }
>> }
>>
>> if (!dwc->dis_u2_susphy_quirk) {
>> - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> - reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>> - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>> + reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>> + }
>> }
>> }
>>
>> @@ -1290,7 +1358,9 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>> {
>> struct device *dev = dwc->dev;
>> struct device_node *node = dev->of_node;
>> + char phy_name[11];
>> int ret;
>> + int i;
>>
>> if (node) {
>> dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
>> @@ -1316,22 +1386,36 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>> return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>> }
>>
>> - dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
>> - if (IS_ERR(dwc->usb2_generic_phy)) {
>> - ret = PTR_ERR(dwc->usb2_generic_phy);
>> - if (ret == -ENOSYS || ret == -ENODEV)
>> - dwc->usb2_generic_phy = NULL;
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + if (dwc->num_usb2_ports == 1)
>> + sprintf(phy_name, "usb2-phy");
>> else
>> - return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>> - }
>> + sprintf(phy_name, "usb2-port%d", i);
>> +
>> + dwc->usb2_generic_phy[i] = devm_phy_get(dev, phy_name);
>> + if (IS_ERR(dwc->usb2_generic_phy[i])) {
>> + ret = PTR_ERR(dwc->usb2_generic_phy[i]);
>> + if (ret == -ENOSYS || ret == -ENODEV)
>> + dwc->usb2_generic_phy[i] = NULL;
>> + else
>> + return dev_err_probe(dev, ret,
>> + "no %s phy configured\n", phy_name);
>> + }
>>
>> - dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
>> - if (IS_ERR(dwc->usb3_generic_phy)) {
>> - ret = PTR_ERR(dwc->usb3_generic_phy);
>> - if (ret == -ENOSYS || ret == -ENODEV)
>> - dwc->usb3_generic_phy = NULL;
>> + if (dwc->num_usb2_ports == 1)
>> + sprintf(phy_name, "usb3-phy");
>> else
>> - return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>> + sprintf(phy_name, "usb3-port%d", i);
>> +
>> + dwc->usb3_generic_phy[i] = devm_phy_get(dev, phy_name);
>> + if (IS_ERR(dwc->usb3_generic_phy[i])) {
>> + ret = PTR_ERR(dwc->usb3_generic_phy[i]);
>> + if (ret == -ENOSYS || ret == -ENODEV)
>> + dwc->usb3_generic_phy[i] = NULL;
>> + else
>> + return dev_err_probe(dev, ret,
>> + "no %s phy configured\n", phy_name);
>> + }
>> }
>>
>> return 0;
>> @@ -1341,6 +1425,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>> {
>> struct device *dev = dwc->dev;
>> int ret;
>> + int i;
>>
>> switch (dwc->dr_mode) {
>> case USB_DR_MODE_PERIPHERAL:
>> @@ -1348,8 +1433,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>>
>> if (dwc->usb2_phy)
>> otg_set_vbus(dwc->usb2_phy->otg, false);
>> - phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
>> - phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
>> + phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
>> + phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>>
>> ret = dwc3_gadget_init(dwc);
>> if (ret)
>> @@ -1360,8 +1445,10 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>>
>> if (dwc->usb2_phy)
>> otg_set_vbus(dwc->usb2_phy->otg, true);
>> - phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>> - phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
>> + phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
>> + }
>>
>> ret = dwc3_host_init(dwc);
>> if (ret)
>> @@ -1821,6 +1908,9 @@ static int dwc3_read_port_info(struct dwc3 *dwc)
>> dev_dbg(dwc->dev, "hs-ports: %u ss-ports: %u\n",
>> dwc->num_usb2_ports, dwc->num_usb3_ports);
>>
>> + if (dwc->num_usb2_ports > DWC3_MAX_PORTS)
>> + ret = -ENOMEM;
>> +
>> iounmap(base);
>> return ret;
>> }
>> @@ -2057,6 +2147,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>> {
>> unsigned long flags;
>> u32 reg;
>> + int i;
>>
>> switch (dwc->current_dr_role) {
>> case DWC3_GCTL_PRTCAP_DEVICE:
>> @@ -2075,17 +2166,21 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>> /* Let controller to suspend HSPHY before PHY driver suspends */
>> if (dwc->dis_u2_susphy_quirk ||
>> dwc->dis_enblslpm_quirk) {
>> - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> - reg |= DWC3_GUSB2PHYCFG_ENBLSLPM |
>> - DWC3_GUSB2PHYCFG_SUSPHY;
>> - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>> + reg |= DWC3_GUSB2PHYCFG_ENBLSLPM |
>> + DWC3_GUSB2PHYCFG_SUSPHY;
>> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>> + }
>>
>> /* Give some time for USB2 PHY to suspend */
>> usleep_range(5000, 6000);
>> }
>>
>> - phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
>> - phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
>> + phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
>> + }
>> break;
>> case DWC3_GCTL_PRTCAP_OTG:
>> /* do nothing during runtime_suspend */
>> @@ -2115,6 +2210,7 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>> unsigned long flags;
>> int ret;
>> u32 reg;
>> + int i;
>>
>> switch (dwc->current_dr_role) {
>> case DWC3_GCTL_PRTCAP_DEVICE:
>> @@ -2134,17 +2230,21 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>> break;
>> }
>> /* Restore GUSB2PHYCFG bits that were modified in suspend */
>> - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> - if (dwc->dis_u2_susphy_quirk)
>> - reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>
> Nit: Extra spacing before the "for"?
>
>> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>> + if (dwc->dis_u2_susphy_quirk)
>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>
>> - if (dwc->dis_enblslpm_quirk)
>> - reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>> + if (dwc->dis_enblslpm_quirk)
>> + reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>>
>> - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>> + }
>>
>> - phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
>> - phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
>> + phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
>> + }
>> break;
>> case DWC3_GCTL_PRTCAP_OTG:
>> /* nothing to do on runtime_resume */
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 42fb17aa66fa..b2bab23ca22b 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -37,6 +37,9 @@
>> #define XHCI_EXT_PORT_MINOR(x) (((x) >> 16) & 0xff)
>> #define XHCI_EXT_PORT_COUNT(x) (((x) >> 8) & 0xff)
>>
>> +/* Number of ports supported by a multiport controller */
>> +#define DWC3_MAX_PORTS 4
>> +
>> #define DWC3_MSG_MAX 500
>>
>> /* Global constants */
>> @@ -1031,8 +1034,8 @@ struct dwc3_scratchpad_array {
>> * @usb_psy: pointer to power supply interface.
>> * @usb2_phy: pointer to USB2 PHY
>> * @usb3_phy: pointer to USB3 PHY
>> - * @usb2_generic_phy: pointer to USB2 PHY
>> - * @usb3_generic_phy: pointer to USB3 PHY
>> + * @usb2_generic_phy: pointer to array of USB2 PHY
>> + * @usb3_generic_phy: pointer to array of USB3 PHY
>> * @num_usb2_ports: number of USB2 ports.
>> * @num_usb3_ports: number of USB3 ports.
>> * @phys_ready: flag to indicate that PHYs are ready
>> @@ -1171,8 +1174,8 @@ struct dwc3 {
>> struct usb_phy *usb2_phy;
>> struct usb_phy *usb3_phy;
>>
>> - struct phy *usb2_generic_phy;
>> - struct phy *usb3_generic_phy;
>> + struct phy *usb2_generic_phy[DWC3_MAX_PORTS];
>> + struct phy *usb3_generic_phy[DWC3_MAX_PORTS];
>>
>> u8 num_usb2_ports;
>> u8 num_usb3_ports;
>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>> index 039bf241769a..18a247ff75ac 100644
>> --- a/drivers/usb/dwc3/drd.c
>> +++ b/drivers/usb/dwc3/drd.c
>> @@ -327,10 +327,11 @@ static void dwc3_otg_device_exit(struct dwc3 *dwc)
>>
>> void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>> {
>> + unsigned long flags;
>> int ret;
>> u32 reg;
>> int id;
>> - unsigned long flags;
>> + int i;
>>
>> if (dwc->dr_mode != USB_DR_MODE_OTG)
>> return;
>> @@ -386,9 +387,11 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>> } else {
>> if (dwc->usb2_phy)
>> otg_set_vbus(dwc->usb2_phy->otg, true);
>> - if (dwc->usb2_generic_phy)
>> - phy_set_mode(dwc->usb2_generic_phy,
>> - PHY_MODE_USB_HOST);
>> + for (i = 0; i < dwc->num_usb2_ports; i++) {
>> + if (dwc->usb2_generic_phy[i])
>> + phy_set_mode(dwc->usb2_generic_phy[i],
>> + PHY_MODE_USB_HOST);
>> + }
>> }
>> break;
>> case DWC3_OTG_ROLE_DEVICE:
>> @@ -400,8 +403,8 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>>
>> if (dwc->usb2_phy)
>> otg_set_vbus(dwc->usb2_phy->otg, false);
>> - if (dwc->usb2_generic_phy)
>> - phy_set_mode(dwc->usb2_generic_phy,
>> + if (dwc->usb2_generic_phy[0])
>> + phy_set_mode(dwc->usb2_generic_phy[0],
>> PHY_MODE_USB_DEVICE);
>> ret = dwc3_gadget_init(dwc);
>> if (ret)
>> --
>> 2.40.0
>>
>
> After fixing the minor nits mentioned above, you can add my Ack:
>
> Acked-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
>
> Thanks,
> Thinh
Powered by blists - more mailing lists