linux-kernel - Re: [PATCH v2 8/9] phy: Add Cadence D-PHY support

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: <20181116091724.4wwdrgn5hfcdao6b@flea>
Date:   Fri, 16 Nov 2018 10:17:24 +0100
From:   Maxime Ripard <maxime.ripard@...tlin.com>
To:     Kishon Vijay Abraham I <kishon@...com>
Cc:     Boris Brezillon <boris.brezillon@...tlin.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        linux-media@...r.kernel.org,
        Archit Taneja <architt@...eaurora.org>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Chen-Yu Tsai <wens@...e.org>, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org,
        Krzysztof Witos <kwitos@...ence.com>,
        Rafal Ciepiela <rafalc@...ence.com>
Subject: Re: [PATCH v2 8/9] phy: Add Cadence D-PHY support

Hi!

On Mon, Nov 12, 2018 at 03:21:56PM +0530, Kishon Vijay Abraham I wrote:
> > +static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode,
> > +			      union phy_configure_opts *opts)
> > +{
> > +	struct cdns_dphy_cfg cfg = { 0 };
> > +
> > +	if (mode != PHY_MODE_MIPI_DPHY)
> > +		return -EINVAL;
> > +
> > +	return cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
> > +}
> > +
> > +static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> > +{
> > +	struct cdns_dphy *dphy = phy_get_drvdata(phy);
> > +	struct cdns_dphy_cfg cfg = { 0 };
> > +	int ret;
> > +
> > +	ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
> > +	if (ret)
> > +		return ret;
> 
> Can you explain why you need the same function to be invoked from both validate
> and configure callback? I see this to be redundant.

Sure.

Validate and configure serve two rather different purposes. validate
is here to make sure that a configuration can work with the PHY, and
to let the phy adjust the configuration to find a more optimal one.

configure, on the other hand, apply a configuration. We still have to
make sure that the configuration can work, since:
  - We might have called validate any number of times, with any number
    of configurations before calling configure, so we don't know which
    configuration we validate is actually going to be applied later on
    (if it's even applied)
  - If we don't care about the validation at all, we might just call
    configure directly

Does that make sense?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)