From: Arnd Bergmann <arnd@arndb.de>
To: linux-arm-kernel@lists.infradead.org
Cc: Sascha Hauer <s.hauer@pengutronix.de>,
Arnaud Patard <arnaud.patard@rtp-net.org>,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
"Zhang Lily-R58066" <r58066@freescale.com>
Subject: Re: [PATCH 6/9] ARM i.MX51: Add IPU device support
Date: Wed, 15 Dec 2010 16:49:45 +0100 [thread overview]
Message-ID: <201012151649.45199.arnd@arndb.de> (raw)
In-Reply-To: <1291902441-24712-7-git-send-email-s.hauer@pengutronix.de>
On Thursday 09 December 2010, Sascha Hauer wrote:
> +#define imx51_add_ipuv3(pdata) \
> + imx_add_ipuv3(&imx51_ipuv3_data, pdata)
This looks like a pointless abstraction, it does not make
the code smaller or easier to read. I know it's sometimes
tempting to use macros, but in most cases, you should try
not to.
> +#define imx51_ipuv3_data_entry_single(soc) \
> + { \
> + .iobase = soc ## _IPU_CTRL_BASE_ADDR, \
> + .irq_err = soc ## _INT_IPU_ERR, \
> + .irq = soc ## _INT_IPU_SYN, \
> + }
> +
> +#ifdef CONFIG_SOC_IMX51
> +const struct imx_ipuv3_data imx51_ipuv3_data __initconst =
> + imx51_ipuv3_data_entry_single(MX51);
> +#endif /* ifdef CONFIG_SOC_IMX35 */
This looks really strange: You define a macro that is used
in only a single place, and the only user is a data structure
that references data from other files and is used elsewhere
as well.
Avoiding the macro would make it easier to grep for the use
of the identifiers.
If you put the data structure in the place where it is used,
you can avoid the #ifdef and make it static.
Also, the comment on the #endif does not match the #if.
Arnd
WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 6/9] ARM i.MX51: Add IPU device support
Date: Wed, 15 Dec 2010 15:49:45 +0000 [thread overview]
Message-ID: <201012151649.45199.arnd@arndb.de> (raw)
In-Reply-To: <1291902441-24712-7-git-send-email-s.hauer@pengutronix.de>
On Thursday 09 December 2010, Sascha Hauer wrote:
> +#define imx51_add_ipuv3(pdata) \
> + imx_add_ipuv3(&imx51_ipuv3_data, pdata)
This looks like a pointless abstraction, it does not make
the code smaller or easier to read. I know it's sometimes
tempting to use macros, but in most cases, you should try
not to.
> +#define imx51_ipuv3_data_entry_single(soc) \
> + { \
> + .iobase = soc ## _IPU_CTRL_BASE_ADDR, \
> + .irq_err = soc ## _INT_IPU_ERR, \
> + .irq = soc ## _INT_IPU_SYN, \
> + }
> +
> +#ifdef CONFIG_SOC_IMX51
> +const struct imx_ipuv3_data imx51_ipuv3_data __initconst > + imx51_ipuv3_data_entry_single(MX51);
> +#endif /* ifdef CONFIG_SOC_IMX35 */
This looks really strange: You define a macro that is used
in only a single place, and the only user is a data structure
that references data from other files and is used elsewhere
as well.
Avoiding the macro would make it easier to grep for the use
of the identifiers.
If you put the data structure in the place where it is used,
you can avoid the #ifdef and make it static.
Also, the comment on the #endif does not match the #if.
Arnd
WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 6/9] ARM i.MX51: Add IPU device support
Date: Wed, 15 Dec 2010 16:49:45 +0100 [thread overview]
Message-ID: <201012151649.45199.arnd@arndb.de> (raw)
In-Reply-To: <1291902441-24712-7-git-send-email-s.hauer@pengutronix.de>
On Thursday 09 December 2010, Sascha Hauer wrote:
> +#define imx51_add_ipuv3(pdata) \
> + imx_add_ipuv3(&imx51_ipuv3_data, pdata)
This looks like a pointless abstraction, it does not make
the code smaller or easier to read. I know it's sometimes
tempting to use macros, but in most cases, you should try
not to.
> +#define imx51_ipuv3_data_entry_single(soc) \
> + { \
> + .iobase = soc ## _IPU_CTRL_BASE_ADDR, \
> + .irq_err = soc ## _INT_IPU_ERR, \
> + .irq = soc ## _INT_IPU_SYN, \
> + }
> +
> +#ifdef CONFIG_SOC_IMX51
> +const struct imx_ipuv3_data imx51_ipuv3_data __initconst =
> + imx51_ipuv3_data_entry_single(MX51);
> +#endif /* ifdef CONFIG_SOC_IMX35 */
This looks really strange: You define a macro that is used
in only a single place, and the only user is a data structure
that references data from other files and is used elsewhere
as well.
Avoiding the macro would make it easier to grep for the use
of the identifiers.
If you put the data structure in the place where it is used,
you can avoid the #ifdef and make it static.
Also, the comment on the #endif does not match the #if.
Arnd
next prev parent reply other threads:[~2010-12-15 15:50 UTC|newest]
Thread overview: 110+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-09 13:47 [PATCH RFC] i.MX51 Framebuffer support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 1/9] ARM i.MX51: Add ipu clock support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-15 15:40 ` Arnd Bergmann
2010-12-15 15:40 ` Arnd Bergmann
2010-12-15 15:40 ` Arnd Bergmann
2010-12-15 16:34 ` Russell King - ARM Linux
2010-12-15 16:34 ` Russell King - ARM Linux
2010-12-15 16:34 ` Russell King - ARM Linux
2010-12-15 16:49 ` Arnd Bergmann
2010-12-15 16:49 ` Arnd Bergmann
2010-12-15 16:49 ` Arnd Bergmann
2010-12-15 17:12 ` Russell King - ARM Linux
2010-12-15 17:12 ` Russell King - ARM Linux
2010-12-15 17:12 ` Russell King - ARM Linux
2010-12-09 13:47 ` [PATCH 2/9] ARM i.MX51: rename IPU irqs Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 14:34 ` Uwe Kleine-König
2010-12-09 14:34 ` Uwe Kleine-König
2010-12-09 14:34 `
2010-12-09 13:47 ` [PATCH 3/9] Add a mfd IPUv3 driver Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-12 5:21 ` Liu Ying
2010-12-12 5:21 ` Liu Ying
2010-12-13 11:23 ` Sascha Hauer
2010-12-13 11:23 ` Sascha Hauer
2010-12-13 11:23 ` Sascha Hauer
2010-12-14 4:05 ` Liu Ying
2010-12-14 4:05 ` Liu Ying
2010-12-14 4:05 ` Liu Ying
2010-12-14 8:40 ` Sascha Hauer
2010-12-14 8:40 ` Sascha Hauer
2010-12-14 8:40 ` Sascha Hauer
2010-12-14 13:13 ` Liu Ying
2010-12-14 13:13 ` Liu Ying
2010-12-14 13:13 ` Liu Ying
2010-12-09 13:47 ` [PATCH 4/9] fb: export fb mode db table Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2011-01-06 7:26 ` Paul Mundt
2011-01-06 7:26 ` Paul Mundt
2011-01-06 7:26 ` Paul Mundt
2011-01-06 10:04 ` Sascha Hauer
2011-01-06 10:04 ` Sascha Hauer
2011-01-06 10:04 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 5/9] Add i.MX5 framebuffer driver Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-12 6:13 ` Liu Ying
2010-12-12 6:13 ` Liu Ying
2010-12-12 6:13 ` Liu Ying
2010-12-13 7:23 ` Lothar Waßmann
2010-12-13 7:23 ` Lothar Waßmann
2010-12-13 7:23 ` Lothar Waßmann
2010-12-13 11:35 ` Liu Ying
2010-12-13 11:35 ` Liu Ying
2010-12-13 11:35 ` Liu Ying
2010-12-13 11:38 ` Sascha Hauer
2010-12-13 11:38 ` Sascha Hauer
2010-12-13 11:38 ` Sascha Hauer
2010-12-14 6:40 ` Liu Ying
2010-12-14 6:40 ` Liu Ying
2010-12-14 6:40 ` Liu Ying
2010-12-14 8:45 ` Sascha Hauer
2010-12-14 8:45 ` Sascha Hauer
2010-12-14 8:45 ` Sascha Hauer
2010-12-14 13:23 ` Liu Ying
2010-12-14 13:23 ` Liu Ying
2010-12-14 13:23 ` Liu Ying
2010-12-15 11:17 ` Sascha Hauer
2010-12-15 11:17 ` Sascha Hauer
2010-12-15 11:17 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 6/9] ARM i.MX51: Add IPU device support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-15 15:49 ` Arnd Bergmann [this message]
2010-12-15 15:49 ` Arnd Bergmann
2010-12-15 15:49 ` Arnd Bergmann
2010-12-15 16:26 ` Arnaud Patard
2010-12-15 16:26 ` Arnaud Patard (Rtp)
2010-12-15 16:26 ` Arnaud Patard
2010-12-15 16:29 ` Arnd Bergmann
2010-12-15 16:29 ` Arnd Bergmann
2010-12-15 16:29 ` Arnd Bergmann
2010-12-09 13:47 ` [PATCH 7/9] ARM i.MX5: Allow to increase max zone order Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 8/9] ARM i.MX5: increase dma consistent size for IPU support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 9/9] ARM i.MX51 babbage: Add framebuffer support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-12 1:37 ` Liu Ying
2010-12-12 1:37 ` Liu Ying
2010-12-12 1:37 ` Liu Ying
2010-12-13 11:43 ` Sascha Hauer
2010-12-13 11:43 ` Sascha Hauer
2010-12-13 11:43 ` Sascha Hauer
2010-12-14 6:47 ` Liu Ying
2010-12-14 6:47 ` Liu Ying
2010-12-14 6:47 ` Liu Ying
2010-12-15 9:35 ` [PATCH RFC] i.MX51 Framebuffer support Peter Horton
2010-12-15 11:24 ` Sascha Hauer
2010-12-20 10:48 [PATCH v2] " Sascha Hauer
2010-12-20 10:48 ` [PATCH 6/9] ARM i.MX51: Add IPU device support Sascha Hauer
2010-12-20 10:48 ` Sascha Hauer
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=201012151649.45199.arnd@arndb.de \
--to=arnd@arndb.de \
--cc=arnaud.patard@rtp-net.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=r58066@freescale.com \
--cc=s.hauer@pengutronix.de \
/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.