> -----Original Message----- > From: Sa, Nuno <Nuno.Sa@xxxxxxxxxx> > Sent: Tuesday, November 30, 2021 3:43 PM > To: Jonathan Cameron <jic23@xxxxxxxxxx> > Cc: linux-iio@xxxxxxxxxxxxxxx > Subject: RE: [RFC PATCH 0/1] LTC2688 support > > [External] > > > > > -----Original Message----- > > From: Jonathan Cameron <jic23@xxxxxxxxxx> > > Sent: Sunday, November 21, 2021 1:18 PM > > To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx> > > Cc: linux-iio@xxxxxxxxxxxxxxx > > Subject: Re: [RFC PATCH 0/1] LTC2688 support > > > > [External] > > > > On Mon, 15 Nov 2021 10:28:51 +0000 > > "Sa, Nuno" <Nuno.Sa@xxxxxxxxxx> wrote: > > > > > Hi Jonathan, > > > > > > Thanks for your inputs... > > > > > > > From: Jonathan Cameron <jic23@xxxxxxxxxx> > > > > Sent: Friday, November 12, 2021 5:15 PM > > > > To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx> > > > > Cc: linux-iio@xxxxxxxxxxxxxxx > > > > Subject: Re: [RFC PATCH 0/1] LTC2688 support > > > > > > > > [External] > > > > > > > > On Thu, 11 Nov 2021 12:00:42 +0100 > > > > Nuno Sá <nuno.sa@xxxxxxxxxx> wrote: > > > > > > > > Hi Nuno, > > > > > > > > > The reason why this is a RFC is because I'm still waiting for > proper > > HW > > > > > to test this thing. The reason why I'm sending this already is > > because > > > > > there's some non usual features which might require extra ABI. > > > > Hence, > > > > > while waiting I thought we could already speed up the process > in > > > > regards > > > > > with the ABI. > > > > > > > > Wise move as this is an unusual beast :) > > > > > > > > > > > > > > I still pushed the driver but the intent here is not really to > review > > it. > > > > > Naturally, if someone already wants to give some feedback, > > that's > > > > very > > > > > much appreciated :) > > > > > > > > > > > > > > Now, there are three main interfaces depending on the > channel > > > > mode: > > > > > 1) default (no new ABI) > > > > > 2) toggle mode > > > > > 3) dither mode > > > > > > > > > > The channel mode will be a devicetree property as it does not > > really > > > > > make much sense to change between modes at runtime even > > more > > > > because the > > > > > piece of HW we might want to control with a channel might be > > > > different > > > > > depending on the selected mode (I'm fairly sure on this > between > > > > toggle > > > > > and other modes but not so sure between dither and default > > mode). > > > > > > > > I agree on toggle vs dither definitely being different, but normal > > you > > > > could implement either as software toggle, or dither with a 0 > > > > magnitude > > > > sine wave. So might not be worth implementing default mode at > > all. > > > > No harm in doing so though if there are advantages to having it. > > > > > > My feeling is that we could probably have dither as the "default > > mode". > > > More on this below... > > > > > > > > > > > > > toggle mode special ABI: > > > > > > > > > > * Toggle operation enables fast switching of a DAC output > > between > > > > two > > > > > different DAC codes without any SPI transaction. Two codes are > > set > > > > in > > > > > input_a and input_b and then the output switches according to > > an > > > > input > > > > > signal (input_a -> clk high; input_b -> clk low). > > > > > > > > > > out_voltageY_input_register > > > > > - selects between input_a and input_b. After selecting one > > register, > > > > we > > > > > can write the dac code in out_voltageY_raw. > > > > > out_voltageY_toggle_en > > > > > - Disable/Enable toggle mode. The reason why I think this one > is > > > > > important is because if we wanna change the 2 codes, we > > should > > > > first > > > > > disable this, set the codes and only then enable the mode > > back... > > > > > As I'm writing this, It kind of came to me that we can probably > > > > > achieve this with out_voltageY_powerdown attr (maybe > takes a > > bit > > > > more > > > > > time to see the outputs but...) > > > > > > > > Hmm. These corner cases always take a bit of figuring out. What > > you > > > > have > > > > here is a bit 'device specific' for my liking. > > > > > > > > So there is precedence for ABI in this area, on the frequency > > devices > > > > but only > > > > for devices we still haven't moved out of staging. For those we > > > > needed a means > > > > to define selectable phases for PSK - where the selection was > > either > > > > software or, > > > > much like here, a selection pin. > > > > > > > > out_altvotage0_phase0 etc > > > > > > > > so I guess the equivalent here would be > > > > out_voltageY_raw0 > > > > out_voltageY_raw1 > > > > and the selection would be via something like > > > > out_voltageY_symbol (0 or 1 in this case). - note this is only > > > > relevant if you have the software toggle. Any enable needed for > > > > setting > > > > can be done as part of the write sequence for the raw0 / raw1 > and > > > > should > > > > ideally not be exposed to userspace (controls that require > manual > > > > sequencing > > > > tend to be hard to use / document). > > > > > > Yeah, I thought about that. I was even thinking in having something > > like > > > *_amplitude for dither mode. In some cases, where we might be > left > > > in some non obvious state (eg: moved the select register to input b > > and > > > then we failed to set the code;), I prefer to leave things as flexible > as > > > possible for userspace. But I agree it adds up more complexity and > in > > > this case, I can just always go to 'input_a' when writing 'raw0'... > > > > > > > However, I'm not 100% sure it really maps to this case. What do > > you > > > > think? > > > > > > I think it can work. Though out_voltageY_symbol probably needs to > > be > > > shared by type to be coherent with what we might have with > TGPx. > > > > That's fine. > > > > > Note the sw_toggle register has a bit mask of the channels we > want > > to > > > toggle which means we can toggle multiple channels at the same > > time. > > > > Using that wired up to buffer mode might make sense. You'd > provide > > multiple > > buffers and allow channels to be selected into one of them at a time. > > Each > > buffer is then tied to a different toggle (TGP0, TGP1, etc) > > > > The same could be true for the software toggle. It'll get a bit fiddly > > though. > > > > Perhaps this is an advanced feature to think about later... > > > > > It works the same with TGPx if you map multiple channels to the > > same > > > pin. > > > > > > There's also the question on how to handle this if a TGPx is > provided? > > > I guess it will just override the pin... But most likely having them > both > > > at the same time will lead to non desired results (unless we have a > > > way to gate/ungate the clocks)... > > I don't follow this bit. You mean TGPx and software toggle. As far as I > > can > > tell it's an either/or thing per channel. > > > > Here I meant that if we have a TGPx pin bundled to some channel(s) > we > do not want to also dance with the sw_toggle bit of that channel. Just a note on this. After starting my tests with the device, I can actually say that if we have a TGPx set in the channel settings register, the device will pretty much ignore the sw_toggle settings for that channel. I could see that the output voltage was not toggling at all. As soon as I removed the TGPx setting, then dancing with the sw_toggle started to work. So, for the HW this is not really an issue as it just ignores the sw_toggle. On a SW perspective, I'm still not sure if I just ignore this and write whatever the user sets or if I return some error code in the case a user tries to toggle a channel with a binded TGPx. The first one is appealing as it makes the code much simpler while OTHO it might make sense to be verbose here otherwise the user might think something is happening when it isn't... Anyways, I would argue that if someone has a pin wired, it's highly unlikely that he cares about sw_toggling... - Nuno Sá
