Messages in this thread | | | From | Swapnil Kashinath Jakhade <> | Subject | RE: [PATCH v1 2/2] phy: phy-cadence-torrent: Use PHY kernel APIs to set PHY attributes | Date | Mon, 18 May 2020 06:54:36 +0000 |
| |
Hi Kishon,
> -----Original Message----- > From: Kishon Vijay Abraham I <kishon@ti.com> > Sent: Wednesday, May 13, 2020 8:08 AM > To: Tomi Valkeinen <tomi.valkeinen@ti.com>; Maxime Ripard > <maxime@cerno.tech>; Swapnil Kashinath Jakhade > <sjakhade@cadence.com> > Cc: Yuti Suresh Amonkar <yamonkar@cadence.com>; linux- > kernel@vger.kernel.org; mark.rutland@arm.com; jsarha@ti.com; > praneeth@ti.com; Milind Parab <mparab@cadence.com>; Vinod Koul > <vkoul@kernel.org> > Subject: Re: [PATCH v1 2/2] phy: phy-cadence-torrent: Use PHY kernel APIs to > set PHY attributes > > EXTERNAL MAIL > > > Hi, > > On 5/8/2020 1:20 PM, Tomi Valkeinen wrote: > > On 07/05/2020 20:17, Maxime Ripard wrote: > > > >>> Actually, for this particular case, consumer driver will be the > >>> Cadence MHDP 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 Cadence Torrent PHY. This information is required to > >>> set the host capabilities in the DRM bridge driver, based on which > >>> initial values for DisplayPort link training will be determined. > >>> > >>> The changes in this PHY patch series are based on suggestions in the > >>> review 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 actual 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 there's a lot more parameters that would need to be negociated > >> between the phy and the 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. > > > > I don't think this is negotiation, but just exposing the (max) > > capabilities of PHY, inside which the configure can work. Maybe all > > the capabilities could handled with a struct (struct phy_attrs), > > instead of adding separate functions for each, though. > > yeah, that makes sense. Just that users should take care not to over-write all > the phy attributes with partial information.
It would be really helpful if you could clarify a bit regarding how to handle this exactly. What I could understand from Tomi' suggestion is that all PHY attributes in struct phy_attrs should have single pair of functions to get and set all the PHY attributes (e.g. phy_get_attrs / phy_set_attrs), instead of separate get/set pair of functions for individual attribute (bus_width, mode, max_link_rate etc). Is this understanding correct? If so, how should the existing functions for bus_width and mode be used?
Thanks & regards, Swapnil
> > Thanks > Kishon
| |