Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] serial: imx: fix endless loop during suspend
From: Fabio Estevam @ 2018-01-02 19:31 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sascha Hauer, Philipp Zabel,
	Shawn Guo, linux-serial, linux-kernel, stable
In-Reply-To: <20180102161555.GA4503@botnar.kaiser.cx>

Hi Martin,

On Tue, Jan 2, 2018 at 2:15 PM, Martin Kaiser <martin@kaiser.cx> wrote:

> Fabio, could you post the output of
>
> cat /sys/kernel/debug/suspend_stats
>
> after supend failed, to confirm that we're failing below
> device_suspend_noirq()?

Here it goes:

# cat /sys/kernel/debug/suspend_stats
success: 0
fail: 1
failed_freeze: 0
failed_prepare: 0
failed_suspend: 0
failed_suspend_late: 0
failed_suspend_noirq: 1
failed_resume: 0
failed_resume_early: 0
failed_resume_noirq: 0
failures:
  last_failed_dev:

  last_failed_errno:    -16
                        0
  last_failed_step:     suspend_noirq

^ permalink raw reply

* Re: [RFC v2 8/9] Bluetooth: drop HCI_UART_INIT_PENDING support
From: Martin Blumenstingl @ 2018-01-02 21:06 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Johan Hedberg, Rob Herring, devicetree,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Gustavo F. Padovan, Greg Kroah-Hartman, Jiri Slaby,
	Johan Hovold, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry Finger, Carlo Caione, Daniel Drake
In-Reply-To: <5F8922BB-5A97-43B1-88D5-591EB76FF787-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

Hi Marcel,

thank you for looking into this latest version!

