Patchwork [2/2] PCI: mediatek: Add controller support for MT7629

login
register
mail settings
Submitter Jianjun Wang
Date Dec. 6, 2018, 1:09 a.m.
Message ID <1544058553-10936-3-git-send-email-jianjun.wang@mediatek.com>
Download mbox | patch
Permalink /patch/673711/
State New
Headers show

Comments

Jianjun Wang - Dec. 6, 2018, 1:09 a.m.
MT7629 is an arm platform SoC which has the same PCIe IP with MT7622.

The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
in arm64 but bogus alignment values at arm32, the pcie device and devices
behind this bridge will not be enabled. Fix it's BAR0 resource size to
guarantee the pcie devices will be enabled correctly.

The HW default value of its device id is invalid, fix it's device id to
match the hardware implementation.

Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 26 ++++++++++++++++++++++++++
 include/linux/pci_ids.h                |  1 +
 2 files changed, 27 insertions(+)
honghui.zhang@mediatek.com - Dec. 6, 2018, 5:53 a.m.
On Thu, 2018-12-06 at 09:09 +0800, Jianjun Wang wrote:
> MT7629 is an arm platform SoC which has the same PCIe IP with MT7622.
> 
> The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> in arm64 but bogus alignment values at arm32, the pcie device and devices

:s /the pcie device /the bridge device

> behind this bridge will not be enabled. Fix it's BAR0 resource size to
> guarantee the pcie devices will be enabled correctly.
> 
> The HW default value of its device id is invalid, fix it's device id to
> match the hardware implementation.
> 
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 26 ++++++++++++++++++++++++++
>  include/linux/pci_ids.h                |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index d20cf461ba00..f8937cc3c87c 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -73,6 +73,7 @@
>  #define PCIE_MSI_VECTOR		0x0c0
>  
>  #define PCIE_CONF_VEND_ID	0x100
> +#define PCIE_CONF_DEVICE_ID	0x102
>  #define PCIE_CONF_CLASS_ID	0x106
>  
>  #define PCIE_INT_MASK		0x420
> @@ -135,12 +136,14 @@ struct mtk_pcie_port;
>  /**
>   * struct mtk_pcie_soc - differentiate between host generations
>   * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> + * @need_fix_device_id: whether this host's device ID needed to be fixed or not
>   * @ops: pointer to configuration access functions
>   * @startup: pointer to controller setting functions
>   * @setup_irq: pointer to initialize IRQ functions
>   */
>  struct mtk_pcie_soc {
>  	bool need_fix_class_id;
> +	bool need_fix_device_id;
>  	struct pci_ops *ops;
>  	int (*startup)(struct mtk_pcie_port *port);
>  	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> @@ -692,6 +695,11 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		writew(val, port->base + PCIE_CONF_CLASS_ID);
>  	}
>  
> +	if (soc->need_fix_device_id) {
> +		val = PCI_DEVICE_ID_MEDIATEK_7629;
> +		writew(val, port->base + PCIE_CONF_DEVICE_ID);
> +	}
> +
>  	/* 100ms timeout value should be enough for Gen1/2 training */
>  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
>  				 !!(val & PCIE_PORT_LINKUP_V2), 20,
> @@ -1238,11 +1246,29 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
>  	.setup_irq = mtk_pcie_setup_irq,
>  };
>  
> +static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
> +	.need_fix_class_id = true,
> +	.need_fix_device_id = true,
> +	.ops = &mtk_pcie_ops_v2,
> +	.startup = mtk_pcie_startup_port_v2,
> +	.setup_irq = mtk_pcie_setup_irq,
> +};
> +
> +static void mtk_fixup_bar_size(struct pci_dev *dev)
> +{
> +	struct resource *dev_res = &dev->resource[0];
> +	/* 32bit resource length will calculate size to 0, set it smaller */
> +	dev_res->end = 0xfffffffe;
> +}

I'm not sure assign the BAR0 size in driver to fit in the bogus
alignment is a good idea. Seems the size value of 0xffff_fffe also is an
arbitrary value.
Do we have a chance to change the resource framework code to make it
adopt this scenario?

Thanks.

> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MEDIATEK, PCI_DEVICE_ID_MEDIATEK_7629,
> +			 mtk_fixup_bar_size);
> +
>  static const struct of_device_id mtk_pcie_ids[] = {
>  	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
>  	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
>  	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
>  	{ .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
> +	{ .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
>  	{},
>  };
>  
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 69f0abe1ba1a..77b278bac3a8 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2126,6 +2126,7 @@
>  #define PCI_VENDOR_ID_MYRICOM		0x14c1
>  
>  #define PCI_VENDOR_ID_MEDIATEK		0x14c3
> +#define PCI_DEVICE_ID_MEDIATEK_7629	0x7629
>  
>  #define PCI_VENDOR_ID_TITAN		0x14D2
>  #define PCI_DEVICE_ID_TITAN_010L	0x8001
Jianjun Wang - Dec. 7, 2018, 12:56 p.m.
On Thu, 2018-12-06 at 13:53 +0800, Honghui Zhang wrote:
> On Thu, 2018-12-06 at 09:09 +0800, Jianjun Wang wrote:
> > MT7629 is an arm platform SoC which has the same PCIe IP with MT7622.
> > 
> > The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> > in arm64 but bogus alignment values at arm32, the pcie device and devices
> 
> :s /the pcie device /the bridge device
> 
> > behind this bridge will not be enabled. Fix it's BAR0 resource size to
> > guarantee the pcie devices will be enabled correctly.
> > 
> > The HW default value of its device id is invalid, fix it's device id to
> > match the hardware implementation.
> > 
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 26 ++++++++++++++++++++++++++
> >  include/linux/pci_ids.h                |  1 +
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index d20cf461ba00..f8937cc3c87c 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -73,6 +73,7 @@
> >  #define PCIE_MSI_VECTOR		0x0c0
> >  
> >  #define PCIE_CONF_VEND_ID	0x100
> > +#define PCIE_CONF_DEVICE_ID	0x102
> >  #define PCIE_CONF_CLASS_ID	0x106
> >  
> >  #define PCIE_INT_MASK		0x420
> > @@ -135,12 +136,14 @@ struct mtk_pcie_port;
> >  /**
> >   * struct mtk_pcie_soc - differentiate between host generations
> >   * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> > + * @need_fix_device_id: whether this host's device ID needed to be fixed or not
> >   * @ops: pointer to configuration access functions
> >   * @startup: pointer to controller setting functions
> >   * @setup_irq: pointer to initialize IRQ functions
> >   */
> >  struct mtk_pcie_soc {
> >  	bool need_fix_class_id;
> > +	bool need_fix_device_id;
> >  	struct pci_ops *ops;
> >  	int (*startup)(struct mtk_pcie_port *port);
> >  	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> > @@ -692,6 +695,11 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> >  		writew(val, port->base + PCIE_CONF_CLASS_ID);
> >  	}
> >  
> > +	if (soc->need_fix_device_id) {
> > +		val = PCI_DEVICE_ID_MEDIATEK_7629;
> > +		writew(val, port->base + PCIE_CONF_DEVICE_ID);
> > +	}
> > +
> >  	/* 100ms timeout value should be enough for Gen1/2 training */
> >  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
> >  				 !!(val & PCIE_PORT_LINKUP_V2), 20,
> > @@ -1238,11 +1246,29 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
> >  	.setup_irq = mtk_pcie_setup_irq,
> >  };
> >  
> > +static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
> > +	.need_fix_class_id = true,
> > +	.need_fix_device_id = true,
> > +	.ops = &mtk_pcie_ops_v2,
> > +	.startup = mtk_pcie_startup_port_v2,
> > +	.setup_irq = mtk_pcie_setup_irq,
> > +};
> > +
> > +static void mtk_fixup_bar_size(struct pci_dev *dev)
> > +{
> > +	struct resource *dev_res = &dev->resource[0];
> > +	/* 32bit resource length will calculate size to 0, set it smaller */
> > +	dev_res->end = 0xfffffffe;
> > +}
> 
> I'm not sure assign the BAR0 size in driver to fit in the bogus
> alignment is a good idea. Seems the size value of 0xffff_fffe also is an
> arbitrary value.
> Do we have a chance to change the resource framework code to make it
> adopt this scenario?
> 
> Thanks.

