From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpo.poczta.interia.pl ([217.74.65.208]:57335 "EHLO smtpo.poczta.interia.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728305AbeJ1AaI (ORCPT ); Sat, 27 Oct 2018 20:30:08 -0400 Date: Sat, 27 Oct 2018 17:49:03 +0200 From: Slawomir Stepien To: Nishad Kamdar Cc: Lars-Peter Clausen , Michael Hennerich , Jonathan Cameron , Hartmut Knaack , Peter Meerwald-Stadler , Greg Kroah-Hartman , linux-iio@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 2/2] staging: iio: ad2s1210: Add device tree support. Message-ID: <20181027154903.GA1582@x220.localdomain> References: <1dc6fdf4626e7cf1ca4933fe3008cb3cd267149b.1540489369.git.nishadkamdar@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 In-Reply-To: <1dc6fdf4626e7cf1ca4933fe3008cb3cd267149b.1540489369.git.nishadkamdar@gmail.com> Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org Hi On paź 26, 2018 18:55, Nishad Kamdar wrote: > Add device tree table for matching vendor ID > and support for retrieving platform data > from device tree. So maybe you should make 2 commits? > Signed-off-by: Nishad Kamdar > --- > drivers/staging/iio/resolver/ad2s1210.c | 43 ++++++++++++++++++++++++- > 1 file changed, 42 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c > index 93c3c70ce62e..9fd5461c4ed0 100644 > --- a/drivers/staging/iio/resolver/ad2s1210.c > +++ b/drivers/staging/iio/resolver/ad2s1210.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -669,6 +670,27 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st) > return 0; > } > > +#ifdef CONFIG_OF > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev) > +{ > + struct device_node *np = dev->of_node; > + struct ad2s1210_platform_data *pdata; > + > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return NULL; > + > + pdata->gpioin = of_property_read_bool(np, "adi,gpioin"); Why here you are adding "adi", but you are not adding it to the gpios in prev commit? I've also seen this: https://patchwork.kernel.org/patch/10355839/. However I do not understand why adding vendor id to props is needed... > + > + return pdata; > +} > +#else > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev) > +{ > + return NULL; > +} > +#endif > + > static int ad2s1210_probe(struct spi_device *spi) > { > struct iio_dev *indio_dev; > @@ -682,7 +704,19 @@ static int ad2s1210_probe(struct spi_device *spi) > if (!indio_dev) > return -ENOMEM; > st = iio_priv(indio_dev); > - st->pdata = spi->dev.platform_data; > + if (spi->dev.of_node) { > + st->pdata = ad2s1210_parse_dt(&spi->dev); > + if (!st->pdata) > + return -EINVAL; > + } else { > + st->pdata = spi->dev.platform_data; > + } > + > + if (!st->pdata) { > + dev_err(&spi->dev, "ad2s1210: no platform data supplied\n"); > + return -EINVAL; > + } > + Why not just use only device-tree here? The ad2s1210_platform_data has now only one entry... In that case remember to add "depends on OF" in Kconfig. > ret = ad2s1210_setup_gpios(st); > if (ret < 0) > return ret; > @@ -724,6 +758,12 @@ static int ad2s1210_remove(struct spi_device *spi) > return 0; > } > > +static const struct of_device_id ad2s1210_of_match[] = { > + { .compatible = "adi,ad2s1210", }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ad2s1210_of_match); > + > static const struct spi_device_id ad2s1210_id[] = { > { "ad2s1210" }, > {} > @@ -733,6 +773,7 @@ MODULE_DEVICE_TABLE(spi, ad2s1210_id); > static struct spi_driver ad2s1210_driver = { > .driver = { > .name = DRV_NAME, > + .of_match_table = of_match_ptr(ad2s1210_of_match), > }, > .probe = ad2s1210_probe, > .remove = ad2s1210_remove, -- Slawomir Stepien