On Tue, Jan 2, 2018 at 12:04 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Martin,
>
>> The three-wire (H5) protocol is the only protocol which uses
>> HCI_UART_INIT_PENDING.
>> Unfortunately the benefits of using this flag are currently unknown. It
>> was added in commit 9f2aee848fe6 ("Bluetooth: Add delayed init sequence
>> support for UART controllers"). In my experiments (with the
>> "rtk_hciattach" tool - a customized version of hciattach for Realtek
>> chipsets) I started the tool before and after this patch while the
>> Bluetooth chipset was disabled (by pulling it's enable GPIO LOW). In
>> both cases hci0 was not created - thus HCI_UART_INIT_PENDING is not
>> required in that case.
>>
>> Removing this code also has another benefit: hci_serdev.c does not
>> support the delayed initialization / registration. Thus the protocol
>> implementation (hci_h5) never receives any data with this check still
>> in place. For the H5 protocol this means that the initialization never
>> completes (because the sync response never arrives). Even if the
>> initialization would succeed later on the drivers would call
>> hci_uart_init_ready() which schedules the registration which is
>> currently not implemented by hci_serdev.c.
>>
>> Removing the HCI_UART_INIT_PENDING check makes the code easier to read
>> and also fixes the initalization of devices (implemented with the serdev
>> library) which use the H5 protocol.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>
> I really like Johan to ack this one, but generally I am fine with removing unneeded code.
I would also like as many ACKs/Tested-by/Reviewed-by as possible since
this is all new code for me (so it's easy for me to make mistakes)!

> We might also want to look at hciattach to btattach code and make sure it gets removed there as well. I am not even sure anybody used hciattach with H:5 ever.
a quick glance shows that it's defined in bluez.git but never used
there (which is good in this case)

>> ---
>> drivers/bluetooth/hci_h5.c     |  3 ---
>> drivers/bluetooth/hci_ldisc.c  | 38 --------------------------------------
>> drivers/bluetooth/hci_serdev.c |  3 ---
>> drivers/bluetooth/hci_uart.h   |  4 +---
>> 4 files changed, 1 insertion(+), 47 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
>> index 6a8d0d06aba7..6cfc2f276250 100644
>> --- a/drivers/bluetooth/hci_h5.c
>> +++ b/drivers/bluetooth/hci_h5.c
>> @@ -210,8 +210,6 @@ static int h5_open(struct hci_uart *hu)
>>
>>       h5->tx_win = H5_TX_WIN_MAX;
>>
>> -     set_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags);
>> -
>>       /* Send initial sync request */
>>       h5_link_control(hu, sync, sizeof(sync));
>>       mod_timer(&h5->timer, jiffies + H5_SYNC_TIMEOUT);
>> @@ -316,7 +314,6 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
>>                       h5->tx_win = (data[2] & 0x07);
>>               BT_DBG("Three-wire init complete. tx_win %u", h5->tx_win);
>>               h5->state = H5_ACTIVE;
>> -             hci_uart_init_ready(hu);
>>               return;
>>       } else if (memcmp(data, sleep_req, 2) == 0) {
>>               BT_DBG("Peer went to sleep");
>> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
>> index c823914b3a80..5dd3e1bebfe4 100644
>> --- a/drivers/bluetooth/hci_ldisc.c
>> +++ b/drivers/bluetooth/hci_ldisc.c
>> @@ -195,39 +195,6 @@ static void hci_uart_write_work(struct work_struct *work)
>>       clear_bit(HCI_UART_SENDING, &hu->tx_state);
>> }
>>
>> -static void hci_uart_init_work(struct work_struct *work)
>> -{
>> -     struct hci_uart *hu = container_of(work, struct hci_uart, init_ready);
>> -     int err;
>> -     struct hci_dev *hdev;
>> -
>> -     if (!test_and_clear_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
>> -             return;
>> -
>> -     err = hci_register_dev(hu->hdev);
>> -     if (err < 0) {
>> -             BT_ERR("Can't register HCI device");
>> -             hdev = hu->hdev;
>> -             hu->hdev = NULL;
>> -             hci_free_dev(hdev);
>> -             clear_bit(HCI_UART_PROTO_READY, &hu->flags);
>> -             hu->proto->close(hu);
>> -             return;
>> -     }
>> -
>> -     set_bit(HCI_UART_REGISTERED, &hu->flags);
>> -}
>> -
>> -int hci_uart_init_ready(struct hci_uart *hu)
>> -{
>> -     if (!test_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
>> -             return -EALREADY;
>> -
>> -     schedule_work(&hu->init_ready);
>> -
>> -     return 0;
>> -}
>> -
>> /* ------- Interface to HCI layer ------ */
>> /* Initialize device */
>> static int hci_uart_open(struct hci_dev *hdev)
>> @@ -490,7 +457,6 @@ static int hci_uart_tty_open(struct tty_struct *tty)
>>       hu->alignment = 1;
>>       hu->padding = 0;
>>
>> -     INIT_WORK(&hu->init_ready, hci_uart_init_work);
>>       INIT_WORK(&hu->write_work, hci_uart_write_work);
>>
>>       percpu_init_rwsem(&hu->proto_lock);
>> @@ -653,9 +619,6 @@ static int hci_uart_register_dev(struct hci_uart *hu)
>>       else
>>               hdev->dev_type = HCI_PRIMARY;
>>
>> -     if (test_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
>> -             return 0;
>> -
>>       if (hci_register_dev(hdev) < 0) {
>>               BT_ERR("Can't register HCI device");
>>               hu->hdev = NULL;
>> @@ -699,7 +662,6 @@ static int hci_uart_set_flags(struct hci_uart *hu, unsigned long flags)
>>       unsigned long valid_flags = BIT(HCI_UART_RAW_DEVICE) |
>>                                   BIT(HCI_UART_RESET_ON_INIT) |
>>                                   BIT(HCI_UART_CREATE_AMP) |
>> -                                 BIT(HCI_UART_INIT_PENDING) |
>>                                   BIT(HCI_UART_EXT_CONFIG) |
>>                                   BIT(HCI_UART_VND_DETECT);
>>
>> diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
>> index e0e6461b9200..fe67eb6d4278 100644
>> --- a/drivers/bluetooth/hci_serdev.c
>> +++ b/drivers/bluetooth/hci_serdev.c
>> @@ -333,9 +333,6 @@ int hci_uart_register_device(struct hci_uart *hu,
>>       else
>>               hdev->dev_type = HCI_PRIMARY;
>>
>> -     if (test_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
>> -             return 0;
>> -
>>       if (hci_register_dev(hdev) < 0) {
>>               BT_ERR("Can't register HCI device");
>>               err = -ENODEV;
>> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
>> index 66e8c68e4607..47e755ff4092 100644
>> --- a/drivers/bluetooth/hci_uart.h
>> +++ b/drivers/bluetooth/hci_uart.h
>> @@ -53,7 +53,7 @@
>> #define HCI_UART_RAW_DEVICE   0
>> #define HCI_UART_RESET_ON_INIT        1
>> #define HCI_UART_CREATE_AMP   2
>> -#define HCI_UART_INIT_PENDING        3
>> +/* 3 is unused - was HCI_UART_INIT_PENDING */
>
> #define HCI_UART_INIT_PENDING   3       /* unused */
>
> I prefer it this way since it is easier on the eyes.
OK, I'll do it that way in the next version

>> #define HCI_UART_EXT_CONFIG   4
>> #define HCI_UART_VND_DETECT   5
>>
>> @@ -83,7 +83,6 @@ struct hci_uart {
>>       unsigned long           flags;
>>       unsigned long           hdev_flags;
>>
>> -     struct work_struct      init_ready;
>>       struct work_struct      write_work;
>>
>>       const struct hci_uart_proto *proto;
>> @@ -115,7 +114,6 @@ int hci_uart_register_device(struct hci_uart *hu, const struct hci_uart_proto *p
>> void hci_uart_unregister_device(struct hci_uart *hu);
>>
>> int hci_uart_tx_wakeup(struct hci_uart *hu);
>> -int hci_uart_init_ready(struct hci_uart *hu);
>> void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed);
>> void hci_uart_set_flow_control(struct hci_uart *hu, bool enable);
>> void hci_uart_set_speeds(struct hci_uart *hu, unsigned int init_speed,


Regards
Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 2/9] dt-bindings: net: bluetooth: add support for Realtek Bluetooth chips
From: Martin Blumenstingl @ 2018-01-02 21:10 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Rob Herring, devicetree, open list:BLUETOOTH DRIVERS,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	Gustavo F. Padovan, Johan Hedberg, Greg Kroah-Hartman, Jiri Slaby,
	Johan Hovold, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry Finger, Carlo Caione, Daniel Drake
In-Reply-To: <2A1D60F1-F86F-4A2B-A43A-F3419506DE99-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

Hi Marcel,

On Tue, Jan 2, 2018 at 12:16 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Martin,
>
>> This adds the documentation for Bluetooth functionality of the Realtek
>> RTL8723BS and RTL8723DS.
>> Both are SDIO wifi chips with an additional Bluetooth module which is
>> connected via UART to the host.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>> .../devicetree/bindings/net/realtek-bluetooth.txt  | 41 ++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
>> new file mode 100644
>> index 000000000000..1491329c4cd1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
>> @@ -0,0 +1,41 @@
>> +Realtek Bluetooth Chips
>> +-----------------------
>> +
>> +This documents the binding structure and common properties for serial
>> +attached Realtek devices.
>> +
>> +Serial attached Realtek devices shall be a child node of the host UART
>> +device the slave device is attached to. See ../serial/slave-device.txt
>> +for more information
>> +
>> +Required properties:
>> +- compatible: should contain one of the following:
>> +    * "realtek,rtl8723bs-bluetooth"
>> +    * "realtek,rtl8723ds-bluetooth"
>> +
>> +Optional properties:
>> +- realtek,config-data: Bluetooth chipset configuration data which is
>> +                     needed for communication (it typically contains
>> +                     board specific settings like the baudrate and
>> +                     whether flow-control is used).
>> +                     This is an array of u8 values.
>
> any chance we can at least include the basic format of these config blobs. And I prefer at least an ACK from Rob here.
with including the basic format you mean a description that the config
blob should start with 0x55 0xab 0x23 0x87 (which translates to:
0x8723ab55)?

I think all non-trivial dt-binding patches should be ACKed by the DT
maintainers, so waiting for Rob's ACK is perfectly fine for me

>> +- enable-gpios: GPIO specifier, used to enable/disable the BT module
>> +- reset-gpios: GPIO specifier, used to reset the BT module
>> +
>> +
>> +Example:
>> +
>> +&uart {
>> +     ...
>> +
>> +     bluetooth {
>> +             compatible = "realtek,rtl8723bs-bluetooth";
>> +             enable-gpios = <&gpio 20 GPIO_ACTIVE_HIGH>;
>> +             reset-gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
>> +             realtek,config-data = /bits/ 8 <
>> +                     0x55 0xab 0x23 0x87 0x29 0x00 0xf4 0x00 0x01 0x01 0xf6 0x00
>> +                     0x02 0x81 0x00 0xfa 0x00 0x02 0x12 0x80 0x0c 0x00 0x10 0x02
>> +                     0x80 0x92 0x04 0x50 0xc5 0xea 0x19 0xe1 0x1b 0xfd 0xaf 0x5f
>> +                     0x01 0xa4 0x0b 0xd9 0x00 0x01 0x0f 0xe4 0x00 0x01 0x08>;
>> +     };
>> +};


Regards
Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 1/9] serdev: implement parity configuration
From: Martin Blumenstingl @ 2018-01-02 21:16 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Rob Herring, devicetree, open list:BLUETOOTH DRIVERS,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	Gustavo F. Padovan, Johan Hedberg, Greg Kroah-Hartman, Jiri Slaby,
	Johan Hovold, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry Finger, Carlo Caione, Daniel Drake
In-Reply-To: <4FDCB65A-641A-4134-BAF1-4A777012FDE7-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

Hi Marcel, Hi Rob,

On Tue, Jan 2, 2018 at 12:16 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Martin,
>
>> Some Bluetooth modules (for example the ones found in Realtek RTL8723BS
>> and RTL8723DS) want to communicate with the host with even parity
>> enabled.
>> Add a new function and the corresponding internal callbacks so parity
>> can be configured. This supports enabling and disabling parity as well
>> as setting the type to odd or even.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>> drivers/tty/serdev/core.c           | 12 ++++++++++++
>> drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++
>> include/linux/serdev.h              |  5 +++++
>> 3 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> index 1bef39828ca7..d327b02980f5 100644
>> --- a/drivers/tty/serdev/core.c
>> +++ b/drivers/tty/serdev/core.c
>> @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
>> }
>> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>>
>> +void serdev_device_set_parity(struct serdev_device *serdev, bool enable,
>> +                           bool odd)
>> +{
>> +     struct serdev_controller *ctrl = serdev->ctrl;
>> +
>> +     if (!ctrl || !ctrl->ops->set_parity)
>> +             return;
>> +
>> +     ctrl->ops->set_parity(ctrl, enable, odd);
>> +}
>> +EXPORT_SYMBOL_GPL(serdev_device_set_parity);
>> +
>
> this really needs Rob’s ACK before I take the patch.
sure

I could even live with a NACK in case these two bool parameters are
considered to be ugly
in that case I would propose an enum with three values: DISABLED,
EVEN, ODD so the arguments would look like this:
void serdev_device_set_parity(struct serdev_device *serdev, enum parity)


Regards
Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 9/9] Bluetooth: hci_h5: add support for Realtek UART Bluetooth modules
From: Martin Blumenstingl @ 2018-01-02 21:27 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
	Gustavo F. Padovan, Johan Hedberg,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
	johan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ, carlo-6IF/jdPJHihWk0Htik3J/w,
	drake-6IF/jdPJHihWk0Htik3J/w
In-Reply-To: <A32FB2B7-C20D-4E65-A353-1F7B89D9C702-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

Hi Marcel,

On Tue, Jan 2, 2018 at 12:11 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Martin,
>
>> Realtek RTL8723BS and RTL8723DS are SDIO wifi chips with an embedded
>> Bluetooth controller which connects to the host via UART.
>> The H5 protocol is used for communication between host and device.
>>
>> The Realtek "rtl8723bs_bt" and "rtl8723ds_bt" userspace Bluetooth UART
>> initialization tools (rtk_hciattach) use the following sequence:
>> 1) send H5 sync pattern (already supported by hci_h5)
>> 2) get LMP version (already supported by btrtl)
>> 3) get ROM version (already supported by btrtl)
>> 4) load the firmware and config for the current chipset (already
>>   supported by btrtl)
>> 5) read UART settings from the config blob (already supported by btrtl)
>> 6) send UART settings via a vendor command to the device (which changes
>>   the baudrate of the device and enables or disables flow control
>>   depending on the config)
>> 7) change the baudrate and flow control settings on the host
>> 8) send the firmware and config blob to the device (already supported by
>>   btrtl)
>>
>> This uses the serdev library as well as the existing btrtl driver to
>> initialize the Bluetooth functionality, which consists of:
>> - identifying the device and loading the corresponding firmware and
>>  config blobs (steps #2, #3 and #4)
>> - configuring the baudrate and flow control (steps #6 and #7)
>> - uploading the firmware to the device (step #8)
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>> drivers/bluetooth/Kconfig  |   1 +
>> drivers/bluetooth/hci_h5.c | 205 +++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 206 insertions(+)
>>
>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>> index 60e1c7d6986d..3001f1200c72 100644
>> --- a/drivers/bluetooth/Kconfig
>> +++ b/drivers/bluetooth/Kconfig
>> @@ -146,6 +146,7 @@ config BT_HCIUART_LL
>> config BT_HCIUART_3WIRE
>>       bool "Three-wire UART (H5) protocol support"
>>       depends on BT_HCIUART
>> +     select BT_RTL if SERIAL_DEV_BUS
>>       help
>>         The HCI Three-wire UART Transport Layer makes it possible to
>>         user the Bluetooth HCI over a serial port interface. The HCI
>
> while this is fine initially, I think long term this is not sustainable. So we need to abstract the H:5 part and the vendor specific setup part so that you can have a hci_rtl.c that uses H:5 instead of H:4 and still is as tiny and simple as our H:4 drivers.
I had a look at hci_ath to see how hci_h4 is re-usable and I think I
get your point

do you have any specific solution already in mind or do you want to me
make a suggestion (by providing another patch)?
if you don't have any specific in mind: do you have anything you want
me to "consider" or "avoid"?

> One other think I am not really happy about here is the Kconfig entry itself. You need to know that you need 3WIRE and SERDEV to magically enable RTL UART support. Maybe it would be better to just have a CONFIG_HCIUART_RTL and duplicate the config entry just specific to RTL. It then can select RTL, but in the Makefile it will result in using the same hci_h5.c file.
if I understand you correctly this issue would go away once hci_h5
provides library functions (like hci_h4) since we would have a
dedicated CONFIG_HCIUART_RTL anyways which could simply select
BT_HCIUART_3WIRE

>> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
>> index 6cfc2f276250..a03acc3b1b52 100644
>> --- a/drivers/bluetooth/hci_h5.c
>> +++ b/drivers/bluetooth/hci_h5.c
>> @@ -28,7 +28,14 @@
>> #include <net/bluetooth/bluetooth.h>
>> #include <net/bluetooth/hci_core.h>
>>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/serdev.h>
>> +
>> #include "hci_uart.h"
>> +#include "btrtl.h"
>>
>> #define HCI_3WIRE_ACK_PKT     0
>> #define HCI_3WIRE_LINK_PKT    15
>> @@ -97,6 +104,13 @@ struct h5 {
>>       } sleep;
>> };
>>
>> +struct h5_device {
>> +     struct hci_uart hu;
>> +     struct gpio_desc *enable_gpio;
>> +     struct gpio_desc *reset_gpio;
>> +     int (*vendor_setup)(struct h5_device *h5_dev);
>> +};
>> +
>> static void h5_reset_rx(struct h5 *h5);
>>
>> static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
>> @@ -190,6 +204,7 @@ static int h5_open(struct hci_uart *hu)
>> {
>>       struct h5 *h5;
>>       const unsigned char sync[] = { 0x01, 0x7e };
>> +     int err;
>>
>>       BT_DBG("hu %p", hu);
>>
>> @@ -210,6 +225,14 @@ static int h5_open(struct hci_uart *hu)
>>
>>       h5->tx_win = H5_TX_WIN_MAX;
>>
>> +     if (hu->serdev) {
>> +             err = serdev_device_open(hu->serdev);
>> +             if (err) {
>> +                     bt_dev_err(hu->hdev, "failed to open serdev: %d", err);
>> +                     return err;
>> +             }
>> +     }
>> +
>>       /* Send initial sync request */
>>       h5_link_control(hu, sync, sizeof(sync));
>>       mod_timer(&h5->timer, jiffies + H5_SYNC_TIMEOUT);
>> @@ -217,6 +240,25 @@ static int h5_open(struct hci_uart *hu)
>>       return 0;
>> }
>>
>> +static int h5_setup(struct hci_uart *hu)
>> +{
>> +     int err;
>> +     struct h5_device *h5_dev;
>> +
>> +     if (!hu->serdev)
>> +             return 0;
>> +
>> +     h5_dev = serdev_device_get_drvdata(hu->serdev);
>> +
>> +     if (h5_dev->vendor_setup) {
>> +             err = h5_dev->vendor_setup(h5_dev);
>> +             if (err)
>> +                     return err;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> static int h5_close(struct hci_uart *hu)
>> {
>>       struct h5 *h5 = hu->priv;
>> @@ -227,6 +269,15 @@ static int h5_close(struct hci_uart *hu)
>>       skb_queue_purge(&h5->rel);
>>       skb_queue_purge(&h5->unrel);
>>
>> +     if (hu->serdev) {
>> +             struct h5_device *h5_dev;
>> +
>> +             h5_dev = serdev_device_get_drvdata(hu->serdev);
>> +             gpiod_set_value_cansleep(h5_dev->enable_gpio, 0);
>> +
>> +             serdev_device_close(hu->serdev);
>> +     }
>> +
>>       kfree(h5);
>>
>>       return 0;
>> @@ -736,10 +787,160 @@ static int h5_flush(struct hci_uart *hu)
>>       return 0;
>> }
>>
>> +#if IS_ENABLED(CONFIG_SERIAL_DEV_BUS)
>> +static int h5_setup_realtek(struct h5_device *h5_dev)
>> +{
>> +     struct hci_uart *hu = &h5_dev->hu;
>> +     int err = 0, retry = 3;
>> +     struct sk_buff *skb;
>> +     struct btrtl_device_info *btrtl_dev;
>> +     __le32 baudrate_data;
>> +     u32 device_baudrate;
>> +     unsigned int controller_baudrate;
>> +     bool flow_control;
>> +
>> +     /* devices always start with flow control disabled and even parity */
>> +     serdev_device_set_flow_control(hu->serdev, false);
>> +     serdev_device_set_parity(hu->serdev, true, false);
>> +
>> +     do {
>> +             /* disable the device and put it into reset. some devices only
>> +              * have one of these lines, so we toggle both here to support
>> +              * all combinations.
>> +              */
>> +             gpiod_set_value_cansleep(h5_dev->reset_gpio, 1);
>> +             gpiod_set_value_cansleep(h5_dev->enable_gpio, 0);
>> +
>> +             /* wait until the device is disabled and/or reset. 500ms are
>> +              * chosen by manually testing on a RTL8723BS. shorter wait
>> +              * times lead to a non-responding device.
>> +              */
>> +             msleep(500);
>> +
>> +             /* take the device out of reset and enable it. */
>> +             gpiod_set_value_cansleep(h5_dev->reset_gpio, 0);
>> +             gpiod_set_value_cansleep(h5_dev->enable_gpio, 1);
>> +
>> +             /* after that we need to wait 500ms, otherwise the device might
>> +              * not respond in all cases. this was determined by testing
>> +              * with a RTL8723BS.
>> +              */
>> +             msleep(500);
>> +
>> +             btrtl_dev = btrtl_initialize(hu->hdev);
>> +             if (!IS_ERR(btrtl_dev))
>> +                     break;
>> +
>> +             /* Toggle the enable and reset pins above and try again */
>> +     } while (retry--);
>> +
>> +     if (IS_ERR(btrtl_dev))
>> +             return PTR_ERR(btrtl_dev);
>> +
>> +     err = btrtl_get_uart_settings(hu->hdev, btrtl_dev,
>> +                                   &controller_baudrate, &device_baudrate,
>> +                                   &flow_control);
>> +     if (err)
>> +             goto out_free;
>> +
>> +     baudrate_data = cpu_to_le32(device_baudrate);
>> +     skb = __hci_cmd_sync(hu->hdev, 0xfc17, sizeof(baudrate_data),
>> +                          &baudrate_data, HCI_INIT_TIMEOUT);
>> +     if (IS_ERR(skb)) {
>> +             bt_dev_err(hu->hdev, "set baud rate command failed");
>> +             err = -PTR_ERR(skb);
>> +             goto out_free;
>> +     } else {
>> +             kfree_skb(skb);
>> +     }
>> +
>> +     serdev_device_set_baudrate(hu->serdev, controller_baudrate);
>> +     serdev_device_set_flow_control(hu->serdev, flow_control);
>> +
>> +     err = btrtl_download_firmware(hu->hdev, btrtl_dev);
>> +
>> +out_free:
>> +     btrtl_free(btrtl_dev);
>> +
>> +     return err;
>> +}
>> +
>> +static const struct hci_uart_proto h5p;
>> +
>> +static int hci_h5_probe(struct serdev_device *serdev)
>> +{
>> +     struct hci_uart *hu;
>> +     struct h5_device *h5_dev;
>> +
>> +     h5_dev = devm_kzalloc(&serdev->dev, sizeof(*h5_dev), GFP_KERNEL);
>> +     if (!h5_dev)
>> +             return -ENOMEM;
>> +
>> +     hu = &h5_dev->hu;
>> +     hu->serdev = serdev;
>> +
>> +     serdev_device_set_drvdata(serdev, h5_dev);
>> +
>> +     h5_dev->vendor_setup = of_device_get_match_data(&serdev->dev);
>> +
>> +     h5_dev->enable_gpio = devm_gpiod_get_optional(&serdev->dev, "enable",
>> +                                                    GPIOD_OUT_HIGH);
>
> Indentation mistake.
I'll take care of that in the next version - thanks for spotting!

>> +     if (IS_ERR(h5_dev->enable_gpio))
>> +             return PTR_ERR(h5_dev->enable_gpio);
>> +
>> +     h5_dev->reset_gpio = devm_gpiod_get_optional(&serdev->dev, "reset",
>> +                                                  GPIOD_OUT_LOW);
>> +     if (IS_ERR(h5_dev->reset_gpio))
>> +             return PTR_ERR(h5_dev->reset_gpio);
>> +
>> +     hci_uart_set_speeds(hu, 115200, 0);
>> +
>> +     return hci_uart_register_device(hu, &h5p);
>> +}
>> +
>> +static void hci_h5_remove(struct serdev_device *serdev)
>> +{
>> +     struct h5_device *h5_dev = serdev_device_get_drvdata(serdev);
>> +     struct hci_uart *hu = &h5_dev->hu;
>> +     struct hci_dev *hdev = hu->hdev;
>> +
>> +     cancel_work_sync(&hu->write_work);
>> +
>> +     hci_unregister_dev(hdev);
>> +     hci_free_dev(hdev);
>> +     hu->proto->close(hu);
>> +}
>> +
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id hci_h5_of_match[] = {
>> +     {
>> +             .compatible = "realtek,rtl8723bs-bluetooth",
>> +             .data = h5_setup_realtek
>> +     },
>> +     {
>> +             .compatible = "realtek,rtl8723ds-bluetooth",
>> +             .data = h5_setup_realtek
>> +     },
>> +     {},
>> +};
>> +MODULE_DEVICE_TABLE(of, hci_h5_of_match);
>> +#endif
>> +
>> +static struct serdev_device_driver hci_h5_drv = {
>> +     .driver         = {
>> +             .name   = "hci-h5",
>> +             .of_match_table = of_match_ptr(hci_h5_of_match),
>> +     },
>> +     .probe  = hci_h5_probe,
>> +     .remove = hci_h5_remove,
>> +};
>> +#endif
>> +
>> static const struct hci_uart_proto h5p = {
>>       .id             = HCI_UART_3WIRE,
>>       .name           = "Three-wire (H5)",
>>       .open           = h5_open,
>> +     .setup          = h5_setup,
>>       .close          = h5_close,
>>       .recv           = h5_recv,
>>       .enqueue        = h5_enqueue,
>> @@ -749,10 +950,14 @@ static const struct hci_uart_proto h5p = {
>>
>> int __init h5_init(void)
>> {
>> +     serdev_device_driver_register(&hci_h5_drv);
>> +
>>       return hci_uart_register_proto(&h5p);
>> }
>>
>> int __exit h5_deinit(void)
>> {
>> +     serdev_device_driver_unregister(&hci_h5_drv);
>> +
>>       return hci_uart_unregister_proto(&h5p);
>> }

Regards
Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 1/9] serdev: implement parity configuration
From: Martin Blumenstingl @ 2018-01-02 21:34 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Rob Herring, devicetree, open list:BLUETOOTH DRIVERS,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	Gustavo F. Padovan, Johan Hedberg, Greg Kroah-Hartman, Jiri Slaby,
	Johan Hovold, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry Finger, Carlo Caione, Daniel Drake,
	ulrich.hecht+renesas-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <CAFBinCCq9mYBhzoL1XBVQNGxALg8oK5GB7Md9HJf_mchEMu3-A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Tue, Jan 2, 2018 at 10:16 PM, Martin Blumenstingl
<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> Hi Marcel, Hi Rob,
>
> On Tue, Jan 2, 2018 at 12:16 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
>> Hi Martin,
>>
>>> Some Bluetooth modules (for example the ones found in Realtek RTL8723BS
>>> and RTL8723DS) want to communicate with the host with even parity
>>> enabled.
>>> Add a new function and the corresponding internal callbacks so parity
>>> can be configured. This supports enabling and disabling parity as well
>>> as setting the type to odd or even.
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>> ---
>>> drivers/tty/serdev/core.c           | 12 ++++++++++++
>>> drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++
>>> include/linux/serdev.h              |  5 +++++
>>> 3 files changed, 38 insertions(+)
>>>
>>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>>> index 1bef39828ca7..d327b02980f5 100644
>>> --- a/drivers/tty/serdev/core.c
>>> +++ b/drivers/tty/serdev/core.c
>>> @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
>>> }
>>> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>>>
>>> +void serdev_device_set_parity(struct serdev_device *serdev, bool enable,
>>> +                           bool odd)
>>> +{
>>> +     struct serdev_controller *ctrl = serdev->ctrl;
>>> +
>>> +     if (!ctrl || !ctrl->ops->set_parity)
>>> +             return;
>>> +
>>> +     ctrl->ops->set_parity(ctrl, enable, odd);
>>> +}
>>> +EXPORT_SYMBOL_GPL(serdev_device_set_parity);
>>> +
>>
>> this really needs Rob’s ACK before I take the patch.
> sure
>
> I could even live with a NACK in case these two bool parameters are
> considered to be ugly
> in that case I would propose an enum with three values: DISABLED,
> EVEN, ODD so the arguments would look like this:
> void serdev_device_set_parity(struct serdev_device *serdev, enum parity)
I just discovered: such a patch was already posted by Ulrich Hecht: [0]


[0] https://patchwork.kernel.org/patch/9903787/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 7/9] bluetooth: btrtl: load the config blob from devicetree when available
From: Martin Blumenstingl @ 2018-01-02 21:43 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Carlo Caione, Rob Herring, devicetree,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Gustavo F. Padovan, Johan Hedberg,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
	johan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ, Daniel Drake
In-Reply-To: <F38429FB-2D20-4EF7-8547-E5A5D67D1618-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

Hi Marcel, Hi Carlo,

On Tue, Jan 2, 2018 at 12:38 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Carlo,
>
>>>>>> Some Realtek bluetooth devices need a "config" blob. The btrtl driver
>>>>>> currently only allows loading this config blob via the request_firmware
>>>>>> mechanism.
>>>>>>
>>>>>> The UART Bluetooth chips use this config blob to specify the baudrate,
>>>>>> whether flow control is used and some other unknown bits. This means
>>>>>> that the config blob is board-specific - thus loading it via
>>>>>> request_firmware means that the rootfs is tied to a specific board.
>>>>>>
>>>>>> The UART Bluetooth chips are implemented through serdev. This means
>>>>>> there is also a devicetree node which describes the Bluetooth chip.
>>>>>> Thus we can also load the blob from the devicetree node to keep the
>>>>>> filesystem independent of any board configuration data. In the future
>>>>>> this could be extended to support ACPI as well (in case that's needed).
>>>>>>
>>>>>> Parse the devicetree node if it exists and obtain the config blob from
>>>>>> there. Otherwise fall back to using the "old" request_firmware
>>>>>> mechanism.
>>>>>
>>>>> where are these config blobs coming from? I think we also need to give people a helping hand on how to add them to DT. I still wonder if the only pieces we are using are the UART config, then maybe skipping the config blob and allowing for clear named values in DT might be better.
>>>>
>>>> What about x86 platforms where we do not have DT (I didn't check but I
>>>> don't think that the UART config in that case is shipped in the ACPI
>>>> tables)?
>>>
>>> if we have this hardware in x86 systems, then I would really like to see ACPI table dumps. Some pieces might need hardcoding based on ACPI ID.
>>
>> Yes, we have, especially on cherry-trail SoCs. In [0] the DSDT of a
>> cherry-trail laptop shipping the rtl8723bs (device OBDA8723).
>>
>> [0] https://gist.github.com/carlocaione/82bff95ababb67dd33f52a86e94ce3ff
>
> so the BTHx entries normally come at least with the UART configuration. It would be useful check if it is actually correct. And then I think similar handling like what is done in hci_bcm.c and hci_intel.c needs to happen here.
what I can see is (which also matches the settings I use on my Amlogic
based boards):
- initial speed = 115200
- 8 data bits
- 1 stop bit
- even parity
- HW flow control enabled

> I think that we extended serdev to ACPI as well. Don’t recall of the top of my head if these patches were merged or not. But if they are then it is as simple as a serdev DT based driver. Just add the appropriate _HID and got from there.
yes, serdev recently gained ACPI support iirc
however, can we agree on implementing this step-by-step:
- I do not have any hardware that uses a RTL8723BS or RTL8723DS and
ACPI (I have these chips in Amlogic SoC based "TV boxes" where DT is
the leading firmware, so I cannot test the ACPI part)
- my goal is to get rid of the rtk_hciattach userspace tool dependency
(mostly Allwinner and Amlogic based devices will benefit from this)
- it would be great if I could get feedback what to consider or to
avoid when implementing this so ACPI support can be added easily on
top of my patches

> However now I think that moving towards making hci_h5.c more like generic abstraction like hci_h4.c and having a hci_rtl.c be specific for Realtek chips seems a bit cleaner direction. Frankly only H:4 and H:5 plain protocols should be used by btattach. And all others should go via serdev.
I agree with you here (as already discussed in the other patch)


Regards
Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 7/9] bluetooth: btrtl: load the config blob from devicetree when available
From: Martin Blumenstingl @ 2018-01-02 21:46 UTC (permalink / raw)
  To: Carlo Caione
  Cc: Marcel Holtmann, Rob Herring, devicetree,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Gustavo F. Padovan, Johan Hedberg,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
	johan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ, Daniel Drake
In-Reply-To: <CAL9uMOH0N-4jgQncmPjK-KyU0vY9Q0ZnHYSAGgz3z3ywzc8Avw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Carlo,

On Tue, Jan 2, 2018 at 12:31 PM, Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org> wrote:
> On Tue, Jan 2, 2018 at 11:19 AM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
>> Hi Carlo,
>>
>>>>> Some Realtek bluetooth devices need a "config" blob. The btrtl driver
>>>>> currently only allows loading this config blob via the request_firmware
>>>>> mechanism.
>>>>>
>>>>> The UART Bluetooth chips use this config blob to specify the baudrate,
>>>>> whether flow control is used and some other unknown bits. This means
>>>>> that the config blob is board-specific - thus loading it via
>>>>> request_firmware means that the rootfs is tied to a specific board.
>>>>>
>>>>> The UART Bluetooth chips are implemented through serdev. This means
>>>>> there is also a devicetree node which describes the Bluetooth chip.
>>>>> Thus we can also load the blob from the devicetree node to keep the
>>>>> filesystem independent of any board configuration data. In the future
>>>>> this could be extended to support ACPI as well (in case that's needed).
>>>>>
>>>>> Parse the devicetree node if it exists and obtain the config blob from
>>>>> there. Otherwise fall back to using the "old" request_firmware
>>>>> mechanism.
>>>>
>>>> where are these config blobs coming from? I think we also need to give people a helping hand on how to add them to DT. I still wonder if the only pieces we are using are the UART config, then maybe skipping the config blob and allowing for clear named values in DT might be better.
>>>
>>> What about x86 platforms where we do not have DT (I didn't check but I
>>> don't think that the UART config in that case is shipped in the ACPI
>>> tables)?
>>
>> if we have this hardware in x86 systems, then I would really like to see ACPI table dumps. Some pieces might need hardcoding based on ACPI ID.
>
> Yes, we have, especially on cherry-trail SoCs. In [0] the DSDT of a
> cherry-trail laptop shipping the rtl8723bs (device OBDA8723).
>
> [0] https://gist.github.com/carlocaione/82bff95ababb67dd33f52a86e94ce3ff
so this shows that the UART settings (initial baudrate, HW flow
control, etc.) are part of the DSDT
however, the actual config blob is not

the description of this patch explains: "Parse the devicetree node ...
[or] ... fall back to using the "old" request_firmware mechanism."
do you have any other solution in mind?


Regards
Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 7/9] bluetooth: btrtl: load the config blob from devicetree when available
From: Carlo Caione @ 2018-01-02 23:06 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Marcel Holtmann, Rob Herring, devicetree,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Gustavo F. Padovan, Johan Hedberg,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
	johan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ, Daniel Drake
In-Reply-To: <CAFBinCBB=SmviOMRHGDH6vzjTjV-mrtQEc8nBUhDbJGs9SBpgw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Tue, Jan 2, 2018 at 9:46 PM, Martin Blumenstingl
<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> Hi Carlo,

Hi Martin,

> On Tue, Jan 2, 2018 at 12:31 PM, Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org> wrote:
>> On Tue, Jan 2, 2018 at 11:19 AM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
>>> Hi Carlo,
>>>
>>>>>> Some Realtek bluetooth devices need a "config" blob. The btrtl driver
>>>>>> currently only allows loading this config blob via the request_firmware
>>>>>> mechanism.
>>>>>>
>>>>>> The UART Bluetooth chips use this config blob to specify the baudrate,
>>>>>> whether flow control is used and some other unknown bits. This means
>>>>>> that the config blob is board-specific - thus loading it via
>>>>>> request_firmware means that the rootfs is tied to a specific board.
>>>>>>
>>>>>> The UART Bluetooth chips are implemented through serdev. This means
>>>>>> there is also a devicetree node which describes the Bluetooth chip.
>>>>>> Thus we can also load the blob from the devicetree node to keep the
>>>>>> filesystem independent of any board configuration data. In the future
>>>>>> this could be extended to support ACPI as well (in case that's needed).
>>>>>>
>>>>>> Parse the devicetree node if it exists and obtain the config blob from
>>>>>> there. Otherwise fall back to using the "old" request_firmware
>>>>>> mechanism.
>>>>>
>>>>> where are these config blobs coming from? I think we also need to give people a helping hand on how to add them to DT. I still wonder if the only pieces we are using are the UART config, then maybe skipping the config blob and allowing for clear named values in DT might be better.
>>>>
>>>> What about x86 platforms where we do not have DT (I didn't check but I
>>>> don't think that the UART config in that case is shipped in the ACPI
>>>> tables)?
>>>
>>> if we have this hardware in x86 systems, then I would really like to see ACPI table dumps. Some pieces might need hardcoding based on ACPI ID.
>>
>> Yes, we have, especially on cherry-trail SoCs. In [0] the DSDT of a
>> cherry-trail laptop shipping the rtl8723bs (device OBDA8723).
>>
>> [0] https://gist.github.com/carlocaione/82bff95ababb67dd33f52a86e94ce3ff
> so this shows that the UART settings (initial baudrate, HW flow
> control, etc.) are part of the DSDT
> however, the actual config blob is not
>
> the description of this patch explains: "Parse the devicetree node ...
> [or] ... fall back to using the "old" request_firmware mechanism."
> do you have any other solution in mind?

As Marcel suggested we can assume that the information in the DSDT is
correct so that we can get rid of the config blob also for x86
platforms (assuming that the only useful information in the config
blobs is the UART configuration).

Adding the ACPI support on top of your patches is (hopefully) trivial,
just follow what was done for hci_bcm.c, basically adding a new _HID
and reading the configuration for GPIOs and UART, all the rest should
be transparent for serdev.

I'll test your patches on the hardware I have.

Cheers,

-- 
Carlo Caione  |  +44.7384.69.16.04  |  Endless

^ permalink raw reply

* Re: [RFC v2 1/9] serdev: implement parity configuration
From: Johan Hovold @ 2018-01-03  9:06 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Marcel Holtmann, Rob Herring, devicetree,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Gustavo F. Padovan, Johan Hedberg,
	Greg Kroah-Hartman, Jiri Slaby, Johan Hovold,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Larry Finger,
	Carlo Caione, Daniel Drake,
	ulrich.hecht+renesas-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <CAFBinCD=iuE+ho22kihCFG=AJV5t+JtjRww8PLuq-cAJ2RkAHQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Tue, Jan 02, 2018 at 10:34:04PM +0100, Martin Blumenstingl wrote:
> On Tue, Jan 2, 2018 at 10:16 PM, Martin Blumenstingl
> <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > Hi Marcel, Hi Rob,
> >
> > On Tue, Jan 2, 2018 at 12:16 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> >> Hi Martin,
> >>
> >>> Some Bluetooth modules (for example the ones found in Realtek RTL8723BS
> >>> and RTL8723DS) want to communicate with the host with even parity
> >>> enabled.
> >>> Add a new function and the corresponding internal callbacks so parity
> >>> can be configured. This supports enabling and disabling parity as well
> >>> as setting the type to odd or even.
> >>>
> >>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> >>> ---
> >>> drivers/tty/serdev/core.c           | 12 ++++++++++++
> >>> drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++
> >>> include/linux/serdev.h              |  5 +++++
> >>> 3 files changed, 38 insertions(+)
> >>>
> >>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> >>> index 1bef39828ca7..d327b02980f5 100644
> >>> --- a/drivers/tty/serdev/core.c
> >>> +++ b/drivers/tty/serdev/core.c
> >>> @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
> >>> }
> >>> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
> >>>
> >>> +void serdev_device_set_parity(struct serdev_device *serdev, bool enable,
> >>> +                           bool odd)
> >>> +{
> >>> +     struct serdev_controller *ctrl = serdev->ctrl;
> >>> +
> >>> +     if (!ctrl || !ctrl->ops->set_parity)
> >>> +             return;
> >>> +
> >>> +     ctrl->ops->set_parity(ctrl, enable, odd);
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(serdev_device_set_parity);
> >>> +
> >>
> >> this really needs Rob’s ACK before I take the patch.
> > sure
> >
> > I could even live with a NACK in case these two bool parameters are
> > considered to be ugly
> > in that case I would propose an enum with three values: DISABLED,
> > EVEN, ODD so the arguments would look like this:
> > void serdev_device_set_parity(struct serdev_device *serdev, enum parity)
> I just discovered: such a patch was already posted by Ulrich Hecht: [0]
> 
> 
> [0] https://patchwork.kernel.org/patch/9903787/

Yeah, that would be the preferred way of doing this as it's more
readable and less error prone (and more easily extended if anyone ever
needs mark and space parity).

Also, I know serdev currently fails to check for errors from
tty_set_termios, but we really need to start doing that. Not all serial
drivers support (every) parity mode so you need to check the tty termios
for the actual mode used after tty_set_termios() returns.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 1/9] serdev: implement parity configuration
From: Marcel Holtmann @ 2018-01-03 12:37 UTC (permalink / raw)
  To: Martin Blumenstingl, Rob Herring
  Cc: devicetree, open list:BLUETOOTH DRIVERS,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	Gustavo F. Padovan, Johan Hedberg, Greg Kroah-Hartman, Jiri Slaby,
	Johan Hovold, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry Finger, Carlo Caione, Daniel Drake,
	ulrich.hecht+renesas-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <CAFBinCD=iuE+ho22kihCFG=AJV5t+JtjRww8PLuq-cAJ2RkAHQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Martin,

>>>> Some Bluetooth modules (for example the ones found in Realtek RTL8723BS
>>>> and RTL8723DS) want to communicate with the host with even parity
>>>> enabled.
>>>> Add a new function and the corresponding internal callbacks so parity
>>>> can be configured. This supports enabling and disabling parity as well
>>>> as setting the type to odd or even.
>>>> 
>>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>>> ---
>>>> drivers/tty/serdev/core.c           | 12 ++++++++++++
>>>> drivers/tty/serdev/serdev-ttyport.c | 21 +++++++++++++++++++++
>>>> include/linux/serdev.h              |  5 +++++
>>>> 3 files changed, 38 insertions(+)
>>>> 
>>>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>>>> index 1bef39828ca7..d327b02980f5 100644
>>>> --- a/drivers/tty/serdev/core.c
>>>> +++ b/drivers/tty/serdev/core.c
>>>> @@ -225,6 +225,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
>>>> }
>>>> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>>>> 
>>>> +void serdev_device_set_parity(struct serdev_device *serdev, bool enable,
>>>> +                           bool odd)
>>>> +{
>>>> +     struct serdev_controller *ctrl = serdev->ctrl;
>>>> +
>>>> +     if (!ctrl || !ctrl->ops->set_parity)
>>>> +             return;
>>>> +
>>>> +     ctrl->ops->set_parity(ctrl, enable, odd);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(serdev_device_set_parity);
>>>> +
>>> 
>>> this really needs Rob’s ACK before I take the patch.
>> sure
>> 
>> I could even live with a NACK in case these two bool parameters are
>> considered to be ugly
>> in that case I would propose an enum with three values: DISABLED,
>> EVEN, ODD so the arguments would look like this:
>> void serdev_device_set_parity(struct serdev_device *serdev, enum parity)
> I just discovered: such a patch was already posted by Ulrich Hecht: [0]
> 
> 
> [0] https://patchwork.kernel.org/patch/9903787/

any idea what the status of this one is? It would be good if we get an ACK from Rob and you just include it in your patch series. I do not see it currently in Linus’ tree or net-next. If it goes via a different path to Linus, we will have a bit of a problem getting this all merged for the next kernel.

Regards

Marcel

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

^ permalink raw reply

* Re: [PATCH v5 02/39] openrisc: add ioremap_nocache declaration before include asm-generic/io.h and sync ioremap prototype with it.
From: Stafford Horne @ 2018-01-03 14:38 UTC (permalink / raw)
  To: Greentime Hu
  Cc: greentime, linux-kernel, arnd, linux-arch, tglx, jason,
	marc.zyngier, robh+dt, netdev, deanbo422, devicetree, viro,
	dhowells, will.deacon, daniel.lezcano, linux-serial,
	geert.uytterhoeven, linus.walleij, mark.rutland, greg, ren_guo,
	rdunlap, davem, jonas, stefan.kristiansson
In-Reply-To: <3e5ba33674a883b56e20b35ea9ae34990ea838c8.1514874857.git.green.hu@gmail.com>

Hello,

On Tue, Jan 02, 2018 at 04:24:34PM +0800, Greentime Hu wrote:
> From: Greentime Hu <greentime@andestech.com>
> 
> It will be built failed if commit id: d25ea659 is selected. This patch can fix this
> build error.

Ideally you would mention the commit description since the id is not yet
usptream.  I found it here (its 1 in this series):

  https://github.com/andestech/linux/commit/d25ea659
  asm-generic/io.h: move ioremap_nocache/ioremap_uc/ioremap_wc/ioremap_...

> Signed-off-by: Greentime Hu <greentime@andestech.com>
> ---
>  arch/openrisc/include/asm/io.h |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/openrisc/include/asm/io.h b/arch/openrisc/include/asm/io.h
> index 7c69139..6709b28 100644
> --- a/arch/openrisc/include/asm/io.h
> +++ b/arch/openrisc/include/asm/io.h
> @@ -29,13 +29,14 @@
>  #define PIO_OFFSET		0
>  #define PIO_MASK		0
>  
> +#define ioremap_nocache ioremap_nocache
>  #include <asm-generic/io.h>

Ideally we could move <asm-generic/io.h> include down to the bottom of the file
and not have to do the defines like like this, it seems clumsy to me.  In
'cris', 'nios2' and other architectures I can see they have the generic include
at the bottom of the file and not need for #define's.

I tried that but I get a lot of errors.  Does your patch to asm-generic/io.h
cause build issues for those architectures as well?

-Stafford

>  #include <asm/pgtable.h>
>  
>  extern void __iomem *__ioremap(phys_addr_t offset, unsigned long size,
>  				pgprot_t prot);
>  
> -static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
> +static inline void __iomem *ioremap(phys_addr_t offset, size_t size)
>  {
>  	return __ioremap(offset, size, PAGE_KERNEL);
>  }
> -- 
> 1.7.9.5
> 

^ permalink raw reply

* Re: [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW
From: Bartlomiej Zolnierkiewicz @ 2018-01-03 15:16 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-fbdev, David Airlie, Heiko Carstens, alsa-devel, dri-devel,
	Jaroslav Kysela, Peter Ujfalusi, linux-s390, linux-omap,
	James E.J. Bottomley, linux-scsi, Takashi Iwai, Sebastian Ott,
	James Smart, Cezary Jackiewicz, linux-serial, Jiri Slaby,
	Darren Hart, Zhang Rui, Dick Kennedy, Mathias Nyman, linux-pm,
	Peter Oberparleiter, intel-gfx, Eduardo Valentin <edube>
In-Reply-To: <cd38f14f06e1f447b3159ffe8c69c0df8241bb6e.1513706701.git.joe@perches.com>

On Tuesday, December 19, 2017 10:15:07 AM Joe Perches wrote:
> Convert DEVICE_ATTR uses to DEVICE_ATTR_RW where possible.
> 
> Done with perl script:
> 
> $ git grep -w --name-only DEVICE_ATTR | \
>   xargs perl -i -e 'local $/; while (<>) { s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(\s*S_IRUGO\s*\|\s*S_IWUSR|\s*S_IWUSR\s*\|\s*S_IRUGO\s*|\s*0644\s*)\)?\s*,\s*\1_show\s*,\s*\1_store\s*\)/DEVICE_ATTR_RW(\1)/g; print;}'
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  arch/s390/kernel/topology.c          |  3 +--
>  arch/tile/kernel/sysfs.c             |  2 +-
>  drivers/gpu/drm/i915/i915_sysfs.c    |  6 ++---
>  drivers/platform/x86/compal-laptop.c | 18 +++++----------
>  drivers/s390/cio/device.c            |  2 +-
>  drivers/scsi/lpfc/lpfc_attr.c        | 43 ++++++++++++------------------------
>  drivers/thermal/thermal_sysfs.c      |  9 ++++----
>  drivers/tty/serial/sh-sci.c          |  2 +-
>  drivers/usb/host/xhci-dbgcap.c       |  2 +-
>  drivers/usb/phy/phy-tahvo.c          |  2 +-
>  drivers/video/fbdev/auo_k190x.c      |  4 ++--
>  drivers/video/fbdev/w100fb.c         |  4 ++--
>  lib/test_firmware.c                  | 14 +++++-------
>  lib/test_kmod.c                      | 14 +++++-------
>  sound/soc/omap/mcbsp.c               |  4 ++--
>  15 files changed, 49 insertions(+), 80 deletions(-)

For fbdev changes:

Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH v5 02/39] openrisc: add ioremap_nocache declaration before include asm-generic/io.h and sync ioremap prototype with it.
From: Greentime Hu @ 2018-01-03 15:23 UTC (permalink / raw)
  To: Stafford Horne
  Cc: Greentime, Linux Kernel Mailing List, Arnd Bergmann, linux-arch,
	Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, netdev,
	Vincent Chen, DTML, Al Viro, David Howells, Will Deacon,
	Daniel Lezcano, linux-serial, Geert Uytterhoeven, Linus Walleij,
	Mark Rutland, Greg KH
In-Reply-To: <20180103143803.GI32243@lianli.shorne-pla.net>

Hi, Stafford:

2018-01-03 22:38 GMT+08:00 Stafford Horne <shorne@gmail.com>:
> Hello,
>
> On Tue, Jan 02, 2018 at 04:24:34PM +0800, Greentime Hu wrote:
>> From: Greentime Hu <greentime@andestech.com>
>>
>> It will be built failed if commit id: d25ea659 is selected. This patch can fix this
>> build error.
>
> Ideally you would mention the commit description since the id is not yet
> usptream.  I found it here (its 1 in this series):
>
>   https://github.com/andestech/linux/commit/d25ea659
>   asm-generic/io.h: move ioremap_nocache/ioremap_uc/ioremap_wc/ioremap_...
>
>> Signed-off-by: Greentime Hu <greentime@andestech.com>
>> ---
>>  arch/openrisc/include/asm/io.h |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/openrisc/include/asm/io.h b/arch/openrisc/include/asm/io.h
>> index 7c69139..6709b28 100644
>> --- a/arch/openrisc/include/asm/io.h
>> +++ b/arch/openrisc/include/asm/io.h
>> @@ -29,13 +29,14 @@
>>  #define PIO_OFFSET           0
>>  #define PIO_MASK             0
>>
>> +#define ioremap_nocache ioremap_nocache
>>  #include <asm-generic/io.h>
>
> Ideally we could move <asm-generic/io.h> include down to the bottom of the file
> and not have to do the defines like like this, it seems clumsy to me.  In
> 'cris', 'nios2' and other architectures I can see they have the generic include
> at the bottom of the file and not need for #define's.
>
> I tried that but I get a lot of errors.  Does your patch to asm-generic/io.h
> cause build issues for those architectures as well?
>

I got this email from kbuild test robot. I personally tried arm64/x86
before I sent the generic asm io.h patch.
I tried openrisc/sparc before I sent these v5 patches.

BUILD REGRESSION

tree/branch: https://github.com/0day-ci/linux
Greentime-Hu/Andes-nds32-Linux-Kernel/20171220-155937
branch HEAD: 9353e22157b9b69be3a3beea3553b5a105a45516  dt-bindings:
timer: Add andestech atcpit100 timer binding doc

Regressions in current branch:

arch/cris/mm/ioremap.c:79:15: note: in expansion of macro 'ioremap_nocache'
arch/openrisc/include/asm/io.h:38:29: error: conflicting types for 'ioremap'
arch/openrisc/include/asm/io.h:44:29: note: in expansion of macro
'ioremap_nocache'
arch/sparc/include/asm/io_32.h:129:15: error: conflicting types for 'ioremap'
arch/sparc/include/asm/io_32.h:130:0: warning: "ioremap_nocache" redefined
arch/sparc/include/asm/io_32.h:131:0: warning: "ioremap_wc" redefined
arch/sparc/include/asm/io_32.h:132:0: warning: "ioremap_wt" redefined
arch/sparc/kernel/ioport.c:124:15: error: conflicting types for 'ioremap'
arch/sparc/kernel/ioport.c:131:1: note: in expansion of macro 'EXPORT_SYMBOL'
drivers/net/ethernet/faraday/ftmac100.c:205:32: sparse: restricted
__le32 degrades to integer
drivers/net/ethernet/faraday/ftmac100.c:221:23: sparse: incorrect type
in assignment (different base types)
drivers/net/ethernet/faraday/ftmac100.c:251:16: sparse: cast to
restricted __le32
drivers/net/ethernet/faraday/ftmac100.c:262:23: sparse: invalid assignment: &=
drivers/net/ethernet/faraday/ftmac100.c:274:23: sparse: incorrect type
in assignment (different base types)
drivers/net/ethernet/faraday/ftmac100.c:288:18: warning: cast from
pointer to integer of different size [-Wpointer-to-int-cast]
drivers/net/ethernet/faraday/ftmac100.c:293:9: warning: cast to
pointer from integer of different size [-Wint-to-pointer-cast]
drivers/net/ethernet/faraday/ftmac100.c:534:23: sparse: incorrect type
in assignment (different base types)
include/asm-generic/io.h:864:15: error: conflicting types for 'ioremap'
include/asm-generic/io.h:865:25: error: conflicting types for 'ioremap_nocache'
include/asm-generic/io.h:866:29: note: in expansion of macro 'ioremap_nocache'

Error ids grouped by kconfigs:

recent_errors
├── cris-etrax-100lx_v2_defconfig
│   └── arch-cris-mm-ioremap.c:note:in-expansion-of-macro-ioremap_nocache
├── openrisc-or1ksim_defconfig
│   ├── arch-openrisc-include-asm-io.h:error:conflicting-types-for-ioremap
│   └── arch-openrisc-include-asm-io.h:note:in-expansion-of-macro-ioremap_nocache
├── sparc64-allyesconfig
│   ├── drivers-net-ethernet-faraday-ftmac100.c:warning:cast-from-pointer-to-integer-of-different-size
│   └── drivers-net-ethernet-faraday-ftmac100.c:warning:cast-to-pointer-from-integer-of-different-size
├── sparc-defconfig
│   ├── arch-sparc-include-asm-io_32.h:error:conflicting-types-for-ioremap
│   ├── arch-sparc-include-asm-io_32.h:warning:ioremap_nocache-redefined
│   ├── arch-sparc-include-asm-io_32.h:warning:ioremap_wc-redefined
│   ├── arch-sparc-include-asm-io_32.h:warning:ioremap_wt-redefined
│   ├── arch-sparc-kernel-ioport.c:error:conflicting-types-for-ioremap
│   └── arch-sparc-kernel-ioport.c:note:in-expansion-of-macro-EXPORT_SYMBOL
├── x86_64-allmodconfig
│   ├── drivers-net-ethernet-faraday-ftmac100.c:sparse:cast-to-restricted-__le32
│   ├── drivers-net-ethernet-faraday-ftmac100.c:sparse:incorrect-type-in-assignment-(different-base-types)-expected-unsigned-int-unsigned-rxdes0-got-restrunsigned-int-unsigned-rxdes0
│   ├── drivers-net-ethernet-faraday-ftmac100.c:sparse:incorrect-type-in-assignment-(different-base-types)-expected-unsigned-int-unsigned-rxdes2-got-restrunsigned-int-unsigned-rxdes2
│   ├── drivers-net-ethernet-faraday-ftmac100.c:sparse:incorrect-type-in-assignment-(different-base-types)-expected-unsigned-int-unsigned-txdes2-got-restrunsigned-int-unsigned-txdes2
│   ├── drivers-net-ethernet-faraday-ftmac100.c:sparse:invalid-assignment:
│   ├── drivers-net-ethernet-faraday-ftmac100.c:sparse:restricted-__le32-degrades-to-integer
│   ├── drivers-net-ethernet-faraday-ftmac100.c:warning:cast-from-pointer-to-integer-of-different-size
│   └── drivers-net-ethernet-faraday-ftmac100.c:warning:cast-to-pointer-from-integer-of-different-size
└── xtensa-allmodconfig
    ├── include-asm-generic-io.h:error:conflicting-types-for-ioremap
    ├── include-asm-generic-io.h:error:conflicting-types-for-ioremap_nocache
    └── include-asm-generic-io.h:note:in-expansion-of-macro-ioremap_nocache

elapsed time: 359m

configs tested: 128

i386                               tinyconfig
i386                   randconfig-x016-201751
i386                   randconfig-x011-201751
i386                   randconfig-x014-201751
i386                   randconfig-x017-201751
i386                   randconfig-x019-201751
i386                   randconfig-x018-201751
i386                   randconfig-x010-201751
i386                   randconfig-x013-201751
i386                   randconfig-x015-201751
i386                   randconfig-x012-201751
i386                     randconfig-n0-201751
x86_64                 randconfig-x003-201751
x86_64                 randconfig-x002-201751
x86_64                 randconfig-x006-201751
x86_64                 randconfig-x007-201751
x86_64                 randconfig-x000-201751
x86_64                 randconfig-x005-201751
x86_64                 randconfig-x004-201751
x86_64                 randconfig-x009-201751
x86_64                 randconfig-x008-201751
x86_64                 randconfig-x001-201751
ia64                              allnoconfig
ia64                                defconfig
ia64                             alldefconfig
i386                   randconfig-i0-12180843
i386                   randconfig-i1-12180843
x86_64                 randconfig-x012-201751
x86_64                 randconfig-x010-201751
x86_64                 randconfig-x011-201751
x86_64                 randconfig-x015-201751
x86_64                 randconfig-x019-201751
x86_64                 randconfig-x014-201751
x86_64                 randconfig-x013-201751
x86_64                 randconfig-x016-201751
x86_64                 randconfig-x017-201751
x86_64                 randconfig-x018-201751
i386                     randconfig-a0-201751
i386                     randconfig-a1-201751
c6x                        evmc6678_defconfig
xtensa                       common_defconfig
m32r                       m32104ut_defconfig
score                      spct6600_defconfig
xtensa                          iss_defconfig
m32r                         opsput_defconfig
m32r                           usrv_defconfig
m32r                     mappi3.smp_defconfig
nios2                         10m50_defconfig
h8300                    h8300h-sim_defconfig
cris                 etrax-100lx_v2_defconfig
blackfin                  TCM-BF537_defconfig
blackfin            BF561-EZKIT-SMP_defconfig
blackfin                BF533-EZKIT_defconfig
blackfin                BF526-EZBRD_defconfig
i386                              allnoconfig
i386                                defconfig
i386                             alldefconfig
i386                     randconfig-s1-201751
i386                     randconfig-s0-201751
mn10300                     asb2364_defconfig
openrisc                    or1ksim_defconfig
um                           x86_64_defconfig
um                             i386_defconfig
frv                                 defconfig
tile                         tilegx_defconfig
i386                             allmodconfig
microblaze                      mmu_defconfig
microblaze                    nommu_defconfig
sh                            titan_defconfig
sh                          rsk7269_defconfig
sh                  sh7785lcr_32bit_defconfig
sh                                allnoconfig
i386                   randconfig-x007-201751
i386                   randconfig-x008-201751
i386                   randconfig-x009-201751
i386                   randconfig-x004-201751
i386                   randconfig-x002-201751
i386                   randconfig-x005-201751
i386                   randconfig-x001-201751
i386                   randconfig-x006-201751
i386                   randconfig-x003-201751
i386                   randconfig-x000-201751
m68k                           sun3_defconfig
m68k                          multi_defconfig
m68k                       m5475evb_defconfig
mips                                   jz4740
mips                      malta_kvm_defconfig
mips                         64r6el_defconfig
mips                           32r2_defconfig
mips                              allnoconfig
mips                      fuloong2e_defconfig
mips                                     txx9
sparc                               defconfig
sparc64                           allnoconfig
sparc64                             defconfig
x86_64                           allmodconfig
parisc                        c3000_defconfig
parisc                         b180_defconfig
parisc                              defconfig
alpha                               defconfig
parisc                            allnoconfig
s390                        default_defconfig
arm                         at91_dt_defconfig
arm                               allnoconfig
arm                           efm32_defconfig
arm64                               defconfig
arm                        multi_v5_defconfig
arm                           sunxi_defconfig
arm64                             allnoconfig
arm                          exynos_defconfig
arm                        shmobile_defconfig
arm                        multi_v7_defconfig
i386                   randconfig-x072-201751
i386                   randconfig-x078-201751
i386                   randconfig-x071-201751
i386                   randconfig-x077-201751
i386                   randconfig-x070-201751
i386                   randconfig-x074-201751
i386                   randconfig-x073-201751
i386                   randconfig-x079-201751
i386                   randconfig-x076-201751
i386                   randconfig-x075-201751
x86_64                             acpi-redef
x86_64                           allyesdebian
x86_64                                nfsroot
x86_64                                  kexec
x86_64                                   rhel
x86_64                               rhel-7.2

^ permalink raw reply

* Re: [PATCH 2/2] serial: imx: fix endless loop during suspend
From: Fabio Estevam @ 2018-01-03 16:20 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sascha Hauer, Philipp Zabel,
	Shawn Guo, linux-serial, linux-kernel, stable
In-Reply-To: <20180102161555.GA4503@botnar.kaiser.cx>

Hi Martin,

On Tue, Jan 2, 2018 at 2:15 PM, Martin Kaiser <martin@kaiser.cx> wrote:

> I tested on an imx258.

I am not able to reproduce this problem on a imx25 pdk running 4.14.11
or linux-next.

Is it 100% reproducible on your board?

Care to share its dts? Do you use multiple UART ports? Do any of them
use RTS/CTS?

Do you have any logs to share?

Thanks

^ permalink raw reply

* Re: [RFC v2 8/9] Bluetooth: drop HCI_UART_INIT_PENDING support
From: Loic Poulain @ 2018-01-03 17:14 UTC (permalink / raw)
  To: Martin Blumenstingl, Marcel Holtmann
  Cc: Johan Hedberg, Rob Herring, devicetree,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Gustavo F. Padovan, Greg Kroah-Hartman, Jiri Slaby,
	Johan Hovold, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry Finger, Carlo Caione, Daniel Drake
In-Reply-To: <CAFBinCAxUmxivp_PWkoydha84LbJi-GGv6Uhs6cKR+D_8c8QCw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Martin,

On 2 January 2018 at 22:06, Martin Blumenstingl
<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> Hi Marcel,
>
> thank you for looking into this latest version!
>
> On Tue, Jan 2, 2018 at 12:04 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
>> Hi Martin,
>>
>>> The three-wire (H5) protocol is the only protocol which uses
>>> HCI_UART_INIT_PENDING.
>>> Unfortunately the benefits of using this flag are currently unknown. It
>>> was added in commit 9f2aee848fe6 ("Bluetooth: Add delayed init sequence
>>> support for UART controllers"). In my experiments (with the
>>> "rtk_hciattach" tool - a customized version of hciattach for Realtek
>>> chipsets) I started the tool before and after this patch while the
>>> Bluetooth chipset was disabled (by pulling it's enable GPIO LOW). In
>>> both cases hci0 was not created - thus HCI_UART_INIT_PENDING is not
>>> required in that case.
>>>
>>> Removing this code also has another benefit: hci_serdev.c does not
>>> support the delayed initialization / registration. Thus the protocol
>>> implementation (hci_h5) never receives any data with this check still
>>> in place. For the H5 protocol this means that the initialization never
>>> completes (because the sync response never arrives). Even if the
>>> initialization would succeed later on the drivers would call
>>> hci_uart_init_ready() which schedules the registration which is
>>> currently not implemented by hci_serdev.c.
>>>
>>> Removing the HCI_UART_INIT_PENDING check makes the code easier to read
>>> and also fixes the initalization of devices (implemented with the serdev
>>> library) which use the H5 protocol.
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

I think the original goal is to perform H5 init peacefully.
The H5 protocol needs to be open in order to send/receive H5 link
packets during the H5 initialization/synchronization step.
During this stage, driver prevents upper stack to send any HCI packet
by delaying the HCI device registration.

Regards,
Loic

^ permalink raw reply

* [PATCH] tty: n_gsm: Allow ADM response in addition to UA for control dlci
From: Tony Lindgren @ 2018-01-03 18:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, linux-serial, Alan Cox, Jiri Prchal, Jiri Slaby,
	Marcel Partap, Michael Scott, Peter Hurley, Russ Gorby,
	Sascha Hauer, Sebastian Reichel

Some devices have the control dlci stay in ADM mode instead of the UA
mode. This can seen at least on droid 4 when trying to open the ts
27.010 mux port. Enabling n_gsm debug mode shows the control dlci
always respond with DM to SABM instead of UA:

# modprobe n_gsm debug=0xff
# ldattach -d GSM0710 /dev/ttyS0 &
gsmld_output: 00000000: f9 03 3f 01 1c f9
--> 0) C: SABM(P)
gsmld_receive: 00000000: f9 03 1f 01 36 f9
<-- 0) C: DM(P)
...
$ minicom -D /dev/gsmtty1
minicom: cannot open /dev/gsmtty1: No error information
$ strace minicom -D /dev/gsmtty1
...
open("/dev/gsmtty1", O_RDWR|O_NOCTTY|O_NONBLOCK|O_LARGEFILE) = -1 EL2HLT

Note that this is different issue from other n_gsm -EL2HLT issues such
as timeouts when the control dlci does not respond at all.

The ADM mode seems to be a quite common according to "RF Wireless World"
article "GSM Issue-UE sends SABM and gets a DM response instead of
UA response":

  This issue is most commonly observed in GSM networks where in UE sends
  SABM and expects network to send UA response but it ends up receiving
  DM response from the network. SABM stands for Set asynchronous balanced
  mode, UA stands for Unnumbered Acknowledge and DA stands for
  Disconnected Mode.

  An RLP entity can be in one of two modes:
  - Asynchronous Balanced Mode (ABM)
  - Asynchronous Disconnected Mode (ADM)

Currently Linux kernel closes the control dlci after several retries
in gsm_dlci_t1() on DM. This causes n_gsm /dev/gsmtty ports to produce
error code -EL2HLT when trying to open them as the closing of control
dlci has already set gsm->dead.

Let's fix the issue by allowing control dlci stay in ADM mode after the
retries so the /dev/gsmtty ports can be opened and used. It seems that
it might take several attempts to get any response from the control
dlci, so it's best to allow ADM mode only after the SABM retries are
done.

Note that for droid 4 additional patches are needed to mux the ttyS0
pins and to toggle RTS gpio_149 to wake up the mdm6600 modem are also
needed to use n_gsm. And the mdm6600 modem needs to be powered on.

Cc: linux-serial@vger.kernel.org
Cc: Alan Cox <alan@llwyncelyn.cymru>
Cc: Jiri Prchal <jiri.prchal@aksignal.cz>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Marcel Partap <mpartap@gmx.net>
Cc: Michael Scott <michael.scott@linaro.org>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Russ Gorby <russ.gorby@intel.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/tty/n_gsm.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1451,6 +1451,10 @@ static void gsm_dlci_open(struct gsm_dlci *dlci)
  *	in which case an opening port goes back to closed and a closing port
  *	is simply put into closed state (any further frames from the other
  *	end will get a DM response)
+ *
+ *	Some control dlci can stay in ADM mode with other dlci working just
+ *	fine. In that case we can just keep the control dlci open after the
+ *	DLCI_OPENING retries time out.
  */
 
 static void gsm_dlci_t1(struct timer_list *t)
@@ -1464,8 +1468,15 @@ static void gsm_dlci_t1(struct timer_list *t)
 		if (dlci->retries) {
 			gsm_command(dlci->gsm, dlci->addr, SABM|PF);
 			mod_timer(&dlci->t1, jiffies + gsm->t1 * HZ / 100);
-		} else
+		} else if (!dlci->addr && gsm->control == (DM | PF)) {
+			if (debug & 8)
+				pr_info("DLCI %d opening in ADM mode.\n",
+					dlci->addr);
+			gsm_dlci_open(dlci);
+		} else {
 			gsm_dlci_close(dlci);
+		}
+
 		break;
 	case DLCI_CLOSING:
 		dlci->retries--;
@@ -1483,8 +1494,8 @@ static void gsm_dlci_t1(struct timer_list *t)
  *	@dlci: DLCI to open
  *
  *	Commence opening a DLCI from the Linux side. We issue SABM messages
- *	to the modem which should then reply with a UA, at which point we
- *	will move into open state. Opening is done asynchronously with retry
+ *	to the modem which should then reply with a UA or ADM, at which point
+ *	we will move into open state. Opening is done asynchronously with retry
  *	running off timers and the responses.
  */
 
-- 
2.15.0

^ permalink raw reply

* Re: [RFC v2 8/9] Bluetooth: drop HCI_UART_INIT_PENDING support
From: Rob Herring @ 2018-01-03 18:38 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
	Greg Kroah-Hartman, Jiri Slaby, Johan Hovold,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ, Carlo Caione, Daniel Drake
In-Reply-To: <20180101204217.26165-9-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

On Mon, Jan 1, 2018 at 2:42 PM, Martin Blumenstingl
<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> The three-wire (H5) protocol is the only protocol which uses
> HCI_UART_INIT_PENDING.

I think I'd also found that this flag wasn't really needed.

> Unfortunately the benefits of using this flag are currently unknown. It
> was added in commit 9f2aee848fe6 ("Bluetooth: Add delayed init sequence
> support for UART controllers"). In my experiments (with the
> "rtk_hciattach" tool - a customized version of hciattach for Realtek
> chipsets) I started the tool before and after this patch while the
> Bluetooth chipset was disabled (by pulling it's enable GPIO LOW). In
> both cases hci0 was not created - thus HCI_UART_INIT_PENDING is not
> required in that case.
>
> Removing this code also has another benefit: hci_serdev.c does not
> support the delayed initialization / registration.

This statement is misleading. serdev *always* supports async init as
it forces async probe of drivers. It doesn't need to support the
private workq init mechanism. At least that was my intent.

> Thus the protocol
> implementation (hci_h5) never receives any data with this check still
> in place. For the H5 protocol this means that the initialization never
> completes (because the sync response never arrives). Even if the
> initialization would succeed later on the drivers would call
> hci_uart_init_ready() which schedules the registration which is
> currently not implemented by hci_serdev.c.
>
> Removing the HCI_UART_INIT_PENDING check makes the code easier to read
> and also fixes the initalization of devices (implemented with the serdev
> library) which use the H5 protocol.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  drivers/bluetooth/hci_h5.c     |  3 ---
>  drivers/bluetooth/hci_ldisc.c  | 38 --------------------------------------
>  drivers/bluetooth/hci_serdev.c |  3 ---
>  drivers/bluetooth/hci_uart.h   |  4 +---
>  4 files changed, 1 insertion(+), 47 deletions(-)

^ permalink raw reply

* Re: [RFC v2 2/9] dt-bindings: net: bluetooth: add support for Realtek Bluetooth chips
From: Rob Herring @ 2018-01-03 19:07 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Marcel Holtmann, devicetree, open list:BLUETOOTH DRIVERS,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	Gustavo F. Padovan, Johan Hedberg, Greg Kroah-Hartman, Jiri Slaby,
	Johan Hovold, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry Finger, Carlo Caione, Daniel Drake
In-Reply-To: <CAFBinCBNro18Ak3h1fHFpAioDzimh5KcUeOE8PjQr_CCkAs7PA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Tue, Jan 2, 2018 at 3:10 PM, Martin Blumenstingl
<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> Hi Marcel,
>
> On Tue, Jan 2, 2018 at 12:16 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
>> Hi Martin,
>>
>>> This adds the documentation for Bluetooth functionality of the Realtek
>>> RTL8723BS and RTL8723DS.
>>> Both are SDIO wifi chips with an additional Bluetooth module which is
>>> connected via UART to the host.
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>> ---
>>> .../devicetree/bindings/net/realtek-bluetooth.txt  | 41 ++++++++++++++++++++++
>>> 1 file changed, 41 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
>>> new file mode 100644
>>> index 000000000000..1491329c4cd1
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
>>> @@ -0,0 +1,41 @@
>>> +Realtek Bluetooth Chips
>>> +-----------------------
>>> +
>>> +This documents the binding structure and common properties for serial
>>> +attached Realtek devices.
>>> +
>>> +Serial attached Realtek devices shall be a child node of the host UART
>>> +device the slave device is attached to. See ../serial/slave-device.txt
>>> +for more information
>>> +
>>> +Required properties:
>>> +- compatible: should contain one of the following:
>>> +    * "realtek,rtl8723bs-bluetooth"
>>> +    * "realtek,rtl8723ds-bluetooth"
>>> +
>>> +Optional properties:
>>> +- realtek,config-data: Bluetooth chipset configuration data which is
>>> +                     needed for communication (it typically contains
>>> +                     board specific settings like the baudrate and
>>> +                     whether flow-control is used).
>>> +                     This is an array of u8 values.
>>
>> any chance we can at least include the basic format of these config blobs. And I prefer at least an ACK from Rob here.
> with including the basic format you mean a description that the config
> blob should start with 0x55 0xab 0x23 0x87 (which translates to:
> 0x8723ab55)?

That and the size if it is fixed (or fixed per compatible). Also,
would be useful to state what are the defaults if not present.

If it is all documented somewhere else, a pointer to that is fine.

> I think all non-trivial dt-binding patches should be ACKed by the DT
> maintainers, so waiting for Rob's ACK is perfectly fine for me
>
>>> +- enable-gpios: GPIO specifier, used to enable/disable the BT module
>>> +- reset-gpios: GPIO specifier, used to reset the BT module

These should state active high or low.

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

^ permalink raw reply

* Re: [PATCH v5 26/39] nds32: Device tree support
From: Rob Herring @ 2018-01-03 19:14 UTC (permalink / raw)
  To: Greentime Hu
  Cc: greentime-MUIXKm3Oiri1Z/+hSey0Gg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Arnd Bergmann, linux-arch-u79uwXL29TY76Z2rM5mHXA, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, netdev,
	deanbo422-Re5JQEeQqe8AvxtiuMwx3w,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Al Viro, David Howells, Will Deacon, Daniel Lezcano,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven,
	Linus Walleij, Mark Rutland, Greg KH,
	ren_guo-Y+KPrCd2zL4AvxtiuMwx3w, Randy
In-Reply-To: <7fe1190a9cf8e30f1b8af52dd382ba1176997786.1514874857.git.green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tue, Jan 2, 2018 at 2:24 AM, Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
>
> This patch adds support for device tree.
>
> Signed-off-by: Vincent Chen <vincentc-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
> Signed-off-by: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
> ---
>  arch/nds32/boot/dts/Makefile  |    8 +++++
>  arch/nds32/boot/dts/ae3xx.dts |   73 +++++++++++++++++++++++++++++++++++++++++
>  arch/nds32/kernel/devtree.c   |   19 +++++++++++
>  3 files changed, 100 insertions(+)
>  create mode 100644 arch/nds32/boot/dts/Makefile
>  create mode 100644 arch/nds32/boot/dts/ae3xx.dts
>  create mode 100644 arch/nds32/kernel/devtree.c
>
> diff --git a/arch/nds32/boot/dts/Makefile b/arch/nds32/boot/dts/Makefile
> new file mode 100644
> index 0000000..d31faa8
> --- /dev/null
> +++ b/arch/nds32/boot/dts/Makefile
> @@ -0,0 +1,8 @@
> +ifneq '$(CONFIG_NDS32_BUILTIN_DTB)' '""'
> +BUILTIN_DTB := $(patsubst "%",%,$(CONFIG_NDS32_BUILTIN_DTB)).dtb.o
> +else
> +BUILTIN_DTB :=
> +endif
> +obj-$(CONFIG_OF) += $(BUILTIN_DTB)
> +
> +clean-files := *.dtb *.dtb.S
> diff --git a/arch/nds32/boot/dts/ae3xx.dts b/arch/nds32/boot/dts/ae3xx.dts
> new file mode 100644
> index 0000000..6b23d60
> --- /dev/null
> +++ b/arch/nds32/boot/dts/ae3xx.dts
> @@ -0,0 +1,73 @@
> +/dts-v1/;
> +/ {
> +       compatible = "andestech,ae3xx";
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +       interrupt-parent = <&intc>;
> +
> +       chosen {
> +               stdout-path = &serial0;
> +       };
> +
> +       memory@0 {
> +               device_type = "memory";
> +               reg = <0x00000000 0x40000000>;
> +       };
> +
> +       cpus {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               cpu@0 {
> +                       device_type = "cpu";
> +                       compatible = "andestech,n13", "andestech,nds32v3";
> +                       reg = <0>;
> +                       clock-frequency = <60000000>;
> +                       next-level-cache = <&L2>;
> +               };
> +       };
> +
> +       L2: l2-cache@e0500000 {
> +               compatible = "andestech,atl2c";
> +               reg = <0xe0500000 0x1000>;
> +               cache-unified;
> +               cache-level = <2>;
> +       };
> +
> +       apb: clk@0 {

unit address without reg is not valid. Drop the "@0".

> +               #clock-cells = <0>;
> +               compatible = "fixed-clock";
> +               clock-frequency = <30000000>;
> +       };
> +
> +
> +       intc: interrupt-controller {
> +               compatible = "andestech,ativic32";
> +               #interrupt-cells = <1>;
> +               interrupt-controller;
> +       };
> +
> +       serial0: serial@f0300000 {

All the memory mapped peripherals should be under at least one simple-bus node.

> +               compatible = "andestech,uart16550", "ns16550a";
> +               reg = <0xf0300000 0x1000>;
> +               interrupts = <8>;
> +               clock-frequency = <14745600>;
> +               reg-shift = <2>;
> +               reg-offset = <32>;
> +               no-loopback-test = <1>;
> +       };
> +
> +       timer0: timer@f0400000 {
> +               compatible = "andestech,atcpit100";
> +               reg = <0xf0400000 0x1000>;
> +               interrupts = <2>;
> +               clocks = <&apb>;
> +               clock-names = "PCLK";
> +       };
> +
> +       mac0: mac@e0100000 {

ethernet@...

> +               compatible = "andestech,atmac100";
> +               reg = <0xe0100000 0x1000>;
> +               interrupts = <18>;
> +       };
> +
> +};
> diff --git a/arch/nds32/kernel/devtree.c b/arch/nds32/kernel/devtree.c
> new file mode 100644
> index 0000000..bdce0fe
> --- /dev/null
> +++ b/arch/nds32/kernel/devtree.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2005-2017 Andes Technology Corporation
> +
> +#include <linux/bug.h>
> +#include <linux/printk.h>
> +#include <linux/of_fdt.h>
> +
> +void __init early_init_devtree(void *params)
> +{
> +       if (!params || !early_init_dt_scan(params)) {
> +               pr_crit("\n"
> +                       "Error: invalid device tree blob at (virtual address 0x%p)\n"
> +                       "\nPlease check your bootloader.", params);
> +
> +               BUG_ON(1);
> +       }
> +
> +       dump_stack_set_arch_desc("%s (DT)", of_flat_dt_get_machine_name());
> +}
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 8/9] Bluetooth: drop HCI_UART_INIT_PENDING support
From: Martin Blumenstingl @ 2018-01-03 20:30 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Marcel Holtmann, Johan Hedberg, Rob Herring, devicetree,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Gustavo F. Padovan, Greg Kroah-Hartman, Jiri Slaby,
	Johan Hovold, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry Finger, Carlo Caione, Daniel Drake
In-Reply-To: <CAMZdPi9KkPZvuvrNgfGmRksJnR0cFti8P5XC-HTFeUfkFbCZ_g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Loic,

On Wed, Jan 3, 2018 at 6:14 PM, Loic Poulain <loic.poulain-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Hi Martin,
>
> On 2 January 2018 at 22:06, Martin Blumenstingl
> <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
>> Hi Marcel,
>>
>> thank you for looking into this latest version!
>>
>> On Tue, Jan 2, 2018 at 12:04 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
>>> Hi Martin,
>>>
>>>> The three-wire (H5) protocol is the only protocol which uses
>>>> HCI_UART_INIT_PENDING.
>>>> Unfortunately the benefits of using this flag are currently unknown. It
>>>> was added in commit 9f2aee848fe6 ("Bluetooth: Add delayed init sequence
>>>> support for UART controllers"). In my experiments (with the
>>>> "rtk_hciattach" tool - a customized version of hciattach for Realtek
>>>> chipsets) I started the tool before and after this patch while the
>>>> Bluetooth chipset was disabled (by pulling it's enable GPIO LOW). In
>>>> both cases hci0 was not created - thus HCI_UART_INIT_PENDING is not
>>>> required in that case.
>>>>
>>>> Removing this code also has another benefit: hci_serdev.c does not
>>>> support the delayed initialization / registration. Thus the protocol
>>>> implementation (hci_h5) never receives any data with this check still
>>>> in place. For the H5 protocol this means that the initialization never
>>>> completes (because the sync response never arrives). Even if the
>>>> initialization would succeed later on the drivers would call
>>>> hci_uart_init_ready() which schedules the registration which is
>>>> currently not implemented by hci_serdev.c.
>>>>
>>>> Removing the HCI_UART_INIT_PENDING check makes the code easier to read
>>>> and also fixes the initalization of devices (implemented with the serdev
>>>> library) which use the H5 protocol.
>>>>
>>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>
> I think the original goal is to perform H5 init peacefully.
> The H5 protocol needs to be open in order to send/receive H5 link
> packets during the H5 initialization/synchronization step.
> During this stage, driver prevents upper stack to send any HCI packet
> by delaying the HCI device registration.
thank you for this explanation
do you have a test-case (manual list of steps, a scripts, etc.) which
I can use to compare the old and new behavior?

based on your explanation this would mean that we should add
HCI_UART_INIT_PENDING support to the serdev code rather than removing
it


Regards
Martin

^ permalink raw reply

* Re: [RFC v2 8/9] Bluetooth: drop HCI_UART_INIT_PENDING support
From: Martin Blumenstingl @ 2018-01-03 20:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
	Greg Kroah-Hartman, Jiri Slaby, Johan Hovold,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ, Carlo Caione, Daniel Drake
In-Reply-To: <CAL_JsqLB9aem0TXZyUvFpsfXP4p++oukPmQzGEYxkVwZt+bdvw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Rob,

On Wed, Jan 3, 2018 at 7:38 PM, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Jan 1, 2018 at 2:42 PM, Martin Blumenstingl
> <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
>> The three-wire (H5) protocol is the only protocol which uses
>> HCI_UART_INIT_PENDING.
>
> I think I'd also found that this flag wasn't really needed.
do you remember how you tested this? did you test it with you new
serdev code or with the old hci_ldisc code (with some userspace tool)?

>> Unfortunately the benefits of using this flag are currently unknown. It
>> was added in commit 9f2aee848fe6 ("Bluetooth: Add delayed init sequence
>> support for UART controllers"). In my experiments (with the
>> "rtk_hciattach" tool - a customized version of hciattach for Realtek
>> chipsets) I started the tool before and after this patch while the
>> Bluetooth chipset was disabled (by pulling it's enable GPIO LOW). In
>> both cases hci0 was not created - thus HCI_UART_INIT_PENDING is not
>> required in that case.
>>
>> Removing this code also has another benefit: hci_serdev.c does not
>> support the delayed initialization / registration.
>
> This statement is misleading. serdev *always* supports async init as
> it forces async probe of drivers. It doesn't need to support the
> private workq init mechanism. At least that was my intent.
thank you for having a look at this patch!
I will update the description in the next version

>> Thus the protocol
>> implementation (hci_h5) never receives any data with this check still
>> in place. For the H5 protocol this means that the initialization never
>> completes (because the sync response never arrives). Even if the
>> initialization would succeed later on the drivers would call
>> hci_uart_init_ready() which schedules the registration which is
>> currently not implemented by hci_serdev.c.
>>
>> Removing the HCI_UART_INIT_PENDING check makes the code easier to read
>> and also fixes the initalization of devices (implemented with the serdev
>> library) which use the H5 protocol.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>>  drivers/bluetooth/hci_h5.c     |  3 ---
>>  drivers/bluetooth/hci_ldisc.c  | 38 --------------------------------------
>>  drivers/bluetooth/hci_serdev.c |  3 ---
>>  drivers/bluetooth/hci_uart.h   |  4 +---
>>  4 files changed, 1 insertion(+), 47 deletions(-)


Regards
Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 7/9] bluetooth: btrtl: load the config blob from devicetree when available
From: Martin Blumenstingl @ 2018-01-03 20:50 UTC (permalink / raw)
  To: Carlo Caione
  Cc: Marcel Holtmann, Rob Herring, devicetree,
	open list:BLUETOOTH DRIVERS, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, Gustavo F. Padovan, Johan Hedberg,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
	johan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ, Daniel Drake
In-Reply-To: <CAL9uMOFyfr_qGf5BeTojkMeWeH8LqPoKj=LrdgEMB2wmhGBxmA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Carlo,

On Wed, Jan 3, 2018 at 12:06 AM, Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org> wrote:
> On Tue, Jan 2, 2018 at 9:46 PM, Martin Blumenstingl
> <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
>> Hi Carlo,
>
> Hi Martin,
>
>> On Tue, Jan 2, 2018 at 12:31 PM, Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org> wrote:
>>> On Tue, Jan 2, 2018 at 11:19 AM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
>>>> Hi Carlo,
>>>>
>>>>>>> Some Realtek bluetooth devices need a "config" blob. The btrtl driver
>>>>>>> currently only allows loading this config blob via the request_firmware
>>>>>>> mechanism.
>>>>>>>
>>>>>>> The UART Bluetooth chips use this config blob to specify the baudrate,
>>>>>>> whether flow control is used and some other unknown bits. This means
>>>>>>> that the config blob is board-specific - thus loading it via
>>>>>>> request_firmware means that the rootfs is tied to a specific board.
>>>>>>>
>>>>>>> The UART Bluetooth chips are implemented through serdev. This means
>>>>>>> there is also a devicetree node which describes the Bluetooth chip.
>>>>>>> Thus we can also load the blob from the devicetree node to keep the
>>>>>>> filesystem independent of any board configuration data. In the future
>>>>>>> this could be extended to support ACPI as well (in case that's needed).
>>>>>>>
>>>>>>> Parse the devicetree node if it exists and obtain the config blob from
>>>>>>> there. Otherwise fall back to using the "old" request_firmware
>>>>>>> mechanism.
>>>>>>
>>>>>> where are these config blobs coming from? I think we also need to give people a helping hand on how to add them to DT. I still wonder if the only pieces we are using are the UART config, then maybe skipping the config blob and allowing for clear named values in DT might be better.
>>>>>
>>>>> What about x86 platforms where we do not have DT (I didn't check but I
>>>>> don't think that the UART config in that case is shipped in the ACPI
>>>>> tables)?
>>>>
>>>> if we have this hardware in x86 systems, then I would really like to see ACPI table dumps. Some pieces might need hardcoding based on ACPI ID.
>>>
>>> Yes, we have, especially on cherry-trail SoCs. In [0] the DSDT of a
>>> cherry-trail laptop shipping the rtl8723bs (device OBDA8723).
>>>
>>> [0] https://gist.github.com/carlocaione/82bff95ababb67dd33f52a86e94ce3ff
>> so this shows that the UART settings (initial baudrate, HW flow
>> control, etc.) are part of the DSDT
>> however, the actual config blob is not
>>
>> the description of this patch explains: "Parse the devicetree node ...
>> [or] ... fall back to using the "old" request_firmware mechanism."
>> do you have any other solution in mind?
>
> As Marcel suggested we can assume that the information in the DSDT is
> correct so that we can get rid of the config blob also for x86
> platforms (assuming that the only useful information in the config
> blobs is the UART configuration).
in my tests I tried to send only the firmware without the config to my
RTL8723BS. unfortunately the last firmware chunk (sent to the
controller) times out in that case (even if I set a proper baudrate
before or if I specify no baudrate at all and keep the serdev at
115200)
have you tested this (= uploading the firmware without the config
blob) on your x86 board before?

so far the following solutions for the config blob were discussed:
1) put the config blob in userspace (/lib/firmware/...) -> not good
because it makes the rootfs board-specific
2) auto-generate the config blob -> didn't work for me, last firmware
chunk times out (just as if I had no config at all)
3) putting the config blob in DT -> possible but not very nice to read

I had another idea:
what if we mix solution 1) and 2)?
the idea: load the config blob from userspace (/lib/firmware/...) and
update it's settings with the values we got from devicetree-properties
/ DSDT
I have not tested this yet, but I just want to hear everyone's (at
least Marcel, Rob and Carlo) opinion on that
(this would also allow us to fully auto-generate the config blob in
the future once we figure out how to do that)

