linux-kernel - Re: [PATCH] usb: host: xhci: mvebu: make USB 3.0 PHY optional for Armada 3720

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]

Date:   Mon, 28 Dec 2020 13:11:49 +0100
From:   Marek Behun <marek.behun@....cz>
To:     Pali Rohár <pali@...nel.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Mathias Nyman <mathias.nyman@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Gregory CLEMENT <gregory.clement@...tlin.com>,
        Tomasz Maciej Nowak <tmn505@...il.com>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Peter Chen <peter.chen@....com>
Subject: Re: [PATCH] usb: host: xhci: mvebu: make USB 3.0 PHY optional for
 Armada 3720

Hi Pali and Miquel,

On Wed, 23 Dec 2020 17:24:03 +0100
Pali Rohár <pali@...nel.org> wrote:

>  int xhci_mvebu_a3700_init_quirk(struct usb_hcd *hcd)
>  {
>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
> +	struct device *dev = hcd->self.controller;
> +	struct phy *phy;
> +	int ret;
>  
>  	/* Without reset on resume, the HC won't work at all */
>  	xhci->quirks |= XHCI_RESET_ON_RESUME;
>  
> +	/* Old bindings miss the PHY handle */
> +	phy = of_phy_get(dev->of_node, "usb3-phy");
> +	if (IS_ERR(phy) && PTR_ERR(phy) == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +	else if (IS_ERR(phy))
> +		goto phy_out;
> +
> +	ret = phy_init(phy);
> +	if (ret)
> +		goto phy_put;
> +
> +	ret = phy_set_mode(phy, PHY_MODE_USB_HOST_SS);
> +	if (ret)
> +		goto phy_exit;
> +
> +	ret = phy_power_on(phy);
> +	if (ret == -EOPNOTSUPP) {
> +		/* Skip initializatin of XHCI PHY when it is unsupported by firmware */
> +		dev_warn(dev, "PHY unsupported by firmware\n");
> +		xhci->quirks |= XHCI_SKIP_PHY_INIT;
> +	}
> +	if (ret)
> +		goto phy_exit;

I am not sure if this is the correct way to check whether PHY_INIT
should be skipped.

Moreover the subsequent phy_power_off:

> +
> +	phy_power_off(phy);

won't power off the PHY, because the corresponding handler in ATF is
currently empty. 

I guess the patch needs to be in kernel if users are unwilling to upgrade
ATF firmware.

The SMC calls for Marvell's comphy are designed to be generic for
several Marvell platforms (the constants are the same and so one), but
we still have different drivers for them anyway.

Maybe it would be better to just not use the ATF implementation at all,
and implement the comphy driver for A3720 entirely in kernel...

Miquel, what do you think?

Marek