[PATCH] PCI: Mediatek: Use resource_size function on resource object

[PATCH] PCI: Mediatek: Use resource_size function on resource object

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem

Generated by: scripts/coccinelle/api/resource_size.cocci

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index e307166..0168376 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 	struct resource *mem = &pcie->mem;
 	const struct mtk_pcie_soc *soc = port->pcie->soc;
 	u32 val;
-	size_t size;
 	int err;
 
 	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
@@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 		mtk_pcie_enable_msi(port);
 
 	/* Set AHB to PCIe translation windows */
-	size = mem->end - mem->start;
-	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
+	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));
 	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
 
 	val = upper_32_bits(mem->start);
-- 
2.6.4

Re: [PATCH] PCI: Mediatek: Use resource_size function on resource object

[Lorenzo Pieralisi]

On Wed, Jan 02, 2019 at 02:03:53PM +0800, honghui.zhang@mediatek.com wrote:
> From: Honghui Zhang <honghui.zhang@mediatek.com>
> 
> drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem
> 
> Generated by: scripts/coccinelle/api/resource_size.cocci
> 
> Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index e307166..0168376 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  	struct resource *mem = &pcie->mem;
>  	const struct mtk_pcie_soc *soc = port->pcie->soc;
>  	u32 val;
> -	size_t size;
>  	int err;
>  
>  	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> @@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		mtk_pcie_enable_msi(port);
>  
>  	/* Set AHB to PCIe translation windows */
> -	size = mem->end - mem->start;
> -	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));

checkpatch warns on this line, please make sure patches pass it before
posting them.

I will fix it up myself but please pay attention next time.

Thanks,
Lorenzo

>  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
>  
>  	val = upper_32_bits(mem->start);
> -- 
> 2.6.4
> 

Re: [PATCH] PCI: Mediatek: Use resource_size function on resource object

[Bjorn Helgaas]

Please pay attention to the changelog conventions, e.g., next time use
"PCI: mediatek: ..." so yours matches "git log --oneline
drivers/pci/controller/pcie-mediatek.c"

On Wed, Jan 02, 2019 at 02:03:53PM +0800, honghui.zhang@mediatek.com wrote:
> From: Honghui Zhang <honghui.zhang@mediatek.com>
> 
> drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem
> 
> Generated by: scripts/coccinelle/api/resource_size.cocci

This is not a changelog; it's only a restatement of the warning.  Next
time please include a description of the change.  It's OK if it
repeats the subject.

> Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index e307166..0168376 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  	struct resource *mem = &pcie->mem;
>  	const struct mtk_pcie_soc *soc = port->pcie->soc;
>  	u32 val;
> -	size_t size;
>  	int err;
>  
>  	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> @@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		mtk_pcie_enable_msi(port);
>  
>  	/* Set AHB to PCIe translation windows */
> -	size = mem->end - mem->start;
> -	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));
>  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
>  
>  	val = upper_32_bits(mem->start);
> -- 
> 2.6.4
> 

Re: [PATCH] PCI: Mediatek: Use resource_size function on resource object

[Bjorn Helgaas]

On Wed, Jan 02, 2019 at 02:03:53PM +0800, honghui.zhang@mediatek.com wrote:
> From: Honghui Zhang <honghui.zhang@mediatek.com>
> 
> drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem
> 
> Generated by: scripts/coccinelle/api/resource_size.cocci
> 
> Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index e307166..0168376 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  	struct resource *mem = &pcie->mem;
>  	const struct mtk_pcie_soc *soc = port->pcie->soc;
>  	u32 val;
> -	size_t size;
>  	int err;
>  
>  	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> @@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		mtk_pcie_enable_msi(port);
>  
>  	/* Set AHB to PCIe translation windows */
> -	size = mem->end - mem->start;
> -	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));

This is actually a fairly interesting change because it effectively
changes this:

  fls(mem->end - mem->start)

to this:

  fls(mem->end - mem->start + 1)

And mem->end is the last valid address, so it changes something like
this:

  fls(0xffff)         # == 15

to this:

  fls(0x10000)        # == 16

