* [PATCH v2 0/2] phy: intel: Add Keem Bay USB PHY support @ 2020-11-09 3:16 Wan Ahmad Zainie 2020-11-09 3:16 ` [PATCH v2 1/2] dt-bindings: phy: Add Intel Keem Bay USB PHY bindings Wan Ahmad Zainie 2020-11-09 3:16 ` [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support Wan Ahmad Zainie 0 siblings, 2 replies; 6+ messages in thread From: Wan Ahmad Zainie @ 2020-11-09 3:16 UTC (permalink / raw) To: kishon, vkoul, robh+dt Cc: linux-kernel, devicetree, andriy.shevchenko, mgross, lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad Hi. Intel Keem Bay USB subsystem incorporates DesignWare USB3.1 controller, an USB3.1 (Gen1/2) PHY and an USB2.0 PHY. It is a Dual Role Device (DRD), operating as either a USB host or a USB device. The patchset is tested on Keem Bay EVM. Thank you. Best regards, Zainie Changes since v1: - Remove 2 patches that had been merged. Reduced To, Cc list. - Rebased to v5.10-rc3. - Add Rob's Reviewed-by tag in the first patch. - Use ARCH_KEEMBAY in Kconfig. - Update #include header; remove <linux/of_address.h>, and add <linux/bits.h>. - Remove unnecessary comments. Note: I resend this v2 as my previous v2 sent on Oct 28 not seen in mailing lists. Wan Ahmad Zainie (2): dt-bindings: phy: Add Intel Keem Bay USB PHY bindings phy: intel: Add Keem Bay USB PHY support .../bindings/phy/intel,phy-keembay-usb.yaml | 44 +++ drivers/phy/intel/Kconfig | 12 + drivers/phy/intel/Makefile | 1 + drivers/phy/intel/phy-intel-keembay-usb.c | 288 ++++++++++++++++++ 4 files changed, 345 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/intel,phy-keembay-usb.yaml create mode 100644 drivers/phy/intel/phy-intel-keembay-usb.c -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] dt-bindings: phy: Add Intel Keem Bay USB PHY bindings 2020-11-09 3:16 [PATCH v2 0/2] phy: intel: Add Keem Bay USB PHY support Wan Ahmad Zainie @ 2020-11-09 3:16 ` Wan Ahmad Zainie 2020-11-09 3:16 ` [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support Wan Ahmad Zainie 1 sibling, 0 replies; 6+ messages in thread From: Wan Ahmad Zainie @ 2020-11-09 3:16 UTC (permalink / raw) To: kishon, vkoul, robh+dt Cc: linux-kernel, devicetree, andriy.shevchenko, mgross, lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad Binding description for Intel Keem Bay USB PHY. Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> Reviewed-by: Rob Herring <robh@kernel.org> --- .../bindings/phy/intel,phy-keembay-usb.yaml | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/intel,phy-keembay-usb.yaml diff --git a/Documentation/devicetree/bindings/phy/intel,phy-keembay-usb.yaml b/Documentation/devicetree/bindings/phy/intel,phy-keembay-usb.yaml new file mode 100644 index 000000000000..a217bb8ac5bc --- /dev/null +++ b/Documentation/devicetree/bindings/phy/intel,phy-keembay-usb.yaml @@ -0,0 +1,44 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/intel,phy-keembay-usb.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Intel Keem Bay USB PHY bindings + +maintainers: + - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> + +properties: + compatible: + const: intel,keembay-usb-phy + + reg: + items: + - description: USB APB CPR (clock, power, reset) register + - description: USB APB slave register + + reg-names: + items: + - const: cpr-apb-base + - const: slv-apb-base + + '#phy-cells': + const: 0 + +required: + - compatible + - reg + - '#phy-cells' + +additionalProperties: false + +examples: + - | + usb-phy@20400000 { + compatible = "intel,keembay-usb-phy"; + reg = <0x20400000 0x1c>, + <0x20480000 0xd0>; + reg-names = "cpr-apb-base", "slv-apb-base"; + #phy-cells = <0>; + }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support 2020-11-09 3:16 [PATCH v2 0/2] phy: intel: Add Keem Bay USB PHY support Wan Ahmad Zainie 2020-11-09 3:16 ` [PATCH v2 1/2] dt-bindings: phy: Add Intel Keem Bay USB PHY bindings Wan Ahmad Zainie @ 2020-11-09 3:16 ` Wan Ahmad Zainie 2020-11-09 11:41 ` Andy Shevchenko 1 sibling, 1 reply; 6+ messages in thread From: Wan Ahmad Zainie @ 2020-11-09 3:16 UTC (permalink / raw) To: kishon, vkoul, robh+dt Cc: linux-kernel, devicetree, andriy.shevchenko, mgross, lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad Add support for USB PHY on Intel Keem Bay SoC. Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com> --- drivers/phy/intel/Kconfig | 12 + drivers/phy/intel/Makefile | 1 + drivers/phy/intel/phy-intel-keembay-usb.c | 288 ++++++++++++++++++++++ 3 files changed, 301 insertions(+) create mode 100644 drivers/phy/intel/phy-intel-keembay-usb.c diff --git a/drivers/phy/intel/Kconfig b/drivers/phy/intel/Kconfig index 58ec695c92ec..f8bc2527e6e4 100644 --- a/drivers/phy/intel/Kconfig +++ b/drivers/phy/intel/Kconfig @@ -14,6 +14,18 @@ config PHY_INTEL_KEEMBAY_EMMC To compile this driver as a module, choose M here: the module will be called phy-keembay-emmc.ko. +config PHY_INTEL_KEEMBAY_USB + tristate "Intel Keem Bay USB PHY driver" + depends on ARCH_KEEMBAY || (ARM64 && COMPILE_TEST) + depends on OF && HAS_IOMEM + select GENERIC_PHY + select REGMAP_MMIO + help + Choose this option if you have an Intel Keem Bay SoC. + + To compile this driver as a module, choose M here: the module + will be called phy-keembay-usb.ko. + config PHY_INTEL_LGM_COMBO bool "Intel Lightning Mountain ComboPHY driver" depends on X86 || COMPILE_TEST diff --git a/drivers/phy/intel/Makefile b/drivers/phy/intel/Makefile index a5e0af5ccd75..14550981a707 100644 --- a/drivers/phy/intel/Makefile +++ b/drivers/phy/intel/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_PHY_INTEL_KEEMBAY_EMMC) += phy-intel-keembay-emmc.o +obj-$(CONFIG_PHY_INTEL_KEEMBAY_USB) += phy-intel-keembay-usb.o obj-$(CONFIG_PHY_INTEL_LGM_COMBO) += phy-intel-lgm-combo.o obj-$(CONFIG_PHY_INTEL_LGM_EMMC) += phy-intel-lgm-emmc.o diff --git a/drivers/phy/intel/phy-intel-keembay-usb.c b/drivers/phy/intel/phy-intel-keembay-usb.c new file mode 100644 index 000000000000..4f5628d77fe1 --- /dev/null +++ b/drivers/phy/intel/phy-intel-keembay-usb.c @@ -0,0 +1,288 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Intel Keem Bay USB PHY driver + * Copyright (C) 2020 Intel Corporation + */ + +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> + +/* USS (USB Subsystem) clock control registers */ +#define USS_CPR_CLK_EN 0x00 +#define USS_CPR_CLK_SET 0x04 +#define USS_CPR_CLK_CLR 0x08 +#define USS_CPR_RST_EN 0x10 +#define USS_CPR_RST_SET 0x14 +#define USS_CPR_RST_CLR 0x18 + +/* USS clock/reset bit fields */ +#define USS_CPR_PHY_TST BIT(6) +#define USS_CPR_LOW_JIT BIT(5) +#define USS_CPR_CORE BIT(4) +#define USS_CPR_SUSPEND BIT(3) +#define USS_CPR_ALT_REF BIT(2) +#define USS_CPR_REF BIT(1) +#define USS_CPR_SYS BIT(0) +#define USS_CPR_MASK 0x7f + +/* USS APB slave registers */ +#define USS_USB_CTRL_CFG0 0x10 +#define VCC_RESET_N_MASK BIT(31) + +#define USS_USB_PHY_CFG0 0x30 +#define POR_MASK BIT(15) +#define PHY_RESET_MASK BIT(14) +#define PHY_REF_USE_PAD_MASK BIT(5) + +#define USS_USB_PHY_CFG6 0x64 +#define PHY0_SRAM_EXT_LD_DONE_MASK BIT(23) + +#define USS_USB_PARALLEL_IF_CTRL 0xa0 +#define USB_PHY_CR_PARA_SEL_MASK BIT(2) + +#define USS_USB_TSET_SIGNALS_AND_GLOB 0xac +#define USB_PHY_CR_PARA_CLK_EN_MASK BIT(7) + +#define USS_USB_STATUS_REG 0xb8 +#define PHY0_SRAM_INIT_DONE_MASK BIT(3) + +#define USS_USB_TIEOFFS_CONSTANTS_REG1 0xc0 +#define IDDQ_ENABLE_MASK BIT(10) + +struct keembay_usb_phy { + struct device *dev; + struct regmap *regmap_cpr; + struct regmap *regmap_slv; +}; + +static const struct regmap_config keembay_regmap_config = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, +}; + +static int keembay_usb_clocks_on(struct keembay_usb_phy *priv) +{ + int ret; + + ret = regmap_update_bits(priv->regmap_cpr, USS_CPR_CLK_SET, + USS_CPR_MASK, USS_CPR_MASK); + if (ret) { + dev_err(priv->dev, "error clock set: %d\n", ret); + return ret; + } + + ret = regmap_update_bits(priv->regmap_cpr, USS_CPR_RST_SET, + USS_CPR_MASK, USS_CPR_MASK); + if (ret) { + dev_err(priv->dev, "error reset set: %d\n", ret); + return ret; + } + + ret = regmap_update_bits(priv->regmap_slv, + USS_USB_TIEOFFS_CONSTANTS_REG1, + IDDQ_ENABLE_MASK, + FIELD_PREP(IDDQ_ENABLE_MASK, 0)); + if (ret) { + dev_err(priv->dev, "error iddq enable: %d\n", ret); + return ret; + } + + usleep_range(30, 50); + + ret = regmap_update_bits(priv->regmap_slv, USS_USB_PHY_CFG0, + PHY_REF_USE_PAD_MASK, + FIELD_PREP(PHY_REF_USE_PAD_MASK, 1)); + if (ret) + dev_err(priv->dev, "error ref clock select: %d\n", ret); + + return ret; +} + +static int keembay_usb_core_off(struct keembay_usb_phy *priv) +{ + int ret; + + ret = regmap_update_bits(priv->regmap_slv, USS_USB_CTRL_CFG0, + VCC_RESET_N_MASK, + FIELD_PREP(VCC_RESET_N_MASK, 0)); + if (ret) + dev_err(priv->dev, "error core reset: %d\n", ret); + + return ret; +} + +static int keembay_usb_core_on(struct keembay_usb_phy *priv) +{ + int ret; + + ret = regmap_update_bits(priv->regmap_slv, USS_USB_CTRL_CFG0, + VCC_RESET_N_MASK, + FIELD_PREP(VCC_RESET_N_MASK, 1)); + if (ret) + dev_err(priv->dev, "error core on: %d\n", ret); + + return ret; +} + +static int keembay_usb_phys_on(struct keembay_usb_phy *priv) +{ + int ret; + + ret = regmap_update_bits(priv->regmap_slv, USS_USB_PHY_CFG0, + POR_MASK | PHY_RESET_MASK, + FIELD_PREP(POR_MASK | PHY_RESET_MASK, 0)); + if (ret) + dev_err(priv->dev, "error phys on: %d\n", ret); + + return ret; +} + +static int keembay_usb_phy_init(struct phy *phy) +{ + struct keembay_usb_phy *priv = phy_get_drvdata(phy); + u32 val; + int ret; + + ret = keembay_usb_core_off(priv); + if (ret) + return ret; + + usleep_range(20, 50); + + ret = keembay_usb_phys_on(priv); + if (ret) + return ret; + + ret = regmap_update_bits(priv->regmap_slv, + USS_USB_TSET_SIGNALS_AND_GLOB, + USB_PHY_CR_PARA_CLK_EN_MASK, + FIELD_PREP(USB_PHY_CR_PARA_CLK_EN_MASK, 0)); + if (ret) { + dev_err(priv->dev, "error cr clock disable: %d\n", ret); + return ret; + } + + usleep_range(2, 10); + + ret = regmap_update_bits(priv->regmap_slv, + USS_USB_PARALLEL_IF_CTRL, + USB_PHY_CR_PARA_SEL_MASK, + FIELD_PREP(USB_PHY_CR_PARA_SEL_MASK, 1)); + if (ret) { + dev_err(priv->dev, "error cr select: %d\n", ret); + return ret; + } + + ret = regmap_update_bits(priv->regmap_slv, + USS_USB_TSET_SIGNALS_AND_GLOB, + USB_PHY_CR_PARA_CLK_EN_MASK, + FIELD_PREP(USB_PHY_CR_PARA_CLK_EN_MASK, 1)); + if (ret) { + dev_err(priv->dev, "error cr clock enable: %d\n", ret); + return ret; + } + + ret = regmap_read_poll_timeout(priv->regmap_slv, USS_USB_STATUS_REG, + val, val & PHY0_SRAM_INIT_DONE_MASK, + USEC_PER_MSEC, 10 * USEC_PER_MSEC); + if (ret) { + dev_err(priv->dev, "SRAM init not done: %d\n", ret); + return ret; + } + + ret = regmap_update_bits(priv->regmap_slv, USS_USB_PHY_CFG6, + PHY0_SRAM_EXT_LD_DONE_MASK, + FIELD_PREP(PHY0_SRAM_EXT_LD_DONE_MASK, 1)); + if (ret) { + dev_err(priv->dev, "error SRAM init done set: %d\n", ret); + return ret; + } + + usleep_range(20, 50); + + return keembay_usb_core_on(priv); +} + +static const struct phy_ops ops = { + .init = keembay_usb_phy_init, + .owner = THIS_MODULE, +}; + +static int keembay_usb_phy_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + struct keembay_usb_phy *priv; + struct phy *generic_phy; + struct phy_provider *phy_provider; + void __iomem *base; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + base = devm_platform_ioremap_resource_byname(pdev, "cpr-apb-base"); + if (IS_ERR(base)) + return PTR_ERR(base); + + priv->regmap_cpr = devm_regmap_init_mmio(dev, base, + &keembay_regmap_config); + if (IS_ERR(priv->regmap_cpr)) + return PTR_ERR(priv->regmap_cpr); + + base = devm_platform_ioremap_resource_byname(pdev, "slv-apb-base"); + if (IS_ERR(base)) + return PTR_ERR(base); + + priv->regmap_slv = devm_regmap_init_mmio(dev, base, + &keembay_regmap_config); + if (IS_ERR(priv->regmap_slv)) + return PTR_ERR(priv->regmap_slv); + + generic_phy = devm_phy_create(dev, np, &ops); + if (IS_ERR(generic_phy)) + return dev_err_probe(dev, PTR_ERR(generic_phy), + "failed to create PHY\n"); + + phy_set_drvdata(generic_phy, priv); + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); + if (IS_ERR(phy_provider)) + return dev_err_probe(dev, PTR_ERR(phy_provider), + "failed to register phy provider\n"); + + /* Setup USB subsystem clocks */ + ret = keembay_usb_clocks_on(priv); + if (ret) + return ret; + + /* and turn on the DWC3 core, prior to DWC3 driver init. */ + return keembay_usb_core_on(priv); +} + +static const struct of_device_id keembay_usb_phy_dt_ids[] = { + { .compatible = "intel,keembay-usb-phy" }, + {} +}; +MODULE_DEVICE_TABLE(of, keembay_usb_phy_dt_ids); + +static struct platform_driver keembay_usb_phy_driver = { + .probe = keembay_usb_phy_probe, + .driver = { + .name = "keembay-usb-phy", + .of_match_table = keembay_usb_phy_dt_ids, + }, +}; +module_platform_driver(keembay_usb_phy_driver); + +MODULE_AUTHOR("Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>"); +MODULE_DESCRIPTION("Intel Keem Bay USB PHY driver"); +MODULE_LICENSE("GPL v2"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support 2020-11-09 3:16 ` [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support Wan Ahmad Zainie @ 2020-11-09 11:41 ` Andy Shevchenko 2020-11-11 9:28 ` Wan Mohamad, Wan Ahmad Zainie 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2020-11-09 11:41 UTC (permalink / raw) To: Wan Ahmad Zainie Cc: kishon, vkoul, robh+dt, linux-kernel, devicetree, mgross, lakshmi.bai.raja.subramanian On Mon, Nov 09, 2020 at 11:16:54AM +0800, Wan Ahmad Zainie wrote: > Add support for USB PHY on Intel Keem Bay SoC. ... > +config PHY_INTEL_KEEMBAY_USB > + tristate "Intel Keem Bay USB PHY driver" > + depends on ARCH_KEEMBAY || (ARM64 && COMPILE_TEST) > + depends on OF && HAS_IOMEM Do you really need dependency to OF (yes, I see that it will be not functional, but still can be compile tested)? > + select GENERIC_PHY > + select REGMAP_MMIO > + help > + Choose this option if you have an Intel Keem Bay SoC. > + > + To compile this driver as a module, choose M here: the module > + will be called phy-keembay-usb.ko. ... > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/module.h> > +#include <linux/of.h> No evidence of anything being used in this code. mod_devicetable.h is missed, though. > +#include <linux/phy/phy.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> ... > + usleep_range(30, 50); Why 30-50? ... > + usleep_range(20, 50); Why these numbers? ... > + usleep_range(2, 10); Ditto. ... > + usleep_range(20, 50); Ditto. ... > + struct device_node *np = dev->of_node; It's being used only once and it doesn't bring any benefit. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support 2020-11-09 11:41 ` Andy Shevchenko @ 2020-11-11 9:28 ` Wan Mohamad, Wan Ahmad Zainie 2020-11-11 9:49 ` Andy Shevchenko 0 siblings, 1 reply; 6+ messages in thread From: Wan Mohamad, Wan Ahmad Zainie @ 2020-11-11 9:28 UTC (permalink / raw) To: Andy Shevchenko Cc: kishon, vkoul, robh+dt, linux-kernel, devicetree, mgross, Raja Subramanian, Lakshmi Bai Hi Andy. Thanks for the review and sorry for the late reply. > -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Sent: Monday, November 9, 2020 7:41 PM > To: Wan Mohamad, Wan Ahmad Zainie > <wan.ahmad.zainie.wan.mohamad@intel.com> > Cc: kishon@ti.com; vkoul@kernel.org; robh+dt@kernel.org; linux- > kernel@vger.kernel.org; devicetree@vger.kernel.org; > mgross@linux.intel.com; Raja Subramanian, Lakshmi Bai > <lakshmi.bai.raja.subramanian@intel.com> > Subject: Re: [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support > > On Mon, Nov 09, 2020 at 11:16:54AM +0800, Wan Ahmad Zainie wrote: > > Add support for USB PHY on Intel Keem Bay SoC. > > ... > > > +config PHY_INTEL_KEEMBAY_USB > > + tristate "Intel Keem Bay USB PHY driver" > > + depends on ARCH_KEEMBAY || (ARM64 && COMPILE_TEST) > > > + depends on OF && HAS_IOMEM > > Do you really need dependency to OF (yes, I see that it will be not functional, > but still can be compile tested)? No, I can drop OF. I will remove in v3. > > > + select GENERIC_PHY > > + select REGMAP_MMIO > > + help > > + Choose this option if you have an Intel Keem Bay SoC. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called phy-keembay-usb.ko. > > ... > > > +#include <linux/bitfield.h> > > +#include <linux/bits.h> > > +#include <linux/clk.h> > > +#include <linux/delay.h> > > +#include <linux/module.h> > > > +#include <linux/of.h> > > No evidence of anything being used in this code. > mod_devicetable.h is missed, though. I will fix in v3. Remove of.h and add mod_devicetable.h. > > > +#include <linux/phy/phy.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > ... > > > + usleep_range(30, 50); > > Why 30-50? I take this value from boot firmware. There is a delay of 30us after clearing IDDQ_enable bit. I believe the purpose is to ensure all analog blocks are powered up. > > ... > > > + usleep_range(20, 50); > > Why these numbers? In Keem Bay data book, under USB initialization section, there is step that there must be a minimum 20us wait after clock enable, before bringing PHYs out of reset. 50 is the value that I picked randomly. Is usleep_range(20, 20) Better? > > ... > > > + usleep_range(2, 10); > > Ditto. Under the same section above, there is a step for 2us wait. I believe it is for register write to go through. > > ... > > > + usleep_range(20, 50); > > Ditto. Under the same section above, there is a step to wait 20us after setting SRAM load bit, before release the controller reset. I will add comment for those 4 delay above. Before I proceed with v3, I would like to know if I should use udelay(), instead of usleep_range()? I refer to https://www.kernel.org/doc/Documentation/timers/timers-howto.txt. > > > ... > > > + struct device_node *np = dev->of_node; > > It's being used only once and it doesn't bring any benefit. I will fix in v3. Use dev->of_node directly. > > -- > With Best Regards, > Andy Shevchenko > Best regards, Zainie ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support 2020-11-11 9:28 ` Wan Mohamad, Wan Ahmad Zainie @ 2020-11-11 9:49 ` Andy Shevchenko 0 siblings, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2020-11-11 9:49 UTC (permalink / raw) To: Wan Mohamad, Wan Ahmad Zainie Cc: kishon, vkoul, robh+dt, linux-kernel, devicetree, mgross, Raja Subramanian, Lakshmi Bai On Wed, Nov 11, 2020 at 09:28:34AM +0000, Wan Mohamad, Wan Ahmad Zainie wrote: > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Sent: Monday, November 9, 2020 7:41 PM > > To: Wan Mohamad, Wan Ahmad Zainie > > On Mon, Nov 09, 2020 at 11:16:54AM +0800, Wan Ahmad Zainie wrote: ... > > > + usleep_range(30, 50); > > > > Why 30-50? > > I take this value from boot firmware. > There is a delay of 30us after clearing IDDQ_enable bit. > I believe the purpose is to ensure all analog blocks are powered up. Then put it into comment. ... > > > + usleep_range(20, 50); > > > > Why these numbers? > > In Keem Bay data book, under USB initialization section, > there is step that there must be a minimum 20us wait > after clock enable, before bringing PHYs out of reset. > > 50 is the value that I picked randomly. Is usleep_range(20, 20) > Better? No, the better as I told you already few times is to comment "why?" these numbers. Above can be like: "According to datasheet this step requires 20us wait..." ... > > > + usleep_range(2, 10); > > > > Ditto. > > Under the same section above, there is a step for 2us wait. > I believe it is for register write to go through. Ditto. > > > > ... > > > > > + usleep_range(20, 50); > > > > Ditto. > > Under the same section above, there is a step to wait 20us > after setting SRAM load bit, before release the controller > reset. > > I will add comment for those 4 delay above. Yes, please. ... > Before I proceed with v3, I would like to know if I should > use udelay(), instead of usleep_range()? > I refer to https://www.kernel.org/doc/Documentation/timers/timers-howto.txt. You should know your code better than me. That howto is clear about when of which API calls can be used. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-11 9:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-09 3:16 [PATCH v2 0/2] phy: intel: Add Keem Bay USB PHY support Wan Ahmad Zainie 2020-11-09 3:16 ` [PATCH v2 1/2] dt-bindings: phy: Add Intel Keem Bay USB PHY bindings Wan Ahmad Zainie 2020-11-09 3:16 ` [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support Wan Ahmad Zainie 2020-11-09 11:41 ` Andy Shevchenko 2020-11-11 9:28 ` Wan Mohamad, Wan Ahmad Zainie 2020-11-11 9:49 ` Andy Shevchenko
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).