Hello,
This patch series contains five simple cleanups and fixes for the rcar-du-drm driver, as well as an argument constification patch for video/of.
The patches themselves are straightforward, see individual commit messages for details. Patch 2/6 (normally merged through the DRM tree) depends on patch 1/6 (normally merged through the fbdev tree). As they don't conflict with patches 3/6 to 6/6, we can either merge the whole series through the DRM tree, or merge patches 1/6 and 2/6 through the fbdev tree and the rest through the DRM tree.
My preference would go for merging the whole series through the DRM tree to avoid potential conflicts with the other patches I'm working on for v4.10. There is no foreseen conflict at the moment, but I might rework encoder handling in the driver that could possibly result in a conflict. Dave, Tomi, any preference ? If you're fine with patches not going through your tree, could you please ack them ?
Cc: David Airlie airlied@linux.ie Cc: Tomi Valkeinen tomi.valkeinen@ti.com
Laurent Pinchart (6): video: of: Constify node argument to display timing functions drm: rcar-du: Constify node argument to rcar_du_lvds_connector_init() drm: rcar-du: Bring HDMI encoder comments in line with the driver drm: rcar-du: Remove test for impossible error condition drm: rcar-du: Remove memory allocation error message drm: rcar-du: Fix crash in encoder failure error path
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 6 ------ drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c | 4 ++-- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 10 +++++----- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 4 +--- drivers/video/of_display_timing.c | 6 +++--- include/video/of_display_timing.h | 15 ++++++++------- 8 files changed, 21 insertions(+), 28 deletions(-)
The node pointer passed to the display timing functions is never modified, make it const.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/video/of_display_timing.c | 6 +++--- include/video/of_display_timing.h | 15 ++++++++------- 2 files changed, 11 insertions(+), 10 deletions(-)
Cc: David Airlie airlied@linux.ie Cc: Tomi Valkeinen tomi.valkeinen@ti.com
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c index 8a1076beecd3..26c88f7839d2 100644 --- a/drivers/video/of_display_timing.c +++ b/drivers/video/of_display_timing.c @@ -110,7 +110,7 @@ static int of_parse_display_timing(const struct device_node *np, * @name: name of the timing node * @dt: display_timing struct to fill **/ -int of_get_display_timing(struct device_node *np, const char *name, +int of_get_display_timing(const struct device_node *np, const char *name, struct display_timing *dt) { struct device_node *timing_np; @@ -133,7 +133,7 @@ EXPORT_SYMBOL_GPL(of_get_display_timing); * of_get_display_timings - parse all display_timing entries from a device_node * @np: device_node with the subnodes **/ -struct display_timings *of_get_display_timings(struct device_node *np) +struct display_timings *of_get_display_timings(const struct device_node *np) { struct device_node *timings_np; struct device_node *entry; @@ -249,7 +249,7 @@ EXPORT_SYMBOL_GPL(of_get_display_timings); * of_display_timings_exist - check if a display-timings node is provided * @np: device_node with the timing **/ -int of_display_timings_exist(struct device_node *np) +int of_display_timings_exist(const struct device_node *np) { struct device_node *timings_np;
diff --git a/include/video/of_display_timing.h b/include/video/of_display_timing.h index ea755b5616d8..956455fc9f9a 100644 --- a/include/video/of_display_timing.h +++ b/include/video/of_display_timing.h @@ -16,21 +16,22 @@ struct display_timings; #define OF_USE_NATIVE_MODE -1
#ifdef CONFIG_OF -int of_get_display_timing(struct device_node *np, const char *name, +int of_get_display_timing(const struct device_node *np, const char *name, struct display_timing *dt); -struct display_timings *of_get_display_timings(struct device_node *np); -int of_display_timings_exist(struct device_node *np); +struct display_timings *of_get_display_timings(const struct device_node *np); +int of_display_timings_exist(const struct device_node *np); #else -static inline int of_get_display_timing(struct device_node *np, const char *name, - struct display_timing *dt) +static inline int of_get_display_timing(const struct device_node *np, + const char *name, struct display_timing *dt) { return -ENOSYS; } -static inline struct display_timings *of_get_display_timings(struct device_node *np) +static inline struct display_timings * +of_get_display_timings(const struct device_node *np) { return NULL; } -static inline int of_display_timings_exist(struct device_node *np) +static inline int of_display_timings_exist(const struct device_node *np) { return -ENOSYS; }
On 04/10/16 15:31, Laurent Pinchart wrote:
The node pointer passed to the display timing functions is never modified, make it const.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
drivers/video/of_display_timing.c | 6 +++--- include/video/of_display_timing.h | 15 ++++++++------- 2 files changed, 11 insertions(+), 10 deletions(-)
Cc: David Airlie airlied@linux.ie Cc: Tomi Valkeinen tomi.valkeinen@ti.com
Reviewed-by: Tomi Valkeinen tomi.valkeinen@ti.com
And ack for merging via drm.
Tomi
The node passed as a pointer to the rcar_du_lvds_connector_init() function is never modified, make it const.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Cc: David Airlie airlied@linux.ie Cc: Tomi Valkeinen tomi.valkeinen@ti.com
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c index 6afd0af312ba..64e9f0b86e58 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c @@ -79,7 +79,7 @@ static const struct drm_connector_funcs connector_funcs = {
int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu, struct rcar_du_encoder *renc, - /* TODO const */ struct device_node *np) + const struct device_node *np) { struct drm_encoder *encoder = rcar_encoder_to_drm_encoder(renc); struct rcar_du_lvds_connector *lvdscon; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h index d4881ee0be7e..639071dd235c 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h @@ -19,6 +19,6 @@ struct rcar_du_encoder;
int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu, struct rcar_du_encoder *renc, - struct device_node *np); + const struct device_node *np);
#endif /* __RCAR_DU_LVDSCON_H__ */
Capitalize acronyms and use determiners and punctuation.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c index e03004f4588d..f9515f53cc5b 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c @@ -108,7 +108,7 @@ int rcar_du_hdmienc_init(struct rcar_du_device *rcdu, if (hdmienc == NULL) return -ENOMEM;
- /* Locate drm bridge from the hdmi encoder DT node */ + /* Locate the DRM bridge from the HDMI encoder DT node. */ bridge = of_drm_find_bridge(np); if (!bridge) return -EPROBE_DEFER; @@ -123,7 +123,7 @@ int rcar_du_hdmienc_init(struct rcar_du_device *rcdu, renc->hdmi = hdmienc; hdmienc->renc = renc;
- /* Link drm_bridge to encoder */ + /* Link the bridge to the encoder. */ bridge->encoder = encoder; encoder->bridge = bridge;
The driver has lost platform data support a long time ago. R-Car DU devices can only be instantiated through DT now, making it impossible to have a NULL DT node pointer. Remove the error check.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 73c971e39b1c..b619a8024ec9 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -294,17 +294,11 @@ static int rcar_du_remove(struct platform_device *pdev)
static int rcar_du_probe(struct platform_device *pdev) { - struct device_node *np = pdev->dev.of_node; struct rcar_du_device *rcdu; struct drm_device *ddev; struct resource *mem; int ret;
- if (np == NULL) { - dev_err(&pdev->dev, "no device tree node\n"); - return -ENODEV; - } - /* Allocate and initialize the DRM and R-Car device structures. */ rcdu = devm_kzalloc(&pdev->dev, sizeof(*rcdu), GFP_KERNEL); if (rcdu == NULL)
Memory allocation failures print messages to the kernel log, there's no need to print an extra one. Remove the duplicate message.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c index ef3a50321ecc..b74105a80a6e 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c @@ -241,10 +241,8 @@ int rcar_du_lvdsenc_init(struct rcar_du_device *rcdu)
for (i = 0; i < rcdu->info->num_lvds; ++i) { lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL); - if (lvds == NULL) { - dev_err(&pdev->dev, "failed to allocate private data\n"); + if (lvds == NULL) return -ENOMEM; - }
lvds->dev = rcdu; lvds->index = i;
When an encoder fails to initialize the driver prints an error message to the kernel log. The message contains the name of the encoder's DT node, which is NULL for internal encoders. Use the of_node_full_name() macro to avoid dereferencing a NULL pointer, print the output number to add more context to the error, and make sure we still own a reference to the encoder's DT node by delaying the of_node_put() call.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index bd9c3bb9252c..d3ab10602e3e 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -445,13 +445,13 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, }
ret = rcar_du_encoder_init(rcdu, enc_type, output, encoder, connector); - of_node_put(encoder); - of_node_put(connector); - if (ret && ret != -EPROBE_DEFER) dev_warn(rcdu->dev, - "failed to initialize encoder %s (%d), skipping\n", - encoder->full_name, ret); + "failed to initialize encoder %s on output %u (%d), skipping\n", + of_node_full_name(encoder), output, ret); + + of_node_put(encoder); + of_node_put(connector);
return ret; }
Hi Laurent,
2016-10-04 Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com:
Hello,
This patch series contains five simple cleanups and fixes for the rcar-du-drm driver, as well as an argument constification patch for video/of.
The patches themselves are straightforward, see individual commit messages for details. Patch 2/6 (normally merged through the DRM tree) depends on patch 1/6 (normally merged through the fbdev tree). As they don't conflict with patches 3/6 to 6/6, we can either merge the whole series through the DRM tree, or merge patches 1/6 and 2/6 through the fbdev tree and the rest through the DRM tree.
My preference would go for merging the whole series through the DRM tree to avoid potential conflicts with the other patches I'm working on for v4.10. There is no foreseen conflict at the moment, but I might rework encoder handling in the driver that could possibly result in a conflict. Dave, Tomi, any preference ? If you're fine with patches not going through your tree, could you please ack them ?
Cc: David Airlie airlied@linux.ie Cc: Tomi Valkeinen tomi.valkeinen@ti.com
Laurent Pinchart (6): video: of: Constify node argument to display timing functions drm: rcar-du: Constify node argument to rcar_du_lvds_connector_init() drm: rcar-du: Bring HDMI encoder comments in line with the driver drm: rcar-du: Remove test for impossible error condition drm: rcar-du: Remove memory allocation error message drm: rcar-du: Fix crash in encoder failure error path
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 6 ------ drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c | 4 ++-- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 10 +++++----- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 4 +--- drivers/video/of_display_timing.c | 6 +++--- include/video/of_display_timing.h | 15 ++++++++------- 8 files changed, 21 insertions(+), 28 deletions(-)
For 1 to 6
Reviewed-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
Gustavo
Hi Tomi and Dave,
On Tuesday 04 Oct 2016 15:31:33 Laurent Pinchart wrote:
Hello,
This patch series contains five simple cleanups and fixes for the rcar-du-drm driver, as well as an argument constification patch for video/of.
The patches themselves are straightforward, see individual commit messages for details. Patch 2/6 (normally merged through the DRM tree) depends on patch 1/6 (normally merged through the fbdev tree). As they don't conflict with patches 3/6 to 6/6, we can either merge the whole series through the DRM tree, or merge patches 1/6 and 2/6 through the fbdev tree and the rest through the DRM tree.
My preference would go for merging the whole series through the DRM tree to avoid potential conflicts with the other patches I'm working on for v4.10. There is no foreseen conflict at the moment, but I might rework encoder handling in the driver that could possibly result in a conflict. Dave, Tomi, any preference ? If you're fine with patches not going through your tree, could you please ack them ?
Ping ? Tomi, would you be fine with merging 1/6 through the DRM tree ? If so, could you please ack it ?
Cc: David Airlie airlied@linux.ie Cc: Tomi Valkeinen tomi.valkeinen@ti.com
Laurent Pinchart (6): video: of: Constify node argument to display timing functions drm: rcar-du: Constify node argument to rcar_du_lvds_connector_init() drm: rcar-du: Bring HDMI encoder comments in line with the driver drm: rcar-du: Remove test for impossible error condition drm: rcar-du: Remove memory allocation error message drm: rcar-du: Fix crash in encoder failure error path
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 6 ------ drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c | 4 ++-- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 10 +++++----- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 4 +--- drivers/video/of_display_timing.c | 6 +++--- include/video/of_display_timing.h | 15 ++++++++------- 8 files changed, 21 insertions(+), 28 deletions(-)
dri-devel@lists.freedesktop.org
-
Gustavo Padovan -
Laurent Pinchart -
Laurent Pinchart -
Tomi Valkeinen