I'm afraid not, the resource size length defined by phys_addr_t, which
related to the hardware's physical address length. 
It will be much more better if the BAR0 size is not related with the
pcie to axi window, when we set the window to 4GB, the BAR0 size still
have initial value, and we can set the BAR0 size value or just disable
it independently.
The BAR0 size value need bigger than the MMIO space size, so the
software will think it's a invalid resource but not a bogus alignment
one, since we can't disable the BAR0 by hardware, and the flow of enable
devices will keep going. 
> 
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MEDIATEK, PCI_DEVICE_ID_MEDIATEK_7629,
> > +			 mtk_fixup_bar_size);
> > +
> >  static const struct of_device_id mtk_pcie_ids[] = {
> >  	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
> >  	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
> >  	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
> >  	{ .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
> > +	{ .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
> >  	{},
> >  };
> >  
> > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > index 69f0abe1ba1a..77b278bac3a8 100644
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -2126,6 +2126,7 @@
> >  #define PCI_VENDOR_ID_MYRICOM		0x14c1
> >  
> >  #define PCI_VENDOR_ID_MEDIATEK		0x14c3
> > +#define PCI_DEVICE_ID_MEDIATEK_7629	0x7629
> >  
> >  #define PCI_VENDOR_ID_TITAN		0x14D2
> >  #define PCI_DEVICE_ID_TITAN_010L	0x8001
> 
>
honghui.zhang@mediatek.com - Dec. 12, 2018, 5:43 a.m.
On Fri, 2018-12-07 at 20:56 +0800, Jianjun Wang wrote:
> On Thu, 2018-12-06 at 13:53 +0800, Honghui Zhang wrote:
> > On Thu, 2018-12-06 at 09:09 +0800, Jianjun Wang wrote:
> > > MT7629 is an arm platform SoC which has the same PCIe IP with MT7622.
> > > 
> > > The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> > > in arm64 but bogus alignment values at arm32, the pcie device and devices
> > 
> > :s /the pcie device /the bridge device
> > 
> > > behind this bridge will not be enabled. Fix it's BAR0 resource size to
> > > guarantee the pcie devices will be enabled correctly.
> > > 
> > > The HW default value of its device id is invalid, fix it's device id to
> > > match the hardware implementation.
> > > 
> > > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > > ---
> > >  drivers/pci/controller/pcie-mediatek.c | 26 ++++++++++++++++++++++++++
> > >  include/linux/pci_ids.h                |  1 +
> > >  2 files changed, 27 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > > index d20cf461ba00..f8937cc3c87c 100644
> > > --- a/drivers/pci/controller/pcie-mediatek.c
> > > +++ b/drivers/pci/controller/pcie-mediatek.c
> > > @@ -73,6 +73,7 @@
> > >  #define PCIE_MSI_VECTOR		0x0c0
> > >  
> > >  #define PCIE_CONF_VEND_ID	0x100
> > > +#define PCIE_CONF_DEVICE_ID	0x102
> > >  #define PCIE_CONF_CLASS_ID	0x106
> > >  
> > >  #define PCIE_INT_MASK		0x420
> > > @@ -135,12 +136,14 @@ struct mtk_pcie_port;
> > >  /**
> > >   * struct mtk_pcie_soc - differentiate between host generations
> > >   * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> > > + * @need_fix_device_id: whether this host's device ID needed to be fixed or not
> > >   * @ops: pointer to configuration access functions
> > >   * @startup: pointer to controller setting functions
> > >   * @setup_irq: pointer to initialize IRQ functions
> > >   */
> > >  struct mtk_pcie_soc {
> > >  	bool need_fix_class_id;
> > > +	bool need_fix_device_id;
> > >  	struct pci_ops *ops;
> > >  	int (*startup)(struct mtk_pcie_port *port);
> > >  	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> > > @@ -692,6 +695,11 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> > >  		writew(val, port->base + PCIE_CONF_CLASS_ID);
> > >  	}
> > >  
> > > +	if (soc->need_fix_device_id) {
> > > +		val = PCI_DEVICE_ID_MEDIATEK_7629;
> > > +		writew(val, port->base + PCIE_CONF_DEVICE_ID);
> > > +	}
> > > +
> > >  	/* 100ms timeout value should be enough for Gen1/2 training */
> > >  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
> > >  				 !!(val & PCIE_PORT_LINKUP_V2), 20,
> > > @@ -1238,11 +1246,29 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
> > >  	.setup_irq = mtk_pcie_setup_irq,
> > >  };
> > >  
> > > +static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
> > > +	.need_fix_class_id = true,
> > > +	.need_fix_device_id = true,
> > > +	.ops = &mtk_pcie_ops_v2,
> > > +	.startup = mtk_pcie_startup_port_v2,
> > > +	.setup_irq = mtk_pcie_setup_irq,
> > > +};
> > > +
> > > +static void mtk_fixup_bar_size(struct pci_dev *dev)
> > > +{
> > > +	struct resource *dev_res = &dev->resource[0];
> > > +	/* 32bit resource length will calculate size to 0, set it smaller */
> > > +	dev_res->end = 0xfffffffe;
> > > +}
> > 
> > I'm not sure assign the BAR0 size in driver to fit in the bogus
> > alignment is a good idea. Seems the size value of 0xffff_fffe also is an
> > arbitrary value.
> > Do we have a chance to change the resource framework code to make it
> > adopt this scenario?

I have take a look at the resource assign routine, it's better keep it
in current way.

Thanks.

