On 26/4/2023 3:56 pm, Christophe JAILLET wrote:
> Le 26/04/2023 à 08:58, Jia Jie Ho a écrit :
>> Adding device probe and DMA init for StarFive cryptographic module.
>>
>> Co-developed-by: Huan Feng <huan.feng-bONrM45KWFOXmMXjJBpWqg@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Huan Feng <huan.feng-bONrM45KWFOXmMXjJBpWqg@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Jia Jie Ho <jiajie.ho-bONrM45KWFOXmMXjJBpWqg@xxxxxxxxxxxxxxxx>
>> ---
>> MAINTAINERS | 7 +
>> drivers/crypto/Kconfig | 1 +
>> drivers/crypto/Makefile | 1 +
>> drivers/crypto/starfive/Kconfig | 17 +++
>> drivers/crypto/starfive/Makefile | 4 +
>> drivers/crypto/starfive/jh7110-cryp.c | 199 ++++++++++++++++++++++++++
>> drivers/crypto/starfive/jh7110-cryp.h | 63 ++++++++
>> 7 files changed, 292 insertions(+)
>> create mode 100644 drivers/crypto/starfive/Kconfig
>> create mode 100644 drivers/crypto/starfive/Makefile
>> create mode 100644 drivers/crypto/starfive/jh7110-cryp.c
>> create mode 100644 drivers/crypto/starfive/jh7110-cryp.h
>>
[...]
>> +
>> +static int starfive_dma_init(struct starfive_cryp_dev *cryp)
>> +{
>> + dma_cap_mask_t mask;
>> +
>> + cryp->tx = NULL;
>> + cryp->rx = NULL;
>
> Harmless, but 'crypt' is kzalloc()'ed, so these fields are already NULL.
>
Hi Christophe,
I'll remove these in the next version.
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_SLAVE, mask);
>> +
>> + cryp->tx = dma_request_chan(cryp->dev, "tx");
>> + if (IS_ERR(cryp->tx))
>> + return dev_err_probe(cryp->dev, PTR_ERR(cryp->tx),
>> + "Error requesting tx dma channel.\n");
>> +
>> + cryp->rx = dma_request_chan(cryp->dev, "rx");
>> + if (IS_ERR(cryp->rx)) {
>> + dma_release_channel(cryp->tx);
>> + return dev_err_probe(cryp->dev, PTR_ERR(cryp->rx),
>> + "Error requesting rx dma channel.\n");
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void starfive_dma_cleanup(struct starfive_cryp_dev *cryp)
>> +{
>> + dma_release_channel(cryp->tx);
>> + dma_release_channel(cryp->rx);
>> +}
>> +
>> +static int starfive_cryp_probe(struct platform_device *pdev)
>> +{
[...]
>> +
>> + ret = crypto_engine_start(cryp->engine);
>> + if (ret)
>> + goto err_engine_start;
>> +
>> + return 0;
>> +
>> +err_engine_start:
>> + crypto_engine_exit(cryp->engine);
>> +err_engine:
>> + starfive_dma_cleanup(cryp);
>> +err_dma_init:
>> + spin_lock(&dev_list.lock);
>> + list_del(&cryp->list);
>> + spin_unlock(&dev_list.lock);
>
> I think that there should be:
> clk_disable_unprepare(cryp->hclk);
> clk_disable_unprepare(cryp->ahb);
> reset_control_assert(cryp->rst);
>
> as in the remove function.
>
Will add these in next version.
>> +
>> + return ret;
>> +}
>> +
>> +static int starfive_cryp_remove(struct platform_device *pdev)
>> +{
>> + struct starfive_cryp_dev *cryp = platform_get_drvdata(pdev);
>> +
>> + if (!cryp)
>> + return -ENODEV;
>
> I don't think that this can happen.
>
I'll update this too along with your other comments.
Thanks for taking time reviewing this patch series.
Best regards,
Jia Jie