So while this *looks* like a trivial warning fix, it likely fixes an
important bug, and it's worth pointing out what that bug is in the
changelog.

>  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
>  
>  	val = upper_32_bits(mem->start);
> -- 
> 2.6.4
> 

Re: [PATCH] PCI: Mediatek: Use resource_size function on resource object

[Bjorn Helgaas]

On Wed, Jan 30, 2019 at 12:33:47PM +0000, Lorenzo Pieralisi wrote:
> On Wed, Jan 02, 2019 at 02:03:53PM +0800, honghui.zhang@mediatek.com wrote:
> > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > 
> > drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem
> > 
> > Generated by: scripts/coccinelle/api/resource_size.cocci
> > 
> > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index e307166..0168376 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> >  	struct resource *mem = &pcie->mem;
> >  	const struct mtk_pcie_soc *soc = port->pcie->soc;
> >  	u32 val;
> > -	size_t size;
> >  	int err;
> >  
> >  	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> > @@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> >  		mtk_pcie_enable_msi(port);
> >  
> >  	/* Set AHB to PCIe translation windows */
> > -	size = mem->end - mem->start;
> > -	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> > +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));
> 
> checkpatch warns on this line, please make sure patches pass it before
> posting them.

I didn't actually run checkpatch myself, so I don't know why it
complained.

The patch you merged moves "mem_size = resource_size(mem)" higher up,
away from the previous location and its use, which makes it a little
harder to read.

Bjorn

Re: [PATCH] PCI: Mediatek: Use resource_size function on resource object

[Bjorn Helgaas]

On Wed, Jan 02, 2019 at 02:03:53PM +0800, honghui.zhang@mediatek.com wrote:
> From: Honghui Zhang <honghui.zhang@mediatek.com>
> 
> drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem
> 
> Generated by: scripts/coccinelle/api/resource_size.cocci
> 
> Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index e307166..0168376 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  	struct resource *mem = &pcie->mem;
>  	const struct mtk_pcie_soc *soc = port->pcie->soc;
>  	u32 val;
> -	size_t size;
>  	int err;
>  
>  	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> @@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		mtk_pcie_enable_msi(port);
>  
>  	/* Set AHB to PCIe translation windows */
> -	size = mem->end - mem->start;
> -	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));
>  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
>  
>  	val = upper_32_bits(mem->start);

Unrelated to this patch, but just below this:

        /* Set PCIe to AXI translation memory space.*/
        val = fls(0xffffffff) | WIN_ENABLE;
        writel(val, port->base + PCIE_AXI_WINDOW0);

Can you double-check the use of "fls(0xffffffff)"?  That expression is
a constant and I think evaluates to 31 (0x1f), i.e.,

        val = 0x1f | WIN_ENABLE;

I don't know the hardware, so this might be correct, but
"fls(0xffffffff)" looks funny because I think it's the same as
"fls(0x80000000)".

Bjorn

Re: [PATCH] PCI: Mediatek: Use resource_size function on resource object

[Lorenzo Pieralisi]

On Wed, Jan 30, 2019 at 09:58:53AM -0600, Bjorn Helgaas wrote:
> On Wed, Jan 30, 2019 at 12:33:47PM +0000, Lorenzo Pieralisi wrote:
> > On Wed, Jan 02, 2019 at 02:03:53PM +0800, honghui.zhang@mediatek.com wrote:
> > > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > > 
> > > drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem
> > > 
> > > Generated by: scripts/coccinelle/api/resource_size.cocci
> > > 
> > > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > > ---
> > >  drivers/pci/controller/pcie-mediatek.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > > index e307166..0168376 100644
> > > --- a/drivers/pci/controller/pcie-mediatek.c
> > > +++ b/drivers/pci/controller/pcie-mediatek.c
> > > @@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> > >  	struct resource *mem = &pcie->mem;
> > >  	const struct mtk_pcie_soc *soc = port->pcie->soc;
> > >  	u32 val;
> > > -	size_t size;
> > >  	int err;
> > >  
> > >  	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> > > @@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> > >  		mtk_pcie_enable_msi(port);
> > >  
> > >  	/* Set AHB to PCIe translation windows */
> > > -	size = mem->end - mem->start;
> > > -	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> > > +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));
> > 
> > checkpatch warns on this line, please make sure patches pass it before
> > posting them.
> 
> I didn't actually run checkpatch myself, so I don't know why it
> complained.

WARNING: line over 80 characters
#35: FILE: drivers/pci/controller/pcie-mediatek.c:708:
+	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));

I do run it as a sanity check.

> The patch you merged moves "mem_size = resource_size(mem)" higher up,
> away from the previous location and its use, which makes it a little
> harder to read.

That's because it was how the original code (which as you pointed out
is likely buggy) was written.

Anyway patch dropped waiting for a v2 consistent with your review -
apologies for missing some key review points.

Lorenzo

Re: [PATCH] PCI: Mediatek: Use resource_size function on resource object

[Honghui Zhang]

On Wed, 2019-01-30 at 16:31 +0000, Lorenzo Pieralisi wrote:
> On Wed, Jan 30, 2019 at 09:58:53AM -0600, Bjorn Helgaas wrote:
> > On Wed, Jan 30, 2019 at 12:33:47PM +0000, Lorenzo Pieralisi wrote:
> > > On Wed, Jan 02, 2019 at 02:03:53PM +0800, honghui.zhang@mediatek.com wrote:
> > > > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > > > 
> > > > drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem
> > > > 
> > > > Generated by: scripts/coccinelle/api/resource_size.cocci
> > > > 
> > > > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > > > ---
> > > >  drivers/pci/controller/pcie-mediatek.c | 4 +---
> > > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > > > index e307166..0168376 100644
> > > > --- a/drivers/pci/controller/pcie-mediatek.c
> > > > +++ b/drivers/pci/controller/pcie-mediatek.c
> > > > @@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> > > >  	struct resource *mem = &pcie->mem;
> > > >  	const struct mtk_pcie_soc *soc = port->pcie->soc;
> > > >  	u32 val;
> > > > -	size_t size;
> > > >  	int err;
> > > >  
> > > >  	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> > > > @@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> > > >  		mtk_pcie_enable_msi(port);
> > > >  
> > > >  	/* Set AHB to PCIe translation windows */
> > > > -	size = mem->end - mem->start;
> > > > -	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> > > > +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));
> > > 
> > > checkpatch warns on this line, please make sure patches pass it before
> > > posting them.
> > 
> > I didn't actually run checkpatch myself, so I don't know why it
> > complained.
> 
> WARNING: line over 80 characters
> #35: FILE: drivers/pci/controller/pcie-mediatek.c:708:
> +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));
> 
> I do run it as a sanity check.
> 
> > The patch you merged moves "mem_size = resource_size(mem)" higher up,
> > away from the previous location and its use, which makes it a little
> > harder to read.
> 
> That's because it was how the original code (which as you pointed out
> is likely buggy) was written.
> 
> Anyway patch dropped waiting for a v2 consistent with your review -
> apologies for missing some key review points.
> 

Thanks for your comments, I will send a v2 and fix all the issue you
pointed out.

> Lorenzo

Re: [PATCH] PCI: Mediatek: Use resource_size function on resource object

[Honghui Zhang]

On Wed, 2019-01-30 at 09:49 -0600, Bjorn Helgaas wrote:
> On Wed, Jan 02, 2019 at 02:03:53PM +0800, honghui.zhang@mediatek.com wrote:
> > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > 
> > drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem
> > 
> > Generated by: scripts/coccinelle/api/resource_size.cocci
> > 
> > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index e307166..0168376 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> >  	struct resource *mem = &pcie->mem;
> >  	const struct mtk_pcie_soc *soc = port->pcie->soc;
> >  	u32 val;
> > -	size_t size;
> >  	int err;
> >  
> >  	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> > @@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> >  		mtk_pcie_enable_msi(port);
> >  
> >  	/* Set AHB to PCIe translation windows */
> > -	size = mem->end - mem->start;
> > -	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> > +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));
> 
> This is actually a fairly interesting change because it effectively
> changes this:
> 
>   fls(mem->end - mem->start)
> 
> to this:
> 
>   fls(mem->end - mem->start + 1)
> 
> And mem->end is the last valid address, so it changes something like
> this:
> 
>   fls(0xffff)         # == 15
> 
> to this:
> 
>   fls(0x10000)        # == 16
> 
> So while this *looks* like a trivial warning fix, it likely fixes an
> important bug, and it's worth pointing out what that bug is in the
> changelog.
> 
> >  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
> >  
> >  	val = upper_32_bits(mem->start);