> Adding the ACPI support on top of your patches is (hopefully) trivial,
> just follow what was done for hci_bcm.c, basically adding a new _HID
> and reading the configuration for GPIOs and UART, all the rest should
> be transparent for serdev.
thanks for the reference to hci_bcm.c - I will have a look at this for
the next version
one question: "_HID" would be OBDA8723 in our case?

> I'll test your patches on the hardware I have.
great, thank you!


Regards
Martin

^ permalink raw reply

* Re: [PATCH v5 32/39] dt-bindings: nds32 L2 cache controller Bindings
From: Rob Herring @ 2018-01-03 21:10 UTC (permalink / raw)
  To: Greentime Hu
  Cc: greentime-MUIXKm3Oiri1Z/+hSey0Gg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
	linux-arch-u79uwXL29TY76Z2rM5mHXA, tglx-hfZtesqFncYOwBW4kG4KsQ,
	jason-NLaQJdtUoK4Be96aLqz0jA, marc.zyngier-5wv7dgnIgG8,
	netdev-u79uwXL29TY76Z2rM5mHXA, deanbo422-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
	dhowells-H+wXaHxf7aLQT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	geert.uytterhoeven-Re5JQEeQqe8AvxtiuMwx3w,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	greg-U8xfFu+wG4EAvxtiuMwx3w, ren_guo-Y+KPrCd2zL4AvxtiuMwx3w,
	rdunlap-wEGCiKHe2LqWVfeAwA7xHQ, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	jonas-A9uVI2HLR7kOP4wsBPIw7w,
	stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA,
	shorne-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <fc1f58a97003b4bedfbb8b6e3d29b1628ff61f9a.1514874858.git.green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tue, Jan 02, 2018 at 04:25:04PM +0800, Greentime Hu wrote:
