Sebastian,
On Fri, Jun 06, 2014 at 12:54:06PM +0200, Sebastian Hesselbarth wrote:
> On 06/05/2014 05:48 PM, Antoine Ténart wrote:
> >+
> >+#include <linux/gpio.h>
> >+#include <linux/io.h>
> >+#include <linux/module.h>
> >+#include <linux/of_gpio.h>
> >+#include <linux/usb/phy.h>
> >+#include <linux/platform_device.h>
> >+#include <linux/reset.h>
>
> nit: keep above alphabetically sorted.
Oops. Sure.
> >+
> >+/* USB_PHY_PLL */
> >+#define CLK_REF_DIV(x) ((x) << 4)
> >+#define FEEDBACK_CLK_DIV(x) ((x) << 8)
> >+
> >+/* USB_PHY_PLL_CONTROL */
> >+#define CLK_STABLE (0x1 << 0)
> >+#define PLL_CTRL_PIN (0x1 << 1)
> >+#define PLL_CTRL_REG (0x1 << 2)
> >+#define PLL_ON (0x1 << 3)
> >+#define PHASE_OFF_TOL_125 (0x0 << 5)
> >+#define PHASE_OFF_TOL_250 (0x1 << 5)
> >+#define KVC0_CALIB (0x0 << 9)
> >+#define KVC0_REG_CTRL (0x1 << 9)
> >+#define KVC0_HIGH (0x0 << 10)
> >+#define KVC0_LOW (0x3 << 10)
> >+#define CLK_BLK_EN (0x1 << 13)
>
> BIT() for the single bit flags above and below.
I'll update with BIT().
> >+
> >+struct berlin_phy_priv {
> >+ void __iomem *base;
> >+ struct usb_phy phy;
> >+ struct reset_control *rst_ctrl;
> >+ int pwr_gpio;
>
> Is the GPIO used for USB power? If so, we should not rely on
> GPIO at all but use regulator API. Thinking of Chromecast which
> is externally powered over USB, there will be no regulator nor
> GPIO at all.
[…]
>
> As mentioned above, this should be using regulator API. And also, if
> there is no dummy regulator allowed, it should be optional.
I'll make this optional.
>
> >+static int berlin_phy_probe(struct platform_device *pdev)
> >+{
> >+ struct device_node *np = pdev->dev.of_node;
> >+ struct berlin_phy_priv *priv;
> >+ struct resource *res;
> >+ int ret, gpio;
> >+
> >+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> >+ if (!priv)
> >+ return -ENOMEM;
> >+
> >+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >+ priv->base = devm_ioremap_resource(&pdev->dev, res);
> >+ if (IS_ERR(priv->base))
> >+ return PTR_ERR(priv->base);
> >+
> >+ priv->rst_ctrl = devm_reset_control_get(&pdev->dev, NULL);
> >+ if (IS_ERR(priv->rst_ctrl)) {
> >+ ret = PTR_ERR(priv->rst_ctrl);
> >+ dev_err(&pdev->dev, "cannot get reset controller: %d\n", ret);
>
> Hmm, considering a non arch_init call registered reset driver, it does
> also spit out an error for -EPROBE_DEFER, does it?
Yes, it does.
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
