[bug report] spi: Add Amlogic SPISG driver

Dan Carpenter dan.carpenter at linaro.org
Fri Aug 1 06:02:26 PDT 2025


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



More information about the linux-amlogic mailing list