Re: [PATCH 4/4] usb: phy: add phy-hi6220

[Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>]

On Fri, Feb 06, 2015 at 07:47:12PM +0800, Zhangfei Gao wrote:
> On 6 February 2015 at 16:41, Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> > On Thu, Feb 05, 2015 at 10:47:00PM +0800, Zhangfei Gao wrote:
> 
> >> @@ -18,6 +18,7 @@ obj-$(CONFIG_SAMSUNG_USBPHY)                += phy-samsung-usb.o
> >>  obj-$(CONFIG_TWL6030_USB)            += phy-twl6030-usb.o
> >>  obj-$(CONFIG_USB_EHCI_TEGRA)         += phy-tegra-usb.o
> >>  obj-$(CONFIG_USB_GPIO_VBUS)          += phy-gpio-vbus-usb.o
> >> +obj-$(CONFIG_USB_HI6220_PHY)         += phy-hi6220.o
> >
> > To align the naming method, phy-hi6220-usb is better.
> Sure,
> 
> 
> >> +enum usb_mode {
> >> +     USB_EMPTY,
> >> +     GADGET_DEVICE,
> >> +     OTG_HOST,
> >> +};
> >
> > This usb_mode is a little strange, what state you would like to
> > use?
> 
> it is internal state machine, to distinguish otg gadget mode and host mode.
> There are two gpio, we use gpio_vbus interrupt as well as gpio_id
> status to distinguish gadget or host.
> 
> >> +static irqreturn_t hiusb_gpio_intr(int irq, void *data)
> >> +{
> >> +     struct hi6220_priv *priv = (struct hi6220_priv *)data;
> >> +
> >> +     /* add debounce time */
> >> +     schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
> >> +     return IRQ_HANDLED;
> >> +}
> >> +
> >> +static int mv_otg_set_peripheral(struct usb_otg *otg,
> >
> > mv? You may want to use hi
> Yes, my bad.
> 
> 
> >> +static void hi6220_phy_setup(struct hi6220_priv *priv)
> >> +{
> >> +     u32 val, mask;
> >> +     int ret;
> >> +
> >> +     if (priv->reg == NULL)
> >> +             return;
> >> +
> >> +     val = PERIPH_RSTDIS0_USBOTG_BUS | PERIPH_RSTDIS0_POR_PICOPHY |
> >> +           PERIPH_RSTDIS0_USBOTG | PERIPH_RSTDIS0_USBOTG_32K;
> >> +     mask = val;
> >> +     ret = regmap_update_bits(priv->reg, SC_PERIPH_RSTDIS0, mask, val);
> >> +     if (ret)
> >> +             return;
> >> +
> >> +     ret = regmap_read(priv->reg, SC_PERIPH_CTRL5, &val);
> >> +     val = PERIPH_CTRL5_USBOTG_RES_SEL | PERIPH_CTRL5_PICOPHY_ACAENB;
> >> +     mask = val | PERIPH_CTRL5_PICOPHY_BC_MODE;
> >> +     ret = regmap_update_bits(priv->reg, SC_PERIPH_CTRL5, mask, val);
> >> +     if (ret)
> >> +             return;
> >> +
> >> +     val =  PERIPH_CTRL4_PICO_VBUSVLDEXT | PERIPH_CTRL4_PICO_VBUSVLDEXTSEL |
> >> +            PERIPH_CTRL4_OTG_PHY_SEL;
> >> +     mask = val | PERIPH_CTRL4_PICO_SIDDQ | PERIPH_CTRL4_PICO_OGDISABLE;
> >> +     ret = regmap_update_bits(priv->reg, SC_PERIPH_CTRL4, mask, val);
> >> +     if (ret)
> >> +             return;
> >> +
> >> +     ret = regmap_write(priv->reg, SC_PERIPH_CTRL8, EYE_PATTERN_PARA);
> >> +     if (ret)
> >> +             return;
> >> +}
> >> +
> >> +static int hi6220_phy_probe(struct platform_device *pdev)
> >> +{
> >> +     struct hi6220_priv *priv;
> >> +     struct usb_otg *otg;
> >> +     struct device_node *np = pdev->dev.of_node;
> >> +     int ret, irq;
> >> +
> >> +     priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> >> +     if (!priv)
> >> +             return -ENOMEM;
> >> +
> >> +     otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
> >> +     if (!otg)
> >> +             return -ENOMEM;
> >> +
> >> +     priv->phy.dev = &pdev->dev;
> >> +     priv->phy.otg = otg;
> >> +     priv->phy.label = "hi6220";
> >> +     platform_set_drvdata(pdev, priv);
> >> +     otg->set_peripheral = mv_otg_set_peripheral;
> >> +
> >> +     priv->gpio_vbus_det = of_get_named_gpio(np, "hisilicon,gpio_vbus_det", 0);
> >> +     if (priv->gpio_vbus_det == -EPROBE_DEFER)
> >> +             return -EPROBE_DEFER;
> >> +     if (!gpio_is_valid(priv->gpio_vbus_det)) {
> >> +             dev_err(&pdev->dev, "invalid gpio %d\n", priv->gpio_vbus_det);
> >> +             return -ENODEV;
> >> +     }
> >> +
> >> +     priv->gpio_id_det = of_get_named_gpio(np, "hisilicon,gpio_id_det", 0);
> >> +     if (priv->gpio_id_det == -EPROBE_DEFER)
> >> +             return -EPROBE_DEFER;
> >> +     if (!gpio_is_valid(priv->gpio_id_det)) {
> >> +             dev_err(&pdev->dev, "invalid gpio %d\n", priv->gpio_id_det);
> >> +             return -ENODEV;
> >> +     }
> >> +
> >> +     priv->reg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> >> +                                     "hisilicon,peripheral-syscon");
> >> +     if (IS_ERR(priv->reg))
> >> +             priv->reg = NULL;
> >
> > You may differentiate -ENODEV and other errors, for other errors, you
> > can show an error, and return directly.
> 
> Here I want to set this property as optional, in case other platform
> do not need this property.
> So phy_setup also add protection if (priv->reg == NULL) return;
> 

If syscon_regmap_lookup_by_phandle returns -EPROBE_DEFER, you may want
to try later.

Peter
> >
> >> +
> >> +     INIT_DELAYED_WORK(&priv->work, hi6220_detect_work);
> >> +
> >> +     ret = devm_gpio_request_one(&pdev->dev, priv->gpio_vbus_det,
> >> +                                 GPIOF_IN, "gpio_vbus_det");
> >> +     if (ret < 0) {
> >> +             dev_err(&pdev->dev, "gpio request failed for gpio_vbus_det\n");
> >> +             return ret;
> >> +     }
> >> +
> >> +     ret = devm_gpio_request_one(&pdev->dev, priv->gpio_id_det,
> >> +                                 GPIOF_IN, "gpio_id_det");
> >> +     if (ret < 0) {
> >> +             dev_err(&pdev->dev, "gpio request failed for gpio_id_det\n");
> >> +             return ret;
> >> +     }
> >> +
> >> +     priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
> >> +     if (!IS_ERR(priv->vcc)) {
> >> +             ret = regulator_enable(priv->vcc);
> >> +             if (ret) {
> >> +                     dev_err(&pdev->dev, "Failed to enable regulator\n");
> >> +                     return -ENODEV;
> >> +             }
> >> +     }
> >> +
> >> +     priv->clk = devm_clk_get(&pdev->dev, NULL);
> >> +     if (IS_ERR(priv->clk)) {
> >> +             regulator_disable(priv->vcc);
> >> +             return PTR_ERR(priv->clk);
> >> +     }
> >> +     clk_prepare_enable(priv->clk);
> >> +
> >> +     irq = gpio_to_irq(priv->gpio_vbus_det);
> >> +     ret = devm_request_irq(&pdev->dev, gpio_to_irq(priv->gpio_vbus_det),
> >> +                            hiusb_gpio_intr, IRQF_NO_SUSPEND |
> >> +                            IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> >> +                            "vbus_gpio_intr", priv);
> >> +     if (ret) {
> >> +             dev_err(&pdev->dev, "request gpio irq failed.\n");
> >> +             goto err_irq;
> >> +     }
> >

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html