All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
	David Miller <davem@davemloft.net>
Cc: geert@linux-m68k.org, netdev@vger.kernel.org, wg@grandegger.com,
	linux-can@vger.kernel.org, linux-sh@vger.kernel.org,
	vksavl@gmail.com
Subject: Re: [PATCH v5] can: add Renesas R-Car CAN driver
Date: Mon, 20 Jan 2014 21:17:47 +0000	[thread overview]
Message-ID: <52DD927B.2060500@pengutronix.de> (raw)
In-Reply-To: <52DD9F3C.7000408@cogentembedded.com>

[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]

On 01/20/2014 11:12 PM, Sergei Shtylyov wrote:
[...]

>> I truly think using packed here is rediculous, please remove it unless
>> you can prove that things won't work without it.
> 
>    They will. But how about the following 'struct rcar_can_regs'?

The strcuts in question are:

> +/* Mailbox registers structure */
> +struct rcar_can_mbox_regs {
> +	u32 id;		/* IDE and RTR bits, SID and EID */
> +	u8 stub;	/* Not used */
> +	u8 dlc;		/* Data Length Code - bits [0..3] */
> +	u8 data[8];	/* Data Bytes */
> +	u8 tsh;		/* Time Stamp Higher Byte */
> +	u8 tsl;		/* Time Stamp Lower Byte */
> +} __packed;
> +
> +struct rcar_can_regs {
> +	struct rcar_can_mbox_regs mb[RCAR_CAN_N_MBX]; /* Mailbox registers */
> +	u32 mkr_2_9[8];	/* Mask Registers 2-9 */
> +	u32 fidcr[2];	/* FIFO Received ID Compare Register */
> +	u32 mkivlr1;	/* Mask Invalid Register 1 */
> +	u32 mier1;	/* Mailbox Interrupt Enable Register 1 */
> +	u32 mkr_0_1[2];	/* Mask Registers 0-1 */
> +	u32 mkivlr0;    /* Mask Invalid Register 0*/
> +	u32 mier0;      /* Mailbox Interrupt Enable Register 0 */
> +	u8 pad_440[0x3c0];
> +	u8 mctl[64];	/* Message Control Registers */
> +	u16 ctlr;	/* Control Register */
> +	u16 str;	/* Status register */
> +	u8 bcr[3];	/* Bit Configuration Register */
> +	u8 clkr;	/* Clock Select Register */
> +	u8 rfcr;	/* Receive FIFO Control Register */
> +	u8 rfpcr;	/* Receive FIFO Pointer Control Register */
> +	u8 tfcr;	/* Transmit FIFO Control Register */
> +	u8 tfpcr;       /* Transmit FIFO Pointer Control Register */
> +	u8 eier;	/* Error Interrupt Enable Register */
> +	u8 eifr;	/* Error Interrupt Factor Judge Register */
> +	u8 recr;	/* Receive Error Count Register */
> +	u8 tecr;        /* Transmit Error Count Register */
> +	u8 ecsr;	/* Error Code Store Register */
> +	u8 cssr;	/* Channel Search Support Register */
> +	u8 mssr;	/* Mailbox Search Status Register */
> +	u8 msmr;	/* Mailbox Search Mode Register */
> +	u16 tsr;	/* Time Stamp Register */
> +	u8 afsr;	/* Acceptance Filter Support Register */
> +	u8 pad_857;
> +	u8 tcr;		/* Test Control Register */
> +	u8 pad_859[7];
> +	u8 ier;		/* Interrupt Enable Register */
> +	u8 isr;		/* Interrupt Status Register */
> +	u8 pad_862;
> +	u8 mbsmr;	/* Mailbox Search Mask Register */
> +} __packed;

I think they should work without packed, too.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
	David Miller <davem@davemloft.net>
Cc: geert@linux-m68k.org, netdev@vger.kernel.org, wg@grandegger.com,
	linux-can@vger.kernel.org, linux-sh@vger.kernel.org,
	vksavl@gmail.com
