From: Dan Carpenter <dan.carpenter@linaro.org>
To: Sunny Luo <sunny.luo@amlogic.com>
Cc: linux-amlogic@lists.infradead.org, linux-spi@vger.kernel.org
Subject: [bug report] spi: Add Amlogic SPISG driver
Date: Fri, 1 Aug 2025 16:02:26 +0300 [thread overview]
Message-ID: <aIy64j1zj316IF_h@stanley.mountain> (raw)
Hello Sunny Luo,
Commit cef9991e04ae ("spi: Add Amlogic SPISG driver") from Jul 18,
2025 (linux-next), leads to the following Smatch static checker
warning:
drivers/spi/spi-amlogic-spisg.c:652 aml_spisg_clk_init()
warn: passing zero to 'PTR_ERR'
drivers/spi/spi-amlogic-spisg.c
640 static int aml_spisg_clk_init(struct spisg_device *spisg, void __iomem *base)
641 {
642 struct device *dev = &spisg->pdev->dev;
643 struct clk_init_data init;
644 struct clk_divider *div;
645 struct clk_div_table *tbl;
646 char name[32];
647 int ret, i;
648
649 spisg->core = devm_clk_get_enabled(dev, "core");
650 if (IS_ERR_OR_NULL(spisg->core)) {
This should just be an IS_ERR() check...
651 dev_err(dev, "core clock request failed\n");
Otherwise, let's pretend that devm_clk_get_enabled() could really return
NULL because there was a bug in the Kconfig or somewhere else, then it
would need to be treated as success
--> 652 return PTR_ERR(spisg->core);
This returns success, so I guess that would be fine except that really
it's not fine at all.
For more information see my blog:
https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/
653 }
654
655 spisg->pclk = devm_clk_get_enabled(dev, "pclk");
656 if (IS_ERR_OR_NULL(spisg->pclk)) {
657 dev_err(dev, "pclk clock request failed\n");
658 return PTR_ERR(spisg->pclk);
Same.
659 }
660
661 clk_set_min_rate(spisg->pclk, SPISG_PCLK_RATE_MIN);
662
663 clk_disable_unprepare(spisg->pclk);
664
665 tbl = devm_kzalloc(dev, sizeof(struct clk_div_table) * (DIV_NUM + 1), GFP_KERNEL);
666 if (!tbl)
667 return -ENOMEM;
668
669 for (i = 0; i < DIV_NUM; i++) {
670 tbl[i].val = i + SPISG_CLK_DIV_MIN - 1;
671 tbl[i].div = i + SPISG_CLK_DIV_MIN;
672 }
673 spisg->tbl = tbl;
674
675 div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
676 if (!div)
677 return -ENOMEM;
678
679 div->flags = CLK_DIVIDER_ROUND_CLOSEST;
680 div->reg = base + SPISG_REG_CFG_BUS;
681 div->shift = __bf_shf(CFG_CLK_DIV);
682 div->width = CLK_DIV_WIDTH;
683 div->table = tbl;
684
685 /* Register value should not be outside of the table */
686 regmap_update_bits(spisg->map, SPISG_REG_CFG_BUS, CFG_CLK_DIV,
687 FIELD_PREP(CFG_CLK_DIV, SPISG_CLK_DIV_MIN - 1));
688
689 /* Register clk-divider */
690 snprintf(name, sizeof(name), "%s_div", dev_name(dev));
691 init.name = name;
692 init.ops = &clk_divider_ops;
693 init.flags = CLK_SET_RATE_PARENT;
694 init.parent_data = &(const struct clk_parent_data) {
695 .fw_name = "pclk",
696 };
697 init.num_parents = 1;
698 div->hw.init = &init;
699 ret = devm_clk_hw_register(dev, &div->hw);
700 if (ret) {
701 dev_err(dev, "clock registration failed\n");
702 return ret;
703 }
704
705 spisg->sclk = devm_clk_hw_get_clk(dev, &div->hw, NULL);
706 if (IS_ERR_OR_NULL(spisg->sclk)) {
Same.
707 dev_err(dev, "get clock failed\n");
708 return PTR_ERR(spisg->sclk);
709 }
710
711 clk_prepare_enable(spisg->sclk);
712
713 return 0;
714 }
regards,
dan carpenter
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Sunny Luo <sunny.luo@amlogic.com>
Cc: linux-amlogic@lists.infradead.org, linux-spi@vger.kernel.org
Subject: [bug report] spi: Add Amlogic SPISG driver
Date: Fri, 1 Aug 2025 16:02:26 +0300 [thread overview]
Message-ID: <aIy64j1zj316IF_h@stanley.mountain> (raw)
Hello Sunny Luo,
Commit cef9991e04ae ("spi: Add Amlogic SPISG driver") from Jul 18,
2025 (linux-next), leads to the following Smatch static checker
warning:
drivers/spi/spi-amlogic-spisg.c:652 aml_spisg_clk_init()
warn: passing zero to 'PTR_ERR'
drivers/spi/spi-amlogic-spisg.c
640 static int aml_spisg_clk_init(struct spisg_device *spisg, void __iomem *base)
641 {
642 struct device *dev = &spisg->pdev->dev;
643 struct clk_init_data init;
644 struct clk_divider *div;
645 struct clk_div_table *tbl;
646 char name[32];
647 int ret, i;
648
649 spisg->core = devm_clk_get_enabled(dev, "core");
650 if (IS_ERR_OR_NULL(spisg->core)) {
This should just be an IS_ERR() check...
651 dev_err(dev, "core clock request failed\n");
Otherwise, let's pretend that devm_clk_get_enabled() could really return
NULL because there was a bug in the Kconfig or somewhere else, then it
would need to be treated as success
--> 652 return PTR_ERR(spisg->core);
This returns success, so I guess that would be fine except that really
it's not fine at all.
For more information see my blog:
https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/
653 }
654
655 spisg->pclk = devm_clk_get_enabled(dev, "pclk");
656 if (IS_ERR_OR_NULL(spisg->pclk)) {
657 dev_err(dev, "pclk clock request failed\n");
658 return PTR_ERR(spisg->pclk);
Same.
659 }
660
661 clk_set_min_rate(spisg->pclk, SPISG_PCLK_RATE_MIN);
662
663 clk_disable_unprepare(spisg->pclk);
664
665 tbl = devm_kzalloc(dev, sizeof(struct clk_div_table) * (DIV_NUM + 1), GFP_KERNEL);
666 if (!tbl)
667 return -ENOMEM;
668
669 for (i = 0; i < DIV_NUM; i++) {
670 tbl[i].val = i + SPISG_CLK_DIV_MIN - 1;
671 tbl[i].div = i + SPISG_CLK_DIV_MIN;
672 }
673 spisg->tbl = tbl;
674
675 div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
676 if (!div)
677 return -ENOMEM;
678
679 div->flags = CLK_DIVIDER_ROUND_CLOSEST;
680 div->reg = base + SPISG_REG_CFG_BUS;
681 div->shift = __bf_shf(CFG_CLK_DIV);
682 div->width = CLK_DIV_WIDTH;
683 div->table = tbl;
684
685 /* Register value should not be outside of the table */
686 regmap_update_bits(spisg->map, SPISG_REG_CFG_BUS, CFG_CLK_DIV,
687 FIELD_PREP(CFG_CLK_DIV, SPISG_CLK_DIV_MIN - 1));
688
689 /* Register clk-divider */
690 snprintf(name, sizeof(name), "%s_div", dev_name(dev));
691 init.name = name;
692 init.ops = &clk_divider_ops;
693 init.flags = CLK_SET_RATE_PARENT;
694 init.parent_data = &(const struct clk_parent_data) {
695 .fw_name = "pclk",
696 };
697 init.num_parents = 1;
698 div->hw.init = &init;
699 ret = devm_clk_hw_register(dev, &div->hw);
700 if (ret) {
701 dev_err(dev, "clock registration failed\n");
702 return ret;
703 }
704
705 spisg->sclk = devm_clk_hw_get_clk(dev, &div->hw, NULL);
706 if (IS_ERR_OR_NULL(spisg->sclk)) {
Same.
707 dev_err(dev, "get clock failed\n");
708 return PTR_ERR(spisg->sclk);
709 }
710
711 clk_prepare_enable(spisg->sclk);
712
713 return 0;
714 }
regards,
dan carpenter
next reply other threads:[~2025-08-01 13:07 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-01 13:02 Dan Carpenter [this message]
2025-08-01 13:02 ` [bug report] spi: Add Amlogic SPISG driver Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2025-08-01 13:04 Dan Carpenter
2025-08-01 13:04 ` Dan Carpenter
2025-08-04 8:14 Dan Carpenter
2025-08-04 8:14 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aIy64j1zj316IF_h@stanley.mountain \
--to=dan.carpenter@linaro.org \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-spi@vger.kernel.org \
--cc=sunny.luo@amlogic.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.