| Message ID | 2942b7ca9ecf86b6bff75c10ccfca25c173c3f0d.1570194906.git.mchehab+samsung@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] media: si2168: use bits instead of bool for flags | expand |
On Fri, Oct 04, 2019 at 10:15:22AM -0300, Mauro Carvalho Chehab wrote: > Using bool on struct is not recommended, as it wastes lots of > space. So, instead, let's use bits. Wouldn't "bool b:1;" even be better? I performed a little test: #include <stdbool.h> #include <stdio.h> struct uints { unsigned int a0; unsigned int a1; unsigned int a2; unsigned int a3; unsigned int a4; unsigned int a5; unsigned int a6; unsigned int a7; }; struct bools { bool a0; bool a1; bool a2; bool a3; bool a4; bool a5; bool a6; bool a7; }; struct bit_uints { unsigned int a0:1; unsigned int a1:1; unsigned int a2:1; unsigned int a3:1; unsigned int a4:1; unsigned int a5:1; unsigned int a6:1; unsigned int a7:1; }; struct bit_bools { bool a0:1; bool a1:1; bool a2:1; bool a3:1; bool a4:1; bool a5:1; bool a6:1; bool a7:1; }; int main() { printf("bit_uints: %ld\n", sizeof(struct bit_uints)); printf("bit_bools: %ld\n", sizeof(struct bit_bools)); printf("uints: %ld\n", sizeof(struct uints)); printf("bools: %ld\n", sizeof(struct bools)); } Result: bit_uints: 4 bit_bools: 1 uints: 32 bools: 8 I know with different types within the struct it looks different, but still. g
Em Thu, 10 Oct 2019 12:55:44 +0200 Gon Solo <gonsolo@gmail.com> escreveu: > On Fri, Oct 04, 2019 at 10:15:22AM -0300, Mauro Carvalho Chehab wrote: > > Using bool on struct is not recommended, as it wastes lots of > > space. So, instead, let's use bits. > > Wouldn't "bool b:1;" even be better? I performed a little test: > > #include <stdbool.h> > #include <stdio.h> > > struct uints { > unsigned int a0; > unsigned int a1; > unsigned int a2; > unsigned int a3; > unsigned int a4; > unsigned int a5; > unsigned int a6; > unsigned int a7; > }; > > struct bools { > bool a0; > bool a1; > bool a2; > bool a3; > bool a4; > bool a5; > bool a6; > bool a7; > }; > > struct bit_uints { > unsigned int a0:1; > unsigned int a1:1; > unsigned int a2:1; > unsigned int a3:1; > unsigned int a4:1; > unsigned int a5:1; > unsigned int a6:1; > unsigned int a7:1; > }; > > struct bit_bools { > bool a0:1; > bool a1:1; > bool a2:1; > bool a3:1; > bool a4:1; > bool a5:1; > bool a6:1; > bool a7:1; > }; > > int main() { > printf("bit_uints: %ld\n", sizeof(struct bit_uints)); > printf("bit_bools: %ld\n", sizeof(struct bit_bools)); > printf("uints: %ld\n", sizeof(struct uints)); > printf("bools: %ld\n", sizeof(struct bools)); > } > > Result: > > bit_uints: 4 > bit_bools: 1 > uints: 32 > bools: 8 > > I know with different types within the struct it looks different, but > still. No. In practice, the compiler will add 3 bytes of pad after bit_bools (on 32-bit archs), due to performance reasons. Using "unsigned int" makes it clearer. Thanks, Mauro
Em Thu, 10 Oct 2019 08:34:23 -0300 Mauro Carvalho Chehab <mchehab+samsung@kernel.org> escreveu: > Em Thu, 10 Oct 2019 12:55:44 +0200 > Gon Solo <gonsolo@gmail.com> escreveu: > > > On Fri, Oct 04, 2019 at 10:15:22AM -0300, Mauro Carvalho Chehab wrote: > > > Using bool on struct is not recommended, as it wastes lots of > > > space. So, instead, let's use bits. > > > > Wouldn't "bool b:1;" even be better? I performed a little test: > > Result: > > > > bit_uints: 4 > > bit_bools: 1 > > I know with different types within the struct it looks different, but > > still. > > No. In practice, the compiler will add 3 bytes of pad after bit_bools > (on 32-bit archs), due to performance reasons. Btw, if you want to test, just add something after the bits, and you'll see that it will now report the PAD bytes too: struct bit_uints { unsigned int a0:1; unsigned int a1:1; unsigned int a2:1; unsigned int a3:1; unsigned int a4:1; unsigned int a5:1; unsigned int a6:1; unsigned int a7:1; int i; }; struct bit_bools { bool a0:1; bool a1:1; bool a2:1; bool a3:1; bool a4:1; bool a5:1; bool a6:1; bool a7:1; int i; }; bit_uints: 8 bit_bools: 8 Thanks, Mauro
diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h index 50dccb394efa..ecd21adf8950 100644 --- a/drivers/media/dvb-frontends/si2168.h +++ b/drivers/media/dvb-frontends/si2168.h @@ -9,38 +9,43 @@ #define SI2168_H #include <linux/dvb/frontend.h> -/* - * I2C address - * 0x64 +/** + * struct si2168_config - configuration parameters for si2168 + * + * @fe: + * frontend returned by driver + * @i2c_adapter: + * tuner I2C adapter returned by driver + * @ts_mode: + * Transport Stream mode. Can be: + * - %SI2168_TS_PARALLEL + * - %SI2168_TS_SERIAL + * - %SI2168_TS_TRISTATE + * - %SI2168_TS_CLK_MANUAL + * @ts_clock_inv: + * TS clock inverted + * @ts_clock_gapped: + * TS clock gapped + * @spectral_inversion: + * Inverted spectrum + * + * Note: + * The I2C address of this demod is 0x64. */ struct si2168_config { - /* - * frontend - * returned by driver - */ struct dvb_frontend **fe; - - /* - * tuner I2C adapter - * returned by driver - */ struct i2c_adapter **i2c_adapter; - /* TS mode */ #define SI2168_TS_PARALLEL 0x06 #define SI2168_TS_SERIAL 0x03 #define SI2168_TS_TRISTATE 0x00 #define SI2168_TS_CLK_MANUAL 0x20 u8 ts_mode; - /* TS clock inverted */ - bool ts_clock_inv; - - /* TS clock gapped */ - bool ts_clock_gapped; - - /* Inverted spectrum */ - bool spectral_inversion; + /* Flags */ + unsigned int ts_clock_inv:1; + unsigned int ts_clock_gapped:1; + unsigned int spectral_inversion:1; }; #endif diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h index 804d5b30c697..18bea5222082 100644 --- a/drivers/media/dvb-frontends/si2168_priv.h +++ b/drivers/media/dvb-frontends/si2168_priv.h @@ -34,12 +34,12 @@ struct si2168_dev { unsigned int chip_id; unsigned int version; const char *firmware_name; - bool active; - bool warm; u8 ts_mode; - bool ts_clock_inv; - bool ts_clock_gapped; - bool spectral_inversion; + unsigned int active:1; + unsigned int warm:1; + unsigned int ts_clock_inv:1; + unsigned int ts_clock_gapped:1; + unsigned int spectral_inversion:1; }; /* firmware command struct */
Using bool on struct is not recommended, as it wastes lots of space. So, instead, let's use bits. While here, convert the comments to kernel-doc format. Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> --- drivers/media/dvb-frontends/si2168.h | 47 +++++++++++++---------- drivers/media/dvb-frontends/si2168_priv.h | 10 ++--- 2 files changed, 31 insertions(+), 26 deletions(-)