| 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
| ||
|
Message-ID:
<SEYPR06MB5134623F42A5B204C20E14A99D79A@SEYPR06MB5134.apcprd06.prod.outlook.com>
Date: Mon, 23 Jun 2025 02:42:06 +0000
From: Jacky Chou <jacky_chou@...eedtech.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, "bhelgaas@...gle.com"
<bhelgaas@...gle.com>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
"kwilczynski@...nel.org" <kwilczynski@...nel.org>, "mani@...nel.org"
<mani@...nel.org>, "robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"joel@....id.au" <joel@....id.au>, "andrew@...econstruct.com.au"
<andrew@...econstruct.com.au>, "vkoul@...nel.org" <vkoul@...nel.org>,
"kishon@...nel.org" <kishon@...nel.org>, "linus.walleij@...aro.org"
<linus.walleij@...aro.org>, "p.zabel@...gutronix.de"
<p.zabel@...gutronix.de>, "linux-aspeed@...ts.ozlabs.org"
<linux-aspeed@...ts.ozlabs.org>, "linux-pci@...r.kernel.org"
<linux-pci@...r.kernel.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-phy@...ts.infradead.org"
<linux-phy@...ts.infradead.org>, "openbmc@...ts.ozlabs.org"
<openbmc@...ts.ozlabs.org>, "linux-gpio@...r.kernel.org"
<linux-gpio@...r.kernel.org>
CC: "elbadrym@...gle.com" <elbadrym@...gle.com>, "romlem@...gle.com"
<romlem@...gle.com>, "anhphan@...gle.com" <anhphan@...gle.com>,
"wak@...gle.com" <wak@...gle.com>, "yuxiaozhang@...gle.com"
<yuxiaozhang@...gle.com>, BMC-SW <BMC-SW@...eedtech.com>
Subject:
回覆: [PATCH 7/7] pci: aspeed: Add ASPEED PCIe host controller driver
> > This selects a driver for the MediaTek MT7621 PCIe Controller.
> >
> > +config PCIE_ASPEED
> > + bool "ASPEED PCIe controller"
> > + depends on PCI
>
> depends ARCH_ASPEED || COMPILE_TEST
>
Agreed.
> > + depends on OF || COMPILE_TEST
> > + select PCI_MSI_ARCH_FALLBACKS
> > + help
> > + Enable this option to add support for the PCIe controller
> > + found on ASPEED SoCs.
> > + This driver provides initialization and management for PCIe
> > + Root Complex functionality, including interrupt and MSI support.
> > + Select Y if your platform uses an ASPEED SoC and requires PCIe
> > + connectivity.
> > +
> > config PCI_HYPERV_INTERFACE
> > tristate "Microsoft Hyper-V PCI Interface"
> > depends on ((X86 && X86_64) || ARM64) && HYPERV && PCI_MSI diff
> > --git a/drivers/pci/controller/Makefile
> > b/drivers/pci/controller/Makefile index 038ccbd9e3ba..1339f88e153d
> > 100644
> > --- a/drivers/pci/controller/Makefile
> > +++ b/drivers/pci/controller/Makefile
> > @@ -39,6 +39,7 @@ obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
> > obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
> > obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o
> > obj-$(CONFIG_PCIE_MT7621) += pcie-mt7621.o
> > +obj-$(CONFIG_PCIE_ASPEED) += pcie-aspeed.o
> >
> > # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
> > obj-y += dwc/
> > diff --git a/drivers/pci/controller/pcie-aspeed.c
> > b/drivers/pci/controller/pcie-aspeed.c
> > new file mode 100644
> > index 000000000000..c745684a7f9b
> > --- /dev/null
> > +++ b/drivers/pci/controller/pcie-aspeed.c
> > @@ -0,0 +1,1039 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2025 Aspeed Technology Inc.
> > + */
> > +#include <linux/irqchip/chained_irq.h> #include <linux/irqdomain.h>
> > +#include <linux/mfd/syscon.h> #include <linux/kernel.h> #include
> > +<linux/msi.h> #include <linux/module.h> #include
> > +<linux/platform_device.h> #include <linux/of_platform.h>
>
> Where do you use it?
No, I will remove it in next version.
>
> > +#include <linux/of_address.h>
>
> Where do you use it?
>
No, I will remove it in next version.
>
> > +#include <linux/of_irq.h>
>
> Where do you use it?
>
No, I will remove it in next version.
>
> > +#include <linux/of_pci.h>
>
> Where do you use it?
>
No, I will remove it in next version.
> > +#include <linux/pci.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset.h>
> > +#include <linux/irq.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/workqueue.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +
>
>
>
> ...
>
> > +
> > +static int aspeed_pcie_probe(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + struct pci_host_bridge *host;
> > + struct aspeed_pcie *pcie;
> > + struct device_node *node = dev->of_node;
> > + const void *md = of_device_get_match_data(dev);
>
> Not void, but specific type. This is not Javascript, we have here types.
>
Agreed.
I will add this type in next version.
> > + int irq, ret;
> > +
> > + if (!md)
> > + return -ENODEV
...
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0)
> > + goto err_irq;
> > +
> > + ret = devm_request_irq(dev, irq, aspeed_pcie_intr_handler, IRQF_SHARED,
> dev_name(dev),
> > + pcie);
> > + if (ret)
> > + goto err_irq;
> > +
> > + pcie->clock = clk_get(dev, NULL);
>
> Huh...
>
> > + if (IS_ERR(pcie->clock))
> > + goto err_clk;
> > + ret = clk_prepare_enable(pcie->clock);
>
> devm_clk_get_enabled.
>
I will change it in next version.
> > + if (ret)
> > + goto err_clk_enable;
> > +
> > + ret = pci_host_probe(host);
> > + if (ret)
> > + goto err_clk_enable;
> > +
> > + return 0;
> > +
> > +err_clk_enable:
> > + clk_put(pcie->clock);
> > +err_clk:
> > +err_irq:
> > + aspeed_pcie_irq_domain_free(pcie);
> > +err_irq_init:
> > +err_setup:
> > + return dev_err_probe(dev, ret, "Failed to setup PCIe RC\n"); }
> > +
> > +static void aspeed_pcie_remove(struct platform_device *pdev) {
> > + struct aspeed_pcie *pcie = platform_get_drvdata(pdev);
> > +
> > + if (pcie->clock) {
> > + clk_disable_unprepare(pcie->clock);
> > + clk_put(pcie->clock);
> > + }
> > +
> > + pci_stop_root_bus(pcie->host->bus);
> > + pci_remove_root_bus(pcie->host->bus);
> > + aspeed_pcie_irq_domain_free(pcie);
> > +}
> > +
> > +static struct aspeed_pcie_rc_platform pcie_rc_ast2600 = {
>
> This should be const. Why it cannot?
>
Agreed.
> > + .setup = aspeed_ast2600_setup,
> > + .reg_intx_en = 0x04,
> > + .reg_intx_sts = 0x08,
> > + .reg_msi_en = 0x20,
> > + .reg_msi_sts = 0x28,
> > +};
> > +
> > +static struct aspeed_pcie_rc_platform pcie_rc_ast2700 = {
>
> This should be const. Why it cannot?
>
Agreed.
> > + .setup = aspeed_ast2700_setup,
> > + .reg_intx_en = 0x40,
> > + .reg_intx_sts = 0x48,
> > + .reg_msi_en = 0x50,
> > + .reg_msi_sts = 0x58,
> > +};
> > +
> > +static const struct of_device_id aspeed_pcie_of_match[] = {
> > + { .compatible = "aspeed,ast2600-pcie", .data = &pcie_rc_ast2600 },
> > + { .compatible = "aspeed,ast2700-pcie", .data = &pcie_rc_ast2700 },
> > + {}
> > +};
> > +
> > +static struct platform_driver aspeed_pcie_driver = {
> > + .driver = {
> > + .name = "aspeed-pcie",
> > + .suppress_bind_attrs = true,
>
> Why?
>
I will remove it in next version.
> > + .of_match_table = aspeed_pcie_of_match,
> > + },
> > + .probe = aspeed_pcie_probe,
> > + .remove = aspeed_pcie_remove,
>
> So how exactly remove can be triggered?
>
Agreed.
I think it may be triggered when rebooting.
I will remove it in next version.
Thanks,
Jacky
Powered by blists - more mailing lists