Devicetree
 help / color / mirror / Atom feed
* Re: [RESEND PATCH v8 0/3] Add Intel ComboPhy driver
From: Dilip Kota @ 2020-05-19  3:56 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, linux-kernel, vkoul, devicetree
  Cc: robh, andriy.shevchenko, cheol.yong.kim, chuanhua.lei, qi-ming.wu,
	yixin.zhu
In-Reply-To: <1d58e6e6-4860-dbde-1b9e-e0804180cddb@ti.com>


On 5/18/2020 9:49 PM, Kishon Vijay Abraham I wrote:
> Dilip,
>
> On 5/15/2020 1:43 PM, Dilip Kota wrote:
>> This patch series adds Intel ComboPhy driver, respective yaml schemas
>>
>> Changes on v8:
>>    As per PHY Maintainer's request add description in comments for doing
>>    register access through register map framework.
>>
>> Changes on v7:
>>    As per System control driver maintainer's inputs remove
>>      fwnode_to_regmap() definition and use device_node_get_regmap()
> Can you fix this warning and resend the patch?
> drivers/phy/intel/phy-intel-combo.c:229:6: warning: ‘cb_mode’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
>    ret = regmap_write(cbphy->hsiocfg, REG_COMBO_MODE(cbphy->bid), cb_mode);
>    ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/phy/intel/phy-intel-combo.c:204:24: note: ‘cb_mode’ was declared here
>    enum intel_combo_mode cb_mode;
>                          ^~~~~~~
I noticed this warning while preparing the patch.
It sounds like false warning because:
1.) "cb_mode" is initialized in the switch case based on the "mode = 
cbphy->phy_mode;"
2.) cbphy->phy_mode is initialized during the probe in 
"intel_cbphy_fwnode_parse()" with one of the 3 values.
PHY_PCIE_MODE, PHY_SATA_MODE, PHY_XPCS_MODE.
3.) There is no chance of "cbphy->phy_mode" having different value.
4.) And "cb_mode" will be initialized according to the "mode = 
cbphy->phy_mode;"
5.) Hence, there is no chance of "cb_mode" getting accessed uninitialized.

Regards,
Dilip
> Thanks
> Kishon
>>      
>> Changes on v6:
>>    Rebase patches on the latest maintainer's branch
>>    https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git/?h=phy-for-5.7
>> Dilip Kota (3):
>>    dt-bindings: phy: Add PHY_TYPE_XPCS definition
>>    dt-bindings: phy: Add YAML schemas for Intel ComboPhy
>>    phy: intel: Add driver support for ComboPhy
>>
>>   .../devicetree/bindings/phy/intel,combo-phy.yaml   | 101 ++++
>>   drivers/phy/intel/Kconfig                          |  14 +
>>   drivers/phy/intel/Makefile                         |   1 +
>>   drivers/phy/intel/phy-intel-combo.c                | 632 +++++++++++++++++++++
>>   include/dt-bindings/phy/phy.h                      |   1 +
>>   5 files changed, 749 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
>>   create mode 100644 drivers/phy/intel/phy-intel-combo.c
>>

^ permalink raw reply

* [PATCH V3] dt-bindings: reset: Convert i.MX reset to json-schema
From: Anson Huang @ 2020-05-19  3:42 UTC (permalink / raw)
  To: p.zabel, robh+dt, shawnguo, s.hauer, kernel, festevam, devicetree,
	linux-arm-kernel, linux-kernel
  Cc: Linux-imx

Convert the i.MX reset binding to DT schema format using json-schema.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
---
Changes since V2:
	- remove unnecessary compatible item descriptions.
---
 .../devicetree/bindings/reset/fsl,imx-src.txt      | 49 -------------
 .../devicetree/bindings/reset/fsl,imx-src.yaml     | 82 ++++++++++++++++++++++
 2 files changed, 82 insertions(+), 49 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/reset/fsl,imx-src.txt
 create mode 100644 Documentation/devicetree/bindings/reset/fsl,imx-src.yaml

diff --git a/Documentation/devicetree/bindings/reset/fsl,imx-src.txt b/Documentation/devicetree/bindings/reset/fsl,imx-src.txt
deleted file mode 100644
index 6ed79e6..0000000
--- a/Documentation/devicetree/bindings/reset/fsl,imx-src.txt
+++ /dev/null
@@ -1,49 +0,0 @@
-Freescale i.MX System Reset Controller
-======================================
-
-Please also refer to reset.txt in this directory for common reset
-controller binding usage.
-
-Required properties:
-- compatible: Should be "fsl,<chip>-src"
-- reg: should be register base and length as documented in the
-  datasheet
-- interrupts: Should contain SRC interrupt and CPU WDOG interrupt,
-  in this order.
-- #reset-cells: 1, see below
-
-example:
-
-src: src@20d8000 {
-        compatible = "fsl,imx6q-src";
-        reg = <0x020d8000 0x4000>;
-        interrupts = <0 91 0x04 0 96 0x04>;
-        #reset-cells = <1>;
-};
-
-Specifying reset lines connected to IP modules
-==============================================
-
-The system reset controller can be used to reset the GPU, VPU,
-IPU, and OpenVG IP modules on i.MX5 and i.MX6 ICs. Those device
-nodes should specify the reset line on the SRC in their resets
-property, containing a phandle to the SRC device node and a
-RESET_INDEX specifying which module to reset, as described in
-reset.txt
-
-example:
-
-        ipu1: ipu@2400000 {
-                resets = <&src 2>;
-        };
-        ipu2: ipu@2800000 {
-                resets = <&src 4>;
-        };
-
-The following RESET_INDEX values are valid for i.MX5:
-GPU_RESET     0
-VPU_RESET     1
-IPU1_RESET    2
-OPEN_VG_RESET 3
-The following additional RESET_INDEX value is valid for i.MX6:
-IPU2_RESET    4
diff --git a/Documentation/devicetree/bindings/reset/fsl,imx-src.yaml b/Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
new file mode 100644
index 0000000..27c5e34
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/fsl,imx-src.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX System Reset Controller
+
+maintainers:
+  - Philipp Zabel <p.zabel@pengutronix.de>
+
+description: |
+  The system reset controller can be used to reset the GPU, VPU,
+  IPU, and OpenVG IP modules on i.MX5 and i.MX6 ICs. Those device
+  nodes should specify the reset line on the SRC in their resets
+  property, containing a phandle to the SRC device node and a
+  RESET_INDEX specifying which module to reset, as described in
+  reset.txt
+
+  The following RESET_INDEX values are valid for i.MX5:
+    GPU_RESET     0
+    VPU_RESET     1
+    IPU1_RESET    2
+    OPEN_VG_RESET 3
+  The following additional RESET_INDEX value is valid for i.MX6:
+    IPU2_RESET    4
+
+properties:
+  compatible:
+    oneOf:
+      - const: "fsl,imx51-src"
+      - items:
+          - const: "fsl,imx50-src"
+          - const: "fsl,imx51-src"
+      - items:
+          - const: "fsl,imx53-src"
+          - const: "fsl,imx51-src"
+      - items:
+          - const: "fsl,imx6q-src"
+          - const: "fsl,imx51-src"
+      - items:
+          - const: "fsl,imx6sx-src"
+          - const: "fsl,imx51-src"
+      - items:
+          - const: "fsl,imx6sl-src"
+          - const: "fsl,imx51-src"
+      - items:
+          - const: "fsl,imx6ul-src"
+          - const: "fsl,imx51-src"
+      - items:
+          - const: "fsl,imx6sll-src"
+          - const: "fsl,imx51-src"
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: SRC interrupt
+      - description: CPU WDOG interrupts out of SRC
+    minItems: 1
+    maxItems: 2
+
+  '#reset-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    reset-controller@73fd0000 {
+        compatible = "fsl,imx51-src";
+        reg = <0x73fd0000 0x4000>;
+        interrupts = <75>;
+        #reset-cells = <1>;
+    };
-- 
2.7.4


^ permalink raw reply related

* Re: [RFC] dt-bindings: mailbox: add doorbell support to ARM MHU
From: Viresh Kumar @ 2020-05-19  3:40 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Jassi Brar, Arnd Bergmann, Rob Herring, Frank Rowand,
	Vincent Guittot, linux-arm-kernel, Sudeep Holla, devicetree,
	linux-kernel
In-Reply-To: <20200519012927.GT2165@builder.lan>

On 18-05-20, 18:29, Bjorn Andersson wrote:
> On Thu 14 May 22:17 PDT 2020, Viresh Kumar wrote:
> > This stuff has been doing rounds on the mailing list since several years
> > now with no agreed conclusion by all the parties. And here is another
> > attempt to get some feedback from everyone involved to close this once
> > and for ever. Your comments will very much be appreciated.
> > 
> > The ARM MHU is defined here in the TRM [1] for your reference, which
> > states following:
> > 
> > 	"The MHU drives the signal using a 32-bit register, with all 32
> > 	bits logically ORed together. The MHU provides a set of
> > 	registers to enable software to set, clear, and check the status
> > 	of each of the bits of this register independently.  The use of
> > 	32 bits for each interrupt line enables software to provide more
> > 	information about the source of the interrupt. For example, each
> > 	bit of the register can be associated with a type of event that
> > 	can contribute to raising the interrupt."
> > 
> 
> Does this mean that there are 32 different signals and they are all ORed
> into the same interrupt line to trigger software action when something
> happens?
> 
> Or does it mean that this register is used to pass multi-bit information
> and when any such information is passed an interrupt will be triggered?
> If so, what does that information mean? How is it tied into other Linux
> drivers/subsystems?

I have started to believe the hardware is written badly at this point
:)

The thing is that the register can be used to send a 32 bit data
(which the driver already provides), or it can be used by writing
different bits to the SET register concurrently, without corrupting
the other bits as writing 0 to a bit has no effect, we have a separate
CLEAR register for that. And so it says that all the bits are ORed
together to generate the interrupt, i.e. any bit set will generate an
interrupt. Which also means that you can't send data 0 with the
register.

-- 
viresh

^ permalink raw reply

* Re: [PATCH v7 2/4] usb: dwc3: qcom: Add interconnect support in dwc3 driver
From: Viresh Kumar @ 2020-05-19  3:29 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Felipe Balbi, Georgi Djakov, Matthias Kaehlcke,
	Sandeep Maheswaram, Andy Gross, Greg Kroah-Hartman, Rob Herring,
	Mark Rutland, Stephen Boyd, Doug Anderson, linux-arm-msm,
	linux-usb, devicetree, linux-kernel, Manu Gautam,
	Chandana Kishori Chiluveru
In-Reply-To: <20200518183512.GE2165@builder.lan>

