* [PATCH] pci: Disable I/O forwarding during autoconfiguration if unsupported @ 2021-11-25 10:32 Pali Rohár 2021-11-30 6:09 ` 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:32 UTC (permalink / raw) To: Stefan Roese, Simon Glass, Bin Meng; +Cc: u-boot If U-Boot does not have any I/O resource for assignment then disable I/O forwarding in PCI bridge autoconfiguration code. Default initial state of PCI bridge IO registers is unspecified, therefore they can be in enabled if U-Boot does not touch them. Signed-off-by: Pali Rohár <pali@kernel.org> --- drivers/pci/pci_auto.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index 7e6ee54be087..6e5ed194f247 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -265,6 +265,14 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus) (pci_io->bus_lower & 0xffff0000) >> 16); cmdstat |= PCI_COMMAND_IO; + } else { + /* Disable I/O if unsupported */ + dm_pci_write_config8(dev, PCI_IO_BASE, 0xf0 | io_32); + dm_pci_write_config8(dev, PCI_IO_LIMIT, 0x0 | io_32); + if (io_32 == PCI_IO_RANGE_TYPE_32) { + dm_pci_write_config16(dev, PCI_IO_BASE_UPPER16, 0x0); + dm_pci_write_config16(dev, PCI_IO_LIMIT_UPPER16, 0x0); + } } /* Enable memory and I/O accesses, enable bus master */ -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] pci: Disable I/O forwarding during autoconfiguration if unsupported 2021-11-25 10:32 [PATCH] pci: Disable I/O forwarding during autoconfiguration if unsupported Pali Rohár @ 2021-11-30 6:09 ` Stefan Roese 2021-12-01 23:08 ` 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:09 UTC (permalink / raw) To: Pali Rohár, Simon Glass, Bin Meng; +Cc: u-boot On 11/25/21 11:32, Pali Rohár wrote: > If U-Boot does not have any I/O resource for assignment then disable I/O > forwarding in PCI bridge autoconfiguration code. Default initial state of > PCI bridge IO registers is unspecified, therefore they can be in enabled if > U-Boot does not touch them. > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > drivers/pci/pci_auto.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c > index 7e6ee54be087..6e5ed194f247 100644 > --- a/drivers/pci/pci_auto.c > +++ b/drivers/pci/pci_auto.c > @@ -265,6 +265,14 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus) > (pci_io->bus_lower & 0xffff0000) >> 16); > > cmdstat |= PCI_COMMAND_IO; > + } else { > + /* Disable I/O if unsupported */ > + dm_pci_write_config8(dev, PCI_IO_BASE, 0xf0 | io_32); > + dm_pci_write_config8(dev, PCI_IO_LIMIT, 0x0 | io_32); Does it perhaps make sense to add / use a macro for this 0xf0 above? Other than this: Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > + if (io_32 == PCI_IO_RANGE_TYPE_32) { > + dm_pci_write_config16(dev, PCI_IO_BASE_UPPER16, 0x0); > + dm_pci_write_config16(dev, PCI_IO_LIMIT_UPPER16, 0x0); > + } > } > > /* Enable memory and I/O accesses, enable bus master */ > 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: Disable I/O forwarding during autoconfiguration if unsupported 2021-11-30 6:09 ` Stefan Roese @ 2021-12-01 23:08 ` Pali Rohár 2021-12-02 15:36 ` Stefan Roese 0 siblings, 1 reply; 5+ messages in thread From: Pali Rohár @ 2021-12-01 23:08 UTC (permalink / raw) To: Stefan Roese; +Cc: Simon Glass, Bin Meng, u-boot On Tuesday 30 November 2021 07:09:36 Stefan Roese wrote: > On 11/25/21 11:32, Pali Rohár wrote: > > If U-Boot does not have any I/O resource for assignment then disable I/O > > forwarding in PCI bridge autoconfiguration code. Default initial state of > > PCI bridge IO registers is unspecified, therefore they can be in enabled if > > U-Boot does not touch them. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > --- > > drivers/pci/pci_auto.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c > > index 7e6ee54be087..6e5ed194f247 100644 > > --- a/drivers/pci/pci_auto.c > > +++ b/drivers/pci/pci_auto.c > > @@ -265,6 +265,14 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus) > > (pci_io->bus_lower & 0xffff0000) >> 16); > > cmdstat |= PCI_COMMAND_IO; > > + } else { > > + /* Disable I/O if unsupported */ > > + dm_pci_write_config8(dev, PCI_IO_BASE, 0xf0 | io_32); > > + dm_pci_write_config8(dev, PCI_IO_LIMIT, 0x0 | io_32); > > Does it perhaps make sense to add / use a macro for this 0xf0 above? For setting range bits we already have macro PCI_IO_RANGE_MASK. So for setting all base range bits to one (which is this patch is suppose to do) I can write something like this: (~0 & PCI_IO_RANGE_MASK) or (0xff & PCI_IO_RANGE_MASK). (write_config8 denotes that we use 8bit numbers...) I/O forwarding is disabled when base address is higher than limit address and it is common to choose base address as the highest possible (hence all ones) and limit address to lowest possible (hence all zeros). So question is, do we need macro which evaluates to number with all zeros and another macro which evaluates to number with all ones? I'm not sure. For me is "0xf0 | io_32" more readable as "the verbose usage with macro" "(0xff & PCI_IO_RANGE_MASK) | io_32". But this is all just about "naming conventions" and "coding style". If you think that for consistency is better to create macro or use another construction, please let me know other ideas. I would follow any style which is expected here... > Other than this: > > Reviewed-by: Stefan Roese <sr@denx.de> > > Thanks, > Stefan > > > > + if (io_32 == PCI_IO_RANGE_TYPE_32) { > > + dm_pci_write_config16(dev, PCI_IO_BASE_UPPER16, 0x0); > > + dm_pci_write_config16(dev, PCI_IO_LIMIT_UPPER16, 0x0); > > + } > > } > > /* Enable memory and I/O accesses, enable bus master */ > > > > 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: Disable I/O forwarding during autoconfiguration if unsupported 2021-12-01 23:08 ` Pali Rohár @ 2021-12-02 15:36 ` Stefan Roese 0 siblings, 0 replies; 5+ messages in thread From: Stefan Roese @ 2021-12-02 15:36 UTC (permalink / raw) To: Pali Rohár; +Cc: Simon Glass, Bin Meng, u-boot On 12/2/21 00:08, Pali Rohár wrote: > On Tuesday 30 November 2021 07:09:36 Stefan Roese wrote: >> On 11/25/21 11:32, Pali Rohár wrote: >>> If U-Boot does not have any I/O resource for assignment then disable I/O >>> forwarding in PCI bridge autoconfiguration code. Default initial state of >>> PCI bridge IO registers is unspecified, therefore they can be in enabled if >>> U-Boot does not touch them. >>> >>> Signed-off-by: Pali Rohár <pali@kernel.org> >>> >>> --- >>> drivers/pci/pci_auto.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c >>> index 7e6ee54be087..6e5ed194f247 100644 >>> --- a/drivers/pci/pci_auto.c >>> +++ b/drivers/pci/pci_auto.c >>> @@ -265,6 +265,14 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus) >>> (pci_io->bus_lower & 0xffff0000) >> 16); >>> cmdstat |= PCI_COMMAND_IO; >>> + } else { >>> + /* Disable I/O if unsupported */ >>> + dm_pci_write_config8(dev, PCI_IO_BASE, 0xf0 | io_32); >>> + dm_pci_write_config8(dev, PCI_IO_LIMIT, 0x0 | io_32); >> >> Does it perhaps make sense to add / use a macro for this 0xf0 above? > > For setting range bits we already have macro PCI_IO_RANGE_MASK. > > So for setting all base range bits to one (which is this patch is > suppose to do) I can write something like this: (~0 & PCI_IO_RANGE_MASK) > or (0xff & PCI_IO_RANGE_MASK). (write_config8 denotes that we use 8bit > numbers...) > > I/O forwarding is disabled when base address is higher than limit > address and it is common to choose base address as the highest possible > (hence all ones) and limit address to lowest possible (hence all zeros). > > So question is, do we need macro which evaluates to number with all > zeros and another macro which evaluates to number with all ones? > > I'm not sure. For me is "0xf0 | io_32" more readable as "the verbose > usage with macro" "(0xff & PCI_IO_RANGE_MASK) | io_32". I have no hard feeling here to really change this - was just checking here. > But this is all just about "naming conventions" and "coding style". If > you think that for consistency is better to create macro or use another > construction, please let me know other ideas. I would follow any style > which is expected here... You already have my RB tag, so all is fine IMHO. Thanks, Stefan > >> Other than this: >> >> Reviewed-by: Stefan Roese <sr@denx.de> >> >> Thanks, >> Stefan >> >> >>> + if (io_32 == PCI_IO_RANGE_TYPE_32) { >>> + dm_pci_write_config16(dev, PCI_IO_BASE_UPPER16, 0x0); >>> + dm_pci_write_config16(dev, PCI_IO_LIMIT_UPPER16, 0x0); >>> + } >>> } >>> /* Enable memory and I/O accesses, enable bus master */ >>> >> >> 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: Disable I/O forwarding during autoconfiguration if unsupported 2021-11-25 10:32 [PATCH] pci: Disable I/O forwarding during autoconfiguration if unsupported Pali Rohár 2021-11-30 6:09 ` 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: 468 bytes --] On Thu, Nov 25, 2021 at 11:32:43AM +0100, Pali Rohár wrote: > If U-Boot does not have any I/O resource for assignment then disable I/O > forwarding in PCI bridge autoconfiguration code. Default initial state of > PCI bridge IO registers is unspecified, therefore they can be in enabled if > U-Boot does not touch them. > > 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:51 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:32 [PATCH] pci: Disable I/O forwarding during autoconfiguration if unsupported Pali Rohár 2021-11-30 6:09 ` Stefan Roese 2021-12-01 23:08 ` Pali Rohár 2021-12-02 15:36 ` 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