Re: [RFC PATCH v2 1/4] drm: Introduce generic probe function for component based masters.

[Russell King - ARM Linux]

On Mon, Oct 19, 2015 at 01:21:50PM +0100, Liviu Dudau wrote:
> A lot of component based DRM drivers use a variant of the same code
> as the probe function. They bind the crtc ports in the first iteration
> and then scan through the child nodes and bind the encoders attached
> to the remote endpoints. Factor the common code into a separate
> function called drm_of_component_probe() in order to increase code
> reuse.
> 
> Cc: David Airlie <airlied@xxxxxxxx>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@xxxxxxx>
> ---
>  drivers/gpu/drm/drm_of.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_of.h     | 12 +++++++
>  2 files changed, 104 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index be38840..cf16240 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -1,3 +1,4 @@
> +#include <linux/component.h>
>  #include <linux/export.h>
>  #include <linux/list.h>
>  #include <linux/of_graph.h>
> @@ -61,3 +62,94 @@ uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
>  	return possible_crtcs;
>  }
>  EXPORT_SYMBOL(drm_of_find_possible_crtcs);
> +
> +/**
> + * drm_of_component_probe - Generic probe function for a component based master
> + * @dev: master device containing the OF node
> + * @compare_of: compare function used for matching components
> + * @master_ops: component master ops to be used
> + *
> + * Parse the platform device OF node and bind all the components associated
> + * with the master. Interface ports are added before the encoders in order to
> + * satisfy their .bind requirements
> + * See Documentation/devicetree/bindings/graph.txt for the bindings.
> + *
> + * Returns zero if successful, or one of the standard error codes if it fails.
> + */
> +int drm_of_component_probe(struct device *dev,
> +			   int (*compare_of)(struct device *, void *),
> +			   const struct component_master_ops *master_ops)
> +{
...
> +	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> +	if (ret)
> +		return ret;

Please don't move this into here, it's completely inappropriate.  Just
because something makes use of this does not mean they only support
32-bit DMA.  Besides, this has nothing to do with whether or not it's
OF-based or not.
> +
> +	return component_master_add_with_match(dev, master_ops, match);
> +}
> +EXPORT_SYMBOL(drm_of_component_probe);
> diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
> index 2441f71..7c0c05b 100644
> --- a/include/drm/drm_of.h
> +++ b/include/drm/drm_of.h
> @@ -1,18 +1,30 @@
>  #ifndef __DRM_OF_H__
>  #define __DRM_OF_H__
>  
> +struct component_master_ops;
> +struct device;
>  struct drm_device;
>  struct device_node;
>  
>  #ifdef CONFIG_OF
>  extern uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
>  					   struct device_node *port);
> +extern int drm_of_component_probe(struct device *dev,
> +				  int (*compare_of)(struct device *, void *),
> +				  const struct component_master_ops *master_ops);
>  #else
>  static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
>  						  struct device_node *port)
>  {
>  	return 0;
>  }
> +
> +static inline int drm_of_component_probe(struct device *dev,
> +					int (*compare_of)(struct device *, void *),
> +					 const struct component_master_ops *master_ops)

Some of these lines will fail checkpatch.  It would be nice to avoid
lines longer than 80 columns.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/