On 18-05-20, 11:35, Bjorn Andersson wrote:
> This misses the case where INTERCONNECT=n and USB_DWC3_QCOM=[ym] which
> I don't see a reason for breaking.
> 
> But if only INTERCONNECT where a bool, then we don't need to specify a
> depends on, because it will either be there, or the stubs will.
> We've come to this conclusion in a lot of different frameworks and I
> don't see why we should do this differently with INTERCONNECT.

INTERCONNECT is a bool now and the patch has been pushed to linux-next
already.

-- 
viresh

^ permalink raw reply

* Re: [PATCH v3] arm64: dts: qcom: apq8016-sbc-d3: Add Qualcomm APQ8016 SBC + D3Camera mezzanine
From: Bjorn Andersson @ 2020-05-19  3:21 UTC (permalink / raw)
  To: Robert Foss
  Cc: Vinod Koul, Rob Herring, linux-arm-msm, devicetree, linux-kernel,
	Nicolas Dechesne
In-Reply-To: <20200518082129.2103683-1-robert.foss@linaro.org>

On Mon 18 May 01:21 PDT 2020, Robert Foss wrote:

> Add device treee support for the Qualcomm APQ8016 SBC, otherwise known as
> the Dragonboard 410c with the D3Camera mezzanine expansion board.
> 
> The D3Camera mezzanine ships in a kit with a OmniVision 5640 sensor module,
> which is what this DT targets.
> 
> Signed-off-by: Robert Foss <robert.foss@linaro.org>
> ---
> 
> Changes since v2:
>  - Vinod: Change copyright assignment to Linaro
> 
> Changes since v1:
>  - Vinod: Changed license to GPL+BSD
>  - Vinod: Changed copyright year to 2020
>  - Nico: Changed name of mezzanine to d3camera
> 
>  arch/arm64/boot/dts/qcom/Makefile             |  1 +
>  .../boot/dts/qcom/apq8016-sbc-d3camera.dts    | 45 +++++++++++++++++++
>  2 files changed, 46 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/apq8016-sbc-d3camera.dts
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index cc103f7020fd..3f95b522694e 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc-d3camera.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-db820c.dtb
>  dtb-$(CONFIG_ARCH_QCOM) += apq8096-ifc6640.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= ipq6018-cp01-c1.dtb
> diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-d3camera.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3camera.dts
> new file mode 100644
> index 000000000000..752e5ec47499
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3camera.dts
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.

That's not how you spell "Linaro Ltd" :)

> + */
> +
> +/dts-v1/;
> +
> +#include "apq8016-sbc.dtsi"
> +
> +/ {
> +	model = "Qualcomm Technologies, Inc. APQ 8016 SBC w/ D3Camera Mezzanine";
> +	compatible = "qcom,apq8016-sbc", "qcom,apq8016", "qcom,sbc";

Please add qcom,apq8016-sbc-d3camera" (or qcom,db410c-d3camera :)) and
drop the vague "qcom,sbc".

> +};
> +
> +&cci_i2c0 {
> +	/delete-node/ camera_rear@3b;

camera_rear@3b is disabled already, so you shouldn't need this.

Should we really carry the node in apq8016-sbc.dtsi? (Unrelated/separate
change).

Regards,
Bjorn

> +
> +	camera_rear@76 {
> +		compatible = "ovti,ov5640";
> +		reg = <0x76>;
> +
> +		enable-gpios = <&msmgpio 34 GPIO_ACTIVE_HIGH>;
> +		reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&camera_rear_default>;
> +
> +		clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
> +		clock-names = "xclk";
> +		clock-frequency = <23880000>;
> +
> +		vdddo-supply = <&camera_vdddo_1v8>;
> +		vdda-supply = <&camera_vdda_2v8>;
> +		vddd-supply = <&camera_vddd_1v5>;
> +
> +		status = "ok";
> +
> +		port {
> +			ov5640_ep: endpoint {
> +				clock-lanes = <1>;
> +				data-lanes = <0 2>;
> +				remote-endpoint = <&csiphy0_ep>;
> +			};
> +		};
> +	};
> +};
> -- 
> 2.25.1
> 

^ permalink raw reply

* Re: [PATCH V1 2/3] mmc: sdhci-msm: Use internal voltage control
From: Bjorn Andersson @ 2020-05-19  3:11 UTC (permalink / raw)
  To: Veerabhadrarao Badiganti
  Cc: adrian.hunter, ulf.hansson, robh+dt, linux-mmc, linux-kernel,
	linux-arm-msm, devicetree, Vijay Viswanath, Asutosh Das,
	Andy Gross
In-Reply-To: <20200518195711.GH2165@builder.lan>

