linux-kernel - Re: [PATCH v11 06/13] usb: dwc3: core: Refactor PHY logic to support Multiport Controller

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <565edf1c-96a5-83e3-50ce-9c328f457abf@quicinc.com>
Date:   Sat, 2 Sep 2023 03:24:46 +0530
From:   Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
To:     Wesley Cheng <quic_wcheng@...cinc.com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        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>,
        "Johan Hovold" <johan@...nel.org>,
        Mathias Nyman <mathias.nyman@...el.com>
CC:     <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <quic_pkondeti@...cinc.com>, <quic_ppratap@...cinc.com>,
        <quic_jackp@...cinc.com>, <ahalaney@...hat.com>,
        <quic_shazhuss@...cinc.com>,
        Harsh Agarwal <quic_harshq@...cinc.com>
Subject: Re: [PATCH v11 06/13] usb: dwc3: core: Refactor PHY logic to support
 Multiport Controller



On 9/1/2023 6:43 AM, Wesley Cheng wrote:
> Hi Krishna,
> 
>>           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);
> 
> Throughout this patch, you are looping across all PHYs irrespective of 
> if we are in device mode or not.  This is the only exception where you 
> are setting only PHY index 0 (for both SS and HS PHYs).  Do you think we 
> should also only modify PHY index#0 for other PHY related sequences?
> 
Hi Wesley,

  Multiport controllers are host only capable currently. So if the 
GHWPARAMS indicate we are DRD/peripheral capable, we set 
num_usb2/3_ports to "1" unconditionally. So there would not be any 
looping necessary here.

>>           if (ret)
>> @@ -1804,9 +1887,12 @@ 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);
>> -
>>       iounmap(base);
>> +    if ((dwc->num_usb2_ports > DWC3_MAX_PORTS) ||
>> +        (dwc->num_usb3_ports > DWC3_MAX_PORTS))
>> +        return -ENOMEM;
>> +
> 
> Shouldn't this be more applicable to be included in patch#4 in this series?
> 
The read_port_info function was only initially intended to read only 
port count and later the macro was added. So the check was put here.

Regards,
Krishna,