RE: [PATCH v5 19/19] dt-bindings: usb: intel,keembay-dwc3: Validate DWC3 sub-node
Hi Serge. > -Original Message- > From: Serge Semin > Sent: Saturday, December 5, 2020 11:24 PM > To: Nyman, Mathias ; Felipe Balbi > ; Krzysztof Kozlowski ; Greg Kroah- > Hartman ; Rob Herring > ; Chunfeng Yun ; > Wan Mohamad, Wan Ahmad Zainie > > Cc: Serge Semin ; Serge Semin > ; Alexey Malahov > ; Pavel Parkhomenko > ; Andy Gross > ; Bjorn Andersson ; > Manu Gautam ; Roger Quadros > ; Lad Prabhakar lad...@bp.renesas.com>; Yoshihiro Shimoda > ; narmstrong > ; Kevin Hilman ; > Martin Blumenstingl ; linux-arm- > ker...@lists.infradead.org; linux-snps-...@lists.infradead.org; linux- > m...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; linux- > u...@vger.kernel.org; devicet...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: [PATCH v5 19/19] dt-bindings: usb: intel,keembay-dwc3: Validate > DWC3 sub-node > > Intel Keem Bay DWC3 compatible DT nodes are supposed to have a DWC > USB3 compatible sub-node to describe a fully functioning USB interface. Let's > use the available DWC USB3 DT schema to validate the Qualcomm DWC3 sub- > nodes. > > Note since the generic DWC USB3 DT node is supposed to be named as > generic USB HCD ("^usb(@.*)?") one we have to accordingly fix the sub- > nodes name regexp and fix the DT node example. > > Signed-off-by: Serge Semin LGTM. With minor change to fix the typo above, Qualcomm to Intel Keem Bay, Acked-by: Wan Ahmad Zainie > > --- > > Changelog v5: > - This is a new patch created for the new Intel Keem Bay bindings file, > which has been added just recently. > --- > .../devicetree/bindings/usb/intel,keembay-dwc3.yaml | 9 +++-- > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/intel,keembay- > dwc3.yaml b/Documentation/devicetree/bindings/usb/intel,keembay- > dwc3.yaml > index dd32c10ce6c7..43b91ab62004 100644 > --- a/Documentation/devicetree/bindings/usb/intel,keembay-dwc3.yaml > +++ b/Documentation/devicetree/bindings/usb/intel,keembay-dwc3.yaml > @@ -34,11 +34,8 @@ properties: > # Required child node: > > patternProperties: > - "^dwc3@[0-9a-f]+$": > -type: object > -description: > - A child node must exist to represent the core DWC3 IP block. > - The content of the node is defined in dwc3.txt. > + "^usb@[0-9a-f]+$": > +$ref: snps,dwc3.yaml# > > required: >- compatible > @@ -68,7 +65,7 @@ examples: >#address-cells = <1>; >#size-cells = <1>; > > - dwc3@3400 { > + usb@3400 { > compatible = "snps,dwc3"; > reg = <0x3400 0x1>; > interrupts = ; > -- > 2.29.2 Best regards, Zainie
RE: [PATCH v4 0/2] phy: intel: Add Keem Bay USB PHY support
Thanks Vinod. > -Original Message- > From: Vinod Koul > Sent: Monday, November 30, 2020 6:02 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; robh...@kernel.org; linux-kernel@vger.kernel.org; > devicet...@vger.kernel.org; andriy.shevche...@linux.intel.com; > mgr...@linux.intel.com; Raja Subramanian, Lakshmi Bai > > Subject: Re: [PATCH v4 0/2] phy: intel: Add Keem Bay USB PHY support > > On 16-11-20, 20:08, Wan Ahmad Zainie wrote: > > 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. > > Applied all, thanks > > -- > ~Vinod
RE: [PATCH v3 2/2] phy: intel: Add Keem Bay USB PHY support
Hi Andy. > -Original Message- > From: Andy Shevchenko > Sent: Thursday, November 12, 2020 10:29 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; vk...@kernel.org; robh...@kernel.org; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org; > mgr...@linux.intel.com; Raja Subramanian, Lakshmi Bai > > Subject: Re: [PATCH v3 2/2] phy: intel: Add Keem Bay USB PHY support > > On Thu, Nov 12, 2020 at 05:58:21PM +0800, Wan Ahmad Zainie wrote: > > Add support for USB PHY on Intel Keem Bay SoC. > > Any elaboration here? What is this PHY (USB2 or USB3 or?.. etc)? Yes, I can elaborate this. > > ... > > > +config PHY_INTEL_KEEMBAY_USB > > + tristate "Intel Keem Bay USB PHY driver" > > > + depends on ARCH_KEEMBAY || (ARM64 && COMPILE_TEST) > > It seems other drivers that are not using ARM specific calls moved to > > depends on ARCH_KEEMBAY || COMPILE_TEST I will fix in v4. > > > + depends on HAS_IOMEM > > + select GENERIC_PHY > > + select REGMAP_MMIO > > ... > > > +#define USS_CPR_MASK 0x7f > > GENMASK() ? I will fix in v4. > > ... > > > +static const struct regmap_config keembay_regmap_config = { > > + .reg_bits = 32, > > + .val_bits = 32, > > + .reg_stride = 4, > > .max_register? It is optional. But yes, I can add, .max_register = USS_USB_TIEOFFS_CONSTANTS_REG1 > > > +}; > > -- > With Best Regards, > Andy Shevchenko > Best regards, Zainie
RE: [PATCH v2 2/2] phy: intel: Add Keem Bay USB PHY support
Hi Andy. Thanks for the review and sorry for the late reply. > -Original Message- > From: Andy Shevchenko > Sent: Monday, November 9, 2020 7:41 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; vk...@kernel.org; robh...@kernel.org; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org; > mgr...@linux.intel.com; Raja Subramanian, Lakshmi Bai > > 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 > > +#include > > +#include > > +#include > > +#include > > > +#include > > 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 > > +#include > > +#include > > ... > > > + 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
RE: [PATCH v7 2/3] dt-bindings: phy: intel: Add Keem Bay eMMC PHY bindings
Resend the reply. > -Original Message- > From: Vinod Koul > Sent: Tuesday, September 1, 2020 1:52 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; robh...@kernel.org; Shevchenko, Andriy > ; eswara.k...@linux.intel.com; > vadivel.muruganx.ramuthe...@linux.intel.com; Raja Subramanian, Lakshmi > Bai ; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org > Subject: Re: [PATCH v7 2/3] dt-bindings: phy: intel: Add Keem Bay eMMC > PHY bindings > > On 01-09-20, 04:58, Wan Mohamad, Wan Ahmad Zainie wrote: > > > > > @@ -0,0 +1,44 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > > > +--- > > > > +$id: "http://devicetree.org/schemas/phy/intel,keembay-emmc- > > > phy.yaml#" > > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; > > > > + > > > > +title: Intel Keem Bay eMMC PHY bindings > > > > > > This seems same as > > > Documentation/devicetree/bindings/phy/intel,lgm-emmc-phy.yaml, > why > > > not add a new compatible in lgm binding, or did I miss a difference? > > > > AFAIK, LGM make use of syscon node, whilst KMB does not. > > And LGM and KMB belongs to different SoC family. So, I prefer them to > > be in separate file. > > > > Having said that, with few changes in wordings in title and > > description, I think we can make it generic and can be used across few > products. > > The bindings seems quite similar. We can have two drivers loaded using two > compatible but binding description can be made same Noted. I can make the change i.e. add Keem Bay compatible string in lgm binding document and drop Keem Bay binding document. Rob and Vadivel, is there any objection? If not, I will proceed with v9 in the next one or two days. > > -- > ~Vinod
RE: [PATCH v8 1/3] phy: intel: Rename phy-intel to phy-intel-lgm
> -Original Message- > From: Andy Shevchenko > Sent: Tuesday, September 1, 2020 4:11 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; vk...@kernel.org; robh...@kernel.org; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org; > vadivel.muruganx.ramuthe...@linux.intel.com; > eswara.k...@linux.intel.com; Raja Subramanian, Lakshmi Bai > > Subject: Re: [PATCH v8 1/3] phy: intel: Rename phy-intel to phy-intel-lgm > > On Tue, Sep 01, 2020 at 12:41:59PM +0800, Wan Ahmad Zainie wrote: > > Rename phy-intel-{combo,emmc}.c to phy-intel-lgm-{combo,emmc}.c to > > make drivers/phy/intel directory more generic for future use. > > > > Signed-off-by: Wan Ahmad Zainie > > > > > Reviewed-by: Ramuthevar Vadivel Murugan > > > > This shall be one line. I will fix this in v9. > > > --- > > drivers/phy/intel/Kconfig | 10 +- > > drivers/phy/intel/Makefile | 4 ++-- > > .../intel/{phy-intel-combo.c => phy-intel-lgm-combo.c} | 0 > > .../intel/{phy-intel-emmc.c => phy-intel-lgm-emmc.c} | 0 > > 4 files changed, 7 insertions(+), 7 deletions(-) rename > > drivers/phy/intel/{phy-intel-combo.c => phy-intel-lgm-combo.c} (100%) > > rename drivers/phy/intel/{phy-intel-emmc.c => phy-intel-lgm-emmc.c} > > (100%) > > > > diff --git a/drivers/phy/intel/Kconfig b/drivers/phy/intel/Kconfig > > index 7b47682a4e0e..db8586c3eed8 100644 > > --- a/drivers/phy/intel/Kconfig > > +++ b/drivers/phy/intel/Kconfig > > @@ -1,9 +1,9 @@ > > # SPDX-License-Identifier: GPL-2.0 > > # > > -# Phy drivers for Intel Lightning Mountain(LGM) platform > > +# Phy drivers for Intel platforms > > # > > -config PHY_INTEL_COMBO > > - bool "Intel ComboPHY driver" > > +config PHY_INTEL_LGM_COMBO > > + bool "Intel Lightning Mountain ComboPHY driver" > > depends on X86 || COMPILE_TEST > > depends on OF && HAS_IOMEM > > select MFD_SYSCON > > @@ -16,8 +16,8 @@ config PHY_INTEL_COMBO > > chipsets which provides PHYs for various controllers, EMAC, > > SATA and PCIe. > > > > -config PHY_INTEL_EMMC > > - tristate "Intel EMMC PHY driver" > > +config PHY_INTEL_LGM_EMMC > > + tristate "Intel Lightning Mountain EMMC PHY driver" > > depends on X86 || COMPILE_TEST > > select GENERIC_PHY > > help > > diff --git a/drivers/phy/intel/Makefile b/drivers/phy/intel/Makefile > > index 233d530dadde..662385d0a366 100644 > > --- a/drivers/phy/intel/Makefile > > +++ b/drivers/phy/intel/Makefile > > @@ -1,3 +1,3 @@ > > # SPDX-License-Identifier: GPL-2.0 > > -obj-$(CONFIG_PHY_INTEL_COMBO) += phy-intel-combo.o > > -obj-$(CONFIG_PHY_INTEL_EMMC)+= phy-intel-emmc.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-combo.c > > b/drivers/phy/intel/phy-intel-lgm-combo.c > > similarity index 100% > > rename from drivers/phy/intel/phy-intel-combo.c > > rename to drivers/phy/intel/phy-intel-lgm-combo.c > > diff --git a/drivers/phy/intel/phy-intel-emmc.c > > b/drivers/phy/intel/phy-intel-lgm-emmc.c > > similarity index 100% > > rename from drivers/phy/intel/phy-intel-emmc.c > > rename to drivers/phy/intel/phy-intel-lgm-emmc.c > > -- > > 2.17.1 > > > > -- > With Best Regards, > Andy Shevchenko >
RE: [PATCH v7 2/3] dt-bindings: phy: intel: Add Keem Bay eMMC PHY bindings
> -Original Message- > From: Vinod Koul > Sent: Monday, August 31, 2020 5:10 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; robh...@kernel.org; Shevchenko, Andriy > ; eswara.k...@linux.intel.com; > vadivel.muruganx.ramuthe...@linux.intel.com; Raja Subramanian, Lakshmi > Bai ; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org > Subject: Re: [PATCH v7 2/3] dt-bindings: phy: intel: Add Keem Bay eMMC > PHY bindings > > On 21-08-20, 19:37, Wan Ahmad Zainie wrote: > > Binding description for Intel Keem Bay eMMC PHY. > > > > Signed-off-by: Wan Ahmad Zainie > > > > Reviewed-by: Rob Herring > > --- > > .../bindings/phy/intel,keembay-emmc-phy.yaml | 44 > > +++ > > 1 file changed, 44 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/phy/intel,keembay-emmc-phy.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/phy/intel,keembay-emmc- > phy.yaml > > b/Documentation/devicetree/bindings/phy/intel,keembay-emmc- > phy.yaml > > new file mode 100644 > > index ..4cbbd3887c13 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/intel,keembay-emmc- > phy.yam > > +++ l > > @@ -0,0 +1,44 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/phy/intel,keembay-emmc- > phy.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; > > + > > +title: Intel Keem Bay eMMC PHY bindings > > This seems same as > Documentation/devicetree/bindings/phy/intel,lgm-emmc-phy.yaml, why > not add a new compatible in lgm binding, or did I miss a difference? AFAIK, LGM make use of syscon node, whilst KMB does not. And LGM and KMB belongs to different SoC family. So, I prefer them to be in separate file. Having said that, with few changes in wordings in title and description, I think we can make it generic and can be used across few products. > > > + > > +maintainers: > > + - Wan Ahmad Zainie > > + > > +properties: > > + compatible: > > +const: intel,keembay-emmc-phy > > + > > + reg: > > +maxItems: 1 > > + > > + clocks: > > +maxItems: 1 > > + > > + clock-names: > > +items: > > + - const: emmcclk > > + > > + "#phy-cells": > > +const: 0 > > + > > +required: > > + - compatible > > + - reg > > + - "#phy-cells" > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > +phy@2029 { > > + compatible = "intel,keembay-emmc-phy"; > > + reg = <0x2029 0x54>; > > + clocks = <&emmc>; > > + clock-names = "emmcclk"; > > + #phy-cells = <0>; > > +}; > > -- > > 2.17.1 > > -- > ~Vinod
RE: [PATCH v7 3/3] phy: intel: Add Keem Bay eMMC PHY support
Hi Vinod. Thanks for the review. > -Original Message- > From: Vinod Koul > Sent: Monday, August 31, 2020 5:20 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; robh...@kernel.org; Shevchenko, Andriy > ; eswara.k...@linux.intel.com; > vadivel.muruganx.ramuthe...@linux.intel.com; Raja Subramanian, Lakshmi > Bai ; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org > Subject: Re: [PATCH v7 3/3] phy: intel: Add Keem Bay eMMC PHY support > > On 21-08-20, 19:37, Wan Ahmad Zainie wrote: > > > +/* From ACS_eMMC51_16nFFC_RO1100_Userguide_v1p0.pdf p17 */ > > +#define FREQSEL_200M_170M 0x0 > > +#define FREQSEL_170M_140M 0x1 > > +#define FREQSEL_140M_110M 0x2 > > +#define FREQSEL_110M_80M 0x3 > > +#define FREQSEL_80M_50M0x4 > > + > > +#define maskval(mask, val) (((val) << (ffs(mask) - 1)) & mask) > > Kernel has a macro do this for you, please use FIELD_PREP instead of I have updated to v8, to remove this macro and use FIELD_PREP. I also add changes based on Andy's comments. > > your own macro > -- > ~Vinod Best regards, Zainie
RE: [PATCH v6 2/2] phy: intel: Add Keem Bay eMMC PHY support
Hi Vinod. Thanks for the review. > -Original Message- > From: Vinod Koul > Sent: Monday, July 13, 2020 1:40 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; robh...@kernel.org; Shevchenko, Andriy > ; linux-kernel@vger.kernel.org; > devicet...@vger.kernel.org; MP, Sureshkumar > ; Raja Subramanian, Lakshmi Bai > > Subject: Re: [PATCH v6 2/2] phy: intel: Add Keem Bay eMMC PHY support > > On 02-07-20, 08:09, Wan Ahmad Zainie wrote: > > Add support for eMMC PHY on Intel Keem Bay SoC. > > > > Signed-off-by: Wan Ahmad Zainie > > > > --- > > drivers/phy/intel/Kconfig| 12 + > > drivers/phy/intel/Makefile | 1 + > > drivers/phy/intel/phy-keembay-emmc.c | 314 > > +++ > > 3 files changed, 327 insertions(+) > > create mode 100644 drivers/phy/intel/phy-keembay-emmc.c > > > > diff --git a/drivers/phy/intel/Kconfig b/drivers/phy/intel/Kconfig > > index 7b47682a4e0e..8ddda4fb95d2 100644 > > --- a/drivers/phy/intel/Kconfig > > +++ b/drivers/phy/intel/Kconfig > > @@ -22,3 +22,15 @@ config PHY_INTEL_EMMC > > select GENERIC_PHY > > help > > Enable this to support the Intel EMMC PHY > > + > > +config PHY_KEEMBAY_EMMC > > Pls keep this in alphabetical sort PHY_INTEL_ followed by PHY_KEEMBAY_ is alphabetically sorted. Could you please help to clarify? > > > + tristate "Intel Keem Bay EMMC PHY driver" > > + depends on ARM64 || COMPILE_TEST > > Intel and ARM64, aha, fun times! 😊 > > > + 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-emmc. > > phy-keembay-emmc.ko ? Is it a must? I saw few Kconfig files omit .ko. I can fix this in next version. > > > diff --git a/drivers/phy/intel/Makefile b/drivers/phy/intel/Makefile > > index 233d530dadde..6566334e7b77 100644 > > --- a/drivers/phy/intel/Makefile > > +++ b/drivers/phy/intel/Makefile > > @@ -1,3 +1,4 @@ > > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_PHY_INTEL_COMBO) += phy-intel-combo.o > > obj-$(CONFIG_PHY_INTEL_EMMC)+= phy-intel-emmc.o > > +obj-$(CONFIG_PHY_KEEMBAY_EMMC) += phy-keembay- > emmc.o > > here as well > > > +/* eMMC/SD/SDIO core/phy configuration registers */ > > +#define PHY_CFG_0 0x24 > > +#define SEL_DLY_TXCLK_MASKBIT(29) > > +#define SEL_DLY_TXCLK(x) (((x) << 29) & SEL_DLY_TXCLK_MASK) > > +#define OTAP_DLY_ENA_MASK BIT(27) > > +#define OTAP_DLY_ENA(x) (((x) << 27) & OTAP_DLY_ENA_MASK) > > +#define OTAP_DLY_SEL_MASK GENMASK(26, 23) > > +#define OTAP_DLY_SEL(x) (((x) << 23) & OTAP_DLY_SEL_MASK) > > why not a generic helper to do (x) << ffs(reg - 1) & reg ? > You can skip defining for each register that way! Is it something like this following? #define maskval(mask, val) (((val) << (ffs(mask) - 1)) & mask) > > -- > ~Vinod
RE: [RESEND v5 2/2] phy: intel: Add Keem Bay eMMC PHY support
> -Original Message- > From: Shevchenko, Andriy > Sent: Wednesday, June 17, 2020 10:01 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; vk...@kernel.org; robh...@kernel.org; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org; Hunter, Adrian > > Subject: Re: [RESEND v5 2/2] phy: intel: Add Keem Bay eMMC PHY support > > On Wed, Jun 17, 2020 at 07:32:52AM +0800, Wan Ahmad Zainie wrote: > > Add support for eMMC PHY on Intel Keem Bay SoC. > > ... > > > + ret = regmap_read_poll_timeout(priv->syscfg, PHY_STAT, > > + dllrdy, IS_DLLRDY(dllrdy), > > + 0, 50 * USEC_PER_MSEC); > > + if (ret) { > > + dev_err(&phy->dev, "dllrdy failed, ret=%d\n", ret); > > + return ret; > > + } > > + > > + return ret; > > +} > > This has no changes. > > Are you sure the version is correct? Ah. Sorry Andy. I misunderstood your earlier review comment. You mean return inside the last if(...) should be discarded. Kishon, should I send another version, or is it acceptable? > > -- > With Best Regards, > Andy Shevchenko >
RE: [PATCH v5 2/2] phy: intel: Add Keem Bay eMMC PHY support
> -Original Message- > From: Shevchenko, Andriy > Sent: Wednesday, June 17, 2020 12:44 AM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; vk...@kernel.org; robh...@kernel.org; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org; Hunter, Adrian > > Subject: Re: [PATCH v5 2/2] phy: intel: Add Keem Bay eMMC PHY support > > On Tue, Jun 16, 2020 at 10:38:18PM +0800, Wan Ahmad Zainie wrote: > > Add support for eMMC PHY on Intel Keem Bay SoC. > > ... > > > + ret = regmap_read_poll_timeout(priv->syscfg, PHY_STAT, > > + dllrdy, IS_DLLRDY(dllrdy), > > + 0, 50 * USEC_PER_MSEC); > > + if (ret) { > > + dev_err(&phy->dev, "dllrdy failed, ret=%d\n", ret); > > > + return ret; > > + } > > + > > + return ret; > > return ret; > > (Since it's only one minor issue, it's up to maintainers to decide if new > version is needed) This was addressed in v4, together with another 2 changes. I was careless when modifying Kconfig in my older working branch. I resend v5. > > -- > With Best Regards, > Andy Shevchenko >
RE: [PATCH v3 2/2] phy: intel: Add Keem Bay eMMC PHY support
> -Original Message- > From: Shevchenko, Andriy > Sent: Monday, June 8, 2020 6:08 PM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; vk...@kernel.org; robh...@kernel.org; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org; Hunter, Adrian > > Subject: Re: [PATCH v3 2/2] phy: intel: Add Keem Bay eMMC PHY support > > On Mon, Jun 08, 2020 at 04:15:01PM +0800, Wan Ahmad Zainie wrote: > > Add support for eMMC PHY on Intel Keem Bay SoC. > > I think I commented on something already. I don't recall any. May be on other similar patches. I will wait for few days before sending next revision, if it is okay with you. > > ... > > > + if (ret) { > > + dev_err(&phy->dev, "dllrdy failed, ret=%d\n", ret); > > > + return ret; > > + } > > + > > + return 0; > > return ret; I will change in next revision. > > ... > > > + if (IS_ERR(priv->emmcclk)) { > > + dev_err(&phy->dev, "ERROR: getting emmcclk\n"); > > > + return PTR_ERR(priv->emmcclk); > > + } > > + > > + return 0; > > return PTR_ERR_OR_ZERO(...); To clarify, I should replace the block above with, return PTR_ERR_OR_ZERO(priv->emmcclk); > > ... > > > + priv->syscfg = devm_regmap_init_mmio(dev, base, > > +&keembay_regmap_config); > > One line. I will change in next revision. > > -- > With Best Regards, > Andy Shevchenko >
RE: [PATCH 1/2] dt-bindings: phy: intel: Add documentation for Keem Bay eMMC PHY
> -Original Message- > From: Rob Herring > Sent: Tuesday, March 31, 2020 4:23 AM > To: Wan Mohamad, Wan Ahmad Zainie > > Cc: kis...@ti.com; mark.rutl...@arm.com; linux-kernel@vger.kernel.org; > devicet...@vger.kernel.org > Subject: Re: [PATCH 1/2] dt-bindings: phy: intel: Add documentation for > Keem Bay eMMC PHY > > On Mon, Mar 16, 2020 at 06:37:25PM +0800, Wan Ahmad Zainie wrote: > > Document Intel Keem Bay eMMC PHY DT bindings. > > > > Signed-off-by: Wan Ahmad Zainie > > > --- > > .../bindings/phy/intel,keembay-emmc-phy.yaml | 57 > +++ > > 1 file changed, 57 insertions(+) > > create mode 100644 > Documentation/devicetree/bindings/phy/intel,keembay-emmc-phy.yaml > > > > diff --git a/Documentation/devicetree/bindings/phy/intel,keembay- > emmc-phy.yaml b/Documentation/devicetree/bindings/phy/intel,keembay- > emmc-phy.yaml > > new file mode 100644 > > index ..af1d62fc8323 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/intel,keembay-emmc- > phy.yaml > > @@ -0,0 +1,57 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > Dual license new bindings: > > (GPL-2.0-only OR BSD-2-Clause) Will change in v2. > > > +# Copyright 2020 Intel Corporation > > +%YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/phy/intel,keembay-emmc- > phy.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; > > + > > +title: Intel Keem Bay eMMC PHY > > + > > +maintainers: > > + - Wan Ahmad Zainie > > + > > +properties: > > + compatible: > > +enum: > > + - intel,keembay-emmc-phy > > + > > + reg: > > +maxItems: 1 > > + > > + clocks: > > +maxItems: 1 > > + > > + clock-names: > > +items: > > + - const: emmcclk > > + > > + intel,syscon: > > +$ref: '/schemas/types.yaml#/definitions/phandle' > > Make this binding a child of the syscon and get rid of this. > > > +description: > > + A phandle to a syscon device used to access core/phy configuration > > + registers. > > + > > + "#phy-cells": > > +const: 0 > > + > > +required: > > + - compatible > > + - reg > > + - intel,syscon > > + - "#phy-cells" > > + > > +examples: > > + - | > > +mmc_phy_syscon: syscon@2029 { > > + compatible = "simple-mfd", "syscon"; > > + reg = <0x0 0x2029 0x0 0x54>; > > +}; > > + > > +emmc_phy: mmc_phy@2029 { > > phy@... Will change in v2. > > > + compatible = "intel,keembay-emmc-phy"; > > + reg = <0x0 0x2029 0x0 0x54>; > > Here you have overlapping register regions. Don't do that. > > Given they are the same size, why do you need the syscon at all? In v2, the driver will use regmap_mmio. With that, can remove intel,syscon. I will send out once reviewed internally. > > > + clocks = <&mmc>; > > + clock-names = "emmcclk"; > > + intel,syscon = <&mmc_phy_syscon>; > > + #phy-cells = <0>; > > +}; > > -- > > 2.17.1 > >