[PATCH v2 1/2] soc: loongson: add GUTS driver for loongson2 platforms

[PATCH v2 1/2] soc: loongson: add GUTS driver for loongson2 platforms

[Yinbo Zhu]

The global utilities block controls PCIE device enabling, alternate
function selection for multiplexed signals, consistency of HDA, USB
and PCIE, configuration of memory controller, rtc controller, lio
controller, and clock control.

This patch adds a driver to manage and access global utilities block
for loongarch architecture loongson2 SoCs. Initially only reading SVR
and registering soc device are supported. Other guts accesses, such
as reading PMON configuration by default, should eventually be added
into this driver as well.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
Change in v2:
		1. Add architecture support commit log description.
		2. Add other guts accesses plan commit log description.
		3. Add "depends on LOONGARCH || COMPILE_TEST" for
		   LOONGSON2_GUTS in Kconfig.
		4. Move the scfg_guts to .c file from .h and delete .h.
		5. Remove __packed on scfg_guts.

 MAINTAINERS                           |   6 +
 drivers/soc/Kconfig                   |   1 +
 drivers/soc/Makefile                  |   1 +
 drivers/soc/loongson/Kconfig          |  20 +++
 drivers/soc/loongson/Makefile         |   6 +
 drivers/soc/loongson/loongson2_guts.c | 189 ++++++++++++++++++++++++++
 6 files changed, 223 insertions(+)
 create mode 100644 drivers/soc/loongson/Kconfig
 create mode 100644 drivers/soc/loongson/Makefile
 create mode 100644 drivers/soc/loongson/loongson2_guts.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2d002509fc65..0f06dc83f7c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11930,6 +11930,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/pinctrl/loongson,ls2k-pinctrl.yaml
 F:	drivers/pinctrl/pinctrl-loongson2.c
 
+LOONGSON2 SOC SERIES GUTS DRIVER
+M:	Yinbo Zhu <zhuyinbo@loongson.cn>
+L:	loongarch@lists.linux.dev
+S:	Maintained
+F:	drivers/soc/loongson/loongson2_guts.c
+
 LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
 M:	Sathya Prakash <sathya.prakash@broadcom.com>
 M:	Sreekanth Reddy <sreekanth.reddy@broadcom.com>
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index e461c071189b..5dbb09f843f7 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -13,6 +13,7 @@ source "drivers/soc/fujitsu/Kconfig"
 source "drivers/soc/imx/Kconfig"
 source "drivers/soc/ixp4xx/Kconfig"
 source "drivers/soc/litex/Kconfig"
+source "drivers/soc/loongson/Kconfig"
 source "drivers/soc/mediatek/Kconfig"
 source "drivers/soc/microchip/Kconfig"
 source "drivers/soc/pxa/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 69ba6508cf2c..fff513bd522d 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -18,6 +18,7 @@ obj-y				+= imx/
 obj-y				+= ixp4xx/
 obj-$(CONFIG_SOC_XWAY)		+= lantiq/
 obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex/
+obj-y				+= loongson/
 obj-y				+= mediatek/
 obj-y				+= microchip/
 obj-y				+= pxa/
