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

From: Viresh Kumar
Date: Fri Mar 19 2021 - 02:36:46 EST


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.

--
viresh