Hi, On Tue, Oct 15, 2013 at 12:23:45PM +0530, George Cherian wrote: > This patch adds a compatible for AM437x "ti,am43xx-usb2" to > reuse the same phy-omap-usb2 driver.
it does more than just adding a new compatible flag.
> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> index 3e5f08c..7c0bc6c 100644
> --- a/drivers/phy/phy-omap-usb2.c
> +++ b/drivers/phy/phy-omap-usb2.c
> @@ -144,6 +144,35 @@ static struct phy_ops ops = {
> .owner = THIS_MODULE,
> };
>
> +#ifdef CONFIG_OF
> +static const struct usb_phy_data omap_usb2_data = {
> + .label = "omap_usb2",
> + .set_host = omap_usb_set_host,
> + .set_peripheral = omap_usb_set_peripheral,
> + .set_vbus = omap_usb_set_vbus,
> + .start_srp = omap_usb_start_srp,
> +};
> +
> +static const struct usb_phy_data am437x_usb2_data = {
> + .label = "am437x_usb2",
> + .set_host = omap_usb_set_host,
> + .set_peripheral = omap_usb_set_peripheral,
> +};
> +
> +static const struct of_device_id omap_usb2_id_table[] = {
> + {
> + .compatible = "ti,omap-usb2",
> + .data = &omap_usb2_data,
> + },
> + {
> + .compatible = "ti,am437x-usb2",
> + .data = &am437x_usb2_data,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, omap_usb2_id_table);
> +#endif
> +
> static int omap_usb2_probe(struct platform_device *pdev)
> {
> struct omap_usb *phy;
> @@ -153,9 +182,14 @@ static int omap_usb2_probe(struct platform_device *pdev)
> struct device_node *node = pdev->dev.of_node;
> struct device_node *control_node;
> struct platform_device *control_pdev;
> + const struct of_device_id *of_id;
> + struct usb_phy_data *phy_data;
> +
> + of_id = of_match_device(of_match_ptr(omap_usb2_id_table), &pdev->dev);
>
> - if (!node)
> + if (!of_id)
> return -EINVAL;
> + phy_data = (struct usb_phy_data *)of_id->data;
>
> phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> if (!phy) {
> @@ -172,7 +206,7 @@ static int omap_usb2_probe(struct platform_device *pdev)
> phy->dev = &pdev->dev;
>
> phy->phy.dev = phy->dev;
> - phy->phy.label = "omap-usb2";
> + phy->phy.label = phy_data->label;
label can be set based on compatible flag:
if (of_device_is_compatible(node, "ti,omap-usb2"))
phy->phy.label = "omap-usb2";
else /* if (of_device_is_compatible(node, "ti,am437x-usb2")) */ /* default to
newest */
phy->phy.label = "am437x-usb2"; /* keep name
consistency */
> phy->phy.set_suspend = omap_usb2_suspend;
> phy->phy.otg = otg;
> phy->phy.type = USB_PHY_TYPE_USB2;
> @@ -199,10 +233,10 @@ static int omap_usb2_probe(struct platform_device *pdev)
> phy->is_suspended = 1;
> omap_control_usb_phy_power(phy->control_dev, 0);
>
> - otg->set_host = omap_usb_set_host;
> - otg->set_peripheral = omap_usb_set_peripheral;
> - otg->set_vbus = omap_usb_set_vbus;
> - otg->start_srp = omap_usb_start_srp;
> + otg->set_host = phy_data->set_host;
> + otg->set_peripheral = phy_data->set_peripheral;
> + otg->set_vbus = phy_data->set_vbus;
> + otg->start_srp = phy_data->start_srp;
this doesn't look right. I would rather have feature flags so that you
could:
if (test_bit(&phy_data->flags, OMAP_USB2_HAS_SRP))
otg->start_srp = omap_usb_start_srp;
and so on. Note that you might need to add __maybe_unused annotations to
those functions otherwise you'll get unused warnings.
--
balbi
signature.asc
Description: Digital signature
