linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shawn Lin <shawn.lin@rock-chips.com>
To: Kishon Vijay Abraham I <kishon@ti.com>
Cc: shawn.lin@rock-chips.com, linux-kernel@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Heiko Stuebner <heiko@sntech.de>,
	Doug Anderson <dianders@chromium.org>,
	Wenrui Li <wenrui.li@rock-chips.com>,
	Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v3 2/2] phy: add a driver for the Rockchip SoC internal PCIe PHY
Date: Mon, 20 Jun 2016 08:58:23 +0800	[thread overview]
Message-ID: <c88eb16f-e976-ad52-02fb-5ec7b6e261b9@rock-chips.com> (raw)
In-Reply-To: <5763F665.6070403@ti.com>

Hi Kishon,

On 2016/6/17 21:08, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Thursday 16 June 2016 06:52 AM, Shawn Lin wrote:
>> This patch to add a generic PHY driver for rockchip PCIe PHY.
>> Access the PHY via registers provided by GRF (general register
>> files) module.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>>  drivers/phy/Kconfig             |   7 +
>>  drivers/phy/Makefile            |   1 +
>>  drivers/phy/phy-rockchip-pcie.c | 378 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 386 insertions(+)
>>  create mode 100644 drivers/phy/phy-rockchip-pcie.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index b869b98..d4fc293 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -361,6 +361,13 @@ config PHY_ROCKCHIP_DP
>>  	help
>>  	  Enable this to support the Rockchip Display Port PHY.
>>
>> +config PHY_ROCKCHIP_PCIE
>> +	tristate "Rockchip PCIe PHY Driver"
>> +	depends on ARCH_ROCKCHIP && OF
>> +	select GENERIC_PHY
>> +	help
>> +	  Enable this to support the Rockchip PCIe PHY.
>> +
>>  config PHY_ST_SPEAR1310_MIPHY
>>  	tristate "ST SPEAR1310-MIPHY driver"
>>  	select GENERIC_PHY
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 9c3e73c..725e55d 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -39,6 +39,7 @@ obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
>>  obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>>  obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>>  obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
>> +obj-$(CONFIG_PHY_ROCKCHIP_PCIE) += phy-rockchip-pcie.o
>>  obj-$(CONFIG_PHY_ROCKCHIP_DP)		+= phy-rockchip-dp.o
>>  obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
>>  obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)	+= phy-spear1310-miphy.o
>> diff --git a/drivers/phy/phy-rockchip-pcie.c b/drivers/phy/phy-rockchip-pcie.c
>> new file mode 100644
>> index 0000000..bc6cd17
>> --- /dev/null
>> +++ b/drivers/phy/phy-rockchip-pcie.c
>> @@ -0,0 +1,378 @@
>> +/*
>> + * Rockchip PCIe PHY driver
>> + *
>> + * Copyright (C) 2016 Shawn Lin <shawn.lin@rock-chips.com>
>> + * Copyright (C) 2016 ROCKCHIP, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License.
>> + *
>> + * 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/io.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +
>> +/*
>> + * The higher 16-bit of this register is used for write protection
>> + * only if BIT(x + 16) set to 1 the BIT(x) can be written.
>> + */
>> +#define HIWORD_UPDATE(val, mask, shift) \
>> +		((val) << (shift) | (mask) << ((shift) + 16))
>> +
>> +#define PHY_MAX_LANE_NUM      4
>> +#define PHY_CFG_DATA_SHIFT    7
>> +#define PHY_CFG_ADDR_SHIFT    1
>> +#define PHY_CFG_DATA_MASK     0xf
>> +#define PHY_CFG_ADDR_MASK     0x3f
>> +#define PHY_CFG_RD_MASK       0x3ff
>> +#define PHY_CFG_WR_ENABLE     1
>> +#define PHY_CFG_WR_DISABLE    1
>> +#define PHY_CFG_WR_SHIFT      0
>> +#define PHY_CFG_WR_MASK       1
>> +#define PHY_CFG_PLL_LOCK      0x10
>> +#define PHY_CFG_CLK_TEST      0x10
>> +#define PHY_CFG_CLK_SCC       0x12
>> +#define PHY_CFG_SEPE_RATE     BIT(3)
>> +#define PHY_CFG_PLL_100M      BIT(3)
>> +#define PHY_PLL_LOCKED        BIT(9)
>> +#define PHY_PLL_OUTPUT        BIT(10)
>> +#define PHY_LANE_A_STATUS     0x30
>> +#define PHY_LANE_B_STATUS     0x31
>> +#define PHY_LANE_C_STATUS     0x32
>> +#define PHY_LANE_D_STATUS     0x33
>> +#define PHY_LANE_RX_DET_SHIFT 11
>> +#define PHY_LANE_RX_DET_TH    0x1
>> +#define PHY_LANE_IDLE_OFF     0x1
>> +#define PHY_LANE_IDLE_MASK    0x1
>> +#define PHY_LANE_IDLE_A_SHIFT 3
>> +#define PHY_LANE_IDLE_B_SHIFT 4
>> +#define PHY_LANE_IDLE_C_SHIFT 5
>> +#define PHY_LANE_IDLE_D_SHIFT 6
>> +
>> +struct rockchip_pcie_data {
>> +	unsigned int pcie_conf;
>> +	unsigned int pcie_status;
>> +	unsigned int pcie_laneoff;
>> +};
>> +
>> +struct rockchip_pcie_phy {
>> +	struct rockchip_pcie_data *phy_data;
>> +	struct regmap *reg_base;
>> +	struct reset_control *phy_rst;
>> +	struct clk *clk_pciephy_ref;
>> +};
>> +
>> +static inline void phy_wr_cfg(struct rockchip_pcie_phy *rk_phy,
>> +			      u32 addr, u32 data)
>> +{
>> +	regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
>> +		     HIWORD_UPDATE(data,
>> +				   PHY_CFG_DATA_MASK,
>> +				   PHY_CFG_DATA_SHIFT) |
>> +		     HIWORD_UPDATE(addr,
>> +				   PHY_CFG_ADDR_MASK,
>> +				   PHY_CFG_ADDR_SHIFT));
>> +	udelay(1);
>> +	regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
>> +		     HIWORD_UPDATE(PHY_CFG_WR_ENABLE,
>> +				   PHY_CFG_WR_MASK,
>> +				   PHY_CFG_WR_SHIFT));
>> +	udelay(1);
>> +	regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
>> +		     HIWORD_UPDATE(PHY_CFG_WR_DISABLE,
>> +				   PHY_CFG_WR_MASK,
>> +				   PHY_CFG_WR_SHIFT));
>> +}
>> +
>> +static inline u32 phy_rd_cfg(struct rockchip_pcie_phy *rk_phy,
>> +			     u32 addr)
>> +{
>> +	u32 val;
>> +
>> +	regmap_write(rk_phy->reg_base, rk_phy->phy_data->pcie_conf,
>> +		     HIWORD_UPDATE(addr,
>> +				   PHY_CFG_RD_MASK,
>> +				   PHY_CFG_ADDR_SHIFT));
>> +	regmap_read(rk_phy->reg_base,
>> +		    rk_phy->phy_data->pcie_status,
>> +		    &val);
>> +	return val;
>> +}
>> +
>> +void rockchip_pcie_phy_laneoff(struct phy *phy)
>> +{
>> +	u32 status;
>> +	struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
>> +	int i;
>> +
>> +	for (i = 0; i < PHY_MAX_LANE_NUM; i++) {
>> +		status = phy_rd_cfg(rk_phy, PHY_LANE_A_STATUS + i);
>> +		if (!((status >> PHY_LANE_RX_DET_SHIFT) &
>> +		       PHY_LANE_RX_DET_TH))
>> +			pr_debug("lane %d is used\n", i);
>> +		else
>> +			regmap_write(rk_phy->reg_base,
>> +				     rk_phy->phy_data->pcie_laneoff,
>> +				     HIWORD_UPDATE(PHY_LANE_IDLE_OFF,
>> +						   PHY_LANE_IDLE_MASK,
>> +						   PHY_LANE_IDLE_A_SHIFT + i));
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(rockchip_pcie_phy_laneoff);
>
> Er.. don't use export symbols from phy driver. I think it would be nice if you
> can model the driver in such a way that the PCIe driver can control individual
> phy's.
>

Yes, I was trying to look for a way not to export symbols from
phy... But I failed to find it as there at least need three
interaction between controller and phy which made me believe we
at least need to export one symbol without adding new API for phy.

And I found lots of phy drivers export symbols the same way as mine.
Does it sound okay for me to do the same thing here? :)


> Thanks
> Kishon
>
>
>


-- 
Best Regards
Shawn Lin

  reply	other threads:[~2016-06-20  0:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-16  1:22 Shawn Lin
2016-06-17 13:08 ` Kishon Vijay Abraham I
2016-06-20  0:58   ` Shawn Lin [this message]
2016-06-20  6:36     ` Kishon Vijay Abraham I
     [not found]       ` <428a1393-c6b4-163c-ceec-9b79fdd8ad4a@rock-chips.com>
2016-06-23 23:37         ` Brian Norris
2016-06-24  1:37           ` Shawn Lin
2016-06-27 23:23             ` Brian Norris
2016-06-27  5:24           ` Kishon Vijay Abraham I
2016-06-27 23:24             ` Brian Norris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c88eb16f-e976-ad52-02fb-5ec7b6e261b9@rock-chips.com \
    --to=shawn.lin@rock-chips.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=heiko@sntech.de \
    --cc=kishon@ti.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=robh+dt@kernel.org \
    --cc=wenrui.li@rock-chips.com \
    --subject='Re: [PATCH v3 2/2] phy: add a driver for the Rockchip SoC internal PCIe PHY' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).