> > 
> > Thanks.
> 
> I'm afraid not, the resource size length defined by phys_addr_t, which
> related to the hardware's physical address length. 
> It will be much more better if the BAR0 size is not related with the
> pcie to axi window, when we set the window to 4GB, the BAR0 size still
> have initial value, and we can set the BAR0 size value or just disable
> it independently.
> The BAR0 size value need bigger than the MMIO space size, so the
> software will think it's a invalid resource but not a bogus alignment
> one, since we can't disable the BAR0 by hardware, and the flow of enable
> devices will keep going. 
> > 
> > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MEDIATEK, PCI_DEVICE_ID_MEDIATEK_7629,
> > > +			 mtk_fixup_bar_size);
> > > +
> > >  static const struct of_device_id mtk_pcie_ids[] = {
> > >  	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
> > >  	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
> > >  	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
> > >  	{ .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
> > > +	{ .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
> > >  	{},
> > >  };
> > >  
> > > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > > index 69f0abe1ba1a..77b278bac3a8 100644
> > > --- a/include/linux/pci_ids.h
> > > +++ b/include/linux/pci_ids.h
> > > @@ -2126,6 +2126,7 @@
> > >  #define PCI_VENDOR_ID_MYRICOM		0x14c1
> > >  
> > >  #define PCI_VENDOR_ID_MEDIATEK		0x14c3
> > > +#define PCI_DEVICE_ID_MEDIATEK_7629	0x7629
> > >  
> > >  #define PCI_VENDOR_ID_TITAN		0x14D2
> > >  #define PCI_DEVICE_ID_TITAN_010L	0x8001
> > 
> > 
> 
>
Ryder Lee - Dec. 13, 2018, 3:39 a.m.
Hi,

On Thu, 2018-12-06 at 09:09 +0800, Jianjun Wang wrote:
> MT7629 is an arm platform SoC which has the same PCIe IP with MT7622.
> 
> The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> in arm64 but bogus alignment values at arm32, the pcie device and devices
> behind this bridge will not be enabled. Fix it's BAR0 resource size to
> guarantee the pcie devices will be enabled correctly.
> 
> The HW default value of its device id is invalid, fix it's device id to
> match the hardware implementation.
> 
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 26 ++++++++++++++++++++++++++
>  include/linux/pci_ids.h                |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index d20cf461ba00..f8937cc3c87c 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -73,6 +73,7 @@
>  #define PCIE_MSI_VECTOR		0x0c0
>  
>  #define PCIE_CONF_VEND_ID	0x100
> +#define PCIE_CONF_DEVICE_ID	0x102
>  #define PCIE_CONF_CLASS_ID	0x106
>  
>  #define PCIE_INT_MASK		0x420
> @@ -135,12 +136,14 @@ struct mtk_pcie_port;
>  /**
>   * struct mtk_pcie_soc - differentiate between host generations
>   * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> + * @need_fix_device_id: whether this host's device ID needed to be fixed or not
>   * @ops: pointer to configuration access functions
>   * @startup: pointer to controller setting functions
>   * @setup_irq: pointer to initialize IRQ functions
>   */
>  struct mtk_pcie_soc {
>  	bool need_fix_class_id;
> +	bool need_fix_device_id;
>  	struct pci_ops *ops;
>  	int (*startup)(struct mtk_pcie_port *port);
>  	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> @@ -692,6 +695,11 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		writew(val, port->base + PCIE_CONF_CLASS_ID);
>  	}
>  
> +	if (soc->need_fix_device_id) {
> +		val = PCI_DEVICE_ID_MEDIATEK_7629;

<just_checking>
  Could we have a generic way to fix it? It's better not to add SoC
specific settings in shared function.
</just_checking>

> +		writew(val, port->base + PCIE_CONF_DEVICE_ID);
> +	}
> +
>  	/* 100ms timeout value should be enough for Gen1/2 training */
>  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
>  				 !!(val & PCIE_PORT_LINKUP_V2), 20,
> @@ -1238,11 +1246,29 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
>  	.setup_irq = mtk_pcie_setup_irq,
>  };
>  
> +static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
> +	.need_fix_class_id = true,
> +	.need_fix_device_id = true,
> +	.ops = &mtk_pcie_ops_v2,
> +	.startup = mtk_pcie_startup_port_v2,
> +	.setup_irq = mtk_pcie_setup_irq,
> +};
> +
> +static void mtk_fixup_bar_size(struct pci_dev *dev)
> +{
> +	struct resource *dev_res = &dev->resource[0];
> +	/* 32bit resource length will calculate size to 0, set it smaller */
> +	dev_res->end = 0xfffffffe;
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MEDIATEK, PCI_DEVICE_ID_MEDIATEK_7629,
> +			 mtk_fixup_bar_size);
> +
>  static const struct of_device_id mtk_pcie_ids[] = {
>  	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
>  	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
>  	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
>  	{ .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
> +	{ .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
>  	{},
>  };
>  
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 69f0abe1ba1a..77b278bac3a8 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2126,6 +2126,7 @@
>  #define PCI_VENDOR_ID_MYRICOM		0x14c1
>  
>  #define PCI_VENDOR_ID_MEDIATEK		0x14c3
> +#define PCI_DEVICE_ID_MEDIATEK_7629	0x7629
>  
>  #define PCI_VENDOR_ID_TITAN		0x14D2
>  #define PCI_DEVICE_ID_TITAN_010L	0x8001
Bjorn Helgaas - Dec. 13, 2018, 2:55 p.m.
On Thu, Dec 06, 2018 at 09:09:13AM +0800, Jianjun Wang wrote:
> MT7629 is an arm platform SoC which has the same PCIe IP with MT7622.

s/arm/ARM/

> The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> in arm64 but bogus alignment values at arm32, the pcie device and devices
> behind this bridge will not be enabled. Fix it's BAR0 resource size to
> guarantee the pcie devices will be enabled correctly.

So this is a hardware erratum?  Per spec, a memory BAR has bit 0 hardwired
to 0, and an IO BAR has bit 1 hardwired to 0.

> The HW default value of its device id is invalid, fix it's device id to
> match the hardware implementation.

s/pcie/PCIe/ (all places above)
s/it's/its/ (all places above)
s/device id/Device ID/

> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 26 ++++++++++++++++++++++++++
>  include/linux/pci_ids.h                |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index d20cf461ba00..f8937cc3c87c 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -73,6 +73,7 @@
>  #define PCIE_MSI_VECTOR		0x0c0
>  
>  #define PCIE_CONF_VEND_ID	0x100
> +#define PCIE_CONF_DEVICE_ID	0x102
>  #define PCIE_CONF_CLASS_ID	0x106
>  
>  #define PCIE_INT_MASK		0x420
> @@ -135,12 +136,14 @@ struct mtk_pcie_port;
>  /**
>   * struct mtk_pcie_soc - differentiate between host generations
>   * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> + * @need_fix_device_id: whether this host's device ID needed to be fixed or not
>   * @ops: pointer to configuration access functions
>   * @startup: pointer to controller setting functions
>   * @setup_irq: pointer to initialize IRQ functions
>   */
>  struct mtk_pcie_soc {
>  	bool need_fix_class_id;
> +	bool need_fix_device_id;
>  	struct pci_ops *ops;
>  	int (*startup)(struct mtk_pcie_port *port);
>  	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> @@ -692,6 +695,11 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		writew(val, port->base + PCIE_CONF_CLASS_ID);
>  	}
>  
> +	if (soc->need_fix_device_id) {
> +		val = PCI_DEVICE_ID_MEDIATEK_7629;
> +		writew(val, port->base + PCIE_CONF_DEVICE_ID);
> +	}
> +
>  	/* 100ms timeout value should be enough for Gen1/2 training */
>  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
>  				 !!(val & PCIE_PORT_LINKUP_V2), 20,
> @@ -1238,11 +1246,29 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
>  	.setup_irq = mtk_pcie_setup_irq,
>  };
>  
> +static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
> +	.need_fix_class_id = true,
> +	.need_fix_device_id = true,
> +	.ops = &mtk_pcie_ops_v2,
> +	.startup = mtk_pcie_startup_port_v2,
> +	.setup_irq = mtk_pcie_setup_irq,
> +};
> +
> +static void mtk_fixup_bar_size(struct pci_dev *dev)
> +{
> +	struct resource *dev_res = &dev->resource[0];

Add a blank line here.

> +	/* 32bit resource length will calculate size to 0, set it smaller */
> +	dev_res->end = 0xfffffffe;

Presumably you know the size of the BAR because that's fixed by the
hardware and documented in the spec.  You should do something like
this so you don't depend on what the BAR contains, e.g.,

  res->end = res->start + size - 1;

> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MEDIATEK, PCI_DEVICE_ID_MEDIATEK_7629,
> +			 mtk_fixup_bar_size);
> +
>  static const struct of_device_id mtk_pcie_ids[] = {
>  	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
>  	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
>  	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
>  	{ .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
> +	{ .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
>  	{},
>  };
>  
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 69f0abe1ba1a..77b278bac3a8 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2126,6 +2126,7 @@
>  #define PCI_VENDOR_ID_MYRICOM		0x14c1
>  
>  #define PCI_VENDOR_ID_MEDIATEK		0x14c3
> +#define PCI_DEVICE_ID_MEDIATEK_7629	0x7629
>  
>  #define PCI_VENDOR_ID_TITAN		0x14D2
>  #define PCI_DEVICE_ID_TITAN_010L	0x8001
> -- 
> 2.19.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Jianjun Wang - Dec. 17, 2018, 7:51 a.m.
On Thu, 2018-12-13 at 11:39 +0800, Ryder Lee wrote:
> Hi,
> 
> On Thu, 2018-12-06 at 09:09 +0800, Jianjun Wang wrote:
> > MT7629 is an arm platform SoC which has the same PCIe IP with MT7622.
> > 
> > The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> > in arm64 but bogus alignment values at arm32, the pcie device and devices
> > behind this bridge will not be enabled. Fix it's BAR0 resource size to
> > guarantee the pcie devices will be enabled correctly.
> > 
> > The HW default value of its device id is invalid, fix it's device id to
> > match the hardware implementation.
> > 
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 26 ++++++++++++++++++++++++++
> >  include/linux/pci_ids.h                |  1 +
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index d20cf461ba00..f8937cc3c87c 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -73,6 +73,7 @@
> >  #define PCIE_MSI_VECTOR		0x0c0
> >  
> >  #define PCIE_CONF_VEND_ID	0x100
> > +#define PCIE_CONF_DEVICE_ID	0x102
> >  #define PCIE_CONF_CLASS_ID	0x106
> >  
> >  #define PCIE_INT_MASK		0x420
> > @@ -135,12 +136,14 @@ struct mtk_pcie_port;
> >  /**
> >   * struct mtk_pcie_soc - differentiate between host generations
> >   * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> > + * @need_fix_device_id: whether this host's device ID needed to be fixed or not
> >   * @ops: pointer to configuration access functions
> >   * @startup: pointer to controller setting functions
> >   * @setup_irq: pointer to initialize IRQ functions
> >   */
> >  struct mtk_pcie_soc {
> >  	bool need_fix_class_id;
> > +	bool need_fix_device_id;
> >  	struct pci_ops *ops;
> >  	int (*startup)(struct mtk_pcie_port *port);
> >  	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> > @@ -692,6 +695,11 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> >  		writew(val, port->base + PCIE_CONF_CLASS_ID);
> >  	}
> >  
> > +	if (soc->need_fix_device_id) {
> > +		val = PCI_DEVICE_ID_MEDIATEK_7629;
> 
> <just_checking>
>   Could we have a generic way to fix it? It's better not to add SoC
> specific settings in shared function.
> </just_checking>
> 
Yes, I will modify the code in next patch, thanks.

> > +		writew(val, port->base + PCIE_CONF_DEVICE_ID);
> > +	}
> > +
> >  	/* 100ms timeout value should be enough for Gen1/2 training */
> >  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
> >  				 !!(val & PCIE_PORT_LINKUP_V2), 20,
> > @@ -1238,11 +1246,29 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
> >  	.setup_irq = mtk_pcie_setup_irq,
> >  };
> >  
> > +static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
> > +	.need_fix_class_id = true,
> > +	.need_fix_device_id = true,
> > +	.ops = &mtk_pcie_ops_v2,
> > +	.startup = mtk_pcie_startup_port_v2,
> > +	.setup_irq = mtk_pcie_setup_irq,
> > +};
> > +
> > +static void mtk_fixup_bar_size(struct pci_dev *dev)
> > +{
> > +	struct resource *dev_res = &dev->resource[0];
> > +	/* 32bit resource length will calculate size to 0, set it smaller */
> > +	dev_res->end = 0xfffffffe;
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MEDIATEK, PCI_DEVICE_ID_MEDIATEK_7629,
> > +			 mtk_fixup_bar_size);
> > +
> >  static const struct of_device_id mtk_pcie_ids[] = {
> >  	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
> >  	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
> >  	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
> >  	{ .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
> > +	{ .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
> >  	{},
> >  };
> >  
> > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > index 69f0abe1ba1a..77b278bac3a8 100644
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -2126,6 +2126,7 @@
> >  #define PCI_VENDOR_ID_MYRICOM		0x14c1
> >  
> >  #define PCI_VENDOR_ID_MEDIATEK		0x14c3
> > +#define PCI_DEVICE_ID_MEDIATEK_7629	0x7629
> >  
> >  #define PCI_VENDOR_ID_TITAN		0x14D2
> >  #define PCI_DEVICE_ID_TITAN_010L	0x8001
> 
>
Jianjun Wang - Dec. 17, 2018, 8:19 a.m.
On Thu, 2018-12-13 at 08:55 -0600, Bjorn Helgaas wrote:
> On Thu, Dec 06, 2018 at 09:09:13AM +0800, Jianjun Wang wrote:
> > MT7629 is an arm platform SoC which has the same PCIe IP with MT7622.
> 
> s/arm/ARM/
> 
> > The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> > in arm64 but bogus alignment values at arm32, the pcie device and devices
> > behind this bridge will not be enabled. Fix it's BAR0 resource size to
> > guarantee the pcie devices will be enabled correctly.
> 
> So this is a hardware erratum?  Per spec, a memory BAR has bit 0 hardwired
> to 0, and an IO BAR has bit 1 hardwired to 0.

Yes, it only works properly on 64bit platform.
> 
> > The HW default value of its device id is invalid, fix it's device id to
> > match the hardware implementation.
> 
> s/pcie/PCIe/ (all places above)
> s/it's/its/ (all places above)
> s/device id/Device ID/

OK, thanks.
> 
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 26 ++++++++++++++++++++++++++
> >  include/linux/pci_ids.h                |  1 +
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index d20cf461ba00..f8937cc3c87c 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -73,6 +73,7 @@
> >  #define PCIE_MSI_VECTOR		0x0c0
> >  
> >  #define PCIE_CONF_VEND_ID	0x100
> > +#define PCIE_CONF_DEVICE_ID	0x102
> >  #define PCIE_CONF_CLASS_ID	0x106
> >  
> >  #define PCIE_INT_MASK		0x420
> > @@ -135,12 +136,14 @@ struct mtk_pcie_port;
> >  /**
> >   * struct mtk_pcie_soc - differentiate between host generations
> >   * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> > + * @need_fix_device_id: whether this host's device ID needed to be fixed or not
> >   * @ops: pointer to configuration access functions
> >   * @startup: pointer to controller setting functions
> >   * @setup_irq: pointer to initialize IRQ functions
> >   */
> >  struct mtk_pcie_soc {
> >  	bool need_fix_class_id;
> > +	bool need_fix_device_id;
> >  	struct pci_ops *ops;
> >  	int (*startup)(struct mtk_pcie_port *port);
> >  	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> > @@ -692,6 +695,11 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> >  		writew(val, port->base + PCIE_CONF_CLASS_ID);
> >  	}
> >  
> > +	if (soc->need_fix_device_id) {
> > +		val = PCI_DEVICE_ID_MEDIATEK_7629;
> > +		writew(val, port->base + PCIE_CONF_DEVICE_ID);
> > +	}
> > +
> >  	/* 100ms timeout value should be enough for Gen1/2 training */
> >  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
> >  				 !!(val & PCIE_PORT_LINKUP_V2), 20,
> > @@ -1238,11 +1246,29 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
> >  	.setup_irq = mtk_pcie_setup_irq,
> >  };
> >  
> > +static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
> > +	.need_fix_class_id = true,
> > +	.need_fix_device_id = true,
> > +	.ops = &mtk_pcie_ops_v2,
> > +	.startup = mtk_pcie_startup_port_v2,
> > +	.setup_irq = mtk_pcie_setup_irq,
> > +};
> > +
> > +static void mtk_fixup_bar_size(struct pci_dev *dev)
> > +{
> > +	struct resource *dev_res = &dev->resource[0];
> 
> Add a blank line here.

OK.
> 
> > +	/* 32bit resource length will calculate size to 0, set it smaller */
> > +	dev_res->end = 0xfffffffe;
> 
> Presumably you know the size of the BAR because that's fixed by the
> hardware and documented in the spec.  You should do something like
> this so you don't depend on what the BAR contains, e.g.,
> 
>   res->end = res->start + size - 1;

Yes, it will be much more better, I will modify the code in next
version, thanks.
> 
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MEDIATEK, PCI_DEVICE_ID_MEDIATEK_7629,
> > +			 mtk_fixup_bar_size);
> > +
> >  static const struct of_device_id mtk_pcie_ids[] = {
> >  	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
> >  	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
> >  	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
> >  	{ .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
> > +	{ .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
> >  	{},
> >  };
> >  
> > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > index 69f0abe1ba1a..77b278bac3a8 100644
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -2126,6 +2126,7 @@
> >  #define PCI_VENDOR_ID_MYRICOM		0x14c1
> >  
> >  #define PCI_VENDOR_ID_MEDIATEK		0x14c3
> > +#define PCI_DEVICE_ID_MEDIATEK_7629	0x7629
> >  
> >  #define PCI_VENDOR_ID_TITAN		0x14D2
> >  #define PCI_DEVICE_ID_TITAN_010L	0x8001
> > -- 
> > 2.19.1
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Bjorn Helgaas - Dec. 17, 2018, 2:32 p.m.
On Mon, Dec 17, 2018 at 04:19:39PM +0800, Jianjun Wang wrote:
> On Thu, 2018-12-13 at 08:55 -0600, Bjorn Helgaas wrote:
> > On Thu, Dec 06, 2018 at 09:09:13AM +0800, Jianjun Wang wrote:
> > > The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> > > in arm64 but bogus alignment values at arm32, the pcie device and devices
> > > behind this bridge will not be enabled. Fix it's BAR0 resource size to
> > > guarantee the pcie devices will be enabled correctly.
> > 
> > So this is a hardware erratum?  Per spec, a memory BAR has bit 0 hardwired
> > to 0, and an IO BAR has bit 1 hardwired to 0.
> 
> Yes, it only works properly on 64bit platform.

I don't understand.  BARs are supposed to work the same regardless of
whether it's a 32- or 64-bit platform.  If this is a workaround for a
hardware defect, please just say that explicitly.

Bjorn
Lorenzo Pieralisi - Dec. 17, 2018, 3:46 p.m.
On Mon, Dec 17, 2018 at 08:32:47AM -0600, Bjorn Helgaas wrote:
> On Mon, Dec 17, 2018 at 04:19:39PM +0800, Jianjun Wang wrote:
> > On Thu, 2018-12-13 at 08:55 -0600, Bjorn Helgaas wrote:
> > > On Thu, Dec 06, 2018 at 09:09:13AM +0800, Jianjun Wang wrote:
> > > > The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> > > > in arm64 but bogus alignment values at arm32, the pcie device and devices
> > > > behind this bridge will not be enabled. Fix it's BAR0 resource size to
> > > > guarantee the pcie devices will be enabled correctly.
> > > 
> > > So this is a hardware erratum?  Per spec, a memory BAR has bit 0 hardwired
> > > to 0, and an IO BAR has bit 1 hardwired to 0.
> > 
> > Yes, it only works properly on 64bit platform.
> 
> I don't understand.  BARs are supposed to work the same regardless of
> whether it's a 32- or 64-bit platform.  If this is a workaround for a
> hardware defect, please just say that explicitly.

I do not understand this either. First thing to do is to describe the
problem properly so that we can actually find a solution to it.

Lorenzo
Jianjun Wang - Dec. 18, 2018, 9:19 a.m.
On Mon, 2018-12-17 at 15:46 +0000, Lorenzo Pieralisi wrote:
> On Mon, Dec 17, 2018 at 08:32:47AM -0600, Bjorn Helgaas wrote:
> > On Mon, Dec 17, 2018 at 04:19:39PM +0800, Jianjun Wang wrote:
> > > On Thu, 2018-12-13 at 08:55 -0600, Bjorn Helgaas wrote:
> > > > On Thu, Dec 06, 2018 at 09:09:13AM +0800, Jianjun Wang wrote:
> > > > > The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> > > > > in arm64 but bogus alignment values at arm32, the pcie device and devices
> > > > > behind this bridge will not be enabled. Fix it's BAR0 resource size to
> > > > > guarantee the pcie devices will be enabled correctly.
> > > > 
> > > > So this is a hardware erratum?  Per spec, a memory BAR has bit 0 hardwired
> > > > to 0, and an IO BAR has bit 1 hardwired to 0.
> > > 
> > > Yes, it only works properly on 64bit platform.
> > 
> > I don't understand.  BARs are supposed to work the same regardless of
> > whether it's a 32- or 64-bit platform.  If this is a workaround for a
> > hardware defect, please just say that explicitly.
> 
> I do not understand this either. First thing to do is to describe the
> problem properly so that we can actually find a solution to it.
> 
> Lorenzo

This BAR0 is a 64-bit memory BAR, the HW default values for this BAR is
0xffff_ffff_0000_0000 and it could not be changed except by config write
operation.

The calculated BAR size will be 0 in 32-bit platform since the
phys_addr_t is a 32bit value in 32-bit platform.

Actually MediaTek's HW does not using this BAR0, just omit it when
assign resource is totally fine.

When assign the resource for each device, software will check the
resource alignment first, and the resource of length zero will be
regarded as a bogus alignment resource, it will be ignored and won't
claim a resource parent for it.

When drivers try to enable the PCIe devices, the software will enable
it's resources, but it will return an error number when found a
unclaimed resource, in that case, the flow of enable devices will be
interrupted and PCIe devices won't work properly.

Thanks.
Lorenzo Pieralisi - Dec. 18, 2018, 3:32 p.m.
On Tue, Dec 18, 2018 at 05:19:24PM +0800, Jianjun Wang wrote:
> On Mon, 2018-12-17 at 15:46 +0000, Lorenzo Pieralisi wrote:
> > On Mon, Dec 17, 2018 at 08:32:47AM -0600, Bjorn Helgaas wrote:
> > > On Mon, Dec 17, 2018 at 04:19:39PM +0800, Jianjun Wang wrote:
> > > > On Thu, 2018-12-13 at 08:55 -0600, Bjorn Helgaas wrote:
> > > > > On Thu, Dec 06, 2018 at 09:09:13AM +0800, Jianjun Wang wrote:
> > > > > > The read value of BAR0 is 0xffff_ffff, it's size will be calculated as 4GB
> > > > > > in arm64 but bogus alignment values at arm32, the pcie device and devices
> > > > > > behind this bridge will not be enabled. Fix it's BAR0 resource size to
> > > > > > guarantee the pcie devices will be enabled correctly.
> > > > > 
> > > > > So this is a hardware erratum?  Per spec, a memory BAR has bit 0 hardwired
> > > > > to 0, and an IO BAR has bit 1 hardwired to 0.
> > > > 
> > > > Yes, it only works properly on 64bit platform.
> > > 
> > > I don't understand.  BARs are supposed to work the same regardless of
> > > whether it's a 32- or 64-bit platform.  If this is a workaround for a
> > > hardware defect, please just say that explicitly.
> > 
> > I do not understand this either. First thing to do is to describe the
> > problem properly so that we can actually find a solution to it.
> > 
> > Lorenzo
> 
> This BAR0 is a 64-bit memory BAR, the HW default values for this BAR is
> 0xffff_ffff_0000_0000 and it could not be changed except by config write
> operation.
> 
> The calculated BAR size will be 0 in 32-bit platform since the
> phys_addr_t is a 32bit value in 32-bit platform.
> 
> Actually MediaTek's HW does not using this BAR0, just omit it when
> assign resource is totally fine.
> 
> When assign the resource for each device, software will check the
> resource alignment first, and the resource of length zero will be
> regarded as a bogus alignment resource, it will be ignored and won't
> claim a resource parent for it.
> 
> When drivers try to enable the PCIe devices, the software will enable
> it's resources, but it will return an error number when found a
> unclaimed resource, in that case, the flow of enable devices will be
> interrupted and PCIe devices won't work properly.

As a starting point, please provide kernel logs for both 64-bit and
32-bit platforms (without this patch applied) and also a:

cat /proc/iomem

and

lspci

output for both configurations.

Thanks,
Lorenzo

Patch

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index d20cf461ba00..f8937cc3c87c 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -73,6 +73,7 @@ 
 #define PCIE_MSI_VECTOR		0x0c0
 
 #define PCIE_CONF_VEND_ID	0x100
+#define PCIE_CONF_DEVICE_ID	0x102
 #define PCIE_CONF_CLASS_ID	0x106
 
 #define PCIE_INT_MASK		0x420
@@ -135,12 +136,14 @@  struct mtk_pcie_port;
 /**
  * struct mtk_pcie_soc - differentiate between host generations
  * @need_fix_class_id: whether this host's class ID needed to be fixed or not
+ * @need_fix_device_id: whether this host's device ID needed to be fixed or not
  * @ops: pointer to configuration access functions
  * @startup: pointer to controller setting functions
  * @setup_irq: pointer to initialize IRQ functions
  */
 struct mtk_pcie_soc {
 	bool need_fix_class_id;
+	bool need_fix_device_id;
 	struct pci_ops *ops;
 	int (*startup)(struct mtk_pcie_port *port);
 	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
@@ -692,6 +695,11 @@  static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 		writew(val, port->base + PCIE_CONF_CLASS_ID);
 	}
 
+	if (soc->need_fix_device_id) {
+		val = PCI_DEVICE_ID_MEDIATEK_7629;
+		writew(val, port->base + PCIE_CONF_DEVICE_ID);
+	}
+
 	/* 100ms timeout value should be enough for Gen1/2 training */
 	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
 				 !!(val & PCIE_PORT_LINKUP_V2), 20,
@@ -1238,11 +1246,29 @@  static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
 	.setup_irq = mtk_pcie_setup_irq,
 };
 
+static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
+	.need_fix_class_id = true,
+	.need_fix_device_id = true,
+	.ops = &mtk_pcie_ops_v2,
+	.startup = mtk_pcie_startup_port_v2,
+	.setup_irq = mtk_pcie_setup_irq,
+};
+
+static void mtk_fixup_bar_size(struct pci_dev *dev)
+{
+	struct resource *dev_res = &dev->resource[0];
+	/* 32bit resource length will calculate size to 0, set it smaller */
+	dev_res->end = 0xfffffffe;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MEDIATEK, PCI_DEVICE_ID_MEDIATEK_7629,
+			 mtk_fixup_bar_size);
+
 static const struct of_device_id mtk_pcie_ids[] = {
 	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
 	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
 	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
 	{ .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
+	{ .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
 	{},
 };
 
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 69f0abe1ba1a..77b278bac3a8 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2126,6 +2126,7 @@ 
 #define PCI_VENDOR_ID_MYRICOM		0x14c1
 
 #define PCI_VENDOR_ID_MEDIATEK		0x14c3
+#define PCI_DEVICE_ID_MEDIATEK_7629	0x7629
 
 #define PCI_VENDOR_ID_TITAN		0x14D2
 #define PCI_DEVICE_ID_TITAN_010L	0x8001