Re: [PATCH v2 2/2] input: gpio_keys_polled: Add support for abs/rel axis

[Hans de Goede <hdegoede@redhat.com>]

<I seem to have deleted the original mail without reply-ing, so from a threaded mailer pov this is a top-post, sorry>

Hi,

Dmitry Torokhov wrote:

 > > +	for_each_set_bit(i, input->relbit, REL_CNT) {
 > > +		if (!test_bit(i, bdev->rel_axis_seen))
 > > +			input_event(input, EV_REL, i, 0);
 > > +	}
 >
 > I wonder if this should be written as
 >
 >	for_each_set_bit(i, bdev->rel_axis_seen, REL_CNT)
 >		input_event(input, EV_REL, i, 0);
 >
 > i.e. the 2nd bit test is not really needed because we should not see
 > unsupported bits in "seen" axes.

Yes that makes sense, I'll make this change, re-test and post a new version.

 > > +		if (fwnode_property_read_u32(child, "linux,input-value",
 > > +					     (u32 *)&button->value))
 > > +			button->value = 1;
 >
 > Umm, we need negative values too... but there is no
 > fwnode_property_read_s32 :(. We need to document in the bindings that
 > value is treated as signed so that users can still achieve the needed
 > effect.

Right, I was looking how to deal with this, and the generic fwnode
interface has no s32 version, but the devicetree  linux/of.h code has:

static inline int of_property_read_s32(const struct device_node *np,
                                        const char *propname,
                                        s32 *out_value)
{
         return of_property_read_u32(np, propname, (u32*) out_value);
}

So this seemed like the best way to deal with this. You're right that
the devicetree binding docs should explicitly state that negative
numbers are allowed though, I will update the dt-bindings doc
patch accordingly.

Regards,

Hans