Hi Gabriel,

On Wed, Dec 8, 2021 at 9:14 PM Gabriel L. Somlo [off-list ref] wrote:

I did *some* of this for v3, but since figured out how to use `pahole` :)

Right, pahole.

On Mon, Dec 06, 2021 at 11:07:56AM +0100, Geert Uytterhoeven wrote:

quoted

quoted

+struct litex_mmc_host {
+       struct mmc_host *mmc;
+       struct platform_device *dev;
+
+       void __iomem *sdphy;
+       void __iomem *sdcore;
+       void __iomem *sdreader;
+       void __iomem *sdwriter;
+       void __iomem *sdirq;
+
+       u32 resp[4];
+       u16 rca;
+
+       void *buffer;
+       size_t buf_size;
+       dma_addr_t dma;
+
+       unsigned int freq;
+       unsigned int clock;
+       bool is_bus_width_set;
+       bool app_cmd;
+
+       int irq;
+       struct completion cmd_done;

You may want to reorder the members to avoid implicit gaps
(i.e. structs first, followed by integral types in decreasing size).

So, for v4, I'll have it looking like this, which `pahole` says is
optimally packed:

struct litex_mmc_host {
        struct mmc_host *          mmc;                  /*     0     8 */
        struct platform_device *   dev;                  /*     8     8 */
        void *                     sdphy;                /*    16     8 */
        void *                     sdcore;               /*    24     8 */
        void *                     sdreader;             /*    32     8 */
        void *                     sdwriter;             /*    40     8 */
        void *                     sdirq;                /*    48     8 */
        void *                     buffer;               /*    56     8 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        size_t                     buf_size;             /*    64     8 */

size_t is 32-bit on RV32, so you may want to move it below cmd_done.

        dma_addr_t                 dma;                  /*    72     8 */
        struct completion          cmd_done;             /*    80    32 */
        int                        irq;                  /*   112     4 */
        unsigned int               ref_clk;              /*   116     4 */
        unsigned int               sd_clk;               /*   120     4 */
        u32                        resp[4];              /*   124    16 */
        /* --- cacheline 2 boundary (128 bytes) was 12 bytes ago --- */
        u16                        rca;                  /*   140     2 */
        bool                       is_bus_width_set;     /*   142     1 */
        bool                       app_cmd;              /*   143     1 */

        /* size: 144, cachelines: 3, members: 18 */
        /* last cacheline: 16 bytes */
};

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds