diff for duplicates of <20181028145515.03c03d5b@archlinux> diff --git a/a/1.txt b/N1/1.txt index 910776a..9c38e99 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -2,21 +2,20 @@ On Sat, 27 Oct 2018 17:49:03 +0200 Slawomir Stepien <sst@poczta.fm> wrote: > Hi ->=20 -> On pa=C5=BA 26, 2018 18:55, Nishad Kamdar wrote: +> +> 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. =20 ->=20 +> > from device tree. +> > So maybe you should make 2 commits? ->=20 +> > > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com> > > --- > > drivers/staging/iio/resolver/ad2s1210.c | 43 ++++++++++++++++++++++++- > > 1 file changed, 42 insertions(+), 1 deletion(-) -> >=20 -> > diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/= -iio/resolver/ad2s1210.c +> > +> > 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 @@ -25,33 +24,29 @@ iio/resolver/ad2s1210.c > > #include <linux/gpio/consumer.h> > > #include <linux/module.h> > > +#include <linux/of.h> -> > =20 +> > > > #include <linux/iio/iio.h> > > #include <linux/iio/sysfs.h> -> > @@ -669,6 +670,27 @@ static int ad2s1210_setup_gpios(struct ad2s1210_st= -ate *st) +> > @@ -669,6 +670,27 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st) > > return 0; > > } -> > =20 +> > > > +#ifdef CONFIG_OF -> > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device = -*dev) +> > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev) > > +{ -> > + struct device_node *np =3D dev->of_node; +> > + struct device_node *np = dev->of_node; > > + struct ad2s1210_platform_data *pdata; > > + -> > + pdata =3D devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); +> > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > > + if (!pdata) > > + return NULL; > > + -> > + pdata->gpioin =3D of_property_read_bool(np, "adi,gpioin"); =20 ->=20 -> Why here you are adding "adi", but you are not adding it to the gpios in = -prev +> > + 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? ->=20 -> I've also seen this: https://patchwork.kernel.org/patch/10355839/. Howeve= -r I do +> +> I've also seen this: https://patchwork.kernel.org/patch/10355839/. However I do > not understand why adding vendor id to props is needed... This is not standard ABI, hence we define it as being under the scope of a particular manufacturer (and hope they use it consistently ;) @@ -66,13 +61,12 @@ use the same name for different things). Jonathan ->=20 +> > > + > > + return pdata; > > +} > > +#else -> > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device = -*dev) +> > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev) > > +{ > > + return NULL; > > +} @@ -84,48 +78,47 @@ Jonathan > > @@ -682,7 +704,19 @@ static int ad2s1210_probe(struct spi_device *spi) > > if (!indio_dev) > > return -ENOMEM; -> > st =3D iio_priv(indio_dev); -> > - st->pdata =3D spi->dev.platform_data; +> > st = iio_priv(indio_dev); +> > - st->pdata = spi->dev.platform_data; > > + if (spi->dev.of_node) { -> > + st->pdata =3D ad2s1210_parse_dt(&spi->dev); +> > + st->pdata = ad2s1210_parse_dt(&spi->dev); > > + if (!st->pdata) > > + return -EINVAL; > > + } else { -> > + st->pdata =3D spi->dev.platform_data; +> > + st->pdata = spi->dev.platform_data; > > + } > > + > > + if (!st->pdata) { > > + dev_err(&spi->dev, "ad2s1210: no platform data supplied\n"); > > + return -EINVAL; > > + } -> > + =20 ->=20 -> Why not just use only device-tree here? The ad2s1210_platform_data has no= -w only +> > + +> +> 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. ->=20 -> > ret =3D ad2s1210_setup_gpios(st); +> +> > ret = ad2s1210_setup_gpios(st); > > if (ret < 0) > > return ret; > > @@ -724,6 +758,12 @@ static int ad2s1210_remove(struct spi_device *spi) > > return 0; > > } -> > =20 -> > +static const struct of_device_id ad2s1210_of_match[] =3D { -> > + { .compatible =3D "adi,ad2s1210", }, +> > +> > +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[] =3D { +> > 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 =3D { -> > .driver =3D { -> > .name =3D DRV_NAME, -> > + .of_match_table =3D of_match_ptr(ad2s1210_of_match), +> > static struct spi_driver ad2s1210_driver = { +> > .driver = { +> > .name = DRV_NAME, +> > + .of_match_table = of_match_ptr(ad2s1210_of_match), > > }, -> > .probe =3D ad2s1210_probe, -> > .remove =3D ad2s1210_remove, =20 ->=20 +> > .probe = ad2s1210_probe, +> > .remove = ad2s1210_remove, +> diff --git a/a/content_digest b/N1/content_digest index 095bd44..267218f 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -20,21 +20,20 @@ "Slawomir Stepien <sst@poczta.fm> wrote:\n" "\n" "> Hi\n" - ">=20\n" - "> On pa=C5=BA 26, 2018 18:55, Nishad Kamdar wrote:\n" + "> \n" + "> On pa\305\272 26, 2018 18:55, Nishad Kamdar wrote:\n" "> > Add device tree table for matching vendor ID\n" "> > and support for retrieving platform data\n" - "> > from device tree. =20\n" - ">=20\n" + "> > from device tree. \n" + "> \n" "> So maybe you should make 2 commits?\n" - ">=20\n" + "> \n" "> > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>\n" "> > ---\n" "> > drivers/staging/iio/resolver/ad2s1210.c | 43 ++++++++++++++++++++++++-\n" "> > 1 file changed, 42 insertions(+), 1 deletion(-)\n" - "> >=20\n" - "> > diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/=\n" - "iio/resolver/ad2s1210.c\n" + "> > \n" + "> > diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c\n" "> > index 93c3c70ce62e..9fd5461c4ed0 100644\n" "> > --- a/drivers/staging/iio/resolver/ad2s1210.c\n" "> > +++ b/drivers/staging/iio/resolver/ad2s1210.c\n" @@ -43,33 +42,29 @@ "> > #include <linux/gpio/consumer.h>\n" "> > #include <linux/module.h>\n" "> > +#include <linux/of.h>\n" - "> > =20\n" + "> > \n" "> > #include <linux/iio/iio.h>\n" "> > #include <linux/iio/sysfs.h>\n" - "> > @@ -669,6 +670,27 @@ static int ad2s1210_setup_gpios(struct ad2s1210_st=\n" - "ate *st)\n" + "> > @@ -669,6 +670,27 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)\n" "> > \treturn 0;\n" "> > }\n" - "> > =20\n" + "> > \n" "> > +#ifdef CONFIG_OF\n" - "> > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device =\n" - "*dev)\n" + "> > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev)\n" "> > +{\n" - "> > +\tstruct device_node *np =3D dev->of_node;\n" + "> > +\tstruct device_node *np = dev->of_node;\n" "> > +\tstruct ad2s1210_platform_data *pdata;\n" "> > +\n" - "> > +\tpdata =3D devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);\n" + "> > +\tpdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);\n" "> > +\tif (!pdata)\n" "> > +\t\treturn NULL;\n" "> > +\n" - "> > +\tpdata->gpioin =3D of_property_read_bool(np, \"adi,gpioin\"); =20\n" - ">=20\n" - "> Why here you are adding \"adi\", but you are not adding it to the gpios in =\n" - "prev\n" + "> > +\tpdata->gpioin = of_property_read_bool(np, \"adi,gpioin\"); \n" + "> \n" + "> Why here you are adding \"adi\", but you are not adding it to the gpios in prev\n" "> commit?\n" - ">=20\n" - "> I've also seen this: https://patchwork.kernel.org/patch/10355839/. Howeve=\n" - "r I do\n" + "> \n" + "> I've also seen this: https://patchwork.kernel.org/patch/10355839/. However I do\n" "> not understand why adding vendor id to props is needed...\n" "This is not standard ABI, hence we define it as being under the scope of\n" "a particular manufacturer (and hope they use it consistently ;)\n" @@ -84,13 +79,12 @@ "\n" "Jonathan\n" "\n" - ">=20\n" + "> \n" "> > +\n" "> > +\treturn pdata;\n" "> > +}\n" "> > +#else\n" - "> > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device =\n" - "*dev)\n" + "> > +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev)\n" "> > +{\n" "> > +\treturn NULL;\n" "> > +}\n" @@ -102,50 +96,49 @@ "> > @@ -682,7 +704,19 @@ static int ad2s1210_probe(struct spi_device *spi)\n" "> > \tif (!indio_dev)\n" "> > \t\treturn -ENOMEM;\n" - "> > \tst =3D iio_priv(indio_dev);\n" - "> > -\tst->pdata =3D spi->dev.platform_data;\n" + "> > \tst = iio_priv(indio_dev);\n" + "> > -\tst->pdata = spi->dev.platform_data;\n" "> > +\tif (spi->dev.of_node) {\n" - "> > +\t\tst->pdata =3D ad2s1210_parse_dt(&spi->dev);\n" + "> > +\t\tst->pdata = ad2s1210_parse_dt(&spi->dev);\n" "> > +\t\tif (!st->pdata)\n" "> > +\t\t\treturn -EINVAL;\n" "> > +\t} else {\n" - "> > +\t\tst->pdata =3D spi->dev.platform_data;\n" + "> > +\t\tst->pdata = spi->dev.platform_data;\n" "> > +\t}\n" "> > +\n" "> > +\tif (!st->pdata) {\n" "> > +\t\tdev_err(&spi->dev, \"ad2s1210: no platform data supplied\\n\");\n" "> > +\t\treturn -EINVAL;\n" "> > +\t}\n" - "> > + =20\n" - ">=20\n" - "> Why not just use only device-tree here? The ad2s1210_platform_data has no=\n" - "w only\n" + "> > + \n" + "> \n" + "> Why not just use only device-tree here? The ad2s1210_platform_data has now only\n" "> one entry... In that case remember to add \"depends on OF\" in Kconfig.\n" - ">=20\n" - "> > \tret =3D ad2s1210_setup_gpios(st);\n" + "> \n" + "> > \tret = ad2s1210_setup_gpios(st);\n" "> > \tif (ret < 0)\n" "> > \t\treturn ret;\n" "> > @@ -724,6 +758,12 @@ static int ad2s1210_remove(struct spi_device *spi)\n" "> > \treturn 0;\n" "> > }\n" - "> > =20\n" - "> > +static const struct of_device_id ad2s1210_of_match[] =3D {\n" - "> > +\t{ .compatible =3D \"adi,ad2s1210\", },\n" + "> > \n" + "> > +static const struct of_device_id ad2s1210_of_match[] = {\n" + "> > +\t{ .compatible = \"adi,ad2s1210\", },\n" "> > +\t{ }\n" "> > +};\n" "> > +MODULE_DEVICE_TABLE(of, ad2s1210_of_match);\n" "> > +\n" - "> > static const struct spi_device_id ad2s1210_id[] =3D {\n" + "> > static const struct spi_device_id ad2s1210_id[] = {\n" "> > \t{ \"ad2s1210\" },\n" "> > \t{}\n" "> > @@ -733,6 +773,7 @@ MODULE_DEVICE_TABLE(spi, ad2s1210_id);\n" - "> > static struct spi_driver ad2s1210_driver =3D {\n" - "> > \t.driver =3D {\n" - "> > \t\t.name =3D DRV_NAME,\n" - "> > +\t\t.of_match_table =3D of_match_ptr(ad2s1210_of_match),\n" + "> > static struct spi_driver ad2s1210_driver = {\n" + "> > \t.driver = {\n" + "> > \t\t.name = DRV_NAME,\n" + "> > +\t\t.of_match_table = of_match_ptr(ad2s1210_of_match),\n" "> > \t},\n" - "> > \t.probe =3D ad2s1210_probe,\n" - "> > \t.remove =3D ad2s1210_remove, =20\n" - >=20 + "> > \t.probe = ad2s1210_probe,\n" + "> > \t.remove = ad2s1210_remove, \n" + > -5877e0eb0d9349893d740de0dc477e6cfea37a99f9a7c0563e0d231fd9a0ff7e +d9e2d619a29e4f5a1bce03b62cb0b0d0608be327d48888a62d2c16830a38b2f4
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.