Hi Pavitrakumar,
CC devicetree
On Tue, 18 Jun 2024, Pavitrakumar M wrote:
Signed-off-by: shwetar <shwetar@xxxxxxxxxxxxxxx>
Signed-off-by: Pavitrakumar M <pavitrakumarm@xxxxxxxxxxxxxxx>
Acked-by: Ruud Derwig <Ruud.Derwig@xxxxxxxxxxxx>
Thanks for your patch, which is now commit fc61c658c94cb740 ("crypto:
spacc - Enable Driver compilation in crypto Kconfig and Makefile")
in crypto/master.
--- /dev/null
+++ b/drivers/crypto/dwc-spacc/spacc_core.c
+int spacc_probe(struct platform_device *pdev,
+ const struct of_device_id snps_spacc_id[])
There should not be a need to pass snps_spacc_id[] around.
+{
+ int spacc_idx = -1;
+ struct resource *mem;
+ int spacc_endian = 0;
+ void __iomem *baseaddr;
+ struct pdu_info info;
+ int spacc_priority = -1;
+ struct spacc_priv *priv;
+ int x = 0, err, oldmode, irq_num;
+ const struct of_device_id *match, *id;
+ u64 oldtimer = 100000, timer = 100000;
+
+ if (pdev->dev.of_node) {
+ id = of_match_node(snps_spacc_id, pdev->dev.of_node);
+ if (!id) {
+ dev_err(&pdev->dev, "DT node did not match\n");
+ return -EINVAL;
+ }
+ }
This check is not needed.
+
+ /* Initialize DDT DMA pools based on this device's resources */
+ if (pdu_mem_init(&pdev->dev)) {
+ dev_err(&pdev->dev, "Could not initialize DMA pools\n");
+ return -ENOMEM;
+ }
+
+ match = of_match_device(of_match_ptr(snps_spacc_id), &pdev->dev);
+ if (!match) {
+ dev_err(&pdev->dev, "SPAcc dtb missing");
+ return -ENODEV;
+ }
This check is also not needed.
Besides, in case of an error, you leak the ddt mem pool.
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!mem) {
+ dev_err(&pdev->dev, "no memory resource for spacc\n");
+ err = -ENXIO;
+ goto free_ddt_mem_pool;
+ }
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ err = -ENOMEM;
+ goto free_ddt_mem_pool;
+ }
+
+ /* Read spacc priority and index and save inside priv.spacc.config */
+ if (of_property_read_u32(pdev->dev.of_node, "spacc_priority",
Please no underscores in DT properties.
+ &spacc_priority)) {
+ dev_err(&pdev->dev, "No vspacc priority specified\n");
+ err = -EINVAL;
+ goto free_ddt_mem_pool;
+ }
+
+ if (spacc_priority < 0 && spacc_priority > VSPACC_PRIORITY_MAX) {
+ dev_err(&pdev->dev, "Invalid vspacc priority\n");
+ err = -EINVAL;
+ goto free_ddt_mem_pool;
+ }
+ priv->spacc.config.priority = spacc_priority;
+
+ if (of_property_read_u32(pdev->dev.of_node, "spacc_index",
+ &spacc_idx)) {
+ dev_err(&pdev->dev, "No vspacc index specified\n");
+ err = -EINVAL;
+ goto free_ddt_mem_pool;
+ }
+ priv->spacc.config.idx = spacc_idx;
+
+ if (of_property_read_u32(pdev->dev.of_node, "spacc_endian",
Please use the standard big-endian / little-endian properties.
+ &spacc_endian)) {
+ dev_dbg(&pdev->dev, "No spacc_endian specified\n");
+ dev_dbg(&pdev->dev, "Default spacc Endianness (0==little)\n");
+ spacc_endian = 0;
+ }
+ priv->spacc.config.spacc_endian = spacc_endian;
+
+ if (of_property_read_u64(pdev->dev.of_node, "oldtimer",
+ &oldtimer)) {
+ dev_dbg(&pdev->dev, "No oldtimer specified\n");
+ dev_dbg(&pdev->dev, "Default oldtimer (100000)\n");
+ oldtimer = 100000;
+ }
+ priv->spacc.config.oldtimer = oldtimer;
+
+ if (of_property_read_u64(pdev->dev.of_node, "timer", &timer)) {
+ dev_dbg(&pdev->dev, "No timer specified\n");
+ dev_dbg(&pdev->dev, "Default timer (100000)\n");
+ timer = 100000;
+ }
+ priv->spacc.config.timer = timer;
This device lacks DT binding documentation.
+static struct platform_driver spacc_driver = {
+ .probe = spacc_crypto_probe,
+ .remove = spacc_crypto_remove,
+ .driver = {
+ .name = "spacc",
+ .of_match_table = of_match_ptr(snps_spacc_id),
Please drop the of_match_ptr(), as your driver requires DT to function.
+ .owner = THIS_MODULE,
+ },
+};
I didn't review the rest.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
