* [PATCH 2/5] imx-drm: ipu-dp: Check the return value of devm_kzalloc()
2013-06-28 3:32 [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id Fabio Estevam
@ 2013-06-28 3:32 ` Fabio Estevam
2013-06-28 3:32 ` [PATCH 3/5] imx-drm: ipu-dp: Remove unneeded braces Fabio Estevam
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Fabio Estevam @ 2013-06-28 3:32 UTC (permalink / raw)
To: gregkh; +Cc: shawn.guo, kernel, linux-kernel, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
devm_kzalloc() may fail, so let's check its return value.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
index 113b046..0449515 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
@@ -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;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] imx-drm: ipu-dp: Remove unneeded braces
2013-06-28 3:32 [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id Fabio Estevam
2013-06-28 3:32 ` [PATCH 2/5] imx-drm: ipu-dp: Check the return value of devm_kzalloc() Fabio Estevam
@ 2013-06-28 3:32 ` Fabio Estevam
2013-06-28 3:32 ` [PATCH 4/5] imx-drm: ipu-dp: Introduce IPUV3_NUM_FLOWS Fabio Estevam
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Fabio Estevam @ 2013-06-28 3:32 UTC (permalink / raw)
To: gregkh; +Cc: shawn.guo, kernel, linux-kernel, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
No need to have braces for a single line 'if' block
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
index 0449515..6e0e9f1 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
@@ -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);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/5] imx-drm: ipu-dp: Introduce IPUV3_NUM_FLOWS
2013-06-28 3:32 [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id Fabio Estevam
2013-06-28 3:32 ` [PATCH 2/5] imx-drm: ipu-dp: Check the return value of devm_kzalloc() Fabio Estevam
2013-06-28 3:32 ` [PATCH 3/5] imx-drm: ipu-dp: Remove unneeded braces Fabio Estevam
@ 2013-06-28 3:32 ` Fabio Estevam
2013-06-28 3:32 ` [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows Fabio Estevam
2013-06-28 7:29 ` [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id Uwe Kleine-König
4 siblings, 0 replies; 11+ messages in thread
From: Fabio Estevam @ 2013-06-28 3:32 UTC (permalink / raw)
To: gregkh; +Cc: shawn.guo, kernel, linux-kernel, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
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.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
index 6e0e9f1..3bdff6af 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
@@ -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;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows
2013-06-28 3:32 [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id Fabio Estevam
` (2 preceding siblings ...)
2013-06-28 3:32 ` [PATCH 4/5] imx-drm: ipu-dp: Introduce IPUV3_NUM_FLOWS Fabio Estevam
@ 2013-06-28 3:32 ` Fabio Estevam
2013-06-28 7:30 ` Uwe Kleine-König
2013-06-28 13:53 ` Philipp Zabel
2013-06-28 7:29 ` [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id Uwe Kleine-König
4 siblings, 2 replies; 11+ messages in thread
From: Fabio Estevam @ 2013-06-28 3:32 UTC (permalink / raw)
To: gregkh; +Cc: shawn.guo, kernel, linux-kernel, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
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.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
index 3bdff6af..ae2c199 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
@@ -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)
--
1.8.1.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows
2013-06-28 3:32 ` [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows Fabio Estevam
@ 2013-06-28 7:30 ` Uwe Kleine-König
2013-06-28 13:25 ` Fabio Estevam
2013-06-28 13:53 ` Philipp Zabel
1 sibling, 1 reply; 11+ messages in thread
From: Uwe Kleine-König @ 2013-06-28 7:30 UTC (permalink / raw)
To: Fabio Estevam; +Cc: gregkh, shawn.guo, kernel, linux-kernel, Fabio Estevam
On Fri, Jun 28, 2013 at 12:32:05AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> 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.
Maybe squash this into patch 4?
Uwe
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
> index 3bdff6af..ae2c199 100644
> --- a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
> +++ b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
> @@ -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)
> --
> 1.8.1.2
>
>
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows
2013-06-28 7:30 ` Uwe Kleine-König
@ 2013-06-28 13:25 ` Fabio Estevam
0 siblings, 0 replies; 11+ messages in thread
From: Fabio Estevam @ 2013-06-28 13:25 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: gregkh, shawn.guo, kernel, linux-kernel, Fabio Estevam
Hi Uwe,
On Fri, Jun 28, 2013 at 4:30 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Fri, Jun 28, 2013 at 12:32:05AM -0300, Fabio Estevam wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> 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.
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows
2013-06-28 3:32 ` [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows Fabio Estevam
2013-06-28 7:30 ` Uwe Kleine-König
@ 2013-06-28 13:53 ` Philipp Zabel
2013-06-28 14:09 ` Fabio Estevam
1 sibling, 1 reply; 11+ messages in thread
From: Philipp Zabel @ 2013-06-28 13:53 UTC (permalink / raw)
To: Fabio Estevam; +Cc: gregkh, shawn.guo, kernel, linux-kernel, Fabio Estevam
Hi Fabio,
Am Freitag, den 28.06.2013, 00:32 -0300 schrieb Fabio Estevam:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> 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.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/staging/imx-drm/ipu-v3/ipu-dp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
> index 3bdff6af..ae2c199 100644
> --- a/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
> +++ b/drivers/staging/imx-drm/ipu-v3/ipu-dp.c
> @@ -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?
> return ERR_PTR(-EINVAL);
>
> if (flow & 1)
regards
Philipp
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows
2013-06-28 13:53 ` Philipp Zabel
@ 2013-06-28 14:09 ` Fabio Estevam
0 siblings, 0 replies; 11+ messages in thread
From: Fabio Estevam @ 2013-06-28 14:09 UTC (permalink / raw)
To: Philipp Zabel; +Cc: gregkh, shawn.guo, kernel, linux-kernel, Fabio Estevam
Hi Philipp,
On Fri, Jun 28, 2013 at 10:53 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id
2013-06-28 3:32 [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id Fabio Estevam
` (3 preceding siblings ...)
2013-06-28 3:32 ` [PATCH 5/5] imx-drm: ipu-dp: Adjust the maximum number of flows Fabio Estevam
@ 2013-06-28 7:29 ` Uwe Kleine-König
2013-06-28 14:43 ` Fabio Estevam
4 siblings, 1 reply; 11+ messages in thread
From: Uwe Kleine-König @ 2013-06-28 7:29 UTC (permalink / raw)
To: Fabio Estevam; +Cc: gregkh, shawn.guo, kernel, linux-kernel, Fabio Estevam
On Fri, Jun 28, 2013 at 12:32:01AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> 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!
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
>
> Export the required function to avoid this problem.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/staging/imx-drm/imx-drm-core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
> index 9854a1d..6bc205b 100644
> --- a/drivers/staging/imx-drm/imx-drm-core.c
> +++ b/drivers/staging/imx-drm/imx-drm-core.c
> @@ -678,6 +678,7 @@ found:
>
> return i;
> }
> +EXPORT_SYMBOL_GPL(imx_drm_encoder_get_mux_id);
>
> /*
> * imx_drm_remove_encoder - remove an encoder
> --
> 1.8.1.2
>
>
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id
2013-06-28 7:29 ` [PATCH 1/5] imx-drm: imx-drm-core: Export imx_drm_encoder_get_mux_id Uwe Kleine-König
@ 2013-06-28 14:43 ` Fabio Estevam
0 siblings, 0 replies; 11+ messages in thread
From: Fabio Estevam @ 2013-06-28 14:43 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: gregkh, shawn.guo, kernel, linux-kernel, Fabio Estevam
Hi Uwe,
On Fri, Jun 28, 2013 at 4:29 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Fri, Jun 28, 2013 at 12:32:01AM -0300, Fabio Estevam wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> 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!
> 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.
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
^ permalink raw reply [flat|nested] 11+ messages in thread