On Tue, 17 Oct 2023 11:10:54 +0000 Billy Tsai <billy_tsai@xxxxxxxxxxxxxx> wrote: > > > > > > > Create event sysfs for applying the deglitch condition. When > > > > > > > in_voltageY_thresh_rising_en/in_voltageY_thresh_falling_en is set to true, > > > > > > > the driver will use the in_voltageY_thresh_rising_value and > > > > > > > in_voltageY_thresh_falling_value as threshold values. If the ADC value > > > > > > > falls outside this threshold, the driver will wait for the ADC sampling > > > > > > > period and perform an additional read once to achieve the deglitching > > > > > > > purpose. > > > > > > > > > > > > > > Signed-off-by: Billy Tsai <billy_tsai@xxxxxxxxxxxxxx> > > > > > > > > > > > Hi Billy > > > > > > > > > > > This is pushing the meaning of the events interface too far. > > > > > > You can't use it to hide a value you don't like from userspace. > > > > > > > > > > > If you can explain what the condition is that you are seeing > > > > > > and what you need to prevent happening if it is seen that would help > > > > > > us figure out if there is another way to do this. > > > > > > > > > > > Jonathan > > > > > > > > > > Hi Jonathan, > > > > > > > > > > Currently, we are experiencing some voltage glitches while reading from our > > > > > controller, but we do not wish to report these false alarms to the user space. > > > > > Instead, we want to retry the operation as soon as possible. This is why the > > > > > driver requires this patch to handle retries internally, rather than relying on user > > > > > space which could introduce unpredictable timing for retrying the reading process. > > > > > This software approach aims to minimize the possibility of false alarms as much as possible. > > > > > > > Thanks for the extra detail. Perhaps share more of that in the cover letter for v2. > > > > > > Okay, I will incorporate more details into my commit message for v2. > > > > > > > > > > > > > If you have any suggestions or recommendations regarding this situation, please feel free to > > > > > share them with me. > > > > > > > Why do you need userspace control for the thresholds? > > > > Perhaps this is something that belongs in DT for a particular board design? > > > > > > If the input voltage remains constant, these settings can be incorporated into the DTS properties for configuring the threshold. However, if the input voltage is subject to change, adding user-space control may offer more flexibility. > > > My concern is that it's an interface userspace probably won't know how to use, or > > will misuse given this seems to be papering over bad hardware. > > > If there is a 'safe' value to put in DT I'd prefer to see that. I guess it might be per > > channel thing to adjust for different expected voltage ranges? > > Yes, the voltage ranges should be adjusted based on each individual channel. > I'm not entirely sure what you mean by the term 'safe' value. > Are you suggesting that a DTS property should be used to constrain the threshold ranges? Or someting else? > From my point of view, I think misusing this feature will only increase the sampling period of the ADC values and decrease the sensitivity. > As the name of this feature , it is primarily used to eliminate or mitigate glitches in readings. > Note a good choice of wording from me. I meant, for a given design (and hence DTS) is there a reasonable value that will work for all instances of that design? If there is it is a characteristic of the board design and should be in DT. If there isn't, how do you calibrate it for individual devices? Jonathan > Thanks > > >