> From: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
> 
> This patch adds nds32 L2 cache controller binding documents.
> 
> Signed-off-by: Greentime Hu <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/nds32/atl2c.txt |   29 +++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nds32/atl2c.txt
> 
> diff --git a/Documentation/devicetree/bindings/nds32/atl2c.txt b/Documentation/devicetree/bindings/nds32/atl2c.txt
> new file mode 100644
> index 0000000..db9f7ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nds32/atl2c.txt
> @@ -0,0 +1,29 @@
> +* Andestech L2 cache Controller
> +
> +The level-2 cache controller plays an important role in reducing memory latency
> +for high performance systems, such as thoese designs with AndesCore processors.
> +Level-2 cache controller in general enhances overall system performance
> +signigicantly and the system power consumption might be reduced as well by
> +reducing DRAM accesses.
> +
> +This binding specifies what properties must be available in the device tree
> +representation of an Andestech L2 cache controller.
> +
> +Required properties:
> +	- compatible:
> +		Usage: required
> +		Value type: <string>
> +		Definition: "andestech,atl2c"
> +	- reg : Physical base address and size of cache controller's memory mapped
> +	- cache-unified : Specifies the cache is a unified cache.
> +	- cache-level : Should be set to 2 for a level 2 cache.
> +
> +* Example
> +
> +	L2: l2-cache@e0500000 {

cache-controller@...

With that,

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> +		compatible = "andestech,atl2c";
> +		reg = <0xe0500000 0x1000>;
> +		cache-unified;
> +		cache-level = <2>;
> +	};
> +
> -- 
> 1.7.9.5
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] serial: imx: fix endless loop during suspend
From: Martin Kaiser @ 2018-01-03 21:43 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sascha Hauer, Philipp Zabel,
	Shawn Guo, linux-serial, linux-kernel, stable
