* Re: [PATCH v2 5/6] gpio-exar/8250-exar: Make set of exported GPIOs configurable
From: Jan Kiszka @ 2017-05-22 17:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
Linux Kernel Mailing List, linux-serial@vger.kernel.org,
linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75VeWqxHD7eS5FZGoyYw1B4QGMaWzVUtY-0akC764qjCjng@mail.gmail.com>
On 2017-05-22 18:33, Andy Shevchenko wrote:
> On Sun, May 21, 2017 at 2:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-05-18 19:43, Andy Shevchenko wrote:
>>> On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On the SIMATIC, IOT2040 only a single pin is exportable as GPIO, the
>
>>>> + pdata.first_gpio = first_gpio;
>>>> + pdata.ngpio = ngpio;
>>>
>>> Still thinking about device properties ("ngpios" and something like
>>> "exar8250,gpio-start").
>>
>> Changed back to properties, removing all platform data.
>>
>> But what's the purpose of prefixing the name here? This does not have
>> anything to do with device trees. It's a private parameter channel
>> between the creating device driver and the gpio driver, and there will
>> be no other bindings.
>
> To avoid potential collision with registered official property, that's
> why better to use prefix.
> (I didn't find anything like GPIO start / pin in registered
> properties, maybe there is one)
When using the "public" channel devices properties, we cannot prevent
that people set some for the device, despite it is not supposed to be
controlled by DT or ACPI. But I don't see where default properties
should come from, except via intentionally designed DTs or ACPI tables.
Anyway, I can prefix.
>
>>>> + unsigned int first_gpio;
>>>> + unsigned int ngpio;
>>>
>>> u16 ?
>
>> If we do that, then we would rather have to choose u8. But this is
>> pointless restriction. I prefer to stay with the native type.
>
> Still for properties it would be u32, wouldn't it?
>
Because properties ask for a type width, yes. I can align both to u32,
though.
Jan
^ permalink raw reply
* Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Jan Kiszka @ 2017-05-22 16:40 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Greg Kroah-Hartman, Alexandre Courbot,
Linux Kernel Mailing List, linux-serial@vger.kernel.org,
linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75Ve-Y3VrjrAOOMkEs4OkAGG-EFJY=FhGmsi8ryL-dNZXUw@mail.gmail.com>
On 2017-05-22 18:34, Andy Shevchenko wrote:
> On Mon, May 22, 2017 at 7:28 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2017-05-22 17:46, Linus Walleij wrote:
>>> On Thu, May 18, 2017 at 4:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
>> This patch has to come last. It depends on patch 5, e.g., and that has
>> dependencies as well.
>
> Btw, what about patch 1? I thought it might be shifted close to the
> end of the series.
Why? Patch 1 is the first step to fix the gpio-exar driver breakages.
But maybe I should split up those fixes and cleanups from the
preparations and enabling of the IOT2000.
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply
* Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Andy Shevchenko @ 2017-05-22 16:34 UTC (permalink / raw)
To: Jan Kiszka
Cc: Linus Walleij, Greg Kroah-Hartman, Alexandre Courbot,
Linux Kernel Mailing List, linux-serial@vger.kernel.org,
linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <5aaf3441-2cbb-fc9e-6df3-80b62fc044f8@siemens.com>
On Mon, May 22, 2017 at 7:28 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-05-22 17:46, Linus Walleij wrote:
>> On Thu, May 18, 2017 at 4:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> This patch has to come last. It depends on patch 5, e.g., and that has
> dependencies as well.
Btw, what about patch 1? I thought it might be shifted close to the
end of the series.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 5/6] gpio-exar/8250-exar: Make set of exported GPIOs configurable
From: Andy Shevchenko @ 2017-05-22 16:33 UTC (permalink / raw)
To: Jan Kiszka
Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
Linux Kernel Mailing List, linux-serial@vger.kernel.org,
linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <83279eaf-69aa-333c-58b8-202d221375d3@siemens.com>
On Sun, May 21, 2017 at 2:43 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-05-18 19:43, Andy Shevchenko wrote:
>> On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On the SIMATIC, IOT2040 only a single pin is exportable as GPIO, the
>>> + pdata.first_gpio = first_gpio;
>>> + pdata.ngpio = ngpio;
>>
>> Still thinking about device properties ("ngpios" and something like
>> "exar8250,gpio-start").
>
> Changed back to properties, removing all platform data.
>
> But what's the purpose of prefixing the name here? This does not have
> anything to do with device trees. It's a private parameter channel
> between the creating device driver and the gpio driver, and there will
> be no other bindings.
To avoid potential collision with registered official property, that's
why better to use prefix.
(I didn't find anything like GPIO start / pin in registered
properties, maybe there is one)
>>> + unsigned int first_gpio;
>>> + unsigned int ngpio;
>>
>> u16 ?
> If we do that, then we would rather have to choose u8. But this is
> pointless restriction. I prefer to stay with the native type.
Still for properties it would be u32, wouldn't it?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Jan Kiszka @ 2017-05-22 16:28 UTC (permalink / raw)
To: Linus Walleij
Cc: Greg Kroah-Hartman, Alexandre Courbot, Linux Kernel Mailing List,
linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <CACRpkdbeXdOWiYpmtQTU1B66mxq1gGpukZWvUepgzNk5LLa1PQ@mail.gmail.com>
On 2017-05-22 17:46, Linus Walleij wrote:
> On Thu, May 18, 2017 at 4:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
>> This implements the setup of RS232 and the switch-over to RS485 or RS422
>> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
>> to switch between the different modes. The external logic is controlled
>> via MPIO pins of the EXAR controller.
>>
>> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
>> an LED.
>>
>> As the XR17V352 used on the IOT2040 is not equipped with an external
>> EEPROM, it cannot present itself as IOT2040-variant via subvendor/
>> subdevice IDs. Thus, we have to check via DMI for the target platform.
>>
>> Co-developed with Sascha Weisenberger.
>>
>> Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>
> Will this thing collide with your fixups so that Greg should take all
> exar patches or can this be applied to the serial tree orthogonally?
This patch has to come last. It depends on patch 5, e.g., and that has
dependencies as well.
FWIW, v3 of this series is coming, just requires another testing round.
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply
* Re: gpio-exar: Why filtering out Commtech devices?
From: Jan Kiszka @ 2017-05-22 16:24 UTC (permalink / raw)
To: Technical Support
Cc: Sudip Mukherjee, Linux Kernel Mailing List, linux-serial,
linux-gpio
In-Reply-To: <CAPpjZG9MGDzS0Bu2c6M-Pob5JkzeAptDu8QNnDJETinCenee0w@mail.gmail.com>
On 2017-05-22 17:17, Technical Support wrote:
> Hello,
>
> The Exar MPIO pins are used by our device driver to control features of
> the line driver and can't be used as GPIO pins. I agree, the condition
> can be moved to 8250_exar prior to a platform device being created for
> the gpio_exar driver.
>
Thanks a lot for the feedback! I will send a refactoring patch.
Jan
> Regards,
> Landon Unruh
>
> Commtech, Inc.
> Voice: 316-636-1131
> http://www.fastcomproducts.com
>
>
>
>
> On Mon, May 22, 2017 at 12:51 AM, Jan Kiszka <jan.kiszka@siemens.com
> <mailto:jan.kiszka@siemens.com>> wrote:
>
> On 2017-05-22 07:48, Jan Kiszka wrote:
> > On 2017-05-21 22:08, Sudip Mukherjee wrote:
> >> Hi Jan,
> >>
> >> On 21/05/17 12:46, Jan Kiszka wrote:
> >>> Hi Sudip,
> >>>
> >>> why do we carry
> >>>
> >>> if (pcidev->vendor != PCI_VENDOR_ID_EXAR)
> >>> return -ENODEV;
> >>>
> >>> in gpio_exar_probe? This effectively prevents that
> >>>
> >>> EXAR_DEVICE(COMMTECH, COMMTECH_4222PCIE, pbn_exar_XR17V35x),
> >>> EXAR_DEVICE(COMMTECH, COMMTECH_4224PCIE, pbn_exar_XR17V35x),
> >>> EXAR_DEVICE(COMMTECH, COMMTECH_4228PCIE, pbn_exar_XR17V35x),
> >>>
> >>> gain GPIO support. Do those devices lack access to the pins? Or can we
> >>> drop the filter. I don't have access to those devices, just wondering
> >>> because the code is not explaining the reason.
> >>
> >> Same here. I do not have these devices and have no idea if they support
> >> the gpio pins or not.
> >>
> >> Adding Matt Schulte in the Cc list, maybe he can comment.
> >>
> >>
> >
> > If we need to keep the condition, it should be moved over to 8250_exar:
> > there is no point in creating the platform device at all then. But let's
> > wait for Matt's comment.
>
> Unfortunately, his account is no longer existing. Is there anyone else
> we can ask?
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA ITP SES-DE
> Corporate Competence Center Embedded Linux
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-serial" in
> the body of a message to majordomo@vger.kernel.org
> <mailto:majordomo@vger.kernel.org>
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> <http://vger.kernel.org/majordomo-info.html>
>
>
^ permalink raw reply
* Re: [PATCH v2 6/6] serial: exar: Add support for IOT2040 device
From: Linus Walleij @ 2017-05-22 15:46 UTC (permalink / raw)
To: Jan Kiszka
Cc: Greg Kroah-Hartman, Alexandre Courbot, Linux Kernel Mailing List,
linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <f0d2245527bc5700be7a4ac707a81db798cb5519.1495119548.git.jan.kiszka@siemens.com>
On Thu, May 18, 2017 at 4:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> This implements the setup of RS232 and the switch-over to RS485 or RS422
> for the Siemens IOT2040. That uses an EXAR XR17V352 with external logic
> to switch between the different modes. The external logic is controlled
> via MPIO pins of the EXAR controller.
>
> Only pin 10 can be exported as GPIO on the IOT2040. It is connected to
> an LED.
>
> As the XR17V352 used on the IOT2040 is not equipped with an external
> EEPROM, it cannot present itself as IOT2040-variant via subvendor/
> subdevice IDs. Thus, we have to check via DMI for the target platform.
>
> Co-developed with Sascha Weisenberger.
>
> Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Will this thing collide with your fixups so that Greg should take all
exar patches or can this be applied to the serial tree orthogonally?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 5/6] gpio-exar/8250-exar: Make set of exported GPIOs configurable
From: Linus Walleij @ 2017-05-22 15:44 UTC (permalink / raw)
To: Jan Kiszka
Cc: Greg Kroah-Hartman, Alexandre Courbot, Linux Kernel Mailing List,
linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <357209721396d57cb871ae69d72308b1a7c9cbd7.1495119548.git.jan.kiszka@siemens.com>
On Thu, May 18, 2017 at 4:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On the SIMATIC, IOT2040 only a single pin is exportable as GPIO, the
> rest is required to operate the UART. To allow modeling this case,
> expand the platform device data structure to specify a (consecutive) pin
> subset for exporting by the gpio-exar driver.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
I will need Greg's ACK on this too, but it seems
you are respinning it.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 1/6] gpio: exar: Fix passing in of parent PCI device
From: Linus Walleij @ 2017-05-22 15:43 UTC (permalink / raw)
To: Jan Kiszka, Greg Kroah-Hartman
Cc: Andy Shevchenko, Linux Kernel Mailing List,
linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <7d5b0099-b4d9-881d-fc63-0c7f8229e096@siemens.com>
On Sun, May 21, 2017 at 1:44 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-05-18 19:14, Andy Shevchenko wrote:
>> On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> This fixes reloading of the GPIO driver for the same platform device
>>> instance as created by the exar UART driver: First of all, the driver
>>> sets drvdata to its own value during probing and does not restore the
>>> original value on exit. But this won't help anyway as the core clears
>>> drvdata after the driver left.
>>>
>>> Use stable platform_data instead.
>>
>> Okay, basically what we are trying to do here is to reinvent part of
>> MFD framework.
>>
>> I'd like to hear Linus' and others opinions if it worth to use it instead.
>
> I've looked into MFD modeling, but it would only make sense if we break
> up the exar driver, change its xr17v35x part into a platform device and
> create a dual-cell MFD for the PCI device. I don't think that would be
> beneficial here. There are also dependencies between the UART part and
> the MPIOs, specifically during init. All that would create a lot of
> churn to the existing exar code.
>
> I'm now passing the parent reference via device.parent instead of using
> platform data.
Actually I am pretty much OK with either, there are gray areas in
the device model and so it has to be sometimes.
I'd just like Greg's ACK on this so I can merge the whole series
through the GPIO tree.
Incidentally, he is the device model maintainer so he might have
some comments. Or be as tolerant as me. I don't know.
Greg?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 3/8] gpio: exar: Allocate resources on behalf of the platform device
From: Linus Walleij @ 2017-05-22 15:39 UTC (permalink / raw)
To: Jan Kiszka
Cc: Greg Kroah-Hartman, Alexandre Courbot, Linux Kernel Mailing List,
linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <39cc60caa4f5c38d1449590b6c67cf026184306b.1494660546.git.jan.kiszka@siemens.com>
On Sat, May 13, 2017 at 9:29 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Do not allocate resources on behalf of the parent device but on our own.
> Otherwise, cleanup does not properly work if gpio-exar is removed but
> not the parent device.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/8] serial: exar: Preconfigure xr17v35x MPIOs as output
From: Linus Walleij @ 2017-05-22 15:38 UTC (permalink / raw)
To: Jan Kiszka
Cc: Greg Kroah-Hartman, Alexandre Courbot, Linux Kernel Mailing List,
linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <6155891d19531be018d30fd55ffd6d42493af8c1.1494660546.git.jan.kiszka@siemens.com>
On Sat, May 13, 2017 at 9:28 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> This is the safe default for GPIOs with unknown external wiring.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] serial: meson: hide an unused function
From: Arnd Bergmann @ 2017-05-22 13:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arnd Bergmann, Martin Blumenstingl, Kevin Hilman, linux-kernel,
linux-serial, Jiri Slaby, Carlo Caione, linux-amlogic,
linux-arm-kernel, Heiner Kallweit
The newly added meson_uart_enable_tx_engine function is only called
from the console setup, not the runtime uart, which has an open-coded
version of the same register access. This produces a harmless warning
when the console code is disabled:
drivers/tty/serial/meson_uart.c:127:13: error: 'meson_uart_enable_tx_engine' defined but not used [-Werror=unused-function]
Let's move the function inside of the #ifdef to avoid the warning.
Fixes: ba50f1df13c8 ("serial: meson: remove unneeded variable assignment in meson_serial_port_write")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/tty/serial/meson_uart.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 082e038e67f8..c0e34dabadd8 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -124,15 +124,6 @@ static void meson_uart_stop_rx(struct uart_port *port)
writel(val, port->membase + AML_UART_CONTROL);
}
-static void meson_uart_enable_tx_engine(struct uart_port *port)
-{
- u32 val;
-
- val = readl(port->membase + AML_UART_CONTROL);
- val |= AML_UART_TX_EN;
- writel(val, port->membase + AML_UART_CONTROL);
-}
-
static void meson_uart_shutdown(struct uart_port *port)
{
unsigned long flags;
@@ -451,6 +442,14 @@ static struct uart_ops meson_uart_ops = {
};
#ifdef CONFIG_SERIAL_MESON_CONSOLE
+static void meson_uart_enable_tx_engine(struct uart_port *port)
+{
+ u32 val;
+
+ val = readl(port->membase + AML_UART_CONTROL);
+ val |= AML_UART_TX_EN;
+ writel(val, port->membase + AML_UART_CONTROL);
+}
static void meson_console_putchar(struct uart_port *port, int ch)
{
--
2.9.0
^ permalink raw reply related
* [PATCH] serial: sh-sci: Update warning message in sci_request_dma_chan()
From: Geert Uytterhoeven @ 2017-05-22 13:15 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Laurent Pinchart, linux-serial, linux-kernel, Geert Uytterhoeven
The commit below changed a function call from
dma_request_slave_channel_compat() to dma_request_slave_channel(), but
forgot to update the printed failure message.
Fixes: 219fb0c1436e4893 ("serial: sh-sci: Remove the platform data dma slave rx/tx channel IDs")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/sh-sci.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 21b06cf1f15bc642..d36d735cd0099e83 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1450,8 +1450,7 @@ static struct dma_chan *sci_request_dma_chan(struct uart_port *port,
chan = dma_request_slave_channel(port->dev,
dir == DMA_MEM_TO_DEV ? "tx" : "rx");
if (!chan) {
- dev_warn(port->dev,
- "dma_request_slave_channel_compat failed\n");
+ dev_warn(port->dev, "dma_request_slave_channel failed\n");
return NULL;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts
From: Michal Simek @ 2017-05-22 7:02 UTC (permalink / raw)
To: Alan Cox, Sam Povilus
Cc: gregkh, jslaby, michal.simek, soren.brinkmann, linux-serial,
linux-arm-kernel, linux-kernel
In-Reply-To: <20170520172008.5a979e8a@alans-desktop>
Hi Alan,
On 20.5.2017 18:20, Alan Cox wrote:
> On Fri, 19 May 2017 20:21:54 -0600
> Sam Povilus <kernel.development@povil.us> wrote:
>
>> The number of xilinx ps uart should be set by a kernel parameter instead of
>> using a #define. This allows the user to set the number of xilinx ps uart
>> using only kconfig and not modifying kernel source.
>>
>> The ps uart is used in Xilnx Zynq chips usually in quantities maxing at
>> two, but there may be other chips that use more in the future or that I
>> don't know about.
>
> If it maxes at two then just set it to two. If in in future it maxes at
> lots then when it's going to happen change the code to do dynamic
> allocation and test it versus actual hardware.
>
> Otherwise you break some of the basic ideas of having one kernel for many
> systems.
We have in soc vendor tree similar patch but the reason is different.
tty: serial: Added a CONFIG_SERIAL_XILINX_NR_UARTS option.
This patch Adds CONFIG_SERIAL_XILINX_NR_UARTS option to allow
the user to provide the Max number of uart ports information.
If multiple cards (or) PL UARTS are present, the default limit
of 2 ports should be increased.
I haven't checked all drivers but in our case we have added this as
quick fix for scenarios where you use serial aliases where alias is
pointed to serial2 or more.
In cdns_uart_init() cdns_uart_uart_driver is passed which contains .nr
which is required to be passed.
What's the best driver to look at dynamic allocation?
Thanks,
Michal
^ permalink raw reply
* Re: gpio-exar: Why filtering out Commtech devices?
From: Jan Kiszka @ 2017-05-22 5:51 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: Linux Kernel Mailing List, linux-serial, linux-gpio
In-Reply-To: <c7b03839-c2ab-fa3e-3af6-5d4678fa3bbd@siemens.com>
On 2017-05-22 07:48, Jan Kiszka wrote:
> On 2017-05-21 22:08, Sudip Mukherjee wrote:
>> Hi Jan,
>>
>> On 21/05/17 12:46, Jan Kiszka wrote:
>>> Hi Sudip,
>>>
>>> why do we carry
>>>
>>> if (pcidev->vendor != PCI_VENDOR_ID_EXAR)
>>> return -ENODEV;
>>>
>>> in gpio_exar_probe? This effectively prevents that
>>>
>>> EXAR_DEVICE(COMMTECH, COMMTECH_4222PCIE, pbn_exar_XR17V35x),
>>> EXAR_DEVICE(COMMTECH, COMMTECH_4224PCIE, pbn_exar_XR17V35x),
>>> EXAR_DEVICE(COMMTECH, COMMTECH_4228PCIE, pbn_exar_XR17V35x),
>>>
>>> gain GPIO support. Do those devices lack access to the pins? Or can we
>>> drop the filter. I don't have access to those devices, just wondering
>>> because the code is not explaining the reason.
>>
>> Same here. I do not have these devices and have no idea if they support
>> the gpio pins or not.
>>
>> Adding Matt Schulte in the Cc list, maybe he can comment.
>>
>>
>
> If we need to keep the condition, it should be moved over to 8250_exar:
> there is no point in creating the platform device at all then. But let's
> wait for Matt's comment.
Unfortunately, his account is no longer existing. Is there anyone else
we can ask?
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply
* Re: gpio-exar: Why filtering out Commtech devices?
From: Jan Kiszka @ 2017-05-22 5:48 UTC (permalink / raw)
To: Sudip Mukherjee, Matt Schulte
Cc: Linux Kernel Mailing List, linux-serial, linux-gpio
In-Reply-To: <7a25751e-931f-b713-dcdd-7a7483aae115@codethink.co.uk>
On 2017-05-21 22:08, Sudip Mukherjee wrote:
> Hi Jan,
>
> On 21/05/17 12:46, Jan Kiszka wrote:
>> Hi Sudip,
>>
>> why do we carry
>>
>> if (pcidev->vendor != PCI_VENDOR_ID_EXAR)
>> return -ENODEV;
>>
>> in gpio_exar_probe? This effectively prevents that
>>
>> EXAR_DEVICE(COMMTECH, COMMTECH_4222PCIE, pbn_exar_XR17V35x),
>> EXAR_DEVICE(COMMTECH, COMMTECH_4224PCIE, pbn_exar_XR17V35x),
>> EXAR_DEVICE(COMMTECH, COMMTECH_4228PCIE, pbn_exar_XR17V35x),
>>
>> gain GPIO support. Do those devices lack access to the pins? Or can we
>> drop the filter. I don't have access to those devices, just wondering
>> because the code is not explaining the reason.
>
> Same here. I do not have these devices and have no idea if they support
> the gpio pins or not.
>
> Adding Matt Schulte in the Cc list, maybe he can comment.
>
>
If we need to keep the condition, it should be moved over to 8250_exar:
there is no point in creating the platform device at all then. But let's
wait for Matt's comment.
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply
* Re: gpio-exar: Why filtering out Commtech devices?
From: Sudip Mukherjee @ 2017-05-21 20:08 UTC (permalink / raw)
To: Jan Kiszka, Matt Schulte
Cc: Linux Kernel Mailing List, linux-serial, linux-gpio
In-Reply-To: <18f4b5cb-cba0-aab4-7958-84a5b8d28935@siemens.com>
Hi Jan,
On 21/05/17 12:46, Jan Kiszka wrote:
> Hi Sudip,
>
> why do we carry
>
> if (pcidev->vendor != PCI_VENDOR_ID_EXAR)
> return -ENODEV;
>
> in gpio_exar_probe? This effectively prevents that
>
> EXAR_DEVICE(COMMTECH, COMMTECH_4222PCIE, pbn_exar_XR17V35x),
> EXAR_DEVICE(COMMTECH, COMMTECH_4224PCIE, pbn_exar_XR17V35x),
> EXAR_DEVICE(COMMTECH, COMMTECH_4228PCIE, pbn_exar_XR17V35x),
>
> gain GPIO support. Do those devices lack access to the pins? Or can we
> drop the filter. I don't have access to those devices, just wondering
> because the code is not explaining the reason.
Same here. I do not have these devices and have no idea if they support
the gpio pins or not.
Adding Matt Schulte in the Cc list, maybe he can comment.
--
Regards
Sudip
^ permalink raw reply
* gpio-exar: Why filtering out Commtech devices?
From: Jan Kiszka @ 2017-05-21 11:46 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: Linux Kernel Mailing List, linux-serial, linux-gpio
Hi Sudip,
why do we carry
if (pcidev->vendor != PCI_VENDOR_ID_EXAR)
return -ENODEV;
in gpio_exar_probe? This effectively prevents that
EXAR_DEVICE(COMMTECH, COMMTECH_4222PCIE, pbn_exar_XR17V35x),
EXAR_DEVICE(COMMTECH, COMMTECH_4224PCIE, pbn_exar_XR17V35x),
EXAR_DEVICE(COMMTECH, COMMTECH_4228PCIE, pbn_exar_XR17V35x),
gain GPIO support. Do those devices lack access to the pins? Or can we
drop the filter. I don't have access to those devices, just wondering
because the code is not explaining the reason.
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply
* Re: [PATCH v2 1/6] gpio: exar: Fix passing in of parent PCI device
From: Jan Kiszka @ 2017-05-21 11:44 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
Linux Kernel Mailing List, linux-serial@vger.kernel.org,
linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75VcRRxgzoPio2zWrR5rE_ac9uQjcphcXTQF6TrM13qxe+A@mail.gmail.com>
On 2017-05-18 19:14, Andy Shevchenko wrote:
> On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> This fixes reloading of the GPIO driver for the same platform device
>> instance as created by the exar UART driver: First of all, the driver
>> sets drvdata to its own value during probing and does not restore the
>> original value on exit. But this won't help anyway as the core clears
>> drvdata after the driver left.
>>
>> Use stable platform_data instead.
>
> Okay, basically what we are trying to do here is to reinvent part of
> MFD framework.
>
> I'd like to hear Linus' and others opinions if it worth to use it instead.
>
I've looked into MFD modeling, but it would only make sense if we break
up the exar driver, change its xr17v35x part into a platform device and
create a dual-cell MFD for the PCI device. I don't think that would be
beneficial here. There are also dependencies between the UART part and
the MPIOs, specifically during init. All that would create a lot of
churn to the existing exar code.
I'm now passing the parent reference via device.parent instead of using
platform data.
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply
* Re: [PATCH v2 5/6] gpio-exar/8250-exar: Make set of exported GPIOs configurable
From: Jan Kiszka @ 2017-05-21 11:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot,
Linux Kernel Mailing List, linux-serial@vger.kernel.org,
linux-gpio@vger.kernel.org, Sudip Mukherjee, Sascha Weisenberger
In-Reply-To: <CAHp75VcOHq05ZHc=arV9URQ9C6_guLiERB=FQ7FQgBTBYNrvPQ@mail.gmail.com>
On 2017-05-18 19:43, Andy Shevchenko wrote:
> On Thu, May 18, 2017 at 5:59 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On the SIMATIC, IOT2040 only a single pin is exportable as GPIO, the
>> rest is required to operate the UART. To allow modeling this case,
>> expand the platform device data structure to specify a (consecutive) pin
>> subset for exporting by the gpio-exar driver.
>
>> + unsigned int first_gpio;
>
> Perhaps pin?
> Or shift?
>
> Because first_gpio a bit confusing with Linux side of GPIO.
Ack, going for "pin".
>
>> - unsigned int bank = offset / 8;
>> - unsigned int addr;
>> + struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
>> + unsigned int bank, addr;
>>
>> + offset += exar_gpio->first_gpio;
>> + bank = offset / 8;
>
> Can't we instead do something like the following:
>
> struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
> unsigned int bank = (offset + exar_gpio->pin) / 8;
> unsigned int line = (offset + exar_gpio->pin) % 8;
>
OK, I'm using this pattern now:
unsigned int addr = (offset + exar_gpio->first_pin) / 8 ?
EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
unsigned int bit = (offset + exar_gpio->first_pin) % 8;
>
>> + pdata.first_gpio = first_gpio;
>> + pdata.ngpio = ngpio;
>
> Still thinking about device properties ("ngpios" and something like
> "exar8250,gpio-start").
Changed back to properties, removing all platform data.
But what's the purpose of prefixing the name here? This does not have
anything to do with device trees. It's a private parameter channel
between the creating device driver and the gpio driver, and there will
be no other bindings.
>
>> + unsigned int first_gpio;
>> + unsigned int ngpio;
>
> u16 ?
>
If we do that, then we would rather have to choose u8. But this is
pointless restriction. I prefer to stay with the native type.
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply
* Re: [PATCH] serial: of_serial: remove NULL pointer check for clk_disable_unprepare
From: Masahiro Yamada @ 2017-05-20 19:04 UTC (permalink / raw)
To: linux-serial
Cc: Masahiro Yamada, Arnd Bergmann, Robert Jarzmik, Sekhar Nori,
Jiri Slaby, Linux Kernel Mailing List, David Lechner, Thor Thayer,
Greg Kroah-Hartman
In-Reply-To: <1495302286-1503-1-git-send-email-yamada.masahiro@socionext.com>
2017-05-21 2:44 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> After long term efforts of fixing non-common clock implementations,
> clk_disable() is a no-op for a NULL pointer input, and this is now
> tree-wide consistent.
>
> All clock consumers can safely call clk_disable(_unprepare) without
> NULL pointer check.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Sorry, I retract this patch.
Krzysztof pointed out
cleanups only for clk_disable_unprepare() will lose the code symmetry.
NULL pointer checks for clk_prepare_enable() should be
removed to keep the code symmetrical.
This is possible for common-clock framework because
clk_prepare_enable() is also a no-op for a NULL clk input.
But it is not necessarily true for non-common clock implementations.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH] serial: of_serial: remove NULL pointer check for clk_disable_unprepare
From: Masahiro Yamada @ 2017-05-20 17:44 UTC (permalink / raw)
To: linux-serial
Cc: Masahiro Yamada, Arnd Bergmann, Robert Jarzmik, Sekhar Nori,
Jiri Slaby, linux-kernel, David Lechner, Thor Thayer,
Greg Kroah-Hartman
After long term efforts of fixing non-common clock implementations,
clk_disable() is a no-op for a NULL pointer input, and this is now
tree-wide consistent.
All clock consumers can safely call clk_disable(_unprepare) without
NULL pointer check.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/tty/serial/8250/8250_of.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 1cbadaf..89ba2b5 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -159,8 +159,8 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
return 0;
out:
- if (info->clk)
- clk_disable_unprepare(info->clk);
+ clk_disable_unprepare(info->clk);
+
return ret;
}
@@ -229,8 +229,8 @@ static int of_platform_serial_remove(struct platform_device *ofdev)
serial8250_unregister_port(info->line);
- if (info->clk)
- clk_disable_unprepare(info->clk);
+ clk_disable_unprepare(info->clk);
+
kfree(info);
return 0;
}
@@ -244,7 +244,7 @@ static int of_serial_suspend(struct device *dev)
serial8250_suspend_port(info->line);
- if (info->clk && (!uart_console(port) || console_suspend_enabled))
+ if (!uart_console(port) || console_suspend_enabled)
clk_disable_unprepare(info->clk);
return 0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts
From: Alan Cox @ 2017-05-20 16:20 UTC (permalink / raw)
To: Sam Povilus
Cc: gregkh, michal.simek, linux-kernel, soren.brinkmann, linux-serial,
jslaby, linux-arm-kernel
In-Reply-To: <20170520022154.6766-1-kernel.development@povil.us>
On Fri, 19 May 2017 20:21:54 -0600
Sam Povilus <kernel.development@povil.us> wrote:
> The number of xilinx ps uart should be set by a kernel parameter instead of
> using a #define. This allows the user to set the number of xilinx ps uart
> using only kconfig and not modifying kernel source.
>
> The ps uart is used in Xilnx Zynq chips usually in quantities maxing at
> two, but there may be other chips that use more in the future or that I
> don't know about.
If it maxes at two then just set it to two. If in in future it maxes at
lots then when it's going to happen change the code to do dynamic
allocation and test it versus actual hardware.
Otherwise you break some of the basic ideas of having one kernel for many
systems.
Alan
^ permalink raw reply
* [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts
From: Sam Povilus @ 2017-05-20 2:21 UTC (permalink / raw)
To: gregkh, jslaby, michal.simek, soren.brinkmann, linux-serial,
linux-arm-kernel, linux-kernel
Cc: Sam Povilus
The number of xilinx ps uart should be set by a kernel parameter instead of
using a #define. This allows the user to set the number of xilinx ps uart
using only kconfig and not modifying kernel source.
The ps uart is used in Xilnx Zynq chips usually in quantities maxing at
two, but there may be other chips that use more in the future or that I
don't know about.
Signed-off-by: Sam Povilus <kernel.development@povil.us>
---
drivers/tty/serial/Kconfig | 9 +++++++++
drivers/tty/serial/xilinx_uartps.c | 2 +-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 5c8850f7a2a0..fef25f17a4cc 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1461,6 +1461,15 @@ config SERIAL_XILINX_PS_UART_CONSOLE
help
Enable a Cadence UART port to be the system console.
+config SERIAL_XILINX_PS_UART_NR_UARTS
+ int "Maximum number of Cadence UART ports"
+ depends on CONFIG_SERIAL_XILINX_NR_UARTS
+ range 1 64
+ default 2
+ help
+ Set this to the number of Cadence UARTS in your system, or the number
+ you think you might implement.
+
config SERIAL_AR933X
tristate "AR933X serial port support"
depends on HAVE_CLK && SOC_AR933X
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index c0539950f8d7..a2c51c35da65 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -36,7 +36,7 @@
#define CDNS_UART_NAME "xuartps"
#define CDNS_UART_MAJOR 0 /* use dynamic node allocation */
#define CDNS_UART_MINOR 0 /* works best with devtmpfs */
-#define CDNS_UART_NR_PORTS 2
+#define CDNS_UART_NR_PORTS CONFIG_SERIAL_XILINX_NR_UARTS
#define CDNS_UART_FIFO_SIZE 64 /* FIFO size */
#define CDNS_UART_REGISTER_SPACE 0x1000
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] serial: 8250: Add CAP_MINI, set for bcm2835aux
From: Eric Anholt @ 2017-05-19 18:09 UTC (permalink / raw)
To: Phil Elwell, Greg Kroah-Hartman, Stefan Wahren, linux-serial,
linux-rpi-kernel, linux-arm-kernel
In-Reply-To: <8d9c605a-bff7-607c-2c5b-4ec75e68ac2d@raspberrypi.org>
[-- Attachment #1.1: Type: text/plain, Size: 820 bytes --]
Phil Elwell <phil@raspberrypi.org> writes:
> The AUX/mini-UART in the BCM2835 family of procesors is a cut-down
> 8250 clone. In particular it is lacking support for the following
> features: CSTOPB PARENB PARODD CMSPAR CS5 CS6
>
> Add a new capability (UART_CAP_MINI) that exposes the restrictions to
> the user of the termios API by turning off the unsupported features in
> the request.
>
> N.B. It is almost possible to automatically discover the missing
> features by reading back the LCR register, but the CSIZE bits don't
> cooperate (contrary to the documentation, both bits are significant,
> but CS5 and CS6 are mapped to CS7) and the code is much longer.
>
> See: https://github.com/raspberrypi/linux/issues/1561
>
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
Acked-by: Eric Anholt <eric@anholt.net>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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