On Fri, Feb 06, 2015 at 07:47:12PM +0800, Zhangfei Gao wrote:
> On 6 February 2015 at 16:41, Peter Chen <peter.chen@xxxxxxxxxxxxx> 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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
