From: Boris Brezillon <bbrezillon@kernel.org>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>,
Lucas Stach <dev@lynxeye.de>, Marek Vasut <marek.vasut@gmail.com>,
Richard Weinberger <richard@nod.at>,
Linus Walleij <linus.walleij@linaro.org>,
Janusz Krzysztofik <jmkrzyszt@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Stefan Agner <stefan@agner.ch>,
Jonathan Hunter <jonathanh@nvidia.com>,
Boris Brezillon <boris.brezillon@bootlin.com>,
Thierry Reding <thierry.reding@gmail.com>,
linux-mtd <linux-mtd@lists.infradead.org>,
linux-tegra@vger.kernel.org,
Brian Norris <computersforpeace@gmail.com>,
David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH 2/2] mtd: rawnand: use unnamed union in struct nand_op_parser_pattern_elem
Date: Tue, 22 Jan 2019 09:33:13 +0100 [thread overview]
Message-ID: <20190122093313.67375948@bbrezillon> (raw)
In-Reply-To: <20190122090830.118eb6be@xps13>
On Tue, 22 Jan 2019 09:08:30 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Hi Masahiro,
>
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote on Tue, 22 Jan
> 2019 17:00:54 +0900:
>
> > On Tue, Jan 22, 2019 at 4:50 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Masahiro,
> > >
> > > Masahiro Yamada <yamada.masahiro@socionext.com> wrote on Tue, 22 Jan
> > > 2019 16:42:55 +0900:
> > >
> > > > Although drivers do not directly get access to the private data of
> > > > instruction patterns, let's use unnamed union field to be consistent
> > > > with nand_op_instr.
> > > >
> > >
> > > Actually this is how we wrote it the first time. Then we got robots
> > > reporting that anonymous unions where not allowed with older (still
> > > supported) GCC versions and I had to do this:
> > >
> > >
> > > commit c1a72e2dbb4abb90bd408480d7c48ba40cb799ce
> > > Author: Miquel Raynal <miquel.raynal@free-electrons.com>
> > > Date: Fri Jan 19 19:11:27 2018 +0100
> > >
> > > mtd: nand: Fix build issues due to an anonymous union
> > >
> > > GCC-4.4.4 raises errors when assigning a parameter in an anonymous
> > > union, leading to this kind of failure:
> > >
> > > drivers/mtd/nand/marvell_nand.c:1936:
> > > warning: missing braces around initializer
> > > warning: (near initialization for '(anonymous)[1].<anonymous>')
> > > error: unknown field 'data' specified in initializer
> > > error: unknown field 'addr' specified in initializer
> > >
> > > Work around the situation by naming these unions.
> > >
> > > Fixes: 8878b126df76 ("mtd: nand: add ->exec_op() implementation")
> > > Reported-by: Andrew Morton <akpm@linux-foundation.org>
> > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
> > > Tested-by: Andrew Morton <akpm@linux-foundation.org>
> > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > >
> >
> >
> > Hmm, how come Andrew's compiler was fine with the following?
> >
> > struct nand_flash_dev {
> > char *name;
> > union {
> > struct {
> > uint8_t mfr_id;
> > uint8_t dev_id;
> > };
> > uint8_t id[NAND_MAX_ID_LEN];
> > };
> > unsigned int pagesize;
> > ...
> > };
> >
>
> It is probably not :)
It was compile fine. I don't know all the subtleties, but maybe it's
because ->id[] is a base type and not a struct.
>
> >
> >
> > The current minimum version is GCC 4.6
> > (commit cafa0010cd51fb7)
> > but I am not sure if this restriction is remaining.
> >
>
> That's right, can you please test if this limitation is still
> ongoing wit GCC 4.6?
I have a more important question: why should we go bad back to unnamed
unions? Why is that a problem to have a named union? Sure, we initially
started with an unnamed ones because it made lines shorter, but now that
we switched to named unions I don't see the point of going back and
patching all drivers again (at the risk of seeing this problem appear
again when compiled with an old compiler version).
next prev parent reply other threads:[~2019-01-22 8:33 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-22 7:42 [PATCH 0/2] mtd: nand: use unnamed union field Masahiro Yamada
2019-01-22 7:42 ` [PATCH 1/2] mtd: rawnand: use unnamed union in struct nand_op_instr Masahiro Yamada
2019-01-22 7:42 ` [PATCH 2/2] mtd: rawnand: use unnamed union in struct nand_op_parser_pattern_elem Masahiro Yamada
2019-01-22 7:49 ` Miquel Raynal
2019-01-22 8:00 ` Masahiro Yamada
2019-01-22 8:08 ` Miquel Raynal
2019-01-22 8:33 ` Boris Brezillon [this message]
2019-01-22 8:55 ` Masahiro Yamada
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=20190122093313.67375948@bbrezillon \
--to=bbrezillon@kernel.org \
--cc=boris.brezillon@bootlin.com \
--cc=computersforpeace@gmail.com \
--cc=dev@lynxeye.de \
--cc=dwmw2@infradead.org \
--cc=jmkrzyszt@gmail.com \
--cc=jonathanh@nvidia.com \
--cc=linus.walleij@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=linux-tegra@vger.kernel.org \
--cc=marek.vasut@gmail.com \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=stefan@agner.ch \
--cc=thierry.reding@gmail.com \
--cc=yamada.masahiro@socionext.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).