* [RESEND PATCH v8 0/2] phy: Add USB PHY support on Intel LGM SoC @ 2020-08-17 7:05 Ramuthevar,Vadivel MuruganX 2020-08-17 7:05 ` [RESEND PATCH v8 1/2] dt-bindings: phy: Add USB PHY support for " Ramuthevar,Vadivel MuruganX 2020-08-17 7:05 ` [RESEND PATCH v8 2/2] phy: Add USB3 " Ramuthevar,Vadivel MuruganX 0 siblings, 2 replies; 5+ messages in thread From: Ramuthevar,Vadivel MuruganX @ 2020-08-17 7:05 UTC (permalink / raw) To: linux-kernel, kishon, vkoul, devicetree Cc: robh+dt, gregkh, p.zabel, balbi, andriy.shevchenko, cheol.yong.kim, qi-ming.wu, yin1.li, Ramuthevar,Vadivel MuruganX The USB PHY provides the optimized for low power dissipation while active, idle, or on standby. Requires minimal external components, a single resistor, for best operation. Supports 10/5-Gbps high-speed data transmission rates through 3-m USB 3.x cable --- v8-resend: - Correct the typo error in my previous patch v8: - Rebase to V5.9-rc1 v7: - No Change v6: - No Change v5: - As per Felipe and Greg's suggestion usb phy driver reviewed patches changed the folder from drivers/usb/phy to drivers/phy - Reviewed-By tag added in commit message v4: - Andy's review comments addressed - drop the excess error debug prints - error check optimized - merge the split line to one line v3: - Andy's review comments update - hardcode return value changed to actual return value from the callee - add error check is fixed according to the above - correct the assignment in redundant - combine the split line into one line v2: - Address Phillip's review comments - replace devm_reset_control_get() by devm_reset_control_get_exclusive() - re-design the assert and deassert fucntion calls as per review comments - address kbuild bot warnings - add the comments v1: - initial version --- dt-bindings: usb: Add USB PHY support for Intel LGM SoC v8-resend: - No change v8: - No Change v7: - Fixed the bot issue: usb-phy@e7e00000: '#phy-cells' is a required property v6: - Fixed the bot issue. - replace node-name by usb-phy@ in example v5: - Reviewed-By tag added v4: - No Change v3: - No Change v2: - No Change v1: - initial version Ramuthevar Vadivel Murugan (2): dt-bindings: phy: Add USB PHY support for Intel LGM SoC phy: Add USB3 PHY support for Intel LGM SoC .../devicetree/bindings/phy/intel,lgm-usb-phy.yaml | 58 +++++ drivers/phy/Kconfig | 11 + drivers/phy/Makefile | 1 + drivers/phy/phy-lgm-usb.c | 278 +++++++++++++++++++++ 4 files changed, 348 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/intel,lgm-usb-phy.yaml create mode 100644 drivers/phy/phy-lgm-usb.c -- 2.11.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [RESEND PATCH v8 1/2] dt-bindings: phy: Add USB PHY support for Intel LGM SoC 2020-08-17 7:05 [RESEND PATCH v8 0/2] phy: Add USB PHY support on Intel LGM SoC Ramuthevar,Vadivel MuruganX @ 2020-08-17 7:05 ` Ramuthevar,Vadivel MuruganX 2020-08-17 7:05 ` [RESEND PATCH v8 2/2] phy: Add USB3 " Ramuthevar,Vadivel MuruganX 1 sibling, 0 replies; 5+ messages in thread From: Ramuthevar,Vadivel MuruganX @ 2020-08-17 7:05 UTC (permalink / raw) To: linux-kernel, kishon, vkoul, devicetree Cc: robh+dt, gregkh, p.zabel, balbi, andriy.shevchenko, cheol.yong.kim, qi-ming.wu, yin1.li, Ramuthevar Vadivel Murugan From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> Add the dt-schema to support USB PHY on Intel LGM SoC Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> Reviewed-by: Rob Herring <robh@kernel.org> --- .../devicetree/bindings/phy/intel,lgm-usb-phy.yaml | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/intel,lgm-usb-phy.yaml diff --git a/Documentation/devicetree/bindings/phy/intel,lgm-usb-phy.yaml b/Documentation/devicetree/bindings/phy/intel,lgm-usb-phy.yaml new file mode 100644 index 000000000000..ce62c0b94daf --- /dev/null +++ b/Documentation/devicetree/bindings/phy/intel,lgm-usb-phy.yaml @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/intel,lgm-usb-phy.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Intel LGM USB PHY Device Tree Bindings + +maintainers: + - Vadivel Murugan Ramuthevar <vadivel.muruganx.ramuthevar@linux.intel.com> + +properties: + compatible: + const: intel,lgm-usb-phy + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + resets: + items: + - description: USB PHY and Host controller reset + - description: APB BUS reset + - description: General Hardware reset + + reset-names: + items: + - const: phy + - const: apb + - const: phy31 + + "#phy-cells": + const: 0 + +required: + - compatible + - clocks + - reg + - resets + - reset-names + - "#phy-cells" + +additionalProperties: false + +examples: + - | + usb-phy@e7e00000 { + compatible = "intel,lgm-usb-phy"; + reg = <0xe7e00000 0x10000>; + clocks = <&cgu0 153>; + resets = <&rcu 0x70 0x24>, + <&rcu 0x70 0x26>, + <&rcu 0x70 0x28>; + reset-names = "phy", "apb", "phy31"; + #phy-cells = <0>; + }; -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RESEND PATCH v8 2/2] phy: Add USB3 PHY support for Intel LGM SoC 2020-08-17 7:05 [RESEND PATCH v8 0/2] phy: Add USB PHY support on Intel LGM SoC Ramuthevar,Vadivel MuruganX 2020-08-17 7:05 ` [RESEND PATCH v8 1/2] dt-bindings: phy: Add USB PHY support for " Ramuthevar,Vadivel MuruganX @ 2020-08-17 7:05 ` Ramuthevar,Vadivel MuruganX 2020-08-23 16:06 ` Vinod Koul 1 sibling, 1 reply; 5+ messages in thread From: Ramuthevar,Vadivel MuruganX @ 2020-08-17 7:05 UTC (permalink / raw) To: linux-kernel, kishon, vkoul, devicetree Cc: robh+dt, gregkh, p.zabel, balbi, andriy.shevchenko, cheol.yong.kim, qi-ming.wu, yin1.li, Ramuthevar Vadivel Murugan From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> Add support for USB PHY on Intel LGM SoC. Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> --- drivers/phy/Kconfig | 11 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-lgm-usb.c | 278 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 290 insertions(+) create mode 100644 drivers/phy/phy-lgm-usb.c diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index de9362c25c07..01b53f86004c 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -49,6 +49,17 @@ config PHY_XGENE help This option enables support for APM X-Gene SoC multi-purpose PHY. +config USB_LGM_PHY + tristate "INTEL Lightning Mountain USB PHY Driver" + depends on USB_SUPPORT + select USB_PHY + select REGULATOR + select REGULATOR_FIXED_VOLTAGE + help + Enable this to support Intel DWC3 PHY USB phy. This driver provides + interface to interact with USB GEN-II and USB 3.x PHY that is part + of the Intel network SOC. + source "drivers/phy/allwinner/Kconfig" source "drivers/phy/amlogic/Kconfig" source "drivers/phy/broadcom/Kconfig" diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index c27408e4daae..6eb2916773c5 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY) += phy-core-mipi-dphy.o obj-$(CONFIG_PHY_LPC18XX_USB_OTG) += phy-lpc18xx-usb-otg.o obj-$(CONFIG_PHY_XGENE) += phy-xgene.o obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o +obj-$(CONFIG_USB_LGM_PHY) += phy-lgm-usb.o obj-y += allwinner/ \ amlogic/ \ broadcom/ \ diff --git a/drivers/phy/phy-lgm-usb.c b/drivers/phy/phy-lgm-usb.c new file mode 100644 index 000000000000..1ec9ab266e08 --- /dev/null +++ b/drivers/phy/phy-lgm-usb.c @@ -0,0 +1,278 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Intel LGM USB PHY driver + * + * Copyright (C) 2020 Intel Corporation. + */ + +#include <linux/bitfield.h> +#include <linux/delay.h> +#include <linux/iopoll.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/regulator/consumer.h> +#include <linux/reset.h> +#include <linux/usb/phy.h> +#include <linux/workqueue.h> + +#define CTRL1_OFFSET 0x14 +#define SRAM_EXT_LD_DONE BIT(25) +#define SRAM_INIT_DONE BIT(26) + +#define TCPC_OFFSET 0x1014 +#define TCPC_MUX_CTL GENMASK(1, 0) +#define MUX_NC 0 +#define MUX_USB 1 +#define MUX_DP 2 +#define MUX_USBDP 3 +#define TCPC_FLIPPED BIT(2) +#define TCPC_LOW_POWER_EN BIT(3) +#define TCPC_VALID BIT(4) +#define TCPC_CONN \ + (TCPC_VALID | FIELD_PREP(TCPC_MUX_CTL, MUX_USB)) +#define TCPC_DISCONN \ + (TCPC_VALID | FIELD_PREP(TCPC_MUX_CTL, MUX_NC) | TCPC_LOW_POWER_EN) + +static const char *const PHY_RESETS[] = { "phy31", "phy", }; +static const char *const CTL_RESETS[] = { "apb", "ctrl", }; + +struct tca_apb { + struct reset_control *resets[ARRAY_SIZE(PHY_RESETS)]; + struct regulator *vbus; + struct work_struct wk; + struct usb_phy phy; + + bool phy_initialized; + bool connected; +}; + +static int get_flipped(struct tca_apb *ta, bool *flipped) +{ + union extcon_property_value property; + int ret; + + ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_TYPEC_POLARITY, &property); + if (ret) { + dev_err(ta->phy.dev, "no polarity property from extcon\n"); + return ret; + } + + *flipped = property.intval; + + return ret; +} + +static int phy_init(struct usb_phy *phy) +{ + struct tca_apb *ta = container_of(phy, struct tca_apb, phy); + void __iomem *ctrl1 = phy->io_priv + CTRL1_OFFSET; + int val, ret, i; + + if (ta->phy_initialized) + return 0; + + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) + reset_control_deassert(ta->resets[i]); + + ret = readl_poll_timeout(ctrl1, val, val & SRAM_INIT_DONE, 10, 10 * 1000); + if (ret) { + dev_err(ta->phy.dev, "SRAM init failed, 0x%x\n", val); + return ret; + } + + writel(readl(ctrl1) | SRAM_EXT_LD_DONE, ctrl1); + + ta->phy_initialized = true; + if (!ta->phy.edev) { + writel(TCPC_CONN, ta->phy.io_priv + TCPC_OFFSET); + return phy->set_vbus(phy, true); + } + + schedule_work(&ta->wk); + + return ret; +} + +static void phy_shutdown(struct usb_phy *phy) +{ + struct tca_apb *ta = container_of(phy, struct tca_apb, phy); + int i; + + if (!ta->phy_initialized) + return; + + ta->phy_initialized = false; + flush_work(&ta->wk); + ta->phy.set_vbus(&ta->phy, false); + + ta->connected = false; + writel(TCPC_DISCONN, ta->phy.io_priv + TCPC_OFFSET); + + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) + reset_control_assert(ta->resets[i]); +} + +static int phy_set_vbus(struct usb_phy *phy, int on) +{ + struct tca_apb *ta = container_of(phy, struct tca_apb, phy); + int ret; + + if (on) { + ret = regulator_enable(ta->vbus); + if (ret) + dev_err(ta->phy.dev, "regulator not enabled\n"); + } else { + ret = regulator_disable(ta->vbus); + if (ret) + dev_err(ta->phy.dev, "regulator not disabled\n"); + } + + return ret; +} + +static void tca_work(struct work_struct *work) +{ + struct tca_apb *ta = container_of(work, struct tca_apb, wk); + bool connected; + bool flipped = false; + u32 val; + int ret; + + ret = get_flipped(ta, &flipped); + connected = extcon_get_state(ta->phy.edev, EXTCON_USB_HOST) && !ret; + if (connected == ta->connected) + return; + + ta->connected = connected; + if (connected) { + val = TCPC_CONN; + if (flipped) + val |= TCPC_FLIPPED; + dev_info(ta->phy.dev, "connected%s\n", flipped ? " flipped" : ""); + } else { + val = TCPC_DISCONN; + dev_info(ta->phy.dev, "disconnected\n"); + } + + writel(val, ta->phy.io_priv + TCPC_OFFSET); + + ret = ta->phy.set_vbus(&ta->phy, connected); + if (ret) + dev_err(ta->phy.dev, "failed to set VBUS\n"); +} + +static int id_notifier(struct notifier_block *nb, unsigned long event, void *ptr) +{ + struct tca_apb *ta = container_of(nb, struct tca_apb, phy.id_nb); + + if (ta->phy_initialized) + schedule_work(&ta->wk); + + return NOTIFY_DONE; +} + +static int vbus_notifier(struct notifier_block *nb, unsigned long evnt, void *ptr) +{ + return NOTIFY_DONE; +} + +static int phy_probe(struct platform_device *pdev) +{ + struct reset_control *resets[ARRAY_SIZE(CTL_RESETS)]; + struct device *dev = &pdev->dev; + struct usb_phy *phy; + struct tca_apb *ta; + int i; + + ta = devm_kzalloc(dev, sizeof(*ta), GFP_KERNEL); + if (!ta) + return -ENOMEM; + + platform_set_drvdata(pdev, ta); + INIT_WORK(&ta->wk, tca_work); + + phy = &ta->phy; + phy->dev = dev; + phy->label = dev_name(dev); + phy->type = USB_PHY_TYPE_USB3; + phy->init = phy_init; + phy->shutdown = phy_shutdown; + phy->set_vbus = phy_set_vbus; + phy->id_nb.notifier_call = id_notifier; + phy->vbus_nb.notifier_call = vbus_notifier; + + phy->io_priv = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(phy->io_priv)) + return PTR_ERR(phy->io_priv); + + ta->vbus = devm_regulator_get(dev, "vbus"); + if (IS_ERR(ta->vbus)) + return PTR_ERR(ta->vbus); + + for (i = 0; i < ARRAY_SIZE(CTL_RESETS); i++) { + resets[i] = devm_reset_control_get_exclusive(dev, CTL_RESETS[i]); + if (IS_ERR(resets[i])) { + dev_err(dev, "%s reset not found\n", CTL_RESETS[i]); + return PTR_ERR(resets[i]); + } + } + + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) { + ta->resets[i] = devm_reset_control_get_exclusive(dev, PHY_RESETS[i]); + if (IS_ERR(ta->resets[i])) { + dev_err(dev, "%s reset not found\n", PHY_RESETS[i]); + return PTR_ERR(ta->resets[i]); + } + } + + for (i = 0; i < ARRAY_SIZE(CTL_RESETS); i++) + reset_control_assert(resets[i]); + + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) + reset_control_assert(ta->resets[i]); + /* + * Out-of-band reset of the controller after PHY reset will cause + * controller malfunctioning, so we should use in-band controller + * reset only and leave the controller de-asserted here. + */ + for (i = 0; i < ARRAY_SIZE(CTL_RESETS); i++) + reset_control_deassert(resets[i]); + + /* Need to wait at least 20us after de-assert the controller */ + usleep_range(20, 100); + + return usb_add_phy_dev(phy); +} + +static int phy_remove(struct platform_device *pdev) +{ + struct tca_apb *ta = platform_get_drvdata(pdev); + + usb_remove_phy(&ta->phy); + + return 0; +} + +static const struct of_device_id intel_usb_phy_dt_ids[] = { + { .compatible = "intel,lgm-usb-phy" }, + { } +}; +MODULE_DEVICE_TABLE(of, intel_usb_phy_dt_ids); + +static struct platform_driver lgm_phy_driver = { + .driver = { + .name = "lgm-usb-phy", + .of_match_table = intel_usb_phy_dt_ids, + }, + .probe = phy_probe, + .remove = phy_remove, +}; + +module_platform_driver(lgm_phy_driver); + +MODULE_DESCRIPTION("Intel LGM USB PHY driver"); +MODULE_AUTHOR("Li Yin <yin1.li@intel.com>"); +MODULE_AUTHOR("Vadivel Murugan R <vadivel.muruganx.ramuthevar@linux.intel.com>"); +MODULE_LICENSE("GPL v2"); -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH v8 2/2] phy: Add USB3 PHY support for Intel LGM SoC 2020-08-17 7:05 ` [RESEND PATCH v8 2/2] phy: Add USB3 " Ramuthevar,Vadivel MuruganX @ 2020-08-23 16:06 ` Vinod Koul 2020-08-25 3:05 ` Ramuthevar, Vadivel MuruganX 0 siblings, 1 reply; 5+ messages in thread From: Vinod Koul @ 2020-08-23 16:06 UTC (permalink / raw) To: Ramuthevar,Vadivel MuruganX Cc: linux-kernel, kishon, devicetree, robh+dt, gregkh, p.zabel, balbi, andriy.shevchenko, cheol.yong.kim, qi-ming.wu, yin1.li On 17-08-20, 15:05, Ramuthevar,Vadivel MuruganX wrote: > From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> > > Add support for USB PHY on Intel LGM SoC. > > Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> > --- > drivers/phy/Kconfig | 11 ++ > drivers/phy/Makefile | 1 + > drivers/phy/phy-lgm-usb.c | 278 ++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 290 insertions(+) > create mode 100644 drivers/phy/phy-lgm-usb.c > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index de9362c25c07..01b53f86004c 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -49,6 +49,17 @@ config PHY_XGENE > help > This option enables support for APM X-Gene SoC multi-purpose PHY. > > +config USB_LGM_PHY > + tristate "INTEL Lightning Mountain USB PHY Driver" > + depends on USB_SUPPORT Why is the dependent on USB..? Should that not be other way around? > +static int get_flipped(struct tca_apb *ta, bool *flipped) > +{ > + union extcon_property_value property; > + int ret; > + > + ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST, > + EXTCON_PROP_USB_TYPEC_POLARITY, &property); > + if (ret) { > + dev_err(ta->phy.dev, "no polarity property from extcon\n"); > + return ret; > + } > + > + *flipped = property.intval; > + > + return ret; return 0 here? > +static int phy_init(struct usb_phy *phy) > +{ > + struct tca_apb *ta = container_of(phy, struct tca_apb, phy); > + void __iomem *ctrl1 = phy->io_priv + CTRL1_OFFSET; > + int val, ret, i; > + > + if (ta->phy_initialized) > + return 0; > + > + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) > + reset_control_deassert(ta->resets[i]); > + > + ret = readl_poll_timeout(ctrl1, val, val & SRAM_INIT_DONE, 10, 10 * 1000); > + if (ret) { > + dev_err(ta->phy.dev, "SRAM init failed, 0x%x\n", val); > + return ret; > + } > + > + writel(readl(ctrl1) | SRAM_EXT_LD_DONE, ctrl1); > + > + ta->phy_initialized = true; > + if (!ta->phy.edev) { > + writel(TCPC_CONN, ta->phy.io_priv + TCPC_OFFSET); > + return phy->set_vbus(phy, true); > + } > + > + schedule_work(&ta->wk); why work for init? > +static void tca_work(struct work_struct *work) > +{ > + struct tca_apb *ta = container_of(work, struct tca_apb, wk); > + bool connected; > + bool flipped = false; > + u32 val; > + int ret; > + > + ret = get_flipped(ta, &flipped); so every time this work is scheduled you are reading from firmware, why.. Typically we should read in probe and store it.. > + connected = extcon_get_state(ta->phy.edev, EXTCON_USB_HOST) && !ret; lets handle ret and extcon_get_state separately please > + if (connected == ta->connected) > + return; > + > + ta->connected = connected; > + if (connected) { > + val = TCPC_CONN; > + if (flipped) > + val |= TCPC_FLIPPED; > + dev_info(ta->phy.dev, "connected%s\n", flipped ? " flipped" : ""); debug perhaps > + } else { > + val = TCPC_DISCONN; > + dev_info(ta->phy.dev, "disconnected\n"); here too > +static int vbus_notifier(struct notifier_block *nb, unsigned long evnt, void *ptr) > +{ > + return NOTIFY_DONE; > +} empty notifier, why bother? > +static int phy_probe(struct platform_device *pdev) > +{ > + struct reset_control *resets[ARRAY_SIZE(CTL_RESETS)]; > + struct device *dev = &pdev->dev; > + struct usb_phy *phy; > + struct tca_apb *ta; > + int i; > + > + ta = devm_kzalloc(dev, sizeof(*ta), GFP_KERNEL); > + if (!ta) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, ta); > + INIT_WORK(&ta->wk, tca_work); > + > + phy = &ta->phy; > + phy->dev = dev; > + phy->label = dev_name(dev); > + phy->type = USB_PHY_TYPE_USB3; > + phy->init = phy_init; > + phy->shutdown = phy_shutdown; > + phy->set_vbus = phy_set_vbus; > + phy->id_nb.notifier_call = id_notifier; > + phy->vbus_nb.notifier_call = vbus_notifier; > + > + phy->io_priv = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(phy->io_priv)) > + return PTR_ERR(phy->io_priv); > + > + ta->vbus = devm_regulator_get(dev, "vbus"); > + if (IS_ERR(ta->vbus)) > + return PTR_ERR(ta->vbus); > + > + for (i = 0; i < ARRAY_SIZE(CTL_RESETS); i++) { > + resets[i] = devm_reset_control_get_exclusive(dev, CTL_RESETS[i]); > + if (IS_ERR(resets[i])) { > + dev_err(dev, "%s reset not found\n", CTL_RESETS[i]); > + return PTR_ERR(resets[i]); > + } > + } > + > + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) { > + ta->resets[i] = devm_reset_control_get_exclusive(dev, PHY_RESETS[i]); > + if (IS_ERR(ta->resets[i])) { > + dev_err(dev, "%s reset not found\n", PHY_RESETS[i]); > + return PTR_ERR(ta->resets[i]); > + } > + } > + > + for (i = 0; i < ARRAY_SIZE(CTL_RESETS); i++) > + reset_control_assert(resets[i]); > + > + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) > + reset_control_assert(ta->resets[i]); no time lag b/w assert and dessert below? > + /* > + * Out-of-band reset of the controller after PHY reset will cause > + * controller malfunctioning, so we should use in-band controller > + * reset only and leave the controller de-asserted here. > + */ > + for (i = 0; i < ARRAY_SIZE(CTL_RESETS); i++) > + reset_control_deassert(resets[i]); > + > + /* Need to wait at least 20us after de-assert the controller */ > + usleep_range(20, 100); > + > + return usb_add_phy_dev(phy); aha, this is usb_phy stuff. Kishon this is not really a generic phy driver, should it be in drivers/phy..? -- ~Vinod ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH v8 2/2] phy: Add USB3 PHY support for Intel LGM SoC 2020-08-23 16:06 ` Vinod Koul @ 2020-08-25 3:05 ` Ramuthevar, Vadivel MuruganX 0 siblings, 0 replies; 5+ messages in thread From: Ramuthevar, Vadivel MuruganX @ 2020-08-25 3:05 UTC (permalink / raw) To: Vinod Koul Cc: linux-kernel, kishon, devicetree, robh+dt, gregkh, p.zabel, balbi, andriy.shevchenko, cheol.yong.kim, qi-ming.wu, yin1.li Hi Vinod, Thank you for the review comments... On 24/8/2020 12:06 am, Vinod Koul wrote: > On 17-08-20, 15:05, Ramuthevar,Vadivel MuruganX wrote: >> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> >> >> Add support for USB PHY on Intel LGM SoC. >> >> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> >> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> >> --- >> drivers/phy/Kconfig | 11 ++ >> drivers/phy/Makefile | 1 + >> drivers/phy/phy-lgm-usb.c | 278 ++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 290 insertions(+) >> create mode 100644 drivers/phy/phy-lgm-usb.c >> >> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >> index de9362c25c07..01b53f86004c 100644 >> --- a/drivers/phy/Kconfig >> +++ b/drivers/phy/Kconfig >> @@ -49,6 +49,17 @@ config PHY_XGENE >> help >> This option enables support for APM X-Gene SoC multi-purpose PHY. >> >> +config USB_LGM_PHY >> + tristate "INTEL Lightning Mountain USB PHY Driver" >> + depends on USB_SUPPORT > > Why is the dependent on USB..? Should that not be other way around? Good catch, it's not required. > >> +static int get_flipped(struct tca_apb *ta, bool *flipped) >> +{ >> + union extcon_property_value property; >> + int ret; >> + >> + ret = extcon_get_property(ta->phy.edev, EXTCON_USB_HOST, >> + EXTCON_PROP_USB_TYPEC_POLARITY, &property); >> + if (ret) { >> + dev_err(ta->phy.dev, "no polarity property from extcon\n"); >> + return ret; >> + } >> + >> + *flipped = property.intval; >> + >> + return ret; > > return 0 here? Noted. > >> +static int phy_init(struct usb_phy *phy) >> +{ >> + struct tca_apb *ta = container_of(phy, struct tca_apb, phy); >> + void __iomem *ctrl1 = phy->io_priv + CTRL1_OFFSET; >> + int val, ret, i; >> + >> + if (ta->phy_initialized) >> + return 0; >> + >> + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) >> + reset_control_deassert(ta->resets[i]); >> + >> + ret = readl_poll_timeout(ctrl1, val, val & SRAM_INIT_DONE, 10, 10 * 1000); >> + if (ret) { >> + dev_err(ta->phy.dev, "SRAM init failed, 0x%x\n", val); >> + return ret; >> + } >> + >> + writel(readl(ctrl1) | SRAM_EXT_LD_DONE, ctrl1); >> + >> + ta->phy_initialized = true; >> + if (!ta->phy.edev) { >> + writel(TCPC_CONN, ta->phy.io_priv + TCPC_OFFSET); >> + return phy->set_vbus(phy, true); >> + } >> + >> + schedule_work(&ta->wk); > > why work for init? Yes, it's required during the reboot and already device is connected status update to controller, well tested. > >> +static void tca_work(struct work_struct *work) >> +{ >> + struct tca_apb *ta = container_of(work, struct tca_apb, wk); >> + bool connected; >> + bool flipped = false; >> + u32 val; >> + int ret; >> + >> + ret = get_flipped(ta, &flipped); > > so every time this work is scheduled you are reading from firmware, why.. > Typically we should read in probe and store it.. > >> + connected = extcon_get_state(ta->phy.edev, EXTCON_USB_HOST) && !ret; > > lets handle ret and extcon_get_state separately please Sure, we can handle it separately. > >> + if (connected == ta->connected) >> + return; >> + >> + ta->connected = connected; >> + if (connected) { >> + val = TCPC_CONN; >> + if (flipped) >> + val |= TCPC_FLIPPED; >> + dev_info(ta->phy.dev, "connected%s\n", flipped ? " flipped" : ""); > > debug perhaps Noted. > >> + } else { >> + val = TCPC_DISCONN; >> + dev_info(ta->phy.dev, "disconnected\n"); > > here too Noted. > >> +static int vbus_notifier(struct notifier_block *nb, unsigned long evnt, void *ptr) >> +{ >> + return NOTIFY_DONE; >> +} > > empty notifier, why bother? Let me check and double confirm if it's required or not. > >> +static int phy_probe(struct platform_device *pdev) >> +{ >> + struct reset_control *resets[ARRAY_SIZE(CTL_RESETS)]; >> + struct device *dev = &pdev->dev; >> + struct usb_phy *phy; >> + struct tca_apb *ta; >> + int i; >> + >> + ta = devm_kzalloc(dev, sizeof(*ta), GFP_KERNEL); >> + if (!ta) >> + return -ENOMEM; >> + >> + platform_set_drvdata(pdev, ta); >> + INIT_WORK(&ta->wk, tca_work); >> + >> + phy = &ta->phy; >> + phy->dev = dev; >> + phy->label = dev_name(dev); >> + phy->type = USB_PHY_TYPE_USB3; >> + phy->init = phy_init; >> + phy->shutdown = phy_shutdown; >> + phy->set_vbus = phy_set_vbus; >> + phy->id_nb.notifier_call = id_notifier; >> + phy->vbus_nb.notifier_call = vbus_notifier; >> + >> + phy->io_priv = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(phy->io_priv)) >> + return PTR_ERR(phy->io_priv); >> + >> + ta->vbus = devm_regulator_get(dev, "vbus"); >> + if (IS_ERR(ta->vbus)) >> + return PTR_ERR(ta->vbus); >> + >> + for (i = 0; i < ARRAY_SIZE(CTL_RESETS); i++) { >> + resets[i] = devm_reset_control_get_exclusive(dev, CTL_RESETS[i]); >> + if (IS_ERR(resets[i])) { >> + dev_err(dev, "%s reset not found\n", CTL_RESETS[i]); >> + return PTR_ERR(resets[i]); >> + } >> + } >> + >> + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) { >> + ta->resets[i] = devm_reset_control_get_exclusive(dev, PHY_RESETS[i]); >> + if (IS_ERR(ta->resets[i])) { >> + dev_err(dev, "%s reset not found\n", PHY_RESETS[i]); >> + return PTR_ERR(ta->resets[i]); >> + } >> + } >> + >> + for (i = 0; i < ARRAY_SIZE(CTL_RESETS); i++) >> + reset_control_assert(resets[i]); >> + >> + for (i = 0; i < ARRAY_SIZE(PHY_RESETS); i++) >> + reset_control_assert(ta->resets[i]); > > no time lag b/w assert and dessert below? yes you're right but not required for Intel LGM SoC, it's working fine. Regards Vadivel > >> + /* >> + * Out-of-band reset of the controller after PHY reset will cause >> + * controller malfunctioning, so we should use in-band controller >> + * reset only and leave the controller de-asserted here. >> + */ >> + for (i = 0; i < ARRAY_SIZE(CTL_RESETS); i++) >> + reset_control_deassert(resets[i]); >> + >> + /* Need to wait at least 20us after de-assert the controller */ >> + usleep_range(20, 100); >> + >> + return usb_add_phy_dev(phy); > > aha, this is usb_phy stuff. > > Kishon this is not really a generic phy driver, should it be in > drivers/phy..? > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-25 3:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-17 7:05 [RESEND PATCH v8 0/2] phy: Add USB PHY support on Intel LGM SoC Ramuthevar,Vadivel MuruganX 2020-08-17 7:05 ` [RESEND PATCH v8 1/2] dt-bindings: phy: Add USB PHY support for " Ramuthevar,Vadivel MuruganX 2020-08-17 7:05 ` [RESEND PATCH v8 2/2] phy: Add USB3 " Ramuthevar,Vadivel MuruganX 2020-08-23 16:06 ` Vinod Koul 2020-08-25 3:05 ` Ramuthevar, Vadivel MuruganX
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).