Re: [PATCH 01/12] mfd: Eberspaecher Flexcard PMC II Carrier Board support

[Arnd Bergmann <arnd@arndb.de>]

On Wednesday, December 14, 2016 1:11:42 AM CET Holger Dengler wrote:
>
> diff --git a/include/uapi/linux/flexcard.h b/include/uapi/linux/flexcard.h
> new file mode 100644
> index 0000000..4e9f07b4
> --- /dev/null
> +++ b/include/uapi/linux/flexcard.h
> @@ -0,0 +1,64 @@

Why is this exported to user space?

> +
> +#include <linux/types.h>
> +
> +struct fc_version {
> +	__u8	dev;
> +	__u8	min;
> +	__u8	maj;
> +	__u8	reserved;
> +} __packed;

The __packed attribute is redundant here as all members
are just one byte anyway.

> +/* PCI BAR 0: Flexcard configuration */
> +struct fc_bar0_conf {
> +	__u32 r1;			/* 000 */
> +	struct fc_version fc_fw_ver;	/* 004 */
> +	struct fc_version fc_hw_ver;	/* 008 */
> +	__u32 r2[3];			/* 00c */
> +	__u64 fc_sn;			/* 018 */
> +	__u32 fc_uid;			/* 020 */
> +	__u32 r3[7];			/* 024 */
> +	__u32 fc_lic[6];		/* 040 */
> +	__u32 fc_slic[6];		/* 058 */
> +	__u32 trig_ctrl1;		/* 070 */
> +	__u32 r4;			/* 074 */
> +	__u32 trig_ctrl2;		/* 078 */
> +	__u32 r5[22];			/* 07c */
> +	__u32 amreg;			/* 0d4 */
> +	__u32 tiny_stat;		/* 0d8 */
> +	__u32 r6[5];			/* 0dc */
> +	__u32 can_dat_cnt;		/* 0f0 */
> +	__u32 can_err_cnt;		/* 0f4 */
> +	__u32 fc_data_cnt;		/* 0f8 */
> +	__u32 r7;			/* 0fc */
> +	__u32 fc_rocr;			/* 100 */
> +	__u32 r8;			/* 104 */
> +	__u32 pg_ctrl;			/* 108 */
> +	__u32 pg_term;			/* 10c */
> +	__u32 r9;			/* 110 */
> +	__u32 irs;			/* 114 */
> +	__u32 fr_tx_cnt;		/* 118 */
> +	__u32 irc;			/* 11c */
> +	__u64 pcnt;			/* 120 */
> +	__u32 r10;			/* 128 */
> +	__u32 nmv_cnt;			/* 12c */
> +	__u32 info_cnt;			/* 130 */
> +	__u32 stat_trg_cnt;		/* 134 */
> +	__u32 r11;			/* 138 */
> +	__u32 fr_rx_cnt;		/* 13c */
> +} __packed;

Here the __packed attribute is probably harmful, it prevents you
from accessing the members using 32-bit sized accesses and forces
bytewise accesses on some architectures, which tends to do the
wrong thing on MMIO.

	Arnd