Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1643120 > unrolled thread

[PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660

Started byGuodong Xu <guodong.xu@linaro.org>
First post2017-05-17 10:40 +0200
Last post2017-05-24 10:40 +0200
Articles 8 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660 Guodong Xu <guodong.xu@linaro.org> - 2017-05-17 10:40 +0200
    Re: [PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660 Rob Herring <robh@kernel.org> - 2017-05-23 02:40 +0200
      Re: [PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660 zhangfei <zhangfei.gao@linaro.org> - 2017-05-23 08:00 +0200
        Re: [PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660 zhangfei <zhangfei.gao@linaro.org> - 2017-05-23 08:40 +0200
          Re: [PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660 Rob Herring <robh@kernel.org> - 2017-05-23 14:50 +0200
        Re: [PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660 Rob Herring <robh@kernel.org> - 2017-05-23 14:50 +0200
          Re: [PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660 zhangfei <zhangfei.gao@linaro.org> - 2017-05-24 04:40 +0200
            Re: [PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660 Jarkko Nikula <jarkko.nikula@linux.intel.com> - 2017-05-24 10:40 +0200

#1643120 — [PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660

FromGuodong Xu <guodong.xu@linaro.org>
Date2017-05-17 10:40 +0200
Subject[PATCH 05/12] arm64: dts: Add I2C nodes for Hi3660
Message-ID<tI2EV-J3-9@gated-at.bofh.it>
From: Zhangfei Gao <zhangfei.gao@linaro.org>

Add I2C nodes for Hi3660-hikey960.

On HiKey960,
I2C0, I2C7 is connected to Low Speed Expansion Connector.
I2C1 is connected to ADV7535.
I2C3 is connected to USB5734.

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 18 ++++++++
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi         | 56 +++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
index 64875a5..f685b1e 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
@@ -29,6 +29,24 @@
 	};
 };
 
+&i2c0 {
+	status = "okay";
+};
+
+&i2c1 {
+	status = "okay";
+
+	adv7533: adv7533@39 {
+		status = "ok";
+		compatible = "adi,adv7533";
+		reg = <0x39>;
+	};
+};
+
+&i2c7 {
+	status = "okay";
+};
+
 &uart5 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index f55710a..f217c9d 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -186,6 +186,62 @@
 			#reset-cells = <2>;
 		};
 
+		i2c0: i2c@FFD71000 {
+			compatible = "snps,designware-i2c";
+			reg = <0x0 0xFFD71000 0x0 0x1000>;
+			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clock-frequency = <400000>;
+			clocks = <&crg_ctrl HI3660_CLK_GATE_I2C0>;
+			resets = <&iomcu_rst 0x20 3>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>;
+			status = "disabled";
+		};
+
+		i2c1: i2c@FFD72000 {
+			compatible = "snps,designware-i2c";
+			reg = <0x0 0xFFD72000 0x0 0x1000>;
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clock-frequency = <400000>;
+			clocks = <&crg_ctrl HI3660_CLK_GATE_I2C1>;
+			resets = <&iomcu_rst 0x20 4>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c1_pmx_func &i2c1_cfg_func>;
+			status = "disabled";
+		};
+
+		i2c3: i2c@FDF0C000 {
+			compatible = "snps,designware-i2c";
+			reg = <0x0 0xFDF0C000 0x0 0x1000>;
+			interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clock-frequency = <400000>;
+			clocks = <&crg_ctrl HI3660_CLK_GATE_I2C3>;
+			resets = <&crg_rst 0x78 7>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c3_pmx_func &i2c3_cfg_func>;
+			status = "disabled";
+		};
+
+		i2c7: i2c@FDF0B000 {
+			compatible = "snps,designware-i2c";
+			reg = <0x0 0xFDF0B000 0x0 0x1000>;
+			interrupts = <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clock-frequency = <400000>;
+			clocks = <&crg_ctrl HI3660_CLK_GATE_I2C7>;
+			resets = <&crg_rst 0x60 14>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c7_pmx_func &i2c7_cfg_func>;
+			status = "disabled";
+		};
+
 		uart5: serial@fdf05000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x0 0xfdf05000 0x0 0x1000>;
-- 
2.10.2

[toc] | [next] | [standalone]

#1647516

FromRob Herring <robh@kernel.org>
Date2017-05-23 02:40 +0200
Message-ID<tK61I-4fZ-15@gated-at.bofh.it>
In reply to#1643120
On Wed, May 17, 2017 at 04:37:38PM +0800, Guodong Xu wrote:
> From: Zhangfei Gao <zhangfei.gao@linaro.org>
> 
> Add I2C nodes for Hi3660-hikey960.
> 
> On HiKey960,
> I2C0, I2C7 is connected to Low Speed Expansion Connector.
> I2C1 is connected to ADV7535.
> I2C3 is connected to USB5734.
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
> ---
>  arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 18 ++++++++
>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi         | 56 +++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> index 64875a5..f685b1e 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> @@ -29,6 +29,24 @@
>  	};
>  };
>  
> +&i2c0 {
> +	status = "okay";
> +};
> +
> +&i2c1 {
> +	status = "okay";
> +
> +	adv7533: adv7533@39 {
> +		status = "ok";
> +		compatible = "adi,adv7533";
> +		reg = <0x39>;
> +	};
> +};
> +
> +&i2c7 {
> +	status = "okay";
> +};

labels for the LS connector?

> +
>  &uart5 {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index f55710a..f217c9d 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -186,6 +186,62 @@
>  			#reset-cells = <2>;
>  		};
>  
> +		i2c0: i2c@FFD71000 {

lowercase hex please.

> +			compatible = "snps,designware-i2c";

These should have an SoC specific compatible.

> +			reg = <0x0 0xFFD71000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <400000>;
> +			clocks = <&crg_ctrl HI3660_CLK_GATE_I2C0>;
> +			resets = <&iomcu_rst 0x20 3>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>;
> +			status = "disabled";
> +		};
> +
> +		i2c1: i2c@FFD72000 {

ditto

> +			compatible = "snps,designware-i2c";
> +			reg = <0x0 0xFFD72000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <400000>;
> +			clocks = <&crg_ctrl HI3660_CLK_GATE_I2C1>;
> +			resets = <&iomcu_rst 0x20 4>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&i2c1_pmx_func &i2c1_cfg_func>;
> +			status = "disabled";
> +		};
> +
> +		i2c3: i2c@FDF0C000 {
> +			compatible = "snps,designware-i2c";
> +			reg = <0x0 0xFDF0C000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <400000>;
> +			clocks = <&crg_ctrl HI3660_CLK_GATE_I2C3>;
> +			resets = <&crg_rst 0x78 7>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&i2c3_pmx_func &i2c3_cfg_func>;
> +			status = "disabled";
> +		};
> +
> +		i2c7: i2c@FDF0B000 {
> +			compatible = "snps,designware-i2c";
> +			reg = <0x0 0xFDF0B000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <400000>;
> +			clocks = <&crg_ctrl HI3660_CLK_GATE_I2C7>;
> +			resets = <&crg_rst 0x60 14>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&i2c7_pmx_func &i2c7_cfg_func>;
> +			status = "disabled";
> +		};
> +
>  		uart5: serial@fdf05000 {
>  			compatible = "arm,pl011", "arm,primecell";
>  			reg = <0x0 0xfdf05000 0x0 0x1000>;
> -- 
> 2.10.2
>

[toc] | [prev] | [next] | [standalone]

#1647656

Fromzhangfei <zhangfei.gao@linaro.org>
Date2017-05-23 08:00 +0200
Message-ID<tKb1n-7nt-1@gated-at.bofh.it>
In reply to#1647516
Hi, Rob


Thanks for the review.

On 2017年05月23日 08:39, Rob Herring wrote:
> On Wed, May 17, 2017 at 04:37:38PM +0800, Guodong Xu wrote:
>> From: Zhangfei Gao <zhangfei.gao@linaro.org>
>>
>> Add I2C nodes for Hi3660-hikey960.
>>
>> On HiKey960,
>> I2C0, I2C7 is connected to Low Speed Expansion Connector.
>> I2C1 is connected to ADV7535.
>> I2C3 is connected to USB5734.
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
>> ---
>>   arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 18 ++++++++
>>   arch/arm64/boot/dts/hisilicon/hi3660.dtsi         | 56 +++++++++++++++++++++++
>>   2 files changed, 74 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>> index 64875a5..f685b1e 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>> @@ -29,6 +29,24 @@
>>   	};
>>   };
>>   
>> +&i2c0 {
>> +	status = "okay";
>> +};
>> +
>> +&i2c1 {
>> +	status = "okay";
>> +
>> +	adv7533: adv7533@39 {
>> +		status = "ok";
>> +		compatible = "adi,adv7533";
>> +		reg = <0x39>;
>> +	};
>> +};
>> +
>> +&i2c7 {
>> +	status = "okay";
>> +};
> labels for the LS connector?
Any examples?
There is compile error if only change dts like
ls-connector {
         &i2c7 {
                 status = "okay";
         };
};

>
>> +
>>   &uart5 {
>>   	status = "okay";
>>   };
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> index f55710a..f217c9d 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> @@ -186,6 +186,62 @@
>>   			#reset-cells = <2>;
>>   		};
>>   
>> +		i2c0: i2c@FFD71000 {
> lowercase hex please.
Yes, will change
>
>> +			compatible = "snps,designware-i2c";
> These should have an SoC specific compatible.
We directly use drivers/i2c/busses/i2c-designware-platdrv.c,
do we still an soc specific compatible?
Checked arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi, and other examples,

compatible = "snps,designware-i2c" is used.


Thanks

[toc] | [prev] | [next] | [standalone]

#1647672

Fromzhangfei <zhangfei.gao@linaro.org>
Date2017-05-23 08:40 +0200
Message-ID<tKbE5-7Qs-3@gated-at.bofh.it>
In reply to#1647656

On 2017年05月23日 13:55, zhangfei wrote:
> Hi, Rob
>
>
> Thanks for the review.
>
> On 2017年05月23日 08:39, Rob Herring wrote:
>> On Wed, May 17, 2017 at 04:37:38PM +0800, Guodong Xu wrote:
>>> From: Zhangfei Gao <zhangfei.gao@linaro.org>
>>>
>>> Add I2C nodes for Hi3660-hikey960.
>>>
>>> On HiKey960,
>>> I2C0, I2C7 is connected to Low Speed Expansion Connector.
>>> I2C1 is connected to ADV7535.
>>> I2C3 is connected to USB5734.
>>>
>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>>> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
>>> ---
>>>   arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 18 ++++++++
>>>   arch/arm64/boot/dts/hisilicon/hi3660.dtsi         | 56 
>>> +++++++++++++++++++++++
>>>   2 files changed, 74 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts 
>>> b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>> index 64875a5..f685b1e 100644
>>> --- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>> @@ -29,6 +29,24 @@
>>>       };
>>>   };
>>>   +&i2c0 {
>>> +    status = "okay";
>>> +};
>>> +
>>> +&i2c1 {
>>> +    status = "okay";
>>> +
>>> +    adv7533: adv7533@39 {
>>> +        status = "ok";
>>> +        compatible = "adi,adv7533";
>>> +        reg = <0x39>;
>>> +    };
>>> +};
>>> +
>>> +&i2c7 {
>>> +    status = "okay";
>>> +};
>> labels for the LS connector?

