| Message ID | 1550680435-9706-3-git-send-email-srinath.mannam@broadcom.com |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Add IPROC PCIe new features | expand |
On Wed, Feb 20, 2019 at 10:03:55PM +0530, Srinath Mannam wrote: > In the present driver outbound window configuration is done to map above > 32-bit address I/O regions with corresponding PCI memory range given in > ranges DT property. > > This patch add outbound window configuration to map below 32-bit I/O range > with corresponding PCI memory, which helps to access I/O region in IA32 I think you mean ARM 32-bit here, not IA32. > and one to one mapping of I/O region to PCI memory. > > Ex: > 1. ranges DT property given for current driver is, > ranges = <0x83000000 0x0 0x40000000 0x4 0x00000000 0 0x40000000>; > I/O region address is 0x400000000 > 2. ranges DT property can be given after this patch, > ranges = <0x83000000 0x0 0x42000000 0x0 0x42000000 0 0x2000000>; > I/O region address is 0x42000000 > > Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> > Signed-off-by: Abhishek Shah <abhishek.shah@broadcom.com> > Signed-off-by: Ray Jui <ray.jui@broadcom.com> > Reviewed-by: Scott Branden <scott.branden@broadcom.com> > Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com> I asked you before, please point me at mailing list discussions where these review tags were given, v1 was already carrying them but that's not how the process works, they have to be given on mailing lists. Lorenzo > --- > drivers/pci/controller/pcie-iproc.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c > index b882255..080f142 100644 > --- a/drivers/pci/controller/pcie-iproc.c > +++ b/drivers/pci/controller/pcie-iproc.c > @@ -955,8 +955,25 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr, > resource_size_t window_size = > ob_map->window_sizes[size_idx] * SZ_1M; > > - if (size < window_size) > - continue; > + /* > + * Keep iterating until we reach the last window and > + * with the minimal window size at index zero. In this > + * case, we take a compromise by mapping it using the > + * minimum window size that can be supported > + */ > + if (size < window_size) { > + if (size_idx > 0 || window_idx > 0) > + continue; > + > + /* > + * For the corner case of reaching the minimal > + * window size that can be supported on the > + * last window > + */ > + axi_addr = ALIGN_DOWN(axi_addr, window_size); > + pci_addr = ALIGN_DOWN(pci_addr, window_size); > + size = window_size; > + } > > if (!IS_ALIGNED(axi_addr, window_size) || > !IS_ALIGNED(pci_addr, window_size)) { > -- > 2.7.4 >
Hi Lorenzo, Thanks for the review.. Please see my comments below in line.. Regards, Srinath. On Thu, Feb 28, 2019 at 9:43 PM Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > On Wed, Feb 20, 2019 at 10:03:55PM +0530, Srinath Mannam wrote: > > In the present driver outbound window configuration is done to map above > > 32-bit address I/O regions with corresponding PCI memory range given in > > ranges DT property. > > > > This patch add outbound window configuration to map below 32-bit I/O range > > with corresponding PCI memory, which helps to access I/O region in IA32 > > I think you mean ARM 32-bit here, not IA32. Yes ARM 32 bit.. Thank you. > > > and one to one mapping of I/O region to PCI memory. > > > > Ex: > > 1. ranges DT property given for current driver is, > > ranges = <0x83000000 0x0 0x40000000 0x4 0x00000000 0 0x40000000>; > > I/O region address is 0x400000000 > > 2. ranges DT property can be given after this patch, > > ranges = <0x83000000 0x0 0x42000000 0x0 0x42000000 0 0x2000000>; > > I/O region address is 0x42000000 > > > > Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> > > Signed-off-by: Abhishek Shah <abhishek.shah@broadcom.com> > > Signed-off-by: Ray Jui <ray.jui@broadcom.com> > > Reviewed-by: Scott Branden <scott.branden@broadcom.com> > > Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com> > > I asked you before, please point me at mailing list discussions where > these review tags were given, v1 was already carrying them but that's > not how the process works, they have to be given on mailing lists. Sorry I missed your comment. These tags are our internal review list. Many Thanks for sharing knowledge. I will follow the process from next time onward. I will modify and send next patch set. > > Lorenzo > > > --- > > drivers/pci/controller/pcie-iproc.c | 21 +++++++++++++++++++-- > > 1 file changed, 19 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c > > index b882255..080f142 100644 > > --- a/drivers/pci/controller/pcie-iproc.c > > +++ b/drivers/pci/controller/pcie-iproc.c > > @@ -955,8 +955,25 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr, > > resource_size_t window_size = > > ob_map->window_sizes[size_idx] * SZ_1M; > > > > - if (size < window_size) > > - continue; > > + /* > > + * Keep iterating until we reach the last window and > > + * with the minimal window size at index zero. In this > > + * case, we take a compromise by mapping it using the > > + * minimum window size that can be supported > > + */ > > + if (size < window_size) { > > + if (size_idx > 0 || window_idx > 0) > > + continue; > > + > > + /* > > + * For the corner case of reaching the minimal > > + * window size that can be supported on the > > + * last window > > + */ > > + axi_addr = ALIGN_DOWN(axi_addr, window_size); > > + pci_addr = ALIGN_DOWN(pci_addr, window_size); > > + size = window_size; > > + } > > > > if (!IS_ALIGNED(axi_addr, window_size) || > > !IS_ALIGNED(pci_addr, window_size)) { > > -- > > 2.7.4 > >
diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c index b882255..080f142 100644 --- a/drivers/pci/controller/pcie-iproc.c +++ b/drivers/pci/controller/pcie-iproc.c @@ -955,8 +955,25 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr, resource_size_t window_size = ob_map->window_sizes[size_idx] * SZ_1M; - if (size < window_size) - continue; + /* + * Keep iterating until we reach the last window and + * with the minimal window size at index zero. In this + * case, we take a compromise by mapping it using the + * minimum window size that can be supported + */ + if (size < window_size) { + if (size_idx > 0 || window_idx > 0) + continue; + + /* + * For the corner case of reaching the minimal + * window size that can be supported on the + * last window + */ + axi_addr = ALIGN_DOWN(axi_addr, window_size); + pci_addr = ALIGN_DOWN(pci_addr, window_size); + size = window_size; + } if (!IS_ALIGNED(axi_addr, window_size) || !IS_ALIGNED(pci_addr, window_size)) {
In the present driver outbound window configuration is done to map above 32-bit address I/O regions with corresponding PCI memory range given in ranges DT property. This patch add outbound window configuration to map below 32-bit I/O range with corresponding PCI memory, which helps to access I/O region in IA32 and one to one mapping of I/O region to PCI memory. Ex: 1. ranges DT property given for current driver is, ranges = <0x83000000 0x0 0x40000000 0x4 0x00000000 0 0x40000000>; I/O region address is 0x400000000 2. ranges DT property can be given after this patch, ranges = <0x83000000 0x0 0x42000000 0x0 0x42000000 0 0x2000000>; I/O region address is 0x42000000 Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> Signed-off-by: Abhishek Shah <abhishek.shah@broadcom.com> Signed-off-by: Ray Jui <ray.jui@broadcom.com> Reviewed-by: Scott Branden <scott.branden@broadcom.com> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com> --- drivers/pci/controller/pcie-iproc.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)