linux-kernel - Re: [PATCH v2 04/19] media: camss: Add CSIPHY files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170630080536.7doop7wxn77j6ju5@valkosipuli.retiisi.org.uk>
Date:   Fri, 30 Jun 2017 11:05:37 +0300
From:   Sakari Ailus <sakari.ailus@....fi>
To:     Todor Tomov <todor.tomov@...aro.org>
Cc:     mchehab@...nel.org, hans.verkuil@...co.com, javier@....samsung.com,
        s.nawrocki@...sung.com, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v2 04/19] media: camss: Add CSIPHY files

Hi Todor,

On Fri, Jun 30, 2017 at 10:00:25AM +0300, Todor Tomov wrote:
> Hi Sakari,
> 
> On 06/30/2017 02:53 AM, Sakari Ailus wrote:
> > Hi Todor,
> > 
> > On Thu, Jun 29, 2017 at 07:36:47PM +0300, Todor Tomov wrote:
> >>>> +/*
> >>>> + * csiphy_link_setup - Setup CSIPHY connections
> >>>> + * @entity: Pointer to media entity structure
> >>>> + * @local: Pointer to local pad
> >>>> + * @remote: Pointer to remote pad
> >>>> + * @flags: Link flags
> >>>> + *
> >>>> + * Rreturn 0 on success
> >>>> + */
> >>>> +static int csiphy_link_setup(struct media_entity *entity,
> >>>> +			     const struct media_pad *local,
> >>>> +			     const struct media_pad *remote, u32 flags)
> >>>> +{
> >>>> +	if ((local->flags & MEDIA_PAD_FL_SOURCE) &&
> >>>> +	    (flags & MEDIA_LNK_FL_ENABLED)) {
> >>>> +		struct v4l2_subdev *sd;
> >>>> +		struct csiphy_device *csiphy;
> >>>> +		struct csid_device *csid;
> >>>> +
> >>>> +		if (media_entity_remote_pad((struct media_pad *)local))
> >>>
> >>> This is ugly.
> >>>
> >>> What do you intend to find with media_entity_remote_pad()? The pad flags
> >>> haven't been assigned to the pad yet, so media_entity_remote_pad() could
> >>> give you something else than remote.
> >>
> >> This is an attempt to check whether the pad is already linked - to refuse
> >> a second active connection from the same src pad. As far as I can say, it
> >> was a successful attempt. Do you see any problem with it?
> > 
> > Ah. So you have multiple links here only one of which may be active?
> 
> Exactly. Below I'm adding the output of media-ctl --print-dot as you have
> requested. I can add it in the driver document as well.

Hmm. I think it could be useful there as an example. I wonder what others
think.

> 
> > 
> > I guess you can well use media_entity_remote_pad(), but then
> > media_entity_remote_pad() argument needs to be made const. Feel free to
> > spin a patch. I don't think it'd have further implications elsewhere.
> > 
> 
> Well media_entity_remote_pad() accepts struct media_pad *pad, not a
> const and trying to pass a const triggers a warning. This is why I had
> to cast. Or did I misunderstand you?

No, you don't cast to non-const. Instead, you change the function to accept
a const argument.

-- 
Sakari Ailus
e-mail: sakari.ailus@....fi	XMPP: sailus@...iisi.org.uk