| Message ID | 20231127124529.78203-7-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | bus: mhi: ep: Add async read/write support | expand |
Hello, Manivannan, you will be taking this through the MHI tree, correct? > Now that both eDMA and iATU are prepared to support async transfer, let's > enable MHI async read/write by supplying the relevant callbacks. > > In the absence of eDMA, iATU will be used for both sync and async > operations. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/endpoint/functions/pci-epf-mhi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c > index 3d09a37e5f7c..d3d6a1054036 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c > +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c > @@ -766,12 +766,13 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf) > mhi_cntrl->raise_irq = pci_epf_mhi_raise_irq; > mhi_cntrl->alloc_map = pci_epf_mhi_alloc_map; > mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free; > + mhi_cntrl->read_sync = mhi_cntrl->read_async = pci_epf_mhi_iatu_read; > + mhi_cntrl->write_sync = mhi_cntrl->write_async = pci_epf_mhi_iatu_write; > if (info->flags & MHI_EPF_USE_DMA) { > mhi_cntrl->read_sync = pci_epf_mhi_edma_read; > mhi_cntrl->write_sync = pci_epf_mhi_edma_write; > - } else { > - mhi_cntrl->read_sync = pci_epf_mhi_iatu_read; > - mhi_cntrl->write_sync = pci_epf_mhi_iatu_write; > + mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; > + mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; > } Looks good! Reviewed-by: Krzysztof Wilczyński <kw@linux.com> Krzysztof
On Thu, Dec 14, 2023 at 03:48:29AM +0900, Krzysztof Wilczyński wrote: > Hello, > > Manivannan, you will be taking this through the MHI tree, correct? > Yes, to avoid conflict with other MHI patches, I'm taking this series through MHI tree. > > Now that both eDMA and iATU are prepared to support async transfer, let's > > enable MHI async read/write by supplying the relevant callbacks. > > > > In the absence of eDMA, iATU will be used for both sync and async > > operations. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/pci/endpoint/functions/pci-epf-mhi.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c > > index 3d09a37e5f7c..d3d6a1054036 100644 > > --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c > > +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c > > @@ -766,12 +766,13 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf) > > mhi_cntrl->raise_irq = pci_epf_mhi_raise_irq; > > mhi_cntrl->alloc_map = pci_epf_mhi_alloc_map; > > mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free; > > + mhi_cntrl->read_sync = mhi_cntrl->read_async = pci_epf_mhi_iatu_read; > > + mhi_cntrl->write_sync = mhi_cntrl->write_async = pci_epf_mhi_iatu_write; > > if (info->flags & MHI_EPF_USE_DMA) { > > mhi_cntrl->read_sync = pci_epf_mhi_edma_read; > > mhi_cntrl->write_sync = pci_epf_mhi_edma_write; > > - } else { > > - mhi_cntrl->read_sync = pci_epf_mhi_iatu_read; > > - mhi_cntrl->write_sync = pci_epf_mhi_iatu_write; > > + mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; > > + mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; > > } > > Looks good! > > Reviewed-by: Krzysztof Wilczyński <kw@linux.com> > Thanks! - Mani > Krzysztof
On 11/27/2023 6:15 PM, Manivannan Sadhasivam wrote: > Now that both eDMA and iATU are prepared to support async transfer, let's > enable MHI async read/write by supplying the relevant callbacks. > > In the absence of eDMA, iATU will be used for both sync and async > operations. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/endpoint/functions/pci-epf-mhi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c > index 3d09a37e5f7c..d3d6a1054036 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c > +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c > @@ -766,12 +766,13 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf) > mhi_cntrl->raise_irq = pci_epf_mhi_raise_irq; > mhi_cntrl->alloc_map = pci_epf_mhi_alloc_map; > mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free; > + mhi_cntrl->read_sync = mhi_cntrl->read_async = pci_epf_mhi_iatu_read; > + mhi_cntrl->write_sync = mhi_cntrl->write_async = pci_epf_mhi_iatu_write; > if (info->flags & MHI_EPF_USE_DMA) { > mhi_cntrl->read_sync = pci_epf_mhi_edma_read; > mhi_cntrl->write_sync = pci_epf_mhi_edma_write; > - } else { > - mhi_cntrl->read_sync = pci_epf_mhi_iatu_read; > - mhi_cntrl->write_sync = pci_epf_mhi_iatu_write; > + mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; > + mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; I think the read_async & write async should be updated inside the if condition where MHI_EPF_USE_DMA flag is set. - Krishna Chaitanya. > } > > /* Register the MHI EP controller */
On Thu, Dec 14, 2023 at 03:10:01PM +0530, Krishna Chaitanya Chundru wrote: > > On 11/27/2023 6:15 PM, Manivannan Sadhasivam wrote: > > Now that both eDMA and iATU are prepared to support async transfer, let's > > enable MHI async read/write by supplying the relevant callbacks. > > > > In the absence of eDMA, iATU will be used for both sync and async > > operations. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/pci/endpoint/functions/pci-epf-mhi.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c > > index 3d09a37e5f7c..d3d6a1054036 100644 > > --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c > > +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c > > @@ -766,12 +766,13 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf) > > mhi_cntrl->raise_irq = pci_epf_mhi_raise_irq; > > mhi_cntrl->alloc_map = pci_epf_mhi_alloc_map; > > mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free; > > + mhi_cntrl->read_sync = mhi_cntrl->read_async = pci_epf_mhi_iatu_read; > > + mhi_cntrl->write_sync = mhi_cntrl->write_async = pci_epf_mhi_iatu_write; > > if (info->flags & MHI_EPF_USE_DMA) { > > mhi_cntrl->read_sync = pci_epf_mhi_edma_read; > > mhi_cntrl->write_sync = pci_epf_mhi_edma_write; > > - } else { > > - mhi_cntrl->read_sync = pci_epf_mhi_iatu_read; > > - mhi_cntrl->write_sync = pci_epf_mhi_iatu_write; > > + mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; > > + mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; > > I think the read_async & write async should be updated inside the if > condition where MHI_EPF_USE_DMA flag is set. > That's what being done here. Am I missing anything? - Mani > - Krishna Chaitanya. > > > } > > /* Register the MHI EP controller */
On 12/14/2023 3:39 PM, Manivannan Sadhasivam wrote: > On Thu, Dec 14, 2023 at 03:10:01PM +0530, Krishna Chaitanya Chundru wrote: >> On 11/27/2023 6:15 PM, Manivannan Sadhasivam wrote: >>> Now that both eDMA and iATU are prepared to support async transfer, let's >>> enable MHI async read/write by supplying the relevant callbacks. >>> >>> In the absence of eDMA, iATU will be used for both sync and async >>> operations. >>> >>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >>> --- >>> drivers/pci/endpoint/functions/pci-epf-mhi.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c >>> index 3d09a37e5f7c..d3d6a1054036 100644 >>> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c >>> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c >>> @@ -766,12 +766,13 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf) >>> mhi_cntrl->raise_irq = pci_epf_mhi_raise_irq; >>> mhi_cntrl->alloc_map = pci_epf_mhi_alloc_map; >>> mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free; >>> + mhi_cntrl->read_sync = mhi_cntrl->read_async = pci_epf_mhi_iatu_read; >>> + mhi_cntrl->write_sync = mhi_cntrl->write_async = pci_epf_mhi_iatu_write; >>> if (info->flags & MHI_EPF_USE_DMA) { >>> mhi_cntrl->read_sync = pci_epf_mhi_edma_read; >>> mhi_cntrl->write_sync = pci_epf_mhi_edma_write; >>> - } else { >>> - mhi_cntrl->read_sync = pci_epf_mhi_iatu_read; >>> - mhi_cntrl->write_sync = pci_epf_mhi_iatu_write; >>> + mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; >>> + mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; >> I think the read_async & write async should be updated inside the if >> condition where MHI_EPF_USE_DMA flag is set. >> > That's what being done here. Am I missing anything? > > - Mani It should be like this as edma sync & aysnc read write should be update only if DMA is supported, in the patch I see async function pointers are being updated with the edma function pointers for IATU operations. if (info->flags & MHI_EPF_USE_DMA) { mhi_cntrl->read_sync = pci_epf_mhi_edma_read; mhi_cntrl->write_sync = pci_epf_mhi_edma_write; mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; } - Krishna Chaitanya. >> - Krishna Chaitanya. >> >>> } >>> /* Register the MHI EP controller */
On Thu, Dec 14, 2023 at 03:44:21PM +0530, Krishna Chaitanya Chundru wrote: > > On 12/14/2023 3:39 PM, Manivannan Sadhasivam wrote: > > On Thu, Dec 14, 2023 at 03:10:01PM +0530, Krishna Chaitanya Chundru wrote: > > > On 11/27/2023 6:15 PM, Manivannan Sadhasivam wrote: > > > > Now that both eDMA and iATU are prepared to support async transfer, let's > > > > enable MHI async read/write by supplying the relevant callbacks. > > > > > > > > In the absence of eDMA, iATU will be used for both sync and async > > > > operations. > > > > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > > --- > > > > drivers/pci/endpoint/functions/pci-epf-mhi.c | 7 ++++--- > > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c > > > > index 3d09a37e5f7c..d3d6a1054036 100644 > > > > --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c > > > > +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c > > > > @@ -766,12 +766,13 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf) > > > > mhi_cntrl->raise_irq = pci_epf_mhi_raise_irq; > > > > mhi_cntrl->alloc_map = pci_epf_mhi_alloc_map; > > > > mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free; > > > > + mhi_cntrl->read_sync = mhi_cntrl->read_async = pci_epf_mhi_iatu_read; > > > > + mhi_cntrl->write_sync = mhi_cntrl->write_async = pci_epf_mhi_iatu_write; > > > > if (info->flags & MHI_EPF_USE_DMA) { > > > > mhi_cntrl->read_sync = pci_epf_mhi_edma_read; > > > > mhi_cntrl->write_sync = pci_epf_mhi_edma_write; > > > > - } else { > > > > - mhi_cntrl->read_sync = pci_epf_mhi_iatu_read; > > > > - mhi_cntrl->write_sync = pci_epf_mhi_iatu_write; > > > > + mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; > > > > + mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; > > > I think the read_async & write async should be updated inside the if > > > condition where MHI_EPF_USE_DMA flag is set. > > > > > That's what being done here. Am I missing anything? > > > > - Mani > > It should be like this as edma sync & aysnc read write should be update only > if DMA is supported, in the patch I see async function pointers are being > updated with the edma function pointers for IATU operations. > > if (info->flags & MHI_EPF_USE_DMA) { > > mhi_cntrl->read_sync = pci_epf_mhi_edma_read; > mhi_cntrl->write_sync = pci_epf_mhi_edma_write; > mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; > mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; > } Are you reading the patch correctly? Please take a look at this commit: https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git/tree/drivers/pci/endpoint/functions/pci-epf-mhi.c?h=mhi-next&id=d1c6f4ba4746ed41fde8269cb5fea88bddb60504#n771 - Mani > - Krishna Chaitanya. > > > > - Krishna Chaitanya. > > > > > > > } > > > > /* Register the MHI EP controller */
On 12/14/2023 4:17 PM, Manivannan Sadhasivam wrote: > On Thu, Dec 14, 2023 at 03:44:21PM +0530, Krishna Chaitanya Chundru wrote: >> On 12/14/2023 3:39 PM, Manivannan Sadhasivam wrote: >>> On Thu, Dec 14, 2023 at 03:10:01PM +0530, Krishna Chaitanya Chundru wrote: >>>> On 11/27/2023 6:15 PM, Manivannan Sadhasivam wrote: >>>>> Now that both eDMA and iATU are prepared to support async transfer, let's >>>>> enable MHI async read/write by supplying the relevant callbacks. >>>>> >>>>> In the absence of eDMA, iATU will be used for both sync and async >>>>> operations. >>>>> >>>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >>>>> --- >>>>> drivers/pci/endpoint/functions/pci-epf-mhi.c | 7 ++++--- >>>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c >>>>> index 3d09a37e5f7c..d3d6a1054036 100644 >>>>> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c >>>>> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c >>>>> @@ -766,12 +766,13 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf) >>>>> mhi_cntrl->raise_irq = pci_epf_mhi_raise_irq; >>>>> mhi_cntrl->alloc_map = pci_epf_mhi_alloc_map; >>>>> mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free; >>>>> + mhi_cntrl->read_sync = mhi_cntrl->read_async = pci_epf_mhi_iatu_read; >>>>> + mhi_cntrl->write_sync = mhi_cntrl->write_async = pci_epf_mhi_iatu_write; >>>>> if (info->flags & MHI_EPF_USE_DMA) { >>>>> mhi_cntrl->read_sync = pci_epf_mhi_edma_read; >>>>> mhi_cntrl->write_sync = pci_epf_mhi_edma_write; >>>>> - } else { >>>>> - mhi_cntrl->read_sync = pci_epf_mhi_iatu_read; >>>>> - mhi_cntrl->write_sync = pci_epf_mhi_iatu_write; >>>>> + mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; >>>>> + mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; >>>> I think the read_async & write async should be updated inside the if >>>> condition where MHI_EPF_USE_DMA flag is set. >>>> >>> That's what being done here. Am I missing anything? >>> >>> - Mani >> It should be like this as edma sync & aysnc read write should be update only >> if DMA is supported, in the patch I see async function pointers are being >> updated with the edma function pointers for IATU operations. >> >> if (info->flags & MHI_EPF_USE_DMA) { >> >> mhi_cntrl->read_sync = pci_epf_mhi_edma_read; >> mhi_cntrl->write_sync = pci_epf_mhi_edma_write; >> mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; >> mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; >> } > Are you reading the patch correctly? Please take a look at this commit: > https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git/tree/drivers/pci/endpoint/functions/pci-epf-mhi.c?h=mhi-next&id=d1c6f4ba4746ed41fde8269cb5fea88bddb60504#n771 > > - Mani Sorry for the noise, I didn't notice else is also removed. - Krishna Chaitanya. >> - Krishna Chaitanya. >> >>>> - Krishna Chaitanya. >>>> >>>>> } >>>>> /* Register the MHI EP controller */
diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c index 3d09a37e5f7c..d3d6a1054036 100644 --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c @@ -766,12 +766,13 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf) mhi_cntrl->raise_irq = pci_epf_mhi_raise_irq; mhi_cntrl->alloc_map = pci_epf_mhi_alloc_map; mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free; + mhi_cntrl->read_sync = mhi_cntrl->read_async = pci_epf_mhi_iatu_read; + mhi_cntrl->write_sync = mhi_cntrl->write_async = pci_epf_mhi_iatu_write; if (info->flags & MHI_EPF_USE_DMA) { mhi_cntrl->read_sync = pci_epf_mhi_edma_read; mhi_cntrl->write_sync = pci_epf_mhi_edma_write; - } else { - mhi_cntrl->read_sync = pci_epf_mhi_iatu_read; - mhi_cntrl->write_sync = pci_epf_mhi_iatu_write; + mhi_cntrl->read_async = pci_epf_mhi_edma_read_async; + mhi_cntrl->write_async = pci_epf_mhi_edma_write_async; } /* Register the MHI EP controller */
Now that both eDMA and iATU are prepared to support async transfer, let's enable MHI async read/write by supplying the relevant callbacks. In the absence of eDMA, iATU will be used for both sync and async operations. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/pci/endpoint/functions/pci-epf-mhi.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)