Hi, On 11/30/22 12:06, Andy Shevchenko wrote: > On Wed, Nov 30, 2022 at 11:39:42AM +0100, Hans de Goede wrote: >> On 11/30/22 11:01, Andy Shevchenko wrote: >>> On Wed, Nov 30, 2022 at 1:12 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >>>> >>>> According to: >>>> https://github.com/intel/ipu6-drivers/blob/master/patch/int3472-support-independent-clock-and-LED-gpios-5.17%2B.patch >>>> >>>> Bits 31-24 of the _DSM pin entry integer value codes the active-value, >>>> that is the actual physical signal (0 or 1) which needs to be output on >>>> the pin to turn the sensor chip on (to make it active). >>>> >>>> So if bits 31-24 are 0 for a reset pin, then the actual value of the reset >>>> pin needs to be 0 to take the chip out of reset. IOW in this case the reset >>>> signal is active-high rather then the default active-low. >>>> >>>> And if bits 31-24 are 0 for a clk-en pin then the actual value of the clk >>>> pin needs to be 0 to enable the clk. So in this case the clk-en signal >>>> is active-low rather then the default active-high. >>>> >>>> IOW if bits 31-24 are 0 for a pin, then the default polarity of the pin >>>> is inverted. >>>> >>>> Add a check for this and also propagate this new polarity to the clock >>>> registration. >>> >>> I like it in this form, thanks! >>> >>> ... >>> >>>> + (polarity == GPIO_ACTIVE_HIGH) ? "high" : "low"); >>> >>> Perhaps >>> >>> static inline str_high_low(bool v) >>> { >>> return v ? "high" : "low"; >>> } >>> >>> In the string_helpers.h? If you are okay with the idea, you may use my >>> tag ahead for that patch. >> >> That is an interesting idea. but IMHO best done in a follow-up series >> after checking for similar code in the rest of the kernel. >> >> This is based on the assumption that "selling" this new helper to >> the string_helpers.h maintainer will be easier if there are multiple >> consumers of it right away. > > strings_helper.h maintainer is any known kernel subsystem maintainer + me > (as a reviewer / main contributor to that library). That's why I proposed > the solution and my tag would be enough to route it via your tree. > > So, offer still stays. Ok I'll add your patch adding the str_high_low() helper to the next version of this series (or when I merge it, depending on how things go) and then move this over to use the str_high_low() helper. Regards, Hans
