Re: [PATCH 3/3] ARM: dts: rk3288 Tinker Board (S) add bluetooth

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi David,

> David Summers <beagleboard@xxxxxxxxxxxxxxxxxxx> hat am 18. Februar 2019 um 21:47 geschrieben:
> 
> 
> On 17/02/2019 21:05, Stefan Wahren wrote:
> > Hi David,
> >
> >> David Summers <beagleboard@xxxxxxxxxxxxxxxxxxx> hat am 17. Februar 2019 um 13:15 geschrieben:
> >>
> >> +++ b/arch/arm/boot/dts/rk3288-tinker.dtsi
> >> @@ -481,6 +481,17 @@
> >>   
> >>   &uart0 {
> >>   	status = "okay";
> >> +	pinctrl-names = "default";
> >> +	pinctrl-0 = <&uart0_xfer>, <&uart0_cts>, <&uart0_rts>;
> >> +
> >> +	bluetooth {
> >> +		clocks = <&rk808 RK808_CLKOUT1>;
> > As we already linked the RK808 clock in the sdio pwrseq, we shouldn't do this here again.
> >
> >> +		reset-gpios = <&gpio4 RK_PD5 GPIO_ACTIVE_LOW>;
> > reset-gpios isn't compatible to hci_h5, please use my preliminary dt-binding
> >
> >> +		device-wake-gpios = <&gpio4 RK_PD2 GPIO_ACTIVE_HIGH>;
> >> +		host-wake-gpios = <&gpio4 RK_PD7 GPIO_ACTIVE_HIGH>;
> > AFAIK hci_h5 doesn't support host-wake yet. So if you want to keep for the future we should make a comment.
> >
> >> +		vcc-18-supply = <&vcc_18>;
> >> +		vcc-io-supply = <&vcc_io>;
> > Please drop these as the driver doesn't use it and it's already linked in the sdio pwrseq.
> >
> > Thanks Stefan
> >
> >> +	};
> > [1] - https://gist.github.com/lategoodbye/79bac99d4f1158a719a48ea3c45eb7f1#file-0002-dt-bindings-add-support-for-realtek-bluetooth-serial-patch
> 
> Sorry I disagree here.
> 
> Last time I posted an example patch like this robh said clearly that 
> these should give clocks, power, etc:
> 
> https://www.spinics.net/lists/linux-bluetooth/msg78545.html
> https://www.spinics.net/lists/linux-bluetooth/msg78585.html 
> <https://www.spinics.net/lists/linux-bluetooth/msg78585.html>
> 
> and as he is maintainer he wins.
> 
> I can understand Robs position, device tree are for describing hardware, 
> they aren't primarily for helping code to load. Yes that is a second 
> effect, its so it knows what is in  the hardware. In this case what if 
> someone wanted Bluetooth, but not WiFi - then the Bluetooth should know 
> what power and clocks are needed. Now that the driver doesn't understand 
> these things is secondary. Maybe that will be cured in future?
>

you need to consider to context of the discussion. The link pointed to a discussion about the dt-binding. A binding should describe all parts (even optional) of the hardware and that is the reason why i explained in Arch board that we need 3 parts for bluetooth (dt-binding, driver change and finally the dts change). Since you avoid that and send only the last part without compatibles causes confusion.

We usually don't change DTS properties because it's considered as an ABI, which means we should do it properly in the first run. So lets make the step one (test bluetooth) and not step four ...

Regards
Stefan



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux