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: <be5ddb68b247b8d3b7305ef46d703d0ba2b3753a.camel@linux.dev> Date: Sat, 18 Mar 2023 23:37:28 +0800 From: Cixi Geng <cixi.geng@...ux.dev> To: Baolin Wang <baolin.wang@...ux.alibaba.com>, gregkh@...uxfoundation.org, orsonzhai@...il.com, zhang.lyra@...il.com, arnd@...db.de, tony@...mide.com, felipe.balbi@...ux.intel.com, paul@...pouillou.net, linus.walleij@...aro.org, cixi.geng1@...soc.com, gengcixi@...il.com Cc: linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org Subject: Re: [RFC PATCH v1] usb/phy add sprd ums512 usbphy On Mon, 2023-03-13 at 17:11 +0800, Baolin Wang wrote: > > > On 3/13/2023 1:14 AM, Cixi Geng wrote: > > From: Cixi Geng <cixi.geng1@...soc.com> > > > > This driver is support USB2 phy for Spreadtrum UMS512 SOC's, > > > > Signed-off-by: Cixi Geng <cixi.geng1@...soc.com> > > --- > > drivers/usb/phy/Kconfig | 10 + > > drivers/usb/phy/Makefile | 1 + > > drivers/usb/phy/phy-sprd-ums512.c | 511 > > ++++++++++++++++++++++++++++++ > > drivers/usb/phy/phy-sprd-ums512.h | 39 +++ > > 4 files changed, 561 insertions(+) > > create mode 100644 drivers/usb/phy/phy-sprd-ums512.c > > create mode 100644 drivers/usb/phy/phy-sprd-ums512.h > > > > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig > > index 5f629d7cad64..fa5564e6f3a3 100644 > > --- a/drivers/usb/phy/Kconfig > > +++ b/drivers/usb/phy/Kconfig > > @@ -158,6 +158,16 @@ config USB_TEGRA_PHY > > This driver provides PHY support for the USB controllers > > found > > on NVIDIA Tegra SoC's. > > > > +config USB_SPRD_UMS512_PHY > > + tristate "Spreadtrum ums512 USB2 PHY Driver" > > + depends on ARCH_SPRD || COMPILE_TEST > > + select USB_PHY > > + select EXTCON_USB_GPIO > > + help > > + Enable this to support the SPRD ums512 USB2 PHY that is > > part of SOC. > > + This driver takes care of all the PHY functionality, > > normally paired with > > + DesignWare USB20 (DRD) Controller. > > + > > config USB_ULPI > > bool "Generic ULPI Transceiver Driver" > > depends on ARM || ARM64 || COMPILE_TEST > > diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile > > index e5d619b4d8f6..ce45ee0f12a8 100644 > > --- a/drivers/usb/phy/Makefile > > +++ b/drivers/usb/phy/Makefile > > @@ -22,4 +22,5 @@ obj-$(CONFIG_USB_MV_OTG) += phy-mv- > > usb.o > > obj-$(CONFIG_USB_MXS_PHY) += phy-mxs-usb.o > > obj-$(CONFIG_USB_ULPI) += phy-ulpi.o > > obj-$(CONFIG_USB_ULPI_VIEWPORT) += phy-ulpi- > > viewport.o > > +obj-$(CONFIG_USB_SPRD_UMS512_PHY) += phy-sprd-ums512.o > > obj-$(CONFIG_KEYSTONE_USB_PHY) += phy-keystone.o > > diff --git a/drivers/usb/phy/phy-sprd-ums512.c > > b/drivers/usb/phy/phy-sprd-ums512.c > > new file mode 100644 > > index 000000000000..025f45e8d509 > > --- /dev/null > > +++ b/drivers/usb/phy/phy-sprd-ums512.c > > @@ -0,0 +1,511 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Unisoc USB PHY driver > > + * > > + * Copyright (C) 2023 Unisoc Inc. > > + */ > > + > > +#include <linux/delay.h> > > +#include <linux/device.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_platform.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > +#include <linux/regulator/consumer.h> > > +#include <linux/slab.h> > > +#include <linux/usb/otg.h> > > +#include <linux/usb/phy.h> > > +#include <uapi/linux/usb/charger.h> > > + > > +#include "phy-sprd-ums512.h" > > + > > +struct sprd_hsphy { > > + struct device *dev; > > + struct usb_phy phy; > > + struct regulator *vdd; > > + struct regmap *hsphy_glb; > > + struct regmap *ana_g2; > > + struct regmap *pmic; > > + u32 vdd_vol; > > + atomic_t reset; > > + atomic_t inited; > > + bool is_host; > > +}; > > + > > +#define TUNEHSAMP_2_6MA (3 << 25) > > +#define TFREGRES_TUNE_VALUE (0x14 << 19) > > +#define SC2730_CHARGE_STATUS 0x1b9c > > +#define BIT_CHG_DET_DONE BIT(11) > > +#define BIT_SDP_INT BIT(7) > > +#define BIT_DCP_INT BIT(6) > > +#define BIT_CDP_INT BIT(5) > > + > > +static enum usb_charger_type sc27xx_charger_detect(struct regmap > > *regmap) > > +{ > > + enum usb_charger_type type; > > + u32 status = 0, val; > > + int ret, cnt = 10; > > + > > + do { > > + ret = regmap_read(regmap, SC2730_CHARGE_STATUS, > > &val); > > + if (ret) > > + return UNKNOWN_TYPE; > > + > > + if (val & BIT_CHG_DET_DONE) { > > + status = val & (BIT_CDP_INT | BIT_DCP_INT | > > BIT_SDP_INT); > > + break; > > + } > > + > > + msleep(200); > > + } while (--cnt > 0); > > + > > + switch (status) { > > + case BIT_CDP_INT: > > + type = CDP_TYPE; > > + break; > > + case BIT_DCP_INT: > > + type = DCP_TYPE; > > + break; > > + case BIT_SDP_INT: > > + type = SDP_TYPE; > > + break; > > + default: > > + type = UNKNOWN_TYPE; > > + } > > + > > + return type; > > +} > > Please do not add duplicate code, and use the > sprd_pmic_detect_charger_type() instead, which had been exported in > the > mainline. ok, I will remove this next patchset. > > > + > > +static inline void sprd_hsphy_enable(struct sprd_hsphy *sprd_phy) > > +{ > > + u32 reg, msk; > > + > > + /* enable usb module */ > > + reg = msk = (MASK_AON_APB_OTG_UTMI_EB | > > MASK_AON_APB_ANA_EB); > > + regmap_update_bits(sprd_phy->hsphy_glb, > > REG_AON_APB_APB_EB1, msk, reg); > > + reg = msk = MASK_AON_APB_CGM_OTG_REF_EN | > > + MASK_AON_APB_CGM_DPHY_REF_EN; > > + regmap_update_bits(sprd_phy->hsphy_glb, > > REG_AON_APB_CGM_REG1, msk, reg); > > +} > > + > > +static inline void sprd_hsphy_power_down(struct sprd_hsphy > > *sprd_phy) > > +{ > > + u32 reg, msk; > > + > > + /* usb power down */ > > + reg = msk = (MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L | > > + MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S); > > + regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL, msk, > > reg); > > +} > > + > > +static inline void sprd_hsphy_reset_core(struct sprd_hsphy > > *sprd_phy) > > +{ > > + u32 reg, msk; > > + > > + /* Reset PHY */ > > + reg = msk = MASK_AON_APB_OTG_PHY_SOFT_RST | > > MASK_AON_APB_OTG_UTMI_SOFT_RST; > > + > > + regmap_update_bits(sprd_phy->hsphy_glb, > > REG_AON_APB_APB_RST1, msk, reg); > > + /* USB PHY reset need to delay 20ms~30ms */ > > + usleep_range(20000, 30000); > > + regmap_update_bits(sprd_phy->hsphy_glb, > > REG_AON_APB_APB_RST1, msk, 0); > > +} > > + > > +static int sprd_hostphy_set(struct usb_phy *u_phy, int on) > > +{ > > + struct sprd_hsphy *sprd_phy = container_of(u_phy, struct > > sprd_hsphy, phy); > > + u32 reg, msk; > > + int ret = 0; > > + > > + if (on) { > > + msk = MASK_AON_APB_USB2_PHY_IDDIG; > > + ret |= regmap_update_bits(sprd_phy->hsphy_glb, > > + REG_AON_APB_OTG_PHY_CTRL, msk, 0); > > + > > + msk = > > MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN | > > + MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20 > > _DPPULLDOWN; > > + ret |= regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0, > > + msk, msk); > > + > > + msk = > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN | > > + MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLD > > OWN; > > + ret |= regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL > > 2, > > + msk, msk); > > + > > + reg = 0x200; > > + msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED; > > + ret |= regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL > > 1, > > + msk, reg); > > + sprd_phy->is_host = true; > > + } else { > > + reg = msk = MASK_AON_APB_USB2_PHY_IDDIG; > > + ret |= regmap_update_bits(sprd_phy->hsphy_glb, > > + REG_AON_APB_OTG_PHY_CTRL, msk, reg); > > + > > + msk = > > MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN | > > + MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20 > > _DPPULLDOWN; > > + ret |= regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0, > > + msk, msk); > > + > > + msk = > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN | > > + MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLD > > OWN; > > + ret |= regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL > > 2, > > + msk, 0); > > + > > + msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED; > > + ret |= regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL > > 1, > > + msk, 0); > > + sprd_phy->is_host = false; > > + } > > + return ret; > > +} > > + > > +static int sprd_hsphy_init(struct usb_phy *u_phy) > > +{ > > + struct sprd_hsphy *sprd_phy = container_of(u_phy, struct > > sprd_hsphy, phy); > > + u32 reg, msk; > > + int ret; > > + > > + if (atomic_read(&sprd_phy->inited)) { > > + dev_dbg(u_phy->dev, "%s is already inited!\n", > > __func__); > > + return 0; > > + } > > + > > + /* Turn On VDD */ > > + regulator_set_voltage(sprd_phy->vdd, sprd_phy->vdd_vol, > > sprd_phy->vdd_vol); > > + if (!regulator_is_enabled(sprd_phy->vdd)) { > > + ret = regulator_enable(sprd_phy->vdd); > > + if (ret) > > + return ret; > > + } > > + > > + sprd_hsphy_enable(sprd_phy); > > + regmap_update_bits(sprd_phy->ana_g2, > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW, > > + MASK_ANLG_PHY_G2_ANALOG_USB20_USB20 > > _ISO_SW_EN, 0); > > + > > + /* usb phy power */ > > + msk = (MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L | > > + MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S); > > + regmap_update_bits(sprd_phy->ana_g2, > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL, > > + msk, 0); > > + > > + /* usb vbus valid */ > > + reg = msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG; > > + regmap_update_bits(sprd_phy->hsphy_glb, > > REG_AON_APB_OTG_PHY_TEST, msk, reg); > > + > > + reg = msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT; > > + regmap_update_bits(sprd_phy->ana_g2, > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1, > > + msk, reg); > > + > > + /* for SPRD phy utmi_width sel */ > > + reg = msk = MASK_AON_APB_UTMI_WIDTH_SEL; > > + regmap_update_bits(sprd_phy->hsphy_glb, > > REG_AON_APB_OTG_PHY_CTRL, msk, reg); > > + > > + reg = msk = > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DATABUS16_8; > > + regmap_update_bits(sprd_phy->ana_g2, > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1, > > + msk, reg); > > + > > + reg = TUNEHSAMP_2_6MA; > > + msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TUNEHSAMP; > > + regmap_update_bits(sprd_phy->ana_g2, > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING, > > + msk, reg); > > + > > + reg = TFREGRES_TUNE_VALUE; > > + msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TFREGRES; > > + regmap_update_bits(sprd_phy->ana_g2, > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING, > > + msk, reg); > > + > > + if (!atomic_read(&sprd_phy->reset)) { > > + sprd_hsphy_reset_core(sprd_phy); > > + atomic_set(&sprd_phy->reset, 1); > > + } > > + > > + atomic_set(&sprd_phy->inited, 1); > > + > > + return 0; > > +} > > + > > +static void sprd_hsphy_shutdown(struct usb_phy *u_phy) > > +{ > > + struct sprd_hsphy *sprd_phy = container_of(u_phy, struct > > sprd_hsphy, phy); > > + u32 reg, msk; > > + > > + if (!atomic_read(&sprd_phy->inited)) { > > + dev_dbg(sprd_phy->dev, "%s is already shut down\n", > > __func__); > > + return; > > + } > > + > > + /* usb vbus */ > > + msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG; > > + regmap_update_bits(sprd_phy->hsphy_glb, > > REG_AON_APB_OTG_PHY_TEST, msk, 0); > > + msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT; > > + regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1, msk, > > 0); > > + > > + sprd_hsphy_power_down(sprd_phy); > > + regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW, > > + msk, reg); > > + > > + /* usb cgm ref */ > > + msk = MASK_AON_APB_CGM_OTG_REF_EN | > > + MASK_AON_APB_CGM_DPHY_REF_EN; > > + regmap_update_bits(sprd_phy->hsphy_glb, > > REG_AON_APB_CGM_REG1, msk, 0); > > + > > + if (regulator_is_enabled(sprd_phy->vdd)) > > + regulator_disable(sprd_phy->vdd); > > + > > + atomic_set(&sprd_phy->inited, 0); > > + atomic_set(&sprd_phy->reset, 0); > > +} > > + > > +static ssize_t vdd_voltage_show(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + struct sprd_hsphy *sprd_phy = dev_get_drvdata(dev); > > + > > + if (!sprd_phy) > > + return -EINVAL; > > + > > + return sprintf(buf, "%d\n", sprd_phy->vdd_vol); > > +} > > + > > +static ssize_t vdd_voltage_store(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t size) > > +{ > > + struct sprd_hsphy *sprd_phy = dev_get_drvdata(dev); > > + u32 vol; > > + > > + if (!sprd_phy) > > + return -EINVAL; > > + > > + if (kstrtouint(buf, 16, &vol) < 0) > > + return -EINVAL; > > + > > + if (vol < 1200000 || vol > 3750000) { > > + dev_err(dev, "Invalid voltage value %d\n", vol); > > + return -EINVAL; > > + } > > + sprd_phy->vdd_vol = vol; > > + > > + return size; > > +} > > +static DEVICE_ATTR_RW(vdd_voltage); > > Why add voltage setting interface in a usb phy driver? should move to > regulator driver? > > > + > > +static struct attribute *usb_hsphy_attrs[] = { > > + &dev_attr_vdd_voltage.attr, > > + NULL > > +}; > > +ATTRIBUTE_GROUPS(usb_hsphy); > > + > > +static int sprd_hsphy_vbus_notify(struct notifier_block *nb, > > + unsigned long event, void *data) > > +{ > > + struct usb_phy *u_phy = container_of(nb, struct usb_phy, > > vbus_nb); > > + struct sprd_hsphy *sprd_phy = container_of(u_phy, struct > > sprd_hsphy, phy); > > + u32 reg, msk; > > + > > + if (sprd_phy->is_host || u_phy->last_event == USB_EVENT_ID) > > { > > + dev_info(sprd_phy->dev, "USB PHY is host mode\n"); > > + return 0; > > + } > > + > > + if (event) { > > + /* usb vbus valid */ > > + reg = msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG; > > + regmap_update_bits(sprd_phy->hsphy_glb, > > + REG_AON_APB_OTG_PHY_TEST, msk, reg); > > + > > + reg = msk = > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT; > > + regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL > > 1, msk, reg); > > + usb_phy_set_charger_state(u_phy, > > USB_CHARGER_PRESENT); > > + } else { > > + /* usb vbus invalid */ > > + msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG; > > + regmap_update_bits(sprd_phy->hsphy_glb, > > REG_AON_APB_OTG_PHY_TEST, > > + msk, 0); > > + msk = > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT; > > + regmap_update_bits(sprd_phy->ana_g2, > > + REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL > > 1, msk, 0); > > + usb_phy_set_charger_state(u_phy, > > USB_CHARGER_ABSENT); > > + } > > + > > + return 0; > > +} > > + > > +static enum usb_charger_type sprd_hsphy_charger_detect(struct > > usb_phy *u_phy) > > +{ > > + struct sprd_hsphy *sprd_phy = container_of(u_phy, struct > > sprd_hsphy, phy); > > + > > + return sc27xx_charger_detect(sprd_phy->pmic); > > +} > > + > > +static int sprd_hsphy_probe(struct platform_device *pdev) > > +{ > > + struct device_node *regmap_np; > > + struct platform_device *regmap_pdev; > > + struct sprd_hsphy *sprd_phy; > > + struct device *dev = &pdev->dev; > > + int ret; > > + struct usb_otg *otg; > > + > > + sprd_phy = devm_kzalloc(dev, sizeof(*sprd_phy), > > GFP_KERNEL); > > + if (!sprd_phy) > > + return -ENOMEM; > > + > > + regmap_np = of_find_compatible_node(NULL, NULL, > > "sprd,sc27xx-syscon"); > > + if (!regmap_np) { > > + dev_err(dev, "unable to get syscon node\n"); > > + return -ENODEV; > > + } > > + > > + regmap_pdev = of_find_device_by_node(regmap_np); > > + if (!regmap_pdev) { > > + dev_err(dev, "unable to get syscon platform > > device\n"); > > + ret = -ENODEV; > > + goto device_node_err; > > + } > > + > > + sprd_phy->pmic = dev_get_regmap(regmap_pdev->dev.parent, > > NULL); > > + if (!sprd_phy->pmic) { > > + dev_err(dev, "unable to get pmic regmap device\n"); > > + ret = -ENODEV; > > + goto platform_device_err; > > + } > > + > > + ret = of_property_read_u32(dev->of_node, "sprd,vdd- > > voltage", > > + &sprd_phy->vdd_vol); > > Where is the DT binding secription? > > > + if (ret < 0) { > > + dev_err(dev, "unable to read ssphy vdd voltage\n"); > > + goto platform_device_err; > > + } > > + > > + sprd_phy->vdd = devm_regulator_get(dev, "vdd"); > > ditto. > Please add the DT binding firstly. next patchset I wiil add the bindings file. > > > + if (IS_ERR(sprd_phy->vdd)) { > > + dev_err(dev, "unable to get ssphy vdd supply\n"); > > + ret = PTR_ERR(sprd_phy->vdd); > > + goto platform_device_err; > > + } > > + > > + ret = regulator_set_voltage(sprd_phy->vdd, sprd_phy- > > >vdd_vol, sprd_phy->vdd_vol); > > + if (ret < 0) { > > + dev_err(dev, "fail to set ssphy vdd voltage at > > %dmV\n", sprd_phy->vdd_vol); > > + goto platform_device_err; > > + } > > + > > + otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL); > > + if (!otg) { > > + ret = -ENOMEM; > > + goto platform_device_err; > > + } > > + > > + sprd_phy->ana_g2 = syscon_regmap_lookup_by_phandle(dev- > > >of_node, > > + "sprd,syscon-anag2"); > > + if (IS_ERR(sprd_phy->ana_g2)) { > > + dev_err(&pdev->dev, "ap USB anag2 syscon > > failed!\n"); > > + ret = PTR_ERR(sprd_phy->ana_g2); > > + goto platform_device_err; > > + } > > + > > + sprd_phy->hsphy_glb = syscon_regmap_lookup_by_phandle(dev- > > >of_node, > > + "sprd,syscon-enable"); > > + if (IS_ERR(sprd_phy->hsphy_glb)) { > > + dev_err(&pdev->dev, "ap USB aon apb syscon > > failed!\n"); > > + ret = PTR_ERR(sprd_phy->hsphy_glb); > > + goto platform_device_err; > > + } > > + > > + sprd_hsphy_enable(sprd_phy); > > + > > + sprd_hsphy_power_down(sprd_phy); > > Why doing this? please add comments to explain these strange hardware > operation. the two actions seems redundant, it should in sprd_hsphy_init. I need to test on the Soc. if not effect, I wiil remove it. > > > + > > + sprd_phy->phy.dev = dev; > > + sprd_phy->phy.label = "sprd-hsphy"; > > + sprd_phy->phy.otg = otg; > > + sprd_phy->phy.init = sprd_hsphy_init; > > + sprd_phy->phy.shutdown = sprd_hsphy_shutdown; > > + sprd_phy->phy.set_vbus = sprd_hostphy_set; > > + sprd_phy->phy.type = USB_PHY_TYPE_USB2; > > + sprd_phy->phy.vbus_nb.notifier_call = > > sprd_hsphy_vbus_notify; > > + sprd_phy->phy.charger_detect = sprd_hsphy_charger_detect; > > + otg->usb_phy = &sprd_phy->phy; > > + > > + platform_set_drvdata(pdev, sprd_phy); > > + > > + ret = usb_add_phy_dev(&sprd_phy->phy); > > + if (ret) { > > + dev_err(dev, "fail to add phy\n"); > > + goto platform_device_err; > > + } > > + > > + ret = sysfs_create_groups(&dev->kobj, usb_hsphy_groups); > > + if (ret) > > + dev_warn(dev, "failed to create usb hsphy > > attributes\n"); > > + > > + if (extcon_get_state(sprd_phy->phy.edev, EXTCON_USB) > 0) > > + usb_phy_set_charger_state(&sprd_phy->phy, > > USB_CHARGER_PRESENT); > > + > > + dev_info(dev, "sprd usb phy probe ok !\n"); > > Please remove these useless printing. > > > + > > +platform_device_err: > > +device_node_err: > > 2 same level label? > > > + of_node_put(regmap_np); > > + > > + return ret; > > +} > > + > > +static int sprd_hsphy_remove(struct platform_device *pdev) > > +{ > > + struct sprd_hsphy *sprd_phy = platform_get_drvdata(pdev); > > + > > + sysfs_remove_groups(&pdev->dev.kobj, usb_hsphy_groups); > > + usb_remove_phy(&sprd_phy->phy); > > + if (regulator_is_enabled(sprd_phy->vdd)) > > + regulator_disable(sprd_phy->vdd); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id sprd_hsphy_match[] = { > > + { .compatible = "sprd,ums512-phy"}, > > + {}, > > +}; > > + > > +MODULE_DEVICE_TABLE(of, sprd_hsphy_match); > > + > > +static struct platform_driver sprd_hsphy_driver = { > > + .probe = sprd_hsphy_probe, > > + .remove = sprd_hsphy_remove, > > + .driver = { > > + .name = "sprd-hsphy", > > + .of_match_table = sprd_hsphy_match, > > + }, > > +}; > > + > > +static int __init sprd_hsphy_driver_init(void) > > +{ > > + return platform_driver_register(&sprd_hsphy_driver); > > +} > > + > > +static void __exit sprd_hsphy_driver_exit(void) > > +{ > > + platform_driver_unregister(&sprd_hsphy_driver); > > +} > > + > > +late_initcall(sprd_hsphy_driver_init); > > +module_exit(sprd_hsphy_driver_exit); > > + > > +MODULE_DESCRIPTION("UNISOC USB PHY driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/usb/phy/phy-sprd-ums512.h > > b/drivers/usb/phy/phy-sprd-ums512.h > > new file mode 100644 > > index 000000000000..903da0573eae > > --- /dev/null > > +++ b/drivers/usb/phy/phy-sprd-ums512.h > > @@ -0,0 +1,39 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ OR MIT */ > > +/* > > + * Spreadtrum UMS512 SOC USB registers file > > + * > > + * Copyright C 2022, Spreadtrum Communications Inc. > > + */ > > + > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DATABUS16_8 > > 0x10000000 > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN 0x8 > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLDOWN 0x10 > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW_EN 0x1 > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L 0x8 > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S 0x10 > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED 0xff > > ff > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TFREGRES 0x1f > > 80000 > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TUNEHSAMP 0x60 > > 00000 > > +#define > > MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT 0x10 > > 000 > > +#define > > MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN 0x2 > > +#define > > MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DPPULLDOWN 0x4 > > +#define > > MASK_AON_APB_ANA_EB 0x10 > > 00 > > +#define > > MASK_AON_APB_CGM_DPHY_REF_EN 0x40 > > 0 > > +#define > > MASK_AON_APB_CGM_OTG_REF_EN 0x10 > > 00 > > +#define > > MASK_AON_APB_OTG_PHY_SOFT_RST 0x20 > > 0 > > +#define > > MASK_AON_APB_OTG_UTMI_EB 0x10 > > 0 > > +#define > > MASK_AON_APB_OTG_UTMI_SOFT_RST 0x10 > > 0 > > +#define > > MASK_AON_APB_OTG_VBUS_VALID_PHYREG 0x10 > > 00000 > > +#define > > MASK_AON_APB_USB2_PHY_IDDIG 0x8 > > +#define > > MASK_AON_APB_UTMI_WIDTH_SEL 0x40 > > 000000 > > +#define > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL 0x00 > > 5c > > +#define > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW 0x00 > > 70 > > +#define > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING 0x00 > > 64 > > +#define > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1 0x00 > > 58 > > +#define > > REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL2 0x00 > > 60 > > +#define > > REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0 0x00 > > 74 > > +#define > > REG_AON_APB_APB_EB1 0x00 > > 04 > > +#define > > REG_AON_APB_APB_RST1 0x00 > > 10 > > +#define > > REG_AON_APB_CGM_REG1 0x01 > > 38 > > +#define > > REG_AON_APB_OTG_PHY_CTRL 0x02 > > 08 > > +#define > > REG_AON_APB_OTG_PHY_TEST 0x02 > > 04 > > Move them to the driver file and please rename the ugly macro names, > too > long :( > > And why not move the usb phy driver to be a generic phy driver? I > mean > move it to the drivers/phy. Do you mean all the usb-phy need move to driver/phy? or just this driver?
Powered by blists - more mailing lists