linux-kernel - Re: [PATCH v3 3/4] irqchip/sg2042-msi: introduce configurable chipinfo for sg2042

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <MA0P287MB2262E07EF5871631C6576F9CFEB42@MA0P287MB2262.INDP287.PROD.OUTLOOK.COM>
Date: Wed, 9 Apr 2025 15:44:27 +0800
From: Chen Wang <unicorn_wang@...look.com>
To: Inochi Amaoto <inochiama@...il.com>, Thomas Gleixner
 <tglx@...utronix.de>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 sophgo@...ts.linux.dev, Yixun Lan <dlan@...too.org>,
 Longbin Li <looong.bin@...il.com>
Subject: Re: [PATCH v3 3/4] irqchip/sg2042-msi: introduce configurable
 chipinfo for sg2042


On 2025/4/8 13:01, Inochi Amaoto wrote:
> As the controller on SG2044 uses different msi_parent_ops and irq_chip,
> it is necessary to add a structure to hold the configuration across
> controllers.
>
> Add the chipinfo structure and implement necessary logic for it.
>
> Signed-off-by: Inochi Amaoto <inochiama@...il.com>
> ---
>   drivers/irqchip/irq-sg2042-msi.c | 44 ++++++++++++++++++++++++++------
>   1 file changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sg2042-msi.c b/drivers/irqchip/irq-sg2042-msi.c
> index c9bff7ba693d..30a1d2bfd474 100644
> --- a/drivers/irqchip/irq-sg2042-msi.c
> +++ b/drivers/irqchip/irq-sg2042-msi.c
> @@ -21,16 +21,33 @@
>   
>   #define SG2042_MAX_MSI_VECTOR	32
>   
> +struct sg204x_msi_chip_info {
> +	const struct irq_chip		*irqchip;
> +	const struct msi_parent_ops	*parent_ops;
> +};
> +
> +/**
> + * struct sg204x_msi_chipdata - chip data for the SG204x MSI IRQ controller
> + * @reg_clr:		clear reg, see TRM, 10.1.33, GP_INTR0_CLR

Does the TRM chapter information also apply to SG2044? If not, I suggest 
to indicate that this TRM information only applies to SG2042. Or simply 
delete these chapter information and only keep the explanation of member 
variables. If it is convenient, please help to add some comments at the 
beginning of the file to explain that the MSI register information for 
SG2042 can be found in Chapter 10 SYSTEM CONTROL of 
https://github.com/sophgo/sophgo-doc/tree/main/SG2042/TRM.

Thanks,

Chen

> + * @doorbell_addr:	see TRM, 10.1.32, GP_INTR0_SET
The same question as upon.
> + * @irq_first:		First vectors number that MSIs starts
> + * @num_irqs:		Number of vectors for MSIs
> + * @msi_map:		mapping for allocated MSI vectors.
> + * @msi_map_lock:	Lock for msi_map
> + * @chip_info:		chip specific infomations
> + */
>   struct sg204x_msi_chipdata {
> -	void __iomem	*reg_clr;	// clear reg, see TRM, 10.1.33, GP_INTR0_CLR
> +	void __iomem				*reg_clr;
>   
> -	phys_addr_t	doorbell_addr;	// see TRM, 10.1.32, GP_INTR0_SET
> +	phys_addr_t				doorbell_addr;
>   
> -	u32		irq_first;	// The vector number that MSIs starts
> -	u32		num_irqs;	// The number of vectors for MSIs
> +	u32					irq_first;
> +	u32					num_irqs;
>   
>   	DECLARE_BITMAP(msi_map, SG2042_MAX_MSI_VECTOR);
> -	struct mutex	msi_map_lock;	// lock for msi_map
> +	struct mutex				msi_map_lock;
> +
> +	const struct sg204x_msi_chip_info	*chip_info;
>   };

[......]

Otherwise，LGTM.

Reviewed-by: Chen Wang <wangchen20@...as.ac.cn>

>   
>   static int sg204x_msi_allocate_hwirq(struct sg204x_msi_chipdata *data, int num_req)
> @@ -115,7 +132,7 @@ static int sg204x_msi_middle_domain_alloc(struct irq_domain *domain, unsigned in
>   			goto err_hwirq;
>   
>   		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> -					      &sg2042_msi_middle_irq_chip, data);
> +					      data->chip_info->irqchip, data);
>   	}
>   
>   	return 0;
> @@ -174,7 +191,7 @@ static int sg204x_msi_init_domains(struct sg204x_msi_chipdata *data,
>   	irq_domain_update_bus_token(middle_domain, DOMAIN_BUS_NEXUS);
>   
>   	middle_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
> -	middle_domain->msi_parent_ops = &sg2042_msi_parent_ops;
> +	middle_domain->msi_parent_ops = data->chip_info->parent_ops;
>   
>   	return 0;
>   }
> @@ -192,6 +209,12 @@ static int sg2042_msi_probe(struct platform_device *pdev)
>   	if (!data)
>   		return -ENOMEM;
>   
> +	data->chip_info = device_get_match_data(&pdev->dev);
> +	if (!data->chip_info) {
> +		dev_err(&pdev->dev, "Failed to get irqchip\n");
> +		return -EINVAL;
> +	}
> +
>   	data->reg_clr = devm_platform_ioremap_resource_byname(pdev, "clr");
>   	if (IS_ERR(data->reg_clr)) {
>   		dev_err(dev, "Failed to map clear register\n");
> @@ -235,8 +258,13 @@ static int sg2042_msi_probe(struct platform_device *pdev)
>   	return sg204x_msi_init_domains(data, plic_domain, dev);
>   }
>   
> +static const struct sg204x_msi_chip_info sg2042_chip_info = {
> +	.irqchip	= &sg2042_msi_middle_irq_chip,
> +	.parent_ops	= &sg2042_msi_parent_ops,
> +};
> +
>   static const struct of_device_id sg2042_msi_of_match[] = {
> -	{ .compatible	= "sophgo,sg2042-msi" },
> +	{ .compatible	= "sophgo,sg2042-msi", .data	= &sg2042_chip_info },
>   	{ }
>   };
>