Re: [PATCH 12/12] iio: accel: kionix-kx022a: Fix IPOL macro name
Jonathan Cameron <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <20260818030330.62158406@jic23-huawei> |
On Mon, 17 Aug 2026 14:43:18 +0300 Matti Vaittinen <[email protected]> wrote: > On 17/08/2026 04:54, Jonathan Cameron wrote: > > On Mon, 10 Aug 2026 10:55:25 +0300 > > Matti Vaittinen <[email protected]> wrote: > > > >> From: Matti Vaittinen <[email protected]> > >> > >> The "interrupt polarity high" -macro for KX022A variant is defined as: > >> However, the KX022A_MASK_IPOL1 is not defined anywhere, so actually > >> using the KX022A_IPOL_HIGH would produce a compile error. > >> > >> Fix the define by using correct mask. > >> > >> Signed-off-by: Matti Vaittinen <[email protected]> > >> Fixes: 7c1d1677b322 ("iio: accel: Support Kionix/ROHM KX022A accelerometer") > >> > >> --- > >> It appears the KX022A_IPOL_HIGH is unused as the IRQ is currently, > >> unconditionally set level low in kx022a_prepare_irq_pin(). The KX022A > >> hardware would support other configurations though. So, another > >> potential fix would be removing the define altogether. I decided to > >> leave the define there, as having it will allow users who need different > >> configuration to simply change the KX022A_IPOL_LOW to KX022A_IPOL_HIGH, > >> which is kind of a "easy to try" thing. > > > > Handling interrupt polarity options at the device end is typically a > > 'feature' rather than a must have given in most cases we don't have sharing > > going on and these days interrupt controllers can cope with most types > > of input. If anyone needs it they can propose support. > > I take this as a "please, drop the unused define rather than fix it". Nope. I'm fine with fixing the define being there (fixed). This was more of a comment on whether we should 'fix' whether we ignored the flag from firmware or not. > > > Note I've never been happy with how we do this when we do support it > > because the argument for letting DT set the polarity is there may be > > not gates or similar in the path so the driver can't know, yet if we > > read the type to set the polarity at the driver end we are saying we > > do know there aren't any such not gate (trick for level shifting on > > the cheap). I haven't figured out a better binding. > > I was more thinking of people who do downstream projects than a proper > upstream support including nice mechanism to select appropriate > polarity. It's quite easy to experiment (and carry a downstream patch) > with one-line define change. Especially so when defines are already there. I'd rather we support them properly if they actually need it. Ideal case they'd ask even for a downstream project and at that time we can add support - via the ugly path or maybe something better. Jonathan > > Yours, > -- Matti >