From: peter.chen@freescale.com (Peter Chen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/4] usb: phy: add phy-hi6220
Date: Mon, 9 Feb 2015 09:57:35 +0800 [thread overview]
Message-ID: <20150209015734.GA3045@shlinux2> (raw)
In-Reply-To: <CABQgh9GfmoeLo-HnhM=sk-XBw1KoLMsa=RZZWkHu-hGPpMK+=A@mail.gmail.com>
On Fri, Feb 06, 2015 at 07:47:12PM +0800, Zhangfei Gao wrote:
> On 6 February 2015 at 16:41, Peter Chen <peter.chen@freescale.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
To: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>,
"john.youn-HKixBCOQz3hWk0Htik3J/w@public.gmane.org"
<john.youn-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>,
Mian Yousaf Kaukab
<yousaf.kaukab-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
"dan . zhao" <dan.zhao-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>,
"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
"linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH 4/4] usb: phy: add phy-hi6220
Date: Mon, 9 Feb 2015 09:57:35 +0800 [thread overview]
Message-ID: <20150209015734.GA3045@shlinux2> (raw)
In-Reply-To: <CABQgh9GfmoeLo-HnhM=sk-XBw1KoLMsa=RZZWkHu-hGPpMK+=A@mail.gmail.com>
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
next prev parent reply other threads:[~2015-02-09 1:57 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-05 14:46 [PATCH 0/4] add usb support for hi6220 Zhangfei Gao
2015-02-05 14:46 ` Zhangfei Gao
2015-02-05 14:46 ` [PATCH 1/4] Documentation: dt-bindings: add dt binding info for hi6220 dwc2 Zhangfei Gao
2015-02-05 14:46 ` Zhangfei Gao
2015-02-05 14:46 ` [PATCH 2/4] Documentation: dt-bindings: add dt binding info for hi6220 Zhangfei Gao
2015-02-05 14:46 ` Zhangfei Gao
2015-02-05 18:24 ` Sergei Shtylyov
2015-02-05 18:24 ` Sergei Shtylyov
2015-02-06 5:49 ` Zhangfei Gao
2015-02-06 5:49 ` Zhangfei Gao
2015-02-05 14:46 ` [PATCH 3/4] usb: dwc2: platform: add hi6220 support Zhangfei Gao
2015-02-05 14:46 ` Zhangfei Gao
2015-02-05 14:47 ` [PATCH 4/4] usb: phy: add phy-hi6220 Zhangfei Gao
2015-02-05 14:47 ` Zhangfei Gao
2015-02-06 8:41 ` Peter Chen
2015-02-06 8:41 ` Peter Chen
2015-02-06 11:47 ` Zhangfei Gao
2015-02-06 11:47 ` Zhangfei Gao
2015-02-09 1:57 ` Peter Chen [this message]
2015-02-09 1:57 ` Peter Chen
2015-02-09 3:31 ` Zhangfei Gao
2015-02-09 3:31 ` Zhangfei Gao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150209015734.GA3045@shlinux2 \
--to=peter.chen@freescale.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.