2022-08-25 09:27:45

by chenweilong

[permalink] [raw]
Subject: [PATCH next v2 1/2] i2c: i2c-hisi: Add support for initializing control module via DT

The HiSilicon I2C controller can be used on embedded platform, which
boot from devicetree.

Signed-off-by: Weilong Chen <[email protected]>
---
drivers/i2c/busses/Kconfig | 2 +-
drivers/i2c/busses/i2c-hisi.c | 13 ++++++++++++-
2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 7284206b278b..6d0fdf48e97d 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -673,7 +673,7 @@ config I2C_HIGHLANDER

config I2C_HISI
tristate "HiSilicon I2C controller"
- depends on (ARM64 && ACPI) || COMPILE_TEST
+ depends on ARM64 || COMPILE_TEST
help
Say Y here if you want to have Hisilicon I2C controller support
available on the Kunpeng Server.
diff --git a/drivers/i2c/busses/i2c-hisi.c b/drivers/i2c/busses/i2c-hisi.c
index 76c3d8f6fc3c..cba9a6830b23 100644
--- a/drivers/i2c/busses/i2c-hisi.c
+++ b/drivers/i2c/busses/i2c-hisi.c
@@ -16,6 +16,8 @@
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/units.h>
+#include <linux/acpi.h>
+#include <linux/of.h>

#define HISI_I2C_FRAME_CTRL 0x0000
#define HISI_I2C_FRAME_CTRL_SPEED_MODE GENMASK(1, 0)
@@ -483,17 +485,26 @@ static int hisi_i2c_probe(struct platform_device *pdev)
return 0;
}

