Re: [PATCH v2 2/2] asus-wmi: Add support for ROG X13 tablet mode

[Luke Jones]

Hi Andy,
> >  -       { KE_KEY, 0xFA, { KEY_PROG2 } },           /* Lid flip 
> > action */
> >  +       { KE_KEY, 0xFA, { KEY_PROG2 } }, /* Lid flip action */
>
> Have maintainers asked you about this? Otherwise it is irrelevant 
> change.

Fixed

> ...
>
> >  +                       pr_err("This device has lid-flip-rog quirk 
> > but got ENODEV checking it. This is a bug.");
>
> dev_err() ?

Okay, changed here and in previous patch to match it.

So that I'm clearer on dev_err(), this doesn't do something like exit 
the module does it? It's just a more detailed error print?

> >  +static void lid_flip_rog_tablet_mode_get_state(struct asus_wmi 
> > *asus)
> >  +{
> >  +       int result = asus_wmi_get_devstate_simple(asus, 
> > ASUS_WMI_DEVID_LID_FLIP_ROG);
> >  +
> >  +       if (result >= 0) {
>
> First of all, it's better to decouple assignment and definition, and
> move assignment closer to its user. This is usual pattern.

I don't fully understand why you would want the separation given how 
short these two blocks are (I'll change in this and previous patch of 
course, I just don't personally understand it).

Cheers,
Luke.
>