imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id
Subsystems (2)
Fabio Estevam
reviewerPATCH 1/5 imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id
From: Fabio Estevam <[email protected]>
When building imx_v6_v7_defconfig with imx-drm drivers selected as modules, we get the following build error:
ERROR: "imx_drm_encoder_get_mux_id" [drivers/staging/imx-drm/imx-ldb.ko] undefined!
Export the required function to avoid this problem.
--- drivers/staging/imx-drm/imx-drm-core.c | 1 + 1 file changed, 1 insertion(+)
drivers/staging/imx-drm/imx-drm-core.c +1 -0
@@ -678,6 +678,7 @@ found:return i; } +EXPORT_SYMBOL_GPL (imx_drm_encoder_get_mux_id);/* * imx_drm_remove_encoder - remove an encoder
Signature · 1 line
1.8.1.2
Fabio Estevam
reviewerPATCH 2/5 imx-drm: ipu-dp: Check the return value of devm_kzalloc()
From: Fabio Estevam <[email protected]>
devm_kzalloc() may fail, so let's check its return value.
--- drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/staging/imx-drm/ipu-v3/ipu-dp.c +2 -0
@@ -309,6 +309,8 @@ int ipu_dp_init(struct ipu_soc *ipu, struct device *dev, unsigned long base)int i; priv =devm_kzalloc (dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; priv->dev = dev; priv->ipu = ipu;
Signature · 1 line
1.8.1.2
Fabio Estevam
reviewerPATCH 3/5 imx-drm: ipu-dp: Remove unneeded braces
In reply to Fabio EstevamFrom: Fabio Estevam <[email protected]>
No need to have braces for a single line 'if' block
--- drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/imx-drm/ipu-v3/ipu-dp.c +1 -2
@@ -317,9 +317,8 @@ int ipu_dp_init(struct ipu_soc *ipu, struct device *dev, unsigned long base) ipu->dp_priv = priv; priv->base =devm_ioremap (dev, base,PAGE_SIZE ); -if (!priv->base) { +if (!priv->base)return -ENOMEM ; - }mutex_init (&priv->mutex);
Signature · 1 line
1.8.1.2
Fabio Estevam
reviewerPATCH 4/5 imx-drm: ipu-dp: Introduce IPUV3_NUM_FLOWS
In reply to Fabio EstevamFrom: Fabio Estevam <[email protected]>
IPUv3 has a total of 3 flows (one synchronous flow and 2 asynchronous flows).
Let's add a definition for such number in order to let the code easier to understand.
--- drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/imx-drm/ipu-v3/ipu-dp.c +4 -2
@@ -46,6 +46,8 @@#define DP_COM_CONF_CSC_DEF_BG (2 << 8)#define DP_COM_CONF_CSC_DEF_BOTH (1 << 8) +#define IPUV3_NUM_FLOWS 3 +struct ipu_dp_priv ;struct ipu_dp {
@@ -67,7 +69,7 @@ struct ipu_dp_priv {struct ipu_soc *ipu;struct device *dev;void __iomem *base; -struct ipu_flow flow[3 ]; +struct ipu_flow flow[IPUV3_NUM_FLOWS ];struct mutex mutex;int use_count; };
@@ -322,7 +324,7 @@ int ipu_dp_init(struct ipu_soc *ipu, struct device *dev, unsigned long base)mutex_init (&priv->mutex); -for (i =0 ; i <3 ; i++) { +for (i =0 ; i <IPUV3_NUM_FLOWS ; i++) { priv->flow[i].foreground.foreground =1 ; priv->flow[i].base = priv->base + ipu_dp_flow_base[i]; priv->flow[i].priv = priv;
Signature · 1 line
1.8.1.2
Fabio Estevam
reviewerPATCH 5/5 imx-drm: ipu-dp: Adjust the maximum number of flows
In reply to Fabio EstevamFrom: Fabio Estevam <[email protected]>
Later in ipu_dp_get() the index of the flow array is calculated by:
flow >> 1
So adjust its maximum to IPUV3_NUM_FLOWS << 1.
--- drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/imx-drm/ipu-v3/ipu-dp.c +1 -1
@@ -282,7 +282,7 @@ struct ipu_dp *ipu_dp_get(struct ipu_soc *ipu, unsigned int flow)struct ipu_dp_priv *priv = ipu->dp_priv;struct ipu_dp *dp; -if (flow >5 ) +if (flow >IPUV3_NUM_FLOWS <<1 )return ERR_PTR (-EINVAL );if (flow &1 )
Signature · 1 line
1.8.1.2
Re: [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows
Quoted 6 lines · Fabio Estevam
From: Fabio Estevam <[email protected]>
Later in
ipu_dp_get()the index of the flow array is calculated by:
flow >> 1So adjust its maximum to
IPUV3_NUM_FLOWS<< 1.
Maybe squash this into patch 4?
Uwe
Quoted 23 lines
Signed-off-by Fabio Estevam <[email protected]>--- drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/staging/imx-drm/ipu-v3/ipu-dp.c +1
-1@@ -282,7 +282,7 @@ struct ipu_dp *ipu_dp_get(struct ipu_soc *ipu, unsigned int flow)struct ipu_dp_priv *priv = ipu->dp_priv;struct ipu_dp *dp; -if (flow >5 ) +if (flow >IPUV3_NUM_FLOWS <<1 )return ERR_PTR (-EINVAL );if (flow &1 )Signature · 1 line
1.8.1.2
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
Fabio Estevam
reviewerRe: [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows
Hi Uwe,
Quoted 8 lines · Uwe Kleine-König
Quoted 6 lines · Fabio Estevam
From: Fabio Estevam <[email protected]>
Later in
ipu_dp_get()the index of the flow array is calculated by:
flow >> 1So adjust its maximum to
IPUV3_NUM_FLOWS<< 1.Maybe squash this into patch 4?
Patch 4 is just cosmetic, but in this one I am changing the comparison from:
'if (flow > 5)' to 'if (flow > 6)'
,so that's why I splitted them.
I am OK with squashing the two patches together though.
Sascha,
Do you agree with the 'if (flow > 6)' change?
Thanks,
Fabio Estevam
Re: [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows
In reply to Fabio EstevamHi Fabio,
Am Freitag, den 28.06.2013, 00:32 -0300 schrieb Fabio Estevam:
Quoted 17 lines
From: Fabio Estevam <[email protected]>
Later in
ipu_dp_get()the index of the flow array is calculated by:
flow >> 1So adjust its maximum to
IPUV3_NUM_FLOWS<< 1.Signed-off-by Fabio Estevam <[email protected]>--- drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/staging/imx-drm/ipu-v3/ipu-dp.c +0
-0
drivers/staging/imx-drm/ipu-v3/ipu-dp.c struct ipu_dp *ipu_dp_get(struct ipu_soc *ipu, unsigned int flow) · line 285
Quoted 5 lines
drivers/staging/imx-drm/ipu-v3/ipu-dp.c +1
-1@@ -282,7 +282,7 @@ struct ipu_dp *ipu_dp_get(struct ipu_soc *ipu, unsigned int flow)struct ipu_dp_priv *priv = ipu->dp_priv;struct ipu_dp *dp; -if (flow >5 ) +if (flow >IPUV3_NUM_FLOWS <<1 )
If (flow == IPUV3_NUM_FLOWS << 1), this will continue and try to access &priv->flow[3] below, which is invalid. How about
+ if ((flow >> 1) >= IPUV3_NUM_FLOWS)
instead?
drivers/staging/imx-drm/ipu-v3/ipu-dp.c struct ipu_dp *ipu_dp_get(struct ipu_soc *ipu, unsigned int flow) · line 288
Quoted 3 lines
drivers/staging/imx-drm/ipu-v3/ipu-dp.c +0
-0return ERR_PTR (-EINVAL );if (flow &1 )
regards
Philipp
Fabio Estevam
reviewerRe: [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows
Hi Philipp,
Quoted 7 lines · Philipp Zabel
If (flow ==
IPUV3_NUM_FLOWS<< 1), this will continue and try to access &priv->flow[3] below, which is invalid. How about+ if ((flow >> 1) >=
IPUV3_NUM_FLOWS)instead?
Yes, agreed.
Thanks,
Fabio Estevam
Quoted 6 lines · Fabio Estevam
From: Fabio Estevam <[email protected]>
When building
imx_v6_v7_defconfigwith imx-drm drivers selected as modules, we get the following build error:ERROR: "
imx_drm_encoder_get_mux_id" [drivers/staging/imx-drm/imx-ldb.ko] undefined!
So imx-ldb.c needs a symbol from imx-drm-core.c right? The alternative to exporting this symbol for a single user(?) is to link these two files into a single module. Does this make sense? If yes, that would IMHO be the better fix.
Best regards
Uwe
Quoted 24 lines
Export the required function to avoid this problem.
Signed-off-by Fabio Estevam <[email protected]>--- drivers/staging/imx-drm/imx-drm-core.c | 1 + 1 file changed, 1 insertion(+)drivers/staging/imx-drm/imx-drm-core.c +1
-0@@ -678,6 +678,7 @@ found:return i; } +EXPORT_SYMBOL_GPL (imx_drm_encoder_get_mux_id);/* * imx_drm_remove_encoder - remove an encoder Signature · 1 line
1.8.1.2
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
Fabio Estevam
reviewerHi Uwe,
Quoted 11 lines · Uwe Kleine-König
Quoted 6 lines · Fabio Estevam
From: Fabio Estevam <[email protected]>
When building
imx_v6_v7_defconfigwith imx-drm drivers selected as modules, we get the following build error:ERROR: "
imx_drm_encoder_get_mux_id" [drivers/staging/imx-drm/imx-ldb.ko] undefined!So
imx-ldb.cneeds a symbol fromimx-drm-core.cright? The alternative to exporting this symbol for a single user(?) is to link these two files into a single module. Does this make sense? If yes, that would IMHO be the better fix.
The Makefile for these files is:
imxdrm-objs := imx-drm-core.o imx-fb.o
obj-$(CONFIG_DRM_IMX) += imxdrm.o
obj-$(CONFIG_DRM_IMX_PARALLEL_DISPLAY) += parallel-display.o obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o obj-$(CONFIG_DRM_IMX_FB_HELPER) += imx-fbdev.o obj-$(CONFIG_DRM_IMX_IPUV3_CORE) += ipu-v3/
obj-$(CONFIG_DRM_IMX_IPUV3) += ipuv3-crtc.o
so imx-drm-core is always built and imx-ldb is only built when CONFIG_DRM_IMX_LDB is selected.
In this case, we should keep the two modules separate and fix this error using the export.
Regards,
Fabio Estevam