Linux SPI subsystem development
 help / color / mirror / Atom feed
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

             reply	other threads:[~2025-08-01 13:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-01 13:02 Dan Carpenter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2025-08-01 13:04 [bug report] spi: Add Amlogic SPISG driver 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox