[PATCH 1/2] phy-rockchip-pcie: add set_mode callback

Brian Norris briannorris at chromium.org
Mon Jul 10 22:20:32 PDT 2017


Hi Kishon,

On Mon, Jul 10, 2017 at 9:45 PM, Kishon Vijay Abraham I <kishon at ti.com> wrote:
> On Friday 16 June 2017 01:47 PM, Shawn Lin wrote:
> > phy_mode was added for switching USB mode purposely as
> > well as phy_set_mode API. However other types of PHY could
> > also have some miscellaneous setting/modes need to be
> > handled. This patch is gonna support this callback for
> > phy-rockchip-pcie and do some power-saving work there.
> > Note that we just stuff in some other values other that the
> > existing phy_mode and convert it in the coressponding driver
> > instead, otherwise we should extend the phy_mode again which
> > it doesn't make sense to add in new driver's specificed value.
> > Overall it looks fine to me as the controller's driver and the
> > phy's driver are paired so that the caller and the consumer should
> > be able to keep the value(mode) in consistent.
>
> I really don't prefer using an API other that what it is intended for. We have
> to come up with a better way for handling this.
>
> IIUC this patch sets unused lanes in idle mode? If yes, then can't we model
> each lane as a separate phy, so that we can power-on/power-off each lane
> independently.

I suggested the same to Shawn previously. I'm interested in seeing his response.

But personally I'm still not sure if that's the most accurate way to
describe this PHY, except for the fact that inevitably, most device
tree bindings get molded to match the related kernel APIs, rather than
the other way around. What if instead, we added an enum mode entry
that represents "finished link training", at which point the PHY would
then know it can power down the unused lanes (that's assuming the PHY
can figure that part out)?

Or if we did re-model this PHY, how are we supposed to handle backward
compatibility? Can the same PHY driver support 2 different of_xlate
methods? I guess we'd have to provide an implementation that sets the
driver up to behave completely differently based on the number of
#phy-cells?

Brian



More information about the Linux-rockchip mailing list