On Sun, Jan 17, 2021 at 12:21 AM Barnabás Pőcze <pobrn@xxxxxxxxxxxxxx> wrote:
> 2021. január 16., szombat 20:58 keltezéssel, Andy Shevchenko írta:
> > On Wed, Jan 13, 2021 at 8:23 PM Barnabás Pőcze wrote:
...
> > > + struct {
> > > + bool hw_rfkill_switch : 1,
> >
> > Does it make sense? Not to me.
> > Why not put all of them (I don't like comma and single occurrence of
> > the type, it may be problematic in the future) as unsigned int, or
> > something like that?
>
> I will add the full declaration for each, with type and semicolon in each
> line. Would you prefer the type to be `unsigned int` instead of `bool`, or
> could you clarify what you mean by the second question?
I have no preference and in my code I can do either depending on the
case. But your below answer is fine, so choose what you prefer in this
case.
> > Also, is it okay to have bit fields (I mean from synchronization p.o.v.)?
>
> I am fairly certain it is since this bit-field is only ever written in
> `ideapad_check_features()` which is called from `ideapad_acpi_add()`. Apart from
> two instances they are only read indirectly by `ideapad_acpi_add()`, and even
> in those two cases if the values are read as false instead of their real
> value, it cannot cause significant issues as far as I see.
Okay, then choose what you prefer. Bit fields are beasts when it comes
to synchronization / concurrent access matter, but they will take less
size (when > 4, since bool usually takes 1 byte on some architectures
/ compilers).
> > > + fan_mode : 1,
> > > + touchpad_ctrl_via_ec : 1,
> > > + conservation_mode : 1,
> > > + fn_lock : 1;
> > > + } features;
> > > };
--
With Best Regards,
Andy Shevchenko
