linux-kernel - Re: [PATCH 9/9] usb: phy: tegra: Use DT helpers for dr

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: <51D1FC8B.1030505@wwwdotorg.org>
Date:	Mon, 01 Jul 2013 16:02:51 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Tuomas Tynkkynen <ttynkkynen@...dia.com>
CC:	balbi@...com, linux-tegra@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	gregkh@...uxfoundation.org
Subject: Re: [PATCH 9/9] usb: phy: tegra: Use DT helpers for dr_mode

On 06/28/2013 03:37 PM, Tuomas Tynkkynen wrote:
> Use the new of_usb_get_dr_mode helper function for parsing dr_mode
> from the device tree. Also replace the usage of the custom
> tegra_usb_phy_mode enum with the standard enum.

> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> +	tegra_phy->mode = of_usb_get_dr_mode(np);
> +	if (tegra_phy->mode == USB_DR_MODE_UNKNOWN) {
> +		dev_err(&pdev->dev, "dr_mode is invalid\n");
> +		return -EINVAL;
> +	}

Unfortunately, of_usb_get_dr_mode() returns USB_DR_MODE_UNKNOWN if the
property is missing, rather than defaulting to host mode as the original
code here did. I would suggest solving this by:

> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
> index 675384d..6391de5 100644
> --- a/drivers/usb/usb-common.c
> +++ b/drivers/usb/usb-common.c
> @@ -103,7 +103,7 @@ enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
>  
>         err = of_property_read_string(np, "dr_mode", &dr_mode);
>         if (err < 0)
> -               return USB_DR_MODE_UNKNOWN;
> +               return USB_DR_MODE_HOST;
>  
>         for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++)
>                 if (!strcmp(dr_mode, usb_dr_modes[i]))

This can't be done by the caller, since of_usb_get_dr_mode() returns
UNKNOWN in two cases, which the caller can't distinguish without
manually checking whether the property exists first:

a) Property is not present (which should default to HOST mode at least
for the Tegra binding, as in the patch I show above).

b) Property is present, but contains an invalid value, which probably
should cause the driver to error-out.

This is only a problem for dr_mode in the Tegra binding: dr_mode is an
optional property, whereas the phy_type property as parsed by patch 8/9
is mandatory, so this issue doesn't come up.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/