Re: [PATCH v13 2/3] PHY: add APM X-Gene SoC 15Gbps Multi-purpose PHY driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi,

On Thursday 06 March 2014 04:13 AM, Loc Ho wrote:
This patch adds support for the APM X-Gene SoC 15Gbps Multi-purpose PHY.
This is the physical layer interface for the corresponding host
controller. Currently, only external clock and Gen3 SATA mode
are supported.

Signed-off-by: Loc Ho <lho@xxxxxxx>
Signed-off-by: Tuan Phan <tphan@xxxxxxx>
Signed-off-by: Suman Tripathi <stripathi@xxxxxxx>
---
  drivers/phy/Kconfig     |    7 +
  drivers/phy/Makefile    |    2 +
  drivers/phy/phy-xgene.c | 1756 +++++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 1765 insertions(+), 0 deletions(-)
  create mode 100644 drivers/phy/phy-xgene.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index afa2354..221a1b7 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -64,4 +64,11 @@ config BCM_KONA_USB2_PHY
  	help
  	  Enable this to support the Broadcom Kona USB 2.0 PHY.

+config PHY_XGENE
+	tristate "APM X-Gene 15Gbps PHY support"
+	depends on HAS_IOMEM && OF && (ARM64 || COMPILE_TEST)
+	select GENERIC_PHY
+	help
+	  This option enables support for APM X-Gene SoC multi-purpose PHY.
+
  endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index b57c253..dee70f4 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -9,3 +9,5 @@ obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
  obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
  obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
  obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
+obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
+
.
.
<snip>
.
.

+	int i;
+
+	rc = xgene_phy_hw_initialize(ctx, CLK_EXT_DIFF, SSC_DISABLE);
+	if (rc) {
+		dev_err(ctx->dev, "PHY initialize failed %d\n", rc);
+		return rc;
+	}
+
+	/* Setup clock properly after PHY configuration */
+	if (!IS_ERR(ctx->clk)) {
+		/* HW requires an toggle of the clock */
+		clk_prepare_enable(ctx->clk);
+		clk_disable_unprepare(ctx->clk);
+		clk_prepare_enable(ctx->clk);
+	}
+
+	/* Compute average value */
+	for (i = 0; i < MAX_LANE; i++)
+		xgene_phy_gen_avg_val(ctx, i);
+
+	dev_dbg(ctx->dev, "PHY initialized\n");
+	return 0;
+}
+
+static const struct phy_ops xgene_phy_ops = {
+	.init		= xgene_phy_hw_init,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *xgene_phy_xlate(struct device *dev,
+				   struct of_phandle_args *args)
+{
+	struct xgene_phy_ctx *ctx = dev_get_drvdata(dev);
+
+	if (args->args_count > 0) {
+		if (args->args[0] >= MODE_MAX)
+			return NULL;

Are you sure you want to return NULL instead of error? NULL is considered a valid value for optional PHYs.
+		ctx->mode = args->args[0];
+	}

I think it doesn't make sense to return the phy without the 'mode'. So we should report error here too.

+	return ctx->phy;
+}
+
+static void xgene_phy_get_param(struct platform_device *pdev,
+				const char *name, u32 *buffer,
+				int count, u32 *default_val,
+				u32 conv_factor)
+{
+	int i;
+
+	if (!of_property_read_u32_array(pdev->dev.of_node, name, buffer,
+					count)) {
+		for (i = 0; i < count; i++)
+			buffer[i] /= conv_factor;
+		return;
+	}
+	/* Does not exist, load default */
+	for (i = 0; i < count; i++)
+		buffer[i] = default_val[i % 3];
+}
+
+static int xgene_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct xgene_phy_ctx *ctx;
+	struct resource *res;
+	int rc = 0;
+	u32 default_spd[] = DEFAULT_SATA_SPD_SEL;
+	u32 default_txboost_gain[] = DEFAULT_SATA_TXBOOST_GAIN;
+	u32 default_txeye_direction[] = DEFAULT_SATA_TXEYEDIRECTION;
+	u32 default_txeye_tuning[] = DEFAULT_SATA_TXEYETUNING;
+	u32 default_txamp[] = DEFAULT_SATA_TXAMP;
+	u32 default_txcn1[] = DEFAULT_SATA_TXCN1;
+	u32 default_txcn2[] = DEFAULT_SATA_TXCN2;
+	u32 default_txcp1[] = DEFAULT_SATA_TXCP1;
+	int i;
+
+	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx) {
+		dev_err(&pdev->dev, "can't allocate PHY context\n");
remove dev_err here.. kzalloc will do that for you..
+		return -ENOMEM;
+	}
+	ctx->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "no PHY resource address\n");

same here.. you don't have to even check for return value of res. ioremap_resource will take care of that.
+		rc = -EINVAL;
+		goto error;
+	}
+	ctx->sds_base = devm_ioremap_resource(&pdev->dev, res);
+	if (!ctx->sds_base) {
+		dev_err(&pdev->dev, "can't map PHY resource\n");

same here..

Thanks
Kishon
--
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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux