Re: [PATCH v8] i2c: virtio: add a virtio i2c frontend driver

[Jie Deng <jie.deng@intel.com>]

On 2021/3/19 14:35, Viresh Kumar wrote:
> On 19-03-21, 14:29, Jie Deng wrote:
>> I also see example drivers/i2c/busses/i2c-xiic.c. Some people might think
>> this way is more clearer than
>>
>> updating each member in probe. Basically, I think it's just a matter of
>> personal preference which doesn't
> Memory used by one instance of struct i2c_adapter (on arm64):
>
> struct i2c_adapter {
>          struct module *            owner;                /*     0     8 */
>          unsigned int               class;                /*     8     4 */
>
>          /* XXX 4 bytes hole, try to pack */
>
>          const struct i2c_algorithm  * algo;              /*    16     8 */
>          void *                     algo_data;            /*    24     8 */
>          const struct i2c_lock_operations  * lock_ops;    /*    32     8 */
>          struct rt_mutex            bus_lock;             /*    40    32 */
>          /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
>          struct rt_mutex            mux_lock;             /*    72    32 */
>          int                        timeout;              /*   104     4 */
>          int                        retries;              /*   108     4 */
>          struct device              dev;                  /*   112   744 */
>
>          /* XXX last struct has 7 bytes of padding */
>
>          /* --- cacheline 13 boundary (832 bytes) was 24 bytes ago --- */
>          long unsigned int          locked_flags;         /*   856     8 */
>          int                        nr;                   /*   864     4 */
>          char                       name[48];             /*   868    48 */
>
>          /* XXX 4 bytes hole, try to pack */
>
>          /* --- cacheline 14 boundary (896 bytes) was 24 bytes ago --- */
>          struct completion          dev_released;         /*   920    32 */
>          struct mutex               userspace_clients_lock; /*   952    32 */
>          /* --- cacheline 15 boundary (960 bytes) was 24 bytes ago --- */
>          struct list_head           userspace_clients;    /*   984    16 */
>          struct i2c_bus_recovery_info * bus_recovery_info; /*  1000     8 */
>          const struct i2c_adapter_quirks  * quirks;       /*  1008     8 */
>          struct irq_domain *        host_notify_domain;   /*  1016     8 */
>          /* --- cacheline 16 boundary (1024 bytes) --- */
>
>          /* size: 1024, cachelines: 16, members: 19 */
>          /* sum members: 1016, holes: 2, sum holes: 8 */
>          /* paddings: 1, sum paddings: 7 */
> };
>
> So, this extra instance that i2c-xiic.c is creating (and that you want to
> create) is going to waste 1KB of memory and it will never be used.
>
> This is bad coding practice IMHO and it is not really about personal preference.


I will remove that structure and update the members in probe.