Subject: Re: [PATCH v5] can: add Renesas R-Car CAN driver
Date: Mon, 20 Jan 2014 22:17:47 +0100	[thread overview]
Message-ID: <52DD927B.2060500@pengutronix.de> (raw)
In-Reply-To: <52DD9F3C.7000408@cogentembedded.com>

[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]

On 01/20/2014 11:12 PM, Sergei Shtylyov wrote:
[...]

>> I truly think using packed here is rediculous, please remove it unless
>> you can prove that things won't work without it.
> 
>    They will. But how about the following 'struct rcar_can_regs'?

The strcuts in question are:

> +/* Mailbox registers structure */
> +struct rcar_can_mbox_regs {
> +	u32 id;		/* IDE and RTR bits, SID and EID */
> +	u8 stub;	/* Not used */
> +	u8 dlc;		/* Data Length Code - bits [0..3] */
> +	u8 data[8];	/* Data Bytes */
> +	u8 tsh;		/* Time Stamp Higher Byte */
> +	u8 tsl;		/* Time Stamp Lower Byte */
> +} __packed;
> +
> +struct rcar_can_regs {
> +	struct rcar_can_mbox_regs mb[RCAR_CAN_N_MBX]; /* Mailbox registers */
> +	u32 mkr_2_9[8];	/* Mask Registers 2-9 */
> +	u32 fidcr[2];	/* FIFO Received ID Compare Register */
> +	u32 mkivlr1;	/* Mask Invalid Register 1 */
> +	u32 mier1;	/* Mailbox Interrupt Enable Register 1 */
> +	u32 mkr_0_1[2];	/* Mask Registers 0-1 */
> +	u32 mkivlr0;    /* Mask Invalid Register 0*/
> +	u32 mier0;      /* Mailbox Interrupt Enable Register 0 */
> +	u8 pad_440[0x3c0];
> +	u8 mctl[64];	/* Message Control Registers */
> +	u16 ctlr;	/* Control Register */
> +	u16 str;	/* Status register */
> +	u8 bcr[3];	/* Bit Configuration Register */
> +	u8 clkr;	/* Clock Select Register */
> +	u8 rfcr;	/* Receive FIFO Control Register */
> +	u8 rfpcr;	/* Receive FIFO Pointer Control Register */
> +	u8 tfcr;	/* Transmit FIFO Control Register */
> +	u8 tfpcr;       /* Transmit FIFO Pointer Control Register */
> +	u8 eier;	/* Error Interrupt Enable Register */
> +	u8 eifr;	/* Error Interrupt Factor Judge Register */
> +	u8 recr;	/* Receive Error Count Register */
> +	u8 tecr;        /* Transmit Error Count Register */
> +	u8 ecsr;	/* Error Code Store Register */
> +	u8 cssr;	/* Channel Search Support Register */
> +	u8 mssr;	/* Mailbox Search Status Register */
> +	u8 msmr;	/* Mailbox Search Mode Register */
> +	u16 tsr;	/* Time Stamp Register */
> +	u8 afsr;	/* Acceptance Filter Support Register */
> +	u8 pad_857;
> +	u8 tcr;		/* Test Control Register */
> +	u8 pad_859[7];
> +	u8 ier;		/* Interrupt Enable Register */
> +	u8 isr;		/* Interrupt Status Register */
> +	u8 pad_862;
> +	u8 mbsmr;	/* Mailbox Search Mask Register */
> +} __packed;

