| Message ID | E0D3336E15B58B4294723AC879BA5E942634F2@IRVEXCHMB15.corp.ad.broadcom.com |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: > + devicetree lists Please use get_maintainers.pl. > -----Original Message----- > From: Ilya Faenson [mailto:ifaenson@broadcom.com] > Sent: Wednesday, June 17, 2015 5:31 PM > To: Marcel Holtmann > Cc: linux-bluetooth@vger.kernel.org; Arend Van Spriel; Ilya Faenson > Subject: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings > > Device Tree bindings to configure the Broadcom Bluetooth UART device. > > Signed-off-by: Ilya Faenson <ifaenson@broadcom.com> > --- Change history from the last v4 version? > .../devicetree/bindings/net/bluetooth/btbcm.txt | 86 ++++++++++++++++++++++ > 1 file changed, 86 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..5dbcd57 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt > @@ -0,0 +1,86 @@ > +btbcm > +------ > + > +Required properties: > + > + - compatible : must be "brcm,brcm-bt-uart". You need to describe the chip, not the interface. > + - tty : tty device connected to this Bluetooth device. "tty" is a bit of a Linuxism and "ttyS0" certainly is. Further, there is no guarantee which uart is assigned ttyS0. This should be a phandle to the connected uart if not a sub node of the uart. This is complicated since these chips have multiple host connections. It needs to go under either uart or SDIO host and have a reference back to the one it is not under. Given the SDIO interface is discoverable (although sideband gpios and regulators are not), I would put this under the uart node as that is never discoverable. As I've mentioned previously, there's several cases of bindings for UART slave devices being posted. This all needs to be coordinated to use a common structure. > + > +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. > + > + - oper-speed : Bluetooth device operational baud rate. > + Default: 3000000. > + > + - manual-fc : flow control UART in suspend / resume scenarios. > + Default: 0. Can be boolean? I don't really follow the description. > + > + - configure-sleep : configure suspend / resume flag. > + Default: false. Please describe better what this does. Perhaps "idle-sleep-en" would be better. > + > + - idle-timeout : Number of seconds of inactivity before suspending. idle-timeout-sec Is this only applicable when configure-sleep is set? You mix the terms sleep and suspend a lot. Can you be more consistent. > + Default: 5. > + > + - configure-audio : configure platform PCM SCO flag. > + Default: false. So ignore the rest of the settings if not set? Perhaps combine with pcm-routing: <blank> - no audio audio-mode = "pcm"; audio-mode = "hci"; (or "sco-hci"?) > + > + - pcm-clockmode : PCM clock mode. 0-slave, 1-master. > + Default: 0. Can be boolean (property present or not): pcm-clock-mode-master > + > + - pcm-fillmethod : PCM fill method. 0 to 3. pcm-fill-method > + Default: 2. > + > + - pcm-fillnum : PCM number of fill bits. 0 to 3. > + Default: 0. pcm-fill-bits > + > + - pcm-fillvalue : PCM fill value. 0 to 7. > + Default: 3. pcm-fill-value > + > + - pcm-incallbitclock : PCM interface rate. 0-128Kbps, 1-256Kbps, 2-512Kbps, > + 3-1024Kbps, 4-2048Kbps. pcm-incall-bitclock-hz Use the actual rate rather than an enumeration. It is a simple div by 128k and log2 to convert in the driver. This makes the property more compatible with other chips. What does incall mean? What is the bit rate when not in a call? > + Default: 0. > + > + - pcm-lsbfirst : PCM LSB first. 0 or 1. > + Default: 0. Can be boolean > + > + - pcm-rightjustify : PCM Justify. 0-left, 1-right. > + Default: 0. Can be boolean > + - pcm-routing : PCM routing. 0-PCM, 1-SCO over HCI. > + Default: 0. Can be boolean: pcm-routing-hci > + > + - pcm-shortframesync : PCM sync. 0-short, 1-long. > + Default: 0. Can be boolean > + > + - pcm-syncmode : PCM sync mode. 0-slave, 1-master. > + Default: 0. Can be boolean: pcm-sync-mode-master Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: > Hi Rob, Your emails are base64 encoded. They should be plain text for the list. > -----Original Message----- > From: Rob Herring [mailto:robherring2@gmail.com] > Sent: Thursday, June 18, 2015 11:03 AM > To: Ilya Faenson > Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org > Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings > > On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: >> + devicetree lists [...] >> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >> new file mode 100644 >> index 0000000..5dbcd57 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >> @@ -0,0 +1,86 @@ >> +btbcm >> +------ >> + >> +Required properties: >> + >> + - compatible : must be "brcm,brcm-bt-uart". > > You need to describe the chip, not the interface. > > IF: This driver is for all Broadcom Bluetooth UART based chips. BT only chips? Most/many Broadcom chips are combo chips. I understand the driver for BT is *mostly* separate from other chip functions like WiFi, GPS and NFC, but the h/w is a single chip. I say most because likely there are some parts shared: a voltage rail, reset line, or power down line. I think some can do BT over the SDIO interface too, so the interface may be shared. The DT is a description of the h/w (i.e. what part # for a chip) and not a driver data structure. You need to describe the whole chip in the binding. It is a Linux problem if there needs to be multiple separate drivers. > >> + - tty : tty device connected to this Bluetooth device. > > "tty" is a bit of a Linuxism and "ttyS0" certainly is. Further, there > is no guarantee which uart is assigned ttyS0. > > This should be a phandle to the connected uart if not a sub node of > the uart. This is complicated since these chips have multiple host > connections. It needs to go under either uart or SDIO host and have a > reference back to the one it is not under. Given the SDIO interface is > discoverable (although sideband gpios and regulators are not), I would > put this under the uart node as that is never discoverable. > > As I've mentioned previously, there's several cases of bindings for > UART slave devices being posted. This all needs to be coordinated to > use a common structure. > > IF: This driver does not really access the UART. If just needs to have a string of some sort to map instances of the BlueZ protocol into platform devices to employ the right GPIOs and interrupts. I could use anything you recommend available through the tty_struct coming to the protocol from the BlueZ line discipline. Moreover, every end user platform I've ever dealt with in 16 years of working with Bluetooth had a single BT UART device. So these are really rare (typically test platforms) cases only. The mapping is not needed for most platforms at all. I suspect the right thing to do would be to make this parameter optional. The mapping would be done only if the parameter is present. I will use anything tty_struct derived you specify. Makes sense? You've missed my point. I'm not talking about connecting multiple devices to a UART at once. There are several instances of people trying to add UART connected devices into DT[1][2]. My point is these devices all need to have the DT binding done in a common way across different platforms. Otherwise, we can not have common code to parse the DT and find devices attached to a UART. > >> + >> +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. >> + >> + - oper-speed : Bluetooth device operational baud rate. >> + Default: 3000000. >> + >> + - manual-fc : flow control UART in suspend / resume scenarios. >> + Default: 0. > > Can be boolean? > > I don't really follow the description. > > IF: Okay, I will make it boolean. To clarify the description, it controls whether the BlueZ protocol needs to flow control the UART when the BT device is suspended and un-flow control it when the device is resumed. Okay. Discussion of BlueZ is not relevant to bindings. I would say something like "The hardware requires the host UART flow-control to be de-asserted during suspend." [...] >> + - configure-audio : configure platform PCM SCO flag. >> + Default: false. > > So ignore the rest of the settings if not set? Perhaps combine with pcm-routing: > > <blank> - no audio > audio-mode = "pcm"; > audio-mode = "hci"; (or "sco-hci"?) > > IF: That's right: the rest of the parameters are not needed if configure-audio is false. Talking about your suggestions, this driver does nothing if the audio is either sent inbound or not used at all. Would you agree to something like the configure-pcm-audio flag? So why not combine with pcm-routing? [...] > Use the actual rate rather than an enumeration. It is a simple div by > 128k and log2 to convert in the driver. This makes the property more > compatible with other chips. > > IF: These rates are subject to change in future chips with no guarantees of the pattern holding. I would prefer to use the actual value expected by the firmware if you don't mind to avoid maintaining the extra driver code. Exactly my point. If the next chip has 0-64k, 1-256k, 2-2048k, your binding is broken. Just put the bit rate in the binding and do the mapping to register value in the driver. > What does incall mean? What is the bit rate when not in a call? > > IF: That's the name given to me by the hardware guys. What do you think about the "pcm-interface-rate" instead? That's somewhat ambiguous. Is that the clock, sample rate, or bit rate (sample rate * sample size * channels). Rob [1] https://lwn.net/Articles/643878/ [2] http://www.spinics.net/lists/devicetree/msg83596.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob. -----Original Message----- From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Rob Herring Sent: Thursday, June 18, 2015 3:41 PM To: Ilya Faenson Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: > Hi Rob, Your emails are base64 encoded. They should be plain text for the list. IF: The Outlook is configured to respond in the sender's format. I therefore respond in the encoding you've used. > -----Original Message----- > From: Rob Herring [mailto:robherring2@gmail.com] > Sent: Thursday, June 18, 2015 11:03 AM > To: Ilya Faenson > Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org > Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings > > On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: >> + devicetree lists [...] >> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >> new file mode 100644 >> index 0000000..5dbcd57 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >> @@ -0,0 +1,86 @@ >> +btbcm >> +------ >> + >> +Required properties: >> + >> + - compatible : must be "brcm,brcm-bt-uart". > > You need to describe the chip, not the interface. > > IF: This driver is for all Broadcom Bluetooth UART based chips. BT only chips? Most/many Broadcom chips are combo chips. I understand the driver for BT is *mostly* separate from other chip functions like WiFi, GPS and NFC, but the h/w is a single chip. I say most because likely there are some parts shared: a voltage rail, reset line, or power down line. I think some can do BT over the SDIO interface too, so the interface may be shared. The DT is a description of the h/w (i.e. what part # for a chip) and not a driver data structure. You need to describe the whole chip in the binding. It is a Linux problem if there needs to be multiple separate drivers. IF: Defining complete DT description for the Broadcom combo chips for multiple interfaces is well beyond the scope of what I am doing. I just need to define a DT node for the input and output GPIOs connected to the BT UART chip. BT may or may not be part of the combo chip: it does not really matter for this exercise. I thought I would take this opportunity to place some BT device parameters into that node as well. If you're not comfortable with this, I could just call it a "GPIO set" to avoid mentioning BT and UART at all but it would make little sense. Still, I could consider it. Would you have "GPIO set" recommendations? Alternatively, NFC Marvell code you're referring to has parameters configured as Linux module parameters. I could do the same too, avoiding this device tree discussion. Let me know. Generally speaking (pontification hat on :-)), what you're trying to do (description of the whole chip) is well beyond what say ACPI has done (I was involved some in the BT ACPI exercise a few years ago). BT UART interface is described in ACPI independently of other parts of the same combo chip. Requirements like that slow down the DT development in my opinion as companies are understandably reluctant to work with unrealistic goals. End of pontification. :-) > >> + - tty : tty device connected to this Bluetooth device. > > "tty" is a bit of a Linuxism and "ttyS0" certainly is. Further, there > is no guarantee which uart is assigned ttyS0. > > This should be a phandle to the connected uart if not a sub node of > the uart. This is complicated since these chips have multiple host > connections. It needs to go under either uart or SDIO host and have a > reference back to the one it is not under. Given the SDIO interface is > discoverable (although sideband gpios and regulators are not), I would > put this under the uart node as that is never discoverable. > > As I've mentioned previously, there's several cases of bindings for > UART slave devices being posted. This all needs to be coordinated to > use a common structure. > > IF: This driver does not really access the UART. If just needs to have a string of some sort to map instances of the BlueZ protocol into platform devices to employ the right GPIOs and interrupts. I could use anything you recommend available through the tty_struct coming to the protocol from the BlueZ line discipline. Moreover, every end user platform I've ever dealt with in 16 years of working with Bluetooth had a single BT UART device. So these are really rare (typically test platforms) cases only. The mapping is not needed for most platforms at all. I suspect the right thing to do would be to make this parameter optional. The mapping would be done only if the parameter is present. I will use anything tty_struct derived you specify. Makes sense? You've missed my point. I'm not talking about connecting multiple devices to a UART at once. There are several instances of people trying to add UART connected devices into DT[1][2]. My point is these devices all need to have the DT binding done in a common way across different platforms. Otherwise, we can not have common code to parse the DT and find devices attached to a UART. IF: Chances are I was not clear enough. I was not talking about connecting multiple devices to a UART. I was talking about connecting one Broadcom BT device to one serial port and another Broadcom BT device to another serial port (rare enough setup). I do understand your goals though. I would be happy to participate in that exercise (subject to the management approval) once DT has published the UART device parameters and the Linux bluetooth-next has support for DT enumerated devices. I don’t see it happening soon though. Marvell example you've referred me to has nothing of the sort. What do you think of allowing us something to ship now with an understanding that we would support your UART enumerated devices once they are published? > >> + >> +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. >> + >> + - oper-speed : Bluetooth device operational baud rate. >> + Default: 3000000. >> + >> + - manual-fc : flow control UART in suspend / resume scenarios. >> + Default: 0. > > Can be boolean? > > I don't really follow the description. > > IF: Okay, I will make it boolean. To clarify the description, it controls whether the BlueZ protocol needs to flow control the UART when the BT device is suspended and un-flow control it when the device is resumed. Okay. Discussion of BlueZ is not relevant to bindings. I would say something like "The hardware requires the host UART flow-control to be de-asserted during suspend." IF: Okay. [...] >> + - configure-audio : configure platform PCM SCO flag. >> + Default: false. > > So ignore the rest of the settings if not set? Perhaps combine with pcm-routing: > > <blank> - no audio > audio-mode = "pcm"; > audio-mode = "hci"; (or "sco-hci"?) > > IF: That's right: the rest of the parameters are not needed if configure-audio is false. Talking about your suggestions, this driver does nothing if the audio is either sent inbound or not used at all. Would you agree to something like the configure-pcm-audio flag? So why not combine with pcm-routing? IF: Okay, I can derive one from another. [...] > Use the actual rate rather than an enumeration. It is a simple div by > 128k and log2 to convert in the driver. This makes the property more > compatible with other chips. > > IF: These rates are subject to change in future chips with no guarantees of the pattern holding. I would prefer to use the actual value expected by the firmware if you don't mind to avoid maintaining the extra driver code. Exactly my point. If the next chip has 0-64k, 1-256k, 2-2048k, your binding is broken. Just put the bit rate in the binding and do the mapping to register value in the driver. IF: Okay, will implement. > What does incall mean? What is the bit rate when not in a call? > > IF: That's the name given to me by the hardware guys. What do you think about the "pcm-interface-rate" instead? That's somewhat ambiguous. Is that the clock, sample rate, or bit rate (sample rate * sample size * channels). IF: Okay, will use the original name. Rob [1] https://lwn.net/Articles/643878/ [2] http://www.spinics.net/lists/devicetree/msg83596.html -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: > Hi Rob. > > -----Original Message----- > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Rob Herring > Sent: Thursday, June 18, 2015 3:41 PM > To: Ilya Faenson > Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org > Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings > > On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: >> Hi Rob, > > Your emails are base64 encoded. They should be plain text for the list. > > IF: The Outlook is configured to respond in the sender's format. I therefore respond in the encoding you've used. I assure you that that is not the case or I would be banished from lists by now. Outlook is generally incapable of correctly sending mails to lists. The reply header above is one aspect of that. The other problem is your replies don't get wrapped and prefixed properly. That could be my client I guess, but *all* other mails are fine. My sent mails have: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable >> -----Original Message----- >> From: Rob Herring [mailto:robherring2@gmail.com] >> Sent: Thursday, June 18, 2015 11:03 AM >> To: Ilya Faenson >> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org >> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings >> >> On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: >>> + devicetree lists > > [...] > >>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>> new file mode 100644 >>> index 0000000..5dbcd57 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>> @@ -0,0 +1,86 @@ >>> +btbcm >>> +------ >>> + >>> +Required properties: >>> + >>> + - compatible : must be "brcm,brcm-bt-uart". >> >> You need to describe the chip, not the interface. >> >> IF: This driver is for all Broadcom Bluetooth UART based chips. > > BT only chips? Most/many Broadcom chips are combo chips. I understand > the driver for BT is *mostly* separate from other chip functions like > WiFi, GPS and NFC, but the h/w is a single chip. I say most because > likely there are some parts shared: a voltage rail, reset line, or > power down line. I think some can do BT over the SDIO interface too, > so the interface may be shared. The DT is a description of the h/w > (i.e. what part # for a chip) and not a driver data structure. You > need to describe the whole chip in the binding. It is a Linux problem > if there needs to be multiple separate drivers. > > IF: Defining complete DT description for the Broadcom combo chips for multiple interfaces is well beyond the scope of what I am doing. I just need to define a DT node for the input and output GPIOs connected to the BT UART chip. BT may or may not be part of the combo chip: it does not really matter for this exercise. I thought I would take this opportunity to place some BT device parameters into that node as well. If you're not comfortable with this, I could just call it a "GPIO set" to avoid mentioning BT and UART at all but it would make little sense. Still, I could consider it. Would you have "GPIO set" recommendations? Alternatively, NFC Marvell code you're referring to has parameters configured as Linux module parameters. I could do the same too, avoiding this device tree discussion. Let me know. > I don't completely follow what you mean by "GPIO set", but I'm guessing that is not the right path. > Generally speaking (pontification hat on :-)), what you're trying to do (description of the whole chip) is well beyond what say ACPI has done (I was involved some in the BT ACPI exercise a few years ago). BT UART interface is described in ACPI independently of other parts of the same combo chip. Requirements like that slow down the DT development in my opinion as companies are understandably reluctant to work with unrealistic goals. End of pontification. :-) > ACPI and DT are very different models in terms of abstraction and governance. I'm not going to hash through all the details. I'm not necessarily saying we have to have a single node, but that is my default position. You have convince me that the functions are completely independent and I cannot judge that if you are completely ignoring the WiFi part. From Broadcom chips I've worked with, the supplies at least are shared (something ACPI does not expose). Perhaps we could fudge that and just require the same supply has to be connected to each half. I still worry there could be other internal inter-dependencies. Perhaps BT requires the SDIO clock to be active or something like that. Maybe not on Broadcom chips, but on other vendors and I have to care about them all. Let's step back to what I'm asking for: - A more specific compatible string. This should include the chip name/number. You may not need it today, but it is insurance in case you do find differences latter. The only impact is the match table in your driver. You can also have a less specific compatible string if you want that the driver can match on. - Changing the location of the node. The node hierarchy implicitly defines connections. You have a connection from the host UART to the BT device. This needs to be described. Whether splitting BT and WiFi nodes or not, I've already said it probably makes the most sense to put this under the host uart node. - Splitting the nodes. Here we are looking at doing either: serial@1234 { compatible = "some-soc-uart"; brcm43340 { compatible = "brcm,brcm43340"; sdio-host = <&soc-sdhost>; // BT props // WiFi props }; }; Or: serial@1234 { compatible = "some-soc-uart"; brcm43340 { compatible = "brcm,brcm43340-bt"; // BT props }; }; mmc@5678 { compatible = "some-soc-sdhci"; brcm43340@0 { reg = <0>; compatible = "brcm,brcm43340-wifi"; // WiFi props }; }; Or maybe it is the latter example but we just add phandle links between the 2 nodes. We haven't even considered what happens when a chip also has FM, NFC, and/or GPS. Nor have we considered how to describe the audio connection to the host processor, but we've got nothing common and that can probably come latter. We need to agree figuring all this out is needed. Otherwise, this whole conversation is a waste of time. >> >>> + - tty : tty device connected to this Bluetooth device. >> >> "tty" is a bit of a Linuxism and "ttyS0" certainly is. Further, there >> is no guarantee which uart is assigned ttyS0. >> >> This should be a phandle to the connected uart if not a sub node of >> the uart. This is complicated since these chips have multiple host >> connections. It needs to go under either uart or SDIO host and have a >> reference back to the one it is not under. Given the SDIO interface is >> discoverable (although sideband gpios and regulators are not), I would >> put this under the uart node as that is never discoverable. >> >> As I've mentioned previously, there's several cases of bindings for >> UART slave devices being posted. This all needs to be coordinated to >> use a common structure. >> >> IF: This driver does not really access the UART. If just needs to have a string of some sort to map instances of the BlueZ protocol into platform devices to employ the right GPIOs and interrupts. I could use anything you recommend available through the tty_struct coming to the protocol from the BlueZ line discipline. Moreover, every end user platform I've ever dealt with in 16 years of working with Bluetooth had a single BT UART device. So these are really rare (typically test platforms) cases only. The mapping is not needed for most platforms at all. I suspect the right thing to do would be to make this parameter optional. The mapping would be done only if the parameter is present. I will use anything tty_struct derived you specify. Makes sense? > > > You've missed my point. I'm not talking about connecting multiple > devices to a UART at once. There are several instances of people > trying to add UART connected devices into DT[1][2]. My point is these > devices all need to have the DT binding done in a common way across > different platforms. Otherwise, we can not have common code to parse > the DT and find devices attached to a UART. > > IF: Chances are I was not clear enough. I was not talking about connecting multiple devices to a UART. I was talking about connecting one Broadcom BT device to one serial port and another Broadcom BT device to another serial port (rare enough setup). I do understand your goals though. I would be happy to participate in that exercise (subject to the management approval) once DT has published the UART device parameters and the Linux bluetooth-next has support for DT enumerated devices. I don’t see it happening soon though. Marvell example you've referred me to has nothing of the sort. What do you think of allowing us something to ship now with an understanding that we would support your UART enumerated devices once they are published? Okay. Several people want to describe a connection between a host uart and a device connected to the uart (BT, NFC, GPS, etc.). You are doing this with your "tty" property. My goal and requirement is that this connection be described in DT in the same way regardless of the device attached. Just like all I2C device connections are described by being child nodes under the I2C host regardless of the type of I2C device attached. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in
On 06/19/15 17:47, Rob Herring wrote: > On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faenson<ifaenson@broadcom.com> wrote: >> Hi Rob. >> >> -----Original Message----- >> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Rob Herring >> Sent: Thursday, June 18, 2015 3:41 PM >> To: Ilya Faenson >> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org >> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings >> >> On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faenson<ifaenson@broadcom.com> wrote: >>> Hi Rob, >> >> Your emails are base64 encoded. They should be plain text for the list. >> >> IF: The Outlook is configured to respond in the sender's format. I therefore respond in the encoding you've used. > > I assure you that that is not the case or I would be banished from > lists by now. Outlook is generally incapable of correctly sending > mails to lists. The reply header above is one aspect of that. The > other problem is your replies don't get wrapped and prefixed properly. > That could be my client I guess, but *all* other mails are fine. > > My sent mails have: > > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: quoted-printable > > >>> -----Original Message----- >>> From: Rob Herring [mailto:robherring2@gmail.com] >>> Sent: Thursday, June 18, 2015 11:03 AM >>> To: Ilya Faenson >>> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org >>> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings >>> >>> On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson<ifaenson@broadcom.com> wrote: >>>> + devicetree lists >> >> [...] >> >>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>> new file mode 100644 >>>> index 0000000..5dbcd57 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>> @@ -0,0 +1,86 @@ >>>> +btbcm >>>> +------ >>>> + >>>> +Required properties: >>>> + >>>> + - compatible : must be "brcm,brcm-bt-uart". >>> >>> You need to describe the chip, not the interface. >>> >>> IF: This driver is for all Broadcom Bluetooth UART based chips. >> >> BT only chips? Most/many Broadcom chips are combo chips. I understand >> the driver for BT is *mostly* separate from other chip functions like >> WiFi, GPS and NFC, but the h/w is a single chip. I say most because >> likely there are some parts shared: a voltage rail, reset line, or >> power down line. I think some can do BT over the SDIO interface too, >> so the interface may be shared. The DT is a description of the h/w >> (i.e. what part # for a chip) and not a driver data structure. You >> need to describe the whole chip in the binding. It is a Linux problem >> if there needs to be multiple separate drivers. >> >> IF: Defining complete DT description for the Broadcom combo chips for multiple interfaces is well beyond the scope of what I am doing. I just need to define a DT node for the input and output GPIOs connected to the BT UART chip. BT may or may not be part of the combo chip: it does not really matter for this exercise. I thought I would take this opportunity to place some BT device parameters into that node as well. If you're not comfortable with this, I could just call it a "GPIO set" to avoid mentioning BT and UART at all but it would make little sense. Still, I could consider it. Would you have "GPIO set" recommendations? Alternatively, NFC Marvell code you're referring to has parameters configured as Linux module parameters. I could do the same too, avoiding this device tree discussion. Let me know. >> > > I don't completely follow what you mean by "GPIO set", but I'm > guessing that is not the right path. > >> Generally speaking (pontification hat on :-)), what you're trying to do (description of the whole chip) is well beyond what say ACPI has done (I was involved some in the BT ACPI exercise a few years ago). BT UART interface is described in ACPI independently of other parts of the same combo chip. Requirements like that slow down the DT development in my opinion as companies are understandably reluctant to work with unrealistic goals. End of pontification. :-) >> > > ACPI and DT are very different models in terms of abstraction and > governance. I'm not going to hash through all the details. > > I'm not necessarily saying we have to have a single node, but that is > my default position. You have convince me that the functions are > completely independent and I cannot judge that if you are completely > ignoring the WiFi part. From Broadcom chips I've worked with, the > supplies at least are shared (something ACPI does not expose). Perhaps > we could fudge that and just require the same supply has to be > connected to each half. I still worry there could be other internal > inter-dependencies. Perhaps BT requires the SDIO clock to be active or > something like that. Maybe not on Broadcom chips, but on other vendors > and I have to care about them all. All Broadcom combo chips that I know of have separate supplies, ie. wl-reg-on and bt-reg-on. There already is a binding present for the wifi part. Not extending that may seem ignorant, but as wifi and bt can have a separate interface to the host (admittedly they could share SDIO interface, but they would be exposed as a separate SDIO function) I did not see a reason to object against a separate binding for BT. Whether wifi and bt are on the same device does not seem like something that must be expressed in DT. The physical state may help in determining DT bindings, but it should not be mandatory in my opinion. > Let's step back to what I'm asking for: > > - A more specific compatible string. This should include the chip > name/number. You may not need it today, but it is insurance in case > you do find differences latter. The only impact is the match table in > your driver. You can also have a less specific compatible string if > you want that the driver can match on. > > - Changing the location of the node. The node hierarchy implicitly > defines connections. You have a connection from the host UART to the > BT device. This needs to be described. Whether splitting BT and WiFi > nodes or not, I've already said it probably makes the most sense to > put this under the host uart node. > > - Splitting the nodes. Here we are looking at doing either: > > serial@1234 { > compatible = "some-soc-uart"; > > brcm43340 { > compatible = "brcm,brcm43340"; > sdio-host =<&soc-sdhost>; > // BT props > // WiFi props > }; > }; > > Or: > > serial@1234 { > compatible = "some-soc-uart"; > > brcm43340 { > compatible = "brcm,brcm43340-bt"; > // BT props > }; > }; > > mmc@5678 { > compatible = "some-soc-sdhci"; > > brcm43340@0 { > reg =<0>; > compatible = "brcm,brcm43340-wifi"; > // WiFi props > }; > }; This is more or less what is in the wifi binding today. See bindings/net/wireless/brcm,bcm43xx-fmac.txt. Regards, Arend > Or maybe it is the latter example but we just add phandle links > between the 2 nodes. > > We haven't even considered what happens when a chip also has FM, NFC, > and/or GPS. Nor have we considered how to describe the audio > connection to the host processor, but we've got nothing common and > that can probably come latter. > > We need to agree figuring all this out is needed. Otherwise, this > whole conversation is a waste of time. > >>> >>>> + - tty : tty device connected to this Bluetooth device. >>> >>> "tty" is a bit of a Linuxism and "ttyS0" certainly is. Further, there >>> is no guarantee which uart is assigned ttyS0. >>> >>> This should be a phandle to the connected uart if not a sub node of >>> the uart. This is complicated since these chips have multiple host >>> connections. It needs to go under either uart or SDIO host and have a >>> reference back to the one it is not under. Given the SDIO interface is >>> discoverable (although sideband gpios and regulators are not), I would >>> put this under the uart node as that is never discoverable. >>> >>> As I've mentioned previously, there's several cases of bindings for >>> UART slave devices being posted. This all needs to be coordinated to >>> use a common structure. >>> >>> IF: This driver does not really access the UART. If just needs to have a string of some sort to map instances of the BlueZ protocol into platform devices to employ the right GPIOs and interrupts. I could use anything you recommend available through the tty_struct coming to the protocol from the BlueZ line discipline. Moreover, every end user platform I've ever dealt with in 16 years of working with Bluetooth had a single BT UART device. So these are really rare (typically test platforms) cases only. The mapping is not needed for most platforms at all. I suspect the right thing to do would be to make this parameter optional. The mapping would be done only if the parameter is present. I will use anything tty_struct derived you specify. Makes sense? >> >> >> You've missed my point. I'm not talking about connecting multiple >> devices to a UART at once. There are several instances of people >> trying to add UART connected devices into DT[1][2]. My point is these >> devices all need to have the DT binding done in a common way across >> different platforms. Otherwise, we can not have common code to parse >> the DT and find devices attached to a UART. >> >> IF: Chances are I was not clear enough. I was not talking about connecting multiple devices to a UART. I was talking about connecting one Broadcom BT device to one serial port and another Broadcom BT device to another serial port (rare enough setup). I do understand your goals though. I would be happy to participate in that exercise (subject to the management approval) once DT has published the UART device parameters and the Linux bluetooth-next has support for DT enumerated devices. I don’t see it happening soon though. Marvell example you've referred me to has nothing of the sort. What do you think of allowing us something to ship now with an understanding that we would support your UART enumerated devices once they are published? > > Okay. Several people want to describe a connection between a host uart > and a device connected to the uart (BT, NFC, GPS, etc.). You are doing > this with your "tty" property. My goal and requirement is that this > connection be described in DT in the same way regardless of the device > attached. Just like all I2C device connections are described by being > child nodes under the I2C host regardless of the type of I2C device > attached. > > Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in
On Fri, Jun 19, 2015 at 12:06 PM, Arend van Spriel <arend@broadcom.com> wrote: > On 06/19/15 17:47, Rob Herring wrote: >> >> On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faenson<ifaenson@broadcom.com> >> wrote: >>> >>> Hi Rob. >>> >>> -----Original Message----- >>> From: linux-bluetooth-owner@vger.kernel.org >>> [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Rob Herring >>> Sent: Thursday, June 18, 2015 3:41 PM >>> To: Ilya Faenson >>> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; >>> linux-bluetooth@vger.kernel.org >>> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree >>> bindings >>> >>> On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faenson<ifaenson@broadcom.com> >>> wrote: >>>> >>>> Hi Rob, >>> >>> >>> Your emails are base64 encoded. They should be plain text for the list. >>> >>> IF: The Outlook is configured to respond in the sender's format. I >>> therefore respond in the encoding you've used. >> >> >> I assure you that that is not the case or I would be banished from >> lists by now. Outlook is generally incapable of correctly sending >> mails to lists. The reply header above is one aspect of that. The >> other problem is your replies don't get wrapped and prefixed properly. >> That could be my client I guess, but *all* other mails are fine. >> >> My sent mails have: >> >> Content-Type: text/plain; charset=UTF-8 >> Content-Transfer-Encoding: quoted-printable >> >> >>>> -----Original Message----- >>>> From: Rob Herring [mailto:robherring2@gmail.com] >>>> Sent: Thursday, June 18, 2015 11:03 AM >>>> To: Ilya Faenson >>>> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; >>>> linux-bluetooth@vger.kernel.org >>>> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree >>>> bindings >>>> >>>> On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson<ifaenson@broadcom.com> >>>> wrote: >>>>> >>>>> + devicetree lists >>> >>> >>> [...] >>> >>>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>> b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>> new file mode 100644 >>>>> index 0000000..5dbcd57 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>> @@ -0,0 +1,86 @@ >>>>> +btbcm >>>>> +------ >>>>> + >>>>> +Required properties: >>>>> + >>>>> + - compatible : must be "brcm,brcm-bt-uart". >>>> >>>> >>>> You need to describe the chip, not the interface. >>>> >>>> IF: This driver is for all Broadcom Bluetooth UART based chips. >>> >>> >>> BT only chips? Most/many Broadcom chips are combo chips. I understand >>> the driver for BT is *mostly* separate from other chip functions like >>> WiFi, GPS and NFC, but the h/w is a single chip. I say most because >>> likely there are some parts shared: a voltage rail, reset line, or >>> power down line. I think some can do BT over the SDIO interface too, >>> so the interface may be shared. The DT is a description of the h/w >>> (i.e. what part # for a chip) and not a driver data structure. You >>> need to describe the whole chip in the binding. It is a Linux problem >>> if there needs to be multiple separate drivers. >>> >>> IF: Defining complete DT description for the Broadcom combo chips for >>> multiple interfaces is well beyond the scope of what I am doing. I just need >>> to define a DT node for the input and output GPIOs connected to the BT UART >>> chip. BT may or may not be part of the combo chip: it does not really matter >>> for this exercise. I thought I would take this opportunity to place some BT >>> device parameters into that node as well. If you're not comfortable with >>> this, I could just call it a "GPIO set" to avoid mentioning BT and UART at >>> all but it would make little sense. Still, I could consider it. Would you >>> have "GPIO set" recommendations? Alternatively, NFC Marvell code you're >>> referring to has parameters configured as Linux module parameters. I could >>> do the same too, avoiding this device tree discussion. Let me know. >>> >> >> I don't completely follow what you mean by "GPIO set", but I'm >> guessing that is not the right path. >> >>> Generally speaking (pontification hat on :-)), what you're trying to do >>> (description of the whole chip) is well beyond what say ACPI has done (I was >>> involved some in the BT ACPI exercise a few years ago). BT UART interface is >>> described in ACPI independently of other parts of the same combo chip. >>> Requirements like that slow down the DT development in my opinion as >>> companies are understandably reluctant to work with unrealistic goals. End >>> of pontification. :-) >>> >> >> ACPI and DT are very different models in terms of abstraction and >> governance. I'm not going to hash through all the details. >> >> I'm not necessarily saying we have to have a single node, but that is >> my default position. You have convince me that the functions are >> completely independent and I cannot judge that if you are completely >> ignoring the WiFi part. From Broadcom chips I've worked with, the >> supplies at least are shared (something ACPI does not expose). Perhaps >> we could fudge that and just require the same supply has to be >> connected to each half. I still worry there could be other internal >> inter-dependencies. Perhaps BT requires the SDIO clock to be active or >> something like that. Maybe not on Broadcom chips, but on other vendors >> and I have to care about them all. > > > All Broadcom combo chips that I know of have separate supplies, ie. > wl-reg-on and bt-reg-on. There already is a binding present for the wifi GPIOs are not supplies. For the module I'm working with (43340 based) there is a single VDDIO and VBAT supplies which are shared. Now whether the module or the chip are tying things together, I don't know. There is also a 32kHz clock input. Is that part of WiFi or BT? > part. Not extending that may seem ignorant, but as wifi and bt can have a > separate interface to the host (admittedly they could share SDIO interface, > but they would be exposed as a separate SDIO function) I did not see a > reason to object against a separate binding for BT. Whether wifi and bt are > on the same device does not seem like something that must be expressed in > DT. The physical state may help in determining DT bindings, but it should > not be mandatory in my opinion. We don't need it in DT until we do. Soon as there is some some interdependence, something in DT will be needed. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in
On 06/19/15 20:49, Rob Herring wrote: > On Fri, Jun 19, 2015 at 12:06 PM, Arend van Spriel<arend@broadcom.com> wrote: >> On 06/19/15 17:47, Rob Herring wrote: >>> >>> On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faenson<ifaenson@broadcom.com> >>> wrote: >>>> >>>> Hi Rob. >>>> >>>> -----Original Message----- >>>> From: linux-bluetooth-owner@vger.kernel.org >>>> [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Rob Herring >>>> Sent: Thursday, June 18, 2015 3:41 PM >>>> To: Ilya Faenson >>>> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; >>>> linux-bluetooth@vger.kernel.org >>>> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree >>>> bindings >>>> >>>> On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faenson<ifaenson@broadcom.com> >>>> wrote: >>>>> >>>>> Hi Rob, >>>> >>>> >>>> Your emails are base64 encoded. They should be plain text for the list. >>>> >>>> IF: The Outlook is configured to respond in the sender's format. I >>>> therefore respond in the encoding you've used. >>> >>> >>> I assure you that that is not the case or I would be banished from >>> lists by now. Outlook is generally incapable of correctly sending >>> mails to lists. The reply header above is one aspect of that. The >>> other problem is your replies don't get wrapped and prefixed properly. >>> That could be my client I guess, but *all* other mails are fine. >>> >>> My sent mails have: >>> >>> Content-Type: text/plain; charset=UTF-8 >>> Content-Transfer-Encoding: quoted-printable >>> >>> >>>>> -----Original Message----- >>>>> From: Rob Herring [mailto:robherring2@gmail.com] >>>>> Sent: Thursday, June 18, 2015 11:03 AM >>>>> To: Ilya Faenson >>>>> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; >>>>> linux-bluetooth@vger.kernel.org >>>>> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree >>>>> bindings >>>>> >>>>> On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson<ifaenson@broadcom.com> >>>>> wrote: >>>>>> >>>>>> + devicetree lists >>>> >>>> >>>> [...] >>>> >>>>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>>> b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>>> new file mode 100644 >>>>>> index 0000000..5dbcd57 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>>> @@ -0,0 +1,86 @@ >>>>>> +btbcm >>>>>> +------ >>>>>> + >>>>>> +Required properties: >>>>>> + >>>>>> + - compatible : must be "brcm,brcm-bt-uart". >>>>> >>>>> >>>>> You need to describe the chip, not the interface. >>>>> >>>>> IF: This driver is for all Broadcom Bluetooth UART based chips. >>>> >>>> >>>> BT only chips? Most/many Broadcom chips are combo chips. I understand >>>> the driver for BT is *mostly* separate from other chip functions like >>>> WiFi, GPS and NFC, but the h/w is a single chip. I say most because >>>> likely there are some parts shared: a voltage rail, reset line, or >>>> power down line. I think some can do BT over the SDIO interface too, >>>> so the interface may be shared. The DT is a description of the h/w >>>> (i.e. what part # for a chip) and not a driver data structure. You >>>> need to describe the whole chip in the binding. It is a Linux problem >>>> if there needs to be multiple separate drivers. >>>> >>>> IF: Defining complete DT description for the Broadcom combo chips for >>>> multiple interfaces is well beyond the scope of what I am doing. I just need >>>> to define a DT node for the input and output GPIOs connected to the BT UART >>>> chip. BT may or may not be part of the combo chip: it does not really matter >>>> for this exercise. I thought I would take this opportunity to place some BT >>>> device parameters into that node as well. If you're not comfortable with >>>> this, I could just call it a "GPIO set" to avoid mentioning BT and UART at >>>> all but it would make little sense. Still, I could consider it. Would you >>>> have "GPIO set" recommendations? Alternatively, NFC Marvell code you're >>>> referring to has parameters configured as Linux module parameters. I could >>>> do the same too, avoiding this device tree discussion. Let me know. >>>> >>> >>> I don't completely follow what you mean by "GPIO set", but I'm >>> guessing that is not the right path. >>> >>>> Generally speaking (pontification hat on :-)), what you're trying to do >>>> (description of the whole chip) is well beyond what say ACPI has done (I was >>>> involved some in the BT ACPI exercise a few years ago). BT UART interface is >>>> described in ACPI independently of other parts of the same combo chip. >>>> Requirements like that slow down the DT development in my opinion as >>>> companies are understandably reluctant to work with unrealistic goals. End >>>> of pontification. :-) >>>> >>> >>> ACPI and DT are very different models in terms of abstraction and >>> governance. I'm not going to hash through all the details. >>> >>> I'm not necessarily saying we have to have a single node, but that is >>> my default position. You have convince me that the functions are >>> completely independent and I cannot judge that if you are completely >>> ignoring the WiFi part. From Broadcom chips I've worked with, the >>> supplies at least are shared (something ACPI does not expose). Perhaps >>> we could fudge that and just require the same supply has to be >>> connected to each half. I still worry there could be other internal >>> inter-dependencies. Perhaps BT requires the SDIO clock to be active or >>> something like that. Maybe not on Broadcom chips, but on other vendors >>> and I have to care about them all. >> >> >> All Broadcom combo chips that I know of have separate supplies, ie. >> wl-reg-on and bt-reg-on. There already is a binding present for the wifi > > GPIOs are not supplies. For the module I'm working with (43340 based) > there is a single VDDIO and VBAT supplies which are shared. Now > whether the module or the chip are tying things together, I don't > know. There is also a 32kHz clock input. Is that part of WiFi or BT? True and I see where you are going here. The 32kHz clock is input for low-power oscillator in the chip. That LPO provides clock for the interconnect in the chip so it is not part of wifi nor bt. >> part. Not extending that may seem ignorant, but as wifi and bt can have a >> separate interface to the host (admittedly they could share SDIO interface, >> but they would be exposed as a separate SDIO function) I did not see a >> reason to object against a separate binding for BT. Whether wifi and bt are >> on the same device does not seem like something that must be expressed in >> DT. The physical state may help in determining DT bindings, but it should >> not be mandatory in my opinion. > > We don't need it in DT until we do. Soon as there is some some > interdependence, something in DT will be needed. Agree. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe devicetree" in
Hi Rob, -----Original Message----- From: Rob Herring [mailto:robherring2@gmail.com] Sent: Friday, June 19, 2015 11:48 AM To: Ilya Faenson Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: > Hi Rob. > > -----Original Message----- > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Rob Herring > Sent: Thursday, June 18, 2015 3:41 PM > To: Ilya Faenson > Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org > Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings > > On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: >> Hi Rob, > > Your emails are base64 encoded. They should be plain text for the list. > > IF: The Outlook is configured to respond in the sender's format. I therefore respond in the encoding you've used. I assure you that that is not the case or I would be banished from lists by now. Outlook is generally incapable of correctly sending mails to lists. The reply header above is one aspect of that. The other problem is your replies don't get wrapped and prefixed properly. That could be my client I guess, but *all* other mails are fine. My sent mails have: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable IF: Unluckily, Outlook is what I am supposed to use. I post patches from the Ubuntu VM but I have the command line access to it only. >> -----Original Message----- >> From: Rob Herring [mailto:robherring2@gmail.com] >> Sent: Thursday, June 18, 2015 11:03 AM >> To: Ilya Faenson >> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; linux-bluetooth@vger.kernel.org >> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings >> >> On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson <ifaenson@broadcom.com> wrote: >>> + devicetree lists > > [...] > >>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>> new file mode 100644 >>> index 0000000..5dbcd57 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>> @@ -0,0 +1,86 @@ >>> +btbcm >>> +------ >>> + >>> +Required properties: >>> + >>> + - compatible : must be "brcm,brcm-bt-uart". >> >> You need to describe the chip, not the interface. >> >> IF: This driver is for all Broadcom Bluetooth UART based chips. > > BT only chips? Most/many Broadcom chips are combo chips. I understand > the driver for BT is *mostly* separate from other chip functions like > WiFi, GPS and NFC, but the h/w is a single chip. I say most because > likely there are some parts shared: a voltage rail, reset line, or > power down line. I think some can do BT over the SDIO interface too, > so the interface may be shared. The DT is a description of the h/w > (i.e. what part # for a chip) and not a driver data structure. You > need to describe the whole chip in the binding. It is a Linux problem > if there needs to be multiple separate drivers. > > IF: Defining complete DT description for the Broadcom combo chips for multiple interfaces is well beyond the scope of what I am doing. I just need to define a DT node for the input and output GPIOs connected to the BT UART chip. BT may or may not be part of the combo chip: it does not really matter for this exercise. I thought I would take this opportunity to place some BT device parameters into that node as well. If you're not comfortable with this, I could just call it a "GPIO set" to avoid mentioning BT and UART at all but it would make little sense. Still, I could consider it. Would you have "GPIO set" recommendations? Alternatively, NFC Marvell code you're referring to has parameters configured as Linux module parameters. I could do the same too, avoiding this device tree discussion. Let me know. > I don't completely follow what you mean by "GPIO set", but I'm guessing that is not the right path. > Generally speaking (pontification hat on :-)), what you're trying to do (description of the whole chip) is well beyond what say ACPI has done (I was involved some in the BT ACPI exercise a few years ago). BT UART interface is described in ACPI independently of other parts of the same combo chip. Requirements like that slow down the DT development in my opinion as companies are understandably reluctant to work with unrealistic goals. End of pontification. :-) > ACPI and DT are very different models in terms of abstraction and governance. I'm not going to hash through all the details. I'm not necessarily saying we have to have a single node, but that is my default position. You have convince me that the functions are completely independent and I cannot judge that if you are completely ignoring the WiFi part. From Broadcom chips I've worked with, the supplies at least are shared (something ACPI does not expose). Perhaps we could fudge that and just require the same supply has to be connected to each half. I still worry there could be other internal inter-dependencies. Perhaps BT requires the SDIO clock to be active or something like that. Maybe not on Broadcom chips, but on other vendors and I have to care about them all. Let's step back to what I'm asking for: - A more specific compatible string. This should include the chip name/number. You may not need it today, but it is insurance in case you do find differences latter. The only impact is the match table in your driver. You can also have a less specific compatible string if you want that the driver can match on. IF: Okay, I can change that. - Changing the location of the node. The node hierarchy implicitly defines connections. You have a connection from the host UART to the BT device. This needs to be described. Whether splitting BT and WiFi nodes or not, I've already said it probably makes the most sense to put this under the host uart node. IF: Okay, I have just tried placing my node under the UART. The platform driver probe is no longer called then though. What am I doing wrong? Pasting the relevant snippet: IF: &uart1 { IF: status = "okay"; IF: ... IF: bcm4354_bt_uart: bcm4354-bt-uart { IF: compatible = "brcm,brcm4354-bt-uart"; IF: bt-wake-gpios = <&gpio4 30 GPIO_ACTIVE_HIGH>; IF: ... IF: }; IF: }; - Splitting the nodes. Here we are looking at doing either: serial@1234 { compatible = "some-soc-uart"; brcm43340 { compatible = "brcm,brcm43340"; sdio-host = <&soc-sdhost>; // BT props // WiFi props }; }; Or: serial@1234 { compatible = "some-soc-uart"; brcm43340 { compatible = "brcm,brcm43340-bt"; // BT props }; }; mmc@5678 { compatible = "some-soc-sdhci"; brcm43340@0 { reg = <0>; compatible = "brcm,brcm43340-wifi"; // WiFi props }; }; Or maybe it is the latter example but we just add phandle links between the 2 nodes. We haven't even considered what happens when a chip also has FM, NFC, and/or GPS. Nor have we considered how to describe the audio connection to the host processor, but we've got nothing common and that can probably come latter. We need to agree figuring all this out is needed. Otherwise, this whole conversation is a waste of time. IF: Appreciate the detailed elaborations. The latter example with phandle links sounds reasonable to me. I am afraid I'm not in a position to agree to everything though as I'm responsible for the BT only. Arend Van Spriel representing Broadcom WLAN has started talking to you: that's good. >> >>> + - tty : tty device connected to this Bluetooth device. >> >> "tty" is a bit of a Linuxism and "ttyS0" certainly is. Further, there >> is no guarantee which uart is assigned ttyS0. >> >> This should be a phandle to the connected uart if not a sub node of >> the uart. This is complicated since these chips have multiple host >> connections. It needs to go under either uart or SDIO host and have a >> reference back to the one it is not under. Given the SDIO interface is >> discoverable (although sideband gpios and regulators are not), I would >> put this under the uart node as that is never discoverable. >> >> As I've mentioned previously, there's several cases of bindings for >> UART slave devices being posted. This all needs to be coordinated to >> use a common structure. >> >> IF: This driver does not really access the UART. If just needs to have a string of some sort to map instances of the BlueZ protocol into platform devices to employ the right GPIOs and interrupts. I could use anything you recommend available through the tty_struct coming to the protocol from the BlueZ line discipline. Moreover, every end user platform I've ever dealt with in 16 years of working with Bluetooth had a single BT UART device. So these are really rare (typically test platforms) cases only. The mapping is not needed for most platforms at all. I suspect the right thing to do would be to make this parameter optional. The mapping would be done only if the parameter is present. I will use anything tty_struct derived you specify. Makes sense? > > > You've missed my point. I'm not talking about connecting multiple > devices to a UART at once. There are several instances of people > trying to add UART connected devices into DT[1][2]. My point is these > devices all need to have the DT binding done in a common way across > different platforms. Otherwise, we can not have common code to parse > the DT and find devices attached to a UART. > > IF: Chances are I was not clear enough. I was not talking about connecting multiple devices to a UART. I was talking about connecting one Broadcom BT device to one serial port and another Broadcom BT device to another serial port (rare enough setup). I do understand your goals though. I would be happy to participate in that exercise (subject to the management approval) once DT has published the UART device parameters and the Linux bluetooth-next has support for DT enumerated devices. I don’t see it happening soon though. Marvell example you've referred me to has nothing of the sort. What do you think of allowing us something to ship now with an understanding that we would support your UART enumerated devices once they are published? Okay. Several people want to describe a connection between a host uart and a device connected to the uart (BT, NFC, GPS, etc.). You are doing this with your "tty" property. My goal and requirement is that this connection be described in DT in the same way regardless of the device attached. Just like all I2C device connections are described by being child nodes under the I2C host regardless of the type of I2C device attached. IF: All good points, Rob. I will certainly get rid of the tty property if I can make the child device idea work. My complication is in the need to map say the DT device parent (UART) into the tty_struct used by the Linux BlueZ protocol. Any ideas on how to implement that ? Many thanks! Rob
On 06/19/15 21:20, Arend van Spriel wrote: > On 06/19/15 20:49, Rob Herring wrote: >> On Fri, Jun 19, 2015 at 12:06 PM, Arend van Spriel<arend@broadcom.com> >> wrote: >>> On 06/19/15 17:47, Rob Herring wrote: >>>> >>>> On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faenson<ifaenson@broadcom.com> >>>> wrote: >>>>> >>>>> Hi Rob. >>>>> >>>>> -----Original Message----- >>>>> From: linux-bluetooth-owner@vger.kernel.org >>>>> [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Rob >>>>> Herring >>>>> Sent: Thursday, June 18, 2015 3:41 PM >>>>> To: Ilya Faenson >>>>> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; >>>>> linux-bluetooth@vger.kernel.org >>>>> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree >>>>> bindings >>>>> >>>>> On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faenson<ifaenson@broadcom.com> >>>>> wrote: >>>>>> >>>>>> Hi Rob, >>>>> >>>>> >>>>> Your emails are base64 encoded. They should be plain text for the >>>>> list. >>>>> >>>>> IF: The Outlook is configured to respond in the sender's format. I >>>>> therefore respond in the encoding you've used. >>>> >>>> >>>> I assure you that that is not the case or I would be banished from >>>> lists by now. Outlook is generally incapable of correctly sending >>>> mails to lists. The reply header above is one aspect of that. The >>>> other problem is your replies don't get wrapped and prefixed properly. >>>> That could be my client I guess, but *all* other mails are fine. >>>> >>>> My sent mails have: >>>> >>>> Content-Type: text/plain; charset=UTF-8 >>>> Content-Transfer-Encoding: quoted-printable >>>> >>>> >>>>>> -----Original Message----- >>>>>> From: Rob Herring [mailto:robherring2@gmail.com] >>>>>> Sent: Thursday, June 18, 2015 11:03 AM >>>>>> To: Ilya Faenson >>>>>> Cc: marcel@holtmann.org; Arend Van Spriel; >>>>>> devicetree@vger.kernel.org; >>>>>> linux-bluetooth@vger.kernel.org >>>>>> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree >>>>>> bindings >>>>>> >>>>>> On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson<ifaenson@broadcom.com> >>>>>> wrote: >>>>>>> >>>>>>> + devicetree lists >>>>> >>>>> >>>>> [...] >>>>> >>>>>>> diff --git >>>>>>> a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>>>> b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>>>> new file mode 100644 >>>>>>> index 0000000..5dbcd57 >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>>>> @@ -0,0 +1,86 @@ >>>>>>> +btbcm >>>>>>> +------ >>>>>>> + >>>>>>> +Required properties: >>>>>>> + >>>>>>> + - compatible : must be "brcm,brcm-bt-uart". >>>>>> >>>>>> >>>>>> You need to describe the chip, not the interface. >>>>>> >>>>>> IF: This driver is for all Broadcom Bluetooth UART based chips. >>>>> >>>>> >>>>> BT only chips? Most/many Broadcom chips are combo chips. I understand >>>>> the driver for BT is *mostly* separate from other chip functions like >>>>> WiFi, GPS and NFC, but the h/w is a single chip. I say most because >>>>> likely there are some parts shared: a voltage rail, reset line, or >>>>> power down line. I think some can do BT over the SDIO interface too, >>>>> so the interface may be shared. The DT is a description of the h/w >>>>> (i.e. what part # for a chip) and not a driver data structure. You >>>>> need to describe the whole chip in the binding. It is a Linux problem >>>>> if there needs to be multiple separate drivers. >>>>> >>>>> IF: Defining complete DT description for the Broadcom combo chips for >>>>> multiple interfaces is well beyond the scope of what I am doing. I >>>>> just need >>>>> to define a DT node for the input and output GPIOs connected to the >>>>> BT UART >>>>> chip. BT may or may not be part of the combo chip: it does not >>>>> really matter >>>>> for this exercise. I thought I would take this opportunity to place >>>>> some BT >>>>> device parameters into that node as well. If you're not comfortable >>>>> with >>>>> this, I could just call it a "GPIO set" to avoid mentioning BT and >>>>> UART at >>>>> all but it would make little sense. Still, I could consider it. >>>>> Would you >>>>> have "GPIO set" recommendations? Alternatively, NFC Marvell code >>>>> you're >>>>> referring to has parameters configured as Linux module parameters. >>>>> I could >>>>> do the same too, avoiding this device tree discussion. Let me know. >>>>> >>>> >>>> I don't completely follow what you mean by "GPIO set", but I'm >>>> guessing that is not the right path. >>>> >>>>> Generally speaking (pontification hat on :-)), what you're trying >>>>> to do >>>>> (description of the whole chip) is well beyond what say ACPI has >>>>> done (I was >>>>> involved some in the BT ACPI exercise a few years ago). BT UART >>>>> interface is >>>>> described in ACPI independently of other parts of the same combo chip. >>>>> Requirements like that slow down the DT development in my opinion as >>>>> companies are understandably reluctant to work with unrealistic >>>>> goals. End >>>>> of pontification. :-) >>>>> >>>> >>>> ACPI and DT are very different models in terms of abstraction and >>>> governance. I'm not going to hash through all the details. >>>> >>>> I'm not necessarily saying we have to have a single node, but that is >>>> my default position. You have convince me that the functions are >>>> completely independent and I cannot judge that if you are completely >>>> ignoring the WiFi part. From Broadcom chips I've worked with, the >>>> supplies at least are shared (something ACPI does not expose). Perhaps >>>> we could fudge that and just require the same supply has to be >>>> connected to each half. I still worry there could be other internal >>>> inter-dependencies. Perhaps BT requires the SDIO clock to be active or >>>> something like that. Maybe not on Broadcom chips, but on other vendors >>>> and I have to care about them all. >>> >>> >>> All Broadcom combo chips that I know of have separate supplies, ie. >>> wl-reg-on and bt-reg-on. There already is a binding present for the wifi >> >> GPIOs are not supplies. For the module I'm working with (43340 based) >> there is a single VDDIO and VBAT supplies which are shared. Now >> whether the module or the chip are tying things together, I don't >> know. There is also a 32kHz clock input. Is that part of WiFi or BT? > > True and I see where you are going here. The 32kHz clock is input for > low-power oscillator in the chip. That LPO provides clock for the > interconnect in the chip so it is not part of wifi nor bt. Hi Rob, Haven't seen much follow up so how to move forward here. We could go for a devicetree node covering all functional entities on the device. However, we are making the chipset and users more often as not end up with a module that some manufacturer cooked up. So whether signals are optional or not is hard to say for us. Making them optional by default seems safest. So could something like below work? Just wanted to know your opinion before moving in that direction. bcm43341: { compatible: 'brcm,bcm43340'; brcm,lpo-clock: <&32khz_clk>; : brcm_wifi: { compatible: "brcm,bcm4329-fmac"; wifi-port: <&mmc3>; : } brcm_bt: { compatible: "brcm,bcm43xx-bt-uart"; bt-port: <&uart1>; : } brcm_nfc: { compatible: "brcm,bcm43340-nfc"; nfc-port: <&uart2>; : } } Regards, Arend >>> part. Not extending that may seem ignorant, but as wifi and bt can >>> have a >>> separate interface to the host (admittedly they could share SDIO >>> interface, >>> but they would be exposed as a separate SDIO function) I did not see a >>> reason to object against a separate binding for BT. Whether wifi and >>> bt are >>> on the same device does not seem like something that must be >>> expressed in >>> DT. The physical state may help in determining DT bindings, but it >>> should >>> not be mandatory in my opinion. >> >> We don't need it in DT until we do. Soon as there is some some >> interdependence, something in DT will be needed. > > Agree. > > Regards, > Arend > -- > To unsubscribe from this list: send the line "unsubscribe > linux-bluetooth" in -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
+ a few more DT maintainers (and frequent reviewers) as this is a common issue only affecting every single mobile platform. On Mon, Jun 29, 2015 at 1:36 PM, Arend van Spriel <arend@broadcom.com> wrote: > On 06/19/15 21:20, Arend van Spriel wrote: >> >> On 06/19/15 20:49, Rob Herring wrote: >>> >>> On Fri, Jun 19, 2015 at 12:06 PM, Arend van Spriel<arend@broadcom.com> >>> wrote: >>>> >>>> On 06/19/15 17:47, Rob Herring wrote: >>>>> >>>>> >>>>> On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faenson<ifaenson@broadcom.com> >>>>> wrote: >>>>>> >>>>>> >>>>>> Hi Rob. >>>>>> >>>>>> -----Original Message----- >>>>>> From: linux-bluetooth-owner@vger.kernel.org >>>>>> [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Rob >>>>>> Herring >>>>>> Sent: Thursday, June 18, 2015 3:41 PM >>>>>> To: Ilya Faenson >>>>>> Cc: marcel@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; >>>>>> linux-bluetooth@vger.kernel.org >>>>>> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree >>>>>> bindings >>>>>> >>>>>> On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faenson<ifaenson@broadcom.com> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> Hi Rob, >>>>>> >>>>>> >>>>>> >>>>>> Your emails are base64 encoded. They should be plain text for the >>>>>> list. >>>>>> >>>>>> IF: The Outlook is configured to respond in the sender's format. I >>>>>> therefore respond in the encoding you've used. >>>>> >>>>> >>>>> >>>>> I assure you that that is not the case or I would be banished from >>>>> lists by now. Outlook is generally incapable of correctly sending >>>>> mails to lists. The reply header above is one aspect of that. The >>>>> other problem is your replies don't get wrapped and prefixed properly. >>>>> That could be my client I guess, but *all* other mails are fine. >>>>> >>>>> My sent mails have: >>>>> >>>>> Content-Type: text/plain; charset=UTF-8 >>>>> Content-Transfer-Encoding: quoted-printable >>>>> >>>>> >>>>>>> -----Original Message----- >>>>>>> From: Rob Herring [mailto:robherring2@gmail.com] >>>>>>> Sent: Thursday, June 18, 2015 11:03 AM >>>>>>> To: Ilya Faenson >>>>>>> Cc: marcel@holtmann.org; Arend Van Spriel; >>>>>>> devicetree@vger.kernel.org; >>>>>>> linux-bluetooth@vger.kernel.org >>>>>>> Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree >>>>>>> bindings >>>>>>> >>>>>>> On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faenson<ifaenson@broadcom.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> + devicetree lists >>>>>> >>>>>> >>>>>> >>>>>> [...] >>>>>> >>>>>>>> diff --git >>>>>>>> a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>>>>> b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>>>>> new file mode 100644 >>>>>>>> index 0000000..5dbcd57 >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt >>>>>>>> @@ -0,0 +1,86 @@ >>>>>>>> +btbcm >>>>>>>> +------ >>>>>>>> + >>>>>>>> +Required properties: >>>>>>>> + >>>>>>>> + - compatible : must be "brcm,brcm-bt-uart". >>>>>>> >>>>>>> >>>>>>> >>>>>>> You need to describe the chip, not the interface. >>>>>>> >>>>>>> IF: This driver is for all Broadcom Bluetooth UART based chips. >>>>>> >>>>>> >>>>>> >>>>>> BT only chips? Most/many Broadcom chips are combo chips. I understand >>>>>> the driver for BT is *mostly* separate from other chip functions like >>>>>> WiFi, GPS and NFC, but the h/w is a single chip. I say most because >>>>>> likely there are some parts shared: a voltage rail, reset line, or >>>>>> power down line. I think some can do BT over the SDIO interface too, >>>>>> so the interface may be shared. The DT is a description of the h/w >>>>>> (i.e. what part # for a chip) and not a driver data structure. You >>>>>> need to describe the whole chip in the binding. It is a Linux problem >>>>>> if there needs to be multiple separate drivers. >>>>>> >>>>>> IF: Defining complete DT description for the Broadcom combo chips for >>>>>> multiple interfaces is well beyond the scope of what I am doing. I >>>>>> just need >>>>>> to define a DT node for the input and output GPIOs connected to the >>>>>> BT UART >>>>>> chip. BT may or may not be part of the combo chip: it does not >>>>>> really matter >>>>>> for this exercise. I thought I would take this opportunity to place >>>>>> some BT >>>>>> device parameters into that node as well. If you're not comfortable >>>>>> with >>>>>> this, I could just call it a "GPIO set" to avoid mentioning BT and >>>>>> UART at >>>>>> all but it would make little sense. Still, I could consider it. >>>>>> Would you >>>>>> have "GPIO set" recommendations? Alternatively, NFC Marvell code >>>>>> you're >>>>>> referring to has parameters configured as Linux module parameters. >>>>>> I could >>>>>> do the same too, avoiding this device tree discussion. Let me know. >>>>>> >>>>> >>>>> I don't completely follow what you mean by "GPIO set", but I'm >>>>> guessing that is not the right path. >>>>> >>>>>> Generally speaking (pontification hat on :-)), what you're trying >>>>>> to do >>>>>> (description of the whole chip) is well beyond what say ACPI has >>>>>> done (I was >>>>>> involved some in the BT ACPI exercise a few years ago). BT UART >>>>>> interface is >>>>>> described in ACPI independently of other parts of the same combo chip. >>>>>> Requirements like that slow down the DT development in my opinion as >>>>>> companies are understandably reluctant to work with unrealistic >>>>>> goals. End >>>>>> of pontification. :-) >>>>>> >>>>> >>>>> ACPI and DT are very different models in terms of abstraction and >>>>> governance. I'm not going to hash through all the details. >>>>> >>>>> I'm not necessarily saying we have to have a single node, but that is >>>>> my default position. You have convince me that the functions are >>>>> completely independent and I cannot judge that if you are completely >>>>> ignoring the WiFi part. From Broadcom chips I've worked with, the >>>>> supplies at least are shared (something ACPI does not expose). Perhaps >>>>> we could fudge that and just require the same supply has to be >>>>> connected to each half. I still worry there could be other internal >>>>> inter-dependencies. Perhaps BT requires the SDIO clock to be active or >>>>> something like that. Maybe not on Broadcom chips, but on other vendors >>>>> and I have to care about them all. >>>> >>>> >>>> >>>> All Broadcom combo chips that I know of have separate supplies, ie. >>>> wl-reg-on and bt-reg-on. There already is a binding present for the wifi >>> >>> >>> GPIOs are not supplies. For the module I'm working with (43340 based) >>> there is a single VDDIO and VBAT supplies which are shared. Now >>> whether the module or the chip are tying things together, I don't >>> know. There is also a 32kHz clock input. Is that part of WiFi or BT? >> >> >> True and I see where you are going here. The 32kHz clock is input for >> low-power oscillator in the chip. That LPO provides clock for the >> interconnect in the chip so it is not part of wifi nor bt. We could handle this by routing clocks and regulators to every sub node. A device with a shared GPIO (or any resource without refcounting) would be a problem. Perhaps this is good enough. This seemed to be the conclusion on earlier discussions[1]. Primarily, I just want to see the whole chip considered and thought around any complications from shared resources. We can't just do the easy part now and ignore the hard parts that cause changes later. I'm not sure how we would handle a shared reset line. Perhaps the first one to claim it wins control. > Hi Rob, > > Haven't seen much follow up so how to move forward here. Sorry, been (and still am) out on vacation. > We could go for a > devicetree node covering all functional entities on the device. However, we > are making the chipset and users more often as not end up with a module that > some manufacturer cooked up. So whether signals are optional or not is hard > to say for us. Making them optional by default seems safest. > > So could something like below work? Just wanted to know your opinion before > moving in that direction. It does have the advantage of having the full chip described in one place, but there are some issues. > bcm43341: { > compatible: 'brcm,bcm43340'; > brcm,lpo-clock: <&32khz_clk>; What driver would handle this and instantiate the sub devices? An MFD driver? If we place all the child nodes under their respective hosts and had a chip/module level node separately, we would then potentially have probe ordering issues (probably solvable though). > : > brcm_wifi: { > compatible: "brcm,bcm4329-fmac"; > wifi-port: <&mmc3>; This should be something generic like "mmc-parent". This is a departure from how the wifi part is currently done. We could take the stance that since there are no dts files actually using the binding, we can change it breaking compatibility (if not upstream it doesn't exist). Or we need to maintain both. Other combo chips such as TI have the WiFi portion under SDIO host node. The other issue potentially is does the SDIO host need to be able to find the children easily. I think the power on the chip first so the SDIO host can detect it issue is one example. We also have no way to define which SDIO function this is. Putting the reg property here would not make sense. So perhaps the phandle needs to go the other way around having an mmc sub-node point to here. So I think we need to keep the SDIO child node. > : > } > brcm_bt: { > compatible: "brcm,bcm43xx-bt-uart"; > bt-port: <&uart1>; This should also be a generic name such as "uart". This needs to be settled at wider scope of how we describe UART slave devices. It seems to be a popular topic recently with yet another posting in the last few days[2]. Thinking about different potential options now for a few hours, I think we should just continue with having separate nodes for each function under the respective host interface. The complicated cases may just have to be solved with code rather than DT bindings. Rob > : > } > brcm_nfc: { > compatible: "brcm,bcm43340-nfc"; > nfc-port: <&uart2>; > : > } > } > [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-January/226343.html [2] http://www.spinics.net/lists/linux-serial/msg18034.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt new file mode 100644 index 0000000..5dbcd57 --- /dev/null +++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt @@ -0,0 +1,86 @@ +btbcm +------ + +Required properties: + + - compatible : must be "brcm,brcm-bt-uart". + - tty : tty device connected to this Bluetooth device. + +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. + + - oper-speed : Bluetooth device operational baud rate. + Default: 3000000. + + - manual-fc : flow control UART in suspend / resume scenarios. + Default: 0. + + - configure-sleep : configure suspend / resume flag. + Default: false. + + - idle-timeout : Number of seconds of inactivity before suspending. + Default: 5. + + - configure-audio : configure platform PCM SCO flag. + Default: false. + + - pcm-clockmode : PCM clock mode. 0-slave, 1-master. + Default: 0. + + - pcm-fillmethod : PCM fill method. 0 to 3. + Default: 2. + + - pcm-fillnum : PCM number of fill bits. 0 to 3. + Default: 0. + + - pcm-fillvalue : PCM fill value. 0 to 7. + Default: 3. + + - pcm-incallbitclock : PCM interface rate. 0-128Kbps, 1-256Kbps, 2-512Kbps, + 3-1024Kbps, 4-2048Kbps. + Default: 0. + + - pcm-lsbfirst : PCM LSB first. 0 or 1. + Default: 0. + + - pcm-rightjustify : PCM Justify. 0-left, 1-right. + Default: 0. + + - pcm-routing : PCM routing. 0-PCM, 1-SCO over HCI. + Default: 0. + + - pcm-shortframesync : PCM sync. 0-short, 1-long. + Default: 0. + + - pcm-syncmode : PCM sync mode. 0-slave, 1-master. + Default: 0. + + +Example: + + bcm4354_bt_uart: bcm4354-bt-uart { + compatible = "brcm,brcm-bt-uart"; + bt-wake-gpios = <&gpio4 30 GPIO_ACTIVE_HIGH>; + bt-host-wake-gpios = <&gpio4 31 GPIO_ACTIVE_HIGH>; + tty = "ttyS0"; + oper-speed = <3000000>; + configure-sleep; + idle-timeout = <5>; + configure-audio; + pcm-clockmode = <0>; + pcm-fillmethod = <2>; + pcm-fillnum = <0>; + pcm-fillvalue = <3>; + pcm-incallbitclock = <0>; + pcm-lsbfirst = <0>; + pcm-rightjustify = <0>; + pcm-routing = <0>; + pcm-shortframesync = <0>; + pcm-syncmode = <0>; + }; +