On Tue 27 Mar 08:56 PDT 2018, Thierry Escande wrote: > Hi Bjorn, > > On 27/03/2018 00:51, Bjorn Andersson wrote: > > On Tue 20 Mar 23:58 HKT 2018, Marcel Holtmann wrote: > > > > Signed-off-by: Thierry Escande <thierry.escande@xxxxxxxxxx> > > [..] > > > > + - clocks: clock phandle for SUSCLK_32KHZ > > > > > > if I compare this with broadcom-bluetooth.txt or ti-bluetooth.txt then > > > besides compatible, everything else is optional. The > > > nokia-bluetooth.txt has everything required, but that is also a really > > > specific platform. > > > > > > Can we be less restrictive for a QCA general purpose chip? > > > > > > > The way we deal with this in other bindings is that we tie such > > requirements to the compatible; i.e. we would say that qcom,qca6174-bt > > requires a clock and we would have something like qcom,qca-bt that makes > > it optional. > > > > The beauty of this is that the driver will tell you if you forgot to > > specify the clock when it actually is required, which saves you > > considerable amount of debugging time. > > > > > > NB. The way the bcm driver handles this is insufficient, as it treats > > any error from clk_get as "there's no clock specified". The driver > > should accept a clock not being specified, but should fail properly when > > a clock is specified but can't be acquired (e.g. due to clk_get() > > returning EPROBE_DEFER). > > > > > > + > > > > +Example: > > > > + > > > > +serial@7570000 { > > > > + pinctrl-names = "default", "sleep"; > > > > + pinctrl-0 = <&blsp1_uart1_default>; > > > > + pinctrl-1 = <&blsp1_uart1_sleep>; > > > > + > > > > + bluetooth { > > > > + compatible = "qcom,qca6174-bt"; > > > > + > > > > + enable-gpios = <&pm8994_gpios 19 GPIO_ACTIVE_HIGH>; > > > > + > > > > + pinctrl-names = "default"; > > > > + pinctrl-0 = <&bt_en_pin_a>; > > > > > > This one I do not understand and you might want to shed some light > > > into why this is done that way. > > > > > > > This is completely generic and only relates to getting the electrical > > properties of the gpio pin set up correctly. So I would recommend that > > we omit this from the binding and example (including the pinctrl > > properties for the serial above). > > If I remove the pinctrl entry in the bluetooth node, the gpio19 is then > marked as unclaimed. The drive strength also defaults to low but that > doesn't seem to be an issue and the the chip can still be enabled through > gpio19. Is it ok to have it unclaimed? If so I can remove it from the > binding and the doc then. > > Regarding the blsp1_uart1_default of the serial node, I can still enable the > chip if I remove it but the hci commands all end in timeout. It seems that > the function for these pins has to be explicitly set to blsp_uart2. So at > least, the default pinctrl seems mandatory. > Our board needs these properties to get the uart and gpio in the right state, but this is unrelated to BT - that's why I suggested that you omit these properties from the BT binding. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html