| Message ID | 20240815085725.2740390-5-quic_mdalam@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Add cmd descriptor support | expand |
On 15/08/2024 10:57, Md Sadre Alam wrote: > Get crypto base address from DT. This will use for > command descriptor support for crypto register r/w > via BAM/DMA All your commit messages are oddly wrapped. This does not make reading it easy... > > Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com> > --- > Change in [v2] > > * Addressed all comments from v1 > > Change in [v1] > > * Added support to read crypto base address from dt > > drivers/crypto/qce/core.c | 13 ++++++++++++- > drivers/crypto/qce/core.h | 1 + > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c > index 28b5fd823827..9b23a948078a 100644 > --- a/drivers/crypto/qce/core.c > +++ b/drivers/crypto/qce/core.c > @@ -192,6 +192,7 @@ static int qce_crypto_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct qce_device *qce; > + struct resource *res; > int ret; > > qce = devm_kzalloc(dev, sizeof(*qce), GFP_KERNEL); > @@ -201,10 +202,16 @@ static int qce_crypto_probe(struct platform_device *pdev) > qce->dev = dev; > platform_set_drvdata(pdev, qce); > > - qce->base = devm_platform_ioremap_resource(pdev, 0); > + qce->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); > if (IS_ERR(qce->base)) > return PTR_ERR(qce->base); > > + qce->base_dma = dma_map_resource(dev, res->start, > + resource_size(res), > + DMA_BIDIRECTIONAL, 0); > + if (dma_mapping_error(dev, qce->base_dma)) > + return -ENXIO; > + > ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); > if (ret < 0) > return ret; And how do you handle error paths? > @@ -280,6 +287,7 @@ static int qce_crypto_probe(struct platform_device *pdev) > static void qce_crypto_remove(struct platform_device *pdev) > { > struct qce_device *qce = platform_get_drvdata(pdev); > + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > tasklet_kill(&qce->done_tasklet); > qce_unregister_algs(qce); > @@ -287,6 +295,9 @@ static void qce_crypto_remove(struct platform_device *pdev) > clk_disable_unprepare(qce->bus); > clk_disable_unprepare(qce->iface); > clk_disable_unprepare(qce->core); > + > + dma_unmap_resource(&pdev->dev, qce->base_dma, resource_size(res), > + DMA_BIDIRECTIONAL, 0); If you add code to the remove callback, not adding it to error paths is suspicious by itself... Best regards, Krzysztof
On 8/17/2024 2:40 PM, Krzysztof Kozlowski wrote: > On 15/08/2024 10:57, Md Sadre Alam wrote: >> Get crypto base address from DT. This will use for >> command descriptor support for crypto register r/w >> via BAM/DMA > > All your commit messages are oddly wrapped. This does not make reading > it easy... > >> >> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com> >> --- >> Change in [v2] >> >> * Addressed all comments from v1 >> >> Change in [v1] >> >> * Added support to read crypto base address from dt >> >> drivers/crypto/qce/core.c | 13 ++++++++++++- >> drivers/crypto/qce/core.h | 1 + >> 2 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c >> index 28b5fd823827..9b23a948078a 100644 >> --- a/drivers/crypto/qce/core.c >> +++ b/drivers/crypto/qce/core.c >> @@ -192,6 +192,7 @@ static int qce_crypto_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> struct qce_device *qce; >> + struct resource *res; >> int ret; >> >> qce = devm_kzalloc(dev, sizeof(*qce), GFP_KERNEL); >> @@ -201,10 +202,16 @@ static int qce_crypto_probe(struct platform_device *pdev) >> qce->dev = dev; >> platform_set_drvdata(pdev, qce); >> >> - qce->base = devm_platform_ioremap_resource(pdev, 0); >> + qce->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); >> if (IS_ERR(qce->base)) >> return PTR_ERR(qce->base); >> >> + qce->base_dma = dma_map_resource(dev, res->start, >> + resource_size(res), >> + DMA_BIDIRECTIONAL, 0); >> + if (dma_mapping_error(dev, qce->base_dma)) >> + return -ENXIO; >> + >> ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); >> if (ret < 0) >> return ret; > > And how do you handle error paths? Ok , will fix the error path to cleanup the resources. > > >> @@ -280,6 +287,7 @@ static int qce_crypto_probe(struct platform_device *pdev) >> static void qce_crypto_remove(struct platform_device *pdev) >> { >> struct qce_device *qce = platform_get_drvdata(pdev); >> + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> >> tasklet_kill(&qce->done_tasklet); >> qce_unregister_algs(qce); >> @@ -287,6 +295,9 @@ static void qce_crypto_remove(struct platform_device *pdev) >> clk_disable_unprepare(qce->bus); >> clk_disable_unprepare(qce->iface); >> clk_disable_unprepare(qce->core); >> + >> + dma_unmap_resource(&pdev->dev, qce->base_dma, resource_size(res), >> + DMA_BIDIRECTIONAL, 0); > > If you add code to the remove callback, not adding it to error paths is > suspicious by itself... Ok , will fix the error path to cleanup the resources. > > Best regards, > Krzysztof > >
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c index 28b5fd823827..9b23a948078a 100644 --- a/drivers/crypto/qce/core.c +++ b/drivers/crypto/qce/core.c @@ -192,6 +192,7 @@ static int qce_crypto_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct qce_device *qce; + struct resource *res; int ret; qce = devm_kzalloc(dev, sizeof(*qce), GFP_KERNEL); @@ -201,10 +202,16 @@ static int qce_crypto_probe(struct platform_device *pdev) qce->dev = dev; platform_set_drvdata(pdev, qce); - qce->base = devm_platform_ioremap_resource(pdev, 0); + qce->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(qce->base)) return PTR_ERR(qce->base); + qce->base_dma = dma_map_resource(dev, res->start, + resource_size(res), + DMA_BIDIRECTIONAL, 0); + if (dma_mapping_error(dev, qce->base_dma)) + return -ENXIO; + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); if (ret < 0) return ret; @@ -280,6 +287,7 @@ static int qce_crypto_probe(struct platform_device *pdev) static void qce_crypto_remove(struct platform_device *pdev) { struct qce_device *qce = platform_get_drvdata(pdev); + struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); tasklet_kill(&qce->done_tasklet); qce_unregister_algs(qce); @@ -287,6 +295,9 @@ static void qce_crypto_remove(struct platform_device *pdev) clk_disable_unprepare(qce->bus); clk_disable_unprepare(qce->iface); clk_disable_unprepare(qce->core); + + dma_unmap_resource(&pdev->dev, qce->base_dma, resource_size(res), + DMA_BIDIRECTIONAL, 0); } static const struct of_device_id qce_crypto_of_match[] = { diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h index 228fcd69ec51..25e2af45c047 100644 --- a/drivers/crypto/qce/core.h +++ b/drivers/crypto/qce/core.h @@ -39,6 +39,7 @@ struct qce_device { struct qce_dma_data dma; int burst_size; unsigned int pipe_pair_id; + dma_addr_t base_dma; int (*async_req_enqueue)(struct qce_device *qce, struct crypto_async_request *req); void (*async_req_done)(struct qce_device *qce, int ret);
Get crypto base address from DT. This will use for command descriptor support for crypto register r/w via BAM/DMA Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com> --- Change in [v2] * Addressed all comments from v1 Change in [v1] * Added support to read crypto base address from dt drivers/crypto/qce/core.c | 13 ++++++++++++- drivers/crypto/qce/core.h | 1 + 2 files changed, 13 insertions(+), 1 deletion(-)