2022-06-06 05:05:36

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

It feels wrong and actually inconsistent to attach managed resources
to the IIO device object, which is child of the physical device object.
The rest of the ->probe() calls do that against physical device.

Resolve this by reassigning managed resources to the physical device object.

Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
Suggested-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
v3: new fix-patch
drivers/iio/adc/meson_saradc.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 62cc6fb0ef85..4fe6b997cd03 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -650,11 +650,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
void __iomem *base)
{
struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+ struct device *dev = indio_dev->dev.parent;
struct clk_init_data init;
const char *clk_parents[1];

- init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_div",
- dev_name(indio_dev->dev.parent));
+ init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_div", dev_name(dev));
if (!init.name)
return -ENOMEM;

@@ -670,13 +670,11 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
priv->clk_div.hw.init = &init;
priv->clk_div.flags = 0;

- priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
- &priv->clk_div.hw);
+ priv->adc_div_clk = devm_clk_register(dev, &priv->clk_div.hw);
if (WARN_ON(IS_ERR(priv->adc_div_clk)))
return PTR_ERR(priv->adc_div_clk);

- init.name = devm_kasprintf(&indio_dev->dev, GFP_KERNEL, "%s#adc_en",
- dev_name(indio_dev->dev.parent));
+ init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#adc_en", dev_name(dev));
if (!init.name)
return -ENOMEM;

@@ -690,7 +688,7 @@ static int meson_sar_adc_clk_init(struct iio_dev *indio_dev,
priv->clk_gate.bit_idx = __ffs(MESON_SAR_ADC_REG3_CLK_EN);
priv->clk_gate.hw.init = &init;

- priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
+ priv->adc_clk = devm_clk_register(dev, &priv->clk_gate.hw);
if (WARN_ON(IS_ERR(priv->adc_clk)))
return PTR_ERR(priv->adc_clk);

--
2.35.1


2022-06-06 05:19:11

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] iio: adc: meson_saradc: Don't attach managed resource to IIO device object

On Fri, Jun 3, 2022 at 12:00 PM Andy Shevchenko
<[email protected]> wrote:
>
> It feels wrong and actually inconsistent to attach managed resources
> to the IIO device object, which is child of the physical device object.
> The rest of the ->probe() calls do that against physical device.
>
> Resolve this by reassigning managed resources to the physical device object.
>
> Fixes: 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
> Suggested-by: Lars-Peter Clausen <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
Reviewed-by: Martin Blumenstingl <[email protected]>

I am also fine if the Fixes tag is being dropped - please keep my
Reviewed-by in that case.

2022-06-06 05:53:00

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v3 5/6] iio: adc: meson_saradc: Use temporary variable for struct device

Use temporary variable for struct device to make code neater.

Signed-off-by: Andy Shevchenko <[email protected]>
---
v3: rebased on top of previous changes

drivers/iio/adc/meson_saradc.c | 53 ++++++++++++++--------------------
1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 337517a8e1ac..c100f933c12e 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -549,6 +549,7 @@ static int meson_sar_adc_get_sample(struct iio_dev *indio_dev,
int *val)
{
struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+ struct device *dev = indio_dev->dev.parent;
int ret;

if (chan->type == IIO_TEMP && !priv->temperature_sensor_calibrated)
@@ -572,8 +573,7 @@ static int meson_sar_adc_get_sample(struct iio_dev *indio_dev,
meson_sar_adc_unlock(indio_dev);

if (ret) {
- dev_warn(indio_dev->dev.parent,
- "failed to read sample for channel %lu: %d\n",
+ dev_warn(dev, "failed to read sample for channel %lu: %d\n",
chan->address, ret);
return ret;
}
@@ -586,6 +586,7 @@ static int meson_sar_adc_iio_info_read_raw(struct iio_dev *indio_dev,
int *val, int *val2, long mask)
{
struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+ struct device *dev = indio_dev->dev.parent;
int ret;

switch (mask) {
@@ -602,9 +603,7 @@ static int meson_sar_adc_iio_info_read_raw(struct iio_dev *indio_dev,
if (chan->type == IIO_VOLTAGE) {
ret = regulator_get_voltage(priv->vref);
if (ret < 0) {
- dev_err(indio_dev->dev.parent,
- "failed to get vref voltage: %d\n",
- ret);
+ dev_err(dev, "failed to get vref voltage: %d\n", ret);
return ret;
}

@@ -703,8 +702,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
size_t read_len;
int ret;

- temperature_calib = devm_nvmem_cell_get(indio_dev->dev.parent,
- "temperature_calib");
+ temperature_calib = devm_nvmem_cell_get(dev, "temperature_calib");
if (IS_ERR(temperature_calib)) {
ret = PTR_ERR(temperature_calib);

@@ -718,9 +716,7 @@ static int meson_sar_adc_temp_sensor_init(struct iio_dev *indio_dev)
return dev_err_probe(dev, ret, "failed to get temperature_calib cell\n");
}

- priv->tsc_regmap =
- syscon_regmap_lookup_by_phandle(indio_dev->dev.parent->of_node,
- "amlogic,hhi-sysctrl");
+ priv->tsc_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "amlogic,hhi-sysctrl");
if (IS_ERR(priv->tsc_regmap))
return dev_err_probe(dev, PTR_ERR(priv->tsc_regmap),
"failed to get amlogic,hhi-sysctrl regmap\n");
@@ -908,6 +904,7 @@ static void meson_sar_adc_set_bandgap(struct iio_dev *indio_dev, bool on_off)
static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
{
struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+ struct device *dev = indio_dev->dev.parent;
int ret;
u32 regval;

@@ -917,14 +914,13 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)

ret = regulator_enable(priv->vref);
if (ret < 0) {
- dev_err(indio_dev->dev.parent,
- "failed to enable vref regulator\n");
+ dev_err(dev, "failed to enable vref regulator\n");
goto err_vref;
}

ret = clk_prepare_enable(priv->core_clk);
if (ret) {
- dev_err(indio_dev->dev.parent, "failed to enable core clk\n");
+ dev_err(dev, "failed to enable core clk\n");
goto err_core_clk;
}

@@ -942,7 +938,7 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)

ret = clk_prepare_enable(priv->adc_clk);
if (ret) {
- dev_err(indio_dev->dev.parent, "failed to enable adc clk\n");
+ dev_err(dev, "failed to enable adc clk\n");
goto err_adc_clk;
}

@@ -1177,14 +1173,14 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
void __iomem *base;
int irq, ret;

- indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
if (!indio_dev)
return dev_err_probe(dev, -ENOMEM, "failed allocating iio device\n");

priv = iio_priv(indio_dev);
init_completion(&priv->done);

- match_data = of_device_get_match_data(&pdev->dev);
+ match_data = of_device_get_match_data(dev);
if (!match_data)
return dev_err_probe(dev, -ENODEV, "failed to get match data\n");

@@ -1198,29 +1194,25 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);

- priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
- priv->param->regmap_config);
+ priv->regmap = devm_regmap_init_mmio(dev, base, priv->param->regmap_config);
if (IS_ERR(priv->regmap))
return PTR_ERR(priv->regmap);

- irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+ irq = irq_of_parse_and_map(dev->of_node, 0);
if (!irq)
return -EINVAL;

- ret = devm_request_irq(&pdev->dev, irq, meson_sar_adc_irq, IRQF_SHARED,
- dev_name(&pdev->dev), indio_dev);
+ ret = devm_request_irq(dev, irq, meson_sar_adc_irq, IRQF_SHARED, dev_name(dev), indio_dev);
if (ret)
return ret;

- priv->clkin = devm_clk_get(&pdev->dev, "clkin");
+ priv->clkin = devm_clk_get(dev, "clkin");
if (IS_ERR(priv->clkin))
- return dev_err_probe(&pdev->dev, PTR_ERR(priv->clkin),
- "failed to get clkin\n");
+ return dev_err_probe(dev, PTR_ERR(priv->clkin), "failed to get clkin\n");

- priv->core_clk = devm_clk_get(&pdev->dev, "core");
+ priv->core_clk = devm_clk_get(dev, "core");
if (IS_ERR(priv->core_clk))
- return dev_err_probe(&pdev->dev, PTR_ERR(priv->core_clk),
- "failed to get core clk\n");
+ return dev_err_probe(dev, PTR_ERR(priv->core_clk), "failed to get core clk\n");

priv->adc_clk = devm_clk_get_optional(dev, "adc_clk");
if (IS_ERR(priv->adc_clk))
@@ -1237,10 +1229,9 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
return ret;
}

- priv->vref = devm_regulator_get(&pdev->dev, "vref");
+ priv->vref = devm_regulator_get(dev, "vref");
if (IS_ERR(priv->vref))
- return dev_err_probe(&pdev->dev, PTR_ERR(priv->vref),
- "failed to get vref regulator\n");
+ return dev_err_probe(dev, PTR_ERR(priv->vref), "failed to get vref regulator\n");

priv->calibscale = MILLION;

@@ -1270,7 +1261,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)

ret = meson_sar_adc_calib(indio_dev);
if (ret)
- dev_warn(&pdev->dev, "calibration failed\n");
+ dev_warn(dev, "calibration failed\n");

platform_set_drvdata(pdev, indio_dev);

--
2.35.1