[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <587C880E.90803@ti.com>
Date: Mon, 16 Jan 2017 14:15:02 +0530
From: Kishon Vijay Abraham I <kishon@...com>
To: Vivek Gautam <vivek.gautam@...eaurora.org>, <robh+dt@...nel.org>,
<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>
CC: <mark.rutland@....com>, <sboyd@...eaurora.org>,
<bjorn.andersson@...aro.org>, <srinivas.kandagatla@...aro.org>,
<linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom
chips
Hi,
On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> PHY transceiver driver for QUSB2 phy controller that provides
> HighSpeed functionality for DWC3 controller present on
> Qualcomm chipsets.
>
> Signed-off-by: Vivek Gautam <vivek.gautam@...eaurora.org>
> Reviewed-by: Stephen Boyd <sboyd@...eaurora.org>
> ---
>
> Changes since v3:
> - Added 'Reviewed-by' from Stephen.
> - Fixed debug message for qusb2_phy_set_tune2_param().
> - Replaced devm_reset_control_get() with devm_reset_control_get_by_index()
> since we are requesting only one reset.
> - Updated devm_nvmem_cell_get() with a NULL cell id.
> - Made error labels more idiomatic.
> - Refactored qusb2_setbits() and qusb2_clrbits() a little bit to accept
> base address and register offset as two separate arguments.
>
> Changes since v2:
> - Removed selecting 'RESET_CONTROLLER' config.
> - Added error handling for clk_prepare_enable paths.
> - Removed explicitly setting ref_clk rate to 19.2 MHz. Don't need to
> do that since 'xo' is modeled as parent to this clock.
> - Removed 'ref_clk_src' handling. Driver doesn't need to request and
> handle this clock.
> - Moved nvmem_cell_get() to probe function.
> - Simplified phy pll status handling.
> - Using of_device_get_match_data() to get match data.
> - Uniformly using lowercase for hex numbers.
> - Fixed sparse warnings.
> - Using shorter variable names in structure and in functions.
> - Handling various comment style shortcomings.
>
> Changes since v1:
> - removed reference to clk_enabled/pwr_enabled.
> - moved clock and regulator enable code to phy_power_on/off() callbacks.
> - fixed return on EPROBE_DEFER in qusb2_phy_probe().
> - fixed phy create and phy register ordering.
> - removed references to non-lkml links from commit message.
> - took care of other minor nits.
> - Fixed coccinelle warnings -
> 'PTR_ERR applied after initialization to constant'
> - Addressed review comment, regarding qfprom access for tune2 param value.
> This driver is now based on qfprom patch[1] that allows byte access now.
>
> drivers/phy/Kconfig | 10 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-qcom-qusb2.c | 539 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 550 insertions(+)
> create mode 100644 drivers/phy/phy-qcom-qusb2.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f225a88..0ed53d018b23 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -430,6 +430,16 @@ config PHY_STIH407_USB
> Enable this support to enable the picoPHY device used by USB2
> and USB3 controllers on STMicroelectronics STiH407 SoC families.
>
> +config PHY_QCOM_QUSB2
> + tristate "Qualcomm QUSB2 PHY Driver"
> + depends on OF && (ARCH_QCOM || COMPILE_TEST)
> + select GENERIC_PHY
> + help
> + Enable this to support the HighSpeed QUSB2 PHY transceiver for USB
> + controllers on Qualcomm chips. This driver supports the high-speed
> + PHY which is usually paired with either the ChipIdea or Synopsys DWC3
> + USB IPs on MSM SOCs.
> +
> config PHY_QCOM_UFS
> tristate "Qualcomm UFS PHY driver"
> depends on OF && ARCH_QCOM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f436a41..dad1682b80e3 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY) += phy-spear1310-miphy.o
> obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
> obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
> obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o
> +obj-$(CONFIG_PHY_QCOM_QUSB2) += phy-qcom-qusb2.o
> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o
> diff --git a/drivers/phy/phy-qcom-qusb2.c b/drivers/phy/phy-qcom-qusb2.c
> new file mode 100644
> index 000000000000..c69118610164
> --- /dev/null
> +++ b/drivers/phy/phy-qcom-qusb2.c
> @@ -0,0 +1,539 @@
> +/*
> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#define QUSB2PHY_PLL_TEST 0x04
> +#define CLK_REF_SEL BIT(7)
> +
> +#define QUSB2PHY_PLL_TUNE 0x08
> +#define QUSB2PHY_PLL_USER_CTL1 0x0c
> +#define QUSB2PHY_PLL_USER_CTL2 0x10
> +#define QUSB2PHY_PLL_AUTOPGM_CTL1 0x1c
> +#define QUSB2PHY_PLL_PWR_CTRL 0x18
> +
> +#define QUSB2PHY_PLL_STATUS 0x38
> +#define PLL_LOCKED BIT(5)
> +
> +#define QUSB2PHY_PORT_TUNE1 0x80
> +#define QUSB2PHY_PORT_TUNE2 0x84
> +#define QUSB2PHY_PORT_TUNE3 0x88
> +#define QUSB2PHY_PORT_TUNE4 0x8c
> +#define QUSB2PHY_PORT_TUNE5 0x90
> +#define QUSB2PHY_PORT_TEST2 0x9c
> +
> +#define QUSB2PHY_PORT_POWERDOWN 0xb4
> +#define CLAMP_N_EN BIT(5)
> +#define FREEZIO_N BIT(1)
> +#define POWER_DOWN BIT(0)
> +
> +#define QUSB2PHY_REFCLK_ENABLE BIT(0)
> +
> +#define PHY_CLK_SCHEME_SEL BIT(0)
> +
> +struct qusb2_phy_init_tbl {
> + unsigned int offset;
> + unsigned int val;
> +};
> +#define QUSB2_PHY_INIT_CFG(o, v) \
> + { \
> + .offset = o, \
> + .val = v, \
> + }
> +
> +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
> +};
I wish all this data comes from device tree and one API in phy-core can do all
these settings. Your other driver qcom-qmp also seems to have a bunch of
similar settings.
The problem is every vnedor driver adds a bunch of code to perform the same
thing again and again when all of these settings can be done by a single phy API.
Thanks
Kishon
Powered by blists - more mailing lists