LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding
@ 2019-04-30  4:32 Chuanhua Han
  2019-04-30  4:32 ` [PATCH 2/3] i2c: imx: I2C Driver IBC and SCL Divider for MUL=2 and MUL=4 Chuanhua Han
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Chuanhua Han @ 2019-04-30  4:32 UTC (permalink / raw)
  To: robh+dt, mark.rutland, shawnguo, s.hauer, leoyang.li
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-i2c, kernel,
	linux-imx, festevam, wsa+renesas, u.kleine-koenig, eha, linux,
	sumit.batra, l.stach, peda, Chuanhua Han

NXP Layerscape SoC have up to three MUL options available for all
divider values, we choice of MUL determines the internal monitor rate
of the I2C bus (SCL and SDA signals):
A lower MUL value results in a higher sampling rate of the I2C signals.
A higher MUL value results in a lower sampling rate of the I2C signals.

So in Optional properties we added our custom mul-value property in the
binding to select which mul option for the device tree i2c controller
node.

Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
---
 Documentation/devicetree/bindings/i2c/i2c-imx.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
index b967544590e8..ba8e7b7b3fa8 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
@@ -18,6 +18,9 @@ Optional properties:
 - sda-gpios: specify the gpio related to SDA pin
 - pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
   bus recovery, call it "gpio" state
+- mul-value: NXP Layerscape SoC have up to three MUL options available for
+all I2C divider values, it describes which MUL we choose to use for the driver,
+the values should be 1,2,4.
 
 Examples:
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] i2c: imx: I2C Driver IBC and SCL Divider for MUL=2 and MUL=4
  2019-04-30  4:32 [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding Chuanhua Han
@ 2019-04-30  4:32 ` Chuanhua Han
  2019-04-30  4:32 ` [PATCH 3/3] arm64: dts: fsl: ls1046a: Add mul-value property of the i2c controller nodes Chuanhua Han
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Chuanhua Han @ 2019-04-30  4:32 UTC (permalink / raw)
  To: robh+dt, mark.rutland, shawnguo, s.hauer, leoyang.li
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-i2c, kernel,
	linux-imx, festevam, wsa+renesas, u.kleine-koenig, eha, linux,
	sumit.batra, l.stach, peda, Chuanhua Han

NXP Layerscape SoC have up to three MUL options available for all
divider values,we choice of MUL determines the internal monitor rate
of the I2C bus (SCL and SDA signals).

The current kernel driver supports MUL=1 by default ,but doesn't have
the IBC and SCL Divider entries in vf610_i2c_clk_div for MUL=2  and
MUL=4,so we need to add the corresponding support.

Signed-off-by: Sumit Batra <sumit.batra@nxp.com>
Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
---
 drivers/i2c/busses/i2c-imx.c | 71 +++++++++++++++++++++++++++++++++++-
 1 file changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 42fed40198a0..ac5a334b7339 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -38,6 +38,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_dma.h>
+#include <linux/of_address.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_data/i2c-imx.h>
 #include <linux/platform_device.h>
@@ -156,6 +157,44 @@ static struct imx_i2c_clk_pair vf610_i2c_clk_div[] = {
 	{ 3840, 0x3F }, { 4096, 0x7B }, { 5120, 0x7D }, { 6144, 0x7E },
 };
 
+static struct imx_i2c_clk_pair mul2_i2c_clk_div[] = {
+	{ 40,   0x40 }, { 44,   0x41 }, { 48,   0x42 }, { 52,   0x43 },
+	{ 56,   0x44 }, { 60,   0x45 }, { 68,   0x46 }, { 80,   0x47 },
+	{ 56,   0x48 }, { 64,   0x49 }, { 72,   0x4A }, { 80,   0x4B },
+	{ 88,   0x4C }, { 96,   0x4D }, { 112,  0x4E }, { 136,  0x4F },
+	{ 96,   0x50 }, { 112,  0x51 }, { 128,  0x52 }, { 144,  0x53 },
+	{ 160,  0x54 }, { 176,  0x55 }, { 208,  0x56 }, { 256,  0x57 },
+	{ 160,  0x58 }, { 192,  0x59 }, { 224,  0x5A }, { 256,  0x5B },
+	{ 288,  0x5C }, { 320,  0x5D }, { 384,  0x5E }, { 480,  0x5F },
+	{ 320,  0x60 }, { 384,  0x61 }, { 448,  0x62 }, { 512,  0x63 },
+	{ 576,  0x64 }, { 640,  0x65 }, { 768,  0x66 }, { 960,  0x67 },
+	{ 640,  0x68 }, { 768,  0x69 }, { 896,  0x6A }, { 1024, 0x6B },
+	{ 1152, 0x6C }, { 1280, 0x6D }, { 1536, 0x6E }, { 1920, 0x6F },
+	{ 1280, 0x70 }, { 1536, 0x71 }, { 1792, 0x72 }, { 2048, 0x73 },
+	{ 2304, 0x74 }, { 2560, 0x75 }, { 3072, 0x76 }, { 3840, 0x77 },
+	{ 2560, 0x78 }, { 3072, 0x79 }, { 3584, 0x7A }, { 4096, 0x7B },
+	{ 4608, 0x7C }, { 5120, 0x7D }, { 6144, 0x7E }, { 7680, 0x7F },
+};
+
+static struct imx_i2c_clk_pair mul4_i2c_clk_div[] = {
+	{ 80,    0x80 }, { 88,    0x81 }, { 96,    0x82 }, { 104,   0x83 },
+	{ 112,   0x84 }, { 120,   0x85 }, { 136,   0x86 }, { 160,   0x87 },
+	{ 112,   0x88 }, { 128,   0x89 }, { 144,   0x8A }, { 160,   0x8B },
+	{ 176,   0x8C }, { 192,   0x8D }, { 224,   0x8E }, { 272,   0x8F },
+	{ 192,   0x90 }, { 224,   0x91 }, { 256,   0x92 }, { 288,   0x93 },
+	{ 320,   0x94 }, { 352,   0x95 }, { 416,   0x96 }, { 512,   0x97 },
+	{ 320,   0x98 }, { 384,   0x99 }, { 448,   0x9A }, { 512,   0x9B },
+	{ 576,   0x9C }, { 640,   0x9D }, { 768,   0x9E }, { 960,   0x9F },
+	{ 640,   0xA0 }, { 768,   0xA1 }, { 896,   0xA2 }, { 1024,  0xA3 },
+	{ 1152,  0xA4 }, { 1280,  0xA5 }, { 1536,  0xA6 }, { 1792,  0xAA },
+	{ 1280,  0xA8 }, { 1536,  0xA9 }, { 1920,  0xA7 }, { 2048,  0xAB },
+	{ 2304,  0xAC }, { 2560,  0xAD }, { 3072,  0xAE }, { 3584,  0xB2 },
+	{ 2560,  0xB0 }, { 3072,  0xB1 }, { 3820,  0xAF }, { 4096,  0xB3 },
+	{ 4608,  0xB4 }, { 5120,  0xB5 }, { 6144,  0xB6 }, { 7680,  0xB7 },
+	{ 5120,  0xB8 }, { 6144,  0xB9 }, { 7168,  0xBA }, { 8192,  0xBB },
+	{ 9216,  0xBC }, { 10240, 0xBD }, { 12288, 0xBE }, { 15360, 0xBF },
+};
+
 enum imx_i2c_type {
 	IMX1_I2C,
 	IMX21_I2C,
@@ -234,6 +273,24 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata = {
 
 };
 
+static struct imx_i2c_hwdata mul2_i2c_hwdata = {
+	.devtype                = VF610_I2C,
+	.regshift               = VF610_I2C_REGSHIFT,
+	.clk_div                = mul2_i2c_clk_div,
+	.ndivs                  = ARRAY_SIZE(mul2_i2c_clk_div),
+	.i2sr_clr_opcode        = I2SR_CLR_OPCODE_W1C,
+	.i2cr_ien_opcode        = I2CR_IEN_OPCODE_0,
+};
+
+static struct imx_i2c_hwdata mul4_i2c_hwdata = {
+	.devtype                = VF610_I2C,
+	.regshift               = VF610_I2C_REGSHIFT,
+	.clk_div                = mul4_i2c_clk_div,
+	.ndivs                  = ARRAY_SIZE(mul4_i2c_clk_div),
+	.i2sr_clr_opcode        = I2SR_CLR_OPCODE_W1C,
+	.i2cr_ien_opcode        = I2CR_IEN_OPCODE_0,
+};
+
 static const struct platform_device_id imx_i2c_devtype[] = {
 	{
 		.name = "imx1-i2c",
@@ -1058,6 +1115,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int irq, ret;
 	dma_addr_t phy_addr;
+	u32 mul_value;
 
 	dev_dbg(&pdev->dev, "<%s>\n", __func__);
 
@@ -1077,11 +1135,20 @@ static int i2c_imx_probe(struct platform_device *pdev)
 	if (!i2c_imx)
 		return -ENOMEM;
 
-	if (of_id)
+	if (of_id) {
 		i2c_imx->hwdata = of_id->data;
-	else
+		ret = of_property_read_u32(pdev->dev.of_node,
+					   "mul-value", &mul_value);
+		if (!ret) {
+			if (mul_value == 2)
+				i2c_imx->hwdata = &mul2_i2c_hwdata;
+			else if (mul_value == 4)
+				i2c_imx->hwdata = &mul4_i2c_hwdata;
+		}
+	} else {
 		i2c_imx->hwdata = (struct imx_i2c_hwdata *)
 				platform_get_device_id(pdev)->driver_data;
+	}
 
 	/* Setup i2c_imx driver structure */
 	strlcpy(i2c_imx->adapter.name, pdev->name, sizeof(i2c_imx->adapter.name));
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/3] arm64: dts: fsl: ls1046a: Add mul-value property of the i2c controller nodes
  2019-04-30  4:32 [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding Chuanhua Han
  2019-04-30  4:32 ` [PATCH 2/3] i2c: imx: I2C Driver IBC and SCL Divider for MUL=2 and MUL=4 Chuanhua Han
