Re: [PATCH 3/3] max7359_keypad: implement DT bindings

[Dmitry Torokhov]

On Sun, May 17, 2015 at 07:55:02PM +0300, Evgeniy Dushistov wrote:
> On Fri, May 15, 2015 at 11:00:02PM +0200, Dmitry Torokhov wrote:
> > On Thu, May 14, 2015 at 05:38:03AM +0300, Evgeniy Dushistov wrote:
> > > +		maxim,debounce_reg = /bits/ 8 <0x5F>;
> > > +		maxim,ports_reg = /bits/ 8 <0xFF>;
> > 
> > Specifying exact size for properties is quite uncommon; I think the
> > usual recommendation is is to use the "standard" u32 and validate the
> > range in parser function.
> > 
> 
> Using of u8 has advantages, it is possible on compilation stage
> (dts->dtb) check that you enter right value, but
> because of DT validation tools are not part of mainline,
> I replace u8 with u32 as you suggest, see patch bellow.

If you get DT folks to OK using /bits/ - that's fine. Grant, Rob, any
guidance here?

> 
> > > +		MATRIX_KEY(0, 7, KEY_RESERVED)
> > > +		...
> > 
> > 
> > Indent one more level? Also, maybe fill with something other than
> > KEY_RESERVED?
> > 
> 
> Fixed, see patch bellow.
> 
> > 
> > 
>  
> > >  	dev_dbg(&client->dev, "keys FIFO is 0x%02x\n", ret);
> > > +	if (!keymap_data) {
> > > +		error = max7359_parse_dt(&client->dev, &init_state);
> > > +		if (error) {
> > > +			dev_err(&client->dev, "platform data null, and no DT data\n");
> > > +			return error;
> > 
> > Both debounce and ports values are optional and we'll fail building
> > keymap if there are no platform data nor device tree data, so I would
> > drop this check and the stub for max7359_parse_dt() as well - we have
> > stubs for property parsing anyway.
> > 
> 
> Because of u32/u8 check at now max7359_parse_dt can return not 0.
> 
> > -- 
> > Dmitry
> > 
> 
> Thanks, for review. 
> 
> Please add me to CC, because of I'm not the part
> of any mailing list (too huge traffic for me).

Umm, if you want to be replied (or CCed) to, why do you make sure that
replies do not go to you by default??? Why do you set "Mail-Followup-To"
to exclude yourself?

> Mail-Followup-To: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>,
> 	devicetree@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx,
> 	linux-input@xxxxxxxxxxxxxxx

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/