RE: [RFC PATCH 1/1] iio: dac: add support for ltc2688

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Friday, November 12, 2021 4:23 PM
> To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx>
> Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>; linux-iio
> <linux-iio@xxxxxxxxxxxxxxx>
> Subject: Re: [RFC PATCH 1/1] iio: dac: add support for ltc2688
> 
> [External]
> 
> 
> > >
> > > > > > +               chan->overrange =
> fwnode_property_read_bool(child,
> > > > > > +                                                           "adi,overrange");
> > > > >
> > > > > One line?
> > > >
> > > > It will pass the 80 col limit. AFAIR, Jonathan prefers to keep it
> when it
> > > > does not hurt readability...
> > >
> > > I believe it will increase readability being located on one line.
> >
> > I mean, this is perfectly aligned with the open "(", so it's a pretty
> > normal pattern. Anyways, I'm more than happy to move this into a
> one
> > liner and just use the 100 limit. But let's see what Jonathan has to say
> > because I do not want to move back and forward...
> 
> Here it happens to be particularly ugly because of the short first
> parameter,
> so I'm fine with a longer line for this one.
> 

Ok then...

> 
> > > > > > +       st->regmap = devm_regmap_init(&spi->dev, NULL, st,
> > > > > &ltc2688_regmap_config);
> > > > >
> > > > > I'm wondering why it's not a regmap SPI?
> > > >
> > > > The problem is on the read side... In the first transfer we write
> the
> > > command/register
> > > > to read, then we need to release the CS pin so that the device
> > > executes the command,
> > > > and only then we read the data. AFAIK, the regmap spi
> > > implementation won't work like
> > > > this. I think CS is kept asserted the whole time...
> > >
> > > I believe it's configurable, no? Like the cs_change flag somewhere.
> > > Can you double check?
> >
> > Don't think we can... The read part just calls:
> >
> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/s
> ource/drivers/base/regmap/regmap-
> spi.c*L98__;Iw!!A3Ni8CS0y2Y!orMauLgKQzcdnc3Mh7DBQY9nGq9sgf8jl
> KRBR8IOHhKMT2tdkLj9AyvIHDPYmA$
> > and has no control over the spi transfer bits...
> 
> Feature to add then or a custom regmap_bus if you want to keep it in
> the driver.

Hmm I see what you mean with the custom bus. We can actually make
use of more regmap infrastructure if I just define my regmap_bus in 
the driver. Thanks for the tip!

- Nuno Sá





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux