| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <576739B9.4070202@rock-chips.com>
Date: Mon, 20 Jun 2016 08:32:57 +0800
From: Chris Zhong <zyw@...k-chips.com>
To: Kishon Vijay Abraham I <kishon@...com>, dianders@...omium.org,
tfiga@...omium.org, heiko@...ech.de, yzq@...k-chips.com,
groeck@...omium.org
Cc: linux-rockchip@...ts.infradead.org,
Kever Yang <kever.yang@...k-chips.com>,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [v2 PATCH 2/4] phy: Add USB Type-C PHY driver for rk3399
Hi Kishon
On 06/17/2016 08:54 PM, Kishon Vijay Abraham I wrote:
>
> On Monday 13 June 2016 03:09 PM, Chris Zhong wrote:
>> Add a PHY provider driver for the rk3399 SoC Type-c PHY. The USB
>> Type-C PHY is designed to support the USB3 and DP applications. The
>> PHY basically has two main components: USB3 and DisplyPort. USB3
>> operates in SuperSpeed mode and the DP can operate at RBR, HBR and
>> HBR2 data rates.
>>
>> Signed-off-by: Chris Zhong <zyw@...k-chips.com>
>> Signed-off-by: Kever Yang <kever.yang@...k-chips.com>
>>
>> ---
>>
>> Changes in v2:
>> - select RESET_CONTROLLER
>> - alphabetic order
>> - modify some spelling mistakes
>> - make mode cleaner
>> - use bool for enable/disable
>> - check all of the return value
>> - return a better err number
>> - use more readx_poll_timeout()
>> - clk_disable_unprepare(tcphy->clk_ref);
>> - remove unuse functions, rockchip_typec_phy_power_on/off
>> - remove unnecessary typecast from void *
>> - use dts node to distinguish between phys.
>>
>> Changes in v1:
>> - update the licence note
>> - init core clock to 50MHz
>> - use extcon API
>> - remove unused global
>> - add some comments for magic num
>> - change usleep_range(1000, 2000) tousleep_range(1000, 1050)
>> - remove __func__ from dev_err
>> - return err number when get clk failed
>> - remove ADDR_ADJ define
>> - use devm_clk_get(&pdev->dev, "tcpdcore")
>>
>> drivers/phy/Kconfig | 8 +
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-rockchip-typec.c | 952 +++++++++++++++++++++++++++++++++
>> include/linux/phy/phy-rockchip-typec.h | 20 +
>> 4 files changed, 981 insertions(+)
>> create mode 100644 drivers/phy/phy-rockchip-typec.c
>> create mode 100644 include/linux/phy/phy-rockchip-typec.h
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 26566db..ec87b3a 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -351,6 +351,14 @@ config PHY_ROCKCHIP_DP
>> help
>> Enable this to support the Rockchip Display Port PHY.
>>
>> +config PHY_ROCKCHIP_TYPEC
>> + tristate "Rockchip TYPEC PHY Driver"
>> + depends on ARCH_ROCKCHIP && OF
>> + select GENERIC_PHY
>> + select RESET_CONTROLLER
>> + help
>> + Enable this to support the Rockchip USB TYPEC 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 24596a9..91fa413 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -39,6 +39,7 @@ 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_DP) += phy-rockchip-dp.o
>> +obj-$(CONFIG_PHY_ROCKCHIP_TYPEC) += phy-rockchip-typec.o
>> obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
>> obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY) += phy-spear1310-miphy.o
>> obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
>> diff --git a/drivers/phy/phy-rockchip-typec.c b/drivers/phy/phy-rockchip-typec.c
>> new file mode 100644
>> index 0000000..230e074
>> --- /dev/null
>> +++ b/drivers/phy/phy-rockchip-typec.c
> <snip>
>
>> +static const struct phy_ops rockchip_tcphy_ops = {
>> + .owner = THIS_MODULE,
> no ops?
>> +};
>> +
>> +static int tcphy_pd_event(struct notifier_block *nb,
>> + unsigned long event, void *priv)
>> +{
>> + struct rockchip_typec_phy *tcphy;
>> + struct extcon_dev *edev = priv;
>> + int value = edev->state;
>> + int mode;
>> + u8 is_plugged, dfp;
>> +
>> + tcphy = container_of(nb, struct rockchip_typec_phy, event_nb);
>> +
>> + is_plugged = GET_PLUGGED(value);
>> + tcphy->flip = GET_FLIP(value);
>> + dfp = GET_DFP(value);
>> + tcphy->map = GET_PIN_MAP(value);
>> +
>> + if (is_plugged) {
>> + if (!dfp)
>> + mode = MODE_UFP_USB;
>> + else if (tcphy->map & (PIN_MAP_B | PIN_MAP_D | PIN_MAP_F))
>> + mode = MODE_DFP_USB | MODE_DFP_DP;
>> + else if (tcphy->map & (PIN_MAP_A | PIN_MAP_C | PIN_MAP_E))
>> + mode = MODE_DFP_DP;
>> + else
>> + mode = MODE_DFP_USB;
>> + } else {
>> + mode = MODE_DISCONNECT;
>> + }
>> +
>> + if (tcphy->mode != mode) {
>> + tcphy->mode = mode;
>> + schedule_delayed_work_on(0, &tcphy->event_wq, 0);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void tcphy_event_wq(struct work_struct *work)
>> +{
>> + struct rockchip_typec_phy *tcphy;
>> + int ret;
>> +
>> + tcphy = container_of(work, struct rockchip_typec_phy, event_wq.work);
>> +
>> + if (tcphy->mode == MODE_DISCONNECT) {
>> + tcphy_phy_deinit(tcphy);
>> + /* remove hpd sign for DP */
>> + if (tcphy->hpd_status) {
>> + regmap_write(tcphy->grf_regs, GRF_SOC_CON26,
>> + DPTX_HPD_SEL_MASK | DPTX_HPD_DEL);
>> + tcphy->hpd_status = 0;
>> + }
>> + } else {
>> + ret = tcphy_phy_init(tcphy);
>> + if (ret)
>> + return;
>> +
>> + if (tcphy->mode & (MODE_UFP_USB | MODE_DFP_USB))
>> + tcphy_usb3_init(tcphy);
>> +
>> + if (tcphy->mode & MODE_DFP_DP) {
>> + ret = tcphy_dp_init(tcphy);
>> +
>> + /* set hpd sign for DP, if DP phy is ready */
>> + if (!ret) {
>> + regmap_write(tcphy->grf_regs, GRF_SOC_CON26,
>> + DPTX_HPD_SEL_MASK | DPTX_HPD_SEL);
>> + tcphy->hpd_status = 1;
>> + }
>> + }
>> + }
>> +}
>> +
>> +static int tcphy_get_param(struct device *dev,
>> + struct usb3phy_reg *reg,
>> + const char *name)
>> +{
>> + int ret, buffer[3];
>> +
>> + ret = of_property_read_u32_array(dev->of_node, name, buffer, 3);
>> + if (ret) {
>> + dev_err(dev, "Can not parse %s\n", name);
>> + return ret;
>> + }
>> +
>> + reg->offset = buffer[0];
>> + reg->enable_bit = buffer[1];
>> + reg->write_enable = buffer[2];
>> + return 0;
>> +}
>> +
>> +static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
>> + struct device *dev)
>> +{
>> + struct rockchip_usb3phy_port_cfg *cfg = &tcphy->port_cfgs;
>> + int ret;
>> +
>> + ret = tcphy_get_param(dev, &cfg->typec_conn_dir,
>> + "rockchip,typec_conn_dir");
>> + if (ret)
>> + return ret;
>> +
>> + ret = tcphy_get_param(dev, &cfg->usb3tousb2_en,
>> + "rockchip,usb3tousb2_en");
>> + if (ret)
>> + return ret;
>> +
>> + ret = tcphy_get_param(dev, &cfg->external_psm,
>> + "rockchip,external_psm");
>> + if (ret)
>> + return ret;
>> +
>> + ret = tcphy_get_param(dev, &cfg->pipe_status,
>> + "rockchip,pipe_status");
>> + if (ret)
>> + return ret;
>> +
>> + ret = tcphy_get_param(dev, &cfg->uphy_dp_sel,
>> + "rockchip,uphy_dp_sel");
>> + if (ret)
>> + return ret;
>> +
>> + tcphy->grf_regs = syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "rockchip,grf");
>> + if (IS_ERR(tcphy->grf_regs)) {
>> + dev_err(dev, "could not find grf dt node\n");
>> + return PTR_ERR(tcphy->grf_regs);
>> + }
>> +
>> + tcphy->clk_core = devm_clk_get(dev, "tcpdcore");
>> + if (IS_ERR(tcphy->clk_core)) {
>> + dev_err(dev, "could not get uphy core clock\n");
>> + return PTR_ERR(tcphy->clk_core);
>> + }
>> +
>> + tcphy->clk_ref = devm_clk_get(dev, "tcpdphy_ref");
>> + if (IS_ERR(tcphy->clk_ref)) {
>> + dev_err(dev, "could not get uphy ref clock\n");
>> + return PTR_ERR(tcphy->clk_ref);
>> + }
>> +
>> + tcphy->phy_rst = devm_reset_control_get(dev, "tcphy");
>> + if (IS_ERR(tcphy->phy_rst)) {
>> + dev_err(dev, "no phy_rst reset control found\n");
>> + return PTR_ERR(tcphy->phy_rst);
>> + }
>> +
>> + tcphy->pipe_rst = devm_reset_control_get(dev, "tcphy_pipe");
>> + if (IS_ERR(tcphy->pipe_rst)) {
>> + dev_err(dev, "no pipe_rst reset control found\n");
>> + return PTR_ERR(tcphy->pipe_rst);
>> + }
>> +
>> + tcphy->uphy_rst = devm_reset_control_get(dev, "uphy_tcphy");
>> + if (IS_ERR(tcphy->uphy_rst)) {
>> + dev_err(dev, "no uphy_rst reset control found\n");
>> + return PTR_ERR(tcphy->uphy_rst);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int rockchip_typec_phy_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct rockchip_typec_phy *tcphy;
>> + struct resource *res;
>> + struct phy_provider *phy_provider;
>> + int ret;
>> +
>> + tcphy = devm_kzalloc(dev, sizeof(*tcphy), GFP_KERNEL);
>> + if (!tcphy)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + tcphy->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(tcphy->base)) {
>> + dev_err(dev, "failed to remap phy regs\n");
>> + return PTR_ERR(tcphy->base);
>> + }
>> +
>> + tcphy_parse_dt(tcphy, dev);
>> +
>> + tcphy->dev = dev;
>> + platform_set_drvdata(pdev, tcphy);
>> +
>> + tcphy->mode = MODE_DISCONNECT;
>> +
>> + tcphy->phy = devm_phy_create(dev, NULL, &rockchip_tcphy_ops);
>
> your phy_ops doesn't have any ops populated. Is there a reason to use the phy
> framework?
>
> Thanks
> Kishon
>
>
Yes, I am thinking about it, remove it would be better.
Powered by blists - more mailing lists