On Mon 18 May 12:57 PDT 2020, Bjorn Andersson wrote:
> On Fri 15 May 04:18 PDT 2020, Veerabhadrarao Badiganti wrote:
> > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
[..]
> > +static int sdhci_msm_register_vreg(struct sdhci_msm_host *msm_host)
> > +{
> > +	int ret = 0;
> 
> No need to initialize ret, first use is an assignment.
> 
> > +	struct mmc_host *mmc = msm_host->mmc;
> > +
> > +	ret = mmc_regulator_get_supply(msm_host->mmc);
> > +	if (ret)
> > +		return ret;
> > +	device_property_read_u32(&msm_host->pdev->dev,
> > +			"vmmc-max-load-microamp",
> > +			&msm_host->vmmc_load);
> > +	device_property_read_u32(&msm_host->pdev->dev,
> > +			"vqmmc-max-load-microamp",
> > +			&msm_host->vqmmc_load);
> 
> These properties are not documented. Do they vary enough to mandate
> being read from DT or could they simply be approximated by a define
> instead?
> 

Disregard my comment about them not being documented, I wasn't Cc'ed on
the binding patch and missed it.

My question about just defining them statically still stands.

Thanks,
Bjorn

^ permalink raw reply

* Re: [V6, 1/2] media: dt-bindings: media: i2c: Document DW9768 bindings
From: Dongchun Zhu @ 2020-05-19  3:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tomasz Figa, Linus Walleij, Bartosz Golaszewski,
	Mauro Carvalho Chehab, Andy Shevchenko, Mark Rutland,
	Sakari Ailus, Nicolas Boichat, Matthias Brugger, Cao Bing Bu,
	srv_heupstream, moderated list:ARM/Mediatek SoC support,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg  Roedel <joro@8bytes.org>,,
	Sj Huang, Linux Media Mailing List, linux-devicetree, Louis Kuo,
	Shengnan Wang (王圣男)
In-Reply-To: <20200518143148.GA18032@bogus>

Hi Tomasz, Rob,

Thanks for the timely review and good suggestions.

On Mon, 2020-05-18 at 08:31 -0600, Rob Herring wrote:
> On Mon, May 18, 2020 at 04:12:28PM +0200, Tomasz Figa wrote:
> > Hi Dongchun,
> > 
> > On Mon, May 18, 2020 at 3:29 PM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > >
> > > Add DeviceTree binding documentation for Dongwoon Anatech DW9768 voice
> > > coil actuator.
> > 
> > Thanks for the patch. Please see my comments below.
> > 
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > 
> > This version includes significant changes, so the reviewed-by tag
> > shouldn't have been carried out.
> > 

Sorry, this is my fault.
In fact, I've struggled with it at the beginning.
Yes, you are right.
Now the new version of patch-set includes huge changes relative to the
last edition.
It would be removed in next release.

> > > ---
> > >  .../bindings/media/i2c/dongwoon,dw9768.yaml        | 105 +++++++++++++++++++++
> > >  MAINTAINERS                                        |   7 ++
> > >  2 files changed, 112 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> > > new file mode 100644
> > > index 0000000..b909e83
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> > > @@ -0,0 +1,105 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +# Copyright (c) 2020 MediaTek Inc.
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9768.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Dongwoon Anatech DW9768 Voice Coil Motor (VCM) Lens Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > +
> > > +description: |-
> > > +  The Dongwoon DW9768 is a single 10-bit digital-to-analog (DAC) converter
> > > +  with 100 mA output current sink capability. VCM current is controlled with
> > > +  a linear mode driver. The DAC is controlled via a 2-wire (I2C-compatible)
> > > +  serial interface that operates at clock rates up to 1MHz. This chip
> > > +  integrates Advanced Actuator Control (AAC) technology and is intended for
> > > +  driving voice coil lenses in camera modules.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      # for DW9768 VCM
> > > +      - dongwoon,dw9768
> > > +      # for GT9769 VCM
> > > +      - giantec,gt9769
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  vin-supply:
> > > +    description:
> > > +      Definition of the regulator used as I2C I/O interface power supply.
> > > +
> > > +  vdd-supply:
> > > +    description:
> > > +      Definition of the regulator used as VCM chip power supply.
> > > +
> > > +  dongwoon,aac-mode:
> > > +    description:
> > > +      Indication of AAC mode select.
> > > +    allOf:
> > > +      - $ref: "/schemas/types.yaml#/definitions/uint32"
> > > +      - enum:
> > > +          - 0    #  Direct (default)
> 
> Default can be expressed as 'default: 0'.
> 

Thanks for the reminder.
Next release I'd try to write DT property "dongwoon,aac-mode" like this:
dongwoon,aac-mode:
  description:
    Indication of AAC mode select.
  allOf:
    - $ref: "/schemas/types.yaml#/definitions/uint32"
    - enum:
        - 0  # Direct
        - 1  # AAC2 (operation time# 0.48 x Tvib)
        - 2  # AAC3 (operation time# 0.70 x Tvib)
        - 3  # AAC4 (operation time# 0.75 x Tvib)
        - 5  # AAC8 (operation time# 1.13 x Tvib)
      default: 0

> > > +          - 1    #  AAC2 (operation time# 0.48 x Tvib)
> > > +          - 2    #  AAC3 (operation time# 0.70 x Tvib)
> > > +          - 3    #  AAC4 (operation time# 0.75 x Tvib)
> > > +          - 4    #  Reserved
> > > +          - 5    #  AAC8 (operation time# 1.13 x Tvib)
> > > +          - 6    #  Reserved
> > > +          - 7    #  Reserved
> > 
> > I'll ultimately leave it to DT maintainers, but is there any reason to
> > define the reserved values?
> 
> No.
> 
> > 
> > > +
> > > +  dongwoon,aac-timing:
> > > +    description:
> > > +      Indication of AAC Timing count, unit of 0.1 milliseconds.
> 
> Why not just use standard units (-us)?
> 

That sounds nice.
I'd re-write the description like this in next release:
dongwoon,aac-timing:
  description:
    Number of AAC Timing count that controlled by one 6-bit period
    of vibration register AACT[5:0], the unit of which is 100us.
  allOf:
    - $ref: "/schemas/types.yaml#/definitions/uint32"
    - minimum: 0x00
    - maximum: 0x3F
    - default: 0x20

> > > +      Valid values vary from 0 to 63 (default 32).
> 
> Looks like constraints.
> 

Yes. This property is controlled by one 6-bit reg.
So here we need to constrain the data set to a narrow range.

> > > +    allOf:
> > > +      - $ref: "/schemas/types.yaml#/definitions/uint32"
> > > +
> > > +  dongwoon,clock-dividing-rate:
> > > +    description:
> > > +      Indication of VCM internal clock dividing rate select, as one multiple
> > > +      factor to calculate VCM ring periodic time Tvib.
> > > +    allOf:
> > > +      - $ref: "/schemas/types.yaml#/definitions/uint32"
> > > +      - enum:
> > > +          - 0    #  Dividing Rate -  2
> > > +          - 1    #  Dividing Rate -  1 (default)
> > > +          - 2    #  Dividing Rate -  1/2
> > > +          - 3    #  Dividing Rate -  1/4
> > > +          - 4    #  Dividing Rate -  8
> > > +          - 5    #  Dividing Rate -  4
> > > +          - 6    #  Dividing Rate -  Reserved
> > > +          - 7    #  Dividing Rate -  Reserved
> > 
> > Ditto.
> > 

Thanks for kindly reminder.
Reserved values would be removed in next release.
Like this:
dongwoon,clock-dividing-rate:
  description:
    Indication of VCM internal clock dividing rate select, as one    
    multiplier to calculate VCM ring periodic time Tvib.
  allOf:
    - $ref: "/schemas/types.yaml#/definitions/uint32"
    - enum:
        - 0   #  Dividing Rate -  2
        - 1   #  Dividing Rate -  1
        - 2    #  Dividing Rate -  1/2
        - 3    #  Dividing Rate -  1/4
        - 4    #  Dividing Rate -  8
        - 5    #  Dividing Rate -  4
      default: 1

> > Best regards,
> > Tomasz


^ permalink raw reply

* Re: [PATCH] arm64: dts: qcom: sc7180: Correct the pdc interrupt ranges
From: Bjorn Andersson @ 2020-05-19  3:09 UTC (permalink / raw)
  To: Maulik Shah
  Cc: agross, linux-arm-msm, linux-kernel, rnayak, ilina, lsrao, mka,
	swboyd, evgreen, dianders, devicetree
In-Reply-To: <1589804402-27130-1-git-send-email-mkshah@codeaurora.org>

On Mon 18 May 05:20 PDT 2020, Maulik Shah wrote:

> Few PDC interrupts do not map to respective parent GIC interrupt.
> Fix this by correcting the pdc interrupt map.
> 
> Fixes: 22f185ee81d2 ("arm64: dts: qcom: sc7180: Add pdc interrupt controller")
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>

Applied

Thanks,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index f1280e0..f6b4ee8 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -2308,8 +2308,7 @@
>  		pdc: interrupt-controller@b220000 {
>  			compatible = "qcom,sc7180-pdc", "qcom,pdc";
>  			reg = <0 0x0b220000 0 0x30000>;
> -			qcom,pdc-ranges = <0 480 15>, <17 497 98>,
> -					  <119 634 4>, <124 639 1>;
> +			qcom,pdc-ranges = <0 480 94>, <94 609 31>, <125 63 1>;
>  			#interrupt-cells = <2>;
>  			interrupt-parent = <&intc>;
>  			interrupt-controller;
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* RE: [PATCH 1/3] dt-bindings: timer: Convert i.MX GPT to json-schema
From: Anson Huang @ 2020-05-19  3:03 UTC (permalink / raw)
  To: Aisheng Dong, daniel.lezcano@linaro.org, tglx@linutronix.de,
	robh+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, Jacky Bai,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: dl-linux-imx
In-Reply-To: <AM6PR04MB49669345E21DC28009C486E680B90@AM6PR04MB4966.eurprd04.prod.outlook.com>



> Subject: RE: [PATCH 1/3] dt-bindings: timer: Convert i.MX GPT to json-schema
> 
> > From: Anson Huang <Anson.Huang@nxp.com>
> > Sent: Monday, May 18, 2020 10:48 PM
> >
> > Convert the i.MX GPT binding to DT schema format using json-schema.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  .../devicetree/bindings/timer/fsl,imxgpt.txt       |  45 ---------
> >  .../devicetree/bindings/timer/fsl,imxgpt.yaml      | 109
> > +++++++++++++++++++++
> >  2 files changed, 109 insertions(+), 45 deletions(-)  delete mode
> > 100644 Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> >  create mode 100644
> > Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> > b/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> > deleted file mode 100644
> > index 5d8fd5b..0000000
> > --- a/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> > +++ /dev/null
> > @@ -1,45 +0,0 @@
> > -Freescale i.MX General Purpose Timer (GPT)
> > -
> > -Required properties:
> > -
> > -- compatible : should be one of following:
> > -  for i.MX1:
> > -  - "fsl,imx1-gpt";
> > -  for i.MX21:
> > -  - "fsl,imx21-gpt";
> > -  for i.MX27:
> > -  - "fsl,imx27-gpt", "fsl,imx21-gpt";
> > -  for i.MX31:
> > -  - "fsl,imx31-gpt";
> > -  for i.MX25:
> > -  - "fsl,imx25-gpt", "fsl,imx31-gpt";
> > -  for i.MX50:
> > -  - "fsl,imx50-gpt", "fsl,imx31-gpt";
> > -  for i.MX51:
> > -  - "fsl,imx51-gpt", "fsl,imx31-gpt";
> > -  for i.MX53:
> > -  - "fsl,imx53-gpt", "fsl,imx31-gpt";
> > -  for i.MX6Q:
> > -  - "fsl,imx6q-gpt", "fsl,imx31-gpt";
> > -  for i.MX6DL:
> > -  - "fsl,imx6dl-gpt";
> > -  for i.MX6SL:
> > -  - "fsl,imx6sl-gpt", "fsl,imx6dl-gpt";
> > -  for i.MX6SX:
> > -  - "fsl,imx6sx-gpt", "fsl,imx6dl-gpt";
> > -- reg : specifies base physical address and size of the registers.
> > -- interrupts : should be the gpt interrupt.
> > -- clocks : the clocks provided by the SoC to drive the timer, must contain
> > -           an entry for each entry in clock-names.
> > -- clock-names : must include "ipg" entry first, then "per" entry.
> > -
> > -Example:
> > -
> > -gpt1: timer@10003000 {
> > -	compatible = "fsl,imx27-gpt", "fsl,imx21-gpt";
> > -	reg = <0x10003000 0x1000>;
> > -	interrupts = <26>;
> > -	clocks = <&clks IMX27_CLK_GPT1_IPG_GATE>,
> > -		 <&clks IMX27_CLK_PER1_GATE>;
> > -	clock-names = "ipg", "per";
> > -};
> > diff --git a/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> > b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> > new file mode 100644
> > index 0000000..5c7186b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +
> > +title: Freescale i.MX General Purpose Timer (GPT)
> > +
> > +maintainers:
> > +  - Sascha Hauer <s.hauer@pengutronix.de>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - description: on i.MX1 the following compatible must be
> > +specified
> 
> Such a description seems not necessary in order to make the doc more clean

You meant the description is NOT necessary for those simple compatible string platforms or for
all platforms, I feel like having the description is more clear for users.

Anson

^ permalink raw reply

* Re: [PATCH v2 2/2] i2c: mediatek: Add i2c ac-timing adjust support
From: Qii Wang @ 2020-05-19  2:57 UTC (permalink / raw)
  To: Joe Perches, Geert Uytterhoeven
  Cc: Wolfram Sang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	srv_heupstream, leilk.liu, Linux Kernel Mailing List,
	linux-mediatek, Linux I2C, Linux ARM
In-Reply-To: <CAMuHMdXjLakWDDEy=02prC7XjAs_xBnt2mArPFNwyHgUoWw6-g@mail.gmail.com>

On Mon, 2020-05-18 at 17:44 +0200, Geert Uytterhoeven wrote:
> On Thu, May 14, 2020 at 3:13 PM Qii Wang <qii.wang@mediatek.com> wrote:
> > This patch adds a algorithm to calculate some ac-timing parameters
> > which can fully meet I2C Spec.
> >
> > Signed-off-by: Qii Wang <qii.wang@mediatek.com>
> > ---
> >  drivers/i2c/busses/i2c-mt65xx.c | 328 +++++++++++++++++++++++++++++++++-------
> >  1 file changed, 277 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> > index 0ca6c38a..7020618 100644
> > --- a/drivers/i2c/busses/i2c-mt65xx.c
> > +++ b/drivers/i2c/busses/i2c-mt65xx.c
> 
> > +/*
> > + * Check and Calculate i2c ac-timing
> > + *
> > + * Hardware design:
> > + * sample_ns = (1000000000 * (sample_cnt + 1)) / clk_src
> > + * xxx_cnt_div =  spec->min_xxx_ns / sample_ns
> > + *
> > + * Sample_ns is rounded down for xxx_cnt_div would be greater
> > + * than the smallest spec.
> > + * The sda_timing is chosen as the middle value between
> > + * the largest and smallest.
> > + */
> > +static int mtk_i2c_check_ac_timing(struct mtk_i2c *i2c,
> > +                                  unsigned int clk_src,
> > +                                  unsigned int check_speed,
> > +                                  unsigned int step_cnt,
> > +                                  unsigned int sample_cnt)
> > +{
> > +       const struct i2c_spec_values *spec;
> > +       unsigned int su_sta_cnt, low_cnt, high_cnt, max_step_cnt;
> > +       unsigned int sda_max, sda_min, clk_ns, max_sta_cnt = 0x3f;
> > +       long long sample_ns = (1000000000 * (sample_cnt + 1)) / clk_src;
> 
> So sample_ns is a 64-bit value. Is that really needed?
> 

(1000000000 * (sample_cnt + 1)) / clk_src value is a 32-bit, (1000000000
* (sample_cnt + 1)) will over 32-bit if sample_cnt is 7.

I think 1000000000 and clk_src is too big, maybe I can reduce then with
be divided all by 1000.
example:

unsigned int sample_ns;
unsigned int clk_src_khz = clk_src / 1000;

if(clk_src_khz)
	sample_ns = (1000000 * (sample_cnt + 1)) / clk_src_khz;
else
	return -EINVAL;

> > +       if (!i2c->dev_comp->timing_adjust)
> > +               return 0;
> > +
> > +       if (i2c->dev_comp->ltiming_adjust)
> > +               max_sta_cnt = 0x100;
> > +
> > +       spec = mtk_i2c_get_spec(check_speed);
> > +
> > +       if (i2c->dev_comp->ltiming_adjust)
> > +               clk_ns = 1000000000 / clk_src;
> > +       else
> > +               clk_ns = sample_ns / 2;
> > +
> > +       su_sta_cnt = DIV_ROUND_UP(spec->min_su_sta_ns, clk_ns);
> > +       if (su_sta_cnt > max_sta_cnt)
> > +               return -1;
> > +
> > +       low_cnt = DIV_ROUND_UP(spec->min_low_ns, sample_ns);
> 
> So this is a 32-bit by 64-bit division (indeed, not 64-by-32!)
> 
> noreply@ellerman.id.au reports:
> 
>     ERROR: modpost: "__udivdi3" [drivers/i2c/busses/i2c-mt65xx.ko] undefined!
>     ERROR: modpost: "__divdi3" [drivers/i2c/busses/i2c-mt65xx.ko] undefined!
> 
> for 32-bit builds.
> 
> > +       max_step_cnt = mtk_i2c_max_step_cnt(check_speed);
> > +       if ((2 * step_cnt) > low_cnt && low_cnt < max_step_cnt) {
> > +               if (low_cnt > step_cnt) {
> > +                       high_cnt = 2 * step_cnt - low_cnt;
> > +               } else {
> > +                       high_cnt = step_cnt;
> > +                       low_cnt = step_cnt;
> > +               }
> > +       } else {
> > +               return -2;
> > +       }
> > +
> > +       sda_max = spec->max_hd_dat_ns / sample_ns;
> > +       if (sda_max > low_cnt)
> > +               sda_max = 0;
> > +
> > +       sda_min = DIV_ROUND_UP(spec->min_su_dat_ns, sample_ns);
> 
> One more.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds


^ permalink raw reply

* RE: [PATCH V2] dt-bindings: clock: Convert i.MX7D clock to json-schema
From: Anson Huang @ 2020-05-19  2:57 UTC (permalink / raw)
  To: Aisheng Dong, mturquette@baylibre.com, sboyd@kernel.org,
	robh+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, Frank Li,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: dl-linux-imx
In-Reply-To: <AM6PR04MB4966B101D3E0A0820FAD4CDC80B90@AM6PR04MB4966.eurprd04.prod.outlook.com>



> Subject: RE: [PATCH V2] dt-bindings: clock: Convert i.MX7D clock to
> json-schema
> 
> > From: Anson Huang <Anson.Huang@nxp.com>
> > Sent: Monday, May 18, 2020 10:53 PM
> >
> > Convert the i.MX7D clock binding to DT schema format using json-schema.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V1:
> > 	- Update maintainer's e-mail address.
> > ---
> >  .../devicetree/bindings/clock/imx7d-clock.txt      | 13 -----
> >  .../devicetree/bindings/clock/imx7d-clock.yaml     | 64
> > ++++++++++++++++++++++
> >  2 files changed, 64 insertions(+), 13 deletions(-)  delete mode
> > 100644 Documentation/devicetree/bindings/clock/imx7d-clock.txt
> >  create mode 100644
> > Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/clock/imx7d-clock.txt
> > b/Documentation/devicetree/bindings/clock/imx7d-clock.txt
> > deleted file mode 100644
> > index 9d3026d..0000000
> > --- a/Documentation/devicetree/bindings/clock/imx7d-clock.txt
> > +++ /dev/null
> > @@ -1,13 +0,0 @@
> > -* Clock bindings for Freescale i.MX7 Dual
> > -
> > -Required properties:
> > -- compatible: Should be "fsl,imx7d-ccm"
> > -- reg: Address and length of the register set
> > -- #clock-cells: Should be <1>
> > -- clocks: list of clock specifiers, must contain an entry for each
> > required
> > -  entry in clock-names
> > -- clock-names: should include entries "ckil", "osc"
> > -
> > -The clock consumer should specify the desired clock by having the
> > clock -ID in its "clocks" phandle cell.  See
> > include/dt-bindings/clock/imx7d-clock.h
> > -for the full list of i.MX7 Dual clock IDs.
> > diff --git a/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> > b/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> > new file mode 100644
> > index 0000000..8cd0573
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +
> > +title: Clock bindings for Freescale i.MX7 Dual
> > +
> > +maintainers:
> > +  - Frank Li <Frank.Li@nxp.com>
> > +  - Anson Huang <Anson.Huang@nxp.com>
> > +
> > +description: |
> > +  The clock consumer should specify the desired clock by having the
> > +clock
> > +  ID in its "clocks" phandle cell. See
> > +include/dt-bindings/clock/imx7d-clock.h
> > +  for the full list of i.MX7 Dual clock IDs.
> > +
> > +properties:
> > +  compatible:
> > +    const: fsl,imx7d-ccm
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    items:
> > +      - description: CCM interrupt request 1
> > +      - description: CCM interrupt request 2
> 
> Do we have a more specific description from RM?
> Otherwise, I'm fine with this patch.

I checked both RM and design doc before, can NOT find any description about
these 2 interrupts, so I did NOT add detail descriptions for them.

Anson

^ permalink raw reply

* RE: [PATCH V2] dt-bindings: reset: Convert i.MX reset to json-schema
From: Aisheng Dong @ 2020-05-19  2:55 UTC (permalink / raw)
  To: Anson Huang, p.zabel@pengutronix.de, robh+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: dl-linux-imx
In-Reply-To: <1589796212-9993-1-git-send-email-Anson.Huang@nxp.com>

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Monday, May 18, 2020 6:04 PM
> 
> Convert the i.MX reset binding to DT schema format using json-schema.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V1:
> 	- add the compatible strings per SoC;
> 	- use "reset-controller" as node name instead of src;
> 	- add interrupt items description.
> ---
>  .../devicetree/bindings/reset/fsl,imx-src.txt      | 49 -----------
>  .../devicetree/bindings/reset/fsl,imx-src.yaml     | 98
> ++++++++++++++++++++++
>  2 files changed, 98 insertions(+), 49 deletions(-)  delete mode 100644
> Documentation/devicetree/bindings/reset/fsl,imx-src.txt
>  create mode 100644
> Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/fsl,imx-src.txt
> b/Documentation/devicetree/bindings/reset/fsl,imx-src.txt
> deleted file mode 100644
> index 6ed79e6..0000000
> --- a/Documentation/devicetree/bindings/reset/fsl,imx-src.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -Freescale i.MX System Reset Controller
> -======================================
> -
> -Please also refer to reset.txt in this directory for common reset -controller
> binding usage.
> -
> -Required properties:
> -- compatible: Should be "fsl,<chip>-src"
> -- reg: should be register base and length as documented in the
> -  datasheet
> -- interrupts: Should contain SRC interrupt and CPU WDOG interrupt,
> -  in this order.
> -- #reset-cells: 1, see below
> -
> -example:
> -
> -src: src@20d8000 {
> -        compatible = "fsl,imx6q-src";
> -        reg = <0x020d8000 0x4000>;
> -        interrupts = <0 91 0x04 0 96 0x04>;
> -        #reset-cells = <1>;
> -};
> -
> -Specifying reset lines connected to IP modules
> -==============================================
> -
> -The system reset controller can be used to reset the GPU, VPU, -IPU, and
> OpenVG IP modules on i.MX5 and i.MX6 ICs. Those device -nodes should
> specify the reset line on the SRC in their resets -property, containing a phandle
> to the SRC device node and a -RESET_INDEX specifying which module to reset,
> as described in -reset.txt
> -
> -example:
> -
> -        ipu1: ipu@2400000 {
> -                resets = <&src 2>;
> -        };
> -        ipu2: ipu@2800000 {
> -                resets = <&src 4>;
> -        };
> -
> -The following RESET_INDEX values are valid for i.MX5:
> -GPU_RESET     0
> -VPU_RESET     1
> -IPU1_RESET    2
> -OPEN_VG_RESET 3
> -The following additional RESET_INDEX value is valid for i.MX6:
> -IPU2_RESET    4
> diff --git a/Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
> b/Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
> new file mode 100644
> index 0000000..7cd6095
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/fsl,imx-src.yaml
> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---

[...]

> +title: Freescale i.MX System Reset Controller
> +
> +maintainers:
> +  - Philipp Zabel <p.zabel@pengutronix.de>
> +
> +description: |
> +  The system reset controller can be used to reset the GPU, VPU,
> +  IPU, and OpenVG IP modules on i.MX5 and i.MX6 ICs. Those device
> +  nodes should specify the reset line on the SRC in their resets
> +  property, containing a phandle to the SRC device node and a
> +  RESET_INDEX specifying which module to reset, as described in
> +  reset.txt
> +
> +  The following RESET_INDEX values are valid for i.MX5:
> +    GPU_RESET     0
> +    VPU_RESET     1
> +    IPU1_RESET    2
> +    OPEN_VG_RESET 3
> +  The following additional RESET_INDEX value is valid for i.MX6:
> +    IPU2_RESET    4
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: on i.MX51 the following compatible must be specified

Unnecessary description

Otherwise:

Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Aisheng

> +        items:
> +          - const: "fsl,imx51-src"
> +
> +      - description: on i.MX50 the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx50-src"
> +          - const: "fsl,imx51-src"
> +
> +      - description: on i.MX53 the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx53-src"
> +          - const: "fsl,imx51-src"
> +
> +      - description: on i.MX6Q the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx6q-src"
> +          - const: "fsl,imx51-src"
> +
> +      - description: on i.MX6SX the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx6sx-src"
> +          - const: "fsl,imx51-src"
> +
> +      - description: on i.MX6SL the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx6sl-src"
> +          - const: "fsl,imx51-src"
> +
> +      - description: on i.MX6UL the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx6ul-src"
> +          - const: "fsl,imx51-src"
> +
> +      - description: on i.MX6SLL the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx6sll-src"
> +          - const: "fsl,imx51-src"
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: SRC interrupt
> +      - description: CPU WDOG interrupts out of SRC
> +    minItems: 1
> +    maxItems: 2
> +
> +  '#reset-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    reset-controller@73fd0000 {
> +        compatible = "fsl,imx51-src";
> +        reg = <0x73fd0000 0x4000>;
> +        interrupts = <75>;
> +        #reset-cells = <1>;
> +    };
> --
> 2.7.4


^ permalink raw reply

* RE: [PATCH] ARM: dts: imx: make src node name generic
From: Aisheng Dong @ 2020-05-19  2:52 UTC (permalink / raw)
  To: Anson Huang, robh+dt@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: dl-linux-imx
In-Reply-To: <1589805593-14164-1-git-send-email-Anson.Huang@nxp.com>

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Monday, May 18, 2020 8:40 PM
> 
> Node name should be generic, use "reset-controller" instead of "src" for
> i.MX6/i.MX7 SoCs src nodes.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Aisheng

> ---
>  arch/arm/boot/dts/imx6qdl.dtsi | 2 +-
>  arch/arm/boot/dts/imx6sl.dtsi  | 2 +-
>  arch/arm/boot/dts/imx6sx.dtsi  | 2 +-
>  arch/arm/boot/dts/imx6ul.dtsi  | 2 +-
>  arch/arm/boot/dts/imx7s.dtsi   | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 1763c2b..39d4afd 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -858,7 +858,7 @@
>  				interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
>  			};
> 
> -			src: src@20d8000 {
> +			src: reset-controller@20d8000 {
>  				compatible = "fsl,imx6q-src", "fsl,imx51-src";
>  				reg = <0x020d8000 0x4000>;
>  				interrupts = <0 91 IRQ_TYPE_LEVEL_HIGH>, diff --git
> a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi index
> fcb84fe..911d8cf 100644
> --- a/arch/arm/boot/dts/imx6sl.dtsi
> +++ b/arch/arm/boot/dts/imx6sl.dtsi
> @@ -678,7 +678,7 @@
>  				interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
>  			};
> 
> -			src: src@20d8000 {
> +			src: reset-controller@20d8000 {
>  				compatible = "fsl,imx6sl-src", "fsl,imx51-src";
>  				reg = <0x020d8000 0x4000>;
>  				interrupts = <0 91 IRQ_TYPE_LEVEL_HIGH>, diff --git
> a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi index
> d6f8317..e031337 100644
> --- a/arch/arm/boot/dts/imx6sx.dtsi
> +++ b/arch/arm/boot/dts/imx6sx.dtsi
> @@ -754,7 +754,7 @@
>  				interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
>  			};
> 
> -			src: src@20d8000 {
> +			src: reset-controller@20d8000 {
>  				compatible = "fsl,imx6sx-src", "fsl,imx51-src";
>  				reg = <0x020d8000 0x4000>;
>  				interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>, diff --git
> a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index
> 2ccf67c..35e7301 100644
> --- a/arch/arm/boot/dts/imx6ul.dtsi
> +++ b/arch/arm/boot/dts/imx6ul.dtsi
> @@ -676,7 +676,7 @@
>  				interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
>  			};
> 
> -			src: src@20d8000 {
> +			src: reset-controller@20d8000 {
>  				compatible = "fsl,imx6ul-src", "fsl,imx51-src";
>  				reg = <0x020d8000 0x4000>;
>  				interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>, diff --git
> a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi index
> 76e3ffb..8bac491 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -624,7 +624,7 @@
>  				clock-names = "ckil", "osc";
>  			};
> 
> -			src: src@30390000 {
> +			src: reset-controller@30390000 {
>  				compatible = "fsl,imx7d-src", "syscon";
>  				reg = <0x30390000 0x10000>;
>  				interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> --
> 2.7.4


^ permalink raw reply

* RE: [PATCH 3/3] dt-bindings: timer: Convert i.MX SYSCTR to json-schema
From: Aisheng Dong @ 2020-05-19  2:51 UTC (permalink / raw)
  To: Anson Huang, daniel.lezcano@linaro.org, tglx@linutronix.de,
	robh+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, Jacky Bai,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: dl-linux-imx
In-Reply-To: <1589813260-20036-4-git-send-email-Anson.Huang@nxp.com>

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Monday, May 18, 2020 10:48 PM
> 
> Convert the i.MX SYSCTR binding to DT schema format using json-schema.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Aisheng

^ permalink raw reply

* RE: [PATCH 2/3] dt-bindings: timer: Convert i.MX TPM to json-schema
From: Aisheng Dong @ 2020-05-19  2:49 UTC (permalink / raw)
  To: Anson Huang, daniel.lezcano@linaro.org, tglx@linutronix.de,
	robh+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, Jacky Bai,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: dl-linux-imx
In-Reply-To: <1589813260-20036-3-git-send-email-Anson.Huang@nxp.com>

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Monday, May 18, 2020 10:48 PM
> 
> Convert the i.MX TPM binding to DT schema format using json-schema.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  .../devicetree/bindings/timer/nxp,tpm-timer.txt    | 28 ----------
>  .../devicetree/bindings/timer/nxp,tpm-timer.yaml   | 63
> ++++++++++++++++++++++
>  2 files changed, 63 insertions(+), 28 deletions(-)  delete mode 100644
> Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
>  create mode 100644
> Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml
> 
> diff --git a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> deleted file mode 100644
> index f82087b..0000000
> --- a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -NXP Low Power Timer/Pulse Width Modulation Module (TPM)
> -
> -The Timer/PWM Module (TPM) supports input capture, output compare, -and
> the generation of PWM signals to control electric motor and power
> -management applications. The counter, compare and capture registers -are
> clocked by an asynchronous clock that can remain enabled in low -power modes.
> TPM can support global counter bus where one TPM drives -the counter bus for
> the others, provided bit width is the same.
> -
> -Required properties:
> -
> -- compatible :	should be "fsl,imx7ulp-tpm"
> -- reg :		Specifies base physical address and size of the register sets
> -		for the clock event device and clock source device.
> -- interrupts :	Should be the clock event device interrupt.
> -- clocks :	The clocks provided by the SoC to drive the timer, must contain
> -		an entry for each entry in clock-names.
> -- clock-names : Must include the following entries: "ipg" and "per".
> -
> -Example:
> -tpm5: tpm@40260000 {
> -	compatible = "fsl,imx7ulp-tpm";
> -	reg = <0x40260000 0x1000>;
> -	interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> -	clocks = <&clks IMX7ULP_CLK_NIC1_BUS_DIV>,
> -		 <&clks IMX7ULP_CLK_LPTPM5>;
> -	clock-names = "ipg", "per";
> -};
> diff --git a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml
> b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml
> new file mode 100644
> index 0000000..0d34610
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2

[...]

> +
> +title: NXP Low Power Timer/Pulse Width Modulation Module (TPM)
> +
> +maintainers:
> +  - Dong Aisheng <aisheng.dong@nxp.com>
> +
> +description: |
> +  The Timer/PWM Module (TPM) supports input capture, output compare,
> +  and the generation of PWM signals to control electric motor and power
> +  management applications. The counter, compare and capture registers
> +  are clocked by an asynchronous clock that can remain enabled in low
> +  power modes. TPM can support global counter bus where one TPM drives
> +  the counter bus for the others, provided bit width is the same.
> +
> +properties:
> +  compatible:
> +    const: fsl,imx7ulp-tpm
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: SoC TPM ipg clock
> +      - description: SoC TPM per clock

> +    maxItems: 2

Unneeded line

> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: per

> +    maxItems: 2

Ditto

Otherwise:

Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Aisheng

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx7ulp-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    timer@40260000 {
> +        compatible = "fsl,imx7ulp-tpm";
> +        reg = <0x40260000 0x1000>;
> +        interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
> +                 <&pcc2 IMX7ULP_CLK_LPTPM5>;
> +        clock-names = "ipg", "per";
> +    };
> --
> 2.7.4


^ permalink raw reply

* RE: [PATCH 1/3] dt-bindings: timer: Convert i.MX GPT to json-schema
From: Aisheng Dong @ 2020-05-19  2:46 UTC (permalink / raw)
  To: Anson Huang, daniel.lezcano@linaro.org, tglx@linutronix.de,
	robh+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, Jacky Bai,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
  Cc: dl-linux-imx
In-Reply-To: <1589813260-20036-2-git-send-email-Anson.Huang@nxp.com>

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Monday, May 18, 2020 10:48 PM
> 
> Convert the i.MX GPT binding to DT schema format using json-schema.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  .../devicetree/bindings/timer/fsl,imxgpt.txt       |  45 ---------
>  .../devicetree/bindings/timer/fsl,imxgpt.yaml      | 109
> +++++++++++++++++++++
>  2 files changed, 109 insertions(+), 45 deletions(-)  delete mode 100644
> Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
>  create mode 100644
> Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> b/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> deleted file mode 100644
> index 5d8fd5b..0000000
> --- a/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -Freescale i.MX General Purpose Timer (GPT)
> -
> -Required properties:
> -
> -- compatible : should be one of following:
> -  for i.MX1:
> -  - "fsl,imx1-gpt";
> -  for i.MX21:
> -  - "fsl,imx21-gpt";
> -  for i.MX27:
> -  - "fsl,imx27-gpt", "fsl,imx21-gpt";
> -  for i.MX31:
> -  - "fsl,imx31-gpt";
> -  for i.MX25:
> -  - "fsl,imx25-gpt", "fsl,imx31-gpt";
> -  for i.MX50:
> -  - "fsl,imx50-gpt", "fsl,imx31-gpt";
> -  for i.MX51:
> -  - "fsl,imx51-gpt", "fsl,imx31-gpt";
> -  for i.MX53:
> -  - "fsl,imx53-gpt", "fsl,imx31-gpt";
> -  for i.MX6Q:
> -  - "fsl,imx6q-gpt", "fsl,imx31-gpt";
> -  for i.MX6DL:
> -  - "fsl,imx6dl-gpt";
> -  for i.MX6SL:
> -  - "fsl,imx6sl-gpt", "fsl,imx6dl-gpt";
> -  for i.MX6SX:
> -  - "fsl,imx6sx-gpt", "fsl,imx6dl-gpt";
> -- reg : specifies base physical address and size of the registers.
> -- interrupts : should be the gpt interrupt.
> -- clocks : the clocks provided by the SoC to drive the timer, must contain
> -           an entry for each entry in clock-names.
> -- clock-names : must include "ipg" entry first, then "per" entry.
> -
> -Example:
> -
> -gpt1: timer@10003000 {
> -	compatible = "fsl,imx27-gpt", "fsl,imx21-gpt";
> -	reg = <0x10003000 0x1000>;
> -	interrupts = <26>;
> -	clocks = <&clks IMX27_CLK_GPT1_IPG_GATE>,
> -		 <&clks IMX27_CLK_PER1_GATE>;
> -	clock-names = "ipg", "per";
> -};
> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> new file mode 100644
> index 0000000..5c7186b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---
> +
> +title: Freescale i.MX General Purpose Timer (GPT)
> +
> +maintainers:
> +  - Sascha Hauer <s.hauer@pengutronix.de>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: on i.MX1 the following compatible must be specified

Such a description seems not necessary in order to make the doc more clean

> +        items:
> +          - const: "fsl,imx1-gpt"
> +
> +      - description: on i.MX21 the following compatible must be specified
> +        items:
> +          - const: "fsl,imx21-gpt"
> +
> +      - description: on i.MX27 the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx27-gpt"
> +          - const: "fsl,imx21-gpt"
> +
> +      - description: on i.MX31 the following compatible must be specified
> +        items:
> +          - const: "fsl,imx31-gpt"
> +
> +      - description: on i.MX25 the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx25-gpt"
> +          - const: "fsl,imx31-gpt"
> +
> +      - description: on i.MX50 the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx50-gpt"
> +          - const: "fsl,imx31-gpt"
> +
> +      - description: on i.MX51 the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx51-gpt"
> +          - const: "fsl,imx31-gpt"
> +
> +      - description: on i.MX53 the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx53-gpt"
> +          - const: "fsl,imx31-gpt"
> +
> +      - description: on i.MX6Q the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx6q-gpt"
> +          - const: "fsl,imx31-gpt"
> +
> +      - description: on i.MX6DL the following compatible must be specified
> +        items:
> +          - const: "fsl,imx6dl-gpt"
> +
> +      - description: on i.MX6SL the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx6sl-gpt"
> +          - const: "fsl,imx6dl-gpt"
> +
> +      - description: on i.MX6SX the following compatibles must be specified
> +        items:
> +          - const: "fsl,imx6sx-gpt"
> +          - const: "fsl,imx6dl-gpt"
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: SoC GPT ipg clock
> +      - description: SoC GPT per clock
> +    maxItems: 2

maxItems is not needed

> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: per
> +    maxItems: 2

Ditto 

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx27-clock.h>
> +
> +    timer@10003000 {
> +        compatible = "fsl,imx27-gpt", "fsl,imx21-gpt";
> +        reg = <0x10003000 0x1000>;
> +        interrupts = <26>;
> +        clocks = <&clks IMX27_CLK_GPT1_IPG_GATE>,
> +                 <&clks IMX27_CLK_PER1_GATE>;
> +        clock-names = "ipg", "per";
> +    };
> --
> 2.7.4


^ permalink raw reply

* RE: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by device controller
From: Jun Li @ 2020-05-19  2:24 UTC (permalink / raw)
  To: Felipe Balbi, Thinh Nguyen, Jun Li
  Cc: John Stultz, lkml, Yu Chen, Greg Kroah-Hartman, Rob Herring,
	Mark Rutland, ShuFan Lee, Heikki Krogerus, Suzuki K Poulose,
	Chunfeng Yun, Hans de Goede, Andy Shevchenko, Valentin Schneider,
	Jack Pham, Linux USB List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Peter Chen, Thinh Nguyen
In-Reply-To: <87d074xfbq.fsf@kernel.org>



> -----Original Message-----
> From: Felipe Balbi <balbif@gmail.com> On Behalf Of Felipe Balbi
> Sent: 2020年5月16日 19:57
> To: Jun Li <jun.li@nxp.com>; Thinh Nguyen <Thinh.Nguyen@synopsys.com>; Jun Li
> <lijun.kernel@gmail.com>
> Cc: John Stultz <john.stultz@linaro.org>; lkml <linux-kernel@vger.kernel.org>; Yu
> Chen <chenyu56@huawei.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Rob
> Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; ShuFan Lee
> <shufan_lee@richtek.com>; Heikki Krogerus <heikki.krogerus@linux.intel.com>;
> Suzuki K Poulose <suzuki.poulose@arm.com>; Chunfeng Yun
> <chunfeng.yun@mediatek.com>; Hans de Goede <hdegoede@redhat.com>; Andy Shevchenko
> <andy.shevchenko@gmail.com>; Valentin Schneider <valentin.schneider@arm.com>;
> Jack Pham <jackp@codeaurora.org>; Linux USB List <linux-usb@vger.kernel.org>; open
> list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>;
> Peter Chen <peter.chen@nxp.com>; Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> Subject: RE: [PATCH v4 3/9] usb: dwc3: Increase timeout for CmdAct cleared by device
> controller
> 
> 
> Hi,
> 
> Jun Li <jun.li@nxp.com> writes:
> >> >> Hi Thinh, could you comment this?
> >> >
> >> > You only need to wake up the usb2 phy when issuing the command
> >> > while running in highspeed or below. If you're running in SS or
> >> > higher, internally the controller does it for you for usb3 phy. In
> >> > Jun's case, it seems like it takes longer for his phy to wake up.
> >> >
> >> > IMO, in this case, I think it's fine to increase the command timeout.
> >>
> >> Is there an upper limit to this? Is 32k clock the slowest that can be
> >> fed to the PHY as a suspend clock?
> >
> > Yes, 32K clock is the slowest, Per DWC3 document on Power Down Scale
> > (bits 31:19 of GCTL):
> >
> > "Power Down Scale (PwrDnScale)
> > The USB3 suspend_clk input replaces pipe3_rx_pclk as a clock source to
> > a small part of the USB3 controller that operates when the SS PHY is
> > in its lowest power (P3) state, and therefore does not provide a clock.
> > The Power Down Scale field specifies how many suspend_clk periods fit
> > into a 16 kHz clock period. When performing the division, round up the
> > remainder.
> > For example, when using an 8-bit/16-bit/32-bit PHY and 25-MHz Suspend
> > clock, Power Down Scale = 25000 kHz/16 kHz = 13'd1563 (rounder up)
> > Note:
> > - Minimum Suspend clock frequency is 32 kHz
> > - Maximum Suspend clock frequency is 125 MHz"
> 
> Cool, now do we have an upper bound for how many clock cycles it takes to wake up
> the PHY? 
My understanding is this ep command does not wake up the SS PHY,
the SS PHY still stays at P3 when execute this ep command. The time
required here is to wait controller complete something for this ep
command with 32K clock.

> Then we can just set the time to that upper bound.
Per my test with trace, the time is about 400us(~13 cycles).

Thanks
Li Jun
> 
> --
> balbi

^ permalink raw reply

* RE: [PATCH V2] dt-bindings: clock: Convert i.MX7D clock to json-schema
From: Aisheng Dong @ 2020-05-19  2:01 UTC (permalink / raw)
  To: Anson Huang, mturquette@baylibre.com, sboyd@kernel.org,
	robh+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, Frank Li,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: dl-linux-imx
In-Reply-To: <1589813554-20929-1-git-send-email-Anson.Huang@nxp.com>

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Monday, May 18, 2020 10:53 PM
> 
> Convert the i.MX7D clock binding to DT schema format using json-schema.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V1:
> 	- Update maintainer's e-mail address.
> ---
>  .../devicetree/bindings/clock/imx7d-clock.txt      | 13 -----
>  .../devicetree/bindings/clock/imx7d-clock.yaml     | 64
> ++++++++++++++++++++++
>  2 files changed, 64 insertions(+), 13 deletions(-)  delete mode 100644
> Documentation/devicetree/bindings/clock/imx7d-clock.txt
>  create mode 100644
> Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/imx7d-clock.txt
> b/Documentation/devicetree/bindings/clock/imx7d-clock.txt
> deleted file mode 100644
> index 9d3026d..0000000
> --- a/Documentation/devicetree/bindings/clock/imx7d-clock.txt
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -* Clock bindings for Freescale i.MX7 Dual
> -
> -Required properties:
> -- compatible: Should be "fsl,imx7d-ccm"
> -- reg: Address and length of the register set
> -- #clock-cells: Should be <1>
> -- clocks: list of clock specifiers, must contain an entry for each required
> -  entry in clock-names
> -- clock-names: should include entries "ckil", "osc"
> -
> -The clock consumer should specify the desired clock by having the clock -ID in
> its "clocks" phandle cell.  See include/dt-bindings/clock/imx7d-clock.h
> -for the full list of i.MX7 Dual clock IDs.
> diff --git a/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> b/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> new file mode 100644
> index 0000000..8cd0573
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---
> +
> +title: Clock bindings for Freescale i.MX7 Dual
> +
> +maintainers:
> +  - Frank Li <Frank.Li@nxp.com>
> +  - Anson Huang <Anson.Huang@nxp.com>
> +
> +description: |
> +  The clock consumer should specify the desired clock by having the
> +clock
> +  ID in its "clocks" phandle cell. See
> +include/dt-bindings/clock/imx7d-clock.h
> +  for the full list of i.MX7 Dual clock IDs.
> +
> +properties:
> +  compatible:
> +    const: fsl,imx7d-ccm
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: CCM interrupt request 1
> +      - description: CCM interrupt request 2

Do we have a more specific description from RM?
Otherwise, I'm fine with this patch.

> +    maxItems: 2
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  clocks:
> +    items:
> +      - description: 32k osc
> +      - description: 24m osc
> +
> +  clock-names:
> +    items:
> +      - const: ckil
> +      - const: osc
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    clock-controller@30380000 {
> +        compatible = "fsl,imx7d-ccm";
> +        reg = <0x30380000 0x10000>;
> +        interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> +        #clock-cells = <1>;
> +        clocks = <&ckil>, <&osc>;
> +        clock-names = "ckil", "osc";
> +    };
> --
> 2.7.4


^ permalink raw reply

* RE: [PATCH 2/2] ARM: dts: imx5: make src node name generic
From: Aisheng Dong @ 2020-05-19  1:56 UTC (permalink / raw)
  To: Anson Huang, robh+dt@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: dl-linux-imx
In-Reply-To: <1589806460-19592-2-git-send-email-Anson.Huang@nxp.com>

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Monday, May 18, 2020 8:54 PM
> To: robh+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: [PATCH 2/2] ARM: dts: imx5: make src node name generic
> 
> Node name should be generic, use "reset-controller" instead of "src" for
> i.MX5 SoCs src nodes.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Aisheng

^ permalink raw reply

* RE: [PATCH 1/2] ARM: dts: imx50: Add src node interrupt
From: Aisheng Dong @ 2020-05-19  1:55 UTC (permalink / raw)
  To: Anson Huang, robh+dt@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: dl-linux-imx
In-Reply-To: <1589806460-19592-1-git-send-email-Anson.Huang@nxp.com>

> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Monday, May 18, 2020 8:54 PM
> 
> Interrupt is a required property according to SRC binding, add it for SRC node.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Aisheng

^ permalink raw reply

* Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting
From: Xiongfeng Wang @ 2020-05-19  1:50 UTC (permalink / raw)
  To: Rafael J. Wysocki, Serge Semin
  Cc: Serge Semin, Rafael J. Wysocki, Viresh Kumar, Rafael J. Wysocki,
	Thomas Bogendoerfer, Ulf Hansson, Matthias Kaehlcke,
	Alexey Malahov, Paul Burton, Ralf Baechle, Arnd Bergmann,
	Rob Herring, linux-mips, devicetree@vger.kernel.org, Stable,
	Frederic Weisbecker, Ingo Molnar, Yue Hu, Linux PM,
	Linux Kernel Mailing List
In-Reply-To: <CAJZ5v0juP6bsB9TRcned4nTQ=yFEOU5J2M7tt2bokYSYgoPPEg@mail.gmail.com>

Hi Rafael,

On 2020/5/18 19:05, Rafael J. Wysocki wrote:
> On Mon, May 18, 2020 at 12:56 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
>>
>> On Mon, May 18, 2020 at 12:51:15PM +0200, Rafael J. Wysocki wrote:
>>> On Mon, May 18, 2020 at 12:46 PM Serge Semin
>>> <Sergey.Semin@baikalelectronics.ru> wrote:
>>>>
>>>> On Mon, May 18, 2020 at 12:41:19PM +0200, Rafael J. Wysocki wrote:
>>>>> On Monday, May 18, 2020 12:31:02 PM CEST Serge Semin wrote:
>>>>>> On Mon, May 18, 2020 at 03:54:15PM +0530, Viresh Kumar wrote:
>>>>>>> On 18-05-20, 12:22, Rafael J. Wysocki wrote:
>>>>>>>> On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
>>>>>>>>> On 18-05-20, 11:53, Rafael J. Wysocki wrote:
>>>>>>>>>> That said if you really only want it to return 0 on success, you may as well
>>>>>>>>>> add a ret = 0; statement (with a comment explaining why it is needed) after
>>>>>>>>>> the last break in the loop.
>>>>>>>>>
>>>>>>>>> That can be done as well, but will be a bit less efficient as the loop
>>>>>>>>> will execute once for each policy, and so the statement will run
>>>>>>>>> multiple times. Though it isn't going to add any significant latency
>>>>>>>>> in the code.
>>>>>>>>
>>>>>>>> Right.
>>>>>>>>
>>>>>>>> However, the logic in this entire function looks somewhat less than
>>>>>>>> straightforward to me, because it looks like it should return an
>>>>>>>> error on the first policy without a frequency table (having a frequency
>>>>>>>> table depends on the driver and that is the same for all policies, so it
>>>>>>>> is pointless to iterate any further in that case).
>>>>>>>>
>>>>>>>> Also, the error should not be -EINVAL, because that means "invalid
>>>>>>>> argument" which would be the state value.
>>>>>>>>
>>>>>>>> So I would do something like this:
>>>>>>>>
>>>>>>>> ---
>>>>>>>>  drivers/cpufreq/cpufreq.c |   11 ++++++-----
>>>>>>>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> Index: linux-pm/drivers/cpufreq/cpufreq.c
>>>>>>>> ===================================================================
>>>>>>>> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
>>>>>>>> +++ linux-pm/drivers/cpufreq/cpufreq.c
>>>>>>>> @@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
>>>>>>>>  static int cpufreq_boost_set_sw(int state)
>>>>>>>>  {
>>>>>>>>         struct cpufreq_policy *policy;
>>>>>>>> -       int ret = -EINVAL;
>>>>>>>>
>>>>>>>>         for_each_active_policy(policy) {
>>>>>>>> +               int ret;
>>>>>>>> +
>>>>>>>>                 if (!policy->freq_table)
>>>>>>>> -                       continue;
>>>>>>>> +                       return -ENXIO;
>>>>>>>>
>>>>>>>>                 ret = cpufreq_frequency_table_cpuinfo(policy,
>>>>>>>>                                                       policy->freq_table);
>>>>>>>>                 if (ret) {
>>>>>>>>                         pr_err("%s: Policy frequency update failed\n",
>>>>>>>>                                __func__);
>>>>>>>> -                       break;
>>>>>>>> +                       return ret;
>>>>>>>>                 }
>>>>>>>>
>>>>>>>>                 ret = freq_qos_update_request(policy->max_freq_req, policy->max);
>>>>>>>>                 if (ret < 0)
>>>>>>>> -                       break;
>>>>>>>> +                       return ret;
>>>>>>>>         }
>>>>>>>>
>>>>>>>> -       return ret;
>>>>>>>> +       return 0;
>>>>>>>>  }
>>>>>>>>
>>>>>>>>  int cpufreq_boost_trigger_state(int state)
>>>>>>>
>>>>>>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>>>>>>
>>>>>> Ok. Thanks for the comments. Shall I resend the patch with update Rafael
>>>>>> suggests or you'll merge the Rafael's fix in yourself?
>>>>>
>>>>> I'll apply the fix directly, thanks!
>>>>
>>>> Great. Is it going to be available in the repo:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
>>>> ?
>>>
>>> Yes, it is.  Please see the bleeding-edge branch in there, thanks!

Thanks for CCing me. I will write my next version based on this branch.

Thanks,
Xiongfeng

>>
>> No credits with at least Reported-by tag? That's sad.(
> 
> OK, done now, but you are not the only reported of it, so I've added
> the other reporter too.
> 
> Thanks!
> 
> .
> 


^ permalink raw reply

* Re: [RFC] dt-bindings: mailbox: add doorbell support to ARM MHU
From: Bjorn Andersson @ 2020-05-19  1:29 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Jassi Brar, Arnd Bergmann, Rob Herring, Frank Rowand,
	Vincent Guittot, linux-arm-kernel, Sudeep Holla, devicetree,
	linux-kernel
In-Reply-To: <0a50f0cf5593baeb628dc8606c523665e5e2ae6c.1589519600.git.viresh.kumar@linaro.org>

On Thu 14 May 22:17 PDT 2020, Viresh Kumar wrote:

> From: Sudeep Holla <sudeep.holla@arm.com>
> 
> Hi Rob, Arnd and Jassi,
> 
> This stuff has been doing rounds on the mailing list since several years
> now with no agreed conclusion by all the parties. And here is another
> attempt to get some feedback from everyone involved to close this once
> and for ever. Your comments will very much be appreciated.
> 
> The ARM MHU is defined here in the TRM [1] for your reference, which
> states following:
> 
> 	"The MHU drives the signal using a 32-bit register, with all 32
> 	bits logically ORed together. The MHU provides a set of
> 	registers to enable software to set, clear, and check the status
> 	of each of the bits of this register independently.  The use of
> 	32 bits for each interrupt line enables software to provide more
> 	information about the source of the interrupt. For example, each
> 	bit of the register can be associated with a type of event that
> 	can contribute to raising the interrupt."
> 

Does this mean that there are 32 different signals and they are all ORed
into the same interrupt line to trigger software action when something
happens?

Or does it mean that this register is used to pass multi-bit information
and when any such information is passed an interrupt will be triggered?
If so, what does that information mean? How is it tied into other Linux
drivers/subsystems?

> On few other platforms, like qcom, similar doorbell mechanism is present
> with separate interrupt for each of the bits (that's how I understood
> it), while in case of ARM MHU, there is a single interrupt line for all
> the 32 bits. Also in case of ARM MHU, these registers and interrupts
> have 3 copies for different priority levels, i.e. low priority
> non-secure, high priority non-secure and secure channels.
> 

In the Qualcomm case we have 32 bits in a register where each bit
written will trigger an interrupt elsewhere in the SoC, as such the
mailbox is purely write only and the "read" side is handled by some
interrupt-controller.

The three copies just sounds like 3 (or 3 * 32) different mailbox
channels.

> For ARM MHU, both the dt bindings and the Linux driver support 3
> channels for the different priorities right now and support sending a 32
> bit data on every transfer in a locked fashion, i.e. only one transfer
> can be done at once and the other have to wait for it to finish first.
> 
> Here are the point of view of the parties involved on this subject:
> 
> Jassi's viewpoint:
> 
> - Virtualization of channels should be discouraged in software based on
>   specific usecases of the application. This may invite other mailbox
>   driver authors to ask for doing virtualization in their drivers.
> 
> - In mailbox's terminology, every channel is equivalent to a signal,
>   since there is only one signal generated here by the MHU, there should
>   be only one channel per priority level.
> 
> - The clients should send data (of just setting 1 bit or many in the 32
>   bit word) using the existing mechanism as the delays due to
>   serialization shouldn't be significant anyway.
> 
> - The driver supports 90% of the users with the current implementation
>   and it shouldn't be extended to support doorbell and implement two
>   different modes by changing value of #mbox-cells field in bindings.

I interpret Jassi's view as this is a channel that passes 32 bit
"messages".

> 
> Sudeep (ARM) and myself as well to some extent:
> 
> - The hardware gives us the capability to write the register in
>   parallel, i.e. we can write 0x800 and 0x400 together without any
>   software locks, and so these 32 bits should be considered as separate
>   channel even if only one interrupt is issued by the hardware finally.
>   This shouldn't be called as virtualization of the channels, as the
>   hardware supports this (as clearly mentioned in the TRM) and it takes
>   care of handling the signal properly.
> 

But if writes to the register is ORed together than it seems like the
hardware isn't supposed to pass multi-bit messages, but instead is
supposed to carry 32 individual signals - somewhat similar to the
Qualcomm block.

I don't see a problem with having a cascaded IRQ handler for incoming
notifications.

> - With serialization, if we use only one channel as today at every
>   priority, if there are 5 requests to send signal to the receiver and
>   the dvfs request is the last one in queue (which may be called from
>   scheduler's hot path with fast switching), it unnecessarily needs to
>   wait for the first four transfers to finish due to the software
>   locking imposed by the mailbox framework. This adds additional delay,
>   maybe of few ms only, which isn't required by the hardware but just by
>   the software and few ms can be important in scheduler's hotpath.
> 

So these 5 requests, are they conveyed by the signals [1,2,3,4,5] or
[BIT(0), BIT(1), BIT(2), BIT(3), BIT(4)]?

In the first case you have to serialize things given that e.g. signal 1
immediately followed by 2 is indistinguishable from 3.

If you signals are single-bit notifications then you don't need any
serialization.

Regards,
Bjorn

> - With the current approach it isn't possible to assign different bits
>   (or doorbell numbers) to clients from DT and the only way of doing
>   that without adding new bindings is by extending #mbox-cells to accept
>   a value of 2 as done in this patch.
> 
> Jassi and Sudeep, I hope I was able to represent both the view points
> properly here. Please correct me if I have made a mistake here.
> 
> This is it. It would be nice to get the views of everyone now on this
> and how should this be handled.
> 
> Thanks.
> 
> [1] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0515f/DDI0515F_juno_arm_development_platform_soc_trm.pdf , section 3.4.4, page 3-38.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  .../devicetree/bindings/mailbox/arm-mhu.txt   | 39 ++++++++++++++++++-
>  1 file changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt
> index 4971f03f0b33..ba659bcc7109 100644
> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt
> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt
> @@ -10,6 +10,15 @@ STAT register and the remote clears it after having read the data.
>  The last channel is specified to be a 'Secure' resource, hence can't be
>  used by Linux running NS.
>  
> +The MHU drives the interrupt signal using a 32-bit register, with all
> +32-bits logically ORed together. It provides a set of registers to
> +enable software to set, clear and check the status of each of the bits
> +of this register independently. The use of 32 bits per interrupt line
> +enables software to provide more information about the source of the
> +interrupt. For example, each bit of the register can be associated with
> +a type of event that can contribute to raising the interrupt. Each of
> +the 32-bits can be used as "doorbell" to alert the remote processor.
> +
>  Mailbox Device Node:
>  ====================
>  
> @@ -18,13 +27,21 @@ used by Linux running NS.
>  - compatible:		Shall be "arm,mhu" & "arm,primecell"
>  - reg:			Contains the mailbox register address range (base
>  			address and length)
> -- #mbox-cells		Shall be 1 - the index of the channel needed.
> +- #mbox-cells		Shall be 1 - the index of the channel needed when
> +			not used as set of doorbell bits.
> +			Shall be 2 - the index of the channel needed, and
> +			the index of the doorbell bit within the channel
> +			when used in doorbell mode.
>  - interrupts:		Contains the interrupt information corresponding to
> -			each of the 3 links of MHU.
> +			each of the 3 physical channels of MHU namely low
> +			priority non-secure, high priority non-secure and
> +			secure channels.
>  
>  Example:
>  --------
>  
> +1. Controller which doesn't support doorbells
> +
>  	mhu: mailbox@2b1f0000 {
>  		#mbox-cells = <1>;
>  		compatible = "arm,mhu", "arm,primecell";
> @@ -41,3 +58,21 @@ used by Linux running NS.
>  		reg = <0 0x2e000000 0x4000>;
>  		mboxes = <&mhu 1>; /* HP-NonSecure */
>  	};
> +
> +2. Controller which supports doorbells
> +
> +	mhu: mailbox@2b1f0000 {
> +		#mbox-cells = <2>;
> +		compatible = "arm,mhu", "arm,primecell";
> +		reg = <0 0x2b1f0000 0x1000>;
> +		interrupts = <0 36 4>, /* LP-NonSecure */
> +			     <0 35 4>; /* HP-NonSecure */
> +		clocks = <&clock 0 2 1>;
> +		clock-names = "apb_pclk";
> +	};
> +
> +	mhu_client: scb@2e000000 {
> +		compatible = "arm,scpi";
> +		reg = <0 0x2e000000 0x200>;
> +		mboxes = <&mhu 1 4>; /* HP-NonSecure 5th doorbell bit */
> +	};
> -- 
> 2.25.0.rc1.19.g042ed3e048af
> 

^ permalink raw reply

* Re: [RFC] dt-bindings: mailbox: add doorbell support to ARM MHU
From: Jassi Brar @ 2020-05-19  0:53 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Arnd Bergmann, Rob Herring, Frank Rowand, Bjorn Andersson,
	Vincent Guittot, linux-arm-kernel, Sudeep Holla, Devicetree List,
	Linux Kernel Mailing List
In-Reply-To: <20200518073514.tjodf6qxg3wjzyb4@vireshk-i7>

On Mon, May 18, 2020 at 2:42 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 15-05-20, 11:46, Jassi Brar wrote:
> > As I asked you yesterday over the call, it may help if you could share
> > some numbers to back up the doomsday scenario.
>
> Yes, I have already asked Sudeep to get some numbers for this. He will
> get back to us.
>
Thanks, current bottleneck numbers and the patch/changes to improve
that, would help.

> > > - With the current approach it isn't possible to assign different bits
> > >   (or doorbell numbers) to clients from DT and the only way of doing
> > >   that without adding new bindings is by extending #mbox-cells to accept
> > >   a value of 2 as done in this patch.
> > >
> > I am afraid you are confused. You can use bit/doorbell-6 by passing
> > 0x40 to mhu as the data to send.
>
> That's how the code will do it, right I agree. What I was asking was
> the way this information is passed from DT.
>
That is a client/protocol property and has nothing to do with the
controller dt node.

cheers!

^ permalink raw reply

* Re: [PATCH v2 2/2] arm64: dts: rockchip: rk3399: fix pd_tcpc0 and pd_tcpc1 node position
From: Caesar Wang @ 2020-05-19  0:34 UTC (permalink / raw)
  To: Heiko Stübner, Johan Jonker, kever.yang
  Cc: robh+dt, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel
In-Reply-To: <2978296.PvbIKR7951@diego>

Hi Heiko,

在 2020/5/19 上午6:29, Heiko Stübner 写道:
> Hi Kever, Caesar,
>
> could you double check where the type-c power-domains are located
> in the power-tree, as Caesar did add them under pd_vio back in 2016.

Johan's patch is correct, the pd_tcpc0 and pd_tcpc1 are grouped by VDD_LOGIC.
I have a passed test for pd_vio without pd_tcpc*.

>
> Thanks
> Heiko
>
> Am Dienstag, 28. April 2020, 22:30:03 CEST schrieb Johan Jonker:
>> The pd_tcpc0 and pd_tcpc1 nodes are currently a sub node of pd_vio.
>> In the rk3399 TRM figure of the 'Power Domain Partition' and in the
>> table of 'Power Domain and Voltage Domain Summary' these power domains
>> are positioned directly under VD_LOGIC, so fix that in 'rk3399.dtsi'.
>>
>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>


Reviewed-by: Caesar Wang <wxt@rock-chips.com>

Thanks,

-Caesar

>> ---
>>   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 20 ++++++++++----------
>>   1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index 37279db53..a4dc1bf2e 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -1056,6 +1056,16 @@
>>   				clocks = <&cru HCLK_SDIO>;
>>   				pm_qos = <&qos_sdioaudio>;
>>   			};
>> +			pd_tcpc0@RK3399_PD_TCPD0 {
>> +				reg = <RK3399_PD_TCPD0>;
>> +				clocks = <&cru SCLK_UPHY0_TCPDCORE>,
>> +					 <&cru SCLK_UPHY0_TCPDPHY_REF>;
>> +			};
>> +			pd_tcpc1@RK3399_PD_TCPD1 {
>> +				reg = <RK3399_PD_TCPD1>;
>> +				clocks = <&cru SCLK_UPHY1_TCPDCORE>,
>> +					 <&cru SCLK_UPHY1_TCPDPHY_REF>;
>> +			};
>>   			pd_usb3@RK3399_PD_USB3 {
>>   				reg = <RK3399_PD_USB3>;
>>   				clocks = <&cru ACLK_USB3>;
>> @@ -1088,16 +1098,6 @@
>>   					pm_qos = <&qos_isp1_m0>,
>>   						 <&qos_isp1_m1>;
>>   				};
>> -				pd_tcpc0@RK3399_PD_TCPD0 {
>> -					reg = <RK3399_PD_TCPD0>;
>> -					clocks = <&cru SCLK_UPHY0_TCPDCORE>,
>> -						 <&cru SCLK_UPHY0_TCPDPHY_REF>;
>> -				};
>> -				pd_tcpc1@RK3399_PD_TCPD1 {
>> -					reg = <RK3399_PD_TCPD1>;
>> -					clocks = <&cru SCLK_UPHY1_TCPDCORE>,
>> -						 <&cru SCLK_UPHY1_TCPDPHY_REF>;
>> -				};
>>   				pd_vo@RK3399_PD_VO {
>>   					reg = <RK3399_PD_VO>;
>>   					#address-cells = <1>;
>>
>
>
>
>
>



^ permalink raw reply

* Re: [PATCH 2/4] clk/soc: mediatek: mt6797: Bind clock driver from platform device
From: Chun-Kuang Hu @ 2020-05-19  0:23 UTC (permalink / raw)
  To: matthias.bgg
  Cc: Rob Herring, Matthias Brugger, Michael Turquette, Stephen Boyd,
	Kate Stewart, devicetree, Greg Kroah-Hartman, linux-kernel,
	Richard Fontana, moderated list:ARM/Mediatek SoC support,
	Linux ARM, mtk01761, Thomas Gleixner, linux-clk, Allison Randal
In-Reply-To: <20200518113156.25009-2-matthias.bgg@kernel.org>

Hi, Matthias:

<matthias.bgg@kernel.org> 於 2020年5月18日 週一 下午7:32寫道:
>
> From: Matthias Brugger <matthias.bgg@gmail.com>
>
> The mmsys driver is now the top level entry point for the multimedia
> system (mmsys), we bind the clock driver by creating a platform device.
> We also bind the MediaTek DRM driver which is not yet implement and
> therefor will errror out for now.
>

Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>

> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---
>
>  drivers/clk/mediatek/clk-mt6797-mm.c | 9 ++-------
>  drivers/soc/mediatek/mtk-mmsys.c     | 8 ++++++++
>  2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mt6797-mm.c b/drivers/clk/mediatek/clk-mt6797-mm.c
> index 8f05653b387d..01fdce287247 100644
> --- a/drivers/clk/mediatek/clk-mt6797-mm.c
> +++ b/drivers/clk/mediatek/clk-mt6797-mm.c
> @@ -92,16 +92,12 @@ static const struct mtk_gate mm_clks[] = {
>                  "clk26m", 3),
>  };
>
> -static const struct of_device_id of_match_clk_mt6797_mm[] = {
> -       { .compatible = "mediatek,mt6797-mmsys", },
> -       {}
> -};
> -
>  static int clk_mt6797_mm_probe(struct platform_device *pdev)
>  {
> +       struct device *dev = &pdev->dev;
> +       struct device_node *node = dev->parent->of_node;
>         struct clk_onecell_data *clk_data;
>         int r;
> -       struct device_node *node = pdev->dev.of_node;
>
>         clk_data = mtk_alloc_clk_data(CLK_MM_NR);
>
> @@ -121,7 +117,6 @@ static struct platform_driver clk_mt6797_mm_drv = {
>         .probe = clk_mt6797_mm_probe,
>         .driver = {
>                 .name = "clk-mt6797-mm",
> -               .of_match_table = of_match_clk_mt6797_mm,
>         },
>  };
>
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index 783c3dd008b2..fee64c8d3020 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -88,6 +88,10 @@ static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
>         .clk_driver = "clk-mt2712-mm",
>  };
>
> +static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
> +       .clk_driver = "clk-mt6797-mm",
> +};
> +
>  static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
>         .clk_driver = "clk-mt8173-mm",
>  };
> @@ -339,6 +343,10 @@ static const struct of_device_id of_match_mtk_mmsys[] = {
>                 .compatible = "mediatek,mt2712-mmsys",
>                 .data = &mt2712_mmsys_driver_data,
>         },
> +       {
> +               .compatible = "mediatek,mt6797-mmsys",
> +               .data = &mt6797_mmsys_driver_data,
> +       },
>         {
>                 .compatible = "mediatek,mt8173-mmsys",
>                 .data = &mt8173_mmsys_driver_data,
> --
> 2.26.2
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox