| Message ID | 20211205235948.4167075-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] hwmon: (ntc_thermistor): Inline OF data lookup in probe() | expand |
On 12/5/21 3:59 PM, Linus Walleij wrote: > There is no need to allocate the state container and look > up DT properties in a separate function if OF is all we > support. Inline it into probe(). > This is POV. Old fashioned as I am, I still prefer smaller functions over large ones. Is there a _technical_ reason for this change ? Guenter > Cc: Peter Rosin <peda@axentia.se> > Cc: Chris Lesiak <chris.lesiak@licor.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/hwmon/ntc_thermistor.c | 76 ++++++++++++---------------------- > 1 file changed, 27 insertions(+), 49 deletions(-) > > diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c > index ed638ebd0923..44abcb8a2393 100644 > --- a/drivers/hwmon/ntc_thermistor.c > +++ b/drivers/hwmon/ntc_thermistor.c > @@ -403,49 +403,6 @@ static const struct of_device_id ntc_match[] = { > }; > MODULE_DEVICE_TABLE(of, ntc_match); > > -static struct ntc_data *ntc_thermistor_parse_dt(struct device *dev) > -{ > - struct ntc_data *data; > - struct iio_channel *chan; > - enum iio_chan_type type; > - struct device_node *np = dev->of_node; > - int ret; > - > - if (!np) > - return NULL; > - > - data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > - if (!data) > - return ERR_PTR(-ENOMEM); > - > - chan = devm_iio_channel_get(dev, NULL); > - if (IS_ERR(chan)) > - return ERR_CAST(chan); > - > - ret = iio_get_channel_type(chan, &type); > - if (ret < 0) > - return ERR_PTR(ret); > - > - if (type != IIO_VOLTAGE) > - return ERR_PTR(-EINVAL); > - > - if (of_property_read_u32(np, "pullup-uv", &data->pullup_uv)) > - return ERR_PTR(-ENODEV); > - if (of_property_read_u32(np, "pullup-ohm", &data->pullup_ohm)) > - return ERR_PTR(-ENODEV); > - if (of_property_read_u32(np, "pulldown-ohm", &data->pulldown_ohm)) > - return ERR_PTR(-ENODEV); > - > - if (of_find_property(np, "connected-positive", NULL)) > - data->connect = NTC_CONNECTED_POSITIVE; > - else /* status change should be possible if not always on. */ > - data->connect = NTC_CONNECTED_GROUND; > - > - data->chan = chan; > - > - return data; > -} > - > static inline u64 div64_u64_safe(u64 dividend, u64 divisor) > { > if (divisor == 0 && dividend == 0) > @@ -641,20 +598,41 @@ static const struct hwmon_chip_info ntc_chip_info = { > static int ntc_thermistor_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > const struct of_device_id *of_id = > of_match_device(of_match_ptr(ntc_match), dev); > const struct platform_device_id *pdev_id; > struct device *hwmon_dev; > struct ntc_data *data; > + enum iio_chan_type type; > + int ret; > > - data = ntc_thermistor_parse_dt(dev); > - if (IS_ERR(data)) > - return PTR_ERR(data); > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->chan = devm_iio_channel_get(dev, NULL); > + if (IS_ERR(data->chan)) > + return PTR_ERR(data->chan); > + > + ret = iio_get_channel_type(data->chan, &type); > + if (ret < 0) > + return ret; > > - if (!data) { > - dev_err(dev, "No platform init data supplied.\n"); > + if (type != IIO_VOLTAGE) > + return -EINVAL; > + > + if (of_property_read_u32(np, "pullup-uv", &data->pullup_uv)) > return -ENODEV; > - } > + if (of_property_read_u32(np, "pullup-ohm", &data->pullup_ohm)) > + return -ENODEV; > + if (of_property_read_u32(np, "pulldown-ohm", &data->pulldown_ohm)) > + return -ENODEV; > + > + if (of_find_property(np, "connected-positive", NULL)) > + data->connect = NTC_CONNECTED_POSITIVE; > + else /* status change should be possible if not always on. */ > + data->connect = NTC_CONNECTED_GROUND; > > if (data->pullup_uv == 0 || > (data->pullup_ohm == 0 && data->connect == >
On Mon, Dec 6, 2021 at 1:38 AM Guenter Roeck <linux@roeck-us.net> wrote: > On 12/5/21 3:59 PM, Linus Walleij wrote: > > There is no need to allocate the state container and look > > up DT properties in a separate function if OF is all we > > support. Inline it into probe(). > > > > This is POV. Old fashioned as I am, I still prefer smaller > functions over large ones. Is there a _technical_ reason > for this change ? Mainly the *data state container should be allocated in probe() but I can keep the rest in a separate (renamed, moved) function, I can certainly respin it like such, just a minute! Yours, Linus Walleij
diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c index ed638ebd0923..44abcb8a2393 100644 --- a/drivers/hwmon/ntc_thermistor.c +++ b/drivers/hwmon/ntc_thermistor.c @@ -403,49 +403,6 @@ static const struct of_device_id ntc_match[] = { }; MODULE_DEVICE_TABLE(of, ntc_match); -static struct ntc_data *ntc_thermistor_parse_dt(struct device *dev) -{ - struct ntc_data *data; - struct iio_channel *chan; - enum iio_chan_type type; - struct device_node *np = dev->of_node; - int ret; - - if (!np) - return NULL; - - data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); - if (!data) - return ERR_PTR(-ENOMEM); - - chan = devm_iio_channel_get(dev, NULL); - if (IS_ERR(chan)) - return ERR_CAST(chan); - - ret = iio_get_channel_type(chan, &type); - if (ret < 0) - return ERR_PTR(ret); - - if (type != IIO_VOLTAGE) - return ERR_PTR(-EINVAL); - - if (of_property_read_u32(np, "pullup-uv", &data->pullup_uv)) - return ERR_PTR(-ENODEV); - if (of_property_read_u32(np, "pullup-ohm", &data->pullup_ohm)) - return ERR_PTR(-ENODEV); - if (of_property_read_u32(np, "pulldown-ohm", &data->pulldown_ohm)) - return ERR_PTR(-ENODEV); - - if (of_find_property(np, "connected-positive", NULL)) - data->connect = NTC_CONNECTED_POSITIVE; - else /* status change should be possible if not always on. */ - data->connect = NTC_CONNECTED_GROUND; - - data->chan = chan; - - return data; -} - static inline u64 div64_u64_safe(u64 dividend, u64 divisor) { if (divisor == 0 && dividend == 0) @@ -641,20 +598,41 @@ static const struct hwmon_chip_info ntc_chip_info = { static int ntc_thermistor_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; const struct of_device_id *of_id = of_match_device(of_match_ptr(ntc_match), dev); const struct platform_device_id *pdev_id; struct device *hwmon_dev; struct ntc_data *data; + enum iio_chan_type type; + int ret; - data = ntc_thermistor_parse_dt(dev); - if (IS_ERR(data)) - return PTR_ERR(data); + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->chan = devm_iio_channel_get(dev, NULL); + if (IS_ERR(data->chan)) + return PTR_ERR(data->chan); + + ret = iio_get_channel_type(data->chan, &type); + if (ret < 0) + return ret; - if (!data) { - dev_err(dev, "No platform init data supplied.\n"); + if (type != IIO_VOLTAGE) + return -EINVAL; + + if (of_property_read_u32(np, "pullup-uv", &data->pullup_uv)) return -ENODEV; - } + if (of_property_read_u32(np, "pullup-ohm", &data->pullup_ohm)) + return -ENODEV; + if (of_property_read_u32(np, "pulldown-ohm", &data->pulldown_ohm)) + return -ENODEV; + + if (of_find_property(np, "connected-positive", NULL)) + data->connect = NTC_CONNECTED_POSITIVE; + else /* status change should be possible if not always on. */ + data->connect = NTC_CONNECTED_GROUND; if (data->pullup_uv == 0 || (data->pullup_ohm == 0 && data->connect ==
There is no need to allocate the state container and look up DT properties in a separate function if OF is all we support. Inline it into probe(). Cc: Peter Rosin <peda@axentia.se> Cc: Chris Lesiak <chris.lesiak@licor.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/hwmon/ntc_thermistor.c | 76 ++++++++++++---------------------- 1 file changed, 27 insertions(+), 49 deletions(-)