+#ifdef CONFIG_ACPI
static const struct acpi_device_id hisi_i2c_acpi_ids[] = {
{ "HISI03D1", 0 },
{ }
};
MODULE_DEVICE_TABLE(acpi, hisi_i2c_acpi_ids);
+#endif
+
+static const struct of_device_id hisi_i2c_dts_ids[] = {
+ { .compatible = "hisilicon,hisi-i2c", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, hisi_i2c_dts_ids);

static struct platform_driver hisi_i2c_driver = {
.probe = hisi_i2c_probe,
.driver = {
.name = "hisi-i2c",
- .acpi_match_table = hisi_i2c_acpi_ids,
+ .acpi_match_table = ACPI_PTR(hisi_i2c_acpi_ids),
+ .of_match_table = of_match_ptr(hisi_i2c_dts_ids),
},
};
module_platform_driver(hisi_i2c_driver);
--
2.31.GIT


2022-08-25 09:54:20

by chenweilong

[permalink] [raw]
Subject: [PATCH next v2 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c

Add the new compatible for hisi common i2c.

Signed-off-by: Weilong Chen <[email protected]>
---
.../bindings/i2c/hisilicon,hisi-i2c.yaml | 50 +++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
new file mode 100644
index 000000000000..ea967abfe144
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/i2c/hisilicon,hisi-i2c.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Hisilicon common IIC controller Device Tree Bindings
+
+maintainers:
+ - [email protected]
+
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+ compatible:
+ const: hisilicon,hisi-i2c
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clk_rate:
+ maxItems: 1
+
+ clock-frequency:
+ default: 100000
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clk_rate
+ - clock-frequency
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c1: i2c@5038B0000{
+ compatible = "hisilicon,hisi-i2c";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x5 0x038B0000 0 0x10000>;
+ interrupts = <0x0 120 0x4>;
+ clk_rate = <0x0 0xEE6B280>;
+ clock-frequency = <400000>;
+ };
--
2.31.GIT

2022-08-31 10:00:43

by Yicong Yang

[permalink] [raw]
Subject: Re: [PATCH next v2 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c

[+cc xuwei for device tree expertise]

On 2022/8/25 17:24, Weilong Chen wrote:
> Add the new compatible for hisi common i2c.
>

s/hisi/HiSilicon

> Signed-off-by: Weilong Chen <[email protected]>
> ---
> .../bindings/i2c/hisilicon,hisi-i2c.yaml | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
>
> diff --git a/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
> new file mode 100644
> index 000000000000..ea967abfe144
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/i2c/hisilicon,hisi-i2c.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Hisilicon common IIC controller Device Tree Bindings
> +

It's ok to have "HiSilicon common IIC controller"

> +maintainers:
> + - [email protected]
> +
> +allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> + compatible:
> + const: hisilicon,hisi-i2c
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clk_rate:
> + maxItems: 1
> +
> + clock-frequency:
> + default: 100000
> +

I think it misses some properties here?

- i2c-sda-hold-time-ns
- sda_fall_ns
- scl_rise_ns
...

Do we need to mention them here?

Thanks.

> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clk_rate
> + - clock-frequency
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + i2c1: i2c@5038B0000{
> + compatible = "hisilicon,hisi-i2c";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x5 0x038B0000 0 0x10000>;
> + interrupts = <0x0 120 0x4>;
> + clk_rate = <0x0 0xEE6B280>;
> + clock-frequency = <400000>;
> + };
>

2022-08-31 10:40:16

by Yicong Yang

[permalink] [raw]
Subject: Re: [PATCH next v2 1/2] i2c: i2c-hisi: Add support for initializing control module via DT

Hi Weilong,

On 2022/8/25 17:24, Weilong Chen wrote:
> The HiSilicon I2C controller can be used on embedded platform, which
> boot from devicetree.
>

My comments in v1 remain unresolved and they still apply.

https://lore.kernel.org/linux-i2c/[email protected]/

Thanks.

> Signed-off-by: Weilong Chen <[email protected]>
> ---
> drivers/i2c/busses/Kconfig | 2 +-
> drivers/i2c/busses/i2c-hisi.c | 13 ++++++++++++-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 7284206b278b..6d0fdf48e97d 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -673,7 +673,7 @@ config I2C_HIGHLANDER
>
> config I2C_HISI
> tristate "HiSilicon I2C controller"
> - depends on (ARM64 && ACPI) || COMPILE_TEST
> + depends on ARM64 || COMPILE_TEST
> help
> Say Y here if you want to have Hisilicon I2C controller support
> available on the Kunpeng Server.
> diff --git a/drivers/i2c/busses/i2c-hisi.c b/drivers/i2c/busses/i2c-hisi.c
> index 76c3d8f6fc3c..cba9a6830b23 100644
> --- a/drivers/i2c/busses/i2c-hisi.c
> +++ b/drivers/i2c/busses/i2c-hisi.c
> @@ -16,6 +16,8 @@
> #include <linux/platform_device.h>
> #include <linux/property.h>
> #include <linux/units.h>
> +#include <linux/acpi.h>
> +#include <linux/of.h>
>
> #define HISI_I2C_FRAME_CTRL 0x0000
> #define HISI_I2C_FRAME_CTRL_SPEED_MODE GENMASK(1, 0)
> @@ -483,17 +485,26 @@ static int hisi_i2c_probe(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_ACPI
> static const struct acpi_device_id hisi_i2c_acpi_ids[] = {
> { "HISI03D1", 0 },
> { }
> };
> MODULE_DEVICE_TABLE(acpi, hisi_i2c_acpi_ids);
> +#endif
> +
> +static const struct of_device_id hisi_i2c_dts_ids[] = {
> + { .compatible = "hisilicon,hisi-i2c", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, hisi_i2c_dts_ids);
>
> static struct platform_driver hisi_i2c_driver = {
> .probe = hisi_i2c_probe,
> .driver = {
> .name = "hisi-i2c",
> - .acpi_match_table = hisi_i2c_acpi_ids,
> + .acpi_match_table = ACPI_PTR(hisi_i2c_acpi_ids),
> + .of_match_table = of_match_ptr(hisi_i2c_dts_ids),
> },
> };
> module_platform_driver(hisi_i2c_driver);
>

2022-09-02 01:20:59

by chenweilong

[permalink] [raw]
Subject: Re: [PATCH next v2 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c

OK, I'll repost after correction.

tks.
On 2022/8/31 17:53, Yicong Yang wrote:
> [+cc xuwei for device tree expertise]
>
> On 2022/8/25 17:24, Weilong Chen wrote:
>> Add the new compatible for hisi common i2c.
>>
> s/hisi/HiSilicon
>
>> Signed-off-by: Weilong Chen <[email protected]>
>> ---
>> .../bindings/i2c/hisilicon,hisi-i2c.yaml | 50 +++++++++++++++++++
>> 1 file changed, 50 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
>> new file mode 100644
>> index 000000000000..ea967abfe144
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
>> @@ -0,0 +1,50 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/i2c/hisilicon,hisi-i2c.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Hisilicon common IIC controller Device Tree Bindings
>> +
> It's ok to have "HiSilicon common IIC controller"
>
>> +maintainers:
>> + - [email protected]
>> +
>> +allOf:
>> + - $ref: /schemas/i2c/i2c-controller.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: hisilicon,hisi-i2c
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clk_rate:
>> + maxItems: 1
>> +
>> + clock-frequency:
>> + default: 100000
>> +
> I think it misses some properties here?
>
> - i2c-sda-hold-time-ns
> - sda_fall_ns
> - scl_rise_ns
> ...
>
> Do we need to mention them here?
>
> Thanks.
>
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clk_rate
>> + - clock-frequency
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + i2c1: i2c@5038B0000{
>> + compatible = "hisilicon,hisi-i2c";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0x5 0x038B0000 0 0x10000>;
>> + interrupts = <0x0 120 0x4>;
>> + clk_rate = <0x0 0xEE6B280>;
>> + clock-frequency = <400000>;
>> + };
>>
> .


2022-09-07 22:12:46

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH next v2 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c


> I think it misses some properties here?
>
> - i2c-sda-hold-time-ns
> - sda_fall_ns
> - scl_rise_ns
> ...
>
> Do we need to mention them here?

I think so. But for a definitive answer, the devicetree-list should be
on CC for proper review.


Attachments:
(No filename) (258.00 B)
signature.asc (849.00 B)
Download all attachments