linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] arm64: dts: Mediatek: MT8173 updtes
@ 2015-06-17 15:08 Eddie Huang
  2015-06-17 15:08 ` [PATCH v4 1/2] arm64: dts: mt8173: Add watchdog device node Eddie Huang
  2015-06-17 15:08 ` [PATCH v4 2/2] arm64: dts: mt8173: Add I2C " Eddie Huang
  0 siblings, 2 replies; 6+ messages in thread
From: Eddie Huang @ 2015-06-17 15:08 UTC (permalink / raw)
  To: linux-arm-kernel

This series add MT8173 watchdog and I2C device nodes. Both device nodes 
are based on 4.1-rc1, but I2C need two extra CCF patches from Sascha [1][2].

[1] clk: Add common clock support for Mediatek MT8135 and MT8173
    http://lists.infradead.org/pipermail/linux-arm-kernel/2015-April/338763.html
[2] ARM64: dts: mt8173: Add clock controller device nodes
    http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/344753.html

Change in v4:
Move I2C pin from board dts file back to SoC dtsi file after discussion.
Add I2C default pin, address, size cell number in I2C controller node. 
But not clock-frequency because it is optional. (default value is 1000000)

Change in v3:
Move I2C pin from SoC dtsi to board dts file.

Change in v2:
Modify I2C device node accroding register address order.

Eddie Huang (2):
  arm64: dts: mt8173: Add watchdog device node
  arm64: dts: mt8173: Add I2C device node

 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 150 +++++++++++++++++++++++++++++++
 1 file changed, 150 insertions(+)

-- 
1.8.1.1.dirty

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

* [PATCH v4 1/2] arm64: dts: mt8173: Add watchdog device node
  2015-06-17 15:08 [PATCH v4 0/2] arm64: dts: Mediatek: MT8173 updtes Eddie Huang
@ 2015-06-17 15:08 ` Eddie Huang
  2015-06-17 15:08 ` [PATCH v4 2/2] arm64: dts: mt8173: Add I2C " Eddie Huang
  1 sibling, 0 replies; 6+ messages in thread
From: Eddie Huang @ 2015-06-17 15:08 UTC (permalink / raw)
  To: linux-arm-kernel

