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

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4173 bytes --]

> On Wed, Feb 19, 2025 at 11:56:50PM +0530, Manivannan Sadhasivam wrote:
> > On Sun, Feb 02, 2025 at 08:34:24PM +0100, Lorenzo Bianconi wrote:
> > > Configure PBus base address and address mask to allow the hw
> > > to detect if a given address is on PCIE0, PCIE1 or PCIE2.
> > >
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> >
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > - Mani
> >
> > > ---
> > >  drivers/pci/controller/pcie-mediatek-gen3.c | 30 ++++++++++++++++++++++++++++-
> > >  1 file changed, 29 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> > > index aa24ac9aaecc749b53cfc4faf6399913d20cdbf2..9c2a592cae959de8fbe9ca5c5c2253f8eadf2c76 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)
> 
> look like these data should be in dts ?
> 
> > > +#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,24 @@ 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_phandle(dev->of_node,
> > > +					      "mediatek,pbus-csr");
> > > +	if (IS_ERR(map))
> > > +		return PTR_ERR(map);
> > > +
> > > +	/*
> > > +	 * Configure PBus base address and address mask to allow the
> > > +	 * hw to detect if a given address is on PCIE0, PCIE1 or PCIE2.
> > > +	 */
> > > +	slot = of_get_pci_domain_nr(dev->of_node);
> 
> I am not sure if too much abuse domain_id here.
> 
> > > +	if (slot < 0)
> > > +		return slot;
> > > +
> > > +	regmap_write(map, PCIE_EN7581_PBUS_ADDR(slot),
> > > +		     PCIE_EN7581_PBUS_BASE_ADDR(slot));
> > > +	regmap_write(map, PCIE_EN7581_PBUS_ADDR_MASK(slot),
> > > +		     PCIE_EN7581_PBUS_BASE_ADDR_MASK);
> 
> look like
> 	syscon
> 	{
> 		csr1 : csr1 =
> 		{
> 			reg = <0x20000000, >; //or other property
> 		}
> 
> 		csr2: csr2 =
> 		{
> 			....
> 		}
> 	}
> 
> 	pcie1 {
> 		mediatek,pbus-csr = <&csr1>;
> 	}
> 
> 	pcie2 {
>                 mediatek,pbus-csr = <&csr2>;
>         }
> 
> 	...
> 
> Or
> 	pcie1 {
>                 mediatek,pbus-csr = <&csr1 0x20000000>;
>         }
> 	...
> 
> 	you can use syscon_regmap_lookup_by_phandle_args() to get address.
> Frank

ack, thx for the pointer. I will fix in v3.

Regards,
Lorenzo

> 
> 
> > > +
> > >  	/*
> > >  	 * Unlike the other MediaTek Gen3 controllers, the Airoha EN7581
> > >  	 * requires PHY initialization and power-on before PHY reset deassert.
> > >
> > > --
> > > 2.48.1
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]