Do you mean arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

                 i2c@78ba000 {
                 /* On Low speed expansion */
                         label = "LS-I2C1";
                         status = "okay";
                 };

                 spi@78b7000 {
                 /* On High speed expansion */
                         label = "HS-SPI1";
                         status = "okay";
                 };

Thanks
>>
>>> +            compatible = "snps,designware-i2c";
>> These should have an SoC specific compatible.
> We directly use drivers/i2c/busses/i2c-designware-platdrv.c,
> do we still an soc specific compatible?
> Checked arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi, and other examples,
>
> compatible = "snps,designware-i2c" is used.
>
>
> Thanks
>

[toc] | [prev] | [next] | [standalone]

#1647993

FromRob Herring <robh@kernel.org>
Date2017-05-23 14:50 +0200
Message-ID<tKhqa-3g1-13@gated-at.bofh.it>
In reply to#1647672
On Tue, May 23, 2017 at 1:36 AM, zhangfei <zhangfei.gao@linaro.org> wrote:
>
>
> On 2017年05月23日 13:55, zhangfei wrote:
>>
>> Hi, Rob
>>
>>
>> Thanks for the review.
>>
>> On 2017年05月23日 08:39, Rob Herring wrote:
>>>
>>> On Wed, May 17, 2017 at 04:37:38PM +0800, Guodong Xu wrote:
>>>>
>>>> From: Zhangfei Gao <zhangfei.gao@linaro.org>
>>>>
>>>> Add I2C nodes for Hi3660-hikey960.
>>>>
>>>> On HiKey960,
>>>> I2C0, I2C7 is connected to Low Speed Expansion Connector.
>>>> I2C1 is connected to ADV7535.
>>>> I2C3 is connected to USB5734.
>>>>
>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>>>> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
>>>> ---
>>>>   arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 18 ++++++++
>>>>   arch/arm64/boot/dts/hisilicon/hi3660.dtsi         | 56
>>>> +++++++++++++++++++++++
>>>>   2 files changed, 74 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>>> b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>>> index 64875a5..f685b1e 100644
>>>> --- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
>>>> @@ -29,6 +29,24 @@
>>>>       };
>>>>   };
>>>>   +&i2c0 {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&i2c1 {
>>>> +    status = "okay";
>>>> +
>>>> +    adv7533: adv7533@39 {
>>>> +        status = "ok";
>>>> +        compatible = "adi,adv7533";
>>>> +        reg = <0x39>;
>>>> +    };
>>>> +};
>>>> +
>>>> +&i2c7 {
>>>> +    status = "okay";
>>>> +};
>>>
>>> labels for the LS connector?
>
>
> Do you mean arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
>
>                 i2c@78ba000 {
>                 /* On Low speed expansion */
>                         label = "LS-I2C1";
>                         status = "okay";
>                 };
>
>                 spi@78b7000 {
>                 /* On High speed expansion */
>                         label = "HS-SPI1";
>                         status = "okay";
>                 };

Yes. And the names need to align across boards to be useful.

Rob

[toc] | [prev] | [next] | [standalone]

#1647992

FromRob Herring <robh@kernel.org>
Date2017-05-23 14:50 +0200
Message-ID<tKhqa-3g1-11@gated-at.bofh.it>
In reply to#1647656
On Tue, May 23, 2017 at 12:55 AM, zhangfei <zhangfei.gao@linaro.org> wrote:
> Hi, Rob
>
>
> Thanks for the review.
>
>
> On 2017年05月23日 08:39, Rob Herring wrote:
>>
>> On Wed, May 17, 2017 at 04:37:38PM +0800, Guodong Xu wrote:
>>>
>>> From: Zhangfei Gao <zhangfei.gao@linaro.org>
>>>
>>> Add I2C nodes for Hi3660-hikey960.
>>>
>>> On HiKey960,
>>> I2C0, I2C7 is connected to Low Speed Expansion Connector.
>>> I2C1 is connected to ADV7535.
>>> I2C3 is connected to USB5734.

[...]

>>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>>> b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>>> index f55710a..f217c9d 100644
>>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>>> @@ -186,6 +186,62 @@
>>>                         #reset-cells = <2>;
>>>                 };
>>>   +             i2c0: i2c@FFD71000 {
>>
>> lowercase hex please.
>
> Yes, will change
>>
>>
>>> +                       compatible = "snps,designware-i2c";
>>
>> These should have an SoC specific compatible.
>
> We directly use drivers/i2c/busses/i2c-designware-platdrv.c,
> do we still an soc specific compatible?

It's fine if the driver uses the snps compatible, but the dts should
still have an SoC specific one.

> Checked arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi, and other examples,
>
> compatible = "snps,designware-i2c" is used.

That was a mistake in the other platforms. We shouldn't continue repeating it.

Rob

[toc] | [prev] | [next] | [standalone]

#1649066

Fromzhangfei <zhangfei.gao@linaro.org>
Date2017-05-24 04:40 +0200
Message-ID<tKunn-4kt-3@gated-at.bofh.it>
In reply to#1647992
Hi, Jarkko

Would you mind give some suggestion?

On 2017年05月23日 20:48, Rob Herring wrote:
>
>>>> +                       compatible = "snps,designware-i2c";
>>> These should have an SoC specific compatible.
>> We directly use drivers/i2c/busses/i2c-designware-platdrv.c,
>> do we still an soc specific compatible?
> It's fine if the driver uses the snps compatible, but the dts should
> still have an SoC specific one.
>
>> Checked arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi, and other examples,
>>
>> compatible = "snps,designware-i2c" is used.
> That was a mistake in the other platforms. We shouldn't continue repeating it.
>
> Rob

Rob suggest add something like "hisilicon,hi3660-dw-i2c" as well.
"The problem is dw-i2c does not give any clue as to what the 
configuration or version of the IP is.
Is that fully discoverable with version/capability registers? If not 
then you need a specific compatible.
Generally when we have not required them, it ends up being a problem 
later on."


While Documentation/devicetree/bindings/i2c/i2c-designware.txt
compatible : should be "snps,designware-i2c"

Besides, on Hikey960,
[    3.822353] dw_readl(dev, DW_IC_COMP_VERSION)=0x3132302a
[    3.827763] dw_readl(dev, DW_IC_COMP_TYPE)=0x44570140
Are these two registers enough to distinguish version etc?

Thanks

[toc] | [prev] | [next] | [standalone]

#1649291

FromJarkko Nikula <jarkko.nikula@linux.intel.com>
Date2017-05-24 10:40 +0200
Message-ID<tKzZO-8mT-55@gated-at.bofh.it>
In reply to#1649066
On 05/24/2017 05:34 AM, zhangfei wrote:
> Rob suggest add something like "hisilicon,hi3660-dw-i2c" as well.
> "The problem is dw-i2c does not give any clue as to what the
> configuration or version of the IP is.
> Is that fully discoverable with version/capability registers? If not
> then you need a specific compatible.
> Generally when we have not required them, it ends up being a problem
> later on."
>
Some features are discoverable from DW_IC_COMP_PARAM_1, 
DW_IC_COMP_VERSION and DW_IC_COMP_TYPE registers. Although my 
specification does not document what's inside of DW_IC_COMP_VERSION and 
DW_IC_COMP_TYPE but code is using them.

>
> While Documentation/devicetree/bindings/i2c/i2c-designware.txt
> compatible : should be "snps,designware-i2c"
>
> Besides, on Hikey960,
> [    3.822353] dw_readl(dev, DW_IC_COMP_VERSION)=0x3132302a
> [    3.827763] dw_readl(dev, DW_IC_COMP_TYPE)=0x44570140
> Are these two registers enough to distinguish version etc?
>
I've seen DW_IC_COMP_VERSION being 0x3131352a or 0x3132312a in our 
platforms. DW_IC_COMP_TYPE has the same value what you are seeing.

-- 
Jarkko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web