Add MT8173 watchdog device node.

Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 30ac8dd..b52ec43 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -160,6 +160,12 @@
 						<GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		watchdog: watchdog at 10007000 {
+			compatible = "mediatek,mt8173-wdt",
+				     "mediatek,mt6589-wdt";
+			reg = <0 0x10007000 0 0x100>;
+		};
+
 		sysirq: intpol-controller at 10200620 {
 			compatible = "mediatek,mt8173-sysirq",
 					"mediatek,mt6577-sysirq";
-- 
1.8.1.1.dirty

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

* [PATCH v4 2/2] arm64: dts: mt8173: Add I2C device node
  2015-06-17 15:08 [PATCH v4 0/2] arm64: dts: Mediatek: MT8173 updtes Eddie Huang
  2015-06-17 15:08 ` [PATCH v4 1/2] arm64: dts: mt8173: Add watchdog device node Eddie Huang
@ 2015-06-17 15:08 ` Eddie Huang
  2015-06-18 15:16   ` Daniel Kurtz
  1 sibling, 1 reply; 6+ messages in thread
From: Eddie Huang @ 2015-06-17 15:08 UTC (permalink / raw)
  To: linux-arm-kernel

Add MT8173 I2C device nodes, include I2C controllers and pins.
MT8173 has six I2C controllers, from i2c0 to i2c6, exclude i2c5.
The 6th I2C controller register base doesn't next to 5th I2C,
and there is a hardware between 5th and 6th I2C controller. So
SoC designer name 6th controller as "i2c6", not "i2c5".

Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 144 +++++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index b52ec43..1816c8f 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -158,6 +158,54 @@
 			interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
 						<GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>,
 						<GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
+
+			i2c0_pins_a: i2c0 {
+				pins1 {
+					pinmux = <MT8173_PIN_45_SDA0__FUNC_SDA0>,
+						 <MT8173_PIN_46_SCL0__FUNC_SCL0>;
+					bias-disable;
+				};
+			};
+
+			i2c1_pins_a: i2c1 {
+				pins1 {
+					pinmux = <MT8173_PIN_125_SDA1__FUNC_SDA1>,
+						 <MT8173_PIN_126_SCL1__FUNC_SCL1>;
+					bias-disable;
+				};
+			};
+
+			i2c2_pins_a: i2c2 {
+				pins1 {
+					pinmux = <MT8173_PIN_43_SDA2__FUNC_SDA2>,
+						 <MT8173_PIN_44_SCL2__FUNC_SCL2>;
+					bias-disable;
+				};
+			};
+
+			i2c3_pins_a: i2c3 {
+				pins1 {
+					pinmux = <MT8173_PIN_106_SDA3__FUNC_SDA3>,
+						 <MT8173_PIN_107_SCL3__FUNC_SCL3>;
+					bias-disable;
+				};
+			};
+
+			i2c4_pins_a: i2c4 {
+				pins1 {
+					pinmux = <MT8173_PIN_133_SDA4__FUNC_SDA4>,
+						 <MT8173_PIN_134_SCL4__FUNC_SCL4>;
+					bias-disable;
+				};
+			};
+
+			i2c6_pins_a: i2c6 {
+				pins1 {
+					pinmux = <MT8173_PIN_100_MSDC2_DAT0__FUNC_SDA5>,
+						 <MT8173_PIN_101_MSDC2_DAT1__FUNC_SCL5>;
+					bias-disable;
+				};
+			};
 		};
 
 		watchdog: watchdog at 10007000 {
@@ -229,6 +277,102 @@
 			clocks = <&uart_clk>;
 			status = "disabled";
 		};
+
+		i2c0: i2c at 11007000 {
+			compatible = "mediatek,mt8173-i2c";
+			reg = <0 0x11007000 0 0x70>,
+			      <0 0x11000100 0 0x80>;
+			interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_LOW>;
+			clock-div = <16>;
+			clocks = <&pericfg CLK_PERI_I2C0>,
+				 <&pericfg CLK_PERI_AP_DMA>;
+			clock-names = "main", "dma";
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c0_pins_a>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c1: i2c at 11008000 {
+			compatible = "mediatek,mt8173-i2c";
+			reg = <0 0x11008000 0 0x70>,
+			      <0 0x11000180 0 0x80>;
+			interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_LOW>;
+			clock-div = <16>;
+			clocks = <&pericfg CLK_PERI_I2C1>,
+				 <&pericfg CLK_PERI_AP_DMA>;
+			clock-names = "main", "dma";
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c1_pins_a>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c2: i2c at 11009000 {
+			compatible = "mediatek,mt8173-i2c";
+			reg = <0 0x11009000 0 0x70>,
+			      <0 0x11000200 0 0x80>;
+			interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_LOW>;
+			clock-div = <16>;
+			clocks = <&pericfg CLK_PERI_I2C2>,
+				 <&pericfg CLK_PERI_AP_DMA>;
+			clock-names = "main", "dma";
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c2_pins_a>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c3: i2c3 at 11010000 {
+			compatible = "mediatek,mt8173-i2c";
+			reg = <0 0x11010000 0 0x70>,
+			      <0 0x11000280 0 0x80>;
+			interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_LOW>;
+			clock-div = <16>;
+			clocks = <&pericfg CLK_PERI_I2C3>,
+				 <&pericfg CLK_PERI_AP_DMA>;
+			clock-names = "main", "dma";
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c3_pins_a>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c4: i2c4 at 11011000 {
+			compatible = "mediatek,mt8173-i2c";
+			reg = <0 0x11011000 0 0x70>,
+			      <0 0x11000300 0 0x80>;
+			interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_LOW>;
+			clock-div = <16>;
+			clocks = <&pericfg CLK_PERI_I2C4>,
+				 <&pericfg CLK_PERI_AP_DMA>;
+			clock-names = "main", "dma";
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c4_pins_a>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c6: i2c6 at 11013000 {
+			compatible = "mediatek,mt8173-i2c";
+			reg = <0 0x11013000 0 0x70>,
+			      <0 0x11000080 0 0x80>;
+			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_LOW>;
+			clock-div = <16>;
+			clocks = <&pericfg CLK_PERI_I2C6>,
+				 <&pericfg CLK_PERI_AP_DMA>;
+			clock-names = "main", "dma";
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c6_pins_a>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
 	};
 
 };
-- 
1.8.1.1.dirty

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

* [PATCH v4 2/2] arm64: dts: mt8173: Add I2C device node
  2015-06-17 15:08 ` [PATCH v4 2/2] arm64: dts: mt8173: Add I2C " Eddie Huang
@ 2015-06-18 15:16   ` Daniel Kurtz
  2015-06-22  3:36     ` Eddie Huang
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Kurtz @ 2015-06-18 15:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jun 17, 2015 at 11:08 PM, Eddie Huang <eddie.huang@mediatek.com> wrote:
> Add MT8173 I2C device nodes, include I2C controllers and pins.
> MT8173 has six I2C controllers, from i2c0 to i2c6, exclude i2c5.
> The 6th I2C controller register base doesn't next to 5th I2C,
> and there is a hardware between 5th and 6th I2C controller. So
> SoC designer name 6th controller as "i2c6", not "i2c5".
>
> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi | 144 +++++++++++++++++++++++++++++++
>  1 file changed, 144 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index b52ec43..1816c8f 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -158,6 +158,54 @@
>                         interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
>                                                 <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>,
>                                                 <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
> +
> +                       i2c0_pins_a: i2c0 {
> +                               pins1 {
> +                                       pinmux = <MT8173_PIN_45_SDA0__FUNC_SDA0>,
> +                                                <MT8173_PIN_46_SCL0__FUNC_SCL0>;
> +                                       bias-disable;
> +                               };
> +                       };
> +
> +                       i2c1_pins_a: i2c1 {
> +                               pins1 {
> +                                       pinmux = <MT8173_PIN_125_SDA1__FUNC_SDA1>,
> +                                                <MT8173_PIN_126_SCL1__FUNC_SCL1>;
> +                                       bias-disable;
> +                               };
> +                       };
> +
> +                       i2c2_pins_a: i2c2 {
> +                               pins1 {
> +                                       pinmux = <MT8173_PIN_43_SDA2__FUNC_SDA2>,
> +                                                <MT8173_PIN_44_SCL2__FUNC_SCL2>;
> +                                       bias-disable;
> +                               };
> +                       };
> +
> +                       i2c3_pins_a: i2c3 {
> +                               pins1 {
> +                                       pinmux = <MT8173_PIN_106_SDA3__FUNC_SDA3>,
> +                                                <MT8173_PIN_107_SCL3__FUNC_SCL3>;
> +                                       bias-disable;
> +                               };
> +                       };
> +
> +                       i2c4_pins_a: i2c4 {
> +                               pins1 {
> +                                       pinmux = <MT8173_PIN_133_SDA4__FUNC_SDA4>,
> +                                                <MT8173_PIN_134_SCL4__FUNC_SCL4>;
> +                                       bias-disable;
> +                               };
> +                       };
> +
> +                       i2c6_pins_a: i2c6 {
> +                               pins1 {
> +                                       pinmux = <MT8173_PIN_100_MSDC2_DAT0__FUNC_SDA5>,
> +                                                <MT8173_PIN_101_MSDC2_DAT1__FUNC_SCL5>;
> +                                       bias-disable;
> +                               };
> +                       };
>                 };
>
>                 watchdog: watchdog at 10007000 {
> @@ -229,6 +277,102 @@
>                         clocks = <&uart_clk>;
>                         status = "disabled";
>                 };
> +
> +               i2c0: i2c at 11007000 {
> +                       compatible = "mediatek,mt8173-i2c";
> +                       reg = <0 0x11007000 0 0x70>,
> +                             <0 0x11000100 0 0x80>;
> +                       interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_LOW>;
> +                       clock-div = <16>;

According to the i2c-mt6577 dt binding:
   - clock-div: the fixed value for frequency divider of clock source in i2c
     module. Each IC may be different.

For other drivers I've seen this kind of hardware-specific value
implemented as a table in the driver that is indexed based on the
compatible.

Any particular reason to specify it here in every device tree node instead?

-Dan


> +                       clocks = <&pericfg CLK_PERI_I2C0>,
> +                                <&pericfg CLK_PERI_AP_DMA>;
> +                       clock-names = "main", "dma";
> +                       pinctrl-names = "default";
> +                       pinctrl-0 = <&i2c0_pins_a>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       status = "disabled";
> +               };
> +
> +               i2c1: i2c at 11008000 {
> +                       compatible = "mediatek,mt8173-i2c";
> +                       reg = <0 0x11008000 0 0x70>,
> +                             <0 0x11000180 0 0x80>;
> +                       interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_LOW>;
> +                       clock-div = <16>;
> +                       clocks = <&pericfg CLK_PERI_I2C1>,
> +                                <&pericfg CLK_PERI_AP_DMA>;
> +                       clock-names = "main", "dma";
> +                       pinctrl-names = "default";
> +                       pinctrl-0 = <&i2c1_pins_a>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       status = "disabled";
> +               };
> +
> +               i2c2: i2c at 11009000 {
> +                       compatible = "mediatek,mt8173-i2c";
> +                       reg = <0 0x11009000 0 0x70>,
> +                             <0 0x11000200 0 0x80>;
> +                       interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_LOW>;
> +                       clock-div = <16>;
> +                       clocks = <&pericfg CLK_PERI_I2C2>,
> +                                <&pericfg CLK_PERI_AP_DMA>;
> +                       clock-names = "main", "dma";
> +                       pinctrl-names = "default";
> +                       pinctrl-0 = <&i2c2_pins_a>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       status = "disabled";
> +               };
> +
> +               i2c3: i2c3 at 11010000 {
> +                       compatible = "mediatek,mt8173-i2c";
> +                       reg = <0 0x11010000 0 0x70>,
> +                             <0 0x11000280 0 0x80>;
> +                       interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_LOW>;
> +                       clock-div = <16>;
> +                       clocks = <&pericfg CLK_PERI_I2C3>,
> +                                <&pericfg CLK_PERI_AP_DMA>;
> +                       clock-names = "main", "dma";
> +                       pinctrl-names = "default";
> +                       pinctrl-0 = <&i2c3_pins_a>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       status = "disabled";
> +               };
> +
> +               i2c4: i2c4 at 11011000 {
> +                       compatible = "mediatek,mt8173-i2c";
> +                       reg = <0 0x11011000 0 0x70>,
> +                             <0 0x11000300 0 0x80>;
> +                       interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_LOW>;
> +                       clock-div = <16>;
> +                       clocks = <&pericfg CLK_PERI_I2C4>,
> +                                <&pericfg CLK_PERI_AP_DMA>;
> +                       clock-names = "main", "dma";
> +                       pinctrl-names = "default";
> +                       pinctrl-0 = <&i2c4_pins_a>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       status = "disabled";
> +               };
> +
> +               i2c6: i2c6 at 11013000 {
> +                       compatible = "mediatek,mt8173-i2c";
> +                       reg = <0 0x11013000 0 0x70>,
> +                             <0 0x11000080 0 0x80>;
> +                       interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_LOW>;
> +                       clock-div = <16>;
> +                       clocks = <&pericfg CLK_PERI_I2C6>,
> +                                <&pericfg CLK_PERI_AP_DMA>;
> +                       clock-names = "main", "dma";
> +                       pinctrl-names = "default";
> +                       pinctrl-0 = <&i2c6_pins_a>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       status = "disabled";
> +               };
>         };
>
>  };
> --
> 1.8.1.1.dirty
>

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

* [PATCH v4 2/2] arm64: dts: mt8173: Add I2C device node
  2015-06-18 15:16   ` Daniel Kurtz
@ 2015-06-22  3:36     ` Eddie Huang
  2015-06-22  6:32       ` Daniel Kurtz
  0 siblings, 1 reply; 6+ messages in thread
From: Eddie Huang @ 2015-06-22  3:36 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dan,

On Thu, 2015-06-18 at 23:16 +0800, Daniel Kurtz wrote:
> On Wed, Jun 17, 2015 at 11:08 PM, Eddie Huang <eddie.huang@mediatek.com> wrote:
> > Add MT8173 I2C device nodes, include I2C controllers and pins.
> > MT8173 has six I2C controllers, from i2c0 to i2c6, exclude i2c5.
> > The 6th I2C controller register base doesn't next to 5th I2C,
> > and there is a hardware between 5th and 6th I2C controller. So
> > SoC designer name 6th controller as "i2c6", not "i2c5".
> >
> > Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8173.dtsi | 144 +++++++++++++++++++++++++++++++
> >  1 file changed, 144 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > index b52ec43..1816c8f 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > @@ -158,6 +158,54 @@
> >                         interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
> >                                                 <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>,
> >                                                 <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +                       i2c0_pins_a: i2c0 {
> > +                               pins1 {
> > +                                       pinmux = <MT8173_PIN_45_SDA0__FUNC_SDA0>,
> > +                                                <MT8173_PIN_46_SCL0__FUNC_SCL0>;
> > +                                       bias-disable;
> > +                               };
> > +                       };
> > +
> > +                       i2c1_pins_a: i2c1 {
> > +                               pins1 {
> > +                                       pinmux = <MT8173_PIN_125_SDA1__FUNC_SDA1>,
> > +                                                <MT8173_PIN_126_SCL1__FUNC_SCL1>;
> > +                                       bias-disable;
> > +                               };
> > +                       };
> > +
> > +                       i2c2_pins_a: i2c2 {
> > +                               pins1 {
> > +                                       pinmux = <MT8173_PIN_43_SDA2__FUNC_SDA2>,
> > +                                                <MT8173_PIN_44_SCL2__FUNC_SCL2>;
> > +                                       bias-disable;
> > +                               };
> > +                       };
> > +
> > +                       i2c3_pins_a: i2c3 {
> > +                               pins1 {
> > +                                       pinmux = <MT8173_PIN_106_SDA3__FUNC_SDA3>,
> > +                                                <MT8173_PIN_107_SCL3__FUNC_SCL3>;
> > +                                       bias-disable;
> > +                               };
> > +                       };
> > +
> > +                       i2c4_pins_a: i2c4 {
> > +                               pins1 {
> > +                                       pinmux = <MT8173_PIN_133_SDA4__FUNC_SDA4>,
> > +                                                <MT8173_PIN_134_SCL4__FUNC_SCL4>;
> > +                                       bias-disable;
> > +                               };
> > +                       };
> > +
> > +                       i2c6_pins_a: i2c6 {
> > +                               pins1 {
> > +                                       pinmux = <MT8173_PIN_100_MSDC2_DAT0__FUNC_SDA5>,
> > +                                                <MT8173_PIN_101_MSDC2_DAT1__FUNC_SCL5>;
> > +                                       bias-disable;
> > +                               };
> > +                       };
> >                 };
> >
> >                 watchdog: watchdog at 10007000 {
> > @@ -229,6 +277,102 @@
> >                         clocks = <&uart_clk>;
> >                         status = "disabled";
> >                 };
> > +
> > +               i2c0: i2c at 11007000 {
> > +                       compatible = "mediatek,mt8173-i2c";
> > +                       reg = <0 0x11007000 0 0x70>,
> > +                             <0 0x11000100 0 0x80>;
> > +                       interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_LOW>;
> > +                       clock-div = <16>;
> 
> According to the i2c-mt6577 dt binding:
>    - clock-div: the fixed value for frequency divider of clock source in i2c
>      module. Each IC may be different.
> 
> For other drivers I've seen this kind of hardware-specific value
> implemented as a table in the driver that is indexed based on the
> compatible.
> 
> Any particular reason to specify it here in every device tree node instead?
> 

If put in device tree, it is not necessary to add new compatible if new
SoC has the same I2C controller hardware except clock-div.The benefit is
keep driver clean, but the side-effect is add clock-div in device node.I
assume clock-div has the same concept of clock, so I put in device tree.

Eddie
Thanks

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

* [PATCH v4 2/2] arm64: dts: mt8173: Add I2C device node
  2015-06-22  3:36     ` Eddie Huang
