Re: [PATCH 1/2] OMAP4: HDMI: Add OMAP device for HDMI audio CPU DAI

[Steve Calfee <stevecalfee@gmail.com>]

On 05/16/11 20:14, Ricardo Neri wrote:

> +++ b/arch/arm/mach-omap2/devices.c
> @@ -313,6 +313,22 @@ OMAP_MCBSP_PLATFORM_DEVICE(5);
>  
>  static void omap_init_audio(void)
>  {
> +	struct omap_hwmod *oh_hdmi;
> +	struct omap_device *od_hdmi;
> +	char *oh_hdmi_name = "dss_hdmi";
> +	char *dev_hdmi_name = "hdmi-audio-dai";
> +
> +	if (cpu_is_omap44xx()) {
> +		oh_hdmi = omap_hwmod_lookup(oh_hdmi_name);
> +		WARN(!oh_hdmi, "%s: could not find omap_hwmod for %s\n",
> +			__func__, oh_hdmi_name);
> +
> +		od_hdmi = omap_device_build(dev_hdmi_name, -1, oh_hdmi, NULL, 0,
> +			NULL, 0, false);
> +		WARN(IS_ERR(od_hdmi), "%s: could not build omap_device for %s\n",
> +			__func__, dev_hdmi_name);
> +	}
> +
>  	platform_device_register(&omap_mcbsp1);
>  	platform_device_register(&omap_mcbsp2);
>  	if (cpu_is_omap243x() || cpu_is_omap34xx() || cpu_is_omap44xx()) {

I know you did not start this, but this cpu_is stuff is cheating. There
is a rule (maybe a guideline, or desire) in the kernel where they try to
minimize #ifdef in c code. So here we have a runtime ifdef. The code
will never be executed on other omap versions, but it takes up space and
obscures the code flow.

I think the generally accepted method of doing stuff like this is to
have the ifdeffery in a header file where a inline code segment is
defined if it applies to the processor being built. If the code does not
apply to the model being built, a null #define is used, which does not
take any space.

Using a conditional inline enables the only source code change for this
file (device.c) being a line where the inline code is called.

Regards, Steve