On Mon, 8 Jan 2018 07:51:08 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@xxxxxxxxxx> wrote:
> ________________________________________
> From: Jonathan Cameron [jic23@xxxxxxxxxxxxxxxxxxxxx]
> Sent: Saturday, January 06, 2018 2:58 PM
> To: Ardelean, Alexandru
> Cc: linux-iio@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver
>
> On Fri, 5 Jan 2018 17:21:32 +0200
> <alexandru.ardelean@xxxxxxxxxx> wrote:
>
> > From: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> >
> Say what the patch does before going into details of why
> you have done it the way you have.
>
> Ack
One more thing, if you can find an email client (or configure current one)
to keep to indenting quotes with >
Makes it easier to tell what you wrote!
Jonathan
>
> > Admittedly, this will always use up to
> > sizeof(struct ad7192_platform_data) bytes (~12 bytes at most)
> > on the stack, during the probing of the device.
>
> >
> > But this will maintain most of the other
> > init code unchanged.
> >
>
> I'm hopeful that you are going to also do any remaining work necessary
> to get this out of staging once and for all?
>
> Yep.
> Once I get the gist of sending patches to the kernel.
> So far, I've only sent to other smaller projects ; I'll make an effort to learn the rules here.
>
> Anyhow, comments inline.
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> > ---
> > drivers/staging/iio/adc/ad7192.c | 27 +++++++++++++++++++++++++--
> > 1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > index cadfb96734ed..fca988330588 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -610,18 +610,41 @@ static const struct iio_chan_spec ad7193_channels[] = {
> > IIO_CHAN_SOFT_TIMESTAMP(14),
> > };
> >
> > +static void ad7192_parse_dt(struct device_node *np,
> > + struct ad7192_platform_data *pdata)
> > +{
> > + of_property_read_u16(np, "adi,reference-voltage-mv", &pdata->vref_mv);
> > + of_property_read_u8(np, "adi,clock-source-select", &pdata->clock_source_sel);
> > + of_property_read_u32(np, "adi,external-clock-Hz", &pdata->ext_clk_hz);
> > + pdata->refin2_en = of_property_read_bool(np, "adi,refin2-pins-enable");
> > + pdata->rej60_en = of_property_read_bool(np, "adi,rejection-60-Hz-enable");
> > + pdata->sinc3_en = of_property_read_bool(np, "adi,sinc3-filter-enable");
> > + pdata->chop_en = of_property_read_bool(np, "adi,chop-enable");
> > + pdata->buf_en = of_property_read_bool(np, "adi,buffer-enable");
> > + pdata->unipolar_en = of_property_read_bool(np, "adi,unipolar-enable");
> > + pdata->burnout_curr_en = of_property_read_bool(np, "adi,burnout-currents-enable");
>
> Error checking? No particular reason a devicetree must be right and have all of
> these.
>
> Ack.
>
> > +}
> > +
> > static int ad7192_probe(struct spi_device *spi)
> > {
> > const struct ad7192_platform_data *pdata = dev_get_platdata(&spi->dev);
> > + const struct device_node *of_node = dev_of_node(&spi->dev);
> > + struct ad7192_platform_data lpdata;
> > struct ad7192_state *st;
> > struct iio_dev *indio_dev;
> > int ret, voltage_uv = 0;
> >
> > - if (!pdata) {
> > - dev_err(&spi->dev, "no platform data?\n");
> > + if (!pdata && !of_node) {
> > + dev_err(&spi->dev, "no platform data or device tree configuration?\n");
> > return -ENODEV;
> > }
> >
> > + if (!pdata) {
> > + memset(&lpdata, 0, sizeof(lpdata));
> > + pdata = &lpdata;
> > + ad7192_parse_dt(of_node, pdata);
> > + }
> > +
> > if (!spi->irq) {
> > dev_err(&spi->dev, "no IRQ?\n");
> > return -ENODEV;
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
