[PATCH] pci: When disabling pref MEM set all base bits
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);
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
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
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
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!
participants (3)
-
Pali Rohár -
Stefan Roese -
Tom Rini