This size will set the MMIO size, which means that the RC will translate
the MMIO access from mem->start to mem->end.
The real MMIO size is specified in devicetree, which is 0x1000_0000 for
both mt2712 and mt7622.

This change make the size from fls(0x1000_0000) to fls(0x1000_0001), not
really change the values.

I will update the commit message and add the information mentioned
above.

Thanks for your kindly review.

>  > -- 
> > 2.6.4
> > 

Re: [PATCH] PCI: Mediatek: Use resource_size function on resource object

[Honghui Zhang]

On Thu, 2019-01-31 at 11:03 +0800, Honghui Zhang wrote:
> On Wed, 2019-01-30 at 09:49 -0600, Bjorn Helgaas wrote:
> > On Wed, Jan 02, 2019 at 02:03:53PM +0800, honghui.zhang@mediatek.com wrote:
> > > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > > 
> > > drivers/pci/pcie-mediatek.c:720:13-16: WARNING: Suspicious code. resource_size is maybe missing with mem
> > > 
> > > Generated by: scripts/coccinelle/api/resource_size.cocci
> > > 
> > > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > > ---
> > >  drivers/pci/controller/pcie-mediatek.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > > index e307166..0168376 100644
> > > --- a/drivers/pci/controller/pcie-mediatek.c
> > > +++ b/drivers/pci/controller/pcie-mediatek.c
> > > @@ -654,7 +654,6 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> > >  	struct resource *mem = &pcie->mem;
> > >  	const struct mtk_pcie_soc *soc = port->pcie->soc;
> > >  	u32 val;
> > > -	size_t size;
> > >  	int err;
> > >  
> > >  	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> > > @@ -706,8 +705,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> > >  		mtk_pcie_enable_msi(port);
> > >  
> > >  	/* Set AHB to PCIe translation windows */
> > > -	size = mem->end - mem->start;
> > > -	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> > > +	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(resource_size(mem)));
> > 
> > This is actually a fairly interesting change because it effectively
> > changes this:
> > 
> >   fls(mem->end - mem->start)
> > 
> > to this:
> > 
> >   fls(mem->end - mem->start + 1)
> > 
> > And mem->end is the last valid address, so it changes something like
> > this:
> > 
> >   fls(0xffff)         # == 15
> > 
> > to this:
> > 
> >   fls(0x10000)        # == 16
> > 
> > So while this *looks* like a trivial warning fix, it likely fixes an
> > important bug, and it's worth pointing out what that bug is in the
> > changelog.
> > 
> > >  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
> > >  
> > >  	val = upper_32_bits(mem->start);
> 
> This size will set the MMIO size, which means that the RC will translate
> the MMIO access from mem->start to mem->end.
> The real MMIO size is specified in devicetree, which is 0x1000_0000 for
> both mt2712 and mt7622.
> 
> This change make the size from fls(0x1000_0000) to fls(0x1000_0001), not
> really change the values.
> 
> I will update the commit message and add the information mentioned
> above.
> 
> Thanks for your kindly review.

I was wrong, after take a look at the resource parser function, that it
will initialize the res->end as res->start + res->size - 1.

But this change is still Ok since it will change the MMIO from
fls(0xfff_ffff) to fls(0x1000_0000), this just enlarge the MMIO
translate window size. The HW assigned MMIO is 0x1000_0000, but original
code set this translate window to fls(0xfff_ffff) = 0x800_0000 is fine
in most case since all the EP device we connect never asked a MMIO
window bigger than 0x800_0000. (As a matter of fact, most EP device will
only ask for 4MB MMIO window size.)

I will put this in the commit message.

thanks.
> 
> >  > -- 
> > > 2.6.4
> > > 
> 
>