public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH] pci: When disabling pref MEM set all base bits
@ 2021-11-25 10:34 Pali Rohár
  2021-11-30  6:10 ` Stefan Roese
  2022-01-13  1:51 ` Tom Rini
  0 siblings, 2 replies; 5+ messages in thread
From: Pali Rohár @ 2021-11-25 10:34 UTC (permalink / raw)
  To: Stefan Roese, Simon Glass, Bin Meng; +Cc: u-boot

It is common to set all base address bits to one and all limit address bits
to zero for disabling address forwarding. Forwarding is disabled when base
address is higher than limit address, so this change should not have any
effect.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci_auto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 6e5ed194f247..c0acf331398d 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -243,7 +243,7 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
 		cmdstat |= PCI_COMMAND_MEMORY;
 	} else {
 		/* We don't support prefetchable memory for now, so disable */
-		dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0x1000 |
+		dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0xfff0 |
 								prefechable_64);
 		dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT, 0x0 |
 								prefechable_64);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] pci: When disabling pref MEM set all base bits
  2021-11-25 10:34 [PATCH] pci: When disabling pref MEM set all base bits Pali Rohár
@ 2021-11-30  6:10 ` Stefan Roese
  2021-12-01 23:12   ` Pali Rohár
  2022-01-13  1:51 ` Tom Rini
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Roese @ 2021-11-30  6:10 UTC (permalink / raw)
  To: Pali Rohár, Simon Glass, Bin Meng; +Cc: u-boot

On 11/25/21 11:34, Pali Rohár wrote:
> It is common to set all base address bits to one and all limit address bits
> to zero for disabling address forwarding. Forwarding is disabled when base
> address is higher than limit address, so this change should not have any
> effect.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   drivers/pci/pci_auto.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
> index 6e5ed194f247..c0acf331398d 100644
> --- a/drivers/pci/pci_auto.c
> +++ b/drivers/pci/pci_auto.c
> @@ -243,7 +243,7 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
>   		cmdstat |= PCI_COMMAND_MEMORY;
>   	} else {
>   		/* We don't support prefetchable memory for now, so disable */
> -		dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0x1000 |
> +		dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0xfff0 |
>   								prefechable_64);

Again, does it make sense to add / use a macro for this 0xfff0 above?

Other than this:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan


>   		dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT, 0x0 |
>   								prefechable_64);
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] pci: When disabling pref MEM set all base bits
  2021-11-30  6:10 ` Stefan Roese
@ 2021-12-01 23:12   ` Pali Rohár
  2021-12-02 15:38     ` Stefan Roese
  0 siblings, 1 reply; 5+ messages in thread
From: Pali Rohár @ 2021-12-01 23:12 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Simon Glass, Bin Meng, u-boot

On Tuesday 30 November 2021 07:10:37 Stefan Roese wrote:
> On 11/25/21 11:34, Pali Rohár wrote:
> > It is common to set all base address bits to one and all limit address bits
> > to zero for disabling address forwarding. Forwarding is disabled when base
> > address is higher than limit address, so this change should not have any
> > effect.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >   drivers/pci/pci_auto.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
> > index 6e5ed194f247..c0acf331398d 100644
> > --- a/drivers/pci/pci_auto.c
> > +++ b/drivers/pci/pci_auto.c
> > @@ -243,7 +243,7 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
> >   		cmdstat |= PCI_COMMAND_MEMORY;
> >   	} else {
> >   		/* We don't support prefetchable memory for now, so disable */
> > -		dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0x1000 |
> > +		dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0xfff0 |
> >   								prefechable_64);
> 
> Again, does it make sense to add / use a macro for this 0xfff0 above?

This is again setting all range bits in this register to ones. So
(~0 & PCI_PREF_RANGE_MASK) or (0xffff & PCI_PREF_RANGE_MASK).

> Other than this:
> 
> Reviewed-by: Stefan Roese <sr@denx.de>
> 
> Thanks,
> Stefan
> 
> 
> >   		dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT, 0x0 |
> >   								prefechable_64);

And then 0x0 here can be expressed similarly via that macro by:
(0x0 & PCI_PREF_RANGE_MASK)

It is verbose, but would follow style if want to use macros for values.

> > 
> 
> Viele Grüße,
> Stefan Roese
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] pci: When disabling pref MEM set all base bits
  2021-12-01 23:12   ` Pali Rohár
@ 2021-12-02 15:38     ` Stefan Roese
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Roese @ 2021-12-02 15:38 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Simon Glass, Bin Meng, u-boot

On 12/2/21 00:12, Pali Rohár wrote:
> On Tuesday 30 November 2021 07:10:37 Stefan Roese wrote:
>> On 11/25/21 11:34, Pali Rohár wrote:
>>> It is common to set all base address bits to one and all limit address bits
>>> to zero for disabling address forwarding. Forwarding is disabled when base
>>> address is higher than limit address, so this change should not have any
>>> effect.
>>>
>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>> ---
>>>    drivers/pci/pci_auto.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
>>> index 6e5ed194f247..c0acf331398d 100644
>>> --- a/drivers/pci/pci_auto.c
>>> +++ b/drivers/pci/pci_auto.c
>>> @@ -243,7 +243,7 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
>>>    		cmdstat |= PCI_COMMAND_MEMORY;
>>>    	} else {
>>>    		/* We don't support prefetchable memory for now, so disable */
>>> -		dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0x1000 |
>>> +		dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0xfff0 |
>>>    								prefechable_64);
>>
>> Again, does it make sense to add / use a macro for this 0xfff0 above?
> 
> This is again setting all range bits in this register to ones. So
> (~0 & PCI_PREF_RANGE_MASK) or (0xffff & PCI_PREF_RANGE_MASK).

Again, no real issue IMHO - was just checking. Perhaps someone else has
more thoughts on this.

Thanks,
Stefan

>> Other than this:
>>
>> Reviewed-by: Stefan Roese <sr@denx.de>
>>
>> Thanks,
>> Stefan
>>
>>
>>>    		dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT, 0x0 |
>>>    								prefechable_64);
> 
> And then 0x0 here can be expressed similarly via that macro by:
> (0x0 & PCI_PREF_RANGE_MASK)
> 
> It is verbose, but would follow style if want to use macros for values.
> 
>>>
>>
>> Viele Grüße,
>> Stefan Roese
>>
>> -- 
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] pci: When disabling pref MEM set all base bits
  2021-11-25 10:34 [PATCH] pci: When disabling pref MEM set all base bits Pali Rohár
  2021-11-30  6:10 ` Stefan Roese
@ 2022-01-13  1:51 ` Tom Rini
  1 sibling, 0 replies; 5+ messages in thread
From: Tom Rini @ 2022-01-13  1:51 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Stefan Roese, Simon Glass, Bin Meng, u-boot

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

On Thu, Nov 25, 2021 at 11:34:37AM +0100, Pali Rohár wrote:

> It is common to set all base address bits to one and all limit address bits
> to zero for disabling address forwarding. Forwarding is disabled when base
> address is higher than limit address, so this change should not have any
> effect.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Stefan Roese <sr@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-01-13  1:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-25 10:34 [PATCH] pci: When disabling pref MEM set all base bits Pali Rohár
2021-11-30  6:10 ` Stefan Roese
2021-12-01 23:12   ` Pali Rohár
2021-12-02 15:38     ` Stefan Roese
2022-01-13  1:51 ` Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox