* Re: [PATCH v2] serdev: add method to set parity
From: Greg Kroah-Hartman @ 2018-01-23 7:00 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Ulrich Hecht, Rob Herring, linux-renesas-soc, linux-serial,
Johan Hovold, Sebastian Reichel, Bluez mailing list,
Martin Blumenstingl, magnus.damm, laurent.pinchart, wsa, geert
In-Reply-To: <33F02495-73E7-47A6-B10B-FA6940F6DE63@holtmann.org>
On Mon, Jan 22, 2018 at 07:23:00PM +0100, Marcel Holtmann wrote:
> Hi Rob,
>
> > Adds serdev_device_set_parity() and an implementation for ttyport.
> > The interface uses an enum with the values SERIAL_PARITY_NONE,
> > SERIAL_PARITY_EVEN and SERIAL_PARITY_ODD.
> >
> > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > ---
> >
> > Hi!
> >
> > This revision addresses Johan's comments (see below for details) and adds
> > Sebastian's Reviewed-by tag. Thank you for your reviews!
> >
> > CU
> > Uli
> >
> >
> > Changes since v1:
> > - added Reviewed-by tag
> > - expanded commit message
> > - shuffled stuff around to keep line-setting bits together
> > - clear CMSPAR
> > - (hopefully) detect errors correctly by checking tty->termios after call
> > to tty_set_termios().
> >
> >
> > drivers/tty/serdev/core.c | 12 ++++++++++++
> > drivers/tty/serdev/serdev-ttyport.c | 24 ++++++++++++++++++++++++
> > include/linux/serdev.h | 10 ++++++++++
> > 3 files changed, 46 insertions(+)
>
> if there are no objections, I would like to just take this through bluetooth-next tree.
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [PATCH v2] serdev: add method to set parity
From: Johan Hovold @ 2018-01-23 1:34 UTC (permalink / raw)
To: Ulrich Hecht
Cc: marcel-kz+m5ild9QBg9hUCZPvPmw,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA, johan-DgEjT+Ai2ygdnm+yROfE0A,
sre-DgEjT+Ai2ygdnm+yROfE0A,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
robh-DgEjT+Ai2ygdnm+yROfE0A,
martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
wsa-z923LK4zBo2bacvFa/9K2g, geert-Td1EMuHUCqxL1ZNQvxDV9g
In-Reply-To: <1516643792-22840-1-git-send-email-ulrich.hecht+renesas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Jan 22, 2018 at 06:56:32PM +0100, Ulrich Hecht wrote:
> Adds serdev_device_set_parity() and an implementation for ttyport.
> The interface uses an enum with the values SERIAL_PARITY_NONE,
> SERIAL_PARITY_EVEN and SERIAL_PARITY_ODD.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> ---
>
> Hi!
>
> This revision addresses Johan's comments (see below for details) and adds
> Sebastian's Reviewed-by tag. Thank you for your reviews!
>
> CU
> Uli
>
>
> Changes since v1:
> - added Reviewed-by tag
> - expanded commit message
> - shuffled stuff around to keep line-setting bits together
> - clear CMSPAR
> - (hopefully) detect errors correctly by checking tty->termios after call
> to tty_set_termios().
Thanks for the v2. Looks good to me now.
Reviewed-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Johan
^ permalink raw reply
* Re: [PATCH] acpi, spcr: Make SPCR available to x86
From: Timur Tabi @ 2018-01-22 21:49 UTC (permalink / raw)
To: Prarit Bhargava, linux-kernel
Cc: linux-acpi, linux-doc, linux-arm-kernel, linux-pm, linux-serial,
Bhupesh Sharma, Lv Zheng, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, Jonathan Corbet, Catalin Marinas,
Will Deacon, graeme.gregory, mark.salter
In-Reply-To: <20180118150951.28964-1-prarit@redhat.com>
On 01/18/2018 09:09 AM, Prarit Bhargava wrote:
> if (acpi_disabled) {
> - if (earlycon_init_is_deferred)
> + if (earlycon_acpi_spcr_enable)
This patch works for me, so I can ACK it, but first you might want to
rename earlycon_acpi_spcr_enable, because these two lines don't make
much sense.
"If ACPI is disabled, and ACPI SCPR is enabled, then ...."
If ACPI is disabled, then how can a variable called
"earlycon_acpi_spcr_enable" be true?
Would it make more sense to rename it to earlycon_spcr_enable?
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Hans de Goede @ 2018-01-22 20:56 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Andy Shevchenko, Gustavo F. Padovan, Johan Hedberg,
Bluez mailing list, linux-serial, ACPI Devel Maling List,
Lukas Wunner
In-Reply-To: <059DCBBD-4966-4531-9E2D-8A369FC400A7@holtmann.org>
Hi,
On 22-01-18 20:57, Marcel Holtmann wrote:
> Hi Hans,
>
>>>>>> Andy, I see that you added support for bcm bluetooth over a tty
>>>>>> using
>>>>>> platform_data instead of ACPI enumeration. Can you change the code
>>>>>> instantiating the device to instead instantiate a serdev, so that
>>>>>> we
>>>>>> kill the platform device support in hci_bcm.c and so that users
>>>>>> don't
>>>>>> need to do a btattach, but instead the kernel will do the attach
>>>>>> itself
>>>>>> and things will just work ?
>>>>>
>>>>> I'm sorry, I can't do this soon, other more priority tasks in a
>>>>> pocket.
>>>>>
>>>>> The instantiation of the driver is happened in
>>>>> arch/x86/platform/intel-
>>>>> mid/device_libs/platform_bt.c
>>>>>
>>>>> I would help with review of any patches till I would able to look at
>>>>> it
>>>>> myself.
>>>>
>>>> If I manage to come up with patches do you have hardware and time to
>>>> test?
>>> Yes and I would find half an hour for sure.
>>
>> That is great, thank you, but ...
>>
>>>> First point of order to get this working as serdev I think is to
>>>> modify drivers/tty/serdev/core.c a and then the
>>>> serdev_controller_add()
>>>> function to somehow recognize the serial port in question, so
>>>> something akin to the of_serdev_register_devices(ctrl) /
>>>> acpi_serdev_register_devices(ctrl) functions for platform_devs,
>>>> assuming
>>>> the tty-parent-dev on the Edison SOM is a platform_dev ?
>>> tty parent is PCI device there.
>>>> Anyways it looks like this will be really hard to do without access
>>>> to the hardware.
>>> I can do a BAT.
>>
>> Right, sorry I was not clear, when I was talking about hardware
>> access I was not (not only) referring to testing.
>>
>> The problem is that to figure out how to hook all this together
>> will require poking around on the hardware, looking in sysfs, finding
>> some id we can use in a pci_serdev_register_devices() to add to
>> drivers/tty/serdev/core.c so that it only turns the serial port on
>> the Edison into a serdev and not somewhere else, etc.
>>
>> But thinking about this more, I too have higher priority items on
>> my TODO list, so as much as I would like to see this cleaned up,
>> lets shelf this for now until you have time to look into this.
>>
>> When you do find time, if you've any questions I'm happy to help.
>
> so the serial port is driven by drivers/tty/serial/8250/8250_mid.c and using PCI_DEVICE_ID_INTEL_TNG_UART 0x1191 to match the driver. So it is exposed as PCI device. While of course more than one UART port uses 0x1191. This one seems to be mapped to pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(4, 1)) actually.
>
> What I think is that we need to do is some sort of x86_match_cpu like arch/x86/platform/intel-mid/device_libs/platform_bt.c does. But then again, that is also what the Apple Broadcom support does to choose the different ACPI procedures.
>
> I have an Edison board on my desk and can quickly boot any patch you want me to test. Or grab data from sysfs or dmesg.
I'm sorry, I started looking at removing the platform code because
I already spend a lot of time looking at the hci_bcm code to make
the power-management work with serdev devices instantiated from
ACPI, when writing the patch-set for that I added removing the
remaining cruft to my todo list...
But I don't have time / don't want to make time given other priorities
to also look into the Edison stuff, which seems non-trivial to fix.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Marcel Holtmann @ 2018-01-22 19:57 UTC (permalink / raw)
To: Hans de Goede
Cc: Andy Shevchenko, Gustavo F. Padovan, Johan Hedberg,
Bluez mailing list, linux-serial-u79uwXL29TY76Z2rM5mHXA,
ACPI Devel Maling List, Lukas Wunner
In-Reply-To: <0cae9024-887a-45f7-7710-1684f9c0e54e-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hi Hans,
>>>>> Andy, I see that you added support for bcm bluetooth over a tty
>>>>> using
>>>>> platform_data instead of ACPI enumeration. Can you change the code
>>>>> instantiating the device to instead instantiate a serdev, so that
>>>>> we
>>>>> kill the platform device support in hci_bcm.c and so that users
>>>>> don't
>>>>> need to do a btattach, but instead the kernel will do the attach
>>>>> itself
>>>>> and things will just work ?
>>>>
>>>> I'm sorry, I can't do this soon, other more priority tasks in a
>>>> pocket.
>>>>
>>>> The instantiation of the driver is happened in
>>>> arch/x86/platform/intel-
>>>> mid/device_libs/platform_bt.c
>>>>
>>>> I would help with review of any patches till I would able to look at
>>>> it
>>>> myself.
>>>
>>> If I manage to come up with patches do you have hardware and time to
>>> test?
>> Yes and I would find half an hour for sure.
>
> That is great, thank you, but ...
>
>>> First point of order to get this working as serdev I think is to
>>> modify drivers/tty/serdev/core.c a and then the
>>> serdev_controller_add()
>>> function to somehow recognize the serial port in question, so
>>> something akin to the of_serdev_register_devices(ctrl) /
>>> acpi_serdev_register_devices(ctrl) functions for platform_devs,
>>> assuming
>>> the tty-parent-dev on the Edison SOM is a platform_dev ?
>> tty parent is PCI device there.
>>> Anyways it looks like this will be really hard to do without access
>>> to the hardware.
>> I can do a BAT.
>
> Right, sorry I was not clear, when I was talking about hardware
> access I was not (not only) referring to testing.
>
> The problem is that to figure out how to hook all this together
> will require poking around on the hardware, looking in sysfs, finding
> some id we can use in a pci_serdev_register_devices() to add to
> drivers/tty/serdev/core.c so that it only turns the serial port on
> the Edison into a serdev and not somewhere else, etc.
>
> But thinking about this more, I too have higher priority items on
> my TODO list, so as much as I would like to see this cleaned up,
> lets shelf this for now until you have time to look into this.
>
> When you do find time, if you've any questions I'm happy to help.
so the serial port is driven by drivers/tty/serial/8250/8250_mid.c and using PCI_DEVICE_ID_INTEL_TNG_UART 0x1191 to match the driver. So it is exposed as PCI device. While of course more than one UART port uses 0x1191. This one seems to be mapped to pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(4, 1)) actually.
What I think is that we need to do is some sort of x86_match_cpu like arch/x86/platform/intel-mid/device_libs/platform_bt.c does. But then again, that is also what the Apple Broadcom support does to choose the different ACPI procedures.
I have an Edison board on my desk and can quickly boot any patch you want me to test. Or grab data from sysfs or dmesg.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2] serdev: add method to set parity
From: Marcel Holtmann @ 2018-01-22 18:23 UTC (permalink / raw)
To: Ulrich Hecht, Rob Herring, Greg Kroah-Hartman
Cc: linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Johan Hovold,
Sebastian Reichel, Bluez mailing list, Martin Blumenstingl,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
wsa-z923LK4zBo2bacvFa/9K2g, geert-Td1EMuHUCqxL1ZNQvxDV9g
In-Reply-To: <1516643792-22840-1-git-send-email-ulrich.hecht+renesas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Rob,
> Adds serdev_device_set_parity() and an implementation for ttyport.
> The interface uses an enum with the values SERIAL_PARITY_NONE,
> SERIAL_PARITY_EVEN and SERIAL_PARITY_ODD.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> ---
>
> Hi!
>
> This revision addresses Johan's comments (see below for details) and adds
> Sebastian's Reviewed-by tag. Thank you for your reviews!
>
> CU
> Uli
>
>
> Changes since v1:
> - added Reviewed-by tag
> - expanded commit message
> - shuffled stuff around to keep line-setting bits together
> - clear CMSPAR
> - (hopefully) detect errors correctly by checking tty->termios after call
> to tty_set_termios().
>
>
> drivers/tty/serdev/core.c | 12 ++++++++++++
> drivers/tty/serdev/serdev-ttyport.c | 24 ++++++++++++++++++++++++
> include/linux/serdev.h | 10 ++++++++++
> 3 files changed, 46 insertions(+)
if there are no objections, I would like to just take this through bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* [PATCH v2] serdev: add method to set parity
From: Ulrich Hecht @ 2018-01-22 17:56 UTC (permalink / raw)
To: marcel
Cc: linux-renesas-soc, linux-serial, johan, sre, linux-bluetooth,
robh, martin.blumenstingl, gregkh, magnus.damm, laurent.pinchart,
wsa, geert, Ulrich Hecht
Adds serdev_device_set_parity() and an implementation for ttyport.
The interface uses an enum with the values SERIAL_PARITY_NONE,
SERIAL_PARITY_EVEN and SERIAL_PARITY_ODD.
Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
Hi!
This revision addresses Johan's comments (see below for details) and adds
Sebastian's Reviewed-by tag. Thank you for your reviews!
CU
Uli
Changes since v1:
- added Reviewed-by tag
- expanded commit message
- shuffled stuff around to keep line-setting bits together
- clear CMSPAR
- (hopefully) detect errors correctly by checking tty->termios after call
to tty_set_termios().
drivers/tty/serdev/core.c | 12 ++++++++++++
drivers/tty/serdev/serdev-ttyport.c | 24 ++++++++++++++++++++++++
include/linux/serdev.h | 10 ++++++++++
3 files changed, 46 insertions(+)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 5dc88f6..ceee0ca 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -257,6 +257,18 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
}
EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
+int serdev_device_set_parity(struct serdev_device *serdev,
+ enum serdev_parity parity)
+{
+ struct serdev_controller *ctrl = serdev->ctrl;
+
+ if (!ctrl || !ctrl->ops->set_parity)
+ return -ENOTSUPP;
+
+ return ctrl->ops->set_parity(ctrl, parity);
+}
+EXPORT_SYMBOL_GPL(serdev_device_set_parity);
+
void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
{
struct serdev_controller *ctrl = serdev->ctrl;
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index a5abb05..fa16729 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -194,6 +194,29 @@ static void ttyport_set_flow_control(struct serdev_controller *ctrl, bool enable
tty_set_termios(tty, &ktermios);
}
+static int ttyport_set_parity(struct serdev_controller *ctrl,
+ enum serdev_parity parity)
+{
+ struct serport *serport = serdev_controller_get_drvdata(ctrl);
+ struct tty_struct *tty = serport->tty;
+ struct ktermios ktermios = tty->termios;
+
+ ktermios.c_cflag &= ~(PARENB | PARODD | CMSPAR);
+ if (parity != SERDEV_PARITY_NONE) {
+ ktermios.c_cflag |= PARENB;
+ if (parity == SERDEV_PARITY_ODD)
+ ktermios.c_cflag |= PARODD;
+ }
+
+ tty_set_termios(tty, &ktermios);
+
+ if ((tty->termios.c_cflag & (PARENB | PARODD | CMSPAR)) !=
+ (ktermios.c_cflag & (PARENB | PARODD | CMSPAR)))
+ return -EINVAL;
+
+ return 0;
+}
+
static void ttyport_wait_until_sent(struct serdev_controller *ctrl, long timeout)
{
struct serport *serport = serdev_controller_get_drvdata(ctrl);
@@ -231,6 +254,7 @@ static const struct serdev_controller_ops ctrl_ops = {
.open = ttyport_open,
.close = ttyport_close,
.set_flow_control = ttyport_set_flow_control,
+ .set_parity = ttyport_set_parity,
.set_baudrate = ttyport_set_baudrate,
.wait_until_sent = ttyport_wait_until_sent,
.get_tiocm = ttyport_get_tiocm,
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 48d8ce2..f153b2c 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -78,6 +78,12 @@ static inline struct serdev_device_driver *to_serdev_device_driver(struct device
return container_of(d, struct serdev_device_driver, driver);
}
+enum serdev_parity {
+ SERDEV_PARITY_NONE,
+ SERDEV_PARITY_EVEN,
+ SERDEV_PARITY_ODD,
+};
+
/*
* serdev controller structures
*/
@@ -88,6 +94,7 @@ struct serdev_controller_ops {
int (*open)(struct serdev_controller *);
void (*close)(struct serdev_controller *);
void (*set_flow_control)(struct serdev_controller *, bool);
+ int (*set_parity)(struct serdev_controller *, enum serdev_parity);
unsigned int (*set_baudrate)(struct serdev_controller *, unsigned int);
void (*wait_until_sent)(struct serdev_controller *, long);
int (*get_tiocm)(struct serdev_controller *);
@@ -301,6 +308,9 @@ static inline int serdev_device_set_rts(struct serdev_device *serdev, bool enabl
return serdev_device_set_tiocm(serdev, 0, TIOCM_RTS);
}
+int serdev_device_set_parity(struct serdev_device *serdev,
+ enum serdev_parity parity);
+
/*
* serdev hooks into TTY core
*/
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v6 29/36] nds32: Build infrastructure
From: Greentime Hu @ 2018-01-22 16:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greentime, Linux Kernel Mailing List, linux-arch, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Rob Herring, Networking, Vincent Chen,
DTML, Al Viro, David Howells, Will Deacon, Daniel Lezcano,
linux-serial, Geert Uytterhoeven, Linus Walleij, Mark Rutland,
Greg KH, Guo Ren
In-Reply-To: <CAK8P3a20eqjW3_bQf4eMTwtmJSZ8Vp4agV5O--UkLGXnMhbisQ@mail.gmail.com>
Hi, Arnd:
2018-01-22 23:38 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> On Mon, Jan 22, 2018 at 4:20 PM, Greentime Hu <green.hu@gmail.com> wrote:
> BE
>>>
>>> I think it's better to drop GENERIC_IRQ_PROBE here, no modern driver
>>> should rely on that.
>>
>> I will drop it.
>>
>>>> +choice
>>>> + prompt "CPU type"
>>>> + default CPU_V3
>>>> +config CPU_N15
>>>> + bool "AndesCore N15"
>>>> +config CPU_N13
>>>> + bool "AndesCore N13"
>>>> + select CPU_CACHE_ALIASING if ANDES_PAGE_SIZE_4KB
>>>> +config CPU_N10
>>>> + bool "AndesCore N10"
>>>> + select CPU_CACHE_ALIASING
>>>> +config CPU_D15
>>>> + bool "AndesCore D15"
>>>> +config CPU_D10
>>>> + bool "AndesCore D10"
>>>> + select CPU_CACHE_ALIASING
>>>> +config CPU_V3
>>>> + bool "AndesCore v3 compatible"
>>>> + select ANDES_PAGE_SIZE_8KB
>>>> +endchoice
>>>
>>> I forget what we discussed here earlier, but at the very least, there should be
>>> some help text here to explain what the implications are. I assume that you
>>> generally want to be able to build one kernel to run on all of the above, right?
>>>
>>> Will selecting 'CPU_V3' result in a kernel binary that can run on all of them?
>>> If so, please explain it here as that is not obvious.
>>>
>>> For the other CPU types, can you list the what backwards-compatiblity
>>> you get? E.g. will a kernel built for N13 run on any of N15, D15 or N10?
>>>
>> Yes, we would like to build a kernel with CPU_V3 to run on all of the above.
>>
>> Not sure if these help texts clear enough?
>>
>> choice
>> prompt "CPU type"
>> default CPU_V3
>> help
>> The data cache of N15/D15 is implemented as PIPT and it will
>> not cause the
>> cache aliasing issue. The rest cpus(N13, N10 and D10) are
>> implemented as
>> VIPT data cache. It may cause the cache aliasing issue if
>> its cache way
>> size is larger than page size. You can specify the the CPU
>> type direcly or
>> choose CPU_V3 if unsure.
>>
>> A kernel built for N10 is able to run on N15, D15, N13, N10 or D10.
>> A kernel built for N15 is able to run on N15 or D15.
>> A kernel built for D10 is able to run on D10 or D15.
>> A kernel built for D15 is able to run on D15.
>> A kernel built for N13 with CPU_CACHE_ALIASING is able to
>> run on N15, D15, N13, N10 or D10
>> A kernel built for N13 without CPU_CACHE_ALIASING is able to
>> run on N15, N13 or D15
>>
>> config CPU_N15
>> bool "AndesCore N15"
>> config CPU_N13
>> bool "AndesCore N13"
>> select CPU_CACHE_ALIASING if ANDES_PAGE_SIZE_4KB
>> config CPU_N10
>> bool "AndesCore N10"
>> select CPU_CACHE_ALIASING
>> config CPU_D15
>> bool "AndesCore D15"
>> config CPU_D10
>> bool "AndesCore D10"
>> select CPU_CACHE_ALIASING
>> config CPU_V3
>> bool "AndesCore v3 compatible"
>> select CPU_CACHE_ALIASING
>> endchoice
>
> I would drop the description about CPU_CACHE_ALIASING in the list
> of compatibilities text and simply say 'A kernel built for N13 is able to run
> on N15, N13 or D15', it's more logical that way, and it gives you the freedom
> to later change the rules about whether it can or cannot run.
>
> Maybe also change the initial prompt from "CPU type" to "minimum CPU type".
>
Thank you for your suggestion.
I will update it like this.
choice
prompt "minimum CPU type"
default CPU_V3
help
The data cache of N15/D15 is implemented as PIPT and it will not cause
the cache aliasing issue. The rest cpus(N13, N10 and D10) are
implemented as VIPT data cache. It may cause the cache aliasing issue
if its cache way size is larger than page size. You can specify the
CPU type direcly or choose CPU_V3 if unsure.
A kernel built for N10 is able to run on N15, D15, N13, N10 or D10.
A kernel built for N15 is able to run on N15 or D15.
A kernel built for D10 is able to run on D10 or D15.
A kernel built for D15 is able to run on D15.
A kernel built for N13 is able to run on N15, N13 or D15.
config CPU_N15
bool "AndesCore N15"
config CPU_N13
bool "AndesCore N13"
select CPU_CACHE_ALIASING if ANDES_PAGE_SIZE_4KB
config CPU_N10
bool "AndesCore N10"
select CPU_CACHE_ALIASING
config CPU_D15
bool "AndesCore D15"
config CPU_D10
bool "AndesCore D10"
select CPU_CACHE_ALIASING
config CPU_V3
bool "AndesCore v3 compatible"
select CPU_CACHE_ALIASING
endchoice
^ permalink raw reply
* Re: [PATCH v6 29/36] nds32: Build infrastructure
From: Arnd Bergmann @ 2018-01-22 15:38 UTC (permalink / raw)
To: Greentime Hu
Cc: Greentime, Linux Kernel Mailing List, linux-arch, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Rob Herring, Networking, Vincent Chen,
DTML, Al Viro, David Howells, Will Deacon, Daniel Lezcano,
linux-serial, Geert Uytterhoeven, Linus Walleij, Mark Rutland,
Greg KH, Guo Ren
In-Reply-To: <CAEbi=3dZ-Bj+pdhcu1Td1JzgDngyMMeDTFs=XH3LNPnudcUGyg@mail.gmail.com>
On Mon, Jan 22, 2018 at 4:20 PM, Greentime Hu <green.hu@gmail.com> wrote:
BE
>>
>> I think it's better to drop GENERIC_IRQ_PROBE here, no modern driver
>> should rely on that.
>
> I will drop it.
>
>>> +choice
>>> + prompt "CPU type"
>>> + default CPU_V3
>>> +config CPU_N15
>>> + bool "AndesCore N15"
>>> +config CPU_N13
>>> + bool "AndesCore N13"
>>> + select CPU_CACHE_ALIASING if ANDES_PAGE_SIZE_4KB
>>> +config CPU_N10
>>> + bool "AndesCore N10"
>>> + select CPU_CACHE_ALIASING
>>> +config CPU_D15
>>> + bool "AndesCore D15"
>>> +config CPU_D10
>>> + bool "AndesCore D10"
>>> + select CPU_CACHE_ALIASING
>>> +config CPU_V3
>>> + bool "AndesCore v3 compatible"
>>> + select ANDES_PAGE_SIZE_8KB
>>> +endchoice
>>
>> I forget what we discussed here earlier, but at the very least, there should be
>> some help text here to explain what the implications are. I assume that you
>> generally want to be able to build one kernel to run on all of the above, right?
>>
>> Will selecting 'CPU_V3' result in a kernel binary that can run on all of them?
>> If so, please explain it here as that is not obvious.
>>
>> For the other CPU types, can you list the what backwards-compatiblity
>> you get? E.g. will a kernel built for N13 run on any of N15, D15 or N10?
>>
> Yes, we would like to build a kernel with CPU_V3 to run on all of the above.
>
> Not sure if these help texts clear enough?
>
> choice
> prompt "CPU type"
> default CPU_V3
> help
> The data cache of N15/D15 is implemented as PIPT and it will
> not cause the
> cache aliasing issue. The rest cpus(N13, N10 and D10) are
> implemented as
> VIPT data cache. It may cause the cache aliasing issue if
> its cache way
> size is larger than page size. You can specify the the CPU
> type direcly or
> choose CPU_V3 if unsure.
>
> A kernel built for N10 is able to run on N15, D15, N13, N10 or D10.
> A kernel built for N15 is able to run on N15 or D15.
> A kernel built for D10 is able to run on D10 or D15.
> A kernel built for D15 is able to run on D15.
> A kernel built for N13 with CPU_CACHE_ALIASING is able to
> run on N15, D15, N13, N10 or D10
> A kernel built for N13 without CPU_CACHE_ALIASING is able to
> run on N15, N13 or D15
>
> config CPU_N15
> bool "AndesCore N15"
> config CPU_N13
> bool "AndesCore N13"
> select CPU_CACHE_ALIASING if ANDES_PAGE_SIZE_4KB
> config CPU_N10
> bool "AndesCore N10"
> select CPU_CACHE_ALIASING
> config CPU_D15
> bool "AndesCore D15"
> config CPU_D10
> bool "AndesCore D10"
> select CPU_CACHE_ALIASING
> config CPU_V3
> bool "AndesCore v3 compatible"
> select CPU_CACHE_ALIASING
> endchoice
I would drop the description about CPU_CACHE_ALIASING in the list
of compatibilities text and simply say 'A kernel built for N13 is able to run
on N15, N13 or D15', it's more logical that way, and it gives you the freedom
to later change the rules about whether it can or cannot run.
Maybe also change the initial prompt from "CPU type" to "minimum CPU type".
>> I think the 'select ANDES_PAGE_SIZE_8KB' cannot work as expected,
>> since ANDES_PAGE_SIZE_8KB is inside of a 'choice' statement. Since
>> there are only two options (4K and 8K), you can address that by making
>> it a simple bool option and fall back to 4K when ANDES_PAGE_SIZE_8KB
>> is disabled.
>
> After reviewing this config, it seems to make much more sense if we
> select CPU_CACHE_ALIASING.
> A kernel with aliasing cache handling should be able to run on
> aliasing/non-aliasing CPU.
>
> config CPU_V3
> bool "AndesCore v3 compatible"
> select CPU_CACHE_ALIASING
Sure, that's a possible way out.
Arnd
^ permalink raw reply
* Re: [PATCH v6 29/36] nds32: Build infrastructure
From: Greentime Hu @ 2018-01-22 15:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greentime, Linux Kernel Mailing List, linux-arch, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Rob Herring, Networking, Vincent Chen,
DTML, Al Viro, David Howells, Will Deacon, Daniel Lezcano,
linux-serial, Geert Uytterhoeven, Linus Walleij, Mark Rutland,
Greg KH, Guo Ren
In-Reply-To: <CAK8P3a2k18Z5pK-04h4bSVn82m+tvNL4JdtgVT9AJWP5gKXuig@mail.gmail.com>
Hi, Arnd:
2018-01-18 19:00 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu@gmail.com> wrote:
>> From: Greentime Hu <greentime@andestech.com>
>>
>> This patch adds Makefile, Kconfig and vmlinux.lds.S files required for building
>> an nds32 kernel.
>>
>> Signed-off-by: Vincent Chen <vincentc@andestech.com>
>> Signed-off-by: Greentime Hu <greentime@andestech.com>
>
> I find some new details every time I look here ;-)
Thank you for revewing so detailedly. :)
>> @@ -0,0 +1,107 @@
>> +#
>> +# For a description of the syntax of this configuration file,
>> +# see Documentation/kbuild/kconfig-language.txt.
>> +#
>> +
>> +config NDS32
>> + def_bool y
>> + select ARCH_HAS_RAW_COPY_USER
>
> I don't think this symbol was ever merged. Do you remember why you added it?
I will drop it. It must be added in earlier kernel version.
>> + select ARCH_WANT_FRAME_POINTERS if FTRACE
>> + select ARCH_WANT_IPC_PARSE_VERSION
>
> You most certainly don't want IPC_PARSE_VERSION, please drop this
> and adapt your glibc.
ok. I will drop it.
>> + select CLKSRC_MMIO
>> + select CLONE_BACKWARDS
>> + select COMMON_CLK
>> + select FRAME_POINTER
>
> Do you need both ARCH_WANT_FRAME_POINTERS and FRAME_POINTER here?
I will drop FRAME_POINTER.
>> + select GENERIC_ATOMIC64
>> + select GENERIC_CPU_DEVICES
>> + select GENERIC_CLOCKEVENTS
>> + select GENERIC_IRQ_CHIP
>> + select GENERIC_IRQ_PROBE
>
> I think it's better to drop GENERIC_IRQ_PROBE here, no modern driver
> should rely on that.
I will drop it.
>> +choice
>> + prompt "CPU type"
>> + default CPU_V3
>> +config CPU_N15
>> + bool "AndesCore N15"
>> +config CPU_N13
>> + bool "AndesCore N13"
>> + select CPU_CACHE_ALIASING if ANDES_PAGE_SIZE_4KB
>> +config CPU_N10
>> + bool "AndesCore N10"
>> + select CPU_CACHE_ALIASING
>> +config CPU_D15
>> + bool "AndesCore D15"
>> +config CPU_D10
>> + bool "AndesCore D10"
>> + select CPU_CACHE_ALIASING
>> +config CPU_V3
>> + bool "AndesCore v3 compatible"
>> + select ANDES_PAGE_SIZE_8KB
>> +endchoice
>
> I forget what we discussed here earlier, but at the very least, there should be
> some help text here to explain what the implications are. I assume that you
> generally want to be able to build one kernel to run on all of the above, right?
>
> Will selecting 'CPU_V3' result in a kernel binary that can run on all of them?
> If so, please explain it here as that is not obvious.
>
> For the other CPU types, can you list the what backwards-compatiblity
> you get? E.g. will a kernel built for N13 run on any of N15, D15 or N10?
>
Yes, we would like to build a kernel with CPU_V3 to run on all of the above.
Not sure if these help texts clear enough?
choice
prompt "CPU type"
default CPU_V3
help
The data cache of N15/D15 is implemented as PIPT and it will
not cause the
cache aliasing issue. The rest cpus(N13, N10 and D10) are
implemented as
VIPT data cache. It may cause the cache aliasing issue if
its cache way
size is larger than page size. You can specify the the CPU
type direcly or
choose CPU_V3 if unsure.
A kernel built for N10 is able to run on N15, D15, N13, N10 or D10.
A kernel built for N15 is able to run on N15 or D15.
A kernel built for D10 is able to run on D10 or D15.
A kernel built for D15 is able to run on D15.
A kernel built for N13 with CPU_CACHE_ALIASING is able to
run on N15, D15, N13, N10 or D10
A kernel built for N13 without CPU_CACHE_ALIASING is able to
run on N15, N13 or D15
config CPU_N15
bool "AndesCore N15"
config CPU_N13
bool "AndesCore N13"
select CPU_CACHE_ALIASING if ANDES_PAGE_SIZE_4KB
config CPU_N10
bool "AndesCore N10"
select CPU_CACHE_ALIASING
config CPU_D15
bool "AndesCore D15"
config CPU_D10
bool "AndesCore D10"
select CPU_CACHE_ALIASING
config CPU_V3
bool "AndesCore v3 compatible"
select CPU_CACHE_ALIASING
endchoice
> I think the 'select ANDES_PAGE_SIZE_8KB' cannot work as expected,
> since ANDES_PAGE_SIZE_8KB is inside of a 'choice' statement. Since
> there are only two options (4K and 8K), you can address that by making
> it a simple bool option and fall back to 4K when ANDES_PAGE_SIZE_8KB
> is disabled.
After reviewing this config, it seems to make much more sense if we
select CPU_CACHE_ALIASING.
A kernel with aliasing cache handling should be able to run on
aliasing/non-aliasing CPU.
config CPU_V3
bool "AndesCore v3 compatible"
select CPU_CACHE_ALIASING
>> +config CACHE_L2
>> + bool "Support L2 cache"
>> + default y
>> + help
>> + Say Y here to enable L2 cache if your SoC are integrated with L2CC.
>> + If unsure, say N.
>> +
>> +menu "Memory configuration"
>> +
>> +choice
>> + prompt "Memory split"
>> + depends on MMU
>> + default VMSPLIT_3G
>
> Why not default to VMSPLIT_3G_OPT?
>
I will set default to VMSPLIT_3G_OPT and delete
CONFIG_VMSPLIT_3G_OPT=y in defconfig.
^ permalink raw reply
* Re: [PATCH v6 31/36] dt-bindings: nds32 CPU Bindings
From: Greentime Hu @ 2018-01-22 13:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Greentime, Linux Kernel Mailing List,
linux-arch, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Rob Herring, Networking, Vincent Chen, DTML, Al Viro,
David Howells, Will Deacon, Daniel Lezcano, linux-serial,
Linus Walleij, Mark Rutland, Greg KH, Guo Ren
In-Reply-To: <CAK8P3a2noraQFvJbi2=T+rb3KDfZ88zJWQ_h3dYjD2ArhLPM1Q@mail.gmail.com>
Hi, Arnd:
2018-01-22 19:15 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> On Mon, Jan 22, 2018 at 10:53 AM, Greentime Hu <green.hu@gmail.com> wrote:
>> 2018-01-19 23:37 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>>> On Fri, Jan 19, 2018 at 4:35 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>> 2018-01-19 23:29 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>>>>> On Fri, Jan 19, 2018 at 4:18 PM, Greentime Hu <green.hu@gmail.com> wrote:
>
>>>> Thank you and your example.
>>>> I get it. I will update this document like this.
>>>> - compatible: Should be "andestech,<core_name>", "andestech,nds32v3"
>>>> as fallback.
>>>
>>> And please keep a list of supported values of "andestech,<core_name>"
>>> in the DT binding document, so checkpatch can validate compatible values.
>>>
>>
>> Thank you for reminding me this.
>> I will list it like this.
>>
>> - compatible:
>> Usage: required
>> Value type: <string>
>> Definition: Should be "andestech,<core_name>",
>> "andestech,nds32v3" as fallback.
>> Examlpes with core_names are:
>> "andestech,n13"
>> "andestech,n15"
>> "andestech,d15"
>> "andestech,n10"
>> "andestech,d10"
>
> This is still not written as a proper specification, you should not
> give "examples"
> but give a complete list of the available options. You could write it like:
>
> Must contain "andestech,nds32v3" as the most generic value, in addition to
> one of the following identifiers for a particular CPU core:
> "andestech,n13"
> "andestech,n15"
> "andestech,d15"
> "andestech,n10"
> "andestech,d10"
>
> It might be helpful to also list all other existing nds32v3 cores,
> even those that the
> current Linux port does not support them.
>
Thank you for the clear explanation.
I will update it just like you wrote.
^ permalink raw reply
* Re: [PATCH RFC 3/3] MIPS: AR7: ensure the port type's FCR value is used
From: Greg Kroah-Hartman @ 2018-01-22 13:34 UTC (permalink / raw)
To: James Hogan
Cc: stable, Jonas Gorski, linux-mips, linux-serial, Ralf Baechle,
Yoshihiro YUNOMAE, Florian Fainelli, Nicolas Schichan
In-Reply-To: <20180122130718.GA22211@saruman>
On Mon, Jan 22, 2018 at 01:07:19PM +0000, James Hogan wrote:
> Hi stable maintainers,
>
> On Sun, Oct 29, 2017 at 04:27:21PM +0100, Jonas Gorski wrote:
> > Since commit aef9a7bd9b67 ("serial/uart/8250: Add tunable RX interrupt
> > trigger I/F of FIFO buffers"), the port's default FCR value isn't used
> > in serial8250_do_set_termios anymore, but copied over once in
> > serial8250_config_port and then modified as needed.
> >
> > Unfortunately, serial8250_config_port will never be called if the port
> > is shared between kernel and userspace, and the port's flag doesn't have
> > UPF_BOOT_AUTOCONF, which would trigger a serial8250_config_port as well.
> >
> > This causes garbled output from userspace:
> >
> > [ 5.220000] random: procd urandom read with 49 bits of entropy available
> > ers
> > [kee
> >
> > Fix this by forcing it to be configured on boot, resulting in the
> > expected output:
> >
> > [ 5.250000] random: procd urandom read with 50 bits of entropy available
> > Press the [f] key and hit [enter] to enter failsafe mode
> > Press the [1], [2], [3] or [4] key and hit [enter] to select the debug level
> >
> > Fixes: aef9a7bd9b67 ("serial/uart/8250: Add tunable RX interrupt trigger I/F of FIFO buffers")
> > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
>
> Please can this patch be applied to stable branches 3.17+. It is now
> merged into mainline as commit 0a5191efe06b ("MIPS: AR7: ensure the port
> type's FCR value is used").
>
> Commit b084116f8587 ("MIPS: AR7: Ensure that serial ports are properly
> set up") is a prerequisite for it to apply cleanly, but is already
> tagged for stable.
Now snuck into this round of stable -rc review :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH RFC 3/3] MIPS: AR7: ensure the port type's FCR value is used
From: James Hogan @ 2018-01-22 13:07 UTC (permalink / raw)
To: stable
Cc: Jonas Gorski, linux-mips, linux-serial, Ralf Baechle,
Greg Kroah-Hartman, Yoshihiro YUNOMAE, Florian Fainelli,
Nicolas Schichan
In-Reply-To: <20171029152721.6770-4-jonas.gorski@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]
Hi stable maintainers,
On Sun, Oct 29, 2017 at 04:27:21PM +0100, Jonas Gorski wrote:
> Since commit aef9a7bd9b67 ("serial/uart/8250: Add tunable RX interrupt
> trigger I/F of FIFO buffers"), the port's default FCR value isn't used
> in serial8250_do_set_termios anymore, but copied over once in
> serial8250_config_port and then modified as needed.
>
> Unfortunately, serial8250_config_port will never be called if the port
> is shared between kernel and userspace, and the port's flag doesn't have
> UPF_BOOT_AUTOCONF, which would trigger a serial8250_config_port as well.
>
> This causes garbled output from userspace:
>
> [ 5.220000] random: procd urandom read with 49 bits of entropy available
> ers
> [kee
>
> Fix this by forcing it to be configured on boot, resulting in the
> expected output:
>
> [ 5.250000] random: procd urandom read with 50 bits of entropy available
> Press the [f] key and hit [enter] to enter failsafe mode
> Press the [1], [2], [3] or [4] key and hit [enter] to select the debug level
>
> Fixes: aef9a7bd9b67 ("serial/uart/8250: Add tunable RX interrupt trigger I/F of FIFO buffers")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
Please can this patch be applied to stable branches 3.17+. It is now
merged into mainline as commit 0a5191efe06b ("MIPS: AR7: ensure the port
type's FCR value is used").
Commit b084116f8587 ("MIPS: AR7: Ensure that serial ports are properly
set up") is a prerequisite for it to apply cleanly, but is already
tagged for stable.
Thanks
James
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Andy Shevchenko @ 2018-01-22 12:15 UTC (permalink / raw)
To: Marcel Holtmann, Hans de Goede, Ferry Toth gmail
Cc: Gustavo F. Padovan, Johan Hedberg, Bluez mailing list,
linux-serial, ACPI Devel Maling List, Lukas Wunner
In-Reply-To: <83ACB297-D092-4512-AC5B-3AF1137BE194@holtmann.org>
On Mon, 2018-01-22 at 13:01 +0100, Marcel Holtmann wrote:
> > > > Anyways it looks like this will be really hard to do without
> > > > access
> > > > to the hardware.
> > >
> > > I can do a BAT.
> >
> > Right, sorry I was not clear, when I was talking about hardware
> > access I was not (not only) referring to testing.
> >
> > The problem is that to figure out how to hook all this together
> > will require poking around on the hardware, looking in sysfs,
> > finding
> > some id we can use in a pci_serdev_register_devices() to add to
> > drivers/tty/serdev/core.c so that it only turns the serial port on
> > the Edison into a serdev and not somewhere else, etc.
> >
> > But thinking about this more, I too have higher priority items on
> > my TODO list, so as much as I would like to see this cleaned up,
> > lets shelf this for now until you have time to look into this.
> >
> > When you do find time, if you've any questions I'm happy to help.
>
> so in theory the Edison board has SFI. Problem is just that I am not
> sure the SFI on the board is fully correct and contains enough
> information to identify the serial port correctly.
Unfortunately SFI was so badly designed that board (hard coded) data is
needed.
> If no one is willing to help port this over to serdev, then one option
> is just to rip it out and see who complains.
Me in the first place. + I guess at least couple of users as of my
knowledge. + Undefined amount of swindled users of Edison who is trying
or already did a switch to vanilla kernel.
> Or include a version that is trying its best and see who reports
> issues and who helps to fix it. I am now regretting that I accepted
> the Edison support in the first place, but sadly serdev was not yet
> fully baked at that point.
+Cc: Ferry, who might be interested in the helping with it.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH] serdev: add method to set parity
From: Marcel Holtmann @ 2018-01-22 12:03 UTC (permalink / raw)
To: Johan Hovold
Cc: Ulrich Hecht, linux-serial, linux-renesas-soc, Bluez mailing list,
Rob Herring, Martin Blumenstingl, Greg Kroah-Hartman, magnus.damm,
laurent.pinchart, wsa, geert
In-Reply-To: <20180118022355.GA3286@localhost>
Hi Ulrich,
>> Adds serdev_device_set_parity() and an implementation for ttyport.
>
> Perhaps you can mention that the interface uses an enum and the three
> settings that you add here.
>
>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>> ---
>> Broken out of the "[PATCH 0/6] serdev multiplexing support" series
>> because this kind of functionality is apparently also required
>> for "[RFC v2 0/9] Realtek Bluetooth serdev support (H5 protocol)",
>> which contains an earlier revision of this patch.
>
> Thanks for submitting this separately.
>
>> drivers/tty/serdev/core.c | 12 ++++++++++++
>> drivers/tty/serdev/serdev-ttyport.c | 18 ++++++++++++++++++
>> include/linux/serdev.h | 10 ++++++++++
>> 3 files changed, 40 insertions(+)
>>
>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> index 5dc88f6..1f25896 100644
>> --- a/drivers/tty/serdev/core.c
>> +++ b/drivers/tty/serdev/core.c
>> @@ -290,6 +290,18 @@ int serdev_device_set_tiocm(struct serdev_device *serdev, int set, int clear)
>> }
>> EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
>>
>> +int serdev_device_set_parity(struct serdev_device *serdev,
>> + enum serdev_parity parity)
>> +{
>> + struct serdev_controller *ctrl = serdev->ctrl;
>> +
>> + if (!ctrl || !ctrl->ops->set_parity)
>> + return -ENOTSUPP;
>> +
>> + return ctrl->ops->set_parity(ctrl, parity);
>> +}
>> +EXPORT_SYMBOL_GPL(serdev_device_set_parity);
>
> Please place the parity functions (and fields) after the set_flow
> equivalents so that we keep the line-setting functionality grouped
> together.
>
>> +static int ttyport_set_parity(struct serdev_controller *ctrl,
>> + enum serdev_parity parity)
>> +{
>> + struct serport *serport = serdev_controller_get_drvdata(ctrl);
>> + struct tty_struct *tty = serport->tty;
>> + struct ktermios ktermios = tty->termios;
>> +
>> + ktermios.c_cflag &= ~(PARENB | PARODD);
>
> You should also clear CMSPAR.
>
>> + if (parity != SERDEV_PARITY_NONE) {
>> + ktermios.c_cflag |= PARENB;
>> + if (parity == SERDEV_PARITY_ODD)
>> + ktermios.c_cflag |= PARODD;
>> + }
>> +
>> + return tty_set_termios(tty, &ktermios);
>
> Note that tty_set_termios() always return 0. You need to verify that you
> got what you requested by looking at the termios after the function
> returns (and possibly return -EINVAL). Not all drivers support (all)
> parity modes.
>
> Note that we currently do not implement proper error handling for flow
> control, but that should be fixed.
>
> Looks good otherwise.
can I get an updated patch addressing the comments and also including the Reviewed-by tags.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2 1/2] Bluetooth: hci_bcm: For serdev case close serdev on failure to set power
From: Marcel Holtmann @ 2018-01-22 12:02 UTC (permalink / raw)
To: Hans de Goede
Cc: Andy Shevchenko, Gustavo F. Padovan, Johan Hedberg,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, Lukas Wunner
In-Reply-To: <20180122115325.28960-2-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hi Hans,
> Commit 8bfa7e1e03ac ("Bluetooth: hci_bcm: Handle errors properly")
> introduced error checking for the bcm_gpio_set_power() call in bcm_open()
> but the error-path it introduces unsets dev->hu, which is correct for
> platform_device instantiated bcm_dev-s but not for serdev instantiated
> devs. For serdev instantiated devs serdev_device_close() should be called
> instead (and dev->hu should be left set).
>
> Cc: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
> Fixes: 8bfa7e1e03ac ("Bluetooth: hci_bcm: Handle errors properly")
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/bluetooth/hci_bcm.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Marcel Holtmann @ 2018-01-22 12:01 UTC (permalink / raw)
To: Hans de Goede
Cc: Andy Shevchenko, Gustavo F. Padovan, Johan Hedberg,
Bluez mailing list, linux-serial-u79uwXL29TY76Z2rM5mHXA,
ACPI Devel Maling List, Lukas Wunner
In-Reply-To: <0cae9024-887a-45f7-7710-1684f9c0e54e-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hi Hans,
>>>>> Andy, I see that you added support for bcm bluetooth over a tty
>>>>> using
>>>>> platform_data instead of ACPI enumeration. Can you change the code
>>>>> instantiating the device to instead instantiate a serdev, so that
>>>>> we
>>>>> kill the platform device support in hci_bcm.c and so that users
>>>>> don't
>>>>> need to do a btattach, but instead the kernel will do the attach
>>>>> itself
>>>>> and things will just work ?
>>>>
>>>> I'm sorry, I can't do this soon, other more priority tasks in a
>>>> pocket.
>>>>
>>>> The instantiation of the driver is happened in
>>>> arch/x86/platform/intel-
>>>> mid/device_libs/platform_bt.c
>>>>
>>>> I would help with review of any patches till I would able to look at
>>>> it
>>>> myself.
>>>
>>> If I manage to come up with patches do you have hardware and time to
>>> test?
>> Yes and I would find half an hour for sure.
>
> That is great, thank you, but ...
>
>>> First point of order to get this working as serdev I think is to
>>> modify drivers/tty/serdev/core.c a and then the
>>> serdev_controller_add()
>>> function to somehow recognize the serial port in question, so
>>> something akin to the of_serdev_register_devices(ctrl) /
>>> acpi_serdev_register_devices(ctrl) functions for platform_devs,
>>> assuming
>>> the tty-parent-dev on the Edison SOM is a platform_dev ?
>> tty parent is PCI device there.
>>> Anyways it looks like this will be really hard to do without access
>>> to the hardware.
>> I can do a BAT.
>
> Right, sorry I was not clear, when I was talking about hardware
> access I was not (not only) referring to testing.
>
> The problem is that to figure out how to hook all this together
> will require poking around on the hardware, looking in sysfs, finding
> some id we can use in a pci_serdev_register_devices() to add to
> drivers/tty/serdev/core.c so that it only turns the serial port on
> the Edison into a serdev and not somewhere else, etc.
>
> But thinking about this more, I too have higher priority items on
> my TODO list, so as much as I would like to see this cleaned up,
> lets shelf this for now until you have time to look into this.
>
> When you do find time, if you've any questions I'm happy to help.
so in theory the Edison board has SFI. Problem is just that I am not sure the SFI on the board is fully correct and contains enough information to identify the serial port correctly.
If no one is willing to help port this over to serdev, then one option is just to rip it out and see who complains. Or include a version that is trying its best and see who reports issues and who helps to fix it. I am now regretting that I accepted the Edison support in the first place, but sadly serdev was not yet fully baked at that point.
Regards
Marcel
^ permalink raw reply
* [PATCH v2 2/2] Bluetooth: hci_bcm: Remove platform_device support
From: Hans de Goede @ 2018-01-22 11:53 UTC (permalink / raw)
To: Andy Shevchenko, Marcel Holtmann, Gustavo Padovan, Johan Hedberg
Cc: Hans de Goede, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, Lukas Wunner
In-Reply-To: <20180122115325.28960-1-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Now that ACPI and DT devices are both enumerated as serdevs, we can
remove platform_device support and the bcm_device_list lookup hack.
This also removes any races between suspend/resume and hci-uart binding,
also making the suspend/resume code a lot simpler.
This commit leaves manually binding to an uart using btattach supported
(without irq/gpio and thus suspend/resume support, as before).
Cc: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/bluetooth/hci_bcm.c | 258 +++++---------------------------------------
1 file changed, 27 insertions(+), 231 deletions(-)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 0438a64b8185..92d6c461a7e7 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -30,7 +30,6 @@
#include <linux/of.h>
#include <linux/property.h>
#include <linux/platform_data/x86/apple.h>
-#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/gpio/consumer.h>
#include <linux/tty.h>
@@ -80,9 +79,6 @@
* set to 0 if @init_speed is already the preferred baudrate
* @irq: interrupt triggered by HOST_WAKE_BT pin
* @irq_active_low: whether @irq is active low
- * @hu: pointer to HCI UART controller struct,
- * used to disable flow control during runtime suspend and system sleep
- * @is_suspended: whether flow control is currently disabled
*/
struct bcm_device {
/* Must be the first member, hci_serdev.c expects this. */
@@ -107,25 +103,14 @@ struct bcm_device {
u32 oper_speed;
int irq;
bool irq_active_low;
-
-#ifdef CONFIG_PM
- struct hci_uart *hu;
- bool is_suspended;
-#endif
};
/* generic bcm uart resources */
struct bcm_data {
struct sk_buff *rx_skb;
struct sk_buff_head txq;
-
- struct bcm_device *dev;
};
-/* List of BCM BT UART devices */
-static DEFINE_MUTEX(bcm_device_lock);
-static LIST_HEAD(bcm_device_list);
-
static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed)
{
if (hu->serdev)
@@ -183,27 +168,6 @@ static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
return 0;
}
-/* bcm_device_exists should be protected by bcm_device_lock */
-static bool bcm_device_exists(struct bcm_device *device)
-{
- struct list_head *p;
-
-#ifdef CONFIG_PM
- /* Devices using serdev always exist */
- if (device && device->hu && device->hu->serdev)
- return true;
-#endif
-
- list_for_each(p, &bcm_device_list) {
- struct bcm_device *dev = list_entry(p, struct bcm_device, list);
-
- if (device == dev)
- return true;
- }
-
- return false;
-}
-
static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
{
int err;
@@ -249,21 +213,17 @@ static irqreturn_t bcm_host_wake(int irq, void *data)
return IRQ_HANDLED;
}
-static int bcm_request_irq(struct bcm_data *bcm)
+static int bcm_request_irq(struct hci_uart *hu)
{
- struct bcm_device *bdev = bcm->dev;
+ struct bcm_device *bdev;
int err;
- mutex_lock(&bcm_device_lock);
- if (!bcm_device_exists(bdev)) {
- err = -ENODEV;
- goto unlock;
- }
+ if (!hu->serdev)
+ return -ENODEV;
- if (bdev->irq <= 0) {
- err = -EOPNOTSUPP;
- goto unlock;
- }
+ bdev = serdev_device_get_drvdata(hu->serdev);
+ if (bdev->irq <= 0)
+ return -EOPNOTSUPP;
err = devm_request_irq(bdev->dev, bdev->irq, bcm_host_wake,
bdev->irq_active_low ? IRQF_TRIGGER_FALLING :
@@ -271,7 +231,7 @@ static int bcm_request_irq(struct bcm_data *bcm)
"host_wake", bdev);
if (err) {
bdev->irq = err;
- goto unlock;
+ return err;
}
device_init_wakeup(bdev->dev, true);
@@ -282,10 +242,7 @@ static int bcm_request_irq(struct bcm_data *bcm)
pm_runtime_set_active(bdev->dev);
pm_runtime_enable(bdev->dev);
-unlock:
- mutex_unlock(&bcm_device_lock);
-
- return err;
+ return 0;
}
static const struct bcm_set_sleep_mode default_sleep_params = {
@@ -306,11 +263,11 @@ static const struct bcm_set_sleep_mode default_sleep_params = {
static int bcm_setup_sleep(struct hci_uart *hu)
{
- struct bcm_data *bcm = hu->priv;
+ struct bcm_device *bdev = serdev_device_get_drvdata(hu->serdev);
struct sk_buff *skb;
struct bcm_set_sleep_mode sleep_params = default_sleep_params;
- sleep_params.host_wake_active = !bcm->dev->irq_active_low;
+ sleep_params.host_wake_active = !bdev->irq_active_low;
skb = __hci_cmd_sync(hu->hdev, 0xfc27, sizeof(sleep_params),
&sleep_params, HCI_INIT_TIMEOUT);
@@ -326,7 +283,7 @@ static int bcm_setup_sleep(struct hci_uart *hu)
return 0;
}
#else
-static inline int bcm_request_irq(struct bcm_data *bcm) { return 0; }
+static inline int bcm_request_irq(struct hci_uart *hu) { return 0; }
static inline int bcm_setup_sleep(struct hci_uart *hu) { return 0; }
#endif
@@ -356,7 +313,6 @@ static int bcm_set_diag(struct hci_dev *hdev, bool enable)
static int bcm_open(struct hci_uart *hu)
{
struct bcm_data *bcm;
- struct list_head *p;
int err;
bt_dev_dbg(hu->hdev, "hu %p", hu);
@@ -369,57 +325,26 @@ static int bcm_open(struct hci_uart *hu)
hu->priv = bcm;
- mutex_lock(&bcm_device_lock);
-
if (hu->serdev) {
+ struct bcm_device *bdev = serdev_device_get_drvdata(hu->serdev);
+
err = serdev_device_open(hu->serdev);
if (err)
goto err_free;
- bcm->dev = serdev_device_get_drvdata(hu->serdev);
- goto out;
- }
-
- if (!hu->tty->dev)
- goto out;
-
- list_for_each(p, &bcm_device_list) {
- struct bcm_device *dev = list_entry(p, struct bcm_device, list);
-
- /* Retrieve saved bcm_device based on parent of the
- * platform device (saved during device probe) and
- * parent of tty device used by hci_uart
- */
- if (hu->tty->dev->parent == dev->dev->parent) {
- bcm->dev = dev;
-#ifdef CONFIG_PM
- dev->hu = hu;
-#endif
- break;
- }
- }
-
-out:
- if (bcm->dev) {
- hu->init_speed = bcm->dev->init_speed;
- hu->oper_speed = bcm->dev->oper_speed;
- err = bcm_gpio_set_power(bcm->dev, true);
+ hu->init_speed = bdev->init_speed;
+ hu->oper_speed = bdev->oper_speed;
+ err = bcm_gpio_set_power(bdev, true);
if (err)
goto err_unset_hu;
}
- mutex_unlock(&bcm_device_lock);
return 0;
err_unset_hu:
if (hu->serdev)
serdev_device_close(hu->serdev);
-#ifdef CONFIG_PM
- else
- bcm->dev->hu = NULL;
-#endif
err_free:
- mutex_unlock(&bcm_device_lock);
hu->priv = NULL;
kfree(bcm);
return err;
@@ -433,20 +358,10 @@ static int bcm_close(struct hci_uart *hu)
bt_dev_dbg(hu->hdev, "hu %p", hu);
- /* Protect bcm->dev against removal of the device or driver */
- mutex_lock(&bcm_device_lock);
-
if (hu->serdev) {
serdev_device_close(hu->serdev);
bdev = serdev_device_get_drvdata(hu->serdev);
- } else if (bcm_device_exists(bcm->dev)) {
- bdev = bcm->dev;
-#ifdef CONFIG_PM
- bdev->hu = NULL;
-#endif
- }
- if (bdev) {
if (IS_ENABLED(CONFIG_PM) && bdev->irq > 0) {
devm_free_irq(bdev->dev, bdev->irq, bdev);
device_init_wakeup(bdev->dev, false);
@@ -459,7 +374,6 @@ static int bcm_close(struct hci_uart *hu)
else
pm_runtime_set_suspended(bdev->dev);
}
- mutex_unlock(&bcm_device_lock);
skb_queue_purge(&bcm->txq);
kfree_skb(bcm->rx_skb);
@@ -482,7 +396,6 @@ static int bcm_flush(struct hci_uart *hu)
static int bcm_setup(struct hci_uart *hu)
{
- struct bcm_data *bcm = hu->priv;
char fw_name[64];
const struct firmware *fw;
unsigned int speed;
@@ -541,7 +454,7 @@ static int bcm_setup(struct hci_uart *hu)
if (err)
return err;
- if (!bcm_request_irq(bcm))
+ if (!bcm_request_irq(hu))
err = bcm_setup_sleep(hu);
return err;
@@ -583,12 +496,9 @@ static int bcm_recv(struct hci_uart *hu, const void *data, int count)
bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
bcm->rx_skb = NULL;
return err;
- } else if (!bcm->rx_skb) {
+ } else if (!bcm->rx_skb && hu->serdev) {
/* Delay auto-suspend when receiving completed packet */
- mutex_lock(&bcm_device_lock);
- if (bcm->dev && bcm_device_exists(bcm->dev))
- pm_request_resume(bcm->dev->dev);
- mutex_unlock(&bcm_device_lock);
+ pm_request_resume(&hu->serdev->dev);
}
return count;
@@ -613,10 +523,8 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
struct sk_buff *skb = NULL;
struct bcm_device *bdev = NULL;
- mutex_lock(&bcm_device_lock);
-
- if (bcm_device_exists(bcm->dev)) {
- bdev = bcm->dev;
+ if (hu->serdev) {
+ bdev = serdev_device_get_drvdata(hu->serdev);
pm_runtime_get_sync(bdev->dev);
/* Shall be resumed here */
}
@@ -628,8 +536,6 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
pm_runtime_put_autosuspend(bdev->dev);
}
- mutex_unlock(&bcm_device_lock);
-
return skb;
}
@@ -641,20 +547,12 @@ static int bcm_suspend_device(struct device *dev)
bt_dev_dbg(bdev, "");
- if (!bdev->is_suspended && bdev->hu) {
- hci_uart_set_flow_control(bdev->hu, true);
-
- /* Once this returns, driver suspends BT via GPIO */
- bdev->is_suspended = true;
- }
+ hci_uart_set_flow_control(&bdev->serdev_hu, true);
/* Suspend the device */
err = bdev->set_device_wakeup(bdev, false);
if (err) {
- if (bdev->is_suspended && bdev->hu) {
- bdev->is_suspended = false;
- hci_uart_set_flow_control(bdev->hu, false);
- }
+ hci_uart_set_flow_control(&bdev->serdev_hu, false);
return -EBUSY;
}
@@ -681,11 +579,7 @@ static int bcm_resume_device(struct device *dev)
msleep(15);
/* When this executes, the device has woken up already */
- if (bdev->is_suspended && bdev->hu) {
- bdev->is_suspended = false;
-
- hci_uart_set_flow_control(bdev->hu, false);
- }
+ hci_uart_set_flow_control(&bdev->serdev_hu, false);
return 0;
}
@@ -698,18 +592,7 @@ static int bcm_suspend(struct device *dev)
struct bcm_device *bdev = dev_get_drvdata(dev);
int error;
- bt_dev_dbg(bdev, "suspend: is_suspended %d", bdev->is_suspended);
-
- /*
- * When used with a device instantiated as platform_device, bcm_suspend
- * can be called at any time as long as the platform device is bound,
- * so it should use bcm_device_lock to protect access to hci_uart
- * and device_wake-up GPIO.
- */
- mutex_lock(&bcm_device_lock);
-
- if (!bdev->hu)
- goto unlock;
+ bt_dev_dbg(bdev, "");
if (pm_runtime_active(dev))
bcm_suspend_device(dev);
@@ -720,9 +603,6 @@ static int bcm_suspend(struct device *dev)
bt_dev_dbg(bdev, "BCM irq: enabled");
}
-unlock:
- mutex_unlock(&bcm_device_lock);
-
return 0;
}
@@ -732,18 +612,7 @@ static int bcm_resume(struct device *dev)
struct bcm_device *bdev = dev_get_drvdata(dev);
int err = 0;
- bt_dev_dbg(bdev, "resume: is_suspended %d", bdev->is_suspended);
-
- /*
- * When used with a device instantiated as platform_device, bcm_resume
- * can be called at any time as long as platform device is bound,
- * so it should use bcm_device_lock to protect access to hci_uart
- * and device_wake-up GPIO.
- */
- mutex_lock(&bcm_device_lock);
-
- if (!bdev->hu)
- goto unlock;
+ bt_dev_dbg(bdev, "");
if (device_may_wakeup(dev) && bdev->irq > 0) {
disable_irq_wake(bdev->irq);
@@ -751,10 +620,6 @@ static int bcm_resume(struct device *dev)
}
err = bcm_resume_device(dev);
-
-unlock:
- mutex_unlock(&bcm_device_lock);
-
if (!err) {
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
@@ -1005,57 +870,6 @@ static int bcm_of_probe(struct bcm_device *bdev)
return 0;
}
-static int bcm_probe(struct platform_device *pdev)
-{
- struct bcm_device *dev;
- int ret;
-
- dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
- if (!dev)
- return -ENOMEM;
-
- dev->dev = &pdev->dev;
- dev->irq = platform_get_irq(pdev, 0);
-
- if (has_acpi_companion(&pdev->dev)) {
- ret = bcm_acpi_probe(dev);
- if (ret)
- return ret;
- }
-
- ret = bcm_get_resources(dev);
- if (ret)
- return ret;
-
- platform_set_drvdata(pdev, dev);
-
- dev_info(&pdev->dev, "%s device registered.\n", dev->name);
-
- /* Place this instance on the device list */
- mutex_lock(&bcm_device_lock);
- list_add_tail(&dev->list, &bcm_device_list);
- mutex_unlock(&bcm_device_lock);
-
- ret = bcm_gpio_set_power(dev, false);
- if (ret)
- dev_err(&pdev->dev, "Failed to power down\n");
-
- return 0;
-}
-
-static int bcm_remove(struct platform_device *pdev)
-{
- struct bcm_device *dev = platform_get_drvdata(pdev);
-
- mutex_lock(&bcm_device_lock);
- list_del(&dev->list);
- mutex_unlock(&bcm_device_lock);
-
- dev_info(&pdev->dev, "%s device unregistered.\n", dev->name);
-
- return 0;
-}
-
static const struct hci_uart_proto bcm_proto = {
.id = HCI_UART_BCM,
.name = "Broadcom",
@@ -1103,16 +917,6 @@ static const struct dev_pm_ops bcm_pm_ops = {
SET_RUNTIME_PM_OPS(bcm_suspend_device, bcm_resume_device, NULL)
};
-static struct platform_driver bcm_driver = {
- .probe = bcm_probe,
- .remove = bcm_remove,
- .driver = {
- .name = "hci_bcm",
- .acpi_match_table = ACPI_PTR(bcm_acpi_match),
- .pm = &bcm_pm_ops,
- },
-};
-
static int bcm_serdev_probe(struct serdev_device *serdev)
{
struct bcm_device *bcmdev;
@@ -1123,9 +927,6 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
return -ENOMEM;
bcmdev->dev = &serdev->dev;
-#ifdef CONFIG_PM
- bcmdev->hu = &bcmdev->serdev_hu;
-#endif
bcmdev->serdev_hu.serdev = serdev;
serdev_device_set_drvdata(serdev, bcmdev);
@@ -1175,10 +976,6 @@ static struct serdev_device_driver bcm_serdev_driver = {
int __init bcm_init(void)
{
- /* For now, we need to keep both platform device
- * driver (ACPI generated) and serdev driver (DT).
- */
- platform_driver_register(&bcm_driver);
serdev_device_driver_register(&bcm_serdev_driver);
return hci_uart_register_proto(&bcm_proto);
@@ -1186,7 +983,6 @@ int __init bcm_init(void)
int __exit bcm_deinit(void)
{
- platform_driver_unregister(&bcm_driver);
serdev_device_driver_unregister(&bcm_serdev_driver);
return hci_uart_unregister_proto(&bcm_proto);
--
2.14.3
^ permalink raw reply related
* [PATCH v2 1/2] Bluetooth: hci_bcm: For serdev case close serdev on failure to set power
From: Hans de Goede @ 2018-01-22 11:53 UTC (permalink / raw)
To: Andy Shevchenko, Marcel Holtmann, Gustavo Padovan, Johan Hedberg
Cc: Hans de Goede, linux-bluetooth, linux-serial, linux-acpi,
Lukas Wunner
In-Reply-To: <20180122115325.28960-1-hdegoede@redhat.com>
Commit 8bfa7e1e03ac ("Bluetooth: hci_bcm: Handle errors properly")
introduced error checking for the bcm_gpio_set_power() call in bcm_open()
but the error-path it introduces unsets dev->hu, which is correct for
platform_device instantiated bcm_dev-s but not for serdev instantiated
devs. For serdev instantiated devs serdev_device_close() should be called
instead (and dev->hu should be left set).
Cc: Lukas Wunner <lukas@wunner.de>
Fixes: 8bfa7e1e03ac ("Bluetooth: hci_bcm: Handle errors properly")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/bluetooth/hci_bcm.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 64800cd2796c..0438a64b8185 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -412,8 +412,11 @@ static int bcm_open(struct hci_uart *hu)
return 0;
err_unset_hu:
+ if (hu->serdev)
+ serdev_device_close(hu->serdev);
#ifdef CONFIG_PM
- bcm->dev->hu = NULL;
+ else
+ bcm->dev->hu = NULL;
#endif
err_free:
mutex_unlock(&bcm_device_lock);
--
2.14.3
^ permalink raw reply related
* [PATCH v2 0/2] Bluetooth: hci_bcm: For serdev case close serdev on failure to set power
From: Hans de Goede @ 2018-01-22 11:53 UTC (permalink / raw)
To: Andy Shevchenko, Marcel Holtmann, Gustavo Padovan, Johan Hedberg
Cc: Hans de Goede, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA
Hi Marcel,
Here is a v2 of my recent patch-set with the 2 patches swapped in order as
you requested.
Note that 2/2 really is just an RFC / for future reference for now, since
we first need to fix the Edison SOM still using a platform_device + tty
instead of a serdev.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Hans de Goede @ 2018-01-22 11:49 UTC (permalink / raw)
To: Andy Shevchenko, Marcel Holtmann
Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth, linux-serial,
linux-acpi, Lukas Wunner
In-Reply-To: <1516620806.7000.1165.camel@linux.intel.com>
Hi,
On 22-01-18 12:33, Andy Shevchenko wrote:
> On Mon, 2018-01-22 at 12:27 +0100, Hans de Goede wrote:
>> On 22-01-18 10:42, Andy Shevchenko wrote:
>>> On Mon, 2018-01-22 at 09:23 +0100, Hans de Goede wrote:
>>>> On 22-01-18 03:24, Marcel Holtmann wrote:
>
>>>> Andy, I see that you added support for bcm bluetooth over a tty
>>>> using
>>>> platform_data instead of ACPI enumeration. Can you change the code
>>>> instantiating the device to instead instantiate a serdev, so that
>>>> we
>>>> kill the platform device support in hci_bcm.c and so that users
>>>> don't
>>>> need to do a btattach, but instead the kernel will do the attach
>>>> itself
>>>> and things will just work ?
>>>
>>> I'm sorry, I can't do this soon, other more priority tasks in a
>>> pocket.
>>>
>>> The instantiation of the driver is happened in
>>> arch/x86/platform/intel-
>>> mid/device_libs/platform_bt.c
>>>
>>> I would help with review of any patches till I would able to look at
>>> it
>>> myself.
>>
>> If I manage to come up with patches do you have hardware and time to
>> test?
>
> Yes and I would find half an hour for sure.
That is great, thank you, but ...
>> First point of order to get this working as serdev I think is to
>> modify drivers/tty/serdev/core.c a and then the
>> serdev_controller_add()
>> function to somehow recognize the serial port in question, so
>> something akin to the of_serdev_register_devices(ctrl) /
>> acpi_serdev_register_devices(ctrl) functions for platform_devs,
>> assuming
>> the tty-parent-dev on the Edison SOM is a platform_dev ?
>
> tty parent is PCI device there.
>
>> Anyways it looks like this will be really hard to do without access
>> to the hardware.
>
> I can do a BAT.
Right, sorry I was not clear, when I was talking about hardware
access I was not (not only) referring to testing.
The problem is that to figure out how to hook all this together
will require poking around on the hardware, looking in sysfs, finding
some id we can use in a pci_serdev_register_devices() to add to
drivers/tty/serdev/core.c so that it only turns the serial port on
the Edison into a serdev and not somewhere else, etc.
But thinking about this more, I too have higher priority items on
my TODO list, so as much as I would like to see this cleaned up,
lets shelf this for now until you have time to look into this.
When you do find time, if you've any questions I'm happy to help.
Regards,
Hans
p.s.
I will send out a v2 of the patch-set with the patches swapped
as Marcel requested, but this patch (2/2 in the new set) is really
just for future reference until this Edison issue is resolved.
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Andy Shevchenko @ 2018-01-22 11:33 UTC (permalink / raw)
To: Hans de Goede, Marcel Holtmann
Cc: Gustavo F. Padovan, Johan Hedberg, linux-bluetooth, linux-serial,
linux-acpi, Lukas Wunner
In-Reply-To: <0160ba20-9dc9-9921-ab12-3ac58b33e9ae@redhat.com>
On Mon, 2018-01-22 at 12:27 +0100, Hans de Goede wrote:
> On 22-01-18 10:42, Andy Shevchenko wrote:
> > On Mon, 2018-01-22 at 09:23 +0100, Hans de Goede wrote:
> > > On 22-01-18 03:24, Marcel Holtmann wrote:
> > > Andy, I see that you added support for bcm bluetooth over a tty
> > > using
> > > platform_data instead of ACPI enumeration. Can you change the code
> > > instantiating the device to instead instantiate a serdev, so that
> > > we
> > > kill the platform device support in hci_bcm.c and so that users
> > > don't
> > > need to do a btattach, but instead the kernel will do the attach
> > > itself
> > > and things will just work ?
> >
> > I'm sorry, I can't do this soon, other more priority tasks in a
> > pocket.
> >
> > The instantiation of the driver is happened in
> > arch/x86/platform/intel-
> > mid/device_libs/platform_bt.c
> >
> > I would help with review of any patches till I would able to look at
> > it
> > myself.
>
> If I manage to come up with patches do you have hardware and time to
> test?
Yes and I would find half an hour for sure.
> First point of order to get this working as serdev I think is to
> modify drivers/tty/serdev/core.c a and then the
> serdev_controller_add()
> function to somehow recognize the serial port in question, so
> something akin to the of_serdev_register_devices(ctrl) /
> acpi_serdev_register_devices(ctrl) functions for platform_devs,
> assuming
> the tty-parent-dev on the Edison SOM is a platform_dev ?
tty parent is PCI device there.
> Anyways it looks like this will be really hard to do without access
> to the hardware.
I can do a BAT.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: hci_bcm: Remove platform_device support
From: Hans de Goede @ 2018-01-22 11:27 UTC (permalink / raw)
To: Andy Shevchenko, Marcel Holtmann
Cc: Gustavo F. Padovan, Johan Hedberg,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, Lukas Wunner
In-Reply-To: <1516614127.7000.1152.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Hi,
On 22-01-18 10:42, Andy Shevchenko wrote:
> On Mon, 2018-01-22 at 09:23 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 22-01-18 03:24, Marcel Holtmann wrote:
>>> Hi Hans,
>>>
>>>> Now that ACPI and DT devices are both enumerated as serdevs, we
>>>> can
>>>> remove platform_device support and the bcm_device_list lookup
>>>> hack.
>>>>
>>>> This also removes any races between suspend/resume and hci-uart
>>>> binding,
>>>> also making the suspend/resume code a lot simpler.
>>>>
>>>> This commit leaves manually binding to an uart using btattach
>>>> supported
>>>> (without irq/gpio and thus suspend/resume support, as before).
>>>>
>>>> Cc: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>> drivers/bluetooth/hci_bcm.c | 260 +++++---------------------------
>>>> ------------
>>>> 1 file changed, 28 insertions(+), 232 deletions(-)
>>>
>>> so I was under the assumption platforms like Intel Edison still only
>>> do platform data. See commit
>>> 212d71833315c65644efc46223db61dee7b3c68e. Has that changed?
>
> Yes and no.
>
> So, we need that support to satisfy users with classical Edison
> firmware.
>
>> Ugh, I was not aware of that and the whole code to match the tty with
>> the platform_device on btattach is such a mess and I was actually
>> quite
>> happy to be able to delete this.
>
> Good idea.
>
>> Andy, I see that you added support for bcm bluetooth over a tty using
>> platform_data instead of ACPI enumeration. Can you change the code
>> instantiating the device to instead instantiate a serdev, so that we
>> kill the platform device support in hci_bcm.c and so that users don't
>> need to do a btattach, but instead the kernel will do the attach
>> itself
>> and things will just work ?
>
> I'm sorry, I can't do this soon, other more priority tasks in a pocket.
>
> The instantiation of the driver is happened in arch/x86/platform/intel-
> mid/device_libs/platform_bt.c
>
> I would help with review of any patches till I would able to look at it
> myself.
If I manage to come up with patches do you have hardware and time to
test?
First point of order to get this working as serdev I think is to
modify drivers/tty/serdev/core.c a and then the serdev_controller_add()
function to somehow recognize the serial port in question, so
something akin to the of_serdev_register_devices(ctrl) /
acpi_serdev_register_devices(ctrl) functions for platform_devs, assuming
the tty-parent-dev on the Edison SOM is a platform_dev ?
Anyways it looks like this will be really hard to do without access
to the hardware.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v6 31/36] dt-bindings: nds32 CPU Bindings
From: Arnd Bergmann @ 2018-01-22 11:15 UTC (permalink / raw)
To: Greentime Hu
Cc: Geert Uytterhoeven, Greentime, Linux Kernel Mailing List,
linux-arch, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Rob Herring, Networking, Vincent Chen, DTML, Al Viro,
David Howells, Will Deacon, Daniel Lezcano, linux-serial,
Linus Walleij, Mark Rutland, Greg KH, Guo Ren
In-Reply-To: <CAEbi=3es7O6f5T0Z2zsQzbuxzNte+h-_fMnjOOvbvpz8Hu8g_Q@mail.gmail.com>
On Mon, Jan 22, 2018 at 10:53 AM, Greentime Hu <green.hu@gmail.com> wrote:
> 2018-01-19 23:37 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>> On Fri, Jan 19, 2018 at 4:35 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>> 2018-01-19 23:29 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>>>> On Fri, Jan 19, 2018 at 4:18 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>> Thank you and your example.
>>> I get it. I will update this document like this.
>>> - compatible: Should be "andestech,<core_name>", "andestech,nds32v3"
>>> as fallback.
>>
>> And please keep a list of supported values of "andestech,<core_name>"
>> in the DT binding document, so checkpatch can validate compatible values.
>>
>
> Thank you for reminding me this.
> I will list it like this.
>
> - compatible:
> Usage: required
> Value type: <string>
> Definition: Should be "andestech,<core_name>",
> "andestech,nds32v3" as fallback.
> Examlpes with core_names are:
> "andestech,n13"
> "andestech,n15"
> "andestech,d15"
> "andestech,n10"
> "andestech,d10"
This is still not written as a proper specification, you should not
give "examples"
but give a complete list of the available options. You could write it like:
Must contain "andestech,nds32v3" as the most generic value, in addition to
one of the following identifiers for a particular CPU core:
"andestech,n13"
"andestech,n15"
"andestech,d15"
"andestech,n10"
"andestech,d10"
It might be helpful to also list all other existing nds32v3 cores,
even those that the
current Linux port does not support them.
Arnd
^ permalink raw reply
* Re: [PATCH v6 31/36] dt-bindings: nds32 CPU Bindings
From: Greentime Hu @ 2018-01-22 9:53 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, Greentime, Linux Kernel Mailing List, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
Networking, Vincent Chen, DTML, Al Viro, David Howells,
Will Deacon, Daniel Lezcano, linux-serial, Linus Walleij,
Mark Rutland, Greg KH, Guo Ren, Randy Dunlap <rd>
In-Reply-To: <CAMuHMdUqxuiTfM_m80F4XDVkZjx_XCsj_qXRehZxMcKjWhGQ_Q@mail.gmail.com>
Hi, Geert:
2018-01-19 23:37 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> Hi Greentime,
>
> On Fri, Jan 19, 2018 at 4:35 PM, Greentime Hu <green.hu@gmail.com> wrote:
>> 2018-01-19 23:29 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>>> On Fri, Jan 19, 2018 at 4:18 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>> 2018-01-19 22:52 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>>>> On Fri, Jan 19, 2018 at 3:32 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>>>> 2018-01-18 19:02 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>>>>>>> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu@gmail.com> wrote:
>>>>>>>> From: Greentime Hu <greentime@andestech.com>
>>>>>>>>
>>>>>>>> This patch adds nds32 CPU binding documents.
>>>>>>>>
>>>>>>>> Signed-off-by: Vincent Chen <vincentc@andestech.com>
>>>>>>>> Signed-off-by: Rick Chen <rick@andestech.com>
>>>>>>>> Signed-off-by: Zong Li <zong@andestech.com>
>>>>>>>> Signed-off-by: Greentime Hu <greentime@andestech.com>
>>>>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>>>> ---
>>>>>>>> Documentation/devicetree/bindings/nds32/cpus.txt | 37 ++++++++++++++++++++++
>>>>>>>> 1 file changed, 37 insertions(+)
>>>>>>>> create mode 100644 Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/nds32/cpus.txt b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..9a52937
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/nds32/cpus.txt
>>>>>>>> @@ -0,0 +1,37 @@
>>>>>>>> +* Andestech Processor Binding
>>>>>>>> +
>>>>>>>> +This binding specifies what properties must be available in the device tree
>>>>>>>> +representation of a Andestech Processor Core, which is the root node in the
>>>>>>>> +tree.
>>>>>>>> +
>>>>>>>> +Required properties:
>>>>>>>> +
>>>>>>>> + - compatible:
>>>>>>>> + Usage: required
>>>>>>>> + Value type: <string>
>>>>>>>> + Definition: should be one of:
>>>>>>>> + "andestech,n13"
>>>>>>>> + "andestech,n15"
>>>>>>>> + "andestech,d15"
>>>>>>>> + "andestech,n10"
>>>>>>>> + "andestech,d10"
>>>>>>>> + "andestech,nds32v3"
>>>>>>>
>>>>>>> Based on https://lkml.org/lkml/2017/11/27/1290, this should say that
>>>>>>> the device tree should always list 'andestech,nds32v3' as the most
>>>>>>> generic 'compatible' value and list exactly one of the others in
>>>>>>> addition.
>>>>>
>>>>>> I will remove the others and just left "andestech,nds32v3" in here.
>>>>>
>>>>> No, is not what we want here, the CPU node should list exactly which core
>>>>> is used, what we need in the description is a clarification that
>>>>> andestech,nds32v3 must be used in addition to the more specific
>>>>> string.
>>>>
>>>> Hi, Arnd:
>>>>
>>>> Sorry I still don't get your point. Do you mean we should always use
>>>> compatible = "andestech,n13", "andestech,nds32v3";
>>>> instead of
>>>> compatible = "andestech,n13";
>>>
>>> Exactly. The first value is a device-specific compatible value, the second is
>>> a generic fallback.
>>>
>>>> And I need to add the description in this document.
>>>
>>> Indeed. See for example
>>> Documentation/devicetree/bindings/power/renesas,apmu.txt
>>>
>>> Thanks!
>>
>> Hi, Geert:
>>
>> Thank you and your example.
>> I get it. I will update this document like this.
>> - compatible: Should be "andestech,<core_name>", "andestech,nds32v3"
>> as fallback.
>
> And please keep a list of supported values of "andestech,<core_name>"
> in the DT binding document, so checkpatch can validate compatible values.
>
Thank you for reminding me this.
I will list it like this.
- compatible:
Usage: required
Value type: <string>
Definition: Should be "andestech,<core_name>",
"andestech,nds32v3" as fallback.
Examlpes with core_names are:
"andestech,n13"
"andestech,n15"
"andestech,d15"
"andestech,n10"
"andestech,d10"
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox