On Thu, Oct 01, 2026 at 04:56:46PM +0200, Olivier Moysan wrote:
> +static int stm32_mdf_core_of_cck_get(struct platform_device *pdev, struct
> stm32_mdf_priv *priv)
> +{
> + struct device *dev = &pdev->dev;
> + u32 freq;
> + int ret;
> +
> + ret = device_property_read_u32(dev, "clock-frequency", &freq);
How? Your driver depends on OF and there is no such property in OF, so
why can you read it?
Otherwise what is this:
"depends on (ARCH_STM32 && OF)"
> + if (ret < 0) {
> + /* If property does not exist return immediately */
> + if (ret == -EINVAL)
> + return 0;
> +
> + dev_err(dev, "Failed to read clock-frequency property: %d\n",
> ret);
> + return ret;
> + }
> +
> + if (!freq) {
> + dev_err(dev, "Null frequency not allowed for cck output
> frequency\n");
> + return -EINVAL;
> + }
> + priv->cck_freq = freq;
> +
> + return 0;
> +}
> +
> +static int stm32_mdf_core_parse_clocks(struct platform_device *pdev, struct
> stm32_mdf_priv *priv)
> +{
> + struct device *dev = &pdev->dev;
> + struct clk *kclk;
> + int ret;
> +
> + kclk = devm_clk_get(dev, "ker_ck");
> + if (IS_ERR(kclk))
> + return dev_err_probe(dev, PTR_ERR(kclk), "Failed to get kernel
> clock\n");
> +
> + priv->kclk = kclk;
> + priv->kclk_rate = clk_get_rate(kclk);
> +
> + /* CCK0 and CCK1 clocks are optional. Used only in SPI master modes. */
> + ret = stm32_mdf_core_of_cck_get(pdev, priv);
> + if (ret)
> + return ret;
> +
> + if (priv->cck_freq) {
> + ret = stm32_mdf_core_cck_divider_set_rate(pdev, priv,
> priv->kclk_rate);
> + if (ret) {
> + dev_err(dev, "Failed to set cck rate: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + ret = stm32_mdf_core_register_clock_provider(pdev, priv);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int stm32_mdf_core_parse_of(struct platform_device *pdev, struct
> stm32_mdf_priv *priv)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct reset_control *rst;
> + struct device *dev = &pdev->dev;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + struct fwnode_handle *handle;
> + struct fwnode_handle **fh;
> + int count, ret, i;
> +
> + if (!node)
> + return -EINVAL;
> +
> + rst = devm_reset_control_get_optional_exclusive(&pdev->dev, "mdf");
> + if (IS_ERR(rst))
> + return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset
> controller\n");
> +
> + ret = reset_control_reset(rst);
> + if (ret) {
> + dev_err(&pdev->dev, "reset_control_reset failed %d\n", ret);
> + return ret;
> + }
> +
> + ret = stm32_mdf_core_parse_clocks(pdev, priv);
> + if (ret < 0)
> + return ret;
> +
> + if (device_property_present(&pdev->dev, "st,interleave")) {
> + count = fwnode_property_count_u32(fwnode, "st,interleave");
> + if (count < 2 || count > priv->mdf.nbf) {
> + dev_err(dev, "Wrong interleave filters number [%d]\n",
> count);
> + return -EINVAL;
> + }
> +
> + fh = devm_kzalloc(dev, count * sizeof(*fh), GFP_KERNEL);
> + if (!fh)
> + return -ENOMEM;
> + priv->mdf.fh_interleave = fh;
> +
> + for (i = 0; i < count; i++) {
> + handle = fwnode_find_reference(fwnode, "st,interleave",
> i);
> + if (IS_ERR(handle)) {
> + dev_err(dev, "Failed to read filter handle:
> %ld\n",
> + PTR_ERR(handle));
> + return PTR_ERR(handle);
> + }
> + priv->mdf.fh_interleave[i] = handle;
> + }
> +
> + priv->mdf.nb_interleave = count;
> +
> + /* Configure GCR */
> + ret = regmap_update_bits(priv->regmap, MDF_GCR_REG,
> + MDF_GCR_ILVNB_MASK,
> MDF_GCR_ILVNB(count - 1));
> + }
> +
> + return ret;
> +}
> +
> +static const struct of_device_id stm32_mdf_of_match[] = {
> + { .compatible = "st,stm32mp25-mdf" },
> + { .compatible = "st,stm32mp23-mdf", .data = (void
> *)STM32_MDF_MP23_FILTER_NB },
Again odd order of entries. Why doing things reverse from natural sort order?
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, stm32_mdf_of_match);
> +
> +static int stm32_mdf_core_identification(struct platform_device *pdev,
> struct stm32_mdf_priv *priv)
> +{
> + struct stm32_mdf *mdf = &priv->mdf;
> + u32 val;
> + int ret;
> +
> + /* If filter number is explicitly defined, don't check identification
> registers */
> + mdf->nbf = (uintptr_t)device_get_match_data(&pdev->dev);
Don't use uintptr_t, but unsigned long.
> + if (mdf->nbf)
> + return 0;
> +
> + ret = regmap_read(priv->regmap, MDF_IPIDR_REG, &val);
> + if (ret)
> + return ret;
> +
> + if (val == STM32MP25_MDF_IPIDR_NUMBER) {
> + ret = regmap_read(priv->regmap, MDF_HWCFGR_REG, &val);
> + if (ret)
> + return ret;
> +
> + mdf->nbf = FIELD_GET(MDF_HWCFGR_NBF_MASK, val);
> +
> + ret = regmap_read(priv->regmap, MDF_VERR_REG, &val);
> +
> + dev_dbg(&pdev->dev, "MDF version: %lu.%lu\n",
> FIELD_GET(MDF_VERR_MAJREV_MASK, val),
> + FIELD_GET(MDF_VERR_MINREV_MASK, val));
> + } else {
> + dev_err(&pdev->dev, "Unexpected ID number: 0x%x\n", val);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int stm32_mdf_core_probe(struct platform_device *pdev)
> +{
> + struct stm32_mdf_priv *priv;
> + struct resource *res;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> + priv->pdev = pdev;
> + spin_lock_init(&priv->lock);
> +
> + priv->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> + if (IS_ERR(priv->base))
> + return PTR_ERR(priv->base);
> + priv->phys_base = res->start;
> +
> + priv->regmap =
> + devm_regmap_init_mmio_clk(&pdev->dev, "ker_ck", priv->base,
> &stm32_mdf_regmap_cfg);
Odd wrappung.
> + if (IS_ERR(priv->regmap)) {
> + ret = PTR_ERR(priv->regmap);
> + dev_err(&pdev->dev, "Failed to allocate regmap: %d\n", ret);
We do not print allocation errors.
OTOH, every other probe failure should use return dev_err_probe syntax.
Again, same with bindings, please take existing recent code as starting
point, not upstream your old vendor code.
> + return ret;
> + }
> +
> + ret = stm32_mdf_core_identification(pdev, priv);
> + if (ret < 0)
> + return ret;
> +
> + ret = stm32_mdf_core_parse_of(pdev, priv);
> + if (ret < 0)
> + return ret;
> +
> + INIT_LIST_HEAD(&priv->mdf.sitf_list);
> + INIT_LIST_HEAD(&priv->mdf.filter_list);
> +
> + platform_set_drvdata(pdev, priv);
> +
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> +
> + ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> + if (ret)
> + goto pm_put;
> +
> + pm_runtime_put(&pdev->dev);
> +
> + return 0;
> +
> +pm_put:
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> + pm_runtime_put_noidle(&pdev->dev);
> +
> + return ret;
> +}
> +
> +static void stm32_mdf_core_remove(struct platform_device *pdev)
> +{
> + pm_runtime_get_sync(&pdev->dev);
> + of_platform_depopulate(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> + pm_runtime_put_noidle(&pdev->dev);
> +}
> +
> +static int stm32_mdf_core_suspend(struct device *dev)
> +{
> + struct stm32_mdf *mdf = dev_get_drvdata(dev);
> + struct stm32_mdf_priv *priv = to_stm32_mdf_priv(mdf);
> + int ret;
> +
> + ret = pm_runtime_force_suspend(dev);
> + if (ret)
> + return ret;
> +
> + regcache_cache_only(priv->regmap, true);
> + regcache_mark_dirty(priv->regmap);
> +
> + /* Balance devm_regmap_init_mmio_clk() clk_prepare() */
> + clk_unprepare(priv->kclk);
> +
> + return pinctrl_pm_select_sleep_state(dev);
> +}
> +
> +static int stm32_mdf_core_resume(struct device *dev)
> +{
> + struct stm32_mdf *mdf = dev_get_drvdata(dev);
> + struct stm32_mdf_priv *priv = to_stm32_mdf_priv(mdf);
> + int ret;
> +
> + ret = pinctrl_pm_select_default_state(dev);
> + if (ret) {
> + dev_err(dev, "Failed to set pins default state: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare(priv->kclk);
> + if (ret) {
> + dev_err(dev, "Failed to prepare kernel clock: %d\n", ret);
> + goto err_clk;
> + }
> +
> + regcache_cache_only(priv->regmap, false);
> + ret = regcache_sync(priv->regmap);
> + if (ret) {
> + dev_err(dev, "Failed to sync cache: %d\n", ret);
> + goto err_cache;
> + }
> +
> + return pm_runtime_force_resume(dev);
> +
> +err_cache:
> + clk_unprepare(priv->kclk);
> +err_clk:
> + pinctrl_pm_select_sleep_state(dev);
> +
> + return ret;
> +}
> +
> +static int stm32_mdf_core_runtime_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int stm32_mdf_core_runtime_resume(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static const struct dev_pm_ops stm32_mdf_core_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(stm32_mdf_core_suspend, stm32_mdf_core_resume)
> + SET_RUNTIME_PM_OPS(stm32_mdf_core_runtime_suspend,
> stm32_mdf_core_runtime_resume, NULL)
> +};
> +
> +static struct platform_driver stm32_mdf_driver = {
> + .probe = stm32_mdf_core_probe,
> + .remove = stm32_mdf_core_remove,
> + .driver = {
> + .name = "stm32-mdf",
> + .of_match_table = stm32_mdf_of_match,
> + .pm = &stm32_mdf_core_pm_ops,
> + },
> +};
> +
> +module_platform_driver(stm32_mdf_driver);
> +
> +MODULE_AUTHOR("Olivier Moysan <[email protected]>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 MDF driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/iio/adc/stm32-mdf-serial.c
> b/drivers/iio/adc/stm32-mdf-serial.c
> new file mode 100644
> index 000000000000..15a58cf833be
> --- /dev/null
> +++ b/drivers/iio/adc/stm32-mdf-serial.c
> @@ -0,0 +1,309 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * This file is part of STM32 MDF driver
> + *
> + * Copyright (C) 2023, STMicroelectronics - All Rights Reserved
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include "stm32-mdf.h"
> +
> +#define STM32_MDF_MODE_SZ 12
> +
> +enum {
> + STM32_MDF_SCKSRC_CCK0,
> + STM32_MDF_SCKSRC_CCK1,
> + STM32_MDF_SCKSRC_CLK,
> + STM32_MDF_SCKSRC_NONE,
> +};
> +
> +struct stm32_mdf_sf_mode {
> + const char *name;
> + u32 mode;
> +};
> +
> +static const struct stm32_mdf_sf_mode stm32_mdf_mode[STM32_MDF_MODE_NB] = {
> + { "spi", STM32_MDF_MODE_SPI },
> + { "lf_spi", STM32_MDF_MODE_LF_SPI },
> + { "manchester_r", STM32_MDF_MODE_MANCHESTER_R },
> + { "manchester_f", STM32_MDF_MODE_MANCHESTER_F },
> +};
> +
> +static bool stm32_mdf_sitf_readable_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case MDF_SITFCR_REG:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static bool stm32_mdf_sitf_writeable_reg(struct device *dev, unsigned int
> reg)
> +{
> + switch (reg) {
> + case MDF_SITFCR_REG:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct regmap_config stm32_sitf_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = sizeof(u32),
> + .max_register = MDF_SITFCR_REG,
> + .readable_reg = stm32_mdf_sitf_readable_reg,
> + .writeable_reg = stm32_mdf_sitf_writeable_reg,
> + .fast_io = true,
> + /* Do not use regcache as it does not support single register map */
> +};
> +
> +int stm32_mdf_sitf_start(struct stm32_mdf_sitf *sitf)
> +{
> + int ret;
> +
> + spin_lock(&sitf->lock);
> +
> + ret = regmap_set_bits(sitf->regmap, MDF_SITFCR_REG, MDF_SITFCR_SITFEN);
> + if (!ret)
> + sitf->refcnt++;
> +
> + spin_unlock(&sitf->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(stm32_mdf_sitf_start);
You need kerneldoc for all of exports.
> +
> +int stm32_mdf_sitf_stop(struct stm32_mdf_sitf *sitf)
> +{
> + int ret = 0;
> +
> + spin_lock(&sitf->lock);
> +
> + if (!sitf->refcnt) {
> + dev_err(sitf->dev, "Unbalanced serial interface stop ?\n");
> + ret = -EPERM;
> + goto out;
> + } else {
> + sitf->refcnt--;
> + }
> +
> + if (!sitf->refcnt)
> + ret = regmap_clear_bits(sitf->regmap, MDF_SITFCR_REG,
> MDF_SITFCR_SITFEN);
> +
> +out:
> + spin_unlock(&sitf->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(stm32_mdf_sitf_stop);
> +
> +static int stm32_mdf_sitf_get_clk(struct device *dev, struct stm32_mdf_sitf
> *sitf)
> +{
> + struct clk *sck;
> +
> + /* Optional clock. Clock not needed in Manchester mode */
> + sck = clk_get_optional(sitf->dev, 0);
> + if (IS_ERR(sck))
> + return dev_err_probe(sitf->dev, PTR_ERR(sck), "Can't get serial
> clock\n");
> +
> + sitf->sck = sck;
> +
> + if (sitf->sck) {
> + if (!strncmp(__clk_get_name(sck), STM32_MDF_CCK0,
> sizeof(STM32_MDF_CCK0)))
> + sitf->scksrc = STM32_MDF_SCKSRC_CCK0;
> + else if (!strncmp(__clk_get_name(sck), STM32_MDF_CCK1,
> sizeof(STM32_MDF_CCK1)))
> + sitf->scksrc = STM32_MDF_SCKSRC_CCK1;
> + else
> + sitf->scksrc = STM32_MDF_SCKSRC_CLK;
> + }
> +
> + return 0;
> +};
> +
> +static int stm32_mdf_sitf_parse(struct platform_device *pdev, struct
> stm32_mdf_sitf *sitf)
> +{
> + struct device *dev = &pdev->dev;
> + const char *str;
> + int ret, i = 0;
> + u32 idx, mode, sitfcr;
> +
> + ret = device_property_read_u32(dev, "reg", &idx);
> + if (ret) {
> + dev_err(dev, "Could not get interface index: %d\n", ret);
> + return ret;
> + }
> +
> + if (idx % 0x80) {
> + dev_err(dev, "Unexpected reg property value [%x]\n", idx);
> + return -EINVAL;
> + }
> +
> + idx = (idx >> 7) - 1;
> + if (idx > sitf->mdf->nbf) {
> + dev_err(dev, "Interface index [%d] exceeds maximum [%d]\n",
> idx, sitf->mdf->nbf);
> + return -EINVAL;
> + }
> +
> + /* Get SITF mode */
> + ret = device_property_read_string(dev, "st,sitf-mode", &str);
> + if (ret) {
> + dev_err(dev, "Could not get interface mode: %d\n", ret);
> + return ret;
> + }
> +
> + while (i < STM32_MDF_MODE_NB) {
> + if (!strncmp(stm32_mdf_mode[i].name, str, STM32_MDF_MODE_SZ)) {
> + mode = stm32_mdf_mode[i].mode;
> + break;
> + }
> + i++;
> + }
> +
> + if (i >= STM32_MDF_MODE_NB) {
> + dev_err(dev, "Unknown serial interface mode [%s]\n", str);
> + return -EINVAL;
> + }
> +
> + sitf->mode = mode;
> + sitfcr = MDF_SITFCR_SITFMOD(mode);
> +
> + ret = stm32_mdf_sitf_get_clk(dev, sitf);
> + if (ret)
> + return ret;
> +
> + if (mode == STM32_MDF_MODE_SPI && sitf->scksrc ==
> STM32_MDF_SCKSRC_NONE) {
> + dev_err(dev, "Missing clock for serial interface [%d] in SPI
> mode\n", idx);
> + return -EINVAL;
> + }
> +
> + if (mode == STM32_MDF_MODE_LF_SPI && (sitf->scksrc !=
> STM32_MDF_SCKSRC_CCK0 &&
> + sitf->scksrc !=
> STM32_MDF_SCKSRC_CCK1)) {
> + dev_err(dev, "Missing CCKx clock for serial interface [%d] in
> LF_SPI mode\n", idx);
> + return -EINVAL;
> + }
> +
> + sitf->id = idx;
> + sitf->node = dev_fwnode(dev);
> + spin_lock_init(&sitf->lock);
> +
> + sitfcr |= MDF_SITFCR_SCKSRC(sitf->scksrc);
> +
> + /* Configure SITF register */
> + regmap_set_bits(sitf->regmap, MDF_SITFCR_REG, sitfcr);
> +
> + dev_dbg(dev, "Serial interface [%d] registered\n", idx);
Drop, that's just probe success.
> +
> + return 0;
> +}
> +
> +static int stm32_mdf_sitf_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct stm32_mdf_sitf *sitf;
> + struct regmap *regmap;
> + struct resource *res;
> + void __iomem *base;
> + int ret;
> +
> + sitf = devm_kzalloc(&pdev->dev, sizeof(*sitf), GFP_KERNEL);
> + if (!sitf)
> + return -ENOMEM;
> + sitf->dev = dev;
> + sitf->mdf = dev_get_drvdata(dev->parent);
> +
> + base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + regmap = devm_regmap_init_mmio_clk(dev, "ker_ck", base,
> &stm32_sitf_regmap_cfg);
> + if (IS_ERR(regmap))
> + return dev_err_probe(dev, PTR_ERR(regmap), "Failed to init
> regmap\n");
And here quite different choice. Your code is just inconsistent.
> + sitf->regmap = regmap;
> +
> + ret = stm32_mdf_sitf_parse(pdev, sitf);
> + if (ret < 0)
> + return ret;
Best regards,
Krzysztof
