| Message ID | 20230717065459.14138-3-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | Improvements to Qcom PCIe EP and EPF MHI drivers | expand |
On Mon, Jul 17, 2023 at 12:24:54PM +0530, Manivannan Sadhasivam wrote: > Instead of hardcoding the alignment restriction in the EPF_MHI driver, make > use of the info available from the EPF core that reflects the alignment > restriction of the endpoint controller. > > For this purpose, let's introduce the get_align_offset() static function. I thought this might be related to the [1/7] patch since they both mention an alignment restriction in the EPF core, but [1/7] sets pci_epc_features.align and this patch doesn't reference .align, so this must be a different alignment restriction? I'm sure there's nothing wrong here, and this is already applied, so no need to do anything unless .align *should* appear here. > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/endpoint/functions/pci-epf-mhi.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c > index 9c1f5a154fbd..bb7de6884824 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c > +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c > @@ -102,6 +102,11 @@ struct pci_epf_mhi { > int irq; > }; > > +static size_t get_align_offset(struct pci_epc *epc, u64 addr) > +{ > + return addr % epc->mem->window.page_size; > +} > + > static int __pci_epf_mhi_alloc_map(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, > phys_addr_t *paddr, void __iomem **vaddr, > size_t offset, size_t size) > @@ -134,7 +139,7 @@ static int pci_epf_mhi_alloc_map(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, > { > struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); > struct pci_epc *epc = epf_mhi->epf->epc; > - size_t offset = pci_addr & (epc->mem->window.page_size - 1); > + size_t offset = get_align_offset(epc, pci_addr); > > return __pci_epf_mhi_alloc_map(mhi_cntrl, pci_addr, paddr, vaddr, > offset, size); > @@ -161,7 +166,7 @@ static void pci_epf_mhi_unmap_free(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, > struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); > struct pci_epf *epf = epf_mhi->epf; > struct pci_epc *epc = epf->epc; > - size_t offset = pci_addr & (epc->mem->window.page_size - 1); > + size_t offset = get_align_offset(epc, pci_addr); > > __pci_epf_mhi_unmap_free(mhi_cntrl, pci_addr, paddr, vaddr, offset, > size); > @@ -185,7 +190,8 @@ static int pci_epf_mhi_read_from_host(struct mhi_ep_cntrl *mhi_cntrl, u64 from, > void *to, size_t size) > { > struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); > - size_t offset = from % SZ_4K; > + struct pci_epc *epc = epf_mhi->epf->epc; > + size_t offset = get_align_offset(epc, from); > void __iomem *tre_buf; > phys_addr_t tre_phys; > int ret; > @@ -213,7 +219,8 @@ static int pci_epf_mhi_write_to_host(struct mhi_ep_cntrl *mhi_cntrl, > void *from, u64 to, size_t size) > { > struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); > - size_t offset = to % SZ_4K; > + struct pci_epc *epc = epf_mhi->epf->epc; > + size_t offset = get_align_offset(epc, to); > void __iomem *tre_buf; > phys_addr_t tre_phys; > int ret; > -- > 2.25.1 >
On Fri, Aug 25, 2023 at 05:50:06PM -0500, Bjorn Helgaas wrote: > On Mon, Jul 17, 2023 at 12:24:54PM +0530, Manivannan Sadhasivam wrote: > > Instead of hardcoding the alignment restriction in the EPF_MHI driver, make > > use of the info available from the EPF core that reflects the alignment > > restriction of the endpoint controller. > > > > For this purpose, let's introduce the get_align_offset() static function. > > I thought this might be related to the [1/7] patch since they both > mention an alignment restriction in the EPF core, but [1/7] sets > pci_epc_features.align and this patch doesn't reference .align, so > this must be a different alignment restriction? > > I'm sure there's nothing wrong here, and this is already applied, so > no need to do anything unless .align *should* appear here. > You are absolutely right! The patch was intented to make use of "align" but "page_size" was used as per old revision. Even though this patch works (because both "page_size" and "align" were 4K in my setup), it should be fixed. I will send a fix up patch now. Thanks for spotting. - Mani > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/pci/endpoint/functions/pci-epf-mhi.c | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c > > index 9c1f5a154fbd..bb7de6884824 100644 > > --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c > > +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c > > @@ -102,6 +102,11 @@ struct pci_epf_mhi { > > int irq; > > }; > > > > +static size_t get_align_offset(struct pci_epc *epc, u64 addr) > > +{ > > + return addr % epc->mem->window.page_size; > > +} > > + > > static int __pci_epf_mhi_alloc_map(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, > > phys_addr_t *paddr, void __iomem **vaddr, > > size_t offset, size_t size) > > @@ -134,7 +139,7 @@ static int pci_epf_mhi_alloc_map(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, > > { > > struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); > > struct pci_epc *epc = epf_mhi->epf->epc; > > - size_t offset = pci_addr & (epc->mem->window.page_size - 1); > > + size_t offset = get_align_offset(epc, pci_addr); > > > > return __pci_epf_mhi_alloc_map(mhi_cntrl, pci_addr, paddr, vaddr, > > offset, size); > > @@ -161,7 +166,7 @@ static void pci_epf_mhi_unmap_free(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, > > struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); > > struct pci_epf *epf = epf_mhi->epf; > > struct pci_epc *epc = epf->epc; > > - size_t offset = pci_addr & (epc->mem->window.page_size - 1); > > + size_t offset = get_align_offset(epc, pci_addr); > > > > __pci_epf_mhi_unmap_free(mhi_cntrl, pci_addr, paddr, vaddr, offset, > > size); > > @@ -185,7 +190,8 @@ static int pci_epf_mhi_read_from_host(struct mhi_ep_cntrl *mhi_cntrl, u64 from, > > void *to, size_t size) > > { > > struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); > > - size_t offset = from % SZ_4K; > > + struct pci_epc *epc = epf_mhi->epf->epc; > > + size_t offset = get_align_offset(epc, from); > > void __iomem *tre_buf; > > phys_addr_t tre_phys; > > int ret; > > @@ -213,7 +219,8 @@ static int pci_epf_mhi_write_to_host(struct mhi_ep_cntrl *mhi_cntrl, > > void *from, u64 to, size_t size) > > { > > struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); > > - size_t offset = to % SZ_4K; > > + struct pci_epc *epc = epf_mhi->epf->epc; > > + size_t offset = get_align_offset(epc, to); > > void __iomem *tre_buf; > > phys_addr_t tre_phys; > > int ret; > > -- > > 2.25.1 > >
diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c index 9c1f5a154fbd..bb7de6884824 100644 --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c @@ -102,6 +102,11 @@ struct pci_epf_mhi { int irq; }; +static size_t get_align_offset(struct pci_epc *epc, u64 addr) +{ + return addr % epc->mem->window.page_size; +} + static int __pci_epf_mhi_alloc_map(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, phys_addr_t *paddr, void __iomem **vaddr, size_t offset, size_t size) @@ -134,7 +139,7 @@ static int pci_epf_mhi_alloc_map(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, { struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); struct pci_epc *epc = epf_mhi->epf->epc; - size_t offset = pci_addr & (epc->mem->window.page_size - 1); + size_t offset = get_align_offset(epc, pci_addr); return __pci_epf_mhi_alloc_map(mhi_cntrl, pci_addr, paddr, vaddr, offset, size); @@ -161,7 +166,7 @@ static void pci_epf_mhi_unmap_free(struct mhi_ep_cntrl *mhi_cntrl, u64 pci_addr, struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); struct pci_epf *epf = epf_mhi->epf; struct pci_epc *epc = epf->epc; - size_t offset = pci_addr & (epc->mem->window.page_size - 1); + size_t offset = get_align_offset(epc, pci_addr); __pci_epf_mhi_unmap_free(mhi_cntrl, pci_addr, paddr, vaddr, offset, size); @@ -185,7 +190,8 @@ static int pci_epf_mhi_read_from_host(struct mhi_ep_cntrl *mhi_cntrl, u64 from, void *to, size_t size) { struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); - size_t offset = from % SZ_4K; + struct pci_epc *epc = epf_mhi->epf->epc; + size_t offset = get_align_offset(epc, from); void __iomem *tre_buf; phys_addr_t tre_phys; int ret; @@ -213,7 +219,8 @@ static int pci_epf_mhi_write_to_host(struct mhi_ep_cntrl *mhi_cntrl, void *from, u64 to, size_t size) { struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl); - size_t offset = to % SZ_4K; + struct pci_epc *epc = epf_mhi->epf->epc; + size_t offset = get_align_offset(epc, to); void __iomem *tre_buf; phys_addr_t tre_phys; int ret;
Instead of hardcoding the alignment restriction in the EPF_MHI driver, make use of the info available from the EPF core that reflects the alignment restriction of the endpoint controller. For this purpose, let's introduce the get_align_offset() static function. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/pci/endpoint/functions/pci-epf-mhi.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)