diff --git a/drivers/soc/loongson/Kconfig b/drivers/soc/loongson/Kconfig
new file mode 100644
index 000000000000..04a4dd5192ef
--- /dev/null
+++ b/drivers/soc/loongson/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Loongson2 series SoC drivers
+#
+
+menu "Loongson2 series SoC drivers"
+
+config LOONGSON2_GUTS
+	tristate "LOONGSON2 GUTS"
+	depends on LOONGARCH || COMPILE_TEST
+	select SOC_BUS
+	help
+	  The global utilities block controls PCIE device enabling, alternate
+	  function selection for multiplexed signals, consistency of HDA, USB
+	  and PCIE, configuration of memory controller, rtc controller, lio
+	  controller, and clock control. This patch adds a driver to manage
+	  and access global utilities block for loongarch architecture loongson2
+	  SoCs. Initially only reading SVR and registering soc device are
+	  supported. Other guts accesses, such as reading PMON configuration by
+	  default, should eventually be added into this driver as well.
diff --git a/drivers/soc/loongson/Makefile b/drivers/soc/loongson/Makefile
new file mode 100644
index 000000000000..acfc90a624c9
--- /dev/null
+++ b/drivers/soc/loongson/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Makefile for the Linux Kernel SoC loongson2 specific device drivers
+#
+
+obj-$(CONFIG_LOONGSON2_GUTS)		+= loongson2_guts.o
diff --git a/drivers/soc/loongson/loongson2_guts.c b/drivers/soc/loongson/loongson2_guts.c
new file mode 100644
index 000000000000..15db9ebc3f14
--- /dev/null
+++ b/drivers/soc/loongson/loongson2_guts.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Author: Yinbo Zhu <zhuyinbo@loongson.cn>
+ * Copyright (C) 2022-2023 Loongson Technology Corporation Limited
+ */
+
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/of_fdt.h>
+#include <linux/sys_soc.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+static struct soc_device_attribute soc_dev_attr;
+static struct soc_device *soc_dev;
+
+/*
+ * Global Utility Registers.
+ *
+ * Not all registers defined in this structure are available on all chips, so
+ * you are expected to know whether a given register actually exists on your
+ * chip before you access it.
+ *
+ * Also, some registers are similar on different chips but have slightly
+ * different names.  In these cases, one name is chosen to avoid extraneous
+ * #ifdefs.
+ */
+struct scfg_guts {
+	u32     svr;            /* Version Register */
+	u8      res0[4];
+	u16     feature;        /* Feature Register */
+	u32     vendor;         /* Vendor Register */
+	u8      res1[6];
+	u32     id;
+	u8      res2[0x3ff8 - 0x18];
+	u32     chip;
+};
+
+static struct guts {
+	struct scfg_guts __iomem *regs;
+	bool little_endian;
+} *guts;
+
+struct loongson2_soc_die_attr {
+	char	*die;
+	u32	svr;
+	u32	mask;
+};
+
+/* SoC die attribute definition for loongson2 platform */
+static const struct loongson2_soc_die_attr loongson2_soc_die[] = {
+
+	/*
+	 * LA-based SoCs Loongson2 Series
+	 */
+
+	/* Die: 2k1000la, SoC: 2k1000la */
+	{ .die		= "2K1000LA",
+	  .svr		= 0x00000013,
+	  .mask		= 0x000000ff,
+	},
+	{ },
+};
+
+static const struct loongson2_soc_die_attr *loongson2_soc_die_match(
+	u32 svr, const struct loongson2_soc_die_attr *matches)
+{
+	while (matches->svr) {
+		if (matches->svr == (svr & matches->mask))
+			return matches;
+		matches++;
+	};
+
+	return NULL;
+}
+
+static u32 loongson2_guts_get_svr(void)
+{
+	u32 svr = 0;
+
+	if (!guts || !guts->regs)
+		return svr;
+
+	if (guts->little_endian)
+		svr = ioread32(&guts->regs->svr);
+	else
+		svr = ioread32be(&guts->regs->svr);
+
+	return svr;
+}
+
+static int loongson2_guts_probe(struct platform_device *pdev)
+{
+	struct device_node *root, *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	const struct loongson2_soc_die_attr *soc_die;
+	const char *machine;
+	u32 svr;
+
+	/* Initialize guts */
+	guts = devm_kzalloc(dev, sizeof(*guts), GFP_KERNEL);
+	if (!guts)
+		return -ENOMEM;
+
+	guts->little_endian = of_property_read_bool(np, "little-endian");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	guts->regs = ioremap(res->start, res->end - res->start + 1);
+	if (IS_ERR(guts->regs))
+		return PTR_ERR(guts->regs);
+
+	/* Register soc device */
+	root = of_find_node_by_path("/");
+	if (of_property_read_string(root, "model", &machine))
+		of_property_read_string_index(root, "compatible", 0, &machine);
+	of_node_put(root);
+	if (machine)
+		soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL);
+
+	svr = loongson2_guts_get_svr();
+	soc_die = loongson2_soc_die_match(svr, loongson2_soc_die);
+	if (soc_die) {
+		soc_dev_attr.family = devm_kasprintf(dev, GFP_KERNEL,
+						     "Loongson %s", soc_die->die);
+	} else {
+		soc_dev_attr.family = devm_kasprintf(dev, GFP_KERNEL, "Loongson");
+	}
+	if (!soc_dev_attr.family)
+		return -ENOMEM;
+	soc_dev_attr.soc_id = devm_kasprintf(dev, GFP_KERNEL,
+					     "svr:0x%08x", svr);
+	if (!soc_dev_attr.soc_id)
+		return -ENOMEM;
+	soc_dev_attr.revision = devm_kasprintf(dev, GFP_KERNEL, "%d.%d",
+					       (svr >>  4) & 0xf, svr & 0xf);
+	if (!soc_dev_attr.revision)
+		return -ENOMEM;
+
+	soc_dev = soc_device_register(&soc_dev_attr);
+	if (IS_ERR(soc_dev))
+		return PTR_ERR(soc_dev);
+
+	pr_info("Machine: %s\n", soc_dev_attr.machine);
+	pr_info("SoC family: %s\n", soc_dev_attr.family);
+	pr_info("SoC ID: %s, Revision: %s\n",
+		soc_dev_attr.soc_id, soc_dev_attr.revision);
+
+	return 0;
+}
+
+static int loongson2_guts_remove(struct platform_device *dev)
+{
+	soc_device_unregister(soc_dev);
+
+	return 0;
+}
+
+/*
+ * Table for matching compatible strings, for device tree
+ * guts node, for Loongson2 SoCs.
+ */
+static const struct of_device_id loongson2_guts_of_match[] = {
+	{ .compatible = "loongson,ls2k-guts", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, loongson2_guts_of_match);
+
+static struct platform_driver loongson2_guts_driver = {
+	.driver = {
+		.name = "loongson2-guts",
+		.of_match_table = loongson2_guts_of_match,
+	},
+	.probe = loongson2_guts_probe,
+	.remove = loongson2_guts_remove,
+};
+
+static int __init loongson2_guts_init(void)
+{
+	return platform_driver_register(&loongson2_guts_driver);
+}
+core_initcall(loongson2_guts_init);
+
+static void __exit loongson2_guts_exit(void)
+{
+	platform_driver_unregister(&loongson2_guts_driver);
+}
+module_exit(loongson2_guts_exit);
-- 
2.31.1

[PATCH v2 2/2] dt-bindings: soc: add loongson2 guts

[Yinbo Zhu]

Add the loongson2 soc guts driver binding with DT schema format
using json-schema.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
 .../soc/loongson/loongson,ls2k-guts.yaml      | 37 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml

diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
new file mode 100644
index 000000000000..2502f8aeb74d
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-guts.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson2 GUTS driver.
+
+maintainers:
+  - Yinbo Zhu <zhuyinbo@loongson.cn>
+
+description: |
+  GUTS driver was to manage and access global utilities block. Initially
+  only reading SVR and registering soc device are supported.
+
+properties:
+  compatible:
+    const: loongson,ls2k-guts
+
+  reg:
+    maxItems: 1
+
+  little-endian: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    guts: guts@1fe00000 {
+        compatible = "loongson,ls2k-guts";
+        reg = <0x1fe00000 0x3ffc>;
+        little-endian;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 0f06dc83f7c6..b23474a5d8c1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11934,6 +11934,7 @@ LOONGSON2 SOC SERIES GUTS DRIVER
 M:	Yinbo Zhu <zhuyinbo@loongson.cn>
 L:	loongarch@lists.linux.dev
 S:	Maintained
+F:	Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
 F:	drivers/soc/loongson/loongson2_guts.c
 
 LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
-- 
2.31.1

Re: [PATCH v2 2/2] dt-bindings: soc: add loongson2 guts

[Krzysztof Kozlowski]

On 24/10/2022 23:51, Yinbo Zhu wrote:
> Add the loongson2 soc guts driver binding with DT schema format
> using json-schema.
> 
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
>  .../soc/loongson/loongson,ls2k-guts.yaml      | 37 +++++++++++++++++++

Looks like wrong location, although difficult to judge because you did
not describe the hardware at all. If this is chipinfo-like device, then
Documentation/devicetree/bindings/hwinfo/.


>  MAINTAINERS                                   |  1 +
>  2 files changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
> new file mode 100644
> index 000000000000..2502f8aeb74d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
> @@ -0,0 +1,37 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-guts.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson2 GUTS driver.

Drop "driver." unless you refer to some hardware (like motor driver?).

> +
> +maintainers:
> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
> +
> +description: |
> +  GUTS driver was to manage and access global utilities block. Initially

Drop "driver" and describe instead what is GUTS, including its acronym,

> +  only reading SVR and registering soc device are supported.

Entire sentence describe Linux driver - drop it. Instead describe the
device, the hardware.

> +
> +properties:
> +  compatible:
> +    const: loongson,ls2k-guts
> +
> +  reg:
> +    maxItems: 1
> +
> +  little-endian: true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    guts: guts@1fe00000 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


Best regards,
Krzysztof

Re: [PATCH v2 2/2] dt-bindings: soc: add loongson2 guts

[Yinbo Zhu]

在 2022/10/26 上午3:40, Krzysztof Kozlowski 写道:
> On 24/10/2022 23:51, Yinbo Zhu wrote:
>> Add the loongson2 soc guts driver binding with DT schema format
>> using json-schema.
>>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>> ---
>>   .../soc/loongson/loongson,ls2k-guts.yaml      | 37 +++++++++++++++++++
> 
> Looks like wrong location, although difficult to judge because you did
> not describe the hardware at all. If this is chipinfo-like device, then
> Documentation/devicetree/bindings/hwinfo/.
My guts driver is refer fsl platform. It was was to manage and access
global utilities register block for SoC and it was only used in SoC
platform. when driver need use Soc ops to do some function the this 
driver was needed.  the dcfg (device config) was a function in guts 
(global utilities) block.
For these type of driver, other platforms were initially placed on
Documentation/devicetree/bindings/arm/   if it is arm/arm64
architecture. Later, move it to the soc directory.

Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml

So, do you still think it is inappropriate to place it in the soc dir?
> 
> 
>>   MAINTAINERS                                   |  1 +
>>   2 files changed, 38 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>> new file mode 100644
>> index 000000000000..2502f8aeb74d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>> @@ -0,0 +1,37 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-guts.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Loongson2 GUTS driver.
> 
> Drop "driver." unless you refer to some hardware (like motor driver?).
this need refer hardware soc datasheet to gain soc register (global 
utilities register block ).
so keep "driver" string that whether was more appropriate?
> 
>> +
>> +maintainers:
>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>> +
>> +description: |
>> +  GUTS driver was to manage and access global utilities block. Initially
> 
> Drop "driver" and describe instead what is GUTS, including its acronym,
> 
>> +  only reading SVR and registering soc device are supported.
> 
> Entire sentence describe Linux driver - drop it. Instead describe the
> device, the hardware.
> 
>> +
>> +properties:
>> +  compatible:
>> +    const: loongson,ls2k-guts
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  little-endian: true
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    guts: guts@1fe00000 {
> 
> Node names should be generic.
dcfg/scfg (device cfg/ soc cfg)was the key function of guts (global 
utilities) block. and guts name I was refer fsl soc driver. 
"drivers/soc/fsl/guts.c"
this binding file was follows of fsl guts.
Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-scfg.yaml

or, I was use scfg as node name, Do you think it's appropriate?


> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 2/2] dt-bindings: soc: add loongson2 guts

[Krzysztof Kozlowski]

On 26/10/2022 03:22, Yinbo Zhu wrote:
> 
> 
> 在 2022/10/26 上午3:40, Krzysztof Kozlowski 写道:
>> On 24/10/2022 23:51, Yinbo Zhu wrote:
>>> Add the loongson2 soc guts driver binding with DT schema format
>>> using json-schema.
>>>
>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>>> ---
>>>   .../soc/loongson/loongson,ls2k-guts.yaml      | 37 +++++++++++++++++++
>>
>> Looks like wrong location, although difficult to judge because you did
>> not describe the hardware at all. If this is chipinfo-like device, then
>> Documentation/devicetree/bindings/hwinfo/.
> My guts driver is refer fsl platform. It was was to manage and access
> global utilities register block for SoC and it was only used in SoC
> platform. when driver need use Soc ops to do some function the this 
> driver was needed.  the dcfg (device config) was a function in guts 
> (global utilities) block.

I can barely understand it.

> For these type of driver, other platforms were initially placed on
> Documentation/devicetree/bindings/arm/   if it is arm/arm64
> architecture. Later, move it to the soc directory.
> 
> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml

How is this related? This is Layerscape, not Loongson2. Describe the
hardware you are adding bindings for.

> 
> So, do you still think it is inappropriate to place it in the soc dir?
>>
>>
>>>   MAINTAINERS                                   |  1 +
>>>   2 files changed, 38 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>> new file mode 100644
>>> index 000000000000..2502f8aeb74d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>> @@ -0,0 +1,37 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-guts.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Loongson2 GUTS driver.
>>
>> Drop "driver." unless you refer to some hardware (like motor driver?).
> this need refer hardware soc datasheet to gain soc register (global 
> utilities register block ).
> so keep "driver" string that whether was more appropriate?

What? I cannot parse it.

Did you understand my comment? If yes, please point to Wikipedia article
explaining this "Driver" you refer to.


>>
>>> +
>>> +maintainers:
>>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>>> +
>>> +description: |
>>> +  GUTS driver was to manage and access global utilities block. Initially
>>
>> Drop "driver" and describe instead what is GUTS, including its acronym,
>>
>>> +  only reading SVR and registering soc device are supported.
>>
>> Entire sentence describe Linux driver - drop it. Instead describe the
>> device, the hardware.
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: loongson,ls2k-guts
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  little-endian: true
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    guts: guts@1fe00000 {
>>
>> Node names should be generic.
> dcfg/scfg (device cfg/ soc cfg)was the key function of guts (global 
> utilities) block. and guts name I was refer fsl soc driver. 
> "drivers/soc/fsl/guts.c"
> this binding file was follows of fsl guts.
> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-scfg.yaml
> 
> or, I was use scfg as node name, Do you think it's appropriate?

No, these are not generic node names.

> 
> 
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Best regards,
Krzysztof

Re: [PATCH v2 2/2] dt-bindings: soc: add loongson2 guts

[Yinbo Zhu]

在 2022/10/26 下午10:10, Krzysztof Kozlowski 写道:
> On 26/10/2022 03:22, Yinbo Zhu wrote:
>>
>>
>> 在 2022/10/26 上午3:40, Krzysztof Kozlowski 写道:
>>> On 24/10/2022 23:51, Yinbo Zhu wrote:
>>>> Add the loongson2 soc guts driver binding with DT schema format
>>>> using json-schema.
>>>>
>>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>>>> ---
>>>>    .../soc/loongson/loongson,ls2k-guts.yaml      | 37 +++++++++++++++++++
>>>
>>> Looks like wrong location, although difficult to judge because you did
>>> not describe the hardware at all. If this is chipinfo-like device, then
>>> Documentation/devicetree/bindings/hwinfo/.
yes it is a chipinfo/socinfo device, I will following your advice.
>> My guts driver is refer fsl platform. It was was to manage and access
>> global utilities register block for SoC and it was only used in SoC
>> platform. when driver need use Soc ops to do some function the this
>> driver was needed.  the dcfg (device config) was a function in guts
>> (global utilities) block.
> 
> I can barely understand it.
My description is about chipinfo/socinfo definition. and I have a look
/bindings/hwinfo/, I think move binding file to hwinfo that is okay for me.

Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml

> 
>> For these type of driver, other platforms were initially placed on
>> Documentation/devicetree/bindings/arm/   if it is arm/arm64
>> architecture. Later, move it to the soc directory.
>>
>> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
> 
> How is this related? This is Layerscape, not Loongson2. Describe the
> hardware you are adding bindings for.
The driver functions/type are the same, the driver was register a struct 
soc_device_attribute by soc_device_register then other peripheral driver
can call SoC ops, such as soc_device_match.

then layerscape guts module bindings are placed in
Documentation/devicetree/bindings/soc/, the loongson guts module 
bindings was follow that layerscape and are placed in
Documentation/devicetree/bindings/soc/

In a words,  It is a question about where the binding file should be 
placed.  I think move binding file to hwinfo that is okay for me.
> 
>>
>> So, do you still think it is inappropriate to place it in the soc dir?
>>>
>>>
>>>>    MAINTAINERS                                   |  1 +
>>>>    2 files changed, 38 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>> new file mode 100644
>>>> index 000000000000..2502f8aeb74d
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>> @@ -0,0 +1,37 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-guts.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Loongson2 GUTS driver.
>>>
>>> Drop "driver." unless you refer to some hardware (like motor driver?).
>> this need refer hardware soc datasheet to gain soc register (global
>> utilities register block ).
>> so keep "driver" string that whether was more appropriate?
> 
> What? I cannot parse it.
> 
> Did you understand my comment? If yes, please point to Wikipedia article
> explaining this "Driver" you refer to.
I will remove the "driver" string.
> 
> 
>>>
>>>> +
>>>> +maintainers:
>>>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>>>> +
>>>> +description: |
>>>> +  GUTS driver was to manage and access global utilities block. Initially
>>>
>>> Drop "driver" and describe instead what is GUTS, including its acronym,
>>>
>>>> +  only reading SVR and registering soc device are supported.
>>>
>>> Entire sentence describe Linux driver - drop it. Instead describe the
>>> device, the hardware.
>>>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: loongson,ls2k-guts
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  little-endian: true
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    guts: guts@1fe00000 {
>>>
>>> Node names should be generic.
>> dcfg/scfg (device cfg/ soc cfg)was the key function of guts (global
>> utilities) block. and guts name I was refer fsl soc driver.
>> "drivers/soc/fsl/guts.c"
>> this binding file was follows of fsl guts.
>> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
>> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-scfg.yaml
>>
>> or, I was use scfg as node name, Do you think it's appropriate?
> 
> No, these are not generic node names.
I was refer "ti,k3-socinfo.yaml",  Do you think it's appropriate that 
socinfo as node name?
> 
>>
>>
>>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 2/2] dt-bindings: soc: add loongson2 guts

[Krzysztof Kozlowski]

On 26/10/2022 23:52, Yinbo Zhu wrote:
> 
> 
> 在 2022/10/26 下午10:10, Krzysztof Kozlowski 写道:
>> On 26/10/2022 03:22, Yinbo Zhu wrote:
>>>
>>>
>>> 在 2022/10/26 上午3:40, Krzysztof Kozlowski 写道:
>>>> On 24/10/2022 23:51, Yinbo Zhu wrote:
>>>>> Add the loongson2 soc guts driver binding with DT schema format
>>>>> using json-schema.
>>>>>
>>>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>>>>> ---
>>>>>    .../soc/loongson/loongson,ls2k-guts.yaml      | 37 +++++++++++++++++++
>>>>
>>>> Looks like wrong location, although difficult to judge because you did
>>>> not describe the hardware at all. If this is chipinfo-like device, then
>>>> Documentation/devicetree/bindings/hwinfo/.
> yes it is a chipinfo/socinfo device, I will following your advice.
>>> My guts driver is refer fsl platform. It was was to manage and access
>>> global utilities register block for SoC and it was only used in SoC
>>> platform. when driver need use Soc ops to do some function the this
>>> driver was needed.  the dcfg (device config) was a function in guts
>>> (global utilities) block.
>>
>> I can barely understand it.
> My description is about chipinfo/socinfo definition. and I have a look
> /bindings/hwinfo/, I think move binding file to hwinfo that is okay for me.
> 
> Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
> 
>>
>>> For these type of driver, other platforms were initially placed on
>>> Documentation/devicetree/bindings/arm/   if it is arm/arm64
>>> architecture. Later, move it to the soc directory.
>>>
>>> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
>>
>> How is this related? This is Layerscape, not Loongson2. Describe the
>> hardware you are adding bindings for.
> The driver functions/type are the same, the driver was register a struct 
> soc_device_attribute by soc_device_register then other peripheral driver
> can call SoC ops, such as soc_device_match.
> 
> then layerscape guts module bindings are placed in
> Documentation/devicetree/bindings/soc/, the loongson guts module 
> bindings was follow that layerscape and are placed in
> Documentation/devicetree/bindings/soc/
> 
> In a words,  It is a question about where the binding file should be 
> placed.  I think move binding file to hwinfo that is okay for me.

My review is limited to the scope I understand from what you wrote. You
described so far something being only a soc info registers. For this the
place is hwinfo.

The Layerscape dcfg is more than that - it is a syscon, system register
controller with at least one child device.

If your device is exactly like that, describe it in bindings fully, not
partially. These are then incomplete bindings.

>>
>>>
>>> So, do you still think it is inappropriate to place it in the soc dir?
>>>>
>>>>
>>>>>    MAINTAINERS                                   |  1 +
>>>>>    2 files changed, 38 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..2502f8aeb74d
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>>> @@ -0,0 +1,37 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-guts.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Loongson2 GUTS driver.
>>>>
>>>> Drop "driver." unless you refer to some hardware (like motor driver?).
>>> this need refer hardware soc datasheet to gain soc register (global
>>> utilities register block ).
>>> so keep "driver" string that whether was more appropriate?
>>
>> What? I cannot parse it.
>>
>> Did you understand my comment? If yes, please point to Wikipedia article
>> explaining this "Driver" you refer to.
> I will remove the "driver" string.
>>
>>
>>>>
>>>>> +
>>>>> +maintainers:
>>>>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>>>>> +
>>>>> +description: |
>>>>> +  GUTS driver was to manage and access global utilities block. Initially
>>>>
>>>> Drop "driver" and describe instead what is GUTS, including its acronym,
>>>>
>>>>> +  only reading SVR and registering soc device are supported.
>>>>
>>>> Entire sentence describe Linux driver - drop it. Instead describe the
>>>> device, the hardware.
>>>>
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: loongson,ls2k-guts
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  little-endian: true
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>> +  - reg
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> +  - |
>>>>> +    guts: guts@1fe00000 {
>>>>
>>>> Node names should be generic.
>>> dcfg/scfg (device cfg/ soc cfg)was the key function of guts (global
>>> utilities) block. and guts name I was refer fsl soc driver.
>>> "drivers/soc/fsl/guts.c"
>>> this binding file was follows of fsl guts.
>>> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
>>> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-scfg.yaml
>>>
>>> or, I was use scfg as node name, Do you think it's appropriate?
>>
>> No, these are not generic node names.
> I was refer "ti,k3-socinfo.yaml",  Do you think it's appropriate that 
> socinfo as node name?

The blocks are called usually chipid and you can find examples for that.
There are no examples using socinfo name.

If the main purpose of this register block (and/or device) is to provide
socinfo, then call it chipid.

Best regards,
Krzysztof