* Re: [PATCH v4 2/6] dt-bindings: add binding for atmel-usart in SPI mode
From: Rob Herring @ 2018-05-31 1:00 UTC (permalink / raw)
To: Radu Pirea
Cc: broonie, nicolas.ferre, alexandre.belloni, lee.jones,
richard.genoud, mark.rutland, gregkh, linux-spi, linux-arm-kernel,
linux-kernel, devicetree, linux-serial
In-Reply-To: <20180525171941.26766-3-radu.pirea@microchip.com>
On Fri, May 25, 2018 at 08:19:37PM +0300, Radu Pirea wrote:
> This patch moves the bindings for serial from serial/atmel-usart.txt to
> mfd/atmel-usart.txt and adds bindings for USART in SPI mode.
>
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
> .../bindings/{serial => mfd}/atmel-usart.txt | 25 +++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 1/7] dt-bindings: clk: at91: add an I2S mux clock
From: Rob Herring @ 2018-05-31 0:58 UTC (permalink / raw)
To: Codrin Ciubotariu
Cc: linux-arm-kernel, linux-kernel, devicetree, linux-clk, alsa-devel,
nicolas.ferre, boris.brezillon, alexandre.belloni, broonie,
Cristian.Birsan
In-Reply-To: <1527251668-31396-2-git-send-email-codrin.ciubotariu@microchip.com>
On Fri, May 25, 2018 at 03:34:22PM +0300, Codrin Ciubotariu wrote:
> The I2S mux clock can be used to select the I2S input clock. The
> available parents are the peripheral and the generated clocks.
>
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> ---
> .../devicetree/bindings/clock/at91-clock.txt | 34 ++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt
> index 51c259a..1c46b3c 100644
> --- a/Documentation/devicetree/bindings/clock/at91-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
> @@ -90,6 +90,8 @@ Required properties:
> "atmel,sama5d2-clk-audio-pll-pmc"
> at91 audio pll output on AUDIOPLLCLK that feeds the PMC
> and can be used by peripheral clock or generic clock
> + "atmel,sama5d2-clk-i2s-mux":
> + at91 I2S clock source selection
Is this boolean or takes some values. If latter, what are valid values?
>
> Required properties for SCKC node:
> - reg : defines the IO memory reserved for the SCKC.
> @@ -507,3 +509,35 @@ For example:
> atmel,clk-output-range = <0 83000000>;
> };
> };
> +
> +Required properties for I2S mux clocks:
> +- #size-cells : shall be 0 (reg is used to encode I2S bus id).
> +- #address-cells : shall be 1 (reg is used to encode I2S bus id).
> +- name: device tree node describing a specific mux clock.
> + * #clock-cells : from common clock binding; shall be set to 0.
> + * clocks : shall be the mux clock parent phandles; shall be 2 phandles:
> + peripheral and generated clock; the first phandle shall belong to the
> + peripheral clock and the second one shall belong to the generated
> + clock; "clock-indices" property can be user to specify
> + the correct order.
> + * reg: I2S bus id of the corresponding mux clock.
> + e.g. reg = <0>; for i2s0, reg = <1>; for i2s1
> +
> +For example:
> + i2s_clkmux {
What is this a child of?
> + compatible = "atmel,sama5d2-clk-i2s-mux";
> + #address-cells = <1>;
> + #size-cells = <0>;
How do you address this block? My guess is you don't because it is just
part of some other block and you are just creating this node to
instantiate a driver. Just make the node for the actual h/w block a
clock provider and define the clock ids (0 and 1).
> +
> + i2s0muxck: i2s0_muxclk {
> + clocks = <&i2s0_clk>, <&i2s0_gclk>;
> + #clock-cells = <0>;
> + reg = <0>;
> + };
> +
> + i2s1muxck: i2s1_muxclk {
> + clocks = <&i2s1_clk>, <&i2s1_gclk>;
> + #clock-cells = <0>;
> + reg = <1>;
> + };
> + };
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH RESEND V4 6/9] dt-bindings: clock: add imx7ulp clock binding doc
From: Rob Herring @ 2018-05-31 0:53 UTC (permalink / raw)
To: Dong Aisheng
Cc: linux-clk, linux-kernel, linux-arm-kernel, sboyd, mturquette,
shawnguo, Anson.Huang, ping.bai, linux-imx, Mark Rutland,
Stephen Boyd, devicetree
In-Reply-To: <1527234671-31755-7-git-send-email-aisheng.dong@nxp.com>
On Fri, May 25, 2018 at 03:51:08PM +0800, Dong Aisheng wrote:
> i.MX7ULP Clock functions are under joint control of the System
> Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
> modules, and Core Mode Controller (CMC)1 blocks
>
> Note IMX7ULP has two clock domains: M4 and A7. This binding doc
> is only for A7 clock domain.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Anson Huang <Anson.Huang@nxp.com>
> Cc: Bai Ping <ping.bai@nxp.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>
> ---
> ChangeLog:
> v2->v3:
> * no changes
> v1->v2: no changes
> ---
> .../devicetree/bindings/clock/imx7ulp-clock.txt | 62 ++++++++++++
> include/dt-bindings/clock/imx7ulp-clock.h | 105 +++++++++++++++++++++
> 2 files changed, 167 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> create mode 100644 include/dt-bindings/clock/imx7ulp-clock.h
>
> diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> new file mode 100644
> index 0000000..76ea3c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
> @@ -0,0 +1,62 @@
> +* Clock bindings for Freescale i.MX7ULP
> +
> +i.MX7ULP Clock functions are under joint control of the System
> +Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
> +modules, and Core Mode Controller (CMC)1 blocks
> +
> +The clocking scheme provides clear separation between M4 domain
> +and A7 domain. Except for a few clock sources shared between two
> +domains, such as the System Oscillator clock, the Slow IRC (SIRC),
> +and and the Fast IRC clock (FIRCLK), clock sources and clock
> +management are separated and contained within each domain.
> +
> +M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
> +A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
> +
> +Note: this binding doc is only for A7 clock domain.
> +
> +Required properties:
> +
> +- compatible: Should be "fsl,imx7ulp-clock".
> +- reg : Should contain registers location and length for scg1,
> + pcc2 and pcc3.
> +- reg-names: Should contain the according reg names "scg1", "pcc2"
> + and "pcc3".
Sounds like separate blocks. These should each be their own node and
binding.
^ permalink raw reply
* Re: [v5 3/6] dt-bindings: fsl-qdma: Add NXP Layerscpae qDMA controller bindings
From: Rob Herring @ 2018-05-31 0:49 UTC (permalink / raw)
To: Wen He; +Cc: vkoul, dmaengine, devicetree, leoyang.li, jiafei.pan, jiaheng.fan
In-Reply-To: <20180525111920.24498-3-wen.he_1@nxp.com>
On Fri, May 25, 2018 at 07:19:17PM +0800, Wen He wrote:
> Document the devicetree bindings for NXP Layerscape qDMA controller
> which could be found on NXP QorIQ Layerscape SoCs.
>
> Signed-off-by: Wen He <wen.he_1@nxp.com>
> ---
> change in v5:
> - Replace dts node variable 'queues' to 'fsl,queues' that add vendor prefix
>
> change in v4:
> - Rewrite the bindings document that follows generic DMA bindings file
>
> change in v3:
> - no change
>
> change in v2:
> - Remove indentation
> - Add "Should be" before 'fsl,ls1021a-qdma'
> - Replace 'channels' by 'dma-channels'
> - Replace 'qdma@8390000' by 'dma-controller@8390000'
>
> Documentation/devicetree/bindings/dma/fsl-qdma.txt | 41 ++++++++++++++++++++
> 1 files changed, 41 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/dma/fsl-qdma.txt
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v2 3/3] arm64: dts: Update Stingray clock DT nodes
From: Ray Jui @ 2018-05-31 0:36 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
Cc: linux-clk, linux-kernel, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, Pramod Kumar, Ray Jui
In-Reply-To: <1527726980-11723-1-git-send-email-ray.jui@broadcom.com>
From: Pramod Kumar <pramod.kumar@broadcom.com>
Update clock output names in the Stingray clock DT nodes so they match
the binding document and the latest ASIC datasheet. Also add entries
for LCPLL2
Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
.../boot/dts/broadcom/stingray/stingray-clock.dtsi | 26 ++++++++++++++++------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray-clock.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray-clock.dtsi
index 3a4d452..10a106a 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray-clock.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray-clock.dtsi
@@ -52,12 +52,24 @@
reg = <0x0001d104 0x32>,
<0x0001c854 0x4>;
clocks = <&osc>;
- clock-output-names = "genpll0", "clk_125", "clk_scr",
+ clock-output-names = "genpll0", "clk_125m", "clk_scr",
"clk_250", "clk_pcie_axi",
"clk_paxc_axi_x2",
"clk_paxc_axi";
};
+ genpll2: genpll2@1d1ac {
+ #clock-cells = <1>;
+ compatible = "brcm,sr-genpll2";
+ reg = <0x0001d1ac 0x32>,
+ <0x0001c854 0x4>;
+ clocks = <&osc>;
+ clock-output-names = "genpll2", "clk_nic",
+ "clk_ts_500_ref", "clk_125_nitro",
+ "clk_chimp", "clk_nic_flash",
+ "clk_fs";
+ };
+
genpll3: genpll3@1d1e0 {
#clock-cells = <1>;
compatible = "brcm,sr-genpll3";
@@ -75,8 +87,8 @@
<0x0001c854 0x4>;
clocks = <&osc>;
clock-output-names = "genpll4", "clk_ccn",
- "clk_tpiu_pll", "noc_clk",
- "pll_chclk_fs4",
+ "clk_tpiu_pll", "clk_noc",
+ "clk_chclk_fs4",
"clk_bridge_fscpu";
};
@@ -86,8 +98,8 @@
reg = <0x0001d248 0x32>,
<0x0001c870 0x4>;
clocks = <&osc>;
- clock-output-names = "genpll5", "fs4_hf_clk",
- "crypto_ae_clk", "raid_ae_clk";
+ clock-output-names = "genpll5", "clk_fs4_hf",
+ "clk_crypto_ae", "clk_raid_ae";
};
lcpll0: lcpll0@1d0c4 {
@@ -107,9 +119,9 @@
reg = <0x0001d138 0x3c>,
<0x0001c870 0x4>;
clocks = <&osc>;
- clock-output-names = "lcpll1", "clk_wanpn",
+ clock-output-names = "lcpll1", "clk_wan",
"clk_usb_ref",
- "timesync_evt_clk";
+ "clk_crmu_ts";
};
hsls_clk: hsls_clk {
--
2.1.4
^ permalink raw reply related
* [PATCH v2 2/3] clk: bcm: Update and add Stingray clock entries
From: Ray Jui @ 2018-05-31 0:36 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
Cc: linux-clk, linux-kernel, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, Pramod Kumar, Ray Jui
In-Reply-To: <1527726980-11723-1-git-send-email-ray.jui@broadcom.com>
From: Pramod Kumar <pramod.kumar@broadcom.com>
Update and add Stingray clock definitions and tables so they match the
binding document and the latest ASIC datasheet
Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
drivers/clk/bcm/clk-sr.c | 135 ++++++++++++++++++++++++++++++++-----
include/dt-bindings/clock/bcm-sr.h | 24 +++++--
2 files changed, 137 insertions(+), 22 deletions(-)
diff --git a/drivers/clk/bcm/clk-sr.c b/drivers/clk/bcm/clk-sr.c
index adc74f4..7b9efc0 100644
--- a/drivers/clk/bcm/clk-sr.c
+++ b/drivers/clk/bcm/clk-sr.c
@@ -56,8 +56,8 @@ static const struct iproc_pll_ctrl sr_genpll0 = {
};
static const struct iproc_clk_ctrl sr_genpll0_clk[] = {
- [BCM_SR_GENPLL0_SATA_CLK] = {
- .channel = BCM_SR_GENPLL0_SATA_CLK,
+ [BCM_SR_GENPLL0_125M_CLK] = {
+ .channel = BCM_SR_GENPLL0_125M_CLK,
.flags = IPROC_CLK_AON,
.enable = ENABLE_VAL(0x4, 6, 0, 12),
.mdiv = REG_VAL(0x18, 0, 9),
@@ -102,6 +102,65 @@ static int sr_genpll0_clk_init(struct platform_device *pdev)
return 0;
}
+static const struct iproc_pll_ctrl sr_genpll2 = {
+ .flags = IPROC_CLK_AON | IPROC_CLK_PLL_HAS_NDIV_FRAC |
+ IPROC_CLK_PLL_NEEDS_SW_CFG,
+ .aon = AON_VAL(0x0, 1, 13, 12),
+ .reset = RESET_VAL(0x0, 12, 11),
+ .dig_filter = DF_VAL(0x0, 4, 3, 0, 4, 7, 3),
+ .sw_ctrl = SW_CTRL_VAL(0x10, 31),
+ .ndiv_int = REG_VAL(0x10, 20, 10),
+ .ndiv_frac = REG_VAL(0x10, 0, 20),
+ .pdiv = REG_VAL(0x14, 0, 4),
+ .status = REG_VAL(0x30, 12, 1),
+};
+
+static const struct iproc_clk_ctrl sr_genpll2_clk[] = {
+ [BCM_SR_GENPLL2_NIC_CLK] = {
+ .channel = BCM_SR_GENPLL2_NIC_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x4, 6, 0, 12),
+ .mdiv = REG_VAL(0x18, 0, 9),
+ },
+ [BCM_SR_GENPLL2_TS_500_CLK] = {
+ .channel = BCM_SR_GENPLL2_TS_500_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x4, 7, 1, 13),
+ .mdiv = REG_VAL(0x18, 10, 9),
+ },
+ [BCM_SR_GENPLL2_125_NITRO_CLK] = {
+ .channel = BCM_SR_GENPLL2_125_NITRO_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x4, 8, 2, 14),
+ .mdiv = REG_VAL(0x18, 20, 9),
+ },
+ [BCM_SR_GENPLL2_CHIMP_CLK] = {
+ .channel = BCM_SR_GENPLL2_CHIMP_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x4, 9, 3, 15),
+ .mdiv = REG_VAL(0x1c, 0, 9),
+ },
+ [BCM_SR_GENPLL2_NIC_FLASH_CLK] = {
+ .channel = BCM_SR_GENPLL2_NIC_FLASH_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x4, 10, 4, 16),
+ .mdiv = REG_VAL(0x1c, 10, 9),
+ },
+ [BCM_SR_GENPLL2_FS4_CLK] = {
+ .channel = BCM_SR_GENPLL2_FS4_CLK,
+ .enable = ENABLE_VAL(0x4, 11, 5, 17),
+ .mdiv = REG_VAL(0x1c, 20, 9),
+ },
+};
+
+static int sr_genpll2_clk_init(struct platform_device *pdev)
+{
+ iproc_pll_clk_setup(pdev->dev.of_node,
+ &sr_genpll2, NULL, 0, sr_genpll2_clk,
+ ARRAY_SIZE(sr_genpll2_clk));
+ return 0;
+}
+
static const struct iproc_pll_ctrl sr_genpll3 = {
.flags = IPROC_CLK_AON | IPROC_CLK_PLL_HAS_NDIV_FRAC |
IPROC_CLK_PLL_NEEDS_SW_CFG,
@@ -157,6 +216,30 @@ static const struct iproc_clk_ctrl sr_genpll4_clk[] = {
.enable = ENABLE_VAL(0x4, 6, 0, 12),
.mdiv = REG_VAL(0x18, 0, 9),
},
+ [BCM_SR_GENPLL4_TPIU_PLL_CLK] = {
+ .channel = BCM_SR_GENPLL4_TPIU_PLL_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x4, 7, 1, 13),
+ .mdiv = REG_VAL(0x18, 10, 9),
+ },
+ [BCM_SR_GENPLL4_NOC_CLK] = {
+ .channel = BCM_SR_GENPLL4_NOC_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x4, 8, 2, 14),
+ .mdiv = REG_VAL(0x18, 20, 9),
+ },
+ [BCM_SR_GENPLL4_CHCLK_FS4_CLK] = {
+ .channel = BCM_SR_GENPLL4_CHCLK_FS4_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x4, 9, 3, 15),
+ .mdiv = REG_VAL(0x1c, 0, 9),
+ },
+ [BCM_SR_GENPLL4_BRIDGE_FSCPU_CLK] = {
+ .channel = BCM_SR_GENPLL4_BRIDGE_FSCPU_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x4, 10, 4, 16),
+ .mdiv = REG_VAL(0x1c, 10, 9),
+ },
};
static int sr_genpll4_clk_init(struct platform_device *pdev)
@@ -181,18 +264,21 @@ static const struct iproc_pll_ctrl sr_genpll5 = {
};
static const struct iproc_clk_ctrl sr_genpll5_clk[] = {
- [BCM_SR_GENPLL5_FS_CLK] = {
- .channel = BCM_SR_GENPLL5_FS_CLK,
- .flags = IPROC_CLK_AON,
+ [BCM_SR_GENPLL5_FS4_HF_CLK] = {
+ .channel = BCM_SR_GENPLL5_FS4_HF_CLK,
.enable = ENABLE_VAL(0x4, 6, 0, 12),
.mdiv = REG_VAL(0x18, 0, 9),
},
- [BCM_SR_GENPLL5_SPU_CLK] = {
- .channel = BCM_SR_GENPLL5_SPU_CLK,
- .flags = IPROC_CLK_AON,
- .enable = ENABLE_VAL(0x4, 6, 0, 12),
+ [BCM_SR_GENPLL5_CRYPTO_AE_CLK] = {
+ .channel = BCM_SR_GENPLL5_CRYPTO_AE_CLK,
+ .enable = ENABLE_VAL(0x4, 7, 1, 12),
.mdiv = REG_VAL(0x18, 10, 9),
},
+ [BCM_SR_GENPLL5_RAID_AE_CLK] = {
+ .channel = BCM_SR_GENPLL5_RAID_AE_CLK,
+ .enable = ENABLE_VAL(0x4, 8, 2, 14),
+ .mdiv = REG_VAL(0x18, 20, 9),
+ },
};
static int sr_genpll5_clk_init(struct platform_device *pdev)
@@ -214,24 +300,30 @@ static const struct iproc_pll_ctrl sr_lcpll0 = {
};
static const struct iproc_clk_ctrl sr_lcpll0_clk[] = {
- [BCM_SR_LCPLL0_SATA_REF_CLK] = {
- .channel = BCM_SR_LCPLL0_SATA_REF_CLK,
+ [BCM_SR_LCPLL0_SATA_REFP_CLK] = {
+ .channel = BCM_SR_LCPLL0_SATA_REFP_CLK,
.flags = IPROC_CLK_AON,
.enable = ENABLE_VAL(0x0, 7, 1, 13),
.mdiv = REG_VAL(0x14, 0, 9),
},
- [BCM_SR_LCPLL0_USB_REF_CLK] = {
- .channel = BCM_SR_LCPLL0_USB_REF_CLK,
+ [BCM_SR_LCPLL0_SATA_REFN_CLK] = {
+ .channel = BCM_SR_LCPLL0_SATA_REFN_CLK,
.flags = IPROC_CLK_AON,
.enable = ENABLE_VAL(0x0, 8, 2, 14),
.mdiv = REG_VAL(0x14, 10, 9),
},
- [BCM_SR_LCPLL0_SATA_REFPN_CLK] = {
- .channel = BCM_SR_LCPLL0_SATA_REFPN_CLK,
+ [BCM_SR_LCPLL0_SATA_350_CLK] = {
+ .channel = BCM_SR_LCPLL0_SATA_350_CLK,
.flags = IPROC_CLK_AON,
.enable = ENABLE_VAL(0x0, 9, 3, 15),
.mdiv = REG_VAL(0x14, 20, 9),
},
+ [BCM_SR_LCPLL0_SATA_500_CLK] = {
+ .channel = BCM_SR_LCPLL0_SATA_500_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x0, 10, 4, 16),
+ .mdiv = REG_VAL(0x18, 0, 9),
+ },
};
static int sr_lcpll0_clk_init(struct platform_device *pdev)
@@ -259,6 +351,18 @@ static const struct iproc_clk_ctrl sr_lcpll1_clk[] = {
.enable = ENABLE_VAL(0x0, 7, 1, 13),
.mdiv = REG_VAL(0x14, 0, 9),
},
+ [BCM_SR_LCPLL1_USB_REF_CLK] = {
+ .channel = BCM_SR_LCPLL1_USB_REF_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x0, 8, 2, 14),
+ .mdiv = REG_VAL(0x14, 10, 9),
+ },
+ [BCM_SR_LCPLL1_CRMU_TS_CLK] = {
+ .channel = BCM_SR_LCPLL1_CRMU_TS_CLK,
+ .flags = IPROC_CLK_AON,
+ .enable = ENABLE_VAL(0x0, 9, 3, 15),
+ .mdiv = REG_VAL(0x14, 20, 9),
+ },
};
static int sr_lcpll1_clk_init(struct platform_device *pdev)
@@ -298,6 +402,7 @@ static int sr_lcpll_pcie_clk_init(struct platform_device *pdev)
static const struct of_device_id sr_clk_dt_ids[] = {
{ .compatible = "brcm,sr-genpll0", .data = sr_genpll0_clk_init },
+ { .compatible = "brcm,sr-genpll2", .data = sr_genpll2_clk_init },
{ .compatible = "brcm,sr-genpll4", .data = sr_genpll4_clk_init },
{ .compatible = "brcm,sr-genpll5", .data = sr_genpll5_clk_init },
{ .compatible = "brcm,sr-lcpll0", .data = sr_lcpll0_clk_init },
diff --git a/include/dt-bindings/clock/bcm-sr.h b/include/dt-bindings/clock/bcm-sr.h
index cff6c6f..419011b 100644
--- a/include/dt-bindings/clock/bcm-sr.h
+++ b/include/dt-bindings/clock/bcm-sr.h
@@ -35,7 +35,7 @@
/* GENPLL 0 clock channel ID SCR HSLS FS PCIE */
#define BCM_SR_GENPLL0 0
-#define BCM_SR_GENPLL0_SATA_CLK 1
+#define BCM_SR_GENPLL0_125M_CLK 1
#define BCM_SR_GENPLL0_SCR_CLK 2
#define BCM_SR_GENPLL0_250M_CLK 3
#define BCM_SR_GENPLL0_PCIE_AXI_CLK 4
@@ -50,9 +50,11 @@
/* GENPLL 2 clock channel ID NITRO MHB*/
#define BCM_SR_GENPLL2 0
#define BCM_SR_GENPLL2_NIC_CLK 1
-#define BCM_SR_GENPLL2_250_NITRO_CLK 2
+#define BCM_SR_GENPLL2_TS_500_CLK 2
#define BCM_SR_GENPLL2_125_NITRO_CLK 3
#define BCM_SR_GENPLL2_CHIMP_CLK 4
+#define BCM_SR_GENPLL2_NIC_FLASH_CLK 5
+#define BCM_SR_GENPLL2_FS4_CLK 6
/* GENPLL 3 HSLS clock channel ID */
#define BCM_SR_GENPLL3 0
@@ -62,11 +64,16 @@
/* GENPLL 4 SCR clock channel ID */
#define BCM_SR_GENPLL4 0
#define BCM_SR_GENPLL4_CCN_CLK 1
+#define BCM_SR_GENPLL4_TPIU_PLL_CLK 2
+#define BCM_SR_GENPLL4_NOC_CLK 3
+#define BCM_SR_GENPLL4_CHCLK_FS4_CLK 4
+#define BCM_SR_GENPLL4_BRIDGE_FSCPU_CLK 5
/* GENPLL 5 FS4 clock channel ID */
#define BCM_SR_GENPLL5 0
-#define BCM_SR_GENPLL5_FS_CLK 1
-#define BCM_SR_GENPLL5_SPU_CLK 2
+#define BCM_SR_GENPLL5_FS4_HF_CLK 1
+#define BCM_SR_GENPLL5_CRYPTO_AE_CLK 2
+#define BCM_SR_GENPLL5_RAID_AE_CLK 3
/* GENPLL 6 NITRO clock channel ID */
#define BCM_SR_GENPLL6 0
@@ -74,13 +81,16 @@
/* LCPLL0 clock channel ID */
#define BCM_SR_LCPLL0 0
-#define BCM_SR_LCPLL0_SATA_REF_CLK 1
-#define BCM_SR_LCPLL0_USB_REF_CLK 2
-#define BCM_SR_LCPLL0_SATA_REFPN_CLK 3
+#define BCM_SR_LCPLL0_SATA_REFP_CLK 1
+#define BCM_SR_LCPLL0_SATA_REFN_CLK 2
+#define BCM_SR_LCPLL0_SATA_350_CLK 3
+#define BCM_SR_LCPLL0_SATA_500_CLK 4
/* LCPLL1 clock channel ID */
#define BCM_SR_LCPLL1 0
#define BCM_SR_LCPLL1_WAN_CLK 1
+#define BCM_SR_LCPLL1_USB_REF_CLK 2
+#define BCM_SR_LCPLL1_CRMU_TS_CLK 3
/* LCPLL PCIE clock channel ID */
#define BCM_SR_LCPLL_PCIE 0
--
2.1.4
^ permalink raw reply related
* [PATCH v2 1/3] dt-bindings: clk: Update Stingray binding doc
From: Ray Jui @ 2018-05-31 0:36 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
Cc: linux-clk, linux-kernel, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, Pramod Kumar, Ray Jui
In-Reply-To: <1527726980-11723-1-git-send-email-ray.jui@broadcom.com>
From: Pramod Kumar <pramod.kumar@broadcom.com>
Update Stingray clock binding document to add additional clock entries
with names matching the latest ASIC datasheet. Also modify a few existing
entries to make their naming more consistent with the rest of the entries
Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
.../bindings/clock/brcm,iproc-clocks.txt | 26 ++++++++++++----------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
index f8e4a93..ab730ea 100644
--- a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
+++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
@@ -276,36 +276,38 @@ These clock IDs are defined in:
clk_ts_500_ref genpll2 2 BCM_SR_GENPLL2_TS_500_REF_CLK
clk_125_nitro genpll2 3 BCM_SR_GENPLL2_125_NITRO_CLK
clk_chimp genpll2 4 BCM_SR_GENPLL2_CHIMP_CLK
- clk_nic_flash genpll2 5 BCM_SR_GENPLL2_NIC_FLASH
+ clk_nic_flash genpll2 5 BCM_SR_GENPLL2_NIC_FLASH_CLK
+ clk_fs genpll2 6 BCM_SR_GENPLL2_FS_CLK
genpll3 crystal 0 BCM_SR_GENPLL3
clk_hsls genpll3 1 BCM_SR_GENPLL3_HSLS_CLK
clk_sdio genpll3 2 BCM_SR_GENPLL3_SDIO_CLK
genpll4 crystal 0 BCM_SR_GENPLL4
- ccn genpll4 1 BCM_SR_GENPLL4_CCN_CLK
+ clk_ccn genpll4 1 BCM_SR_GENPLL4_CCN_CLK
clk_tpiu_pll genpll4 2 BCM_SR_GENPLL4_TPIU_PLL_CLK
- noc_clk genpll4 3 BCM_SR_GENPLL4_NOC_CLK
+ clk_noc genpll4 3 BCM_SR_GENPLL4_NOC_CLK
clk_chclk_fs4 genpll4 4 BCM_SR_GENPLL4_CHCLK_FS4_CLK
clk_bridge_fscpu genpll4 5 BCM_SR_GENPLL4_BRIDGE_FSCPU_CLK
-
genpll5 crystal 0 BCM_SR_GENPLL5
- fs4_hf_clk genpll5 1 BCM_SR_GENPLL5_FS4_HF_CLK
- crypto_ae_clk genpll5 2 BCM_SR_GENPLL5_CRYPTO_AE_CLK
- raid_ae_clk genpll5 3 BCM_SR_GENPLL5_RAID_AE_CLK
+ clk_fs4_hf genpll5 1 BCM_SR_GENPLL5_FS4_HF_CLK
+ clk_crypto_ae genpll5 2 BCM_SR_GENPLL5_CRYPTO_AE_CLK
+ clk_raid_ae genpll5 3 BCM_SR_GENPLL5_RAID_AE_CLK
genpll6 crystal 0 BCM_SR_GENPLL6
- 48_usb genpll6 1 BCM_SR_GENPLL6_48_USB_CLK
+ clk_48_usb genpll6 1 BCM_SR_GENPLL6_48_USB_CLK
lcpll0 crystal 0 BCM_SR_LCPLL0
clk_sata_refp lcpll0 1 BCM_SR_LCPLL0_SATA_REFP_CLK
clk_sata_refn lcpll0 2 BCM_SR_LCPLL0_SATA_REFN_CLK
- clk_usb_ref lcpll0 3 BCM_SR_LCPLL0_USB_REF_CLK
- sata_refpn lcpll0 3 BCM_SR_LCPLL0_SATA_REFPN_CLK
+ clk_sata_350 lcpll0 3 BCM_SR_LCPLL0_SATA_350_CLK
+ clk_sata_500 lcpll0 4 BCM_SR_LCPLL0_SATA_500_CLK
lcpll1 crystal 0 BCM_SR_LCPLL1
- wan lcpll1 1 BCM_SR_LCPLL0_WAN_CLK
+ clk_wan lcpll1 1 BCM_SR_LCPLL1_WAN_CLK
+ clk_usb_ref lcpll1 2 BCM_SR_LCPLL1_USB_REF_CLK
+ clk_crmu_ts lcpll1 3 BCM_SR_LCPLL1_CRMU_TS_CLK
lcpll_pcie crystal 0 BCM_SR_LCPLL_PCIE
- pcie_phy_ref lcpll1 1 BCM_SR_LCPLL_PCIE_PHY_REF_CLK
+ clk_pcie_phy_ref lcpll1 1 BCM_SR_LCPLL_PCIE_PHY_REF_CLK
--
2.1.4
^ permalink raw reply related
* [PATCH v2 0/3] Update Broadcom Stingray clock entries
From: Ray Jui @ 2018-05-31 0:36 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
Cc: linux-clk, linux-kernel, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, Pramod Kumar, Ray Jui
This patch series updates Broadcom Stingray clock entries so they match the
latest ASIC datasheet
This patch series is based off v4.17-rc5 and is available on GIHUB:
repo: https://github.com/Broadcom/arm64-linux.git
branch: sr-clk-v2
Changes since v1:
- Fix patch author to Pramod Kumar on all 3 patches
- Fix patch subject spelling error on patch 2/3
Pramod Kumar (3):
dt-bindings: clk: Update Stingray binding doc
clk: bcm: Update and add Stingray clock entries
arm64: dts: Update Stingray clock DT nodes
.../bindings/clock/brcm,iproc-clocks.txt | 26 ++--
.../boot/dts/broadcom/stingray/stingray-clock.dtsi | 26 ++--
drivers/clk/bcm/clk-sr.c | 135 ++++++++++++++++++---
include/dt-bindings/clock/bcm-sr.h | 24 ++--
4 files changed, 170 insertions(+), 41 deletions(-)
--
2.1.4
^ permalink raw reply
* Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-05-31 0:34 UTC (permalink / raw)
To: David Collins
Cc: Mark Rutland, devicetree, Rajendra Nayak, Stephen Boyd,
linux-arm-msm, Liam Girdwood, Rob Herring, LKML, Mark Brown,
Linux ARM
In-Reply-To: <75088820-f20d-32ac-780a-5e7dacdf20ff@codeaurora.org>
Hi,
On Wed, May 30, 2018 at 4:39 PM, David Collins <collinsd@codeaurora.org> wrote:
> Consider the case of a regulator with physical 10 mA LPM max current. Say
> that modem and application processors each have a load on the regulator
> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
> vote for LPM. The VRM block in RPMh hardware will aggregate these requests
> together using a max function which will result in the regulator being set
> to LPM, even though the total load is 18 mA (which would require high
> power mode (HPM)). To get around this corner case, a LPM max current of 1
> uA can be used for all LDO supplies that have non-application processor
> consumers. Thus, any non-zero regulator_set_load() current request will
> result in setting the regulator to HPM (which is always safe).
Is there any plan to change the way this works for future versions of RPMh?
> The second situation that needs board-level DRMS mode and current limit
> specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
> SMPS regulators should theoretically always be able to operate in AUTO
> mode as it switches automatically between PWM mode (which can provide the
> maximum current) and PFM mode (which supports lower current but has higher
> efficiency). However, there may be board/system issues that require
> switching to PWM mode for certain use cases as it has better load
> regulation (i.e. no PFM ripple for lower loads) and supports more
> aggressive load current steps (i.e. greater A/ns).
>
> If a Linux consumer requires the ability to force a given SMPS regulator
> from AUTO mode into PWM mode and that SMPS is shared by other Linux
> consumers (which may be the case, but at least must be guaranteed to work
> architecturally), then regulator_set_load() is the only option since it
> provides aggregation, where as regulator_set_mode() does not.
> regulator_set_load() can be utilized in this case by specifying AUTO mode
> and PWM mode as drms modes and specifying some particular AUTO mode
> maximum current (that is known by the consumer) in device tree. The
> consumer can then call regulator_set_load() with the imposed AUTO mode
> limit + delta when PWM mode is required and a lower value when AUTO mode
> is sufficient.
Mark: I'm leaving this firmly in your hands. I can see David's points
here. I could even believe that some of this stuff could be board
specific where one board might have slightly different capacitors or
they might be placed differently and might need a higher power mode to
keep the signal clean.
-Doug
^ permalink raw reply
* Re: [PATCH 2/3] clk: bcm: Update and add tingray clock entries
From: Ray Jui @ 2018-05-31 0:23 UTC (permalink / raw)
To: Stephen Boyd, Mark Rutland, Michael Turquette, Rob Herring
Cc: linux-clk, linux-kernel, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, Pramod Kumar
In-Reply-To: <152772366322.144038.12289577935853704907@swboyd.mtv.corp.google.com>
Hi Stephen,
On 5/30/2018 4:41 PM, Stephen Boyd wrote:
> Subject is missing an 's' on stringray?
>
Yes, will fix this.
> Quoting Ray Jui (2018-05-25 09:45:16)
>> Update and add Stingray clock definitions and tables so they match the
>> binding document and the latest ASIC datasheet
>>
>> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>
> Otherwise it looks ok, but maybe Pramod should be the author?
>
Will fix this for all commits in this series.
I'll fix the above and send out v2.
Thanks,
Ray
^ permalink raw reply
* Re: [PATCH v6 1/2] dt-bindings: pinctrl: qcom: add gpio-ranges, gpio-reserved-ranges
From: Rob Herring @ 2018-05-31 0:14 UTC (permalink / raw)
To: Christian Lamparter
Cc: Mark Rutland, devicetree, David Brown, Stephen Boyd,
linux-arm-msm, Linus Walleij, Bjorn Andersson, linux-gpio,
Sven Eckelmann, Andy Gross, linux-arm-kernel
In-Reply-To: <6dbaadee4bb22638a2c2e6433e8d1740884ecfd9.1527505307.git.chunkeey@gmail.com>
On Mon, May 28, 2018 at 01:06:01PM +0200, Christian Lamparter wrote:
> This patch adds the gpio-ranges and gpio-reserved-ranges property
> definitions to the binding text files supported by the pinctrl-msm
> driver framework.
>
> gpio-ranges:
> For DT-based platforms the pinctrl-msm framework currently relies
> on the deprecated-for-DT gpiochip_add_pin_range() function to add
> the range of GPIOs to be handled by the pin controller. Due to
> interactions within gpiolib code, this causes the pinctrl-msm
> driver to bail out (-517) during boot when a gpio-hog is declared.
> This can be fatal and cause the system to not boot or reset
> (for a detailed explanation and call-trace, refer to patch:
> "pinctrl: msm: fix gpio-hog related boot issues" in this series).
>
> gpio-reserved-ranges:
> The binding has been added as a precaution since the TrustZone
> firmware (aka QSEE), which is running as the hypervisor, might
> have reserved certain, but undisclosed pins. Hence reading or
> writing to the registers for those pins will cause an
> XPU violation and this subsequently crashes the kernel.
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
> .../devicetree/bindings/pinctrl/qcom,apq8064-pinctrl.txt | 6 ++++++
> .../devicetree/bindings/pinctrl/qcom,apq8084-pinctrl.txt | 9 +++++++++
> .../devicetree/bindings/pinctrl/qcom,ipq4019-pinctrl.txt | 6 ++++++
> .../devicetree/bindings/pinctrl/qcom,ipq8064-pinctrl.txt | 6 ++++++
> .../devicetree/bindings/pinctrl/qcom,ipq8074-pinctrl.txt | 9 +++++++++
> .../devicetree/bindings/pinctrl/qcom,mdm9615-pinctrl.txt | 9 +++++++++
> .../devicetree/bindings/pinctrl/qcom,msm8660-pinctrl.txt | 6 ++++++
> .../devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt | 9 +++++++++
> .../devicetree/bindings/pinctrl/qcom,msm8960-pinctrl.txt | 9 +++++++++
> .../devicetree/bindings/pinctrl/qcom,msm8974-pinctrl.txt | 6 ++++++
> .../devicetree/bindings/pinctrl/qcom,msm8994-pinctrl.txt | 9 +++++++++
> .../devicetree/bindings/pinctrl/qcom,msm8996-pinctrl.txt | 9 +++++++++
> 12 files changed, 93 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v6 01/15] dt-bindings: connector: add properties for typec
From: Rob Herring @ 2018-05-31 0:13 UTC (permalink / raw)
To: Li Jun
Cc: gregkh, heikki.krogerus, linux, cw00.choi, a.hajda, shufan_lee,
peter.chen, garsilva, gsomlo, linux-usb, devicetree, linux-imx
In-Reply-To: <1527475967-15201-2-git-send-email-jun.li@nxp.com>
On Mon, May 28, 2018 at 10:52:33AM +0800, Li Jun wrote:
> Add bindings supported by current typec driver, so user can pass
> all those properties via dt.
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
> .../bindings/connector/usb-connector.txt | 44 +++++++++++++++
> include/dt-bindings/usb/pd.h | 62 ++++++++++++++++++++++
> 2 files changed, 106 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 0/2] regulator: add QCOM RPMh regulator driver
From: David Collins @ 2018-05-31 0:11 UTC (permalink / raw)
To: Mark Brown
Cc: lgirdwood, robh+dt, mark.rutland, linux-arm-msm, linux-arm-kernel,
devicetree, linux-kernel, rnayak, sboyd, dianders
In-Reply-To: <20180530163343.GV6920@sirena.org.uk>
Hello Mark,
On 05/30/2018 09:33 AM, Mark Brown wrote:
> On Tue, May 22, 2018 at 07:43:16PM -0700, David Collins wrote:
>> This patch series adds a driver and device tree binding documentation for
>> PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
>> Qualcomm Technologies, Inc. SoCs such as SDM845. RPMh is a hardware block
>
> So, this is a very big driver and obviously it being RPM based it
> doesn't look like other regulators which is causing problems, especially
> when coupled with the desire to implement a bunch of more exotic
> features like the mode setting. I think this review is going to go a
> lot more smoothly if you split this up into a base driver with just
> normal, standard stuff that doesn't add too many custom properties or
> unusual ways of working and then a series of patches on top of that
> adding things like the mode adjustment and interaction with other RPM
> clients.
>
> We've got other RPM based regulators in tree already so the baseline bit
> shouldn't be too hard, that'll make the rest of the patches much smaller
> and easier to review and mean that the bits that are simpler and easier
> to cope with don't need to be reposted.
I'll split up the patches so that reviewing is easier. For the base
patch, would you prefer that I remove *all* mode support (handled by
generic regulator framework DT properties) or only remove the special
purpose drms mode handling support (i.e. qcom,regulator-drms-modes and
qcom,drms-mode-max-microamps)?
Thanks,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v10 2/5] dt-bindings: scsi: ufs: add document for hisi-ufs
From: Rob Herring @ 2018-05-31 0:10 UTC (permalink / raw)
To: Li Wei
Cc: mark.rutland, catalin.marinas, will.deacon, vinholikatti, jejb,
martin.petersen, khilman, arnd, gregory.clement, thomas.petazzoni,
yamada.masahiro, riku.voipio, treding, krzk, devicetree,
linux-kernel, linux-arm-kernel, linux-scsi, zangleigang,
gengjianfeng, guodong.xu, puck.chen, john.stultz, fengbaopeng
In-Reply-To: <20180525091712.37227-3-liwei213@huawei.com>
On Fri, May 25, 2018 at 05:17:09PM +0800, Li Wei wrote:
> add ufs node document for Hisilicon.
>
> Signed-off-by: Li Wei <liwei213@huawei.com>
> ---
> Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 41 ++++++++++++++++++++++
> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 10 ++++--
> 2 files changed, 48 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 2/2] regulator: add QCOM RPMh regulator driver
From: David Collins @ 2018-05-30 23:58 UTC (permalink / raw)
To: Doug Anderson
Cc: Mark Brown, Liam Girdwood, Rob Herring, Mark Rutland,
linux-arm-msm, Linux ARM, devicetree, LKML, Rajendra Nayak,
Stephen Boyd
In-Reply-To: <CAD=FV=WF+onhAAWrbmwcHiWyCjuZ=6vvnhodUsia=Ps-da6_4A@mail.gmail.com>
Hello Doug,
On 05/29/2018 10:32 PM, Doug Anderson wrote:
> On Tue, May 22, 2018 at 7:43 PM, David Collins <collinsd@codeaurora.org> wrote:
>> + * @ever_enabled: Boolean indicating that the regulator has been
>> + * explicitly enabled at least once. Voltage
>> + * requests should be cached when this flag is not
>> + * set.
>
> Do you really need this extra boolean? Can't you just check if
> "enabled" is still "-EINVAL"? If it is then you don't pass the
> voltage along.
>
> ...this would mean that you'd also need to send the voltage vote when
> the regulator core tries to disable unused regulators at the end of
> bootup, but that should be OK right? If we never touched a regulator
> anywhere at probe time and we're about to vote to disable it, we know
> there's nobody requiring it to still be on. We can vote for the
> voltage now without fear of messing up a vote that the BIOS left in
> place.
>
> In theory this should also allow you to assert your vote about the
> voltage of a regulator that has never been enabled, which (if I
> understand correctly) you consider to be a feature.
Removing 'ever_enabled' and caching the voltage when 'enabled == -EINVAL'
seems workable. I'm a little concerned about this resulting in voltage =
regulator-min-microvolt requests being sent for all regulators that are
not explicitly enabled by Linux consumers before late_initcall_sync().
Theoretically all of the boot loader hand-off cases should be taken care
of by this point so it should be safe.
I'll make this change.
Take care,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 2/3] clk: bcm: Update and add tingray clock entries
From: Stephen Boyd @ 2018-05-30 23:41 UTC (permalink / raw)
To: Mark Rutland, Michael Turquette, Rob Herring
Cc: linux-clk, linux-kernel, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel, Pramod Kumar, Ray Jui
In-Reply-To: <1527266717-8406-3-git-send-email-ray.jui@broadcom.com>
Subject is missing an 's' on stringray?
Quoting Ray Jui (2018-05-25 09:45:16)
> Update and add Stingray clock definitions and tables so they match the
> binding document and the latest ASIC datasheet
>
> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Otherwise it looks ok, but maybe Pramod should be the author?
^ permalink raw reply
* Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: David Collins @ 2018-05-30 23:39 UTC (permalink / raw)
To: Doug Anderson, Mark Brown
Cc: Liam Girdwood, Rob Herring, Mark Rutland, linux-arm-msm,
Linux ARM, devicetree, LKML, Rajendra Nayak, Stephen Boyd
In-Reply-To: <CAD=FV=WL9enzYCoSx0fT_ny40ciLJU-hhS9joJ6nySXvWPqAxQ@mail.gmail.com>
Hello Mark and Doug,
On 05/30/2018 09:24 AM, Doug Anderson wrote:
> On Wed, May 30, 2018 at 9:20 AM, Mark Brown <broonie@kernel.org> wrote:
>> On Wed, May 30, 2018 at 09:12:25AM -0700, Doug Anderson wrote:
>>> On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:
>>
>>>> No, I'm saying that I don't know why that property exists at all. This
>>>> sounds like it's intended to be the amount of current the regulator can
>>>> deliver in each mode which is normally a design property of the silicon.
>>
>>> Ah, got it. So the whole point here is to be able to implement either
>>> the function "set_load" or the function "get_optimum_mode". We need
>>> some sort of table to convert from current to mode. That's what this
>>> table does.
>>
>> We do need that table, my expectation would be that this table would be
>> in the driver as it's not something I'd expect to vary between different
>> systems but rather be a property of the silicon design. No sense in
>> every single board having to copy it in.
>
> Ah, got it! I'd be OK with it being hardcoded in the driver.
>
> At one point I think David was making the argument that some boards
> have different noise needs for the rails and thus you might want to
> change modes at different currents. I don't know if this is realistic
> but I believe it was part of his original argument for why this needed
> to be specified. If we can hardcode this in the driver I'm fine with
> it... That would actually solve many of my objections too...
The DRMS modes to use and max allowed current per mode need to be
specified at the board level in device tree instead of hard-coded per
regulator type in the driver. There are at least two use cases driving
this need: LDOs shared between RPMh client processors and SMPSes requiring
PWM mode in certain circumstances.
For LDOs the maximum low power mode (LPM) current is typically 10 mA or 30
mA (depending upon subtype) per hardware documentation. Unfortunately,
sharing control of regulators with other processors adds some subtlety to
the LPM current limit that should actually be applied at runtime.
Consider the case of a regulator with physical 10 mA LPM max current. Say
that modem and application processors each have a load on the regulator
that draws 9 mA. If they each respect the 10 mA limit, then they'd each
vote for LPM. The VRM block in RPMh hardware will aggregate these requests
together using a max function which will result in the regulator being set
to LPM, even though the total load is 18 mA (which would require high
power mode (HPM)). To get around this corner case, a LPM max current of 1
uA can be used for all LDO supplies that have non-application processor
consumers. Thus, any non-zero regulator_set_load() current request will
result in setting the regulator to HPM (which is always safe).
The second situation that needs board-level DRMS mode and current limit
specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
SMPS regulators should theoretically always be able to operate in AUTO
mode as it switches automatically between PWM mode (which can provide the
maximum current) and PFM mode (which supports lower current but has higher
efficiency). However, there may be board/system issues that require
switching to PWM mode for certain use cases as it has better load
regulation (i.e. no PFM ripple for lower loads) and supports more
aggressive load current steps (i.e. greater A/ns).
If a Linux consumer requires the ability to force a given SMPS regulator
from AUTO mode into PWM mode and that SMPS is shared by other Linux
consumers (which may be the case, but at least must be guaranteed to work
architecturally), then regulator_set_load() is the only option since it
provides aggregation, where as regulator_set_mode() does not.
regulator_set_load() can be utilized in this case by specifying AUTO mode
and PWM mode as drms modes and specifying some particular AUTO mode
maximum current (that is known by the consumer) in device tree. The
consumer can then call regulator_set_load() with the imposed AUTO mode
limit + delta when PWM mode is required and a lower value when AUTO mode
is sufficient.
Note that I previously mentioned the need for board-level drms mode and
current limit specification in [1].
Take care,
David
[1]: https://lkml.org/lkml/2018/3/22/802
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v8 0/7] i2c: Add FSI-attached I2C master algorithm
From: Benjamin Herrenschmidt @ 2018-05-30 22:42 UTC (permalink / raw)
To: Andy Shevchenko, Eddie James
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Joel Stanley, Mark Rutland, Greg Kroah-Hartman,
Randy Dunlap
In-Reply-To: <CAHp75VerhOMCTPGz5b8wUNL611AdT1iutRm-wrHfzzT+_GbvWA@mail.gmail.com>
On Thu, 2018-05-31 at 00:31 +0300, Andy Shevchenko wrote:
> On Thu, May 31, 2018 at 12:07 AM, Eddie James
> <eajames@linux.vnet.ibm.com> wrote:
> > This series adds an algorithm for an I2C master physically located on an FSI
> > slave device. The I2C master has multiple ports, each of which may be connected
> > to an I2C slave. Access to the I2C master registers is achieved over FSI bus.
> >
> > Due to the multi-port nature of the I2C master, the driver instantiates a new
> > I2C adapter for each port connected to a slave. The connected ports should be
> > defined in the device tree under the I2C master device.
>
> I'll comment the series later, though you have to address previous
> comments first:
> - understand devm_ purpose and how it works
I think it is perfectly understood and I don't see what your problem
here is. So please be a proper civil human being an express your
concern precisely rather than with aggressive comments.
Now to clarify that specific point, devm purpose is to automatically
clean up the resources used by the device when it is torn down.
However, in this specific case, it makes sense to dispose of the port
structure explicitly because this is a failure in registering an
individual port which doesn't lead to a failure of the entire driver.
Thus not freeing it means the structure would remain allocated
uselessly until the whole driver is torn down.
> - understand list_for_each*() macros
This is a genuine misunderstanding of Eddies, I'm sure he will address
it, your tone however isn't appropriate.
> - discuss with maintainer a design of enumerating ports
I've been at that game for at least a good 2 decades. Maintainers
generally do *not* discuss design until a patch is proposed. I even
still try every now and then, maintainers are like lawyers, they don't
want to tell you what to do in case they still want to reject it after
seeing it later :-) I know I've been one of them for long enough.
If you have specific issues with how this is done, please express them
clearly. It's quite possible that there's some better way to do what
Eddie is doing here, but without *construtive* feedback this is
pointless.
> - ... (whatever I forgot and others added) ...
I believe he addressed most of your comments. There is one mistake left
(the list_for_each) which will be easily fixed. As for the rest, it's
just ramblings unless you or Wolfram can propose a better alternative
in which case I'm sure Eddie will be happy to implement it.
I'm disappointed here because we have an example of somebody rather new
producing what is overall pretty damn good code, despite a few corner
issues, and being (again) treated like crap.
This isn't the right way to operate, and I believe this has been made
clear many times before.
Cheers,
Ben.
> > Changes since v7
> > - Fix grammer in Kconfig (a -> an)
> > - Change I2C registers to use BIT and GENMASK
> > - Remove custom macros and use FIELD_PREP and FIELD_GET
> > - Fix a few unecessary initializations and "return rc" that are always zero
> > - Clean up the read/write fifo functions a bit
> > - Few other clean-up items
> >
> > Changes since v6
> > - Remove spinlock for reset functionality; it's unecessary and doesn't work
> > with the latest FSI core.
> > - Use a mutex instead of a semaphore, and don't wait for timeout to get the
> > lock.
> > - Use usleeps instead of schedule_timeout; it's not worth the overhead when
> > the wait should be very short in between sending the command and receiving
> > the response.
> >
> > Changes since v5
> > - Fix reset functionality and do a reset after every transfer failure
> >
> > Eddie James (7):
> > dt-bindings: i2c: Add FSI-attached I2C master dt binding documentation
> > i2c: Add FSI-attached I2C master algorithm
> > i2c: fsi: Add port structures
> > i2c: fsi: Add abort and hardware reset procedures
> > i2c: fsi: Add transfer implementation
> > i2c: fsi: Add I2C master locking
> > i2c: fsi: Add bus recovery
> >
> > Documentation/devicetree/bindings/i2c/i2c-fsi.txt | 40 ++
> > drivers/i2c/busses/Kconfig | 11 +
> > drivers/i2c/busses/Makefile | 1 +
> > drivers/i2c/busses/i2c-fsi.c | 722 ++++++++++++++++++++++
> > 4 files changed, 774 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/i2c/i2c-fsi.txt
> > create mode 100644 drivers/i2c/busses/i2c-fsi.c
> >
> > --
> > 1.8.3.1
> >
>
>
>
^ permalink raw reply
* Re: [PATCH 00/12] introduce support for early platform drivers
From: Rob Herring @ 2018-05-30 22:36 UTC (permalink / raw)
To: Michael Turquette
Cc: Bartosz Golaszewski, Sekhar Nori, Kevin Hilman, David Lechner,
Stephen Boyd, Arnd Bergmann, Greg Kroah-Hartman, Mark Rutland,
Yoshinori Sato, Rich Felker, Andy Shevchenko, Marc Zyngier,
Rafael J . Wysocki, Peter Rosin, Jiri Slaby, Thomas Gleixner,
Daniel Lezcano, Geert Uytterhoeven, Magnus Damm, Johan Hovold,
Frank
In-Reply-To: <20180530194032.982.41562@harbor.lan>
On Wed, May 30, 2018 at 2:40 PM, Michael Turquette
<mturquette@baylibre.com> wrote:
> Hi Rob,
>
> Quoting Rob Herring (2018-05-14 06:20:57)
>> On Mon, May 14, 2018 at 6:38 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> > 2018-05-11 22:13 GMT+02:00 Rob Herring <robh+dt@kernel.org>:
>> >> On Fri, May 11, 2018 at 11:20 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> >>> This series is a follow-up to the RFC[1] posted a couple days ago.
>> >>>
>> >>> NOTE: this series applies on top of my recent patches[2] that move the previous
>> >>> implementation of early platform devices to arch/sh.
>> >>>
>> >>> Problem:
>> >>>
>> >>> Certain class of devices, such as timers, certain clock drivers and irq chip
>> >>> drivers need to be probed early in the boot sequence. The currently preferred
>> >>> approach is using one of the OF_DECLARE() macros. This however does not create
>> >>> a platform device which has many drawbacks - such as not being able to use
>> >>> devres routines, dev_ log functions or no way of deferring the init OF function
>> >>> if some other resources are missing.
>> >>
>> >> I skimmed though this and it doesn't look horrible (how's that for
>> >> positive feedback? ;) ). But before going into the details, I think
>> >> first there needs to be agreement this is the right direction.
>> >>
>> >> The question does remain though as to whether this class of devices
>> >> should be platform drivers. They can't be modules. They can't be
>> >> hotplugged. Can they be runtime-pm enabled? So the advantage is ...
>> >>
>> >
>> > The main (but not the only) advantage for drivers that can both be
>> > platform drivers and OF_DECLARE drivers is that we get a single entry
>> > point and can reuse code without resorting to checking if (!dev). It
>> > results in more consistent code base. Another big advantage is
>> > consolidation of device tree and machine code for SoC drivers used in
>> > different boards of which some are still using board files and others
>> > are defined in DT (see: DaVinci).
>> >
>> >> I assume that the clock maintainers had some reason to move clocks to
>> >> be platform drivers. It's just not clear to me what that was.
>> >>
>> >>> For drivers that use both platform drivers and OF_DECLARE the situation is even
>> >>> more complicated as the code needs to take into account that there can possibly
>> >>> be no struct device present. For a specific use case that we're having problems
>> >>> with, please refer to the recent DaVinci common-clock conversion patches and
>> >>> the nasty workaround that this problem implies[3].
>> >>
>> >> So devm_kzalloc will work with this solution? Why did we need
>> >> devm_kzalloc in the first place? The clocks can never be removed and
>> >> cleaning up on error paths is kind of pointless. The system would be
>> >> hosed, right?
>> >>
>> >
>> > It depends - not all clocks are necessary for system to boot.
>>
>> That doesn't matter. You have a single driver for all/most of the
>> clocks, so the driver can't be removed.
>
> -ECANOFWORMS
>
> A couple of quick rebuttals, but I imagine we're going to disagree on
> the platform_driver thing as a matter of taste no matter what...
It's really more should the clocksource, clockevents and primary
interrupt controller be drivers. Let's get agreement on that first. If
yes, then it probably does make sense that their dependencies are
drivers too. If not, then making only the dependencies drivers doesn't
seem right to me.
> 1) There should be multiple clk drivers in a properly modeled system.
> Some folks still incorrectly put all clocks in a system into a single
> driver because That's How We Used To Do It, and some systems (simpler
> ones) really only have a single clock generator IP block.
>
> Excepting those two reasons above, we really should have separate
> drivers for clocks controlled by the PMIC, for the (one or more) clock
> generator blocks inside of the AP/SoC, and then even more for the
> drivers that map to IP blocks that have their own clock gens.
I agree those should be separate entities at least. But for a given
h/w block, if you already have to use OF_DECLARE, why would you try to
split that into OF_DECLARE and a driver? what advantage does putting
non-boot essential clocks in a driver or transitioning to a driver get
you?
And I've seen PMIC clocks could be inputs to the SoC's clock
controller(s), so the dependencies get more complicated. Then does the
PMIC driver and its dependencies need to be early drivers too?
> Good examples of the latter are display controllers that generate their
> own PLL and pixel clock. Or MMC controllers that have a
> runtime-programmable clock divider. Examples of these are merged into
> mainline.
But those are drivers of types other than a clock controller that
happen to register some clocks as well. I wasn't saying these cases
can't or shouldn't be part of the driver model. Look at irqchips. We
have some that use the driver model (e.g. every GPIO driver) and some
that don't because there's no need (e.g. GIC).
> 2) Stephen and I wanted clock drivers to actually be represented in the
> driver model. There were these gigantic clock drivers that exclusively
> used CLK_OF_DECLARE and they just sort of floated out there in the
> ether... no representation in sysfs, no struct device to map onto a
> clock controller struct, etc.
>
> I'd be happy to hear why you think that platform_driver is a bad fit for
> a device driver that generally manages memory-mapped system resources
> that are part of the system glue and not really tied to a specific bus.
> Sounds like a good fit to me.
>
> If platform_driver doesn't handle the early boot thing well, that tells
> me that we have a problem to solve in platform_driver, not in the clk
> subsystem or drivers.
Doing things earlier is not the only way to solve the problems.
Perhaps we need to figure out how to start things later. Maybe it's
not feasible here, I don't know.
> 3) Lots of clock controllers should be loadable modules. E.g. i2c clock
> expanders, potentially external PMIC-related drivers, external audio
> codecs, etc.
>
> Again, repeating my point #1 above, just because many platforms have a
> monolithic clock driver does not mean that this is the Right Way to do
> it.
And in those cases, I completely agree they should be part of a driver.
Rob
^ permalink raw reply
* Re: [PATCH v7 3/7] drivers/i2c: Add port structure to FSI algorithm
From: Benjamin Herrenschmidt @ 2018-05-30 22:34 UTC (permalink / raw)
To: Andy Shevchenko, Eddie James
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Joel Stanley, Mark Rutland, Greg Kroah-Hartman,
Edward A. James
In-Reply-To: <CAHp75Vd8M4-mK=Fs_SbSX67NKeNty7vrq0-Y8-GyFdy9u6Q3jA@mail.gmail.com>
On Thu, 2018-05-31 at 00:27 +0300, Andy Shevchenko wrote:
> On Wed, May 30, 2018 at 6:47 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> > On 05/29/2018 06:19 PM, Andy Shevchenko wrote:
> > > On Wed, May 30, 2018 at 1:24 AM, Eddie James <eajames@linux.vnet.ibm.com>
> > > wrote:
> > > > static int fsi_i2c_probe(struct device *dev)
> > > > {
> > >
> > > Isn't below somehow repeats of_i2c_register_devices() ?
> > > Why not to use it?
> >
> >
> > Because I need to assign all these port structure fields. Also looks like
> > of_i2c_register_devices creates new devices; I just want an adapter for each
> > port.
>
> Hmm... Wolfram, what is your opinion on this design?
Andy, I don't understand your issue.
of_i2c_register_devices() is about discovering the i2c devices below a
given bus. This is not what is happening here.
This is a driver for a master that supports multiple busses, so it the
above loop creates all the busses.
> > > > + devm_kfree(dev, port);
> > >
> > > This hurts my eyes. Why?!
> > What would you suggest instead?
>
> You even didn't wait for answer, why to ask then?
Please stop being so rude.
> Moreover, you didn't answer to my question. Why are you doing that
> call implicitly?
"implicitly" ? What's implicit here ? This is just pretty standard
cleanup after failure, you are being very cryptic here.
Please state precisely what it is you dislike with that code instead of
expecting us to guess and being nasty about it. Eddie was a genuine
question, he doesn't see what you think is "hurtful to the eyes" in the
code you quoted.
> > > > + if (!list_empty(&i2c->ports)) {
> > >
> > > My gosh, this is done already in list_for_each*()
> > No, list_for_each_entry does NOT check if the list is empty or if the first
> > entry is NULL.
>
> Please, read the macro source code again.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v8 4/7] i2c: fsi: Add abort and hardware reset procedures
From: Andy Shevchenko @ 2018-05-30 21:42 UTC (permalink / raw)
To: Eddie James
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <1527714464-8642-5-git-send-email-eajames@linux.vnet.ibm.com>
On Thu, May 31, 2018 at 12:07 AM, Eddie James
<eajames@linux.vnet.ibm.com> wrote:
> Add abort procedure for failed transfers. Add engine and bus reset
> procedures to recover from as many faults as possible.
>
Few small comments, after addressing which
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
> ---
> drivers/i2c/busses/i2c-fsi.c | 179 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 179 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> index be8e15c..e5dd0f7 100644
> --- a/drivers/i2c/busses/i2c-fsi.c
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -12,10 +12,12 @@
>
> #include <linux/bitfield.h>
> #include <linux/bitops.h>
> +#include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/errno.h>
> #include <linux/fsi.h>
> #include <linux/i2c.h>
> +#include <linux/jiffies.h>
> #include <linux/kernel.h>
> #include <linux/list.h>
> #include <linux/module.h>
> @@ -122,6 +124,20 @@
> #define I2C_ESTAT_SELF_BUSY BIT(6)
> #define I2C_ESTAT_VERSION GENMASK(4, 0)
>
> +/* port busy register */
> +#define I2C_PORT_BUSY_RESET BIT(31)
> +
> +/* wait for command complete or data request */
> +#define I2C_CMD_SLEEP_MAX_US 500
> +#define I2C_CMD_SLEEP_MIN_US 50
> +
> +/* wait after reset; choose time from legacy driver */
> +#define I2C_RESET_SLEEP_MAX_US 2000
> +#define I2C_RESET_SLEEP_MIN_US 1000
> +
> +/* choose timeout length from legacy driver; it's well tested */
> +#define I2C_ABORT_TIMEOUT msecs_to_jiffies(100)
> +
> struct fsi_i2c_master {
> struct fsi_device *fsi;
> u8 fifo_size;
> @@ -208,6 +224,169 @@ static int fsi_i2c_set_port(struct fsi_i2c_port *port)
> return fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, &dummy);
> }
>
> +static int fsi_i2c_reset_bus(struct fsi_i2c_master *i2c)
> +{
> + int i, rc;
> + u32 mode, stat, ext, dummy = 0;
> +
> + rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> + if (rc)
> + return rc;
> +
> + mode |= I2C_MODE_DIAG;
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> + if (rc)
> + return rc;
> +
> + for (i = 0; i < 9; ++i) {
i++
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_SCL, &dummy);
> + if (rc)
> + return rc;
> +
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_SET_SCL, &dummy);
> + if (rc)
> + return rc;
> + }
> +
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_SCL, &dummy);
> + if (rc)
> + return rc;
> +
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_SDA, &dummy);
> + if (rc)
> + return rc;
> +
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_SET_SCL, &dummy);
> + if (rc)
> + return rc;
> +
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_SET_SDA, &dummy);
> + if (rc)
> + return rc;
> +
> + mode &= ~I2C_MODE_DIAG;
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> + if (rc)
> + return rc;
> +
> + rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_STAT, &stat);
> + if (rc)
> + return rc;
> +
> + /* check for hardware fault */
> + if (!(stat & I2C_STAT_SCL_IN) || !(stat & I2C_STAT_SDA_IN)) {
> + rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_ESTAT, &ext);
> + if (rc)
> + return rc;
> +
> + dev_err(&i2c->fsi->dev, "bus stuck status[%08X] ext[%08X]\n",
> + stat, ext);
> + }
> +
> + return 0;
> +}
> +
> +static int fsi_i2c_reset(struct fsi_i2c_master *i2c, u16 port)
> +{
> + int rc;
> + u32 mode, stat, dummy = 0;
> +
> + /* reset engine */
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_I2C, &dummy);
> + if (rc)
> + return rc;
> +
> + /* re-init engine */
> + rc = fsi_i2c_dev_init(i2c);
> + if (rc)
> + return rc;
> +
> + rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> + if (rc)
> + return rc;
> +
> + /* set port; default after reset is 0 */
> + if (port) {
> + mode = (mode & ~I2C_MODE_PORT) | FIELD_PREP(I2C_MODE_PORT,
> + port);
Hmm... This would look better as two expressions
mode &= ~..._PORT;
mode |= FIELD_PREP(...);
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> + if (rc)
> + return rc;
> + }
> +
> + /* reset busy register; hw workaround */
> + dummy = I2C_PORT_BUSY_RESET;
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_PORT_BUSY, &dummy);
> + if (rc)
> + return rc;
> +
> + /* force bus reset */
> + rc = fsi_i2c_reset_bus(i2c);
> + if (rc)
> + return rc;
> +
> + /* reset errors */
> + dummy = 0;
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_ERR, &dummy);
> + if (rc)
> + return rc;
> +
> + /* wait for command complete */
> + usleep_range(I2C_RESET_SLEEP_MIN_US, I2C_RESET_SLEEP_MAX_US);
> +
> + rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_STAT, &stat);
> + if (rc)
> + return rc;
> +
> + if (stat & I2C_STAT_CMD_COMP)
> + return rc;
> +
> + /* failed to get command complete; reset engine again */
> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_I2C, &dummy);
> + if (rc)
> + return rc;
> +
> + /* re-init engine again */
> + return fsi_i2c_dev_init(i2c);
> +}
> +
> +static int fsi_i2c_abort(struct fsi_i2c_port *port, u32 status)
> +{
> + int rc;
> + unsigned long start;
> + u32 cmd = I2C_CMD_WITH_STOP;
> + struct fsi_device *fsi = port->master->fsi;
> +
> + rc = fsi_i2c_reset(port->master, port->port);
> + if (rc)
> + return rc;
> +
> + /* skip final stop command for these errors */
> + if (status & (I2C_STAT_PARITY | I2C_STAT_LOST_ARB | I2C_STAT_STOP_ERR))
> + return 0;
> +
> + /* write stop command */
> + rc = fsi_i2c_write_reg(fsi, I2C_FSI_CMD, &cmd);
> + if (rc)
> + return rc;
> +
> + /* wait until we see command complete in the master */
> + start = jiffies;
> +
> + do {
> + rc = fsi_i2c_read_reg(fsi, I2C_FSI_STAT, &status);
> + if (rc)
> + return rc;
> +
> + if (status & I2C_STAT_CMD_COMP)
> + return 0;
> +
> + usleep_range(I2C_CMD_SLEEP_MIN_US, I2C_CMD_SLEEP_MAX_US);
> + } while (time_after(start + I2C_ABORT_TIMEOUT, jiffies));
> +
> + return -ETIMEDOUT;
> +}
> +
> static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> int num)
> {
> --
> 1.8.3.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v8 0/7] i2c: Add FSI-attached I2C master algorithm
From: Andy Shevchenko @ 2018-05-30 21:31 UTC (permalink / raw)
To: Eddie James
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <1527714464-8642-1-git-send-email-eajames@linux.vnet.ibm.com>
On Thu, May 31, 2018 at 12:07 AM, Eddie James
<eajames@linux.vnet.ibm.com> wrote:
> This series adds an algorithm for an I2C master physically located on an FSI
> slave device. The I2C master has multiple ports, each of which may be connected
> to an I2C slave. Access to the I2C master registers is achieved over FSI bus.
>
> Due to the multi-port nature of the I2C master, the driver instantiates a new
> I2C adapter for each port connected to a slave. The connected ports should be
> defined in the device tree under the I2C master device.
I'll comment the series later, though you have to address previous
comments first:
- understand devm_ purpose and how it works
- understand list_for_each*() macros
- discuss with maintainer a design of enumerating ports
- ... (whatever I forgot and others added) ...
> Changes since v7
> - Fix grammer in Kconfig (a -> an)
> - Change I2C registers to use BIT and GENMASK
> - Remove custom macros and use FIELD_PREP and FIELD_GET
> - Fix a few unecessary initializations and "return rc" that are always zero
> - Clean up the read/write fifo functions a bit
> - Few other clean-up items
>
> Changes since v6
> - Remove spinlock for reset functionality; it's unecessary and doesn't work
> with the latest FSI core.
> - Use a mutex instead of a semaphore, and don't wait for timeout to get the
> lock.
> - Use usleeps instead of schedule_timeout; it's not worth the overhead when
> the wait should be very short in between sending the command and receiving
> the response.
>
> Changes since v5
> - Fix reset functionality and do a reset after every transfer failure
>
> Eddie James (7):
> dt-bindings: i2c: Add FSI-attached I2C master dt binding documentation
> i2c: Add FSI-attached I2C master algorithm
> i2c: fsi: Add port structures
> i2c: fsi: Add abort and hardware reset procedures
> i2c: fsi: Add transfer implementation
> i2c: fsi: Add I2C master locking
> i2c: fsi: Add bus recovery
>
> Documentation/devicetree/bindings/i2c/i2c-fsi.txt | 40 ++
> drivers/i2c/busses/Kconfig | 11 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-fsi.c | 722 ++++++++++++++++++++++
> 4 files changed, 774 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-fsi.txt
> create mode 100644 drivers/i2c/busses/i2c-fsi.c
>
> --
> 1.8.3.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v7 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
From: Rob Herring @ 2018-05-30 21:30 UTC (permalink / raw)
To: Andrzej Hajda
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, ryadav-sgV2jX0FEOL9JmXXK+q4OQ,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Sandeep Panda,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ, hoegsberg-F7+t8E8rja9g9hUCZPvPmw,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
chandanu-sgV2jX0FEOL9JmXXK+q4OQ
In-Reply-To: <20180529064144eucas1p1ea79686c36c55352549a69703edc0798~zCl_dTVxX0264302643eucas1p1O-MHMrYXj8g+pqW5MlFJXMulaTQe2KTcn/@public.gmane.org>
On Tue, May 29, 2018 at 08:41:42AM +0200, Andrzej Hajda wrote:
> On 24.05.2018 18:41, Sandeep Panda wrote:
> > Document the bindings used for the sn65dsi86 DSI to eDP bridge.
> >
> > Changes in v1:
> > - Rephrase the dt-binding descriptions to be more inline with existing
> > bindings (Andrzej Hajda).
> > - Add missing dt-binding that are parsed by corresponding driver
> > (Andrzej Hajda).
> >
> > Changes in v2:
> > - Remove edp panel specific dt-binding entries. Only keep bridge
> > specific entries (Sean Paul).
> > - Remove custom-modes dt entry since its usage is removed from driver also (Sean Paul).
> > - Remove is-pluggable dt entry since this will not be needed anymore (Sean Paul).
> >
> > Changes in v3:
> > - Remove irq-gpio dt entry and instead populate is an interrupt
> > property (Rob Herring).
> >
> > Changes in v4:
> > - Add link to bridge chip datasheet (Stephen Boyd)
> > - Add vpll and vcc regulator supply bindings (Stephen Boyd)
> > - Add ref clk optional dt binding (Stephen Boyd)
> > - Add gpio-controller optional dt binding (Stephen Boyd)
> >
> > Changes in v5:
> > - Use clock property to specify the input refclk (Stephen Boyd).
> > - Update gpio cell and pwm cell numbers (Stephen Boyd).
> >
> > Changes in v6:
> > - Add property to mention the lane mapping scheme and polarity inversion
> > (Stephen Boyd).
> >
> > Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
> > ---
> > .../bindings/display/bridge/ti,sn65dsi86.txt | 89 ++++++++++++++++++++++
> > 1 file changed, 89 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> > new file mode 100644
> > index 0000000..4a771a3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> > @@ -0,0 +1,89 @@
> > +SN65DSI86 DSI to eDP bridge chip
> > +--------------------------------
> > +
> > +This is the binding for Texas Instruments SN65DSI86 bridge.
> > +http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86&fileType=pdf
> > +
> > +Required properties:
> > +- compatible: Must be "ti,sn65dsi86"
> > +- reg: i2c address of the chip, 0x2d as per datasheet
> > +- enable-gpios: OF device-tree gpio specification for bridge_en pin
>
> info about active high should be added
>
> > +
> > +- vccio-supply: A 1.8V supply that powers up the digital IOs.
> > +- vpll-supply: A 1.8V supply that powers up the displayport PLL.
> > +- vcca-supply: A 1.2V supply that powers up the analog circuits.
> > +- vcc-supply: A 1.2V supply that powers up the digital core.
> > +
> > +Optional properties:
> > +- interrupts: Specifier for the SN65DSI86 interrupt line.
> > +- hpd-gpios: Specifications for HPD gpio pin.
>
> Again, please specify active level.
Having this property in the bridge node is strange. Also, does eDP
normally have a HPD signal? If you are using this for DP, then this
property goes in the connector node (or is absent if the bridge chip has
a dedicated signal).
> > +
> > +- gpio-controller: Marks the device has a GPIO controller.
> > +- #gpio-cells : Should be two. The first cell is the pin number and
> > + the second cell is used to specify flags.
> > + See ../../gpio/gpio.txt for more information.
> > +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of
> > + the cell formats.
> > +
> > +- clock-names: should be "refclk"
> > +- clocks: Specification for input reference clock. The reference
> > + clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
> > +
> > +- lane-config: Specification to describe the logical to physical lane
> > + mapping scheme and polarity inversion of lanes.
>
> Please describe this property, I guess it is about DSI lanes, and it
> should be exact(?) four pair of numbers, what are meaning and ranges of
> both numbers. What should be assumed if property is not present. Btw you
> can look into other bindings for reference, I guess there are already
> bindings having such property.
In fact, IIRC, some QCom bindings already have a property. Maybe it's
the same. If so, don't describe it twice. Document in common location
and just reference the common definition and add any constraints (like
active level for a GPIO).
Is this DSI or eDP lanes?
Rob
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply
* Re: [PATCH v7 3/7] drivers/i2c: Add port structure to FSI algorithm
From: Andy Shevchenko @ 2018-05-30 21:28 UTC (permalink / raw)
To: Eddie James
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Edward A. James
In-Reply-To: <CAHp75Vd8M4-mK=Fs_SbSX67NKeNty7vrq0-Y8-GyFdy9u6Q3jA@mail.gmail.com>
On Thu, May 31, 2018 at 12:27 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, May 30, 2018 at 6:47 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
>> On 05/29/2018 06:19 PM, Andy Shevchenko wrote:
>>>> + devm_kfree(dev, port);
>>> This hurts my eyes. Why?!
>
>> What would you suggest instead?
> Why are you doing that
> call implicitly?
s/implicitly/explicitly/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v7 3/7] drivers/i2c: Add port structure to FSI algorithm
From: Andy Shevchenko @ 2018-05-30 21:27 UTC (permalink / raw)
To: Eddie James
Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
Greg Kroah-Hartman, Edward A. James
In-Reply-To: <ae3de08c-6849-bb3a-7c6c-96309876d48e@linux.vnet.ibm.com>
On Wed, May 30, 2018 at 6:47 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> On 05/29/2018 06:19 PM, Andy Shevchenko wrote:
>> On Wed, May 30, 2018 at 1:24 AM, Eddie James <eajames@linux.vnet.ibm.com>
>> wrote:
>>> static int fsi_i2c_probe(struct device *dev)
>>> {
>>
>> Isn't below somehow repeats of_i2c_register_devices() ?
>> Why not to use it?
>
>
> Because I need to assign all these port structure fields. Also looks like
> of_i2c_register_devices creates new devices; I just want an adapter for each
> port.
Hmm... Wolfram, what is your opinion on this design?
>>> + devm_kfree(dev, port);
>>
>> This hurts my eyes. Why?!
> What would you suggest instead?
You even didn't wait for answer, why to ask then?
Moreover, you didn't answer to my question. Why are you doing that
call implicitly?
>>> + if (!list_empty(&i2c->ports)) {
>>
>> My gosh, this is done already in list_for_each*()
> No, list_for_each_entry does NOT check if the list is empty or if the first
> entry is NULL.
Please, read the macro source code again.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox