Re: [REVIEW PATCH v3 1/2] media: Change media device link_notify behaviour

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

On 06/10/2013 12:49 PM, Laurent Pinchart wrote:
>>>> -static int isp_pipeline_link_notify(struct media_pad *source,
>>>> > >> -				    struct media_pad *sink, u32 flags)
>>>> > >> +static int isp_pipeline_link_notify(struct media_link *link, u32 flags,
>>>> > >> +				    unsigned int notification)
>>>> > >> 
>>>> > >>   {
>>>> > >> 
>>>> > >> -	int source_use = isp_pipeline_pm_use_count(source->entity);
>>>> > >> -	int sink_use = isp_pipeline_pm_use_count(sink->entity);
>>>> > >> +	struct media_entity *source = link->source->entity;
>>>> > >> +	struct media_entity *sink = link->sink->entity;
>>>> > >> +	int source_use = isp_pipeline_pm_use_count(source);
>>>> > >> +	int sink_use = isp_pipeline_pm_use_count(sink);
>>>> > >> 
>>>> > >>   	int ret;
>>>> > >> 
>>>> > >> -	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
>>>> > >> +	if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
>>>> > >> +	    !(link->flags & MEDIA_LNK_FL_ENABLED)) {
>>>> > >> 
>>>> > >>   		/* Powering off entities is assumed to never fail. */
>>>> > >> 
>>>> > >> -		isp_pipeline_pm_power(source->entity, -sink_use);
>>>> > >> -		isp_pipeline_pm_power(sink->entity, -source_use);
>>>> > >> +		isp_pipeline_pm_power(source, -sink_use);
>>>> > >> +		isp_pipeline_pm_power(sink, -source_use);
>>>> > >> 
>>>> > >>   		return 0;
>>>> > >>   	
>>>> > >>   	}
>>>> > >> 
>>>> > >> -	ret = isp_pipeline_pm_power(source->entity, sink_use);
>>>> > >> -	if (ret < 0)
>>>> > >> -		return ret;
>>>> > >> +	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
>>>> > >> +		(flags & MEDIA_LNK_FL_ENABLED)) {
>>> > > 
>>> > > You could return zero here if the opposite was true, and unindent the
>>> > > rest. Up to you --- the patch is fine.
>
> I would personally keep the code as-is, to keep the symmetry, but I'm fine 
> with both :-)

I had also an impression that it looks more symmetric as-is. I would leave it
unchanged then. ;)

Regards,
Sylwester