@ 2019-04-30  4:32 ` Chuanhua Han
  2019-04-30  6:38 ` [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding Uwe Kleine-König
  2019-05-02 20:59 ` Rob Herring
  3 siblings, 0 replies; 9+ messages in thread
From: Chuanhua Han @ 2019-04-30  4:32 UTC (permalink / raw)
  To: robh+dt, mark.rutland, shawnguo, s.hauer, leoyang.li
  Cc: linux-kernel, devicetree, linux-arm-kernel, linux-i2c, kernel,
	linux-imx, festevam, wsa+renesas, u.kleine-koenig, eha, linux,
	sumit.batra, l.stach, peda, Chuanhua Han

According to LS1046A Reference Manual, for the i2c controller, you have
up to three MUL options available for all divider values. Therefore, we
need to determine which MUL to use in the device tree for driver use.

The "mul-value" property provides which mul is used in our driver.

Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index b0ef08b090dd..373310e4c0ea 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -385,6 +385,7 @@
 			dmas = <&edma0 1 39>,
 			       <&edma0 1 38>;
 			dma-names = "tx", "rx";
+			mul-value = <4>;
 			status = "disabled";
 		};
 
@@ -395,6 +396,7 @@
 			reg = <0x0 0x2190000 0x0 0x10000>;
 			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clockgen 4 1>;
+			mul-value = <4>;
 			status = "disabled";
 		};
 
@@ -405,6 +407,7 @@
 			reg = <0x0 0x21a0000 0x0 0x10000>;
 			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clockgen 4 1>;
+			mul-value = <4>;
 			status = "disabled";
 		};
 
@@ -415,6 +418,7 @@
 			reg = <0x0 0x21b0000 0x0 0x10000>;
 			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clockgen 4 1>;
+			mul-value = <4>;
 			status = "disabled";
 		};
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding
  2019-04-30  4:32 [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding Chuanhua Han
  2019-04-30  4:32 ` [PATCH 2/3] i2c: imx: I2C Driver IBC and SCL Divider for MUL=2 and MUL=4 Chuanhua Han
  2019-04-30  4:32 ` [PATCH 3/3] arm64: dts: fsl: ls1046a: Add mul-value property of the i2c controller nodes Chuanhua Han
@ 2019-04-30  6:38 ` Uwe Kleine-König
  2019-04-30  6:56   ` [EXT] " Chuanhua Han
  2019-05-02 20:59 ` Rob Herring
  3 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2019-04-30  6:38 UTC (permalink / raw)
  To: Chuanhua Han
  Cc: robh+dt, mark.rutland, shawnguo, s.hauer, leoyang.li,
	linux-kernel, devicetree, linux-arm-kernel, linux-i2c, kernel,
	linux-imx, festevam, wsa+renesas, eha, linux, sumit.batra,
	l.stach, peda

On Tue, Apr 30, 2019 at 12:32:40PM +0800, Chuanhua Han wrote:
> NXP Layerscape SoC have up to three MUL options available for all
> divider values, we choice of MUL determines the internal monitor rate
> of the I2C bus (SCL and SDA signals):
> A lower MUL value results in a higher sampling rate of the I2C signals.
> A higher MUL value results in a lower sampling rate of the I2C signals.
> 
> So in Optional properties we added our custom mul-value property in the
> binding to select which mul option for the device tree i2c controller
> node.
> 
> Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-imx.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> index b967544590e8..ba8e7b7b3fa8 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> @@ -18,6 +18,9 @@ Optional properties:
>  - sda-gpios: specify the gpio related to SDA pin
>  - pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
>    bus recovery, call it "gpio" state
> +- mul-value: NXP Layerscape SoC have up to three MUL options available for
> +all I2C divider values, it describes which MUL we choose to use for the driver,
> +the values should be 1,2,4.

Indention is broken.

I wonder why this needs to be configurable on a per-machine/device
level. What is the trade-off?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [EXT] Re: [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding
  2019-04-30  6:38 ` [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding Uwe Kleine-König
@ 2019-04-30  6:56   ` Chuanhua Han
  0 siblings, 0 replies; 9+ messages in thread
From: Chuanhua Han @ 2019-04-30  6:56 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: robh+dt, mark.rutland, shawnguo, s.hauer, Leo Li, linux-kernel,
	devicetree, linux-arm-kernel, linux-i2c, kernel, dl-linux-imx,
	festevam, wsa+renesas, eha, linux, Sumit Batra, l.stach, peda



> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Sent: 2019年4月30日 14:38
> To: Chuanhua Han <chuanhua.han@nxp.com>
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; Leo Li <leoyang.li@nxp.com>;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-i2c@vger.kernel.org;
> kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>;
> festevam@gmail.com; wsa+renesas@sang-engineering.com; eha@deif.com;
> linux@rempel-privat.de; Sumit Batra <sumit.batra@nxp.com>;
> l.stach@pengutronix.de; peda@axentia.se
> Subject: [EXT] Re: [PATCH 1/3] dt-bindings: i2c: add optional mul-value
> property to binding
> 
> Caution: EXT Email
> 
> On Tue, Apr 30, 2019 at 12:32:40PM +0800, Chuanhua Han wrote:
> > NXP Layerscape SoC have up to three MUL options available for all
> > divider values, we choice of MUL determines the internal monitor rate
> > of the I2C bus (SCL and SDA signals):
> > A lower MUL value results in a higher sampling rate of the I2C signals.
> > A higher MUL value results in a lower sampling rate of the I2C signals.
> >
> > So in Optional properties we added our custom mul-value property in
> > the binding to select which mul option for the device tree i2c
> > controller node.
> >
> > Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c-imx.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > index b967544590e8..ba8e7b7b3fa8 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > @@ -18,6 +18,9 @@ Optional properties:
> >  - sda-gpios: specify the gpio related to SDA pin
> >  - pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
> >    bus recovery, call it "gpio" state
> > +- mul-value: NXP Layerscape SoC have up to three MUL options
> > +available for all I2C divider values, it describes which MUL we
> > +choose to use for the driver, the values should be 1,2,4.
> 
> Indention is broken.
Yes, I also found this problem, next version I will fix the indent problem
> 
> I wonder why this needs to be configurable on a per-machine/device level.
> What is the trade-off?
According to NXP Layerscape SoC Reference Manual, there are three MUL 
options for i2c controller to configure i2c Bus Frequency Divider Register (IBFD)
to determine the clock Frequency of i2c. 
Some socs (such as ls1046a) have the best performance when MUL=4, 
and the default is MUL=1. 
This option is optional and can be configured by device tree
> 
> Best regards
> Uwe
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König
> |
> Industrial Linux Solutions                 |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> ngutronix.de%2F&amp;data=02%7C01%7Cchuanhua.han%40nxp.com%7C158
> 21c9cf4c449f2d5ea08d6cd367aaa%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C636922031201957736&amp;sdata=8jKPN%2FSJghgOF890NTr
> %2FC%2B9PsFpEr64%2B%2FXHLSX5Cipo%3D&amp;reserved=0  |

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding
  2019-04-30  4:32 [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding Chuanhua Han
                   ` (2 preceding siblings ...)
  2019-04-30  6:38 ` [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding Uwe Kleine-König
@ 2019-05-02 20:59 ` Rob Herring
  2019-05-07  8:40   ` Wolfram Sang
  2019-05-08 11:44   ` [EXT] " Chuanhua Han
  3 siblings, 2 replies; 9+ messages in thread
From: Rob Herring @ 2019-05-02 20:59 UTC (permalink / raw)
  To: Chuanhua Han
  Cc: mark.rutland, shawnguo, s.hauer, leoyang.li, linux-kernel,
	devicetree, linux-arm-kernel, linux-i2c, kernel, linux-imx,
	festevam, wsa+renesas, u.kleine-koenig, eha, linux, sumit.batra,
	l.stach, peda

On Tue, Apr 30, 2019 at 12:32:40PM +0800, Chuanhua Han wrote:
> NXP Layerscape SoC have up to three MUL options available for all
> divider values, we choice of MUL determines the internal monitor rate
> of the I2C bus (SCL and SDA signals):
> A lower MUL value results in a higher sampling rate of the I2C signals.
> A higher MUL value results in a lower sampling rate of the I2C signals.
> 
> So in Optional properties we added our custom mul-value property in the
> binding to select which mul option for the device tree i2c controller
> node.
> 
> Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-imx.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> index b967544590e8..ba8e7b7b3fa8 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> @@ -18,6 +18,9 @@ Optional properties:
>  - sda-gpios: specify the gpio related to SDA pin
>  - pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
>    bus recovery, call it "gpio" state
> +- mul-value: NXP Layerscape SoC have up to three MUL options available for
> +all I2C divider values, it describes which MUL we choose to use for the driver,
> +the values should be 1,2,4.

Needs a vendor prefix. I don't find 'value' to add anything nor do I 
understand what MUL is.

If it is determined by SoC rather than board, then it should perhaps be 
implied by compatible.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding
  2019-05-02 20:59 ` Rob Herring
@ 2019-05-07  8:40   ` Wolfram Sang
  2019-05-08 11:44   ` [EXT] " Chuanhua Han
  1 sibling, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2019-05-07  8:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Chuanhua Han, mark.rutland, shawnguo, s.hauer, leoyang.li,
	linux-kernel, devicetree, linux-arm-kernel, linux-i2c, kernel,
	linux-imx, festevam, wsa+renesas, u.kleine-koenig, eha, linux,
	sumit.batra, l.stach, peda

[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]

On Thu, May 02, 2019 at 03:59:01PM -0500, Rob Herring wrote:
> On Tue, Apr 30, 2019 at 12:32:40PM +0800, Chuanhua Han wrote:
> > NXP Layerscape SoC have up to three MUL options available for all
> > divider values, we choice of MUL determines the internal monitor rate
> > of the I2C bus (SCL and SDA signals):
> > A lower MUL value results in a higher sampling rate of the I2C signals.
> > A higher MUL value results in a lower sampling rate of the I2C signals.
> > 
> > So in Optional properties we added our custom mul-value property in the
> > binding to select which mul option for the device tree i2c controller
> > node.
> > 
> > Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c-imx.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > index b967544590e8..ba8e7b7b3fa8 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > @@ -18,6 +18,9 @@ Optional properties:
> >  - sda-gpios: specify the gpio related to SDA pin
> >  - pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
> >    bus recovery, call it "gpio" state
> > +- mul-value: NXP Layerscape SoC have up to three MUL options available for
> > +all I2C divider values, it describes which MUL we choose to use for the driver,
> > +the values should be 1,2,4.
> 
> Needs a vendor prefix. I don't find 'value' to add anything nor do I 
> understand what MUL is.
> 
> If it is determined by SoC rather than board, then it should perhaps be 
> implied by compatible.

I was wondering the same.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [EXT] Re: [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding
  2019-05-02 20:59 ` Rob Herring
  2019-05-07  8:40   ` Wolfram Sang
@ 2019-05-08 11:44   ` Chuanhua Han
  2019-05-08 18:53     ` Leo Li
  1 sibling, 1 reply; 9+ messages in thread
From: Chuanhua Han @ 2019-05-08 11:44 UTC (permalink / raw)
  To: Rob Herring
  Cc: mark.rutland, shawnguo, s.hauer, Leo Li, linux-kernel,
	devicetree, linux-arm-kernel, linux-i2c, kernel, dl-linux-imx,
	festevam, wsa+renesas, u.kleine-koenig, eha, linux, Sumit Batra,
	l.stach, peda



> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 2019年5月3日 4:59
> To: Chuanhua Han <chuanhua.han@nxp.com>
> Cc: mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> Leo Li <leoyang.li@nxp.com>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-i2c@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> <linux-imx@nxp.com>; festevam@gmail.com;
> wsa+renesas@sang-engineering.com; u.kleine-koenig@pengutronix.de;
> eha@deif.com; linux@rempel-privat.de; Sumit Batra <sumit.batra@nxp.com>;
> l.stach@pengutronix.de; peda@axentia.se
> Subject: [EXT] Re: [PATCH 1/3] dt-bindings: i2c: add optional mul-value
> property to binding
> 
> Caution: EXT Email
> 
> On Tue, Apr 30, 2019 at 12:32:40PM +0800, Chuanhua Han wrote:
> > NXP Layerscape SoC have up to three MUL options available for all
> > divider values, we choice of MUL determines the internal monitor rate
> > of the I2C bus (SCL and SDA signals):
> > A lower MUL value results in a higher sampling rate of the I2C signals.
> > A higher MUL value results in a lower sampling rate of the I2C signals.
> >
> > So in Optional properties we added our custom mul-value property in
> > the binding to select which mul option for the device tree i2c
> > controller node.
> >
> > Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c-imx.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > index b967544590e8..ba8e7b7b3fa8 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > @@ -18,6 +18,9 @@ Optional properties:
> >  - sda-gpios: specify the gpio related to SDA pin
> >  - pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
> >    bus recovery, call it "gpio" state
> > +- mul-value: NXP Layerscape SoC have up to three MUL options
> > +available for all I2C divider values, it describes which MUL we
> > +choose to use for the driver, the values should be 1,2,4.
> 
> Needs a vendor prefix. I don't find 'value' to add anything nor do I understand
> what MUL is.
Yes,you are right!
> 
> If it is determined by SoC rather than board, then it should perhaps be implied
> by compatible.
This is determined by the SOC, but it has three options to choose from, 
so I think it's better to use the optional option instead of compatible

> 
> Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [EXT] Re: [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding
  2019-05-08 11:44   ` [EXT] " Chuanhua Han
@ 2019-05-08 18:53     ` Leo Li
  0 siblings, 0 replies; 9+ messages in thread
From: Leo Li @ 2019-05-08 18:53 UTC (permalink / raw)
  To: Chuanhua Han, Rob Herring
  Cc: mark.rutland, shawnguo, s.hauer, linux-kernel, devicetree,
	linux-arm-kernel, linux-i2c, kernel, dl-linux-imx, festevam,
	wsa+renesas, u.kleine-koenig, eha, linux, Sumit Batra, l.stach,
	peda



> -----Original Message-----
> From: Chuanhua Han
> Sent: Wednesday, May 8, 2019 6:45 AM
> To: Rob Herring <robh@kernel.org>
> Cc: mark.rutland@arm.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; Leo Li <leoyang.li@nxp.com>; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-i2c@vger.kernel.org;
> kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>;
> festevam@gmail.com; wsa+renesas@sang-engineering.com; u.kleine-
> koenig@pengutronix.de; eha@deif.com; linux@rempel-privat.de; Sumit
> Batra <sumit.batra@nxp.com>; l.stach@pengutronix.de; peda@axentia.se
> Subject: RE: [EXT] Re: [PATCH 1/3] dt-bindings: i2c: add optional mul-value
> property to binding
> 
> 
> 
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: 2019年5月3日 4:59
> > To: Chuanhua Han <chuanhua.han@nxp.com>
> > Cc: mark.rutland@arm.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de;
> > Leo Li <leoyang.li@nxp.com>; linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux-i2c@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> > <linux-imx@nxp.com>; festevam@gmail.com;
> > wsa+renesas@sang-engineering.com; u.kleine-koenig@pengutronix.de;
> > eha@deif.com; linux@rempel-privat.de; Sumit Batra
> > <sumit.batra@nxp.com>; l.stach@pengutronix.de; peda@axentia.se
> > Subject: [EXT] Re: [PATCH 1/3] dt-bindings: i2c: add optional
> > mul-value property to binding
> >
> > Caution: EXT Email
> >
> > On Tue, Apr 30, 2019 at 12:32:40PM +0800, Chuanhua Han wrote:
> > > NXP Layerscape SoC have up to three MUL options available for all
> > > divider values, we choice of MUL determines the internal monitor
> > > rate of the I2C bus (SCL and SDA signals):
> > > A lower MUL value results in a higher sampling rate of the I2C signals.
> > > A higher MUL value results in a lower sampling rate of the I2C signals.
> > >
> > > So in Optional properties we added our custom mul-value property in
> > > the binding to select which mul option for the device tree i2c
> > > controller node.
> > >
> > > Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/i2c/i2c-imx.txt | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > > b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > > index b967544590e8..ba8e7b7b3fa8 100644
> > > --- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > > +++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> > > @@ -18,6 +18,9 @@ Optional properties:
> > >  - sda-gpios: specify the gpio related to SDA pin
> > >  - pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
> > >    bus recovery, call it "gpio" state
> > > +- mul-value: NXP Layerscape SoC have up to three MUL options
> > > +available for all I2C divider values, it describes which MUL we
> > > +choose to use for the driver, the values should be 1,2,4.
> >
> > Needs a vendor prefix. I don't find 'value' to add anything nor do I
> > understand what MUL is.
> Yes,you are right!
> >
> > If it is determined by SoC rather than board, then it should perhaps
> > be implied by compatible.
> This is determined by the SOC, but it has three options to choose from, so I
> think it's better to use the optional option instead of compatible

If there is only one best choice for each SoC letting the SoC compatible determine it will be the best.  Unless different board designs(use cases) of the same SoC requires different MUL settings, I also don't see much value of making it defined in device tree.

Regards,
Leo

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-05-08 18:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-30  4:32 [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding Chuanhua Han
2019-04-30  4:32 ` [PATCH 2/3] i2c: imx: I2C Driver IBC and SCL Divider for MUL=2 and MUL=4 Chuanhua Han
2019-04-30  4:32 ` [PATCH 3/3] arm64: dts: fsl: ls1046a: Add mul-value property of the i2c controller nodes Chuanhua Han
2019-04-30  6:38 ` [PATCH 1/3] dt-bindings: i2c: add optional mul-value property to binding Uwe Kleine-König
2019-04-30  6:56   ` [EXT] " Chuanhua Han
2019-05-02 20:59 ` Rob Herring
2019-05-07  8:40   ` Wolfram Sang
2019-05-08 11:44   ` [EXT] " Chuanhua Han
2019-05-08 18:53     ` Leo Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).