Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 7 May 2020 19:17:38 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Swapnil Kashinath Jakhade <sjakhade@cadence.com>
Cc:     Yuti Suresh Amonkar <yamonkar@cadence.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kishon@ti.com" <kishon@ti.com>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "jsarha@ti.com" <jsarha@ti.com>,
        "tomi.valkeinen@ti.com" <tomi.valkeinen@ti.com>,
        "praneeth@ti.com" <praneeth@ti.com>,
        Milind Parab <mparab@cadence.com>
Subject: Re: [PATCH v1 2/2] phy: phy-cadence-torrent: Use PHY kernel APIs to
 set PHY attributes
Message-ID: <20200507171738.atzyfpueo6bjbwpb@gilmour.lan>
References: <1588057804-29161-1-git-send-email-yamonkar@cadence.com>
 <1588057804-29161-3-git-send-email-yamonkar@cadence.com>
 <20200429122750.hup7vbmz3xnpfwa5@gilmour.lan>
 <BY5PR07MB69827A1E2136455BFDD6090BC5AA0@BY5PR07MB6982.namprd07.prod.outlook.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="fnwfzinzf67jig65"
Content-Disposition: inline
In-Reply-To: <BY5PR07MB69827A1E2136455BFDD6090BC5AA0@BY5PR07MB6982.namprd07.prod.outlook.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--fnwfzinzf67jig65
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi!

On Thu, Apr 30, 2020 at 02:06:20PM +0000, Swapnil Kashinath Jakhade wrote:
> Thank you so much for reviewing the patch. Please see inline reply below.
>=20
> > -----Original Message-----
> > From: Maxime Ripard <maxime@cerno.tech>
> > Sent: Wednesday, April 29, 2020 5:58 PM
> > To: Yuti Suresh Amonkar <yamonkar@cadence.com>
> > Cc: linux-kernel@vger.kernel.org; kishon@ti.com; mark.rutland@arm.com;
> > jsarha@ti.com; tomi.valkeinen@ti.com; praneeth@ti.com; Milind Parab
> > <mparab@cadence.com>; Swapnil Kashinath Jakhade
> > <sjakhade@cadence.com>
> > Subject: Re: [PATCH v1 2/2] phy: phy-cadence-torrent: Use PHY kernel AP=
Is to
> > set PHY attributes
> >=20
> > EXTERNAL MAIL
> >=20
> >=20
> > Hi,
> >=20
> > On Tue, Apr 28, 2020 at 09:10:04AM +0200, Yuti Amonkar wrote:
> > > From: Swapnil Jakhade <sjakhade@cadence.com>
> > >
> > > Use generic PHY framework function phy_set_bus_width() to set number
> > > of lanes and function phy_set_max_link_rate() to set maximum link rate
> > > supported by PHY.
> > >
> > > Signed-off-by: Swapnil Jakhade <sjakhade@cadence.com>
> >=20
> > This should have your SoB.
> >=20
> > > ---
> > >  drivers/phy/cadence/phy-cadence-torrent.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/phy/cadence/phy-cadence-torrent.c
> > b/drivers/phy/cadence/phy-cadence-torrent.c
> > > index 7116127358ee..b914e5ddf93c 100644
> > > --- a/drivers/phy/cadence/phy-cadence-torrent.c
> > > +++ b/drivers/phy/cadence/phy-cadence-torrent.c
> > > @@ -1852,6 +1852,9 @@ static int cdns_torrent_phy_probe(struct
> > platform_device *pdev)
> > >  				 cdns_phy->phys[node].num_lanes,
> > >  				 cdns_phy->max_bit_rate / 1000,
> > >  				 cdns_phy->max_bit_rate % 1000);
> > > +
> > > +			phy_set_bus_width(gphy, cdns_phy-
> > >phys[node].num_lanes);
> > > +			phy_set_max_link_rate(gphy, cdns_phy-
> > >max_bit_rate);
> >=20
> > I think what Kishon meant in his previous mail is that it's not really =
clear (to
> > me at least) how that function would be useful.
> >=20
> > In this particular case, what would the consumer make of that informati=
on?
> > Does
> > the phy needs to be reconfigured based on the max rate being changed?
> >=20
> > Some phy_configure_opts structures also have a somewhat similar field t=
hat
> > can
> > be negociated between the provider and the consumer using phy_validate,
> > wouldn't
> > that be redundant?
> >=20
> > Most of that discussion can only happen when you've provided a use-case
> > for that
> > series, so a consumer using it would help greatly there.
>=20
> Actually, for this particular case, consumer driver will be the Cadence M=
HDP
> bridge driver for DisplayPort which is also under review process for
> upstreaming [1]. So this DRM bridge driver will make use of the PHY APIs
> phy_get_bus_width() and phy_get_max_link_rate() during execution of probe
> function to get the number of lanes and maximum link rate supported by Ca=
dence
> Torrent PHY. This information is required to set the host capabilities in=
 the
> DRM bridge driver, based on which initial values for DisplayPort link tra=
ining
> will be determined.
>
> The changes in this PHY patch series are based on suggestions in the revi=
ew
> comments in [1] which asks to use kernel PHY APIs to read these properties
> instead of directly accessing PHY device node. The complete driver and ac=
tual
> use of these APIs can be found in [2]. This is how we are planning to use
> these APIs.

I haven't really looked into the displayport spec, but I'd assume that ther=
e's a
lot more parameters that would need to be negociated between the phy and th=
e DP
block? If so, then it would make more sense to follow the path we did for
MIPI-DSI where the parameters can be negociated through the phy_configure /
phy_validate interface.

Maxime

--fnwfzinzf67jig65
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCXrRCsgAKCRDj7w1vZxhR
xcxNAQCJzdDTI7XfDxwOQTyRA+A0KjWJ540bnKWNjxgcXBGkLAEA81jx2j6ogfZR
BBWqGaYotEewPh3kQu0Uxc5JUJ/6hws=
=HjJD
-----END PGP SIGNATURE-----

--fnwfzinzf67jig65--