Re: [PATCH v7 05/13] media: tvp5150: add input source selection of_graph support
Hi, sorry for the noise but this patch has two bugs. First is a rebasing issue and the second a wrong #ifdef handling. I will prepare a v8 addressing Hans comments and fixing those issues. Please let me know if you have more comments for this patch. Regards, Marco On 19-08-15 13:57, Marco Felsch wrote: > This patch adds the of_graph support to describe the tvp input connections. > Physical the TVP5150 has three ports: AIP1A, AIP1B and YOUT. As result > of discussion [1],[2] the device-tree maps these ports 1:1. Look at the > Documentation for more information. Since the TVP5150 is a converter/bridge > the device-tree must contain at least 1-input and 1-output port. The > mc-connectors and mc-links are only created if the device-tree contains the > corresponding connector nodes. If more than one connector is available the > media_entity_operations.link_setup() callback ensures that only one > connector is active. > > [1] https://www.spinics.net/lists/linux-media/msg138545.html > [2] https://www.spinics.net/lists/linux-media/msg138546.html > > Signed-off-by: Marco Felsch > > --- > Changelog: > > [1] https://patchwork.kernel.org/cover/10794703/ > [2] https://patchwork.kernel.org/cover/10786553/ > > v7: > - don't init enum tvp5150_pads with TVP5150_COMPOSITE0 functionality > - break some 80 character limitation to improve readability > - fix comment style -> always start with capital letters and end with dot > - fix some minor style issues > - fix tvp5150_registered error handling > - simplify tvp5150_mc_init > - make connectors static > -> since now only three connectors are possible (as described in DT) > -> drop tvp5150.endpoints storage > -> squash tvp5150_parse_dt and tvp5150_add_of_connectors > - improve tvp5150_parse_dt: > -> make parsing stricter and fix not detected missconfigured dt-data > -> svideo must be connected now to port@[0,1]/endpoint@1 > > v6: > - fix misspelled comments > - use 'unsigned int' where it's possible > - cleanup ifdef part-2: > * tvp5150_mc_init, tvp5150_add_of_connectors: add surrounding > CONFIG_MEDIA_CONTROLLER if def and stubs to improve quality > - tvp5150_mc_init: uniform interface, use 'struct tvp5150' since all > internal function do this. > - tvp5150_add_of_connectors: call within probe() to make it cleaner > - tvp5150_parse_dt: move local loop vars within the loop. > > v5: > - Fixing build deps: > - tvp5150_mc_init: fix CONFIG_MEDIA_CONTROLLER deps > - struct tvp5150: drop CONFIG_MEDIA_CONTROLLER conditional property > includes. This leads into to complex deps for futher development. > - tvp5150_dt_cleanup: enable function only if CONFIG_OF is enabled > - tvp5150_parse_dt: enable function only if CONFIG_OF is enabled > - tvp5150_probe: call tvp5150_dt_cleanup only if CONFIG_OF is enabled > > - Simplify link_setup routine: > - use generic connector parsing since both series [1,2] are squashed into > one > - struct tvp5150: drop pads_state and modify_second_link property > due to link_setup() rework. > - tvp5150_link_setup: add more comments > - tvp5150_link_setup: simply the link setup routine a lot. Edit the 2nd > link directly within the driver instead of a recursive media-framework > call (__media_entity_setup_link). This improves the readability and > shrinks the driver code. > - tvp5150_link_setup: disable all active links in case user switches > connectors without disable it first. > - tvp5150_registered: simplify default link enable path due to link_setup() > rework. > > - General cleanups > - tvp5150_parse_dt: drop unecessary test > - tvp5150_parse_dt: add err message due to misconfiguration > - tvp5150_parse_dt: make use of V4L2_MBUS_UNKNOWN definition > - s/dev_dbg/dev_dbg_lvl > > v4: > - rebase on top of media_tree/master, fix merge conflict due to commit >60359a28d592 ("media: v4l: fwnode: Initialise the V4L2 fwnode endpoints >to zero") > > v3: > - probe(): s/err/err_free_v4l2_ctrls > - drop MC dependency for tvp5150_pads > > v2: > - adapt commit message > - unify ifdef switches > - rename tvp5150_valid_input -> tvp5150_of_valid_input, to be more precise > - mc: use 2-input and 1-output pad > - mc: link svideo connector to both input pads > - mc: enable/disable svideo links in one go > - mc: change link_setup() behaviour, switch the input src don't require a > explicite disable before. > - mc: rename 'local' media_pad param to tvp5150_pad to avoid confusion > - mc: enable link to the first available connector and set the > corresponding tvp5150 input src per default during registered() > - mc/of: factor out oftree connector allocation > - of: drop svideo dt port > - of: move svideo connector to port@0/endpoint@1 > - of: require at least 1-in and 1-out endpoint > --- > drivers/media/i2c/tvp5150.c | 379 +--- > 1 file changed, 350 insertions(+), 29 deletions(-) > > diff --git a/drivers/media/i2c/tvp5150.c b/d
[PATCH v7 05/13] media: tvp5150: add input source selection of_graph support
This patch adds the of_graph support to describe the tvp input connections. Physical the TVP5150 has three ports: AIP1A, AIP1B and YOUT. As result of discussion [1],[2] the device-tree maps these ports 1:1. Look at the Documentation for more information. Since the TVP5150 is a converter/bridge the device-tree must contain at least 1-input and 1-output port. The mc-connectors and mc-links are only created if the device-tree contains the corresponding connector nodes. If more than one connector is available the media_entity_operations.link_setup() callback ensures that only one connector is active. [1] https://www.spinics.net/lists/linux-media/msg138545.html [2] https://www.spinics.net/lists/linux-media/msg138546.html Signed-off-by: Marco Felsch --- Changelog: [1] https://patchwork.kernel.org/cover/10794703/ [2] https://patchwork.kernel.org/cover/10786553/ v7: - don't init enum tvp5150_pads with TVP5150_COMPOSITE0 functionality - break some 80 character limitation to improve readability - fix comment style -> always start with capital letters and end with dot - fix some minor style issues - fix tvp5150_registered error handling - simplify tvp5150_mc_init - make connectors static -> since now only three connectors are possible (as described in DT) -> drop tvp5150.endpoints storage -> squash tvp5150_parse_dt and tvp5150_add_of_connectors - improve tvp5150_parse_dt: -> make parsing stricter and fix not detected missconfigured dt-data -> svideo must be connected now to port@[0,1]/endpoint@1 v6: - fix misspelled comments - use 'unsigned int' where it's possible - cleanup ifdef part-2: * tvp5150_mc_init, tvp5150_add_of_connectors: add surrounding CONFIG_MEDIA_CONTROLLER if def and stubs to improve quality - tvp5150_mc_init: uniform interface, use 'struct tvp5150' since all internal function do this. - tvp5150_add_of_connectors: call within probe() to make it cleaner - tvp5150_parse_dt: move local loop vars within the loop. v5: - Fixing build deps: - tvp5150_mc_init: fix CONFIG_MEDIA_CONTROLLER deps - struct tvp5150: drop CONFIG_MEDIA_CONTROLLER conditional property includes. This leads into to complex deps for futher development. - tvp5150_dt_cleanup: enable function only if CONFIG_OF is enabled - tvp5150_parse_dt: enable function only if CONFIG_OF is enabled - tvp5150_probe: call tvp5150_dt_cleanup only if CONFIG_OF is enabled - Simplify link_setup routine: - use generic connector parsing since both series [1,2] are squashed into one - struct tvp5150: drop pads_state and modify_second_link property due to link_setup() rework. - tvp5150_link_setup: add more comments - tvp5150_link_setup: simply the link setup routine a lot. Edit the 2nd link directly within the driver instead of a recursive media-framework call (__media_entity_setup_link). This improves the readability and shrinks the driver code. - tvp5150_link_setup: disable all active links in case user switches connectors without disable it first. - tvp5150_registered: simplify default link enable path due to link_setup() rework. - General cleanups - tvp5150_parse_dt: drop unecessary test - tvp5150_parse_dt: add err message due to misconfiguration - tvp5150_parse_dt: make use of V4L2_MBUS_UNKNOWN definition - s/dev_dbg/dev_dbg_lvl v4: - rebase on top of media_tree/master, fix merge conflict due to commit 60359a28d592 ("media: v4l: fwnode: Initialise the V4L2 fwnode endpoints to zero") v3: - probe(): s/err/err_free_v4l2_ctrls - drop MC dependency for tvp5150_pads v2: - adapt commit message - unify ifdef switches - rename tvp5150_valid_input -> tvp5150_of_valid_input, to be more precise - mc: use 2-input and 1-output pad - mc: link svideo connector to both input pads - mc: enable/disable svideo links in one go - mc: change link_setup() behaviour, switch the input src don't require a explicite disable before. - mc: rename 'local' media_pad param to tvp5150_pad to avoid confusion - mc: enable link to the first available connector and set the corresponding tvp5150 input src per default during registered() - mc/of: factor out oftree connector allocation - of: drop svideo dt port - of: move svideo connector to port@0/endpoint@1 - of: require at least 1-in and 1-out endpoint --- drivers/media/i2c/tvp5150.c | 379 +--- 1 file changed, 350 insertions(+), 29 deletions(-) diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c index 11a5fd7e2f58..dfbf5bbc307c 100644 --- a/drivers/media/i2c/tvp5150.c +++ b/drivers/media/i2c/tvp5150.c @@ -32,11 +32,12 @@ #define TVP5150_FIELD V4L2_FIELD_ALTERNATE #define TVP5150_COLORSPACE V4L2_COLORSPACE_SMPTE170M +#define TVP5150_MAX_CONNECTORS 3 /* Check dt-bindings for more informations. */ + MODULE_DESCRIPTION("Texas Instruments TVP5150A/TVP5150AM1/TVP5151 video decoder driver"); MODULE_AUTHOR("Mauro Carvalho Chehab"); MODULE_LICENSE("GPL v2"); - static