@ 2015-06-22  6:32       ` Daniel Kurtz
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Kurtz @ 2015-06-22  6:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jun 22, 2015 at 11:36 AM, Eddie Huang <eddie.huang@mediatek.com> wrote:
> Hi Dan,
>
> On Thu, 2015-06-18 at 23:16 +0800, Daniel Kurtz wrote:
>> On Wed, Jun 17, 2015 at 11:08 PM, Eddie Huang <eddie.huang@mediatek.com> wrote:
>> > Add MT8173 I2C device nodes, include I2C controllers and pins.
>> > MT8173 has six I2C controllers, from i2c0 to i2c6, exclude i2c5.
>> > The 6th I2C controller register base doesn't next to 5th I2C,
>> > and there is a hardware between 5th and 6th I2C controller. So
>> > SoC designer name 6th controller as "i2c6", not "i2c5".
>> >
>> > Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
>> > ---
>> >  arch/arm64/boot/dts/mediatek/mt8173.dtsi | 144 +++++++++++++++++++++++++++++++
>> >  1 file changed, 144 insertions(+)
>> >
>> > diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> > index b52ec43..1816c8f 100644
>> > --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> > +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> > @@ -158,6 +158,54 @@
>> >                         interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
>> >                                                 <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>,
>> >                                                 <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
>> > +
>> > +                       i2c0_pins_a: i2c0 {
>> > +                               pins1 {
>> > +                                       pinmux = <MT8173_PIN_45_SDA0__FUNC_SDA0>,
>> > +                                                <MT8173_PIN_46_SCL0__FUNC_SCL0>;
>> > +                                       bias-disable;
>> > +                               };
>> > +                       };
>> > +
>> > +                       i2c1_pins_a: i2c1 {
>> > +                               pins1 {
>> > +                                       pinmux = <MT8173_PIN_125_SDA1__FUNC_SDA1>,
>> > +                                                <MT8173_PIN_126_SCL1__FUNC_SCL1>;
>> > +                                       bias-disable;
>> > +                               };
>> > +                       };
>> > +
>> > +                       i2c2_pins_a: i2c2 {
>> > +                               pins1 {
>> > +                                       pinmux = <MT8173_PIN_43_SDA2__FUNC_SDA2>,
>> > +                                                <MT8173_PIN_44_SCL2__FUNC_SCL2>;
>> > +                                       bias-disable;
>> > +                               };
>> > +                       };
>> > +
>> > +                       i2c3_pins_a: i2c3 {
>> > +                               pins1 {
>> > +                                       pinmux = <MT8173_PIN_106_SDA3__FUNC_SDA3>,
>> > +                                                <MT8173_PIN_107_SCL3__FUNC_SCL3>;
>> > +                                       bias-disable;
>> > +                               };
>> > +                       };
>> > +
>> > +                       i2c4_pins_a: i2c4 {
>> > +                               pins1 {
>> > +                                       pinmux = <MT8173_PIN_133_SDA4__FUNC_SDA4>,
>> > +                                                <MT8173_PIN_134_SCL4__FUNC_SCL4>;
>> > +                                       bias-disable;
>> > +                               };
>> > +                       };
>> > +
>> > +                       i2c6_pins_a: i2c6 {
>> > +                               pins1 {
>> > +                                       pinmux = <MT8173_PIN_100_MSDC2_DAT0__FUNC_SDA5>,
>> > +                                                <MT8173_PIN_101_MSDC2_DAT1__FUNC_SCL5>;
>> > +                                       bias-disable;
>> > +                               };
>> > +                       };
>> >                 };
>> >
>> >                 watchdog: watchdog at 10007000 {
>> > @@ -229,6 +277,102 @@
>> >                         clocks = <&uart_clk>;
>> >                         status = "disabled";
>> >                 };
>> > +
>> > +               i2c0: i2c at 11007000 {
>> > +                       compatible = "mediatek,mt8173-i2c";
>> > +                       reg = <0 0x11007000 0 0x70>,
>> > +                             <0 0x11000100 0 0x80>;
>> > +                       interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_LOW>;
>> > +                       clock-div = <16>;
>>
>> According to the i2c-mt6577 dt binding:
>>    - clock-div: the fixed value for frequency divider of clock source in i2c
>>      module. Each IC may be different.
>>
>> For other drivers I've seen this kind of hardware-specific value
>> implemented as a table in the driver that is indexed based on the
>> compatible.
>>
>> Any particular reason to specify it here in every device tree node instead?
>>
>
> If put in device tree, it is not necessary to add new compatible if new
> SoC has the same I2C controller hardware except clock-div.The benefit is
> keep driver clean, but the side-effect is add clock-div in device node.I
> assume clock-div has the same concept of clock, so I put in device tree.

Sounds reasonable to me.  This patch is:

Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

> Eddie
> Thanks
>
>

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

end of thread, other threads:[~2015-06-22  6:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17 15:08 [PATCH v4 0/2] arm64: dts: Mediatek: MT8173 updtes Eddie Huang
2015-06-17 15:08 ` [PATCH v4 1/2] arm64: dts: mt8173: Add watchdog device node Eddie Huang
2015-06-17 15:08 ` [PATCH v4 2/2] arm64: dts: mt8173: Add I2C " Eddie Huang
2015-06-18 15:16   ` Daniel Kurtz
2015-06-22  3:36     ` Eddie Huang
2015-06-22  6:32       ` Daniel Kurtz

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).