Re: [PATCH v2 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC

From: Kunihiko Hayashi
Date: Thu Aug 09 2018 - 22:30:14 EST


Hi Kishon,

On Thu, 9 Aug 2018 16:00:19 +0530
Kishon Vijay Abraham I <kishon@xxxxxx> wrote:

> Hi,
>
> On Friday 03 August 2018 03:24 PM, Kunihiko Hayashi wrote:
> > Add a driver for PHY interface built into USB3 controller
> > implemented in UniPhier SoCs.
> > This driver supports High-Speed PHY and Super-Speed PHY.
> >
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@xxxxxxxxxxxxx>
> > Signed-off-by: Motoya Tanigawa <tanigawa.motoya@xxxxxxxxxxxxx>
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@xxxxxxxxxx>
> > ---
> > drivers/phy/Kconfig | 1 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/socionext/Kconfig | 12 +
> > drivers/phy/socionext/Makefile | 6 +
> > drivers/phy/socionext/phy-uniphier-usb3hs.c | 423 ++++++++++++++++++++++++++++
> > drivers/phy/socionext/phy-uniphier-usb3ss.c | 350 +++++++++++++++++++++++
> > 6 files changed, 793 insertions(+)
> > create mode 100644 drivers/phy/socionext/Kconfig
> > create mode 100644 drivers/phy/socionext/Makefile
> > create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
> > create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c

[snip]

> > --- /dev/null
> > +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
> > @@ -0,0 +1,423 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
> > +// Copyright 2015-2018 Socionext Inc.
> > +// Author:
> > +// Kunihiko Hayashi <hayashi.kunihiko@xxxxxxxxxxxxx>
> > +// Contributors:
> > +// Motoya Tanigawa <tanigawa.motoya@xxxxxxxxxxxxx>
> > +// Masami Hiramatsu <masami.hiramatsu@xxxxxxxxxx>
> > +
>
> I'm not sure if there is a standard format for adding SPDX identifiers. But
> other PHY drivers seems to have used single line comment style only for the
> first line. (see drivers/phy/amlogic/phy-meson-gxl-usb3.c)

Okay, I understand that the format differs depending on each sub-system so far.
I'll rewrite it according to other PHY drivers.

Same as phy-uniphier-usb3ss.c.

> > +#include <linux/bitfield.h>
> > +#include <linux/bitops.h>
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/reset.h>
> > +#include <linux/slab.h>

[snip]

> > + phy = devm_phy_create(dev, dev->of_node, &uniphier_u3hsphy_ops);
> > + if (IS_ERR(phy))
> > + return PTR_ERR(phy);
> > +
> > + phy_set_drvdata(phy, priv);
> > + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > + if (IS_ERR(phy_provider))
> > + return PTR_ERR(phy_provider);
>
> return PTR_ERR_OR_ZERO()?

Indeed. I'll replace it as follows.

phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
return PTR_ERR_OR_ZERO(phy_provider);

Same as phy-uniphier-usb3ss.c.

Thank you,

---
Best Regards,
Kunihiko Hayashi