* [PATCH next v4 1/2] i2c: hisi: Add initial device tree support
@ 2022-09-09 7:48 Weilong Chen
2022-09-09 7:48 ` [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c Weilong Chen
2022-09-13 7:48 ` [PATCH next v4 1/2] i2c: hisi: Add initial device tree support Yicong Yang
0 siblings, 2 replies; 11+ messages in thread
From: Weilong Chen @ 2022-09-09 7:48 UTC (permalink / raw)
To: chenweilong, yangyicong, xuwei5, wsa, robh+dt
Cc: linux-i2c, linux-kernel, devicetree
The HiSilicon I2C controller can be used on embedded platform, which
boot from devicetree.
Signed-off-by: Weilong Chen <chenweilong@huawei.com>
---
drivers/i2c/busses/Kconfig | 2 +-
drivers/i2c/busses/i2c-hisi.c | 19 ++++++++++++++++++-
2 files changed, 19 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..7a77f306d05f 100644
--- a/drivers/i2c/busses/i2c-hisi.c
+++ b/drivers/i2c/busses/i2c-hisi.c
@@ -5,6 +5,9 @@
* Copyright (c) 2021 HiSilicon Technologies Co., Ltd.
*/
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+#endif
#include <linux/bits.h>
#include <linux/bitfield.h>
#include <linux/completion.h>
@@ -13,6 +16,9 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
+#ifdef CONFIG_OF
+#include <linux/of.h>
+#endif
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/units.h>
@@ -483,17 +489,28 @@ 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
+
+#ifdef CONFIG_OF
+static const struct of_device_id hisi_i2c_dts_ids[] = {
+ { .compatible = "hisilicon,hisi-i2c", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, hisi_i2c_dts_ids);
+#endif
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c
2022-09-09 7:48 [PATCH next v4 1/2] i2c: hisi: Add initial device tree support Weilong Chen
@ 2022-09-09 7:48 ` Weilong Chen
2022-09-13 7:59 ` Yicong Yang
2022-09-13 12:22 ` Rob Herring
2022-09-13 7:48 ` [PATCH next v4 1/2] i2c: hisi: Add initial device tree support Yicong Yang
1 sibling, 2 replies; 11+ messages in thread
From: Weilong Chen @ 2022-09-09 7:48 UTC (permalink / raw)
To: chenweilong, yangyicong, xuwei5, wsa, robh+dt
Cc: linux-i2c, linux-kernel, devicetree
Add the new compatible for HiSilicon common i2c.
Signed-off-by: Weilong Chen <chenweilong@huawei.com>
---
.../bindings/i2c/hisilicon,hisi-i2c.yaml | 67 +++++++++++++++++++
1 file changed, 67 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..f1cb6a4c70d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
@@ -0,0 +1,67 @@
+# 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:
+ - yangyicong@huawei.com
+
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+ compatible:
+ const: hisilicon,hisi-i2c
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clk_rate:
+ default: 0xEE6B280
+
+ clock-frequency:
+ default: 400000
+
+ i2c-sda-falling-time-ns:
+ default: 343
+
+ i2c-scl-falling-time-ns:
+ default: 203
+
+ i2c-sda-hold-time-ns:
+ default: 0x33E
+
+ i2c-scl-rising-time-ns:
+ default: 365
+
+ i2c-digital-filter-width-ns:
+ default: 0
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c1: i2c@5038B0000{
+ compatible = "hisilicon,hisi-i2c";
+ reg = <0x38B0000 0x10000>;
+ interrupts = <0x0 120 0x4>;
+ i2c-sda-falling-time-ns = <56>;
+ i2c-scl-falling-time-ns = <56>;
+ i2c-sda-hold-time-ns = <56>;
+ i2c-scl-rising-time-ns = <56>;
+ i2c-digital-filter;
+ i2c-digital-filter-width-ns = <0x0>;
+ clk_rate = <0x0 0xEE6B280>;
+ clock-frequency = <400000>;
+ };
--
2.31.GIT
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH next v4 1/2] i2c: hisi: Add initial device tree support
2022-09-09 7:48 [PATCH next v4 1/2] i2c: hisi: Add initial device tree support Weilong Chen
2022-09-09 7:48 ` [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c Weilong Chen
@ 2022-09-13 7:48 ` Yicong Yang
2022-09-13 9:01 ` chenweilong
1 sibling, 1 reply; 11+ messages in thread
From: Yicong Yang @ 2022-09-13 7:48 UTC (permalink / raw)
To: Weilong Chen, yangyicong, xuwei5, wsa, robh+dt
Cc: linux-i2c, linux-kernel, devicetree
On 2022/9/9 15:48, Weilong Chen wrote:
> The HiSilicon I2C controller can be used on embedded platform, which
> boot from devicetree.
>
> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
> ---
> drivers/i2c/busses/Kconfig | 2 +-
> drivers/i2c/busses/i2c-hisi.c | 19 ++++++++++++++++++-
> 2 files changed, 19 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..7a77f306d05f 100644
> --- a/drivers/i2c/busses/i2c-hisi.c
> +++ b/drivers/i2c/busses/i2c-hisi.c
> @@ -5,6 +5,9 @@
> * Copyright (c) 2021 HiSilicon Technologies Co., Ltd.
> */
>
> +#ifdef CONFIG_ACPI
> +#include <linux/acpi.h>
> +#endif
> #include <linux/bits.h>
> #include <linux/bitfield.h>
> #include <linux/completion.h>
> @@ -13,6 +16,9 @@
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> +#ifdef CONFIG_OF
> +#include <linux/of.h>
> +#endif
I don't think the protection for the headers is necessary and common. The
ACPI/OF specific functions should have already been handled well with{out}
ACPI/OF config. Have you met some problems without these?
BTW, it's better to have a changelog when updating the patches.
> #include <linux/platform_device.h>
> #include <linux/property.h>
> #include <linux/units.h>
> @@ -483,17 +489,28 @@ 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
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id hisi_i2c_dts_ids[] = {
> + { .compatible = "hisilicon,hisi-i2c", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, hisi_i2c_dts_ids);
> +#endif
>
> 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);
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c
2022-09-09 7:48 ` [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c Weilong Chen
@ 2022-09-13 7:59 ` Yicong Yang
2022-09-13 9:08 ` chenweilong
2022-09-13 12:22 ` Rob Herring
1 sibling, 1 reply; 11+ messages in thread
From: Yicong Yang @ 2022-09-13 7:59 UTC (permalink / raw)
To: Weilong Chen, yangyicong, xuwei5, wsa, robh+dt
Cc: linux-i2c, linux-kernel, devicetree
On 2022/9/9 15:48, Weilong Chen wrote:
> Add the new compatible for HiSilicon common i2c.
>
> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
> ---
> .../bindings/i2c/hisilicon,hisi-i2c.yaml | 67 +++++++++++++++++++
> 1 file changed, 67 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..f1cb6a4c70d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
> @@ -0,0 +1,67 @@
> +# 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:
> + - yangyicong@huawei.com
> +
> +allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> + compatible:
> + const: hisilicon,hisi-i2c
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clk_rate:
> + default: 0xEE6B280
> +
> + clock-frequency:
> + default: 400000
> +
> + i2c-sda-falling-time-ns:
> + default: 343
> +
> + i2c-scl-falling-time-ns:
> + default: 203
> +
> + i2c-sda-hold-time-ns:
> + default: 0x33E
> +
> + i2c-scl-rising-time-ns:
> + default: 365
> +
> + i2c-digital-filter-width-ns:
> + default: 0
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + i2c1: i2c@5038B0000{
> + compatible = "hisilicon,hisi-i2c";
> + reg = <0x38B0000 0x10000>;
> + interrupts = <0x0 120 0x4>;
> + i2c-sda-falling-time-ns = <56>;
> + i2c-scl-falling-time-ns = <56>;
> + i2c-sda-hold-time-ns = <56>;
> + i2c-scl-rising-time-ns = <56>;
The values used here are different from above. Are they used on different
products?
> + i2c-digital-filter;
Should we discard the empty properties or is it necessary to have it?
Others looks good to me, but the device tree experts may have some comments.
This binding file should also be listed in the MAINTAINERS file.
Thanks.
> + i2c-digital-filter-width-ns = <0x0>;
> + clk_rate = <0x0 0xEE6B280>;
> + clock-frequency = <400000>;
> + };
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next v4 1/2] i2c: hisi: Add initial device tree support
2022-09-13 7:48 ` [PATCH next v4 1/2] i2c: hisi: Add initial device tree support Yicong Yang
@ 2022-09-13 9:01 ` chenweilong
2022-09-13 9:37 ` Yicong Yang
0 siblings, 1 reply; 11+ messages in thread
From: chenweilong @ 2022-09-13 9:01 UTC (permalink / raw)
To: Yicong Yang, yangyicong, xuwei5, wsa, robh+dt
Cc: linux-i2c, linux-kernel, devicetree
On 2022/9/13 15:48, Yicong Yang wrote:
> On 2022/9/9 15:48, Weilong Chen wrote:
>> The HiSilicon I2C controller can be used on embedded platform, which
>> boot from devicetree.
>>
>> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
>> ---
>> drivers/i2c/busses/Kconfig | 2 +-
>> drivers/i2c/busses/i2c-hisi.c | 19 ++++++++++++++++++-
>> 2 files changed, 19 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..7a77f306d05f 100644
>> --- a/drivers/i2c/busses/i2c-hisi.c
>> +++ b/drivers/i2c/busses/i2c-hisi.c
>> @@ -5,6 +5,9 @@
>> * Copyright (c) 2021 HiSilicon Technologies Co., Ltd.
>> */
>>
>> +#ifdef CONFIG_ACPI
>> +#include <linux/acpi.h>
>> +#endif
>> #include <linux/bits.h>
>> #include <linux/bitfield.h>
>> #include <linux/completion.h>
>> @@ -13,6 +16,9 @@
>> #include <linux/io.h>
>> #include <linux/module.h>
>> #include <linux/mod_devicetable.h>
>> +#ifdef CONFIG_OF
>> +#include <linux/of.h>
>> +#endif
> I don't think the protection for the headers is necessary and common. The
> ACPI/OF specific functions should have already been handled well with{out}
> ACPI/OF config. Have you met some problems without these?
>
> BTW, it's better to have a changelog when updating the patches.
This kind usage does exist in the kernel, for example: drivers/rtc/rtc-mc146818-lib.c.
It can be consistent with the protection below MODULE_DEVICE_TABLE.
>> #include <linux/platform_device.h>
>> #include <linux/property.h>
>> #include <linux/units.h>
>> @@ -483,17 +489,28 @@ 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
>> +
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id hisi_i2c_dts_ids[] = {
>> + { .compatible = "hisilicon,hisi-i2c", },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, hisi_i2c_dts_ids);
>> +#endif
>>
>> 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);
>>
> .
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c
2022-09-13 7:59 ` Yicong Yang
@ 2022-09-13 9:08 ` chenweilong
2022-09-13 9:40 ` Yicong Yang
0 siblings, 1 reply; 11+ messages in thread
From: chenweilong @ 2022-09-13 9:08 UTC (permalink / raw)
To: Yicong Yang, yangyicong, xuwei5, wsa, robh+dt
Cc: linux-i2c, linux-kernel, devicetree
On 2022/9/13 15:59, Yicong Yang wrote:
> On 2022/9/9 15:48, Weilong Chen wrote:
>> Add the new compatible for HiSilicon common i2c.
>>
>> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
>> ---
>> .../bindings/i2c/hisilicon,hisi-i2c.yaml | 67 +++++++++++++++++++
>> 1 file changed, 67 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..f1cb6a4c70d1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
>> @@ -0,0 +1,67 @@
>> +# 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:
>> + - yangyicong@huawei.com
>> +
>> +allOf:
>> + - $ref: /schemas/i2c/i2c-controller.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: hisilicon,hisi-i2c
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clk_rate:
>> + default: 0xEE6B280
>> +
>> + clock-frequency:
>> + default: 400000
>> +
>> + i2c-sda-falling-time-ns:
>> + default: 343
>> +
>> + i2c-scl-falling-time-ns:
>> + default: 203
>> +
>> + i2c-sda-hold-time-ns:
>> + default: 0x33E
>> +
>> + i2c-scl-rising-time-ns:
>> + default: 365
>> +
>> + i2c-digital-filter-width-ns:
>> + default: 0
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + i2c1: i2c@5038B0000{
>> + compatible = "hisilicon,hisi-i2c";
>> + reg = <0x38B0000 0x10000>;
>> + interrupts = <0x0 120 0x4>;
>> + i2c-sda-falling-time-ns = <56>;
>> + i2c-scl-falling-time-ns = <56>;
>> + i2c-sda-hold-time-ns = <56>;
>> + i2c-scl-rising-time-ns = <56>;
> The values used here are different from above. Are they used on different
> products?
Yes, it's a specific config for new product.
>> + i2c-digital-filter;
> Should we discard the empty properties or is it necessary to have it?
>
> Others looks good to me, but the device tree experts may have some comments.
>
> This binding file should also be listed in the MAINTAINERS file.
>
> Thanks.
No, i2c-digital-filter is needed to fix the error:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.example.dtb: i2c@5038B0000: 'i2c-digital-filter' is a dependency of 'i2c-digital-filter-width-ns'
And, I'll update the MAINTAINERS file.
Thanks.
>> + i2c-digital-filter-width-ns = <0x0>;
>> + clk_rate = <0x0 0xEE6B280>;
>> + clock-frequency = <400000>;
>> + };
>>
> .
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next v4 1/2] i2c: hisi: Add initial device tree support
2022-09-13 9:01 ` chenweilong
@ 2022-09-13 9:37 ` Yicong Yang
0 siblings, 0 replies; 11+ messages in thread
From: Yicong Yang @ 2022-09-13 9:37 UTC (permalink / raw)
To: chenweilong, yangyicong, xuwei5, wsa, robh+dt
Cc: linux-i2c, linux-kernel, devicetree
On 2022/9/13 17:01, chenweilong wrote:
> On 2022/9/13 15:48, Yicong Yang wrote:
>> On 2022/9/9 15:48, Weilong Chen wrote:
>>> The HiSilicon I2C controller can be used on embedded platform, which
>>> boot from devicetree.
>>>
>>> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
>>> ---
>>> drivers/i2c/busses/Kconfig | 2 +-
>>> drivers/i2c/busses/i2c-hisi.c | 19 ++++++++++++++++++-
>>> 2 files changed, 19 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..7a77f306d05f 100644
>>> --- a/drivers/i2c/busses/i2c-hisi.c
>>> +++ b/drivers/i2c/busses/i2c-hisi.c
>>> @@ -5,6 +5,9 @@
>>> * Copyright (c) 2021 HiSilicon Technologies Co., Ltd.
>>> */
>>>
>>> +#ifdef CONFIG_ACPI
>>> +#include <linux/acpi.h>
>>> +#endif
>>> #include <linux/bits.h>
>>> #include <linux/bitfield.h>
>>> #include <linux/completion.h>
>>> @@ -13,6 +16,9 @@
>>> #include <linux/io.h>
>>> #include <linux/module.h>
>>> #include <linux/mod_devicetable.h>
>>> +#ifdef CONFIG_OF
>>> +#include <linux/of.h>
>>> +#endif
>> I don't think the protection for the headers is necessary and common. The
>> ACPI/OF specific functions should have already been handled well with{out}
>> ACPI/OF config. Have you met some problems without these?
>>
>> BTW, it's better to have a changelog when updating the patches.
>
> This kind usage does exist in the kernel, for example: drivers/rtc/rtc-mc146818-lib.c.
>
The protection for acpi.h used in that driver may also be unnecessary I think, but may need
test.
> It can be consistent with the protection below MODULE_DEVICE_TABLE.
>
MODULE_DEVICE_TABLE is defined in module.h, so not depend on these two headers.
>>> #include <linux/platform_device.h>
>>> #include <linux/property.h>
>>> #include <linux/units.h>
>>> @@ -483,17 +489,28 @@ 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
>>> +
>>> +#ifdef CONFIG_OF
>>> +static const struct of_device_id hisi_i2c_dts_ids[] = {
>>> + { .compatible = "hisilicon,hisi-i2c", },
>>> + { }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, hisi_i2c_dts_ids);
>>> +#endif
>>>
>>> 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);
>>>
>> .
>
>
> .
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c
2022-09-13 9:08 ` chenweilong
@ 2022-09-13 9:40 ` Yicong Yang
0 siblings, 0 replies; 11+ messages in thread
From: Yicong Yang @ 2022-09-13 9:40 UTC (permalink / raw)
To: chenweilong, yangyicong, xuwei5, wsa, robh+dt
Cc: linux-i2c, linux-kernel, devicetree
On 2022/9/13 17:08, chenweilong wrote:
> On 2022/9/13 15:59, Yicong Yang wrote:
>> On 2022/9/9 15:48, Weilong Chen wrote:
>>> Add the new compatible for HiSilicon common i2c.
>>>
>>> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
>>> ---
>>> .../bindings/i2c/hisilicon,hisi-i2c.yaml | 67 +++++++++++++++++++
>>> 1 file changed, 67 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..f1cb6a4c70d1
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
>>> @@ -0,0 +1,67 @@
>>> +# 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:
>>> + - yangyicong@huawei.com
>>> +
>>> +allOf:
>>> + - $ref: /schemas/i2c/i2c-controller.yaml#
>>> +
>>> +properties:
>>> + compatible:
>>> + const: hisilicon,hisi-i2c
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + clk_rate:
>>> + default: 0xEE6B280
>>> +
>>> + clock-frequency:
>>> + default: 400000
>>> +
>>> + i2c-sda-falling-time-ns:
>>> + default: 343
>>> +
>>> + i2c-scl-falling-time-ns:
>>> + default: 203
>>> +
>>> + i2c-sda-hold-time-ns:
>>> + default: 0x33E
>>> +
>>> + i2c-scl-rising-time-ns:
>>> + default: 365
>>> +
>>> + i2c-digital-filter-width-ns:
>>> + default: 0
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - interrupts
>>> +
>>> +unevaluatedProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + i2c1: i2c@5038B0000{
>>> + compatible = "hisilicon,hisi-i2c";
>>> + reg = <0x38B0000 0x10000>;
>>> + interrupts = <0x0 120 0x4>;
>>> + i2c-sda-falling-time-ns = <56>;
>>> + i2c-scl-falling-time-ns = <56>;
>>> + i2c-sda-hold-time-ns = <56>;
>>> + i2c-scl-rising-time-ns = <56>;
>> The values used here are different from above. Are they used on different
>> products?
> Yes, it's a specific config for new product.
>>> + i2c-digital-filter;
>> Should we discard the empty properties or is it necessary to have it?
>>
>> Others looks good to me, but the device tree experts may have some comments.
>>
>> This binding file should also be listed in the MAINTAINERS file.
>>
>> Thanks.
>
> No, i2c-digital-filter is needed to fix the error:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.example.dtb: i2c@5038B0000: 'i2c-digital-filter' is a dependency of 'i2c-digital-filter-width-ns'
>
> And, I'll update the MAINTAINERS file.
>
ok. Thanks for the explanation.
> Thanks.
>
>>> + i2c-digital-filter-width-ns = <0x0>;
>>> + clk_rate = <0x0 0xEE6B280>;
>>> + clock-frequency = <400000>;
>>> + };
>>>
>> .
>
>
> .
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c
2022-09-09 7:48 ` [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c Weilong Chen
2022-09-13 7:59 ` Yicong Yang
@ 2022-09-13 12:22 ` Rob Herring
2022-09-14 4:13 ` chenweilong
2022-09-26 9:30 ` chenweilong
1 sibling, 2 replies; 11+ messages in thread
From: Rob Herring @ 2022-09-13 12:22 UTC (permalink / raw)
To: Weilong Chen; +Cc: yangyicong, xuwei5, wsa, linux-i2c, linux-kernel, devicetree
On Fri, Sep 09, 2022 at 03:48:42PM +0800, Weilong Chen wrote:
> Add the new compatible for HiSilicon common i2c.
>
> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
> ---
> .../bindings/i2c/hisilicon,hisi-i2c.yaml | 67 +++++++++++++++++++
> 1 file changed, 67 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..f1cb6a4c70d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
> @@ -0,0 +1,67 @@
> +# 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:
> + - yangyicong@huawei.com
> +
> +allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> + compatible:
> + const: hisilicon,hisi-i2c
You need SoC specific compatibles.
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clk_rate:
> + default: 0xEE6B280
What is this property for? Use the clock binding.
> +
> + clock-frequency:
> + default: 400000
> +
> + i2c-sda-falling-time-ns:
> + default: 343
> +
> + i2c-scl-falling-time-ns:
> + default: 203
> +
> + i2c-sda-hold-time-ns:
> + default: 0x33E
The rest are in decimal. Be consistent.
> +
> + i2c-scl-rising-time-ns:
> + default: 365
> +
> + i2c-digital-filter-width-ns:
> + default: 0
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + i2c1: i2c@5038B0000{
Space needed ^
Use lowercase hex.
Drop unused labels.
> + compatible = "hisilicon,hisi-i2c";
> + reg = <0x38B0000 0x10000>;
> + interrupts = <0x0 120 0x4>;
> + i2c-sda-falling-time-ns = <56>;
> + i2c-scl-falling-time-ns = <56>;
> + i2c-sda-hold-time-ns = <56>;
> + i2c-scl-rising-time-ns = <56>;
> + i2c-digital-filter;
> + i2c-digital-filter-width-ns = <0x0>;
> + clk_rate = <0x0 0xEE6B280>;
> + clock-frequency = <400000>;
> + };
> --
> 2.31.GIT
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c
2022-09-13 12:22 ` Rob Herring
@ 2022-09-14 4:13 ` chenweilong
2022-09-26 9:30 ` chenweilong
1 sibling, 0 replies; 11+ messages in thread
From: chenweilong @ 2022-09-14 4:13 UTC (permalink / raw)
To: Rob Herring; +Cc: yangyicong, xuwei5, wsa, linux-i2c, linux-kernel, devicetree
Thanks for your review. I'll update the patch.
On 2022/9/13 20:22, Rob Herring wrote:
> On Fri, Sep 09, 2022 at 03:48:42PM +0800, Weilong Chen wrote:
>> Add the new compatible for HiSilicon common i2c.
>>
>> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
>> ---
>> .../bindings/i2c/hisilicon,hisi-i2c.yaml | 67 +++++++++++++++++++
>> 1 file changed, 67 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..f1cb6a4c70d1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
>> @@ -0,0 +1,67 @@
>> +# 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:
>> + - yangyicong@huawei.com
>> +
>> +allOf:
>> + - $ref: /schemas/i2c/i2c-controller.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: hisilicon,hisi-i2c
> You need SoC specific compatibles.
>
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clk_rate:
>> + default: 0xEE6B280
> What is this property for? Use the clock binding.
>
>> +
>> + clock-frequency:
>> + default: 400000
>> +
>> + i2c-sda-falling-time-ns:
>> + default: 343
>> +
>> + i2c-scl-falling-time-ns:
>> + default: 203
>> +
>> + i2c-sda-hold-time-ns:
>> + default: 0x33E
> The rest are in decimal. Be consistent.
>
>> +
>> + i2c-scl-rising-time-ns:
>> + default: 365
>> +
>> + i2c-digital-filter-width-ns:
>> + default: 0
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + i2c1: i2c@5038B0000{
> Space needed ^
>
> Use lowercase hex.
>
> Drop unused labels.
>
>> + compatible = "hisilicon,hisi-i2c";
>> + reg = <0x38B0000 0x10000>;
>> + interrupts = <0x0 120 0x4>;
>> + i2c-sda-falling-time-ns = <56>;
>> + i2c-scl-falling-time-ns = <56>;
>> + i2c-sda-hold-time-ns = <56>;
>> + i2c-scl-rising-time-ns = <56>;
>> + i2c-digital-filter;
>> + i2c-digital-filter-width-ns = <0x0>;
>> + clk_rate = <0x0 0xEE6B280>;
>> + clock-frequency = <400000>;
>> + };
>> --
>> 2.31.GIT
>>
>>
> .
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c
2022-09-13 12:22 ` Rob Herring
2022-09-14 4:13 ` chenweilong
@ 2022-09-26 9:30 ` chenweilong
1 sibling, 0 replies; 11+ messages in thread
From: chenweilong @ 2022-09-26 9:30 UTC (permalink / raw)
To: Rob Herring; +Cc: yangyicong, xuwei5, wsa, linux-i2c, linux-kernel, devicetree
On 2022/9/13 20:22, Rob Herring wrote:
> On Fri, Sep 09, 2022 at 03:48:42PM +0800, Weilong Chen wrote:
>> Add the new compatible for HiSilicon common i2c.
>>
>> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
>> ---
>> .../bindings/i2c/hisilicon,hisi-i2c.yaml | 67 +++++++++++++++++++
>> 1 file changed, 67 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..f1cb6a4c70d1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
>> @@ -0,0 +1,67 @@
>> +# 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:
>> + - yangyicong@huawei.com
>> +
>> +allOf:
>> + - $ref: /schemas/i2c/i2c-controller.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: hisilicon,hisi-i2c
> You need SoC specific compatibles.
Hi Rob,
I add a description for specific compatibles in the next version:
https://lore.kernel.org/lkml/20220920072215.161331-2-chenweilong@huawei.com/T/
Please take a look, Do you think that's OK?
Thank you.
>
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clk_rate:
>> + default: 0xEE6B280
> What is this property for? Use the clock binding.
>
>> +
>> + clock-frequency:
>> + default: 400000
>> +
>> + i2c-sda-falling-time-ns:
>> + default: 343
>> +
>> + i2c-scl-falling-time-ns:
>> + default: 203
>> +
>> + i2c-sda-hold-time-ns:
>> + default: 0x33E
> The rest are in decimal. Be consistent.
>
>> +
>> + i2c-scl-rising-time-ns:
>> + default: 365
>> +
>> + i2c-digital-filter-width-ns:
>> + default: 0
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + i2c1: i2c@5038B0000{
> Space needed ^
>
> Use lowercase hex.
>
> Drop unused labels.
>
>> + compatible = "hisilicon,hisi-i2c";
>> + reg = <0x38B0000 0x10000>;
>> + interrupts = <0x0 120 0x4>;
>> + i2c-sda-falling-time-ns = <56>;
>> + i2c-scl-falling-time-ns = <56>;
>> + i2c-sda-hold-time-ns = <56>;
>> + i2c-scl-rising-time-ns = <56>;
>> + i2c-digital-filter;
>> + i2c-digital-filter-width-ns = <0x0>;
>> + clk_rate = <0x0 0xEE6B280>;
>> + clock-frequency = <400000>;
>> + };
>> --
>> 2.31.GIT
>>
>>
> .
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-09-26 9:32 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-09 7:48 [PATCH next v4 1/2] i2c: hisi: Add initial device tree support Weilong Chen
2022-09-09 7:48 ` [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c Weilong Chen
2022-09-13 7:59 ` Yicong Yang
2022-09-13 9:08 ` chenweilong
2022-09-13 9:40 ` Yicong Yang
2022-09-13 12:22 ` Rob Herring
2022-09-14 4:13 ` chenweilong
2022-09-26 9:30 ` chenweilong
2022-09-13 7:48 ` [PATCH next v4 1/2] i2c: hisi: Add initial device tree support Yicong Yang
2022-09-13 9:01 ` chenweilong
2022-09-13 9:37 ` Yicong Yang
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).