irqchip: Add Sophgo SG2042 MSI controller
cover irqchip: Add Sophgo SG2042 MSI controller
From: Chen Wang <[email protected]>
This controller is on the Sophgo SG2042 SoC to transform interrupts from PCIe MSI to PLIC interrupts.
Chen Wang (3):
dt-bindings: interrupt-controller: Add Sophgo SG2042 MSI
irqchip: Add the Sophgo SG2042 MSI interrupt controller
riscv: sophgo: dts: add msi controller for SG2042
.../sophgo,sg2042-msi.yaml | 78 ++++++ arch/riscv/boot/dts/sophgo/sg2042.dtsi | 13 + drivers/irqchip/Kconfig | 8 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-sg2042-msi.c | 255 ++++++++++++++++++ 5 files changed, 355 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml create mode 100644 drivers/irqchip/irq-sg2042-msi.c
base-commit: 2d5404caa8c7bb5c4e0435f94b28834ae5456623
Signature · 1 line
2.34.1
PATCH 1/3 dt-bindings: interrupt-controller: Add Sophgo SG2042 MSI
From: Chen Wang <[email protected]>
Add binding for Sophgo SG2042 MSI controller.
--- .../sophgo,sg2042-msi.yaml | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +78 -0
@@ -0,0 +1,78 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/sophgo,sg2042-msi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sophgo SG2042 MSI Controller + +maintainers: + - Chen Wang <[email protected]> + +description: + This interrupt controller is in Sophgo SG2042 for transforming interrupts from + PCIe MSI to PLIC interrupts. + +allOf: + - $ref: /schemas/interrupts.yaml# + - $ref: /schemas/interrupt-controller/msi-controller.yaml# + +properties: + compatible: + const: sophgo,sg2042-msi + + reg: + items: + - description: clear register + + reg-names: + items: + - const: clr + + sophgo,msi-doorbell-addr: + description: + u64 value of the MSI doorbell address + $ref: /schemas/types.yaml#/definitions/uint64 + + sophgo,msi-base-vec: + description: + u32 value of the base of parent PLIC vector allocated + to MSI. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 64 + maximum: 95 + + sophgo,msi-num-vecs: + description: + u32 value of the number of parent PLIC vectors allocated + to MSI. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 32 + + msi-controller: true + +required: + - compatible + - reg + - reg-names + - msi-controller + - sophgo,msi-doorbell-addr + - sophgo,msi-base-vec + - sophgo,msi-num-vecs + +additionalProperties: true + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + msi: msi-controller@30000000 { + compatible = "sophgo,sg2042-msi"; + reg = <0x30000000 0x4>; + reg-names = "clr"; + msi-controller; + sophgo,msi-doorbell-addr = <0x00000070 0x30010300>; + sophgo,msi-base-vec = <64>; + sophgo,msi-num-vecs = <32>; + interrupt-parent = <&plic>; + };
Signature · 1 line
2.34.1
Rob Herring
maintainerRe: [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2042 MSI
Quoted 41 lines · Chen Wang
From: Chen Wang <[email protected]>
Add binding for Sophgo SG2042 MSI controller.
Signed-off-by Chen Wang <[email protected]>--- .../sophgo,sg2042-msi.yaml | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yamlDocumentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +25
-0@@ -0,0 +1,78 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/sophgo,sg2042-msi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sophgo SG2042 MSI Controller + +maintainers: + - Chen Wang <[email protected]> + +description: + This interrupt controller is in Sophgo SG2042 for transforming interrupts from + PCIe MSI to PLIC interrupts. + +allOf: + - $ref: /schemas/interrupts.yaml# + - $ref: /schemas/interrupt-controller/msi-controller.yaml# + +properties: + compatible: + const: sophgo,sg2042-msi + + reg: + items:
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml line 35
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +10
-0+ - description: clear register + + reg-names: + items: + - const: clr + + sophgo,msi-doorbell-addr: + description: + u64 value of the MSI doorbell address + $ref: /schemas/types.yaml#/definitions/uint64
Why not use a 'reg' entry?
In any case, this should be a translatable address (i.e. honor #address-cells/#size-cells and ranges).
Quoted 6 lines
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +6
-0+ + sophgo,msi-base-vec: + description: + u32 value of the base of parent PLIC vector allocated + to MSI. + $ref: /schemas/types.yaml#/definitions/uint32
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml line 51
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +10
-0+ minimum: 64 + maximum: 95 + + sophgo,msi-num-vecs: + description: + u32 value of the number of parent PLIC vectors allocated + to MSI. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 32
Use 'msi-ranges' for these.
Quoted 30 lines
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +27
-0+ + msi-controller: true + +required: + - compatible + - reg + - reg-names + - msi-controller + - sophgo,msi-doorbell-addr + - sophgo,msi-base-vec + - sophgo,msi-num-vecs + +additionalProperties: true + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + msi: msi-controller@30000000 { + compatible = "sophgo,sg2042-msi"; + reg = <0x30000000 0x4>; + reg-names = "clr"; + msi-controller; + sophgo,msi-doorbell-addr = <0x00000070 0x30010300>; + sophgo,msi-base-vec = <64>; + sophgo,msi-num-vecs = <32>; + interrupt-parent = <&plic>; + };Signature · 1 line
2.34.1
Re: [PATCH 1/3] dt-bindings: interrupt-controller: Add Sophgo SG2042 MSI
Quoted 54 lines · Rob Herring
Quoted 41 lines · Chen Wang
From: Chen Wang <[email protected]>
Add binding for Sophgo SG2042 MSI controller.
Signed-off-by Chen Wang <[email protected]>--- .../sophgo,sg2042-msi.yaml | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+)create mode 100644
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yamlDocumentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +25
-0@@ -0,0 +1,78 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/sophgo,sg2042-msi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sophgo SG2042 MSI Controller + +maintainers: + - Chen Wang <[email protected]> + +description: + This interrupt controller is in Sophgo SG2042 for transforming interrupts from + PCIe MSI to PLIC interrupts. + +allOf: + - $ref: /schemas/interrupts.yaml# + - $ref: /schemas/interrupt-controller/msi-controller.yaml# + +properties: + compatible: + const: sophgo,sg2042-msi + + reg: + items:Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml line 35
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +10
-0+ - description: clear register + + reg-names: + items: + - const: clr + + sophgo,msi-doorbell-addr: + description: + u64 value of the MSI doorbell address + $ref: /schemas/types.yaml#/definitions/uint64Why not use a 'reg' entry?
Quoted 2 lines
In any case, this should be a translatable address (i.e. honor #address-cells/#size-cells and ranges).
This is just a address to write MSI data, but not a register for driver access. So I think it seems better to configure it separately, rather than as other registers. This address does not need ioremap.
What do you think? I would like to hear your opinion.
Regards
Chen
Quoted 9 lines
Quoted 9 lines
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +9
-0+ + sophgo,msi-base-vec: + description: + u32 value of the base of parent PLIC vector allocated + to MSI. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 64 + maximum: 95 +
Quoted 8 lines
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml line 51
Quoted 7 lines
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +7
-0+ sophgo,msi-num-vecs: + description: + u32 value of the number of parent PLIC vectors allocated + to MSI. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 32Use 'msi-ranges' for these.
Got, will fix this and thanks.
Quoted 30 lines
Quoted 30 lines
Documentation/devicetree/bindings/interrupt-controller/sophgo,sg2042-msi.yaml new file +27
-0+ + msi-controller: true + +required: + - compatible + - reg + - reg-names + - msi-controller + - sophgo,msi-doorbell-addr + - sophgo,msi-base-vec + - sophgo,msi-num-vecs + +additionalProperties: true + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + msi: msi-controller@30000000 { + compatible = "sophgo,sg2042-msi"; + reg = <0x30000000 0x4>; + reg-names = "clr"; + msi-controller; + sophgo,msi-doorbell-addr = <0x00000070 0x30010300>; + sophgo,msi-base-vec = <64>; + sophgo,msi-num-vecs = <32>; + interrupt-parent = <&plic>; + };Signature · 1 line
2.34.1
PATCH 2/3 irqchip: Add the Sophgo SG2042 MSI interrupt controller
In reply to Chen WangFrom: Chen Wang <[email protected]>
Add driver for Sophgo SG2042 MSI interrupt controller.
--- drivers/irqchip/Kconfig | 8 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-sg2042-msi.c | 255 +++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+) create mode 100644 drivers/irqchip/irq-sg2042-msi.c
drivers/irqchip/Kconfig +8 -0
@@ -729,6 +729,14 @@ config MCHP_EIC help Support for Microchip External Interrupt Controller. +config SOPHGO_SG2042_MSI + bool "Sophgo SG2042 MSI controller" + depends on ARCH_SOPHGO || COMPILE_TEST + help + Support for the Sophgo SG2042 MSI Controller. + This on-chip interrupt controller enables MSI sources to be + routed to the primary PLIC controller on SoC. + config SUNPLUS_SP7021_INTC bool "Sunplus SP7021 interrupt controller" if COMPILE_TEST default SOC_SP7021
drivers/irqchip/Makefile +1 -0
@@ -125,4 +125,5 @@ obj-$(CONFIG_WPCM450_AIC) += irq-wpcm450-aic.o obj-$(CONFIG_IRQ_IDT3243X) += irq-idt3243x.o obj-$(CONFIG_APPLE_AIC) += irq-apple-aic.o obj-$(CONFIG_MCHP_EIC) += irq-mchp-eic.o +obj-$(CONFIG_SOPHGO_SG2042_MSI) += irq-sg2042-msi.o obj-$(CONFIG_SUNPLUS_SP7021_INTC) += irq-sp7021-intc.o
drivers/irqchip/irq-sg2042-msi.c new file +255 -0
255 lines not on this page · raw message
Signature · 1 line
2.34.1
Re: [PATCH 2/3] irqchip: Add the Sophgo SG2042 MSI interrupt controller
drivers/irqchip/irq-sg2042-msi.c line 23
Quoted 2 lines · Chen Wang
drivers/irqchip/irq-sg2042-msi.c new file +2
-0+struct sg2042_msi_data { +void __iomem *reg_clr;/* clear reg, see TRM, 10.1.33, GP_INTR0_CLR */
Please make these tail comments tabular aligned so they actually stand out.
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#comment-style
Quoted 5 lines
drivers/irqchip/irq-sg2042-msi.c new file +5
-0+ +u64 doorbell_addr;/* see TRM, 10.1.32, GP_INTR0_SET */ + +u32 irq_first;/* The vector number that MSIs starts */ +u32 num_irqs;/* The number of vectors for MSIs */
drivers/irqchip/irq-sg2042-msi.c line 38
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c new file +10
-0+ +unsigned long *msi_map; +struct mutex msi_map_lock;/* lock for msi_map */ +}; + +static int sg2042_msi_allocate_hwirq (struct sg2042_msi_data *priv,int num_req) +{ +int first; + +mutex_lock (&priv->msi_map_lock);
Please use
guard(mutex)(&priv->msi_map_lock);
which removes all the mutex_unlock() hackery and boils this down
drivers/irqchip/irq-sg2042-msi.c line 49
Quoted 11 lines
drivers/irqchip/irq-sg2042-msi.c new file +11
-0+ + first =bitmap_find_free_region (priv->msi_map, priv->num_irqs, +get_count_order (num_req)); +if (first <0 ) { +mutex_unlock (&priv->msi_map_lock); +return -ENOSPC ; + } + +mutex_unlock (&priv->msi_map_lock); + +return priv->irq_first + first;
to
guard(mutex)(&priv->msi_map_lock); first = bitmap_find_free_region(priv->msi_map, priv->num_irqs, get_count_order(num_req)); return first >= 0 ? priv->irq_first + first : -ENOSPC;
See?
drivers/irqchip/irq-sg2042-msi.c line 57
Quoted 8 lines
drivers/irqchip/irq-sg2042-msi.c new file +8
-0+} + +static void sg2042_msi_free_hwirq (struct sg2042_msi_data *priv, +int hwirq,int num_req) +{ +int first = hwirq - priv->irq_first; + +mutex_lock (&priv->msi_map_lock);
Ditto.
Quoted 5 lines
drivers/irqchip/irq-sg2042-msi.c new file +4
-0+bitmap_release_region (priv->msi_map, first,get_count_order (num_req)); +mutex_unlock (&priv->msi_map_lock); +} +static void sg2042_msi_irq_compose_msi_msg(struct irq_data *data,
drivers/irqchip/irq-sg2042-msi.c line 82
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c new file +10
-0+ struct msi_msg *msg) +{ +struct sg2042_msi_data *priv =irq_data_get_irq_chip_data (data); + + msg->address_hi =upper_32_bits (priv->doorbell_addr); + msg->address_lo =lower_32_bits (priv->doorbell_addr); + msg->data =1 << (data->hwirq - priv->irq_first); + + pr_debug("%s hwirq[%d]: address_hi[%#x], address_lo[%#x], data[%#x]\n", + __func__,
No point in having this line break. You have 100 characters. Please fix this all over the place.
drivers/irqchip/irq-sg2042-msi.c line 83
Quoted 1 line
drivers/irqchip/irq-sg2042-msi.c new file +1
-0+ (int)data->hwirq, msg->address_hi, msg->address_lo, msg->data);
(int) ? Why can't you use the proper conversion specifier instead of %d?
Quoted 8 lines
drivers/irqchip/irq-sg2042-msi.c new file +8
-0+static int sg2042_msi_middle_domain_alloc (struct irq_domain *domain, +unsigned int virq, +unsigned int nr_irqs,void *args) +{ +struct sg2042_msi_data *priv = domain->host_data; +int hwirq, err, i; + + hwirq =sg2042_msi_allocate_hwirq (priv, nr_irqs);
drivers/irqchip/irq-sg2042-msi.c line 134
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c new file +10
-0+if (hwirq <0 ) +return hwirq; + +for (i =0 ; i < nr_irqs; i++) { + err =sg2042_msi_parent_domain_alloc (domain, virq + i, hwirq + i); +if (err) + goto err_hwirq; + +pr_debug ("%s: virq[%d], hwirq[%d]\n" , + __func__, virq + i, (int )hwirq + i);
No line break required.
drivers/irqchip/irq-sg2042-msi.c line 170
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c new file +9
-0+irq_domain_set_hwirq_and_chip (domain, virq + i, hwirq + i, + &sg2042_msi_middle_irq_chip, priv); + } +static int sg2042_msi_init_domains (struct sg2042_msi_data *priv, +struct device_node *node) +{ +struct irq_domain *plic_domain, *middle_domain; +struct device_node *plic_node; +struct fwnode_handle *fwnode =of_node_to_fwnode (node);
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
Quoted 12 lines
drivers/irqchip/irq-sg2042-msi.c new file +12
-0+if (!of_find_property (node,"interrupt-parent" ,NULL )) { +pr_err ("Can't find interrupt-parent!\n" ); +return -EINVAL ; + } + + plic_node =of_irq_find_parent (node); +if (!plic_node) { +pr_err ("Failed to find the PLIC node!\n" ); +return -ENXIO ; + } + + plic_domain =irq_find_host (plic_node);
drivers/irqchip/irq-sg2042-msi.c line 193
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c new file +10
-0+of_node_put (plic_node); +if (!plic_domain) { +pr_err ("Failed to find the PLIC domain\n" ); +return -ENXIO ; + } + + middle_domain =irq_domain_create_hierarchy (plic_domain,0 , priv->num_irqs, + fwnode, + &pch_msi_middle_domain_ops, + priv);
So now you have created a domain. How is that supposed to be used by the PCI layer?
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c new file +9
-0+if (!middle_domain) { +pr_err ("Failed to create the MSI middle domain\n" ); +return -ENOMEM ; + } + +return 0 ; +} +static int sg2042_msi_probe (struct platform_device *pdev) +{
....
drivers/irqchip/irq-sg2042-msi.c line 240
Quoted 5 lines
drivers/irqchip/irq-sg2042-msi.c new file +5
-0+ data->msi_map =bitmap_zalloc (data->num_irqs,GFP_KERNEL ); +if (!data->msi_map) +return -ENOMEM ; + +return sg2042_msi_init_domains (data, pdev->dev.of_node);
In case of error this leaks data->msi_map, no?
drivers/irqchip/irq-sg2042-msi.c line 254
Quoted 7 lines
drivers/irqchip/irq-sg2042-msi.c new file +7
-0+static struct platform_driver sg2042_msi_driver = { + .driver = { + .name ="sg2042-msi" , + .of_match_table =of_match_ptr (sg2042_msi_of_match), + }, + .probe = sg2042_msi_probe, +};
Please see the documentation I pointed you to above and search for struct initializers.
Thanks,
tglx
Re: [PATCH 2/3] irqchip: Add the Sophgo SG2042 MSI interrupt controller
Quoted 7 lines · Thomas Gleixner
drivers/irqchip/irq-sg2042-msi.c line 23
Quoted 2 lines · Chen Wang
drivers/irqchip/irq-sg2042-msi.c new file +2
-0+struct sg2042_msi_data { +void __iomem *reg_clr;/* clear reg, see TRM, 10.1.33, GP_INTR0_CLR */ Please make these tail comments tabular aligned so they actually stand out.
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#comment-style
Got, will fix this.
Quoted 17 lines
Quoted 5 lines
drivers/irqchip/irq-sg2042-msi.c new file +5
-0+ +u64 doorbell_addr;/* see TRM, 10.1.32, GP_INTR0_SET */ + +u32 irq_first;/* The vector number that MSIs starts */ +u32 num_irqs;/* The number of vectors for MSIs */ drivers/irqchip/irq-sg2042-msi.c line 38
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c new file +10
-0+ +unsigned long *msi_map; +struct mutex msi_map_lock;/* lock for msi_map */ +}; + +static int sg2042_msi_allocate_hwirq (struct sg2042_msi_data *priv,int num_req) +{ +int first; + +mutex_lock (&priv->msi_map_lock);Please use
Quoted 3 lines
guard(mutex)(&priv->msi_map_lock);which removes all the
mutex_unlock()hackery and boils this down
Thanks, will double check.
Quoted 31 lines
drivers/irqchip/irq-sg2042-msi.c line 49
Quoted 12 lines
drivers/irqchip/irq-sg2042-msi.c new file +11
-0+ + first =bitmap_find_free_region (priv->msi_map, priv->num_irqs, +get_count_order (num_req)); +if (first <0 ) { +mutex_unlock (&priv->msi_map_lock); +return -ENOSPC ; + } + +mutex_unlock (&priv->msi_map_lock); + +return priv->irq_first + first;to
guard(mutex)(&priv->msi_map_lock); first = bitmap_find_free_region(priv->msi_map, priv->num_irqs, get_count_order(num_req)); return first >= 0 ? priv->irq_first + first : -ENOSPC;See?
drivers/irqchip/irq-sg2042-msi.c line 57
Quoted 8 lines
drivers/irqchip/irq-sg2042-msi.c new file +8
-0+} + +static void sg2042_msi_free_hwirq (struct sg2042_msi_data *priv, +int hwirq,int num_req) +{ +int first = hwirq - priv->irq_first; + +mutex_lock (&priv->msi_map_lock);Ditto.
Quoted 16 lines
Quoted 4 lines
drivers/irqchip/irq-sg2042-msi.c new file +4
-0+bitmap_release_region (priv->msi_map, first,get_count_order (num_req)); +mutex_unlock (&priv->msi_map_lock); +} +static void sg2042_msi_irq_compose_msi_msg(struct irq_data *data,drivers/irqchip/irq-sg2042-msi.c line 82
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c new file +10
-0+ struct msi_msg *msg) +{ +struct sg2042_msi_data *priv =irq_data_get_irq_chip_data (data); + + msg->address_hi =upper_32_bits (priv->doorbell_addr); + msg->address_lo =lower_32_bits (priv->doorbell_addr); + msg->data =1 << (data->hwirq - priv->irq_first); + + pr_debug("%s hwirq[%d]: address_hi[%#x], address_lo[%#x], data[%#x]\n", + __func__,No point in having this line break. You have 100 characters. Please fix this all over the place.
Got.
Quoted 3 lines
drivers/irqchip/irq-sg2042-msi.c line 83
Quoted 2 lines
drivers/irqchip/irq-sg2042-msi.c new file +1
-0+ (int)data->hwirq, msg->address_hi, msg->address_lo, msg->data);(int) ? Why can't you use the proper conversion specifier instead of %d?
Will double-check.
Quoted 21 lines
Quoted 9 lines
drivers/irqchip/irq-sg2042-msi.c new file +8
-0+static int sg2042_msi_middle_domain_alloc (struct irq_domain *domain, +unsigned int virq, +unsigned int nr_irqs,void *args) +{ +struct sg2042_msi_data *priv = domain->host_data; +int hwirq, err, i; + + hwirq =sg2042_msi_allocate_hwirq (priv, nr_irqs);drivers/irqchip/irq-sg2042-msi.c line 134
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c new file +10
-0+if (hwirq <0 ) +return hwirq; + +for (i =0 ; i < nr_irqs; i++) { + err =sg2042_msi_parent_domain_alloc (domain, virq + i, hwirq + i); +if (err) + goto err_hwirq; + +pr_debug ("%s: virq[%d], hwirq[%d]\n" , + __func__, virq + i, (int )hwirq + i);No line break required.
Quoted 10 lines
drivers/irqchip/irq-sg2042-msi.c line 170
Quoted 9 lines
drivers/irqchip/irq-sg2042-msi.c new file +9
-0+irq_domain_set_hwirq_and_chip (domain, virq + i, hwirq + i, + &sg2042_msi_middle_irq_chip, priv); + } +static int sg2042_msi_init_domains (struct sg2042_msi_data *priv, +struct device_node *node) +{ +struct irq_domain *plic_domain, *middle_domain; +struct device_node *plic_node; +struct fwnode_handle *fwnode =of_node_to_fwnode (node);https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
Thanks, will double-check.
Quoted 16 lines
Quoted 16 lines
drivers/irqchip/irq-sg2042-msi.c new file +16
-0+if (!of_find_property (node,"interrupt-parent" ,NULL )) { +pr_err ("Can't find interrupt-parent!\n" ); +return -EINVAL ; + } + + plic_node =of_irq_find_parent (node); +if (!plic_node) { +pr_err ("Failed to find the PLIC node!\n" ); +return -ENXIO ; + } + + plic_domain =irq_find_host (plic_node); +of_node_put (plic_node); +if (!plic_domain) { +pr_err ("Failed to find the PLIC domain\n" ); +return -ENXIO ;
Quoted 8 lines
drivers/irqchip/irq-sg2042-msi.c line 193
Quoted 6 lines
drivers/irqchip/irq-sg2042-msi.c new file +6
-0+ } + + middle_domain =irq_domain_create_hierarchy (plic_domain,0 , priv->num_irqs, + fwnode, + &pch_msi_middle_domain_ops, + priv);So now you have created a domain. How is that supposed to be used by the PCI layer?
Here I create the domain and attached it to the fwnode. In PCI driver, it can set this msi controller as its ""interrupt-parent" and find the domain attached as below:
static int pcie_probe(struct platform_device *pdev) {
struct device *dev = &pdev->dev; parent_node = of_irq_find_parent(dev->of_node); parent_domain = irq_find_host(parent_node);
...
}
Quoted 11 lines
Quoted 9 lines
drivers/irqchip/irq-sg2042-msi.c new file +9
-0+if (!middle_domain) { +pr_err ("Failed to create the MSI middle domain\n" ); +return -ENOMEM ; + } + +return 0 ; +} +static int sg2042_msi_probe (struct platform_device *pdev) +{....
Quoted 6 lines
drivers/irqchip/irq-sg2042-msi.c line 240
Quoted 5 lines
drivers/irqchip/irq-sg2042-msi.c new file +5
-0+ data->msi_map =bitmap_zalloc (data->num_irqs,GFP_KERNEL ); +if (!data->msi_map) +return -ENOMEM ; + +return sg2042_msi_init_domains (data, pdev->dev.of_node);In case of error this leaks
data->msi_map, no?
Thanks, I will correct this.
Quoted 13 lines
drivers/irqchip/irq-sg2042-msi.c line 254
Quoted 7 lines
drivers/irqchip/irq-sg2042-msi.c new file +7
-0+static struct platform_driver sg2042_msi_driver = { + .driver = { + .name ="sg2042-msi" , + .of_match_table =of_match_ptr (sg2042_msi_of_match), + }, + .probe = sg2042_msi_probe, +};Please see the documentation I pointed you to above and search for struct initializers.
Thanks,
tglx
Re: [PATCH 2/3] irqchip: Add the Sophgo SG2042 MSI interrupt controller
Quoted 13 lines · Chen Wang
Quoted 7 lines · Thomas Gleixner
drivers/irqchip/irq-sg2042-msi.c line 193
Quoted 5 lines
drivers/irqchip/irq-sg2042-msi.c new file +5
-0+ + middle_domain =irq_domain_create_hierarchy (plic_domain,0 , priv->num_irqs, + fwnode, + &pch_msi_middle_domain_ops, + priv);So now you have created a domain. How is that supposed to be used by the PCI layer?
Here I create the domain and attached it to the fwnode. In PCI driver, it can set this msi controller as its ""interrupt-parent" and find the domain attached as below:
Quoted 7 lines
static int
pcie_probe(struct platform_device *pdev){
struct device *dev =&pdev->dev;parent_node=of_irq_find_parent(dev->of_node);parent_domain=irq_find_host(parent_node);
...
}
I assume you then want to create a global PCI/MSI domain via pci_msi_create_irq_domain(), right?
That's not the preferred way to do that. Any new implementation should use the MSI parent model, where each PCI device creates it's own per device MSI domain with the MSI interrupt controller as parent domain.
There is a library with helper functions, irq-msi-lib.[ch]. See gicv2m_allocate_domains() or pch_msi_init_domains() for reference.
Thanks
tglx
Re: [PATCH 2/3] irqchip: Add the Sophgo SG2042 MSI interrupt controller
Quoted 20 lines · Thomas Gleixner
Quoted 12 lines · Chen Wang
Quoted 7 lines · Thomas Gleixner
drivers/irqchip/irq-sg2042-msi.c line 193
Quoted 5 lines
drivers/irqchip/irq-sg2042-msi.c new file +5
-0+ + middle_domain =irq_domain_create_hierarchy (plic_domain,0 , priv->num_irqs, + fwnode, + &pch_msi_middle_domain_ops, + priv);So now you have created a domain. How is that supposed to be used by the PCI layer?
Here I create the domain and attached it to the fwnode. In PCI driver, it can set this msi controller as its ""interrupt-parent" and find the domain attached as below:
Quoted 7 lines
static int
pcie_probe(struct platform_device *pdev){
struct device *dev =&pdev->dev;parent_node=of_irq_find_parent(dev->of_node);parent_domain=irq_find_host(parent_node);
...
}
Quoted 2 lines
I assume you then want to create a global PCI/MSI domain via
pci_msi_create_irq_domain(), right?
Yes, I am writing another pcie driver, which will call pci_msi_create_irq_domain() to create a child domain of this middle_domain.
Quoted 8 lines
That's not the preferred way to do that. Any new implementation should use the MSI parent model, where each PCI device creates it's own per device MSI domain with the MSI interrupt controller as parent domain.
There is a library with helper functions, irq-msi-lib.[ch]. See
gicv2m_allocate_domains()orpch_msi_init_domains()for reference.
Thanks, I will check this out.
Regards,
Chen
Quoted 4 lines
Thanks
tglx
PATCH 3/3 riscv: sophgo: dts: add msi controller for SG2042
In reply to Chen WangFrom: Chen Wang <[email protected]>
Add msi-controller node to dts for SG2042.
--- arch/riscv/boot/dts/sophgo/sg2042.dtsi | 13 +++++++++++++ 1 file changed, 13 insertions(+)
arch/riscv/boot/dts/sophgo/sg2042.dtsi +13 -0
@@ -173,6 +173,19 @@ pllclk: clock-controller@70300100c0 { #clock-cells = <1>; }; + msi: msi-controller@7030010304 { + compatible = "sophgo,sg2042-msi"; + reg = <0x70 0x30010304 0x0 0x4>; + reg-names = "clr"; + interrupt-controller; + #interrupt-cells = <1>; + msi-controller; + sophgo,msi-doorbell-addr = <0x00000070 0x30010300>; + sophgo,msi-base-vec = <64>; + sophgo,msi-num-vecs = <32>; + interrupt-parent = <&intc>; + }; + rpgate: clock-controller@7030010368 { compatible = "sophgo,sg2042-rpgate"; reg = <0x70 0x30010368 0x0 0x98>;
Signature · 1 line
2.34.1