All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support
@ 2022-11-22  7:39 Rahul Tanwar
  2022-11-22  7:39 ` [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema Rahul Tanwar
                   ` (4 more replies)
  0 siblings, 5 replies; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22  7:39 UTC (permalink / raw)
  To: bigeasy, robh, tglx, mingo, bp, x86, hpa
  Cc: andriy.shevchenko, dave.hansen, linux-kernel, linux-lgm-soc,
	Rahul Tanwar

Hi All,

This patch series mainly fixes a boot time interrupt delivery mode
configuration hardcoding bug for OF based x86 platforms. Presently,
boot time interrupt delivery mode is hardcoded to legacy PIC mode
with no option to configure it to virtual wire mode. This patch
series aims to fix it by introducing a new optional boolean property
for lapic devicetree node which can be used to configure it to
virtual wire mode where applicable. Please find below detailed
rationale behind it.

Rationale:

References [1], [2] & [6]

For SMP systems, Intel defines three (logically four) interrupt modes
during boot/init time while BIOS/bootloader boots & switches to linux
kernel.

  1. PIC mode - Legacy 8259 PIC interrupt controller.
  2. Virtual wire mode via Local APIC - uses local APIC as virtual wire
  3. Virtual wire mode via I/O APIC - uses I/O APIC as virtual wire
  4. Symmetric I/O mode - final one used by linux for SMP systems. 

BIOS/bootloaders are supposed to boot in either #1 or #2 or #3 and then
switch to #4 in linux for SMP systems.

For our platform, we use #2.

Detection of which interrupt mode the system is booting in is made by using
below global variable in apic.c

int pic_mode __ro_after_init; 

Here pic_mode = 1 means #1 (PIC mode) above.
And pic_mode = 0 means #2 or #3 (basically virtual wire mode via apic).

And apic.c while doing setup_local_APIC() uses below code [3]:

        value = apic_read(APIC_LVT0) & APIC_LVT_MASKED;
        if (!cpu && (pic_mode || !value || skip_ioapic_setup)) {
                value = APIC_DM_EXTINT;
                apic_printk(APIC_VERBOSE, "enabled ExtINT on CPU#%d\n", cpu);
        } else {
                value = APIC_DM_EXTINT | APIC_LVT_MASKED;
                apic_printk(APIC_VERBOSE, "masked ExtINT on CPU#%d\n", cpu);
        }
        apic_write(APIC_LVT0, value);

What i understand from above is that if at this point of time, as long as
it is cpu0 & pic_mode=1, it will set delivery mode to ExtINT (causes the
processor to respond to the interrupt as if the interrupt originated in an
externally connected (8259A-compatible) interrupt controller) and enables/
unmask the interrupts. This causes kernel boot crash for platforms which
does not support 8259 compatible external PIC.

pic_mode is presently set/populated/initialized at only two places:
 1. In  mpparse.c [4]
 2. In devicetree.c [7]

For #1 MPPARSE Kconfig definition is as below:

	config X86_MPPARSE
        	bool "Enable MPS table" if ACPI
        	default y
        	depends on X86_LOCAL_APIC
        	help
          	For old smp systems that do not have proper acpi support. Newer systems
          	(esp with 64bit cpus) with acpi support, MADT and DSDT will override it

As seen above, if ACPI is not enabled, then mpparse by default is always
enabled. Presently, there is no way to disable MPPARSE (if ACPI is not
enabled). This to me appears to be another bug which needs fixing. As per
theory, MPPARSE was to support MPS spec [1] as a temporary solution to
support SMP systems until a final ACPI standard was added. But now if ACPI
is not enabled, it will rely on MPPARSE driver to read MP floating pointer
structure's IMCRP Bit 7 of MP feature info byte 2 [5] to figure out if it
supports PIC mode or virtual wire mode and initialize pic_mode variable
accordingly. If ACPI is enabled, the ACPI code overrides it by using the
MADT table spec'ed in ACPI spec [2]. 

For #2 devicetree.c presently hardcodes pic_mode = 1 (PIC Mode). There is
no support to configure virtual wire mode via devicetree path for OF based
systems.

Now we have a platform which is OF based & does not use legacy 8259 PIC
interrupt controller. Non ACPI compliant as well as non MPPARSE compliant.

For such platforms, it appears to me that hardcoding pic_mode = 1 (PIC Mode)
and giving no other choice to choose virtual wire mode is a bug for OF
based x86 platforms. 

Just like mpparse relies on IMCRP bit 7 of MP feature info byte2 [5] to
select pic_mode to PIC mode or virtual wire mode. arch/x86/kernel/devicetree.c
should also provide some similar configurability to choose interrupt
delivery mode & not hardcode it to PIC mode.

This patch is to fix above explained bug in x86/of support for interrupt
delivery mode configuration. Please let me know if you find any mistake
in above understanding or if you have a alternative better suggestion to
solve it or if you find anything odd here in our platform/system. TIA.

The patch is baselined on below git tree (linux-v6.1.0-rc6):
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

[1] https://pdos.csail.mit.edu/6.828/2008/readings/ia32/MPspec.pdf
[2] https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
[3] https://elixir.bootlin.com/linux/v6.1-rc5/source/arch/x86/kernel/apic/apic.c#L1691
[4] https://elixir.bootlin.com/linux/v6.1-rc5/source/arch/x86/kernel/mpparse.c#L517
[5] https://www.manualslib.com/manual/77733/Intel-Multiprocessor.html?page=40#manual
[6] https://www.intel.com/content/www/us/en/developer/articles/technical/intel-sdm.html
[7] https://elixir.bootlin.com/linux/v6.1-rc5/source/arch/x86/kernel/devicetree.c#L170

v3:
- Address review concerns from Andy Shevchenko
  * Reshuffle patch series changes to make it more logical.
  * Patch 1 just converts existing intel,ce4100-ioapic.txt into
    YAML schema and separates out ioapic & lapic.
  * Patch 2 adds new optional property for lapic.
  * Patch 3 replaces older printk(KERN_LVL) to newer pr_lvl()
  * Patch 4 adds code changes in devicetree.c to support newly
    added property.
- Fix 'make DT_CHECKER_FLAGS=-m dt_binding_check' errors reported
  by Rob Herring's bot.

v2:
- Address review concern from Andy - rename property name to make
  it a bit more positive & self explanatory.