I think they should work without packed, too.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

  reply	other threads:[~2014-01-20 21:17 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-26 20:37 [PATCH v5] can: add Renesas R-Car CAN driver Sergei Shtylyov
2013-12-26 21:37 ` Sergei Shtylyov
2014-01-13 13:46 ` Sergei Shtylyov
2014-01-13 13:46   ` Sergei Shtylyov
2014-01-20  9:18 ` Marc Kleine-Budde
2014-01-20  9:18   ` Marc Kleine-Budde
2014-01-25  0:34   ` Sergei Shtylyov
2014-01-25  1:34     ` Sergei Shtylyov
2014-02-13 12:12     ` Marc Kleine-Budde
2014-02-13 12:12       ` Marc Kleine-Budde
2014-02-20 22:48       ` Sergei Shtylyov
2014-02-20 23:48         ` Sergei Shtylyov
2014-02-28  9:08         ` Marc Kleine-Budde
2014-02-28  9:08           ` Marc Kleine-Budde
2014-02-28 11:16           ` Sergei Shtylyov
2014-02-28 11:16             ` Sergei Shtylyov
2014-02-28 11:37             ` Marc Kleine-Budde
2014-02-28 11:37               ` Marc Kleine-Budde
2014-02-28 11:41               ` Geert Uytterhoeven
2014-02-28 11:41                 ` Geert Uytterhoeven
2014-02-28 11:47                 ` David Laight
2014-02-28 11:47                   ` David Laight
2014-02-28 11:50                   ` Marc Kleine-Budde
2014-02-28 11:50                     ` Marc Kleine-Budde
2014-02-28 12:02                     ` David Laight
2014-02-28 12:02                       ` David Laight
2014-02-28 11:49                 ` Marc Kleine-Budde
2014-02-28 11:49                   ` Marc Kleine-Budde
2014-02-28 12:05                   ` Sergei Shtylyov
2014-02-28 12:05                     ` Sergei Shtylyov
2014-02-28 12:17                     ` David Laight
2014-02-28 12:17                       ` David Laight
2014-02-28 12:34                       ` Sergei Shtylyov
2014-02-28 12:34                         ` Sergei Shtylyov
2014-01-20 11:43 ` Geert Uytterhoeven
2014-01-20 11:43   ` Geert Uytterhoeven
2014-01-20 11:47   ` Marc Kleine-Budde
2014-01-20 11:47     ` Marc Kleine-Budde
2014-01-20 11:52     ` Geert Uytterhoeven
2014-01-20 11:52       ` Geert Uytterhoeven
2014-01-20 11:58       ` Marc Kleine-Budde
2014-01-20 11:58         ` Marc Kleine-Budde
2014-01-20 12:02         ` Ben Dooks
2014-01-20 12:05           ` Geert Uytterhoeven
2014-01-20 12:05             ` Geert Uytterhoeven
2014-01-20 12:08             ` Marc Kleine-Budde
2014-01-20 12:08               ` Marc Kleine-Budde
2014-01-20 12:05           ` Marc Kleine-Budde
2014-01-20 12:05             ` Marc Kleine-Budde
2014-01-20 12:13           ` David Laight
2014-01-20 12:13             ` David Laight
2014-01-20 12:35             ` Marc Kleine-Budde
2014-01-20 12:35               ` Marc Kleine-Budde
2014-01-20 19:16         ` David Miller
2014-01-20 19:16           ` David Miller
2014-01-20 21:12           ` Sergei Shtylyov
2014-01-20 22:12             ` Sergei Shtylyov
2014-01-20 21:17             ` Marc Kleine-Budde [this message]
2014-01-20 21:17               ` Marc Kleine-Budde
2014-01-22 11:52               ` Ben Dooks
2014-01-22 11:54                 ` Geert Uytterhoeven
2014-01-22 11:54                   ` Geert Uytterhoeven
2014-01-22 11:58                 ` David Laight
2014-01-22 11:58                   ` David Laight
2014-01-20 12:12   ` Sergei Shtylyov
2014-01-20 12:12     ` Sergei Shtylyov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52DD927B.2060500@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=davem@davemloft.net \
    --cc=geert@linux-m68k.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sergei.shtylyov@cogentembedded.com \
    --cc=vksavl@gmail.com \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.