Re: [PATCH 2/2] PCI: mediatek-gen3: Configure PBUS_CSR registers for EN7581 SoC

Krzysztof Kozlowski
2025-01-18T15:59:29Z

On Wed, Jan 15, 2025 at 06:32:31PM +0100, Lorenzo Bianconi wrote:
> Configure PBus base address and address mask in order to allow the hw
> detecting if a given address is on PCIE0, PCIE1 or PCIE2.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> drivers/pci/controller/pcie-mediatek-gen3.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index aa24ac9aaecc749b53cfc4faf6399913d20cdbf2..b172a46cf95a9c728291c5b7a88457d3b725681a 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -15,6 +15,7 @@
> #include <linux/irqchip/chained_irq.h>
> #include <linux/irqdomain.h>
> #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/msi.h>
> #include <linux/of_device.h>
> @@ -24,6 +25,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> #include <linux/reset.h>
>
> #include "../pci.h"
> @@ -127,6 +129,13 @@
>
> #define PCIE_MTK_RESET_TIME_US 10
>
> +#define PCIE_EN7581_PBUS_ADDR(_n) (0x00 + ((_n) << 3))
> +#define PCIE_EN7581_PBUS_ADDR_MASK(_n) (0x04 + ((_n) << 3))
> +#define PCIE_EN7581_PBUS_BASE_ADDR(_n) \
> + ((_n) == 2 ? 0x28000000 : \
> + (_n) == 1 ? 0x24000000 : 0x20000000)
> +#define PCIE_EN7581_PBUS_BASE_ADDR_MASK GENMASK(31, 26)
> +
> /* Time in ms needed to complete PCIe reset on EN7581 SoC */
> #define PCIE_EN7581_RESET_TIME_MS 100
>
> @@ -931,7 +940,8 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie)
> static int mtk_pcie_en7581_power_up(struct mtk_gen3_pcie *pcie)
> {
> struct device *dev = pcie->dev;
> - int err;
> + struct regmap *map;
> + int err, slot;
> u32 val;
>
> /*
> @@ -945,6 +955,23 @@ static int mtk_pcie_en7581_power_up(struct mtk_gen3_pcie *pcie)
> /* Wait for the time needed to complete the reset lines assert. */
> msleep(PCIE_EN7581_RESET_TIME_MS);
>
> + map = syscon_regmap_lookup_by_compatible("airoha,en7581-pbus-csr");

No, don't sprinkle compatibles in other drivers. It does not scale, does
not allow reuse and you kind of try to escape ABI break, but you won't.
This is still clear ABI break without any statement in commit msg and
without explanation.

NAK.

Relationship between devices is expressed with phandles. There are
plenty of examples how to do that with syscon.

Best regards,
Krzysztof