[RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

Hi,
this patch came after investigating why the rfkill-gpio driver was failing on
the latest kernel on a machine I have. Sending this out as RFC because I'm
still not sure if this is the right way to approach this problem. I was
honestly expecting not to see the platform devices failing as consequences of
the latest work on SerDev.

Carlo Caione (2):
  net: rfkill: gpio: Fix NULL pointer deference
  net: rfkill: gpio: Convert driver to SerDev

 net/rfkill/Kconfig       |  1 +
 net/rfkill/rfkill-gpio.c | 48 +++++++++++++++++++++++++-----------------------
 2 files changed, 26 insertions(+), 23 deletions(-)

-- 
2.14.1

[RFC PATCH 1/2] net: rfkill: gpio: Fix NULL pointer deference

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

In the rfkill_gpio code the variable type_name is not initialized and it
can point to a spurious memory location. When
device_property_read_string is not able to locate the string we are
passing to rfkill_find_type this random pointer, causing a NULL pointer
dereference when using strcmp.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 net/rfkill/rfkill-gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 41bd496531d4..659d2edae972 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -88,7 +88,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 {
 	struct rfkill_gpio_data *rfkill;
 	struct gpio_desc *gpio;
-	const char *type_name;
+	const char *type_name = NULL;
 	int ret;
 
 	rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
-- 
2.14.1

[RFC PATCH 2/2] net: rfkill: gpio: Convert driver to SerDev

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

With commit e361d1f85855 ("ACPI / scan: Fix enumeration for special UART
devices") UART devices are now expected to be enumerated by
SerDev subsystem. This is breaking the enumeration of platform devices
connected to the UART without a proper SerDev support, like in the
rfkill-gpio case.

Fix this moving the driver from a platform driver to a serdev device
driver.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 net/rfkill/Kconfig       |  1 +
 net/rfkill/rfkill-gpio.c | 46 ++++++++++++++++++++++++----------------------
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/net/rfkill/Kconfig b/net/rfkill/Kconfig
index 060600b03fad..14ee289328d7 100644
--- a/net/rfkill/Kconfig
+++ b/net/rfkill/Kconfig
@@ -27,6 +27,7 @@ config RFKILL_GPIO
 	tristate "GPIO RFKILL driver"
 	depends on RFKILL
 	depends on GPIOLIB || COMPILE_TEST
+	depends on SERIAL_DEV_CTRL_TTYPORT
 	default n
 	help
 	  If you say yes here you get support of a generic gpio RFKILL
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 659d2edae972..d7553f3c3ae7 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -23,11 +23,13 @@
 #include <linux/rfkill.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
+#include <linux/serdev.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
 #include <linux/gpio/consumer.h>
 
 struct rfkill_gpio_data {
+	struct device		*dev;
 	const char		*name;
 	enum rfkill_type	type;
 	struct gpio_desc	*reset_gpio;
@@ -84,40 +86,42 @@ static int rfkill_gpio_acpi_probe(struct device *dev,
 	return devm_acpi_dev_add_driver_gpios(dev, acpi_rfkill_default_gpios);
 }
 
-static int rfkill_gpio_probe(struct platform_device *pdev)
+static int rfkill_gpio_serdev_probe(struct serdev_device *serdev)
 {
 	struct rfkill_gpio_data *rfkill;
 	struct gpio_desc *gpio;
 	const char *type_name = NULL;
 	int ret;
 
-	rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
+	rfkill = devm_kzalloc(&serdev->dev, sizeof(*rfkill), GFP_KERNEL);
 	if (!rfkill)
 		return -ENOMEM;
 
-	device_property_read_string(&pdev->dev, "name", &rfkill->name);
-	device_property_read_string(&pdev->dev, "type", &type_name);
+	rfkill->dev = &serdev->dev;
+
+	device_property_read_string(rfkill->dev, "name", &rfkill->name);
+	device_property_read_string(rfkill->dev, "type", &type_name);
 
 	if (!rfkill->name)
-		rfkill->name = dev_name(&pdev->dev);
+		rfkill->name = dev_name(rfkill->dev);
 
 	rfkill->type = rfkill_find_type(type_name);
 
-	if (ACPI_HANDLE(&pdev->dev)) {
-		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
+	if (ACPI_HANDLE(rfkill->dev)) {
+		ret = rfkill_gpio_acpi_probe(rfkill->dev, rfkill);
 		if (ret)
 			return ret;
 	}
 
-	rfkill->clk = devm_clk_get(&pdev->dev, NULL);
+	rfkill->clk = devm_clk_get(rfkill->dev, NULL);
 
-	gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
+	gpio = devm_gpiod_get_optional(rfkill->dev, "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(gpio))
 		return PTR_ERR(gpio);
 
 	rfkill->reset_gpio = gpio;
 
-	gpio = devm_gpiod_get_optional(&pdev->dev, "shutdown", GPIOD_OUT_LOW);
+	gpio = devm_gpiod_get_optional(rfkill->dev, "shutdown", GPIOD_OUT_LOW);
 	if (IS_ERR(gpio))
 		return PTR_ERR(gpio);
 
@@ -125,11 +129,11 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 
 	/* Make sure at-least one GPIO is defined for this instance */
 	if (!rfkill->reset_gpio && !rfkill->shutdown_gpio) {
-		dev_err(&pdev->dev, "invalid platform data\n");
+		dev_err(rfkill->dev, "invalid platform data\n");
 		return -EINVAL;
 	}
 
-	rfkill->rfkill_dev = rfkill_alloc(rfkill->name, &pdev->dev,
+	rfkill->rfkill_dev = rfkill_alloc(rfkill->name, rfkill->dev,
 					  rfkill->type, &rfkill_gpio_ops,
 					  rfkill);
 	if (!rfkill->rfkill_dev)
@@ -139,21 +143,19 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	platform_set_drvdata(pdev, rfkill);
+	serdev_device_set_drvdata(serdev, rfkill);
 
-	dev_info(&pdev->dev, "%s device registered.\n", rfkill->name);
+	dev_info(rfkill->dev, "%s device registered.\n", rfkill->name);
 
 	return 0;
 }
 
-static int rfkill_gpio_remove(struct platform_device *pdev)
+static void rfkill_gpio_serdev_remove(struct serdev_device *serdev)
 {
-	struct rfkill_gpio_data *rfkill = platform_get_drvdata(pdev);
+	struct rfkill_gpio_data *rfkill = serdev_device_get_drvdata(serdev);
 
 	rfkill_unregister(rfkill->rfkill_dev);
 	rfkill_destroy(rfkill->rfkill_dev);
-
-	return 0;
 }
 
 #ifdef CONFIG_ACPI
@@ -165,16 +167,16 @@ static const struct acpi_device_id rfkill_acpi_match[] = {
 MODULE_DEVICE_TABLE(acpi, rfkill_acpi_match);
 #endif
 
-static struct platform_driver rfkill_gpio_driver = {
-	.probe = rfkill_gpio_probe,
-	.remove = rfkill_gpio_remove,
+static struct serdev_device_driver rfkill_gpio_serdev_driver = {
+	.probe = rfkill_gpio_serdev_probe,
+	.remove = rfkill_gpio_serdev_remove,
 	.driver = {
 		.name = "rfkill_gpio",
 		.acpi_match_table = ACPI_PTR(rfkill_acpi_match),
 	},
 };
 
-module_platform_driver(rfkill_gpio_driver);
+module_serdev_device_driver(rfkill_gpio_serdev_driver);
 
 MODULE_DESCRIPTION("gpio rfkill");
 MODULE_AUTHOR("NVIDIA");
-- 
2.14.1

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Hans de Goede]

Hi Carlo,

Is this for devices with a RTL8723BS chip? If so then they
still will not work after this since there also no longer is
a /dev/ttyS4 created for the UART for the bluetooth, instead
you probably want:

https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41

Which also puts the /dev/ttyS4 back in place.

Regards,

Hans

p.s.

My college Jeremy Cline in the Cc is looking into getting proper
bluetooth support in place for the rtl8723bs using serdev binding
and having everything in the kernel, as we now already do for bcm
uart bluetooth modules.

On 20-02-18 14:46, Carlo Caione wrote:
> From: Carlo Caione <carlo@endlessm.com>
> 
> Hi,
> this patch came after investigating why the rfkill-gpio driver was failing on
> the latest kernel on a machine I have. Sending this out as RFC because I'm
> still not sure if this is the right way to approach this problem. I was
> honestly expecting not to see the platform devices failing as consequences of
> the latest work on SerDev.
> 
> Carlo Caione (2):
>    net: rfkill: gpio: Fix NULL pointer deference
>    net: rfkill: gpio: Convert driver to SerDev
> 
>   net/rfkill/Kconfig       |  1 +
>   net/rfkill/rfkill-gpio.c | 48 +++++++++++++++++++++++++-----------------------
>   2 files changed, 26 insertions(+), 23 deletions(-)
> 

Re: [RFC PATCH 2/2] net: rfkill: gpio: Convert driver to SerDev

[Marcel Holtmann]

Hi Carlo,

> With commit e361d1f85855 ("ACPI / scan: Fix enumeration for special UART
> devices") UART devices are now expected to be enumerated by
> SerDev subsystem. This is breaking the enumeration of platform devices
> connected to the UART without a proper SerDev support, like in the
> rfkill-gpio case.
> 
> Fix this moving the driver from a platform driver to a serdev device
> driver.

I do not like keeping this a RFKILL switch for the two GPS devices and that is really all what is left for this driver.

static const struct acpi_device_id rfkill_acpi_match[] = {
        { "BCM4752", RFKILL_TYPE_GPS },
        { "LNV4752", RFKILL_TYPE_GPS },
        { },
};

Frankly what this needs is a serdev GPS driver. Even if it also exposes the data path via character device or a new TTY. Trying to hook this into RFKILL for powering on the GPS chip is not what we should do anymore. What you really want is that if the GPS daemon or application open the node for the GPS device that it gets powered on and when closed powered back down. Fiddling with RFKILL to do that is not what a RFKILL switch is for. If you want to have an additional RFKILL switch for killing power, then this has to be done the same as WiFi and Bluetooth do it.

So this patch is just a bad hack and we better have a proper serdev GPS driver.

Regards

Marcel

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Marcel Holtmann]

Hi Hans,

> Is this for devices with a RTL8723BS chip? If so then they
> still will not work after this since there also no longer is
> a /dev/ttyS4 created for the UART for the bluetooth, instead
> you probably want:
> 

it is not for that hardware, it is for some GPS devices.

static const struct acpi_device_id rfkill_acpi_match[] = {
        { "BCM4752", RFKILL_TYPE_GPS },
        { "LNV4752", RFKILL_TYPE_GPS },
        { },
};

> https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41
> 
> Which also puts the /dev/ttyS4 back in place.
> 
> Regards,
> 
> Hans
> 
> p.s.
> 
> My college Jeremy Cline in the Cc is looking into getting proper
> bluetooth support in place for the rtl8723bs using serdev binding
> and having everything in the kernel, as we now already do for bcm
> uart bluetooth modules.

Actually this should be done quickly since the Realtek hciattach / btattach hacks are not even upstream either since that is unmergable code. Even a basic hci_rtl.c driver would be better than nothing.

Regards

Marcel

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Carlo Caione]

On Tue, Feb 20, 2018 at 2:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi Carlo,

Hi Hans,

> Is this for devices with a RTL8723BS chip? If so then they
> still will not work after this since there also no longer is
> a /dev/ttyS4 created for the UART for the bluetooth, instead
> you probably want:
>
> https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41
>
> Which also puts the /dev/ttyS4 back in place.

Yeah, this problem came up while working on the RTL8723BS chip but the
driver is also broken for the other two GPS devices supported by this
driver.
Thank you for the patch BTW.

> Regards,
>
> Hans
>
> p.s.
>
> My college Jeremy Cline in the Cc is looking into getting proper
> bluetooth support in place for the rtl8723bs using serdev binding
> and having everything in the kernel, as we now already do for bcm
> uart bluetooth modules.

Wasn't also Martin (+CC) working on this? See
https://www.spinics.net/lists/linux-bluetooth/msg73594.html

Cheers,

-- 
Carlo Caione  |  +44.7384.69.16.04  |  Endless

Re: [RFC PATCH 2/2] net: rfkill: gpio: Convert driver to SerDev

[Carlo Caione]

On Tue, Feb 20, 2018 at 2:04 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Carlo,

Hi Marcel,

>> With commit e361d1f85855 ("ACPI / scan: Fix enumeration for special UART
>> devices") UART devices are now expected to be enumerated by
>> SerDev subsystem. This is breaking the enumeration of platform devices
>> connected to the UART without a proper SerDev support, like in the
>> rfkill-gpio case.
>>
>> Fix this moving the driver from a platform driver to a serdev device
>> driver.
>
> I do not like keeping this a RFKILL switch for the two GPS devices and th=
at is really all what is left for this driver.
>
> static const struct acpi_device_id rfkill_acpi_match[] =3D {
>         { "BCM4752", RFKILL_TYPE_GPS },
>         { "LNV4752", RFKILL_TYPE_GPS },
>         { },
> };
>
> Frankly what this needs is a serdev GPS driver. Even if it also exposes t=
he data path via character device or a new TTY. Trying to hook this into RF=
KILL for powering on the GPS chip is not what we should do anymore. What yo=
u really want is that if the GPS daemon or application open the node for th=
e GPS device that it gets powered on and when closed powered back down. Fid=
dling with RFKILL to do that is not what a RFKILL switch is for. If you wan=
t to have an additional RFKILL switch for killing power, then this has to b=
e done the same as WiFi and Bluetooth do it.
>
> So this patch is just a bad hack and we better have a proper serdev GPS d=
river.

Yeah, I know (that's also why I preferred to send this as RFC).
At least this patch could be useful to have things working until we
have something better in place, but I definitely agree that's a bad
hack.

Cheers,

--=20
Carlo Caione  |  +44.7384.69.16.04  |  Endless

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Marcel Holtmann]

Hi Carlo,

>> Is this for devices with a RTL8723BS chip? If so then they
>> still will not work after this since there also no longer is
>> a /dev/ttyS4 created for the UART for the bluetooth, instead
>> you probably want:
>> 
>> https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41
>> 
>> Which also puts the /dev/ttyS4 back in place.
> 
> Yeah, this problem came up while working on the RTL8723BS chip but the
> driver is also broken for the other two GPS devices supported by this
> driver.
> Thank you for the patch BTW.
> 
>> Regards,
>> 
>> Hans
>> 
>> p.s.
>> 
>> My college Jeremy Cline in the Cc is looking into getting proper
>> bluetooth support in place for the rtl8723bs using serdev binding
>> and having everything in the kernel, as we now already do for bcm
>> uart bluetooth modules.
> 
> Wasn't also Martin (+CC) working on this? See
> https://www.spinics.net/lists/linux-bluetooth/msg73594.html

I remember that. I think we need a non-RFC version against latest bluetooth-next tree.

Regards

Marcel

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Hans de Goede]

Hi,

On 02/20/2018 03:36 PM, Carlo Caione wrote:
> On Tue, Feb 20, 2018 at 2:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi Carlo,
> 
> Hi Hans,
> 
>> Is this for devices with a RTL8723BS chip? If so then they
>> still will not work after this since there also no longer is
>> a /dev/ttyS4 created for the UART for the bluetooth, instead
>> you probably want:
>>
>> https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41
>>
>> Which also puts the /dev/ttyS4 back in place.
> 
> Yeah, this problem came up while working on the RTL8723BS chip but the
> driver is also broken for the other two GPS devices supported by this
> driver.
> Thank you for the patch BTW.
> 
>> Regards,
>>
>> Hans
>>
>> p.s.
>>
>> My college Jeremy Cline in the Cc is looking into getting proper
>> bluetooth support in place for the rtl8723bs using serdev binding
>> and having everything in the kernel, as we now already do for bcm
>> uart bluetooth modules.
> 
> Wasn't also Martin (+CC) working on this? See
> https://www.spinics.net/lists/linux-bluetooth/msg73594.html

Ah I did not know that, cool.  Jeremy, this is probably a good starting
point :)   And you should probably coordinate with Martin on this.

Regards,

Hans

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Martin Blumenstingl]

Hello Jeremy, Hello Hans,

On Tue, Feb 20, 2018 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 02/20/2018 03:36 PM, Carlo Caione wrote:
>>
>> On Tue, Feb 20, 2018 at 2:01 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> Hi Carlo,
>>
>>
>> Hi Hans,
>>
>>> Is this for devices with a RTL8723BS chip? If so then they
>>> still will not work after this since there also no longer is
>>> a /dev/ttyS4 created for the UART for the bluetooth, instead
>>> you probably want:
>>>
>>>
>>> https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41
>>>
>>> Which also puts the /dev/ttyS4 back in place.
>>
>>
>> Yeah, this problem came up while working on the RTL8723BS chip but the
>> driver is also broken for the other two GPS devices supported by this
>> driver.
>> Thank you for the patch BTW.
>>
>>> Regards,
>>>
>>> Hans
>>>
>>> p.s.
>>>
>>> My college Jeremy Cline in the Cc is looking into getting proper
>>> bluetooth support in place for the rtl8723bs using serdev binding
>>> and having everything in the kernel, as we now already do for bcm
>>> uart bluetooth modules.
>>
>>
>> Wasn't also Martin (+CC) working on this? See
>> https://www.spinics.net/lists/linux-bluetooth/msg73594.html
thank you for CC'ing me Carlo!

> Ah I did not know that, cool.  Jeremy, this is probably a good starting
> point :)   And you should probably coordinate with Martin on this.
the status on this is:
- Marcel wrote a tool to parse the config blob: [0]
- the first patch from my series called "serdev: implement parity
configuration" is now obsolete because an improved version from Ulrich
Hecht has been merged: [1] (this requires a trivial change to the
"serdev_device_set_parity" call in patch #9 of my series)
- I still have Realtek serdev support on my TODO-list but with low priority

there was a discussion what has to be done to drop the "RFC" prefix
from my series: [3]
the quick summary (from my point of view):
- if possible we should get rid of the config blob (don't use it at
all or generate it in-memory - I couldn't make either of these work so
far but I've not spent much time on it either)
- create a "library" for the H5 protocol (similar to the H4 protocol)
so the Realtek code doesn't have to be part of hci_h5.c
- add ACPI support (and not just device-tree support)
- testing with existing Realtek USB devices is needed

I have successfully tested v2 of my series on two Amlogic boards I
have which both come with a RTL8723BS SDIO wifi/UART Bluetooth combo
chip.
that said, I only looked at Bluetooth support (I didn't test wifi or
btcoex support) and I don't have any "Realtek USB Bluetooth" dongles
to check that I didn't break support for the existing devices

@Jeremy: I definitely won't be able to update my patches for the v4.17
cycle (and I'm not sure how much time I can dedicate to this for the
v4.18 cycle).
it would be great if you could keep me CC'ed on your patches so I can
learn and test them on the boards I have


Regards
Martin


[0] https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/rtlfw.c?id=8b21a74f2e473b88cadc8ad871c635ace969ee02
[1] https://github.com/torvalds/linux/commit/3a19cfcce105e481b02b14265c5b40c6c10ef60a
[2] https://www.spinics.net/lists/linux-bluetooth/msg73602.html
[3] https://www.spinics.net/lists/linux-bluetooth/msg73602.html

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Jeremy Cline]

Hi Martin,

On 02/20/2018 03:26 PM, Martin Blumenstingl wrote:
> Hello Jeremy, Hello Hans,
> 
> On Tue, Feb 20, 2018 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 02/20/2018 03:36 PM, Carlo Caione wrote:
>>>
>>> On Tue, Feb 20, 2018 at 2:01 PM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> Hi Carlo,
>>>
>>>
>>> Hi Hans,
>>>
>>>> Is this for devices with a RTL8723BS chip? If so then they
>>>> still will not work after this since there also no longer is
>>>> a /dev/ttyS4 created for the UART for the bluetooth, instead
>>>> you probably want:
>>>>
>>>>
>>>> https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41
>>>>
>>>> Which also puts the /dev/ttyS4 back in place.
>>>
>>>
>>> Yeah, this problem came up while working on the RTL8723BS chip but the
>>> driver is also broken for the other two GPS devices supported by this
>>> driver.
>>> Thank you for the patch BTW.
>>>
>>>> Regards,
>>>>
>>>> Hans
>>>>
>>>> p.s.
>>>>
>>>> My college Jeremy Cline in the Cc is looking into getting proper
>>>> bluetooth support in place for the rtl8723bs using serdev binding
>>>> and having everything in the kernel, as we now already do for bcm
>>>> uart bluetooth modules.
>>>
>>>
>>> Wasn't also Martin (+CC) working on this? See
>>> https://www.spinics.net/lists/linux-bluetooth/msg73594.html
> thank you for CC'ing me Carlo!
> 
>> Ah I did not know that, cool.  Jeremy, this is probably a good starting
>> point :)   And you should probably coordinate with Martin on this.
> the status on this is:
> - Marcel wrote a tool to parse the config blob: [0]
> - the first patch from my series called "serdev: implement parity
> configuration" is now obsolete because an improved version from Ulrich
> Hecht has been merged: [1] (this requires a trivial change to the
> "serdev_device_set_parity" call in patch #9 of my series)
> - I still have Realtek serdev support on my TODO-list but with low priority
> 
> there was a discussion what has to be done to drop the "RFC" prefix
> from my series: [3]
> the quick summary (from my point of view):
> - if possible we should get rid of the config blob (don't use it at
> all or generate it in-memory - I couldn't make either of these work so
> far but I've not spent much time on it either)
> - create a "library" for the H5 protocol (similar to the H4 protocol)
> so the Realtek code doesn't have to be part of hci_h5.c
> - add ACPI support (and not just device-tree support)
> - testing with existing Realtek USB devices is needed
> 
> I have successfully tested v2 of my series on two Amlogic boards I
> have which both come with a RTL8723BS SDIO wifi/UART Bluetooth combo
> chip.
> that said, I only looked at Bluetooth support (I didn't test wifi or
> btcoex support) and I don't have any "Realtek USB Bluetooth" dongles
> to check that I didn't break support for the existing devices
> 
> @Jeremy: I definitely won't be able to update my patches for the v4.17
> cycle (and I'm not sure how much time I can dedicate to this for the
> v4.18 cycle).
> it would be great if you could keep me CC'ed on your patches so I can
> learn and test them on the boards I have

Great, thanks for all the info! I'll definitely keep you CC'ed on my
progress. I'm still very new to kernel development so I expect it'll
take me quite a while, but I do have a fair bit of time to devote to
this.


Regards,
Jeremy

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Carlo Caione]

On Tue, Feb 20, 2018 at 9:18 PM, Jeremy Cline <jcline@redhat.com> wrote:

> Great, thanks for all the info! I'll definitely keep you CC'ed on my
> progress. I'm still very new to kernel development so I expect it'll
> take me quite a while, but I do have a fair bit of time to devote to
> this.

Welcome to this crazy world ;)
Keep me on CC also, I have quite a bit of hw I can test the patches
on. Fell free also to reach me in private.
I was actually going to work on this myself but now I guess there are
too many people.

Cheers,

-- 
Carlo Caione  |  +44.7384.69.16.04  |  Endless

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Marcel Holtmann]

Hi Martin,

>>>> Is this for devices with a RTL8723BS chip? If so then they
>>>> still will not work after this since there also no longer is
>>>> a /dev/ttyS4 created for the UART for the bluetooth, instead
>>>> you probably want:
>>>> 
>>>> 
>>>> https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41
>>>> 
>>>> Which also puts the /dev/ttyS4 back in place.
>>> 
>>> 
>>> Yeah, this problem came up while working on the RTL8723BS chip but the
>>> driver is also broken for the other two GPS devices supported by this
>>> driver.
>>> Thank you for the patch BTW.
>>> 
>>>> Regards,
>>>> 
>>>> Hans
>>>> 
>>>> p.s.
>>>> 
>>>> My college Jeremy Cline in the Cc is looking into getting proper
>>>> bluetooth support in place for the rtl8723bs using serdev binding
>>>> and having everything in the kernel, as we now already do for bcm
>>>> uart bluetooth modules.
>>> 
>>> 
>>> Wasn't also Martin (+CC) working on this? See
>>> https://www.spinics.net/lists/linux-bluetooth/msg73594.html
> thank you for CC'ing me Carlo!
> 
>> Ah I did not know that, cool.  Jeremy, this is probably a good starting
>> point :)   And you should probably coordinate with Martin on this.
> the status on this is:
> - Marcel wrote a tool to parse the config blob: [0]
> - the first patch from my series called "serdev: implement parity
> configuration" is now obsolete because an improved version from Ulrich
> Hecht has been merged: [1] (this requires a trivial change to the
> "serdev_device_set_parity" call in patch #9 of my series)
> - I still have Realtek serdev support on my TODO-list but with low priority
> 
> there was a discussion what has to be done to drop the "RFC" prefix
> from my series: [3]
> the quick summary (from my point of view):
> - if possible we should get rid of the config blob (don't use it at
> all or generate it in-memory - I couldn't make either of these work so
> far but I've not spent much time on it either)

so I have no idea what the config blobs are actually doing. However I am afraid they are needed since they are changing setting that should have been in the ROM mask, but they aren’t. What would be interesting to find the vendor command to read out the memory area and match it to the config file.

> - create a "library" for the H5 protocol (similar to the H4 protocol)
> so the Realtek code doesn't have to be part of hci_h5.c

I would prefer to have that from the beginning, but I would accept incremental patches once Realtek support is merged.

> - add ACPI support (and not just device-tree support)

That should be as simple as just adding the PNPIDs to the driver.

> - testing with existing Realtek USB devices is needed
> 
> I have successfully tested v2 of my series on two Amlogic boards I
> have which both come with a RTL8723BS SDIO wifi/UART Bluetooth combo
> chip.
> that said, I only looked at Bluetooth support (I didn't test wifi or
> btcoex support) and I don't have any "Realtek USB Bluetooth" dongles
> to check that I didn't break support for the existing devices
> 
> @Jeremy: I definitely won't be able to update my patches for the v4.17
> cycle (and I'm not sure how much time I can dedicate to this for the
> v4.18 cycle).
> it would be great if you could keep me CC'ed on your patches so I can
> learn and test them on the boards I have

Regards

Marcel

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Marcel Holtmann]

Hi Carlo,

>> Great, thanks for all the info! I'll definitely keep you CC'ed on my
>> progress. I'm still very new to kernel development so I expect it'll
>> take me quite a while, but I do have a fair bit of time to devote to
>> this.
> 
> Welcome to this crazy world ;)
> Keep me on CC also, I have quite a bit of hw I can test the patches
> on. Fell free also to reach me in private.
> I was actually going to work on this myself but now I guess there are
> too many people.

I doubt there are too many people. I think there is a good baseline that needs some love to finish.

Regards

Marcel

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Jeremy Cline]

On 02/20/2018 04:34 PM, Carlo Caione wrote:
> On Tue, Feb 20, 2018 at 9:18 PM, Jeremy Cline <jcline@redhat.com> wrote:
> 
>> Great, thanks for all the info! I'll definitely keep you CC'ed on my
>> progress. I'm still very new to kernel development so I expect it'll
>> take me quite a while, but I do have a fair bit of time to devote to
>> this.
> 
> Welcome to this crazy world ;)

Thanks!

> Keep me on CC also, I have quite a bit of hw I can test the patches
> on. Fell free also to reach me in private.
> I was actually going to work on this myself but now I guess there are
> too many people.

Will do. As for too many people, that's a pretty good problem to have!
It sounds like Martin is pretty busy, though, and I would welcome your
help. I was going to start by rebasing the current RFC and then looking
at adding ACPI support since that sounds pretty straight-forward and I
have the hardware.

Figuring out the mysteries of the config blob and creating an H5
"library" sound fairly independent to me so if you're interested maybe
we could both take one on.

Regards,
Jeremy