Re: [PATCH v2 7/8] ARM: dts: rockchip: assign usbphy480m_src to the new usbphy pll on veyron

[Heiko Stuebner]

Am Montag, 9. November 2015, 13:08:43 schrieb Doug Anderson:
> Heiko,
> 
> On Sun, Nov 8, 2015 at 8:04 AM, Heiko Stuebner <heiko@xxxxxxxxx> wrote:
> > Veyron devices try to always set the source for usbphy480m to the usbphy0
> > that is the phy connected to the otg controller, because the firmware-
> > default is usbphy1, the ehci-controller connected to the internal camera
> > that might get turned off way easier to save power.
> >
> > In the mainline kernel we currently don't use the usbphy480m_src at all,
> > as it mainly powers the uart0 source that is connected to the bluetooth
> > component of the wifi/bt combo.
> >
> > So move that assignment over to the new real pll clock inside the usbphy.
> >
> > Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx>
> > ---
> >  arch/arm/boot/dts/rk3288-veyron.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi 
b/arch/arm/boot/dts/rk3288-veyron.dtsi
> > index d4263ed..c8329b5 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron.dtsi
> > +++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
> > @@ -410,7 +410,7 @@
> >         status = "okay";
> >
> >         assigned-clocks = <&cru SCLK_USBPHY480M_SRC>;
> > -       assigned-clock-parents = <&cru SCLK_OTGPHY0>;
> > +       assigned-clock-parents = <&usbphy0>;
> >         dr_mode = "host";
> >  };
> 
> This is right, hence:
> 
> Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> 
> ...you will slightly break bisectability with this series though,
> right?  In the previous patch in this series you changed the clocks in
> the mux away from the fake ones to be the real ones.  If you have
> either that patch without this one or this patch without that one then
> clock parents won't get assigned properly...
> 
> I seem to remember that one of the USB phy clocks was kinds jittery (I
> want to say it was the EHCI one) and that was causing problems talking
> to the BT UART at 3Mbps.  I kinda doubt we have anyone running BT on
> upstream over that UART (only used on veyron devices with Broadcom
> WiFi), so maybe this is OK.  ...but if you had any other ideas for how
> to avoid breaking bisect that would be nice.
> 
> If nothing else, if we're going to break bisect then the two commits
> should reference each other and say that you need both and that with
> only one you might see a different clock selected...

hmm, I wouldn't say that it breaks bisectability, because that would require 
that some working feature does break between those two changes in the first 
place. On Chromebooks we have either mwifiex, that doesn't even do wifi 
sucessfully right now and the (from what I've read) not well supported bcm, of 
which I don't know if it supports the veyron-variant's bluetooth yet.

The ehci-phy getting turned off was the more pressing reason from what I've 
read, and we're actually fixing that now :-) .

One possible way to still solve that would probably be moving the #clock-cells 
patch before the clock-tree change and merge this dts change into the clk one.
I'll take a look into that tomorrow.


Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/