Hi Kishon,

On Thu, 9 Aug 2018 16:00:19 +0530
Kishon Vijay Abraham I [off-list ref] wrote:

Hi,

On Friday 03 August 2018 03:24 PM, Kunihiko Hayashi wrote:

quoted

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@socionext.com>
Signed-off-by: Motoya Tanigawa <redacted>
Signed-off-by: Masami Hiramatsu <redacted>
---
 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]

quoted

--- /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@socionext.com>
+// Contributors:
+//      Motoya Tanigawa <tanigawa.motoya@socionext.com>
+//      Masami Hiramatsu <masami.hiramatsu@linaro.org>
+

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.

quoted

+#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]

quoted

+	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