Re: [RFC v4 1/4] Broadcom Bluetooth UART Device Tree bindings

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



On Tue, Jun 2, 2015 at 7:20 AM, Ilya Faenson <ifaenson@xxxxxxxxxxxx> wrote:
> Hi Rob,
>
> Appreciate your insights.
>
> -----Original Message-----
> From: Rob Herring [mailto:robherring2@xxxxxxxxx]
> Sent: Monday, June 01, 2015 8:19 PM
> To: Ilya Faenson
> Cc: Marcel Holtmann; linux-bluetooth@xxxxxxxxxxxxxxx; devicetree-spec@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [RFC v4 1/4] Broadcom Bluetooth UART Device Tree bindings
>
> On Fri, May 22, 2015 at 3:10 PM, Ilya Faenson <ifaenson@xxxxxxxxxxxx> wrote:
>> Device Tree bindings to configure the Broadcom Bluetooth UART device.
>>
>> Signed-off-by: Ilya Faenson <ifaenson@xxxxxxxxxxxx>
>> ---
>>  .../devicetree/bindings/net/bluetooth/btbcm.txt    | 82 ++++++++++++++++++++++
>>  1 file changed, 82 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>> new file mode 100644
>> index 0000000..968421b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
>> @@ -0,0 +1,82 @@
>> +btbcm
>> +------
>> +
>> +Required properties:
>> +
>> +       - compatible : must be "brcm,brcm-bt-uart".
>> +       - tty : tty device connected to this Bluetooth device.
>
> Neil Brown has been working on generic bindings for tty/UART slaves
> which should be used here.
>
> IF: I certainly like what is being developed for the UART child devices in the DeviceTree. It will finally catch up with what's been available in the ACPI for years. However, to the best of my understanding, that effort is currently not in the bluetooth-next git repo. As Marcel told me earlier this year, "if it is not in the bluetooth-next, it does not exist". I'm afraid I can't use it therefore. Also, we need to ship this driver sooner rather than later. The integration with the UART slave devices sounds like a worthwhile future enhancement to me.

If you merged this and then used the UART slave devices later, then
you have to support both bindings as the bindings are an ABI. We have
enough cases of that because we did not have the foresight to avoid
it. In this case we do. You have not given me any reason why other
than timing and that's not a valid reason.

>
>> +
>> +Optional properties:
>> +
>> +  - bt-host-wake-gpios : bt-host-wake input GPIO to be used as an interrupt.
>> +
>> +  - bt-wake-gpios : bt-wake output GPIO to be used to suspend / resume device.
>> +
>> +  - bt-reg-on-gpios : reg-on output GPIO to be used to power device on/off.
>> +
>> +  - baud-rate-before-config-download : initial Bluetooth device baud rate.
>> +    Default: 3000000.
>
> What's the baudrate after download? Why not just use standard
> "baud-rate" if you only specify one rate?
>
> IF: You're right: this doc used to list two rates. :-) To be compatible with what Intel is doing in the same area, I will call it "oper-speed" if you don't mind.

So you need 2 values or not?

>> +
>> +  - manual-fc : flow control UART in suspend / resume scenarios.
>> +    Default: 0.
>> +
>> +  - configure-sleep : configure suspend / resume flag.
>> +    Default: false.
>> +
>> +  - configure-audio : configure platform PCM SCO flag.
>> +    Default: false.
>> +
>> +  - PCMClockMode : PCM clock mode. 0-slave, 1-master.
>> +    Default: 0.
>> +
>> +  - PCMFillMethod : PCM fill method. 0 to 3.
>> +    Default: 2.
>> +
>> +  - PCMFillNum : PCM number of fill bits. 0 to 3.
>> +    Default: 0.
>> +
>> +  - PCMFillValue : PCM fill value. 0 to 7.
>> +    Default: 3.
>> +
>> +  - PCMInCallBitclock : PCM interface rate. 0-128Kbps, 1-256Kbps, 2-512Kbps,
>> +    3-1024Kbps, 4-2048Kbps.
>> +    Default: 0.
>> +
>> +  - PCMLSBFirst : PCM LSB first. 0 or 1.
>> +    Default: 0.
>> +
>> +  - PCMRightJustify : PCM Justify. 0-left, 1-right.
>> +    Default: 0.
>> +
>> +  - PCMRouting : PCM routing. 0-PCM, 1-SCO over HCI.
>> +    Default: 0.
>> +
>> +  - PCMShortFrameSync : PCM sync. 0-short, 1-long.
>> +    Default: 0.
>> +
>> +  - PCMSyncMode : PCM sync mode. 0-slave, 1-master.
>
> Please no CamelCase. These need better descriptions.
>
> IF: I am certainly aware that the CamelCase is generally not used on Linux. The Broadcom however have a spec describing what needs to be done to integrate these kinds of chips on hardware platforms (for Windows). Anybody who wishes to use SCO PCM audio will need to read that spec. These parameter names are from that spec. A lot of additional technical info Is there, along with various diagrams. It can't be all placed in this help file. Of course, I can change these parameter names but it would make things harder for those wishing to use the hardware as they would not match the spec. Could you make an exception?

I think humans are capable to translate. Besides, I don't really care
so much what is in Broadcom's spec (besides the fact I probably don't
have access). What I care about is what does the PCM configuration
look like for the next BT uart chip with a DT binding that comes
along. I don't want to see $vendor specific fields just dumped into DT
as is with no regard to whether things should be common. Some of this
is pretty Broadcom specific, but much of it is standard PCM/I2S audio
configuration.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree-spec" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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

  Powered by Linux