| Message ID | 20200702162509.11254-1-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFT] iio: adc: xilinx-xadc: use devm_krealloc() | expand |
On 2020-07-02 17:25, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Use the managed variant of krealloc() and shrink the code a bit. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > NOTE: this patch depends on the series adding devm_krealloc() which is > not yet accepted. > > Greg, > > here's just a quick example of a second user of devm_krealloc(). This time > we're reallocing memory allocated with devm_kmemdup(). Hopefully this is > enough to prove this helper is useful enough to merge it. Ooh, interesting - I also had to open-code an effective devm_krealloc() to subclass an devres-managed allocation in cavium_smmu_impl_init() (drivers/iommu/arm-smmu-impl.c), and we've got patches in-flight for another SMMU quirk that wants to follow the same pattern. I'd gladly have the extra clarity of a proper helper (and save a couple of lines) for those cases as well. Robin. > I can't test it due to lack of HW though. > > drivers/iio/adc/xilinx-xadc-core.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c > index d7fecab9252e..5bdbe502e983 100644 > --- a/drivers/iio/adc/xilinx-xadc-core.c > +++ b/drivers/iio/adc/xilinx-xadc-core.c > @@ -1094,6 +1094,7 @@ MODULE_DEVICE_TABLE(of, xadc_of_match_table); > static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, > unsigned int *conf) > { > + struct device *dev = indio_dev->dev.parent; > struct xadc *xadc = iio_priv(indio_dev); > struct iio_chan_spec *channels, *chan; > struct device_node *chan_node, *child; > @@ -1138,7 +1139,8 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, > *conf |= XADC_CONF0_MUX | XADC_CONF0_CHAN(ext_mux_chan); > } > > - channels = kmemdup(xadc_channels, sizeof(xadc_channels), GFP_KERNEL); > + channels = devm_kmemdup(dev, xadc_channels, > + sizeof(xadc_channels), GFP_KERNEL); > if (!channels) > return -ENOMEM; > > @@ -1174,8 +1176,9 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, > of_node_put(chan_node); > > indio_dev->num_channels = num_channels; > - indio_dev->channels = krealloc(channels, sizeof(*channels) * > - num_channels, GFP_KERNEL); > + indio_dev->channels = devm_krealloc(dev, channels, > + sizeof(*channels) * num_channels, > + GFP_KERNEL); > /* If we can't resize the channels array, just use the original */ > if (!indio_dev->channels) > indio_dev->channels = channels; > @@ -1229,14 +1232,14 @@ static int xadc_probe(struct platform_device *pdev) > > ret = xadc_parse_dt(indio_dev, pdev->dev.of_node, &conf0); > if (ret) > - goto err_device_free; > + return ret; > > if (xadc->ops->flags & XADC_FLAGS_BUFFERED) { > ret = iio_triggered_buffer_setup(indio_dev, > &iio_pollfunc_store_time, &xadc_trigger_handler, > &xadc_buffer_ops); > if (ret) > - goto err_device_free; > + return ret; > > xadc->convst_trigger = xadc_alloc_trigger(indio_dev, "convst"); > if (IS_ERR(xadc->convst_trigger)) { > @@ -1354,8 +1357,6 @@ static int xadc_probe(struct platform_device *pdev) > err_triggered_buffer_cleanup: > if (xadc->ops->flags & XADC_FLAGS_BUFFERED) > iio_triggered_buffer_cleanup(indio_dev); > -err_device_free: > - kfree(indio_dev->channels); > > return ret; > } > @@ -1375,7 +1376,6 @@ static int xadc_remove(struct platform_device *pdev) > cancel_delayed_work_sync(&xadc->zynq_unmask_work); > clk_disable_unprepare(xadc->clk); > kfree(xadc->data); > - kfree(indio_dev->channels); > > return 0; > } >
Hi Bartosz,
I love your patch! Yet something to improve:
[auto build test ERROR on iio/togreg]
[also build test ERROR on staging/staging-testing v5.8-rc3 next-20200702]
[cannot apply to xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Bartosz-Golaszewski/iio-adc-xilinx-xadc-use-devm_krealloc/20200703-002747
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: i386-randconfig-s002-20200702 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-3-gfa153962-dirty
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
drivers/iio/adc/xilinx-xadc-core.c: In function 'xadc_parse_dt':
>> drivers/iio/adc/xilinx-xadc-core.c:1179:24: error: implicit declaration of function 'devm_krealloc'; did you mean 'devm_kcalloc'? [-Werror=implicit-function-declaration]
1179 | indio_dev->channels = devm_krealloc(dev, channels,
| ^~~~~~~~~~~~~
| devm_kcalloc
>> drivers/iio/adc/xilinx-xadc-core.c:1179:22: warning: assignment to 'const struct iio_chan_spec *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
1179 | indio_dev->channels = devm_krealloc(dev, channels,
| ^
cc1: some warnings being treated as errors
vim +1179 drivers/iio/adc/xilinx-xadc-core.c
1093
1094 static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np,
1095 unsigned int *conf)
1096 {
1097 struct device *dev = indio_dev->dev.parent;
1098 struct xadc *xadc = iio_priv(indio_dev);
1099 struct iio_chan_spec *channels, *chan;
1100 struct device_node *chan_node, *child;
1101 unsigned int num_channels;
1102 const char *external_mux;
1103 u32 ext_mux_chan;
1104 u32 reg;
1105 int ret;
1106
1107 *conf = 0;
1108
1109 ret = of_property_read_string(np, "xlnx,external-mux", &external_mux);
1110 if (ret < 0 || strcasecmp(external_mux, "none") == 0)
1111 xadc->external_mux_mode = XADC_EXTERNAL_MUX_NONE;
1112 else if (strcasecmp(external_mux, "single") == 0)
1113 xadc->external_mux_mode = XADC_EXTERNAL_MUX_SINGLE;
1114 else if (strcasecmp(external_mux, "dual") == 0)
1115 xadc->external_mux_mode = XADC_EXTERNAL_MUX_DUAL;
1116 else
1117 return -EINVAL;
1118
1119 if (xadc->external_mux_mode != XADC_EXTERNAL_MUX_NONE) {
1120 ret = of_property_read_u32(np, "xlnx,external-mux-channel",
1121 &ext_mux_chan);
1122 if (ret < 0)
1123 return ret;
1124
1125 if (xadc->external_mux_mode == XADC_EXTERNAL_MUX_SINGLE) {
1126 if (ext_mux_chan == 0)
1127 ext_mux_chan = XADC_REG_VPVN;
1128 else if (ext_mux_chan <= 16)
1129 ext_mux_chan = XADC_REG_VAUX(ext_mux_chan - 1);
1130 else
1131 return -EINVAL;
1132 } else {
1133 if (ext_mux_chan > 0 && ext_mux_chan <= 8)
1134 ext_mux_chan = XADC_REG_VAUX(ext_mux_chan - 1);
1135 else
1136 return -EINVAL;
1137 }
1138
1139 *conf |= XADC_CONF0_MUX | XADC_CONF0_CHAN(ext_mux_chan);
1140 }
1141
1142 channels = devm_kmemdup(dev, xadc_channels,
1143 sizeof(xadc_channels), GFP_KERNEL);
1144 if (!channels)
1145 return -ENOMEM;
1146
1147 num_channels = 9;
1148 chan = &channels[9];
1149
1150 chan_node = of_get_child_by_name(np, "xlnx,channels");
1151 if (chan_node) {
1152 for_each_child_of_node(chan_node, child) {
1153 if (num_channels >= ARRAY_SIZE(xadc_channels)) {
1154 of_node_put(child);
1155 break;
1156 }
1157
1158 ret = of_property_read_u32(child, "reg", ®);
1159 if (ret || reg > 16)
1160 continue;
1161
1162 if (of_property_read_bool(child, "xlnx,bipolar"))
1163 chan->scan_type.sign = 's';
1164
1165 if (reg == 0) {
1166 chan->scan_index = 11;
1167 chan->address = XADC_REG_VPVN;
1168 } else {
1169 chan->scan_index = 15 + reg;
1170 chan->address = XADC_REG_VAUX(reg - 1);
1171 }
1172 num_channels++;
1173 chan++;
1174 }
1175 }
1176 of_node_put(chan_node);
1177
1178 indio_dev->num_channels = num_channels;
> 1179 indio_dev->channels = devm_krealloc(dev, channels,
1180 sizeof(*channels) * num_channels,
1181 GFP_KERNEL);
1182 /* If we can't resize the channels array, just use the original */
1183 if (!indio_dev->channels)
1184 indio_dev->channels = channels;
1185
1186 return 0;
1187 }
1188
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c index d7fecab9252e..5bdbe502e983 100644 --- a/drivers/iio/adc/xilinx-xadc-core.c +++ b/drivers/iio/adc/xilinx-xadc-core.c @@ -1094,6 +1094,7 @@ MODULE_DEVICE_TABLE(of, xadc_of_match_table); static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, unsigned int *conf) { + struct device *dev = indio_dev->dev.parent; struct xadc *xadc = iio_priv(indio_dev); struct iio_chan_spec *channels, *chan; struct device_node *chan_node, *child; @@ -1138,7 +1139,8 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, *conf |= XADC_CONF0_MUX | XADC_CONF0_CHAN(ext_mux_chan); } - channels = kmemdup(xadc_channels, sizeof(xadc_channels), GFP_KERNEL); + channels = devm_kmemdup(dev, xadc_channels, + sizeof(xadc_channels), GFP_KERNEL); if (!channels) return -ENOMEM; @@ -1174,8 +1176,9 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, of_node_put(chan_node); indio_dev->num_channels = num_channels; - indio_dev->channels = krealloc(channels, sizeof(*channels) * - num_channels, GFP_KERNEL); + indio_dev->channels = devm_krealloc(dev, channels, + sizeof(*channels) * num_channels, + GFP_KERNEL); /* If we can't resize the channels array, just use the original */ if (!indio_dev->channels) indio_dev->channels = channels; @@ -1229,14 +1232,14 @@ static int xadc_probe(struct platform_device *pdev) ret = xadc_parse_dt(indio_dev, pdev->dev.of_node, &conf0); if (ret) - goto err_device_free; + return ret; if (xadc->ops->flags & XADC_FLAGS_BUFFERED) { ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time, &xadc_trigger_handler, &xadc_buffer_ops); if (ret) - goto err_device_free; + return ret; xadc->convst_trigger = xadc_alloc_trigger(indio_dev, "convst"); if (IS_ERR(xadc->convst_trigger)) { @@ -1354,8 +1357,6 @@ static int xadc_probe(struct platform_device *pdev) err_triggered_buffer_cleanup: if (xadc->ops->flags & XADC_FLAGS_BUFFERED) iio_triggered_buffer_cleanup(indio_dev); -err_device_free: - kfree(indio_dev->channels); return ret; } @@ -1375,7 +1376,6 @@ static int xadc_remove(struct platform_device *pdev) cancel_delayed_work_sync(&xadc->zynq_unmask_work); clk_disable_unprepare(xadc->clk); kfree(xadc->data); - kfree(indio_dev->channels); return 0; }