In-Reply-To: <CAOMZO5BVQE45W6cSaPVLkDWAvTsaR54FPWD20krKCpgdGLX3CQ@mail.gmail.com>

Hi Fabio,

Thus wrote Fabio Estevam (festevam@gmail.com):

> I am not able to reproduce this problem on a imx25 pdk running 4.14.11
> or linux-next.

this is no surprise. The problem shows up only if the AWAKE bit in UART
Status Register 1 is set before we go into suspend. My understanding is
that this bit will be set when the signal on the rx pin goes from high
to low. 

> Is it 100% reproducible on your board?

On my board, I have one uart port that's connected to an external chip.
A power cycle of this external chip creates the falling edge on rx and
makes the imx uart set the AWAKE bit. The problem can then be reproduced
100%.

It can be argued that this is an obscure scenario, but nevertheless, it
should not put the kernel into an endless loop.

This is my understanding of what happens:
- AWAKE is set in the USR1 register
- there's no uart transfer running, the clocks are disabled
   (As I write this I'm not sure why this is the case, clk_ipg should
    still be enabled but it seems that it's not. If I enable it
    manually, the behaviour is different.)
- we enter suspend
- AWAKEN (UART control register 3) is set so that AWAKE creates an interrupt
- we get an interrupt immediately
   (the imx interrupt is not yet disabled at this point. it'll be
    disabled later and then reenabled if the uart port acts as a wakeup
    source)
   -> we get into the interrupt handler with the clocks disabled
- the interrupt handler tries to clear the AWAKE bit, this does not work
  since the clocks are off, we exit and AWAKE is still set
- we get another interrupt immediately
   -> endless loop

What I'm trying to do with my patch is to clear the AWAKE bit before we
set AWAKEEN. We don't want to be woken up by events that happened before
we started going into suspend.

To do this, I have to find a suitable place where the clocks are
enabled. That's why I tried to move clearing AWAKE and setting AWAKEEN
into suspend_noirq, where the clocks are already enabled to save all
registers before we finally suspend. While this works ok on my board, it
cuases problems on imx6q.

I'm not sure what makes my patch fail for you. I could imagine it is
because now, the imx interrupt is enabled (as a wakeup source) before
AWAKEN is set. The current code sets AWAKEN first and then enables the
interrupt for the wakeup source.

I'll try a different approach that keeps this order.

> Care to share its dts? Do you use multiple UART ports? Do any of them
> use RTS/CTS?

There's nothing special regarding the uarts, There's a couple of them,
none of which are using rts/cts.

It all depends on the awake bit.

Best regards,

   Martin

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox