Le 05/09/2024 à 17:17, Angelo Dureghello a écrit :
From: Angelo Dureghello <adureghello-rdvid1DuHRBWk0Htik3J/w@xxxxxxxxxxxxxxxx>
Extend DAC backend with new features required for the AXI driver
version for the ad3552r DAC. Mainly, a new compatible string has
been added to support a DAC IP very similar to the generic DAC IP
but with some customizations to work with the ad3552r.
Then, a serie of generic functions has been added to match with
ad3552r needs. Function names has been kept generic as much as
possible, to allow re-utilization from other frontend drivers.
Hi,
...
+static int axi_dac_read_raw(struct iio_backend *back,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_FREQUENCY:
+ *val = clk_get_rate(devm_clk_get(st->dev, 0));
Having a devm_clk_get() in such a place is really unusual.
Is it correct?
This look like a memory leak to me.
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
...
+ /*
+ * Both REG_CNTRL_2 and AXI_DAC_CNTRL_DATA_WR need to know
+ * the data size. So keeping data size control here only,
+ * since data size is mandatory for to the current transfer.
"... for to ..." sounds strange to my *non*-native English ears.
+ * DDR state handled separately by specific backend calls,
+ * generally all raw register writes are SDR.
+ */
+ if (data_size == 1)
+ ret = regmap_set_bits(st->regmap, AXI_DAC_REG_CNTRL_2,
+ AXI_DAC_SYMB_8B);
+ else
+ ret = regmap_clear_bits(st->regmap, AXI_DAC_REG_CNTRL_2,
+ AXI_DAC_SYMB_8B);
+ if (ret)
+ return ret;
...
@@ -556,10 +792,12 @@ static int axi_dac_probe(struct platform_device *pdev)
if (!st)
return -ENOMEM;
- expected_ver = device_get_match_data(&pdev->dev);
- if (!expected_ver)
+ info = device_get_match_data(&pdev->dev);
+ if (!info)
writing:
st->info = device_get_match_data(&pdev->dev);
if (!st->info)
would save the 'info' variable and a few lines of code without loosing
(IMHO) readability.
CJ
return -ENODEV;
+ st->info = info;
+
clk = devm_clk_get_enabled(&pdev->dev, NULL);
if (IS_ERR(clk))
return dev_err_probe(&pdev->dev, PTR_ERR(clk),
@@ -588,12 +826,13 @@ static int axi_dac_probe(struct platform_device *pdev)
if (ret)
return ret;
- if (ADI_AXI_PCORE_VER_MAJOR(ver) != ADI_AXI_PCORE_VER_MAJOR(*expected_ver)) {
+ if (ADI_AXI_PCORE_VER_MAJOR(ver) !=
+ ADI_AXI_PCORE_VER_MAJOR(st->info->version)) {
dev_err(&pdev->dev,
"Major version mismatch. Expected %d.%.2d.%c, Reported %d.%.2d.%c\n",
- ADI_AXI_PCORE_VER_MAJOR(*expected_ver),
- ADI_AXI_PCORE_VER_MINOR(*expected_ver),
- ADI_AXI_PCORE_VER_PATCH(*expected_ver),
+ ADI_AXI_PCORE_VER_MAJOR(st->info->version),
+ ADI_AXI_PCORE_VER_MINOR(st->info->version),
+ ADI_AXI_PCORE_VER_PATCH(st->info->version),
ADI_AXI_PCORE_VER_MAJOR(ver),
ADI_AXI_PCORE_VER_MINOR(ver),
ADI_AXI_PCORE_VER_PATCH(ver));
@@ -631,10 +870,18 @@ static int axi_dac_probe(struct platform_device *pdev)
return 0;
}
...
