linux-kernel - Re: [PATCH v3 3/3] usb: dwc: host: add xhci_plat_priv quirk XHCI_SKIP_PHY

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 for Android: free password hash cracker in your pocket

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

Message-ID: <4c2a28ad-b866-1b65-e73a-4eda0596cea2@linux.intel.com>
Date:   Fri, 25 Mar 2022 00:36:22 +0200
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Sandeep Maheswaram <quic_c_sanm@...cinc.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Felipe Balbi <balbi@...nel.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Doug Anderson <dianders@...omium.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Mathias Nyman <mathias.nyman@...el.com>,
        Peter Chen <peter.chen@...nel.org>,
        Pawel Laszczak <pawell@...ence.com>,
        Roger Quadros <rogerq@...nel.org>,
        Aswath Govindraju <a-govindraju@...com>,
        linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, quic_pkondeti@...cinc.com,
        quic_ppratap@...cinc.com
Subject: Re: [PATCH v3 3/3] usb: dwc: host: add xhci_plat_priv quirk
 XHCI_SKIP_PHY_INIT

On 24.3.2022 14.27, Heikki Krogerus wrote:
> On Thu, Mar 24, 2022 at 12:07:11PM +0530, Sandeep Maheswaram wrote:
>> Currently the phy init is done from dwc3 and also xhci which makes the
>> runtime_usage value 2 for the phy which causes issue during runtime
>> suspend. When we run the below command the runtime_status still shows
>> active.
>> echo auto > /sys/bus/platform/devices/88e3000.phy/power/control
>>
>> dwc3 manages PHY by own DRD driver, so skip the management by
>> HCD core by setting this quirk.
>>
>> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@...cinc.com>
>> ---
>>  drivers/usb/dwc3/host.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
>> index eda8719..d4fcf06 100644
>> --- a/drivers/usb/dwc3/host.c
>> +++ b/drivers/usb/dwc3/host.c
>> @@ -13,6 +13,12 @@
>>  #include <linux/platform_device.h>
>>  
>>  #include "core.h"
>> +#include <linux/usb/xhci-plat.h>
>> +#include <linux/usb/xhci-quirks.h>
>> +
>> +static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
>> +	.quirks = XHCI_SKIP_PHY_INIT,
>> +};
>>  
>>  static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc,
>>  					int irq, char *name)
>> @@ -122,6 +128,13 @@ int dwc3_host_init(struct dwc3 *dwc)
>>  		}
>>  	}
>>  
>> +	ret = platform_device_add_data(xhci, &xhci_plat_dwc3_xhci,
>> +			sizeof(xhci_plat_dwc3_xhci));
>> +	if (ret) {
>> +		dev_err(dwc->dev, "failed to add data to xHCI\n");
>> +		goto err;
>> +	}
>> +
>>  	ret = platform_device_add(xhci);
>>  	if (ret) {
>>  		dev_err(dwc->dev, "failed to register xHCI device\n");
> 
> I think you should just use device property:
> 

This was suggested in an earlier series, but was rejected as it also added
the property as a device tree parameter.

I think adding more device properties can be messy in the long run, especially if we
need to add them for many of the existing xhci quirks.
We also end up with a mix where some device properties are listed as device tree
parameters, and some not.

Defining xhci quirks and platform data structure in headers shared with dwc3 and cdns3
allow those drivers to easily set any existing xhci quirk, or other possible optional
callbacks.

cdns3 driver is already doing this, but it includes the full xhci.h header.
This series cleans up that a bit so cdns3 will only include xhci quirk bits and
platform data structure.

On the downside we add a couple xhci related header files to include/linux/usb/
Let me know if you see any other issues I missed with this approach.

Thanks
Mathias