- Found that the bindings document for these HW's (APIC) are a bit
  off/obsolete and still in text format. Created new YAML schemas
  one for each - lapic & ioapic. Updated these schemas with latest
  info and add in new optional property details in the updated
  schema for lapic. Delete/let go of the text binding doc.
- CC devicetree@vger.kernel.org as these changes appear to be
  mainly targeted for devicetree maintainers review & approval.
- Increase CCed list to include all possible people who touched
  and were involved this part of code/feature addition.

v1:
- Initial draft


Rahul Tanwar (4):
  x86/of: Convert Intel's APIC bindings to YAML schema
  x86/of: Introduce new optional bool property for lapic
  x86/of: Replace printk(KERN_LVL) with pr_lvl()
  x86/of: Add support for boot time interrupt delivery mode
    configuration

 .../intel,ce4100-ioapic.txt                   | 26 --------
 .../intel,ce4100-ioapic.yaml                  | 62 ++++++++++++++++++
 .../intel,ce4100-lapic.yaml                   | 63 +++++++++++++++++++
 arch/x86/kernel/devicetree.c                  | 13 +++-
 4 files changed, 135 insertions(+), 29 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml

-- 
2.17.1


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

* [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema
  2022-11-22  7:39 [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Rahul Tanwar
@ 2022-11-22  7:39 ` Rahul Tanwar
  2022-11-22  9:10   ` Andy Shevchenko
  2022-11-23 16:02   ` Krzysztof Kozlowski
  2022-11-22  7:39 ` [PATCH v3 2/4] x86/of: Introduce new optional bool property for lapic Rahul Tanwar
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22  7:39 UTC (permalink / raw)
  To: bigeasy, robh, tglx, mingo, bp, x86, hpa
  Cc: andriy.shevchenko, dave.hansen, linux-kernel, linux-lgm-soc,
	Rahul Tanwar

Intel's APIC family of interrupt controllers support local APIC
(lapic) & I/O APIC (ioapic). Convert existing bindings for lapic
& ioapic from text to YAML schema. Separate lapic & ioapic schemas.
Addditionally, add description which was missing in text file and
add few more required standard properties which were also missing
in text file.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
---
 .../intel,ce4100-ioapic.txt                   | 26 --------
 .../intel,ce4100-ioapic.yaml                  | 62 +++++++++++++++++++
 .../intel,ce4100-lapic.yaml                   | 49 +++++++++++++++
 3 files changed, 111 insertions(+), 26 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
deleted file mode 100644
index 7d19f494f19a..000000000000
--- a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
+++ /dev/null
@@ -1,26 +0,0 @@
-Interrupt chips
----------------
-
-* Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
-
-  Required properties:
-  --------------------
-     compatible = "intel,ce4100-ioapic";
-     #interrupt-cells = <2>;
-
-  Device's interrupt property:
-
-     interrupts = <P S>;
-
-  The first number (P) represents the interrupt pin which is wired to the
-  IO APIC. The second number (S) represents the sense of interrupt which
-  should be configured and can be one of:
-    0 - Edge Rising
-    1 - Level Low
-    2 - Level High
-    3 - Edge Falling
-
-* Local APIC
-  Required property:
-
-     compatible = "intel,ce4100-lapic";
diff --git a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
new file mode 100644
index 000000000000..da966287eec2
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
+
+maintainers:
+  - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
+
+
+description: |
+  Intel's Advanced Programmable Interrupt Controller (APIC) is a
+  family of interrupt controllers. The APIC is a split
+  architecture design, with a local component (LAPIC) integrated
+  into the processor itself and an external I/O APIC. Local APIC
+  (lapic) receives interrupts from the processor's interrupt pins,
+  from internal sources and from an external I/O APIC (ioapic).
+  And it sends these to the processor core for handling.
+  See https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf
+  Chapter 8 for more details.
+
+  Many of the Intel's generic devices like hpet, ioapic, lapic have
+  the ce4100 name in their compatible property names because they
+  first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
+  details on it.
+
+  This schema defines bindings for I/O APIC interrupt controller.
+
+properties:
+  compatible:
+    const: intel,ce4100-ioapic
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - '#interrupt-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    ioapic1: interrupt-controller@fec00000 {
+        compatible = "intel,ce4100-ioapic";
+        reg = <0xfec00000 0x1000>;
+        #interrupt-cells = <2>;
+        #address-cells = <0>;
+        interrupt-controller;
+    };
diff --git a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
new file mode 100644
index 000000000000..d4b99bf7bf6e
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/interrupt-controller/intel,ce4100-lapic.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel Local Advanced Programmable Interrupt Controller (LAPIC)
+
+maintainers:
+  - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
+
+
+description: |
+  Intel's Advanced Programmable Interrupt Controller (APIC) is a
+  family of interrupt controllers. The APIC is a split
+  architecture design, with a local component (LAPIC) integrated
+  into the processor itself and an external I/O APIC. Local APIC
+  (lapic) receives interrupts from the processor's interrupt pins,
+  from internal sources and from an external I/O APIC (ioapic).
+  And it sends these to the processor core for handling.
+  See https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf
+  Chapter 8 for more details.
+
+  Many of the Intel's generic devices like hpet, ioapic, lapic have
+  the ce4100 name in their compatible property names because they
+  first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
+  details on it.
+
+  This schema defines bindings for local APIC interrupt controller.
+
+properties:
+  compatible:
+    const: intel,ce4100-lapic
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    lapic0: interrupt-controller@fee00000 {
+        compatible = "intel,ce4100-lapic";
+        reg = <0xfee00000 0x1000>;
+    };
-- 
2.17.1


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

* [PATCH v3 2/4] x86/of: Introduce new optional bool property for lapic
  2022-11-22  7:39 [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Rahul Tanwar
  2022-11-22  7:39 ` [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema Rahul Tanwar
@ 2022-11-22  7:39 ` Rahul Tanwar
  2022-11-22  7:39 ` [PATCH v3 3/4] x86/of: Replace printk(KERN_LVL) with pr_lvl() Rahul Tanwar
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22  7:39 UTC (permalink / raw)
  To: bigeasy, robh, tglx, mingo, bp, x86, hpa
  Cc: andriy.shevchenko, dave.hansen, linux-kernel, linux-lgm-soc,
	Rahul Tanwar

Intel defines a few possible interrupt delivery modes. With respect
to boot/init time, mainly two interrupt delivery modes are possible.
PIC Mode - Legacy external 8259 compliant PIC interrupt controller.
Virtual Wire Mode - use lapic as virtual wire interrupt delivery mode.

For ACPI or MPS spec compliant systems, it is figured out by some read
only bit field/s available in their respective defined data structures.
But for OF based systems, it is by default set to PIC mode. Presently,
it is hardcoded to legacy PIC mode for OF based x86 systems with no
option to choose the configuration between PIC mode & virtual wire mode.

For this purpose, introduce a new boolean property for interrupt
controller node of lapic which can allow it to be configured to virtual
wire mode as well.

Property name: 'intel,virtual-wire-mode'
Type: Boolean

If not present/not defined, interrupt delivery mode defaults to legacy PIC
mode. If present/defined, interrupt delivery mode is set to virtual wire
mode.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
---
 .../interrupt-controller/intel,ce4100-lapic.yaml   | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
index d4b99bf7bf6e..087f849e31ef 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
@@ -35,6 +35,19 @@ properties:
   reg:
     maxItems: 1
 
+  intel,virtual-wire-mode:
+    description: Intel defines a few possible interrupt delivery
+      modes. With respect to boot/init time, mainly two interrupt
+      delivery modes are possible.
+      PIC Mode - Legacy external 8259 compliant PIC interrupt controller.
+      Virtual Wire Mode - use lapic as virtual wire interrupt delivery mode.
+      For ACPI or MPS spec compliant systems, it is figured out by some read
+      only bit field/s available in their respective defined data structures.
+      For OF based systems, it is by default set to PIC mode.
+      But if this optional boolean property is set, then the interrupt delivery
+      mode is configured to virtual wire compatibility mode.
+    type: boolean
+
 required:
   - compatible
   - reg
@@ -46,4 +59,5 @@ examples:
     lapic0: interrupt-controller@fee00000 {
         compatible = "intel,ce4100-lapic";
         reg = <0xfee00000 0x1000>;
+        intel,virtual-wire-mode;
     };
-- 
2.17.1


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

* [PATCH v3 3/4] x86/of: Replace printk(KERN_LVL) with pr_lvl()
  2022-11-22  7:39 [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Rahul Tanwar
  2022-11-22  7:39 ` [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema Rahul Tanwar
  2022-11-22  7:39 ` [PATCH v3 2/4] x86/of: Introduce new optional bool property for lapic Rahul Tanwar
@ 2022-11-22  7:39 ` Rahul Tanwar
  2022-11-22  9:14   ` Andy Shevchenko
  2022-11-22  7:39 ` [PATCH v3 4/4] x86/of: Add support for boot time interrupt delivery mode configuration Rahul Tanwar
  2022-11-22  9:17 ` [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Andy Shevchenko
  4 siblings, 1 reply; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22  7:39 UTC (permalink / raw)
  To: bigeasy, robh, tglx, mingo, bp, x86, hpa
  Cc: andriy.shevchenko, dave.hansen, linux-kernel, linux-lgm-soc,
	Rahul Tanwar

Use latest available pr_lvl() instead of older printk(KERN_LVL)
Just a upgrade of print utilities usage no functional changes.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
---
 arch/x86/kernel/devicetree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 5cd51f25f446..fcc6f1b7818f 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -248,7 +248,7 @@ static void __init dtb_add_ioapic(struct device_node *dn)
 
 	ret = of_address_to_resource(dn, 0, &r);
 	if (ret) {
-		printk(KERN_ERR "Can't obtain address from device node %pOF.\n", dn);
+		pr_err("Can't obtain address from device node %pOF.\n", dn);
 		return;
 	}
 	mp_register_ioapic(++ioapic_id, r.start, gsi_top, &cfg);
@@ -265,7 +265,7 @@ static void __init dtb_ioapic_setup(void)
 		of_ioapic = 1;
 		return;
 	}
-	printk(KERN_ERR "Error: No information about IO-APIC in OF.\n");
+	pr_err("Error: No information about IO-APIC in OF.\n");
 }
 #else
 static void __init dtb_ioapic_setup(void) {}
-- 
2.17.1


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

* [PATCH v3 4/4] x86/of: Add support for boot time interrupt delivery mode configuration
  2022-11-22  7:39 [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Rahul Tanwar
                   ` (2 preceding siblings ...)
  2022-11-22  7:39 ` [PATCH v3 3/4] x86/of: Replace printk(KERN_LVL) with pr_lvl() Rahul Tanwar
@ 2022-11-22  7:39 ` Rahul Tanwar
  2022-11-22  9:14   ` Andy Shevchenko
  2022-11-22  9:17 ` [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Andy Shevchenko
  4 siblings, 1 reply; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22  7:39 UTC (permalink / raw)
  To: bigeasy, robh, tglx, mingo, bp, x86, hpa
  Cc: andriy.shevchenko, dave.hansen, linux-kernel, linux-lgm-soc,
	Rahul Tanwar

Presently, init/boot time interrupt delivery mode is enumerated
only for ACPI enabled systems by parsing MADT table or for older
systems by parsing MP table. But for OF based x86 systems, it is
assumed & hardcoded to legacy PIC mode. This is a bug for
platforms which are OF based but do not use 8259 compliant legacy
PIC interrupt controller. Such platforms can not even boot because
of this bug/hardcoding.

Fix this bug by adding support for configuration of init time
interrupt delivery mode for x86 OF based systems by introducing a
new optional boolean property 'intel,virtual-wire-mode' for
interrupt-controller node of local APIC. This property emulates
IMCRP Bit 7 of MP feature info byte 2 of MP floating pointer
structure.

Defaults to legacy PIC mode if absent. Configures it to virtual
wire compatibility mode if present.

Fixes: 3879a6f32948 ("x86: dtb: Add early parsing of IO_APIC")
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
---
 arch/x86/kernel/devicetree.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index fcc6f1b7818f..458e43490414 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -167,7 +167,14 @@ static void __init dtb_lapic_setup(void)
 			return;
 	}
 	smp_found_config = 1;
-	pic_mode = 1;
+	if (of_property_read_bool(dn, "intel,virtual-wire-mode")) {
+		pr_info("Virtual Wire compatibility mode.\n");
+		pic_mode = 0;
+	} else {
+		pr_info("IMCR and PIC compatibility mode.\n");
+		pic_mode = 1;
+	}
+
 	register_lapic_address(lapic_addr);
 }
 
-- 
2.17.1


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

* Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema
  2022-11-22  7:39 ` [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema Rahul Tanwar
@ 2022-11-22  9:10   ` Andy Shevchenko
  2022-11-22  9:43     ` Rahul Tanwar
  2022-11-23 16:02   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-22  9:10 UTC (permalink / raw)
  To: Rahul Tanwar
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On Tue, Nov 22, 2022 at 03:39:07PM +0800, Rahul Tanwar wrote:
> Intel's APIC family of interrupt controllers support local APIC
> (lapic) & I/O APIC (ioapic). Convert existing bindings for lapic
> & ioapic from text to YAML schema. Separate lapic & ioapic schemas.
> Addditionally, add description which was missing in text file and
> add few more required standard properties which were also missing
> in text file.

...

> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Intel I/O Advanced Programmable Interrupt Controller (IO APIC)

> +maintainers:
> +  - Sebastian Andrzej Siewior <bigeasy@linutronix.de>

I'm not sure, you need to have a confirmation before putting someone's name here.
Yours is easier to add.

> +description: |
> +  Intel's Advanced Programmable Interrupt Controller (APIC) is a
> +  family of interrupt controllers. The APIC is a split
> +  architecture design, with a local component (LAPIC) integrated
> +  into the processor itself and an external I/O APIC. Local APIC
> +  (lapic) receives interrupts from the processor's interrupt pins,
> +  from internal sources and from an external I/O APIC (ioapic).
> +  And it sends these to the processor core for handling.

> +  See https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf

Dunno if schema has special format for data sheet links...

> +  Chapter 8 for more details.
> +
> +  Many of the Intel's generic devices like hpet, ioapic, lapic have
> +  the ce4100 name in their compatible property names because they

> +  first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more

Shouldn't you change this?

> +  details on it.
> +
> +  This schema defines bindings for I/O APIC interrupt controller.

...

> +maintainers:
> +  - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> +
> +
> +description: |
> +  Intel's Advanced Programmable Interrupt Controller (APIC) is a
> +  family of interrupt controllers. The APIC is a split
> +  architecture design, with a local component (LAPIC) integrated
> +  into the processor itself and an external I/O APIC. Local APIC
> +  (lapic) receives interrupts from the processor's interrupt pins,
> +  from internal sources and from an external I/O APIC (ioapic).
> +  And it sends these to the processor core for handling.
> +  See https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf
> +  Chapter 8 for more details.
> +
> +  Many of the Intel's generic devices like hpet, ioapic, lapic have
> +  the ce4100 name in their compatible property names because they
> +  first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
> +  details on it.
> +
> +  This schema defines bindings for local APIC interrupt controller.

Same two comments as per above.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 4/4] x86/of: Add support for boot time interrupt delivery mode configuration
  2022-11-22  7:39 ` [PATCH v3 4/4] x86/of: Add support for boot time interrupt delivery mode configuration Rahul Tanwar
@ 2022-11-22  9:14   ` Andy Shevchenko
  2022-11-22  9:45     ` Rahul Tanwar
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-22  9:14 UTC (permalink / raw)
  To: Rahul Tanwar
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On Tue, Nov 22, 2022 at 03:39:10PM +0800, Rahul Tanwar wrote:
> Presently, init/boot time interrupt delivery mode is enumerated
> only for ACPI enabled systems by parsing MADT table or for older
> systems by parsing MP table. But for OF based x86 systems, it is
> assumed & hardcoded to legacy PIC mode. This is a bug for
> platforms which are OF based but do not use 8259 compliant legacy
> PIC interrupt controller. Such platforms can not even boot because
> of this bug/hardcoding.
> 
> Fix this bug by adding support for configuration of init time
> interrupt delivery mode for x86 OF based systems by introducing a
> new optional boolean property 'intel,virtual-wire-mode' for
> interrupt-controller node of local APIC. This property emulates
> IMCRP Bit 7 of MP feature info byte 2 of MP floating pointer
> structure.
> 
> Defaults to legacy PIC mode if absent. Configures it to virtual
> wire compatibility mode if present.

> Fixes: 3879a6f32948 ("x86: dtb: Add early parsing of IO_APIC")

If it was never working, there is nothing to fix.
OTOH, without Cc: stable@ this is up to stable maintainers to
backport.


> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I definitely haven't suggested this fix.

...

The code looks good to me.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 3/4] x86/of: Replace printk(KERN_LVL) with pr_lvl()
  2022-11-22  7:39 ` [PATCH v3 3/4] x86/of: Replace printk(KERN_LVL) with pr_lvl() Rahul Tanwar
@ 2022-11-22  9:14   ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-22  9:14 UTC (permalink / raw)
  To: Rahul Tanwar
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On Tue, Nov 22, 2022 at 03:39:09PM +0800, Rahul Tanwar wrote:
> Use latest available pr_lvl() instead of older printk(KERN_LVL)
> Just a upgrade of print utilities usage no functional changes.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
> ---
>  arch/x86/kernel/devicetree.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
> index 5cd51f25f446..fcc6f1b7818f 100644
> --- a/arch/x86/kernel/devicetree.c
> +++ b/arch/x86/kernel/devicetree.c
> @@ -248,7 +248,7 @@ static void __init dtb_add_ioapic(struct device_node *dn)
>  
>  	ret = of_address_to_resource(dn, 0, &r);
>  	if (ret) {
> -		printk(KERN_ERR "Can't obtain address from device node %pOF.\n", dn);
> +		pr_err("Can't obtain address from device node %pOF.\n", dn);
>  		return;
>  	}
>  	mp_register_ioapic(++ioapic_id, r.start, gsi_top, &cfg);
> @@ -265,7 +265,7 @@ static void __init dtb_ioapic_setup(void)
>  		of_ioapic = 1;
>  		return;
>  	}
> -	printk(KERN_ERR "Error: No information about IO-APIC in OF.\n");
> +	pr_err("Error: No information about IO-APIC in OF.\n");
>  }
>  #else
>  static void __init dtb_ioapic_setup(void) {}
> -- 
> 2.17.1
> 

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support
  2022-11-22  7:39 [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Rahul Tanwar
                   ` (3 preceding siblings ...)
  2022-11-22  7:39 ` [PATCH v3 4/4] x86/of: Add support for boot time interrupt delivery mode configuration Rahul Tanwar
@ 2022-11-22  9:17 ` Andy Shevchenko
  2022-11-22  9:49   ` Rahul Tanwar
  4 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-22  9:17 UTC (permalink / raw)
  To: Rahul Tanwar
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On Tue, Nov 22, 2022 at 03:39:06PM +0800, Rahul Tanwar wrote:

> Rahul Tanwar (4):
>   x86/of: Convert Intel's APIC bindings to YAML schema
>   x86/of: Introduce new optional bool property for lapic

You need properly prefix the first two patches. I guess it's something like
"dt-bindings: x86: ioapic:".

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema
  2022-11-22  9:10   ` Andy Shevchenko
@ 2022-11-22  9:43     ` Rahul Tanwar
  2022-11-22 10:09       ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22  9:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On 22/11/2022 5:11 pm, Andy Shevchenko wrote:
> This email was sent from outside of MaxLinear.
> 
> On Tue, Nov 22, 2022 at 03:39:07PM +0800, Rahul Tanwar wrote:
>  > Intel's APIC family of interrupt controllers support local APIC
>  > (lapic) & I/O APIC (ioapic). Convert existing bindings for lapic
>  > & ioapic from text to YAML schema. Separate lapic & ioapic schemas.
>  > Addditionally, add description which was missing in text file and
>  > add few more required standard properties which were also missing
>  > in text file.
> 
> ...
> 
>  > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>  > +%YAML 1.2
>  > +---
>  > +$id: 
> "http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml# <http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml#>"
>  > +$schema: "http://devicetree.org/meta-schemas/core.yaml# 
> <http://devicetree.org/meta-schemas/core.yaml#>"
>  > +
>  > +title: Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
> 
>  > +maintainers:
>  > + - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> I'm not sure, you need to have a confirmation before putting someone's 
> name here.
> Yours is easier to add.
>

Well noted, will update.



>  > +description: |
>  > + Intel's Advanced Programmable Interrupt Controller (APIC) is a
>  > + family of interrupt controllers. The APIC is a split
>  > + architecture design, with a local component (LAPIC) integrated
>  > + into the processor itself and an external I/O APIC. Local APIC
>  > + (lapic) receives interrupts from the processor's interrupt pins,
>  > + from internal sources and from an external I/O APIC (ioapic).
>  > + And it sends these to the processor core for handling.
> 
>  > + See https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf 
> <https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf>
> 
> Dunno if schema has special format for data sheet links...
> 


Example-schema says this is the place to put URL's..



>  > + Chapter 8 for more details.
>  > +
>  > + Many of the Intel's generic devices like hpet, ioapic, lapic have
>  > + the ce4100 name in their compatible property names because they
> 
>  > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
> 
> Shouldn't you change this?
> 


Do you mean change compatibility property prefix from 
"intel,ce4100-ioapic" to "intel,ioapic"? If yes, then i totally agree 
and i will change it (including new file names & all other references to 
ce4100). If not, please clarify more..


>  > + details on it.
>  > +
>  > + This schema defines bindings for I/O APIC interrupt controller.
> 
> ...
> 
>  > +maintainers:
>  > + - Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>  > +
>  > +
>  > +description: |
>  > + Intel's Advanced Programmable Interrupt Controller (APIC) is a
>  > + family of interrupt controllers. The APIC is a split
>  > + architecture design, with a local component (LAPIC) integrated
>  > + into the processor itself and an external I/O APIC. Local APIC
>  > + (lapic) receives interrupts from the processor's interrupt pins,
>  > + from internal sources and from an external I/O APIC (ioapic).
>  > + And it sends these to the processor core for handling.
>  > + See https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf 
> <https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf>
>  > + Chapter 8 for more details.
>  > +
>  > + Many of the Intel's generic devices like hpet, ioapic, lapic have
>  > + the ce4100 name in their compatible property names because they
>  > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
>  > + details on it.
>  > +
>  > + This schema defines bindings for local APIC interrupt controller.
> 
> Same two comments as per above.
>


Well noted.


> -- 
> With Best Regards,
> Andy Shevchenko
> 


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

* Re: [PATCH v3 4/4] x86/of: Add support for boot time interrupt delivery mode configuration
  2022-11-22  9:14   ` Andy Shevchenko
@ 2022-11-22  9:45     ` Rahul Tanwar
  2022-11-22 10:10       ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22  9:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On 22/11/2022 5:14 pm, Andy Shevchenko wrote:
> This email was sent from outside of MaxLinear.
> 
> 
> On Tue, Nov 22, 2022 at 03:39:10PM +0800, Rahul Tanwar wrote:
>> Presently, init/boot time interrupt delivery mode is enumerated
>> only for ACPI enabled systems by parsing MADT table or for older
>> systems by parsing MP table. But for OF based x86 systems, it is
>> assumed & hardcoded to legacy PIC mode. This is a bug for
>> platforms which are OF based but do not use 8259 compliant legacy
>> PIC interrupt controller. Such platforms can not even boot because
>> of this bug/hardcoding.
>>
>> Fix this bug by adding support for configuration of init time
>> interrupt delivery mode for x86 OF based systems by introducing a
>> new optional boolean property 'intel,virtual-wire-mode' for
>> interrupt-controller node of local APIC. This property emulates
>> IMCRP Bit 7 of MP feature info byte 2 of MP floating pointer
>> structure.
>>
>> Defaults to legacy PIC mode if absent. Configures it to virtual
>> wire compatibility mode if present.
> 
>> Fixes: 3879a6f32948 ("x86: dtb: Add early parsing of IO_APIC")
> 
> If it was never working, there is nothing to fix.
> OTOH, without Cc: stable@ this is up to stable maintainers to
> backport.
>

Agree, will remove fixes tag..


> 
>> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> I definitely haven't suggested this fix.
> 

Yes, i copy pasted to entire series. I will remove it from this patch.

> ...
> 
> The code looks good to me.
>

Thanks.

Regards,
Rahul


> --
> With Best Regards,
> Andy Shevchenko
> 
> 
> 


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

* Re: [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support
  2022-11-22  9:17 ` [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Andy Shevchenko
@ 2022-11-22  9:49   ` Rahul Tanwar
  2022-11-22 10:14     ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22  9:49 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On 22/11/2022 5:18 pm, Andy Shevchenko wrote:
> This email was sent from outside of MaxLinear.
> 
> 
> On Tue, Nov 22, 2022 at 03:39:06PM +0800, Rahul Tanwar wrote:
> 
>> Rahul Tanwar (4):
>>    x86/of: Convert Intel's APIC bindings to YAML schema
>>    x86/of: Introduce new optional bool property for lapic
> 
> You need properly prefix the first two patches. I guess it's something like
> "dt-bindings: x86: ioapic:".
> 

Yes, i just checked the git log of devicetree.c and used same prefixes 
here. Thanks for correcting it. I will update it.

Regards,
Rahul


> --
> With Best Regards,
> Andy Shevchenko
> 
> 
> 


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

* Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema
  2022-11-22  9:43     ` Rahul Tanwar
@ 2022-11-22 10:09       ` Andy Shevchenko
  2022-11-22 10:37         ` Rahul Tanwar
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-22 10:09 UTC (permalink / raw)
  To: Rahul Tanwar
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On Tue, Nov 22, 2022 at 09:43:12AM +0000, Rahul Tanwar wrote:
> On 22/11/2022 5:11 pm, Andy Shevchenko wrote:
> > On Tue, Nov 22, 2022 at 03:39:07PM +0800, Rahul Tanwar wrote:

...

> >  > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
> > 
> > Shouldn't you change this?
> 
> Do you mean change compatibility property prefix from 
> "intel,ce4100-ioapic" to "intel,ioapic"? If yes, then i totally agree 
> and i will change it (including new file names & all other references to 
> ce4100). If not, please clarify more..

I specifically emphasized a single line (by putting blank lines around).
For your convenience I removed the unneeded parts of the context, so you can
see better what I meant.

...

> >  > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more


Ditto.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 4/4] x86/of: Add support for boot time interrupt delivery mode configuration
  2022-11-22  9:45     ` Rahul Tanwar
@ 2022-11-22 10:10       ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-22 10:10 UTC (permalink / raw)
  To: Rahul Tanwar
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On Tue, Nov 22, 2022 at 09:45:29AM +0000, Rahul Tanwar wrote:
> On 22/11/2022 5:14 pm, Andy Shevchenko wrote:
> > On Tue, Nov 22, 2022 at 03:39:10PM +0800, Rahul Tanwar wrote:

...

> >> Fixes: 3879a6f32948 ("x86: dtb: Add early parsing of IO_APIC")
> > 
> > If it was never working, there is nothing to fix.
> > OTOH, without Cc: stable@ this is up to stable maintainers to
> > backport.
> 
> Agree, will remove fixes tag..

Don't forget to update cover letter and messages accordingly.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support
  2022-11-22  9:49   ` Rahul Tanwar
@ 2022-11-22 10:14     ` Andy Shevchenko
  2022-11-22 10:45       ` Rahul Tanwar
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-22 10:14 UTC (permalink / raw)
  To: Rahul Tanwar
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On Tue, Nov 22, 2022 at 09:49:04AM +0000, Rahul Tanwar wrote:
> On 22/11/2022 5:18 pm, Andy Shevchenko wrote:
> > On Tue, Nov 22, 2022 at 03:39:06PM +0800, Rahul Tanwar wrote:
> > 
> >> Rahul Tanwar (4):
> >>    x86/of: Convert Intel's APIC bindings to YAML schema
> >>    x86/of: Introduce new optional bool property for lapic
> > 
> > You need properly prefix the first two patches. I guess it's something like
> > "dt-bindings: x86: ioapic:".
> > 
> 
> Yes, i just checked the git log of devicetree.c and used same prefixes 
> here. Thanks for correcting it. I will update it.

Moreover the Cc list in those patches is quite wrong.

I recommend to utilize my "smart" script [1] for sending series.
(You may take an idea from it how to prepare the Cc list close to proper one)

[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema
  2022-11-22 10:09       ` Andy Shevchenko
@ 2022-11-22 10:37         ` Rahul Tanwar
  0 siblings, 0 replies; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22 10:37 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On 22/11/2022 6:09 pm, Andy Shevchenko wrote:
> This email was sent from outside of MaxLinear.
> 
> 
> On Tue, Nov 22, 2022 at 09:43:12AM +0000, Rahul Tanwar wrote:
>> On 22/11/2022 5:11 pm, Andy Shevchenko wrote:
>>> On Tue, Nov 22, 2022 at 03:39:07PM +0800, Rahul Tanwar wrote:
> 
> ...
> 
>>>   > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
>>>
>>> Shouldn't you change this?
>>
>> Do you mean change compatibility property prefix from
>> "intel,ce4100-ioapic" to "intel,ioapic"? If yes, then i totally agree
>> and i will change it (including new file names & all other references to
>> ce4100). If not, please clarify more..
> 
> I specifically emphasized a single line (by putting blank lines around).
> For your convenience I removed the unneeded parts of the context, so you can
> see better what I meant.
>


Got it. I will remove the mention of "See bindings/x86/ce4100.txt" from 
here.



> ...
> 
>>>   > + first appeared in CE4100 SoC. See bindings/x86/ce4100.txt for more
> 
> 
> Ditto.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 
> 


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

* Re: [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support
  2022-11-22 10:14     ` Andy Shevchenko
@ 2022-11-22 10:45       ` Rahul Tanwar
  2022-11-22 12:42         ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-22 10:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On 22/11/2022 6:15 pm, Andy Shevchenko wrote:
> This email was sent from outside of MaxLinear.
> 
> On Tue, Nov 22, 2022 at 09:49:04AM +0000, Rahul Tanwar wrote:
>  > On 22/11/2022 5:18 pm, Andy Shevchenko wrote:
>  > > On Tue, Nov 22, 2022 at 03:39:06PM +0800, Rahul Tanwar wrote:
>  > >
>  > >> Rahul Tanwar (4):
>  > >> x86/of: Convert Intel's APIC bindings to YAML schema
>  > >> x86/of: Introduce new optional bool property for lapic
>  > >
>  > > You need properly prefix the first two patches. I guess it's 
> something like
>  > > "dt-bindings: x86: ioapic:".
>  > >
>  >
>  > Yes, i just checked the git log of devicetree.c and used same prefixes
>  > here. Thanks for correcting it. I will update it.
> 
> Moreover the Cc list in those patches is quite wrong.
> 
> I recommend to utilize my "smart" script [1] for sending series.
> (You may take an idea from it how to prepare the Cc list close to proper 
> one)
> 
> [1]: 
> https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh 
> <https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh>
>

Agree on the Cc list being wrong. Thanks for the script.

Quite useful. Could not understand how the script works for multiple 
commits when you take count & version as inputs. Also, where does it 
create patches (format-patch)? It seems to get suitable to & cc list & 
send emails to them. And the input is COMMIT_HASH. How do i use it for a 
series with multiple commits & do i have to create patches on my own? 
Thanks again.

Regards,
Rahul


> -- 
> With Best Regards,
> Andy Shevchenko
> 


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

* Re: [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support
  2022-11-22 10:45       ` Rahul Tanwar
@ 2022-11-22 12:42         ` Andy Shevchenko
  2022-11-23  9:52           ` Rahul Tanwar
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-22 12:42 UTC (permalink / raw)
  To: Rahul Tanwar
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On Tue, Nov 22, 2022 at 10:45:12AM +0000, Rahul Tanwar wrote:
> On 22/11/2022 6:15 pm, Andy Shevchenko wrote:
> > On Tue, Nov 22, 2022 at 09:49:04AM +0000, Rahul Tanwar wrote:

...

> > I recommend to utilize my "smart" script [1] for sending series.
> > (You may take an idea from it how to prepare the Cc list close to proper 
> > one)
> > 
> > [1]: 
> > https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh 
> 
> Agree on the Cc list being wrong. Thanks for the script.
> 
> Quite useful. Could not understand how the script works for multiple 
> commits when you take count & version as inputs.

It starts from the commit <COMMIT_HASH>~$count and goes up. So it's basically
depth from the given commit (<COMMIT_HASH>).

> Also, where does it 
> create patches (format-patch)? It seems to get suitable to & cc list & 
> send emails to them. And the input is COMMIT_HASH. How do i use it for a 
> series with multiple commits & do i have to create patches on my own? 

It creates patches automatically in the same was as it's done by
`git format-patch`. That's why it accepts a lot of the parameters
which you usually add there.

Typical use for the series is

  ge2maintainer.sh -v 1 -c 4 HEAD~0 --annotate --cover-letter

Note, that your editor should be able to handle several files to edit
(e.g. vim supports that mode).

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support
  2022-11-22 12:42         ` Andy Shevchenko
@ 2022-11-23  9:52           ` Rahul Tanwar
  2022-11-23 13:47             ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-23  9:52 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On 22/11/2022 8:42 pm, Andy Shevchenko wrote:
> This email was sent from outside of MaxLinear.
> 
> On Tue, Nov 22, 2022 at 10:45:12AM +0000, Rahul Tanwar wrote:
>  > On 22/11/2022 6:15 pm, Andy Shevchenko wrote:
>  > > On Tue, Nov 22, 2022 at 09:49:04AM +0000, Rahul Tanwar wrote:
> 
> ...
> 
>  > > I recommend to utilize my "smart" script [1] for sending series.
>  > > (You may take an idea from it how to prepare the Cc list close to 
> proper
>  > > one)
>  > >
>  > > [1]:
>  > > 
> https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh 
> <https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh>
>  >
>  > Agree on the Cc list being wrong. Thanks for the script.
>  >
>  > Quite useful. Could not understand how the script works for multiple
>  > commits when you take count & version as inputs.
> 
> It starts from the commit <COMMIT_HASH>~$count and goes up. So it's 
> basically
> depth from the given commit (<COMMIT_HASH>).
> 
>  > Also, where does it
>  > create patches (format-patch)? It seems to get suitable to & cc list &
>  > send emails to them. And the input is COMMIT_HASH. How do i use it for a
>  > series with multiple commits & do i have to create patches on my own?
> 
> It creates patches automatically in the same was as it's done by
> `git format-patch`. That's why it accepts a lot of the parameters
> which you usually add there.
> 
> Typical use for the series is
> 
> ge2maintainer.sh -v 1 -c 4 HEAD~0 --annotate --cover-letter
> 
> Note, that your editor should be able to handle several files to edit
> (e.g. vim supports that mode).
> 


The script worked like a charm. Very useful. Thanks. But i missed to CC 
you while using the script. I have addressed all your concerns and 
already sent v4. Request to please review the series starting from below 
link:
https://lore.kernel.org/all/20221123093820.21161-1-rtanwar@maxlinear.com/

Regards,
Rahul

> -- 
> With Best Regards,
> Andy Shevchenko
> 


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

* Re: [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support
  2022-11-23  9:52           ` Rahul Tanwar
@ 2022-11-23 13:47             ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-23 13:47 UTC (permalink / raw)
  To: Rahul Tanwar
  Cc: bigeasy, robh, tglx, mingo, bp, x86, hpa, dave.hansen,
	linux-kernel, linux-lgm-soc

On Wed, Nov 23, 2022 at 09:52:03AM +0000, Rahul Tanwar wrote:
> On 22/11/2022 8:42 pm, Andy Shevchenko wrote:
> > On Tue, Nov 22, 2022 at 10:45:12AM +0000, Rahul Tanwar wrote:
> >  > On 22/11/2022 6:15 pm, Andy Shevchenko wrote:
> >  > > On Tue, Nov 22, 2022 at 09:49:04AM +0000, Rahul Tanwar wrote:

...

> > It creates patches automatically in the same was as it's done by
> > `git format-patch`. That's why it accepts a lot of the parameters
> > which you usually add there.
> > 
> > Typical use for the series is
> > 
> > ge2maintainer.sh -v 1 -c 4 HEAD~0 --annotate --cover-letter
> > 
> > Note, that your editor should be able to handle several files to edit
> > (e.g. vim supports that mode).
> 
> The script worked like a charm. Very useful. Thanks.

Thank you for trying it. I'm using it on a daily basis.

> But i missed to CC 
> you while using the script. I have addressed all your concerns and 
> already sent v4. Request to please review the series starting from below 
> link:
> https://lore.kernel.org/all/20221123093820.21161-1-rtanwar@maxlinear.com/

Since we have lore.kernel.org and very powerful tool, called `b4` (you may
install it, if you have one of the most spread Linux distro, with your package
manager) no need to resend. The Cc'ing on cover letter is enough, I can
retrieve the whole thread from lore.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema
  2022-11-22  7:39 ` [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema Rahul Tanwar
  2022-11-22  9:10   ` Andy Shevchenko
@ 2022-11-23 16:02   ` Krzysztof Kozlowski
  2022-11-23 17:24     ` Andy Shevchenko
  2022-11-24  8:33     ` Rahul Tanwar
  1 sibling, 2 replies; 23+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-23 16:02 UTC (permalink / raw)
  To: Rahul Tanwar, bigeasy, robh, tglx, mingo, bp, x86, hpa
  Cc: andriy.shevchenko, dave.hansen, linux-kernel, linux-lgm-soc

On 22/11/2022 08:39, Rahul Tanwar wrote:
> Intel's APIC family of interrupt controllers support local APIC
> (lapic) & I/O APIC (ioapic). Convert existing bindings for lapic
> & ioapic from text to YAML schema. Separate lapic & ioapic schemas.
> Addditionally, add description which was missing in text file and
> add few more required standard properties which were also missing
> in text file.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
> ---
>  .../intel,ce4100-ioapic.txt                   | 26 --------
>  .../intel,ce4100-ioapic.yaml                  | 62 +++++++++++++++++++
>  .../intel,ce4100-lapic.yaml                   | 49 +++++++++++++++
>  3 files changed, 111 insertions(+), 26 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
> 

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.

You miss not only people but also lists, meaning this will not be
automatically tested.

So: No.

Best regards,
Krzysztof


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

* Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema
  2022-11-23 16:02   ` Krzysztof Kozlowski
@ 2022-11-23 17:24     ` Andy Shevchenko
  2022-11-24  8:33     ` Rahul Tanwar
  1 sibling, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2022-11-23 17:24 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rahul Tanwar, bigeasy, robh, tglx, mingo, bp, x86, hpa,
	dave.hansen, linux-kernel, linux-lgm-soc

On Wed, Nov 23, 2022 at 05:02:33PM +0100, Krzysztof Kozlowski wrote:
> On 22/11/2022 08:39, Rahul Tanwar wrote:

...

> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC.  It might happen, that command when run on an older
> kernel, gives you outdated entries.  Therefore please be sure you base
> your patches on recent Linux kernel.
> 
> You miss not only people but also lists, meaning this will not be
> automatically tested.

It seems that v4 manages to get the testing (and it's a good thing since
it found some issues).

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema
  2022-11-23 16:02   ` Krzysztof Kozlowski
  2022-11-23 17:24     ` Andy Shevchenko
@ 2022-11-24  8:33     ` Rahul Tanwar
  1 sibling, 0 replies; 23+ messages in thread
From: Rahul Tanwar @ 2022-11-24  8:33 UTC (permalink / raw)
  To: Krzysztof Kozlowski, bigeasy, robh, tglx, mingo, bp, x86, hpa
  Cc: andriy.shevchenko, dave.hansen, linux-kernel, linux-lgm-soc

On 24/11/2022 12:08 am, Krzysztof Kozlowski wrote:
> This email was sent from outside of MaxLinear.
> 
> 
> On 22/11/2022 08:39, Rahul Tanwar wrote:
>> Intel's APIC family of interrupt controllers support local APIC
>> (lapic) & I/O APIC (ioapic). Convert existing bindings for lapic
>> & ioapic from text to YAML schema. Separate lapic & ioapic schemas.
>> Addditionally, add description which was missing in text file and
>> add few more required standard properties which were also missing
>> in text file.
>>
>> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Signed-off-by: Rahul Tanwar <rtanwar@maxlinear.com>
>> ---
>>   .../intel,ce4100-ioapic.txt                   | 26 --------
>>   .../intel,ce4100-ioapic.yaml                  | 62 +++++++++++++++++++
>>   .../intel,ce4100-lapic.yaml                   | 49 +++++++++++++++
>>   3 files changed, 111 insertions(+), 26 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
>>   create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
>>   create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
>>
> 
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC.  It might happen, that command when run on an older
> kernel, gives you outdated entries.  Therefore please be sure you base
> your patches on recent Linux kernel.
> 
> You miss not only people but also lists, meaning this will not be
> automatically tested.
> 
> So: No.
> 


Agree that i made mistakes in email list earlier. But i fixed that 
problem from v4 onwards thanks to Andy. From v4 onwards, To & Cc should 
be correct. Thanks.

Regards,
Rahul


> Best regards,
> Krzysztof
> 
> 


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

end of thread, other threads:[~2022-11-24  8:34 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-22  7:39 [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Rahul Tanwar
2022-11-22  7:39 ` [PATCH v3 1/4] x86/of: Convert Intel's APIC bindings to YAML schema Rahul Tanwar
2022-11-22  9:10   ` Andy Shevchenko
2022-11-22  9:43     ` Rahul Tanwar
2022-11-22 10:09       ` Andy Shevchenko
2022-11-22 10:37         ` Rahul Tanwar
2022-11-23 16:02   ` Krzysztof Kozlowski
2022-11-23 17:24     ` Andy Shevchenko
2022-11-24  8:33     ` Rahul Tanwar
2022-11-22  7:39 ` [PATCH v3 2/4] x86/of: Introduce new optional bool property for lapic Rahul Tanwar
2022-11-22  7:39 ` [PATCH v3 3/4] x86/of: Replace printk(KERN_LVL) with pr_lvl() Rahul Tanwar
2022-11-22  9:14   ` Andy Shevchenko
2022-11-22  7:39 ` [PATCH v3 4/4] x86/of: Add support for boot time interrupt delivery mode configuration Rahul Tanwar
2022-11-22  9:14   ` Andy Shevchenko
2022-11-22  9:45     ` Rahul Tanwar
2022-11-22 10:10       ` Andy Shevchenko
2022-11-22  9:17 ` [PATCH v3 0/4] x86/of: Fix a bug in x86 arch OF support Andy Shevchenko
2022-11-22  9:49   ` Rahul Tanwar
2022-11-22 10:14     ` Andy Shevchenko
2022-11-22 10:45       ` Rahul Tanwar
2022-11-22 12:42         ` Andy Shevchenko
2022-11-23  9:52           ` Rahul Tanwar
2022-11-23 13:47             ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.