linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sven Van Asbroeck <thesven73@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Sven Van Asbroeck" <svendev@arcx.com>,
	robh+dt@kernel.org, "Linus Walleij" <linus.walleij@linaro.org>,
	"Lee Jones" <lee.jones@linaro.org>,
	mark.rutland@arm.com, "Andreas Färber" <afaerber@suse.de>,
	treding@nvidia.com, "David Lechner" <david@lechnology.com>,
	noralf@tronnes.org, johan@kernel.org,
	"Michal Simek" <monstr@monstr.eu>,
	michal.vokac@ysoft.com, gregkh@linuxfoundation.org,
	john.garry@huawei.com, geert+renesas@glider.be,
	robin.murphy@arm.com, paul.gortmaker@windriver.com,
	sebastien.bourdelin@savoirfairelinux.com, icenowy@aosc.io,
	"Stuart Yoder" <stuyoder@gmail.com>,
	maxime.ripard@bootlin.com,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>
Subject: Re: [PATCH anybus v3 4/6] bus: support HMS Anybus-S bus
Date: Fri, 9 Nov 2018 11:25:09 -0500	[thread overview]
Message-ID: <CAGngYiWxhSbLLsPaRZ90wEMhhF7e1jt06ZhhRcuhjM53-RYRwg@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a3DnKcRxL5dG_pvBVTngFh5eDL2JdEqZzgL0RXJy5dZGw@mail.gmail.com>

On Thu, Nov 8, 2018 at 9:07 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> > +struct anybus_mbox_hdr {
> > +       u16 id;
> > +       u16 info;
> > +       u16 cmd_num;
> > +       u16 data_size;
> > +       u16 frame_count;
> > +       u16 frame_num;
> > +       u16 offset_high;
> > +       u16 offset_low;
> > +       u16 extended[8];
> > +} __packed;
>
> I don't think you want this to be __packed, it won't change the layout
> but instead force byte accesses on architectures that don't have
> fast unaligned load/store instructions.
>
> Instead of the __packed, it's almost always better to ensure that
> the structure itself is aligned to a 16-bit address.
>

A general question about __packed.

My current understanding is this:
(please tell me if it's incorrect or incomplete)

+ without __packed, the compiler is free to pad the struct in whatever
way it feels is best.
+ with __packed, the fields have to be laid out EXACTLY as specified.

Is it possible that someone will compile this on an architecture that 'likes'
16-bit ints to be 32-bit aligned? If such an architecture does not currently
exist, could it appear in the future?

If the compiler changes the padding in the struct, the driver will
break, as the struct layout is tightly defined by the anybus spec.

Would it be better to stay safe, and keep __packed in case of such
tightly defined structures?

  parent reply	other threads:[~2018-11-09 16:25 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-04 15:54 [PATCH anybus v3 0/6] Support HMS Profinet Card over Anybus thesven73
2018-11-04 15:54 ` [PATCH anybus v3 1/6] misc: support the Arcx anybus bridge thesven73
2018-11-05 21:20   ` Rob Herring
2018-11-05 21:50     ` Sven Van Asbroeck
2018-11-06 13:58       ` Rob Herring
2018-11-06 14:45         ` Sven Van Asbroeck
2018-11-06 18:30           ` Rob Herring
2018-11-06 20:05             ` Sven Van Asbroeck
2018-11-08 13:40               ` Arnd Bergmann
2018-11-04 15:54 ` [PATCH anybus v3 2/6] dt-bindings: Add vendor prefix for arcx / Archronix thesven73
2018-11-04 15:57   ` Andreas Färber
2018-11-05 20:44   ` Rob Herring
2018-11-04 15:54 ` [PATCH anybus v3 3/6] dt-bindings: anybus-bridge: document devicetree binding thesven73
2018-11-05 20:45   ` Rob Herring
2018-11-04 15:54 ` [PATCH anybus v3 4/6] bus: support HMS Anybus-S bus thesven73
2018-11-08 14:07   ` Arnd Bergmann
2018-11-08 15:47     ` Sven Van Asbroeck
2018-11-08 16:54       ` Arnd Bergmann
2018-11-09 16:25     ` Sven Van Asbroeck [this message]
2018-11-09 21:03       ` Arnd Bergmann
2018-11-10 10:55         ` Geert Uytterhoeven
2018-11-12 16:23     ` Sven Van Asbroeck
2018-11-04 15:55 ` [PATCH anybus v3 5/6] dt-bindings: anybuss-host: document devicetree binding thesven73
2018-11-08 14:11   ` Arnd Bergmann
2018-11-08 14:21     ` Sven Van Asbroeck
2018-11-08 14:27       ` Arnd Bergmann
2018-11-12 18:05         ` Sven Van Asbroeck
2018-11-04 15:55 ` [PATCH anybus v3 6/6] misc: support HMS Profinet IRT industrial controller thesven73
2018-11-08 14:20   ` Arnd Bergmann
2018-11-08 15:35     ` Sven Van Asbroeck
2018-11-08 16:52       ` Arnd Bergmann
2018-11-09 16:02 ` [PATCH anybus v3 0/6] Support HMS Profinet Card over Anybus Sven Van Asbroeck
2018-11-09 21:22   ` Arnd Bergmann
2018-11-09 21:46     ` Sven Van Asbroeck
2018-11-09 22:32       ` Arnd Bergmann

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=CAGngYiWxhSbLLsPaRZ90wEMhhF7e1jt06ZhhRcuhjM53-RYRwg@mail.gmail.com \
    --to=thesven73@gmail.com \
    --cc=afaerber@suse.de \
    --cc=arnd@arndb.de \
    --cc=david@lechnology.com \
    --cc=devicetree@vger.kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=icenowy@aosc.io \
    --cc=johan@kernel.org \
    --cc=john.garry@huawei.com \
    --cc=lee.jones@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=maxime.ripard@bootlin.com \
    --cc=michal.vokac@ysoft.com \
    --cc=monstr@monstr.eu \
    --cc=noralf@tronnes.org \
    --cc=paul.gortmaker@windriver.com \
    --cc=robh+dt@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=sebastien.bourdelin@savoirfairelinux.com \
    --cc=stuyoder@gmail.com \
    --cc=svendev@arcx.com \
    --cc=treding@nvidia.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).