* spin_lock instead of spin_lock_irqsave in imx uart interuput
From: QIANJUN @ 2018-08-26 15:00 UTC (permalink / raw)
To: gregkh, jslaby, linux-serial, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 61 bytes --]
spin_lock instead of spin_lock_irqsave in imx uart interuput
[-- Attachment #2: 0001-before-the-programe-into-the-uart-interuput-s-handle.patch --]
[-- Type: text/x-diff, Size: 2581 bytes --]
>From 6580790d7be51a78d1376b90df49b04019a1f944 Mon Sep 17 00:00:00 2001
From: jun qian <hangdianqj@163.com>
Date: Sun, 26 Aug 2018 06:58:35 -0700
Subject: [PATCH] before the programe into the uart interuput's handler, the
system has already disabled the local cpu interuput. so the spin_lock
interface is More suitable for here.
Signed-off-by: jun qian <hangdianqj@163.com>
---
drivers/tty/serial/imx.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 239c0fa2e981..3069ee93583e 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -706,27 +706,25 @@ static irqreturn_t imx_uart_rtsint(int irq, void *dev_id)
{
struct imx_port *sport = dev_id;
u32 usr1;
- unsigned long flags;
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
imx_uart_writel(sport, USR1_RTSD, USR1);
usr1 = imx_uart_readl(sport, USR1) & USR1_RTSS;
uart_handle_cts_change(&sport->port, !!usr1);
wake_up_interruptible(&sport->port.state->port.delta_msr_wait);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
return IRQ_HANDLED;
}
static irqreturn_t imx_uart_txint(int irq, void *dev_id)
{
struct imx_port *sport = dev_id;
- unsigned long flags;
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
imx_uart_transmit_buffer(sport);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
return IRQ_HANDLED;
}
@@ -735,9 +733,8 @@ static irqreturn_t imx_uart_rxint(int irq, void *dev_id)
struct imx_port *sport = dev_id;
unsigned int rx, flg, ignored = 0;
struct tty_port *port = &sport->port.state->port;
- unsigned long flags;
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
while (imx_uart_readl(sport, USR2) & USR2_RDR) {
u32 usr2;
@@ -797,7 +794,7 @@ static irqreturn_t imx_uart_rxint(int irq, void *dev_id)
}
out:
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
tty_flip_buffer_push(port);
return IRQ_HANDLED;
}
@@ -903,13 +900,11 @@ static irqreturn_t imx_uart_int(int irq, void *dev_id)
}
if (usr1 & USR1_DTRD) {
- unsigned long flags;
-
imx_uart_writel(sport, USR1_DTRD, USR1);
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
imx_uart_mctrl_check(sport);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
ret = IRQ_HANDLED;
}
--
2.17.1
^ permalink raw reply related
* Re: spin_lock instead of spin_lock_irqsave in imx uart interuput
From: Greg KH @ 2018-08-26 17:04 UTC (permalink / raw)
To: QIANJUN; +Cc: jslaby, linux-serial, linux-kernel
In-Reply-To: <20180826150042.GA7972@ubuntu>
On Sun, Aug 26, 2018 at 08:00:42AM -0700, QIANJUN wrote:
> spin_lock instead of spin_lock_irqsave in imx uart interuput
What does that mean?
> >From 6580790d7be51a78d1376b90df49b04019a1f944 Mon Sep 17 00:00:00 2001
> From: jun qian <hangdianqj@163.com>
> Date: Sun, 26 Aug 2018 06:58:35 -0700
> Subject: [PATCH] before the programe into the uart interuput's handler, the
> system has already disabled the local cpu interuput. so the spin_lock
> interface is More suitable for here.
Why is this here? The changelog text belongs in the changelog, not in
the subject line :(
can you fix this all up and resend?
thanks,
greg k-h
^ permalink raw reply
* [PATCH] serial: mvebu-uart: Fix reporting of effective CSIZE to userspace
From: Jan Kiszka @ 2018-08-26 17:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial
Cc: Linux Kernel Mailing List, Allen Yan, Miquel Raynal, Marc Zyngier
From: Jan Kiszka <jan.kiszka@siemens.com>
Apparently, this driver (or the hardware) does not support character
length settings. It's apparently running in 8-bit mode, but it makes
userspace believe it's in 5-bit mode. That makes tcsetattr with CS8
incorrectly fail, breaking e.g. getty from busybox, thus the login shell
on ttyMVx.
Fix by hard-wiring CS8 into c_cflag.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
It's a bit of a shame that still maturing drivers can break userspace
that easily and subtly. I had to debug my way from old but working
buildroot to busybox, libc (tcsetattr) and then finally this driver.
This wasn't the first bug of this kind, and maybe it's not the last (I
didn't check all that termio flags). Could the kernel help in some way
with sanity checks or sane defaults driver have to make insane
willingly?
drivers/tty/serial/mvebu-uart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index d04b5eeea3c6..170e446a2f62 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -511,6 +511,7 @@ static void mvebu_uart_set_termios(struct uart_port *port,
termios->c_iflag |= old->c_iflag & ~(INPCK | IGNPAR);
termios->c_cflag &= CREAD | CBAUD;
termios->c_cflag |= old->c_cflag & ~(CREAD | CBAUD);
+ termios->c_cflag |= CS8;
}
spin_unlock_irqrestore(&port->lock, flags);
^ permalink raw reply related
* Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
From: Kishon Vijay Abraham I @ 2018-08-27 3:02 UTC (permalink / raw)
To: Tony Lindgren
Cc: Rob Herring, Nishanth Menon, Santosh Shilimkar, Will Deacon,
Catalin Marinas, Greg Kroah-Hartman, Mark Rutland,
open list:SERIAL DRIVERS, linux-kernel@vger.kernel.org,
devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Vignesh R, Tero Kristo, Russell King, Sudeep Holla
In-Reply-To: <20180820143153.GD7523@atomide.com>
Hi Tony,
On Monday 20 August 2018 08:01 PM, Tony Lindgren wrote:
> * Kishon Vijay Abraham I <kishon@ti.com> [180808 06:35]:
>> On Tuesday 05 June 2018 07:35 PM, Rob Herring wrote:
>>> Really need 64-bit addresses and sizes? Use ranges to limit the
>>> address space if possible.
>>
>> We now have address-cells as <1>,
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am65.dtsi#n49
>>
>> However each PCIe instance has 2 data regions and one of the regions
>> (PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT1/PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT1 specified
>> in the "MAIN Domain Memory Map" table of TRM http://www.ti.com/lit/pdf/spruid7)
>> is above the 32bit region and requires 2 cells to specify the start address.
>> This region is used to access MEM_SPACE of PCIe endpoint when operating in root
>> complex mode and access memory of PCI root complex when operating in endpoint mode.
>>
>> In order to describe this, should we change the address-cells back to <2> or do
>> you suggest any other alternatives?
>
> It's probably best to have the top level cbass interconnect use
> #size-cells = <2> and then have it's child interconnects have
> #size-cells = <1> if they don't need ranges above 4GB.
PCIe has a region starting at 0x40_00000000 and size 4GB. We need 2 address
cells and 2 size cells to describe this no?
>
> BTW, what's the difference between all these three similar PCIE
> ranges?
>
> PCIE0_CORE_CORE_DAT_SLV_PCIE_CORE 0x0005500000 0x0005600000 1 MB
> PCIE1_CORE_CORE_DAT_SLV_PCIE_CORE 0x0005600000 0x0005700000 1 MB
This is the register space for the two instances of PCIe controller.
>
> PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT0 0x0010000000 0x0018000000 128 MB
> PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT0 0x0018000000 0x0020000000 128 MB
>
> PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT1 0x4000000000 0x4100000000 4 GB
> PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT1 0x4100000000 0x4200000000 4 GB
The above are regions which can be used by CPU/DMA to access the PCIe address
space. The mapping from the above regions to the PCIe address space will be
programmed in the PCIe controller.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH] serial: mvebu-uart: Fix reporting of effective CSIZE to userspace
From: Jan Kiszka @ 2018-08-27 6:25 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial
Cc: Linux Kernel Mailing List, Allen Yan, Miquel Raynal, Marc Zyngier
In-Reply-To: <69ad54af-8732-b78a-7e0b-482bfcd6d25f@web.de>
On 2018-08-26 19:49, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Apparently, this driver (or the hardware) does not support character
> length settings. It's apparently running in 8-bit mode, but it makes
> userspace believe it's in 5-bit mode. That makes tcsetattr with CS8
> incorrectly fail, breaking e.g. getty from busybox, thus the login shell
> on ttyMVx.
>
> Fix by hard-wiring CS8 into c_cflag.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> It's a bit of a shame that still maturing drivers can break userspace
> that easily and subtly. I had to debug my way from old but working
> buildroot to busybox, libc (tcsetattr) and then finally this driver.
> This wasn't the first bug of this kind, and maybe it's not the last (I
> didn't check all that termio flags). Could the kernel help in some way
> with sanity checks or sane defaults driver have to make insane
> willingly?
>
> drivers/tty/serial/mvebu-uart.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
> index d04b5eeea3c6..170e446a2f62 100644
> --- a/drivers/tty/serial/mvebu-uart.c
> +++ b/drivers/tty/serial/mvebu-uart.c
> @@ -511,6 +511,7 @@ static void mvebu_uart_set_termios(struct uart_port *port,
> termios->c_iflag |= old->c_iflag & ~(INPCK | IGNPAR);
> termios->c_cflag &= CREAD | CBAUD;
> termios->c_cflag |= old->c_cflag & ~(CREAD | CBAUD);
> + termios->c_cflag |= CS8;
> }
>
> spin_unlock_irqrestore(&port->lock, flags);
>
FWIW, below is the analogous fix for stable trees <= 4.14.
Jan
---8<---
From: Jan Kiszka <jan.kiszka@siemens.com>
Subject: [PATCH] serial: mvebu-uart: Fix reporting of effective CSIZE to userspace
Apparently, this driver (or the hardware) does not support character
length settings. It's apparently running in 8-bit mode, but it makes
userspace believe it's in 5-bit mode. That makes tcsetattr with CS8
incorrectly fail, breaking e.g. getty from busybox, thus the login shell
on ttyMVx.
Fix by hard-wiring CS8 into c_cflag.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/tty/serial/mvebu-uart.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 45b57c294d13..401c983ec5f3 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -327,8 +327,10 @@ static void mvebu_uart_set_termios(struct uart_port *port,
if ((termios->c_cflag & CREAD) == 0)
port->ignore_status_mask |= STAT_RX_RDY | STAT_BRK_ERR;
- if (old)
+ if (old) {
tty_termios_copy_hw(termios, old);
+ termios->c_cflag |= CS8;
+ }
baud = uart_get_baud_rate(port, termios, old, 0, 460800);
uart_update_timeout(port, termios->c_cflag, baud);
--
2.16.4
^ permalink raw reply related
* [PATCH] tty:serial:imx: use spin_lock instead of spin_lock_irqsave in isr
From: jun qian @ 2018-08-27 14:49 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Barry song, Jiri Slaby, linux-serial, linux-kernel, jun qian
Before the program enters the uart ISR, the local interrupt has been
disabled by the system, so it's not appropriate to use spin_lock_irqsave
interface in the ISR.
Signed-off-by: jun qian <hangdianqj@163.com>
---
drivers/tty/serial/imx.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 239c0fa2e981..3069ee93583e 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -706,27 +706,25 @@ static irqreturn_t imx_uart_rtsint(int irq, void *dev_id)
{
struct imx_port *sport = dev_id;
u32 usr1;
- unsigned long flags;
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
imx_uart_writel(sport, USR1_RTSD, USR1);
usr1 = imx_uart_readl(sport, USR1) & USR1_RTSS;
uart_handle_cts_change(&sport->port, !!usr1);
wake_up_interruptible(&sport->port.state->port.delta_msr_wait);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
return IRQ_HANDLED;
}
static irqreturn_t imx_uart_txint(int irq, void *dev_id)
{
struct imx_port *sport = dev_id;
- unsigned long flags;
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
imx_uart_transmit_buffer(sport);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
return IRQ_HANDLED;
}
@@ -735,9 +733,8 @@ static irqreturn_t imx_uart_rxint(int irq, void *dev_id)
struct imx_port *sport = dev_id;
unsigned int rx, flg, ignored = 0;
struct tty_port *port = &sport->port.state->port;
- unsigned long flags;
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
while (imx_uart_readl(sport, USR2) & USR2_RDR) {
u32 usr2;
@@ -797,7 +794,7 @@ static irqreturn_t imx_uart_rxint(int irq, void *dev_id)
}
out:
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
tty_flip_buffer_push(port);
return IRQ_HANDLED;
}
@@ -903,13 +900,11 @@ static irqreturn_t imx_uart_int(int irq, void *dev_id)
}
if (usr1 & USR1_DTRD) {
- unsigned long flags;
-
imx_uart_writel(sport, USR1_DTRD, USR1);
- spin_lock_irqsave(&sport->port.lock, flags);
+ spin_lock(&sport->port.lock);
imx_uart_mctrl_check(sport);
- spin_unlock_irqrestore(&sport->port.lock, flags);
+ spin_unlock(&sport->port.lock);
ret = IRQ_HANDLED;
}
--
2.17.1
^ permalink raw reply related
* Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
From: Tony Lindgren @ 2018-08-27 15:55 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Rob Herring, Nishanth Menon, Santosh Shilimkar, Will Deacon,
Catalin Marinas, Greg Kroah-Hartman, Mark Rutland,
open list:SERIAL DRIVERS, linux-kernel@vger.kernel.org,
devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Vignesh R, Tero Kristo, Russell King, Sudeep Holla
In-Reply-To: <40cecb47-bd32-04aa-b7cd-ff16c1eb28f3@ti.com>
* Kishon Vijay Abraham I <kishon@ti.com> [180827 03:06]:
> Hi Tony,
>
> On Monday 20 August 2018 08:01 PM, Tony Lindgren wrote:
> > * Kishon Vijay Abraham I <kishon@ti.com> [180808 06:35]:
> >> On Tuesday 05 June 2018 07:35 PM, Rob Herring wrote:
> >>> Really need 64-bit addresses and sizes? Use ranges to limit the
> >>> address space if possible.
> >>
> >> We now have address-cells as <1>,
> >> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am65.dtsi#n49
> >>
> >> However each PCIe instance has 2 data regions and one of the regions
> >> (PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT1/PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT1 specified
> >> in the "MAIN Domain Memory Map" table of TRM http://www.ti.com/lit/pdf/spruid7)
> >> is above the 32bit region and requires 2 cells to specify the start address.
> >> This region is used to access MEM_SPACE of PCIe endpoint when operating in root
> >> complex mode and access memory of PCI root complex when operating in endpoint mode.
> >>
> >> In order to describe this, should we change the address-cells back to <2> or do
> >> you suggest any other alternatives?
> >
> > It's probably best to have the top level cbass interconnect use
> > #size-cells = <2> and then have it's child interconnects have
> > #size-cells = <1> if they don't need ranges above 4GB.
>
> PCIe has a region starting at 0x40_00000000 and size 4GB. We need 2 address
> cells and 2 size cells to describe this no?
Yes.
> > BTW, what's the difference between all these three similar PCIE
> > ranges?
> >
> > PCIE0_CORE_CORE_DAT_SLV_PCIE_CORE 0x0005500000 0x0005600000 1 MB
> > PCIE1_CORE_CORE_DAT_SLV_PCIE_CORE 0x0005600000 0x0005700000 1 MB
>
> This is the register space for the two instances of PCIe controller.
> >
> > PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT0 0x0010000000 0x0018000000 128 MB
> > PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT0 0x0018000000 0x0020000000 128 MB
> >
> > PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT1 0x4000000000 0x4100000000 4 GB
> > PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT1 0x4100000000 0x4200000000 4 GB
>
> The above are regions which can be used by CPU/DMA to access the PCIe address
> space. The mapping from the above regions to the PCIe address space will be
> programmed in the PCIe controller.
OK so not just somethng for dma-ranges but also accessible by
the CPU.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
From: Stephen Boyd @ 2018-08-27 19:09 UTC (permalink / raw)
To: Songjun Wu, chuanhua.lei, hua.ma, qi-ming.wu, yixin zhu
Cc: linux-mips, linux-clk, linux-serial, devicetree,
Michael Turquette, linux-kernel, Rob Herring, Mark Rutland
In-Reply-To: <571d2d40-8728-fa7c-5d89-73d2a7b6293b@linux.intel.com>
Quoting yixin zhu (2018-08-08 01:52:20)
> On 8/8/2018 1:50 PM, Stephen Boyd wrote:
> > Quoting Songjun Wu (2018-08-02 20:02:21)
> >> + struct clk *clk;
> >> + int idx;
> >> +
> >> + for (idx = 0; idx < nr_clks; idx++, osc++) {
> >> + if (!osc->dt_freq ||
> >> + of_property_read_u32(ctx->np, osc->dt_freq, &freq))
> >> + freq = osc->def_rate;
> >> +
> >> + clk = clk_register_fixed_rate(NULL, osc->name, NULL, 0, freq);
> > Should come from DT itself.
> Yes. It can be defined as fixed-clock node in device tree.
> Do you mean it should be defined in device tree and driver reference it
> via device tree?
Yes the oscillator should be in DT and then the DT node here can call
clk_get() or just hardcode the parent name to be what it knows it is.
Eventually we'd like to be able to move away from string names for
hierarchy descriptions but that's far off. To get there, we would need
DT nodes for clock controllers to indicate their clk parents with the
clocks and clock-names properties. So for the oscillator, DT would
define it and then the driver would eventually have a way to specify
that some parent is index 5 or clock name "foo" and then the clk core
could figure out the linkage. I haven't written that code yet, but I'll
probably do it soon if nobody beats me to it.
> >> +/**
> >> + * struct intel_clk_provider
> >> + * @map: regmap type base address for register.
> >> + * @np: device node
> >> + * @clk_data: array of hw clocks and clk number.
> >> + */
> >> +struct intel_clk_provider {
> >> + struct regmap *map;
> >> + struct device_node *np;
> >> + struct clk_onecell_data clk_data;
> > Please register clk_hw pointers instead of clk pointers with the of
> > provider APIs.
> Sorry. I'm not sure I understand you correctly.
> If only registering clk_hw pointer, not registering of_provider API, then
> how to reference it in the user drivers ?
> Could you please give me more hints ?
Clk provider drivers shouldn't be using clk pointers directly. Usually
when that happens something is wrong. So new clk drivers should register
clk_hw pointers and pretty much only deal with clk_hw pointers instead
of struct clk pointers. You still register an of_provider, but that
provider hands out clk_hw pointers so that clk provider drivers aren't
tempted to use struct clk pointers.
>
>
> >
> >> + */
> >> +struct intel_pll_clk {
> >> + unsigned int id;
> >> + const char *name;
> >> + const char *const *parent_names;
> >> + u8 num_parents;
> > Can the PLL have multiple parents?
> Yes. But not in this platform.
> The define here make it easy to expand to support new platform.
>
Ok, so it has a mux inside.
>
> >> + unsigned int id;
> >> + enum intel_clk_type type;
> >> + const char *name;
> >> + const char *const *parent_names;
> >> + u8 num_parents;
> >> + unsigned long flags;
> >> + unsigned int mux_off;
> >> + u8 mux_shift;
> >> + u8 mux_width;
> >> + unsigned long mux_flags;
> >> + unsigned int mux_val;
> >> + unsigned int div_off;
> >> + u8 div_shift;
> >> + u8 div_width;
> >> + unsigned long div_flags;
> >> + unsigned int div_val;
> >> + const struct clk_div_table *div_table;
> >> + unsigned int gate_off;
> >> + u8 gate_shift;
> >> + unsigned long gate_flags;
> >> + unsigned int gate_val;
> >> + unsigned int mult;
> >> + unsigned int div;
> >> +};
> >> +
> >> +/* clock flags definition */
> >> +#define CLOCK_FLAG_VAL_INIT BIT(16)
> >> +#define GATE_CLK_HW BIT(17)
> >> +#define GATE_CLK_SW BIT(18)
> >> +#define GATE_CLK_VT BIT(19)
> > What does VT mean? Virtual?
> Yes. VT means virtual here.
> Will change to GATE_CLK_VIRT.
>
Is it a hardware concept? Or virtualization with hypervisor?
> >
> >> +}
> >> +
> >> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
> > Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
> It provides CPU clock which is used in early boot stage.
>
Ok. What is the CPU clock doing in early boot stage? Some sort of timer
frequency? If the driver can be split into two pieces, one to handle the
really early stuff that must be in place to get timers up and running
and the other to register the rest of the clks that aren't critical from
a regular platform driver it would be good. That's preferred model if
something is super critical.
^ permalink raw reply
* [PATCH] serial: 8250_omap: Make 8250_omap driver driver depend on ARCH_K3
From: Nishanth Menon @ 2018-08-28 1:03 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, linux-serial, Lokesh Vutla, linux-arm-kernel,
Sekhar Nori, Vignesh R, Nishanth Menon
From: Lokesh Vutla <lokeshvutla@ti.com>
Allow 8250 omap serial driver to be used for K3 platforms.
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
Now that we have the device tree support merged integrated AND we have ARCH_K3,
Lets enable the 820 OMAP Driver to build for ARCH_K3 and make it operational.
drivers/tty/serial/8250/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index f005eaf8bc57..15c2c5463835 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -375,7 +375,7 @@ config SERIAL_8250_RT288X
config SERIAL_8250_OMAP
tristate "Support for OMAP internal UART (8250 based driver)"
- depends on SERIAL_8250 && ARCH_OMAP2PLUS
+ depends on SERIAL_8250 && (ARCH_OMAP2PLUS || ARCH_K3)
help
If you have a machine based on an Texas Instruments OMAP CPU you
can enable its onboard serial ports by enabling this option.
--
2.15.1
^ permalink raw reply related
* Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
From: Nishanth Menon @ 2018-08-28 1:22 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kishon Vijay Abraham I, Rob Herring, Santosh Shilimkar,
Will Deacon, Catalin Marinas, Greg Kroah-Hartman, Mark Rutland,
open list:SERIAL DRIVERS, linux-kernel@vger.kernel.org,
devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Vignesh R, Tero Kristo, Russell King, Sudeep Holla
In-Reply-To: <20180827155535.GJ7523@atomide.com>
On 15:55-20180827, Tony Lindgren wrote:
> * Kishon Vijay Abraham I <kishon@ti.com> [180827 03:06]:
> > Hi Tony,
> >
> > On Monday 20 August 2018 08:01 PM, Tony Lindgren wrote:
> > > * Kishon Vijay Abraham I <kishon@ti.com> [180808 06:35]:
> > >> On Tuesday 05 June 2018 07:35 PM, Rob Herring wrote:
> > >>> Really need 64-bit addresses and sizes? Use ranges to limit the
> > >>> address space if possible.
> > >>
> > >> We now have address-cells as <1>,
> > >> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am65.dtsi#n49
> > >>
> > >> However each PCIe instance has 2 data regions and one of the regions
> > >> (PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT1/PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT1 specified
> > >> in the "MAIN Domain Memory Map" table of TRM http://www.ti.com/lit/pdf/spruid7)
> > >> is above the 32bit region and requires 2 cells to specify the start address.
> > >> This region is used to access MEM_SPACE of PCIe endpoint when operating in root
> > >> complex mode and access memory of PCI root complex when operating in endpoint mode.
> > >>
> > >> In order to describe this, should we change the address-cells back to <2> or do
> > >> you suggest any other alternatives?
> > >
> > > It's probably best to have the top level cbass interconnect use
> > > #size-cells = <2> and then have it's child interconnects have
> > > #size-cells = <1> if they don't need ranges above 4GB.
> >
> > PCIe has a region starting at 0x40_00000000 and size 4GB. We need 2 address
> > cells and 2 size cells to describe this no?
>
> Yes.
>
> > > BTW, what's the difference between all these three similar PCIE
> > > ranges?
> > >
> > > PCIE0_CORE_CORE_DAT_SLV_PCIE_CORE 0x0005500000 0x0005600000 1 MB
> > > PCIE1_CORE_CORE_DAT_SLV_PCIE_CORE 0x0005600000 0x0005700000 1 MB
> >
> > This is the register space for the two instances of PCIe controller.
> > >
> > > PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT0 0x0010000000 0x0018000000 128 MB
> > > PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT0 0x0018000000 0x0020000000 128 MB
> > >
> > > PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT1 0x4000000000 0x4100000000 4 GB
> > > PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT1 0x4100000000 0x4200000000 4 GB
> >
> > The above are regions which can be used by CPU/DMA to access the PCIe address
> > space. The mapping from the above regions to the PCIe address space will be
> > programmed in the PCIe controller.
>
> OK so not just somethng for dma-ranges but also accessible by
> the CPU.
>
Kishon, Sekhar:
Can you guys post patches based on v4.19-rc1 for fixing this? I do have
a bunch of dts nodes to build as well for v4.20, once Tony / Rob acks the
changes.
--
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH] tty: Convert to using %pOFn instead of device_node.name
From: Rob Herring @ 2018-08-28 1:52 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, Jiri Slaby, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, linux-serial, linuxppc-dev
In-Reply-To: <20180828015252.28511-1-robh@kernel.org>
In preparation to remove the node name pointer from struct device_node,
convert printf users to use the %pOFn format specifier.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linux-serial@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/tty/ehv_bytechan.c | 12 ++++++------
drivers/tty/serial/cpm_uart/cpm_uart_core.c | 8 ++++----
drivers/tty/serial/pmac_zilog.c | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
index eea4049b5dcc..769e0a5d1dfc 100644
--- a/drivers/tty/ehv_bytechan.c
+++ b/drivers/tty/ehv_bytechan.c
@@ -128,8 +128,8 @@ static int find_console_handle(void)
*/
iprop = of_get_property(np, "hv-handle", NULL);
if (!iprop) {
- pr_err("ehv-bc: no 'hv-handle' property in %s node\n",
- np->name);
+ pr_err("ehv-bc: no 'hv-handle' property in %pOFn node\n",
+ np);
return 0;
}
stdout_bc = be32_to_cpu(*iprop);
@@ -661,8 +661,8 @@ static int ehv_bc_tty_probe(struct platform_device *pdev)
iprop = of_get_property(np, "hv-handle", NULL);
if (!iprop) {
- dev_err(&pdev->dev, "no 'hv-handle' property in %s node\n",
- np->name);
+ dev_err(&pdev->dev, "no 'hv-handle' property in %pOFn node\n",
+ np);
return -ENODEV;
}
@@ -682,8 +682,8 @@ static int ehv_bc_tty_probe(struct platform_device *pdev)
bc->rx_irq = irq_of_parse_and_map(np, 0);
bc->tx_irq = irq_of_parse_and_map(np, 1);
if ((bc->rx_irq == NO_IRQ) || (bc->tx_irq == NO_IRQ)) {
- dev_err(&pdev->dev, "no 'interrupts' property in %s node\n",
- np->name);
+ dev_err(&pdev->dev, "no 'interrupts' property in %pOFn node\n",
+ np);
ret = -ENODEV;
goto error;
}
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
index 24a5f05e769b..ea7204d75022 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
@@ -1151,8 +1151,8 @@ static int cpm_uart_init_port(struct device_node *np,
if (!pinfo->clk) {
data = of_get_property(np, "fsl,cpm-brg", &len);
if (!data || len != 4) {
- printk(KERN_ERR "CPM UART %s has no/invalid "
- "fsl,cpm-brg property.\n", np->name);
+ printk(KERN_ERR "CPM UART %pOFn has no/invalid "
+ "fsl,cpm-brg property.\n", np);
return -EINVAL;
}
pinfo->brg = *data;
@@ -1160,8 +1160,8 @@ static int cpm_uart_init_port(struct device_node *np,
data = of_get_property(np, "fsl,cpm-command", &len);
if (!data || len != 4) {
- printk(KERN_ERR "CPM UART %s has no/invalid "
- "fsl,cpm-command property.\n", np->name);
+ printk(KERN_ERR "CPM UART %pOFn has no/invalid "
+ "fsl,cpm-command property.\n", np);
return -EINVAL;
}
pinfo->command = *data;
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index 3d21790d961e..a4ec22d1f214 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -1566,9 +1566,9 @@ static int pmz_attach(struct macio_dev *mdev, const struct of_device_id *match)
* to work around bugs in ancient Apple device-trees
*/
if (macio_request_resources(uap->dev, "pmac_zilog"))
- printk(KERN_WARNING "%s: Failed to request resource"
+ printk(KERN_WARNING "%pOFn: Failed to request resource"
", port still active\n",
- uap->node->name);
+ uap->node);
else
uap->flags |= PMACZILOG_FLAG_RSRC_REQUESTED;
--
2.17.1
^ permalink raw reply related
* Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
From: Kishon Vijay Abraham I @ 2018-08-28 3:39 UTC (permalink / raw)
To: Nishanth Menon, Tony Lindgren
Cc: Mark Rutland, devicetree, Sudeep Holla, Vignesh R,
Catalin Marinas, Santosh Shilimkar, Will Deacon,
linux-kernel@vger.kernel.org, Russell King, Tero Kristo,
Rob Herring, open list:SERIAL DRIVERS, Greg Kroah-Hartman,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20180828012224.fbmcmwfhfuwhcm53@kahuna>
Hi,
On Tuesday 28 August 2018 06:52 AM, Nishanth Menon wrote:
> On 15:55-20180827, Tony Lindgren wrote:
>> * Kishon Vijay Abraham I <kishon@ti.com> [180827 03:06]:
>>> Hi Tony,
>>>
>>> On Monday 20 August 2018 08:01 PM, Tony Lindgren wrote:
>>>> * Kishon Vijay Abraham I <kishon@ti.com> [180808 06:35]:
>>>>> On Tuesday 05 June 2018 07:35 PM, Rob Herring wrote:
>>>>>> Really need 64-bit addresses and sizes? Use ranges to limit the
>>>>>> address space if possible.
>>>>>
>>>>> We now have address-cells as <1>,
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am65.dtsi#n49
>>>>>
>>>>> However each PCIe instance has 2 data regions and one of the regions
>>>>> (PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT1/PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT1 specified
>>>>> in the "MAIN Domain Memory Map" table of TRM http://www.ti.com/lit/pdf/spruid7)
>>>>> is above the 32bit region and requires 2 cells to specify the start address.
>>>>> This region is used to access MEM_SPACE of PCIe endpoint when operating in root
>>>>> complex mode and access memory of PCI root complex when operating in endpoint mode.
>>>>>
>>>>> In order to describe this, should we change the address-cells back to <2> or do
>>>>> you suggest any other alternatives?
>>>>
>>>> It's probably best to have the top level cbass interconnect use
>>>> #size-cells = <2> and then have it's child interconnects have
>>>> #size-cells = <1> if they don't need ranges above 4GB.
>>>
>>> PCIe has a region starting at 0x40_00000000 and size 4GB. We need 2 address
>>> cells and 2 size cells to describe this no?
>>
>> Yes.
>>
>>>> BTW, what's the difference between all these three similar PCIE
>>>> ranges?
>>>>
>>>> PCIE0_CORE_CORE_DAT_SLV_PCIE_CORE 0x0005500000 0x0005600000 1 MB
>>>> PCIE1_CORE_CORE_DAT_SLV_PCIE_CORE 0x0005600000 0x0005700000 1 MB
>>>
>>> This is the register space for the two instances of PCIe controller.
>>>>
>>>> PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT0 0x0010000000 0x0018000000 128 MB
>>>> PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT0 0x0018000000 0x0020000000 128 MB
>>>>
>>>> PCIE0_CORE_CORE_DAT_SLV_PCIE_DAT1 0x4000000000 0x4100000000 4 GB
>>>> PCIE1_CORE_CORE_DAT_SLV_PCIE_DAT1 0x4100000000 0x4200000000 4 GB
>>>
>>> The above are regions which can be used by CPU/DMA to access the PCIe address
>>> space. The mapping from the above regions to the PCIe address space will be
>>> programmed in the PCIe controller.
>>
>> OK so not just somethng for dma-ranges but also accessible by
>> the CPU.
>>
>
> Kishon, Sekhar:
>
> Can you guys post patches based on v4.19-rc1 for fixing this? I do have
> a bunch of dts nodes to build as well for v4.20, once Tony / Rob acks the
> changes.
Sure, I'll post that today.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH] tty:serial:imx: use spin_lock instead of spin_lock_irqsave in isr
From: Barry Song @ 2018-08-28 8:36 UTC (permalink / raw)
To: hangdianqj; +Cc: Greg Kroah-Hartman, jslaby, linux-serial, LKML
In-Reply-To: <20180827144904.17226-1-hangdianqj@163.com>
jun qian <hangdianqj@163.com> 于2018年8月27日周一 下午10:49写道：
>
> Before the program enters the uart ISR, the local interrupt has been
> disabled by the system, so it's not appropriate to use spin_lock_irqsave
> interface in the ISR.
>
> Signed-off-by: jun qian <hangdianqj@163.com>
many discussions have been done with jun in wechat regarding this patch. and
Reviewed-by: Barry Song <21cnbao@gmail.com>
> ---
> drivers/tty/serial/imx.c | 21 ++++++++-------------
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
^ permalink raw reply
* [PATCH] tty: serial: lpuart: avoid leaking struct tty_struct
From: Stefan Agner @ 2018-08-28 10:44 UTC (permalink / raw)
To: gregkh; +Cc: jslaby, peter, linux-serial, linux-kernel, Stefan Agner, stable
The function tty_port_tty_get() gets a reference to the tty. Since
the code is not using tty_port_tty_set(), the reference is kept
even after closing the tty.
Avoid using tty_port_tty_get() by directly access the tty instance.
Since lpuart_start_rx_dma() is called from the .startup() and
.set_termios() callback, it is safe to assume the tty instance is
valid.
Cc: stable@vger.kernel.org # v4.9+
Fixes: 5887ad43ee02 ("tty: serial: fsl_lpuart: Use cyclic DMA for Rx")
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
This fixes a memory leak observable when opening/closing the tty in a
loop. This is also reported by kmemleak:
unreferenced object 0xc9d17000 (size 1024):
comm "(agetty)", pid 389, jiffies 4294943045 (age 100.670s)
hex dump (first 32 bytes):
01 54 00 00 01 00 00 00 00 8c 9b c8 80 58 9b c8 .T...........X..
48 58 c4 c0 00 00 00 00 00 00 00 00 00 00 00 00 HX..............
backtrace:
[<(ptrval)>] kmem_cache_alloc_trace+0x160/0x2b8
[<(ptrval)>] alloc_tty_struct+0x44/0x254
[<(ptrval)>] tty_init_dev+0x44/0x1c8
[<(ptrval)>] tty_open+0x268/0x414
[<(ptrval)>] chrdev_open+0xb4/0x1bc
[<(ptrval)>] do_dentry_open+0x1c0/0x388
[<(ptrval)>] vfs_open+0x34/0x38
[<(ptrval)>] path_openat+0x5b0/0x11bc
[<(ptrval)>] do_filp_open+0x7c/0xe8
[<(ptrval)>] do_sys_open+0x188/0x20c
[<(ptrval)>] sys_openat+0x14/0x18
[<(ptrval)>] ret_fast_syscall+0x0/0x28
[<(ptrval)>] 0xbee0a460
[<(ptrval)>] 0xffffffff
I *think* the statement that accessing tty_struct without using
tty_port_tty_get is safe in this case is true. It would be good if
somebody with more TTY knowledge could review the change.
^ permalink raw reply
* Re: [PATCH] serial: 8250_omap: Make 8250_omap driver driver depend on ARCH_K3
From: Tony Lindgren @ 2018-08-28 16:21 UTC (permalink / raw)
To: Nishanth Menon
Cc: Greg Kroah-Hartman, Vignesh R, Lokesh Vutla, Sekhar Nori,
linux-kernel, linux-serial, linux-arm-kernel
In-Reply-To: <20180828010302.2751-1-nm@ti.com>
* Nishanth Menon <nm@ti.com> [180828 01:07]:
> From: Lokesh Vutla <lokeshvutla@ti.com>
>
> Allow 8250 omap serial driver to be used for K3 platforms.
>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>
> Now that we have the device tree support merged integrated AND we have ARCH_K3,
> Lets enable the 820 OMAP Driver to build for ARCH_K3 and make it operational.
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
From: Zhu, Yi Xin @ 2018-08-29 6:56 UTC (permalink / raw)
To: Stephen Boyd, Songjun Wu, chuanhua.lei, hua.ma, qi-ming.wu
Cc: linux-mips, linux-clk, linux-serial, devicetree,
Michael Turquette, linux-kernel, Rob Herring, Mark Rutland
In-Reply-To: <153539697928.129321.2605078315090527674@swboyd.mtv.corp.google.com>
On 8/28/2018 3:09 AM, Stephen Boyd wrote:
> Quoting yixin zhu (2018-08-08 01:52:20)
>> On 8/8/2018 1:50 PM, Stephen Boyd wrote:
>>> Quoting Songjun Wu (2018-08-02 20:02:21)
>>>> + struct clk *clk;
>>>> + int idx;
>>>> +
>>>> + for (idx = 0; idx < nr_clks; idx++, osc++) {
>>>> + if (!osc->dt_freq ||
>>>> + of_property_read_u32(ctx->np, osc->dt_freq, &freq))
>>>> + freq = osc->def_rate;
>>>> +
>>>> + clk = clk_register_fixed_rate(NULL, osc->name, NULL, 0, freq);
>>> Should come from DT itself.
>> Yes. It can be defined as fixed-clock node in device tree.
>> Do you mean it should be defined in device tree and driver reference it
>> via device tree?
> Yes the oscillator should be in DT and then the DT node here can call
> clk_get() or just hardcode the parent name to be what it knows it is.
> Eventually we'd like to be able to move away from string names for
> hierarchy descriptions but that's far off. To get there, we would need
> DT nodes for clock controllers to indicate their clk parents with the
> clocks and clock-names properties. So for the oscillator, DT would
> define it and then the driver would eventually have a way to specify
> that some parent is index 5 or clock name "foo" and then the clk core
> could figure out the linkage. I haven't written that code yet, but I'll
> probably do it soon if nobody beats me to it.
Thanks. Will update.
>
>>>> +/**
>>>> + * struct intel_clk_provider
>>>> + * @map: regmap type base address for register.
>>>> + * @np: device node
>>>> + * @clk_data: array of hw clocks and clk number.
>>>> + */
>>>> +struct intel_clk_provider {
>>>> + struct regmap *map;
>>>> + struct device_node *np;
>>>> + struct clk_onecell_data clk_data;
>>> Please register clk_hw pointers instead of clk pointers with the of
>>> provider APIs.
>> Sorry. I'm not sure I understand you correctly.
>> If only registering clk_hw pointer, not registering of_provider API, then
>> how to reference it in the user drivers ?
>> Could you please give me more hints ?
> Clk provider drivers shouldn't be using clk pointers directly. Usually
> when that happens something is wrong. So new clk drivers should register
> clk_hw pointers and pretty much only deal with clk_hw pointers instead
> of struct clk pointers. You still register an of_provider, but that
> provider hands out clk_hw pointers so that clk provider drivers aren't
> tempted to use struct clk pointers.
Understood. Will update to use clk_hw_onecell_data and change the
registration accordingly.
>>
>>>> + */
>>>> +struct intel_pll_clk {
>>>> + unsigned int id;
>>>> + const char *name;
>>>> + const char *const *parent_names;
>>>> + u8 num_parents;
>>> Can the PLL have multiple parents?
>> Yes. But not in this platform.
>> The define here make it easy to expand to support new platform.
>>
> Ok, so it has a mux inside.
>
>>>> + unsigned int id;
>>>> + enum intel_clk_type type;
>>>> + const char *name;
>>>> + const char *const *parent_names;
>>>> + u8 num_parents;
>>>> + unsigned long flags;
>>>> + unsigned int mux_off;
>>>> + u8 mux_shift;
>>>> + u8 mux_width;
>>>> + unsigned long mux_flags;
>>>> + unsigned int mux_val;
>>>> + unsigned int div_off;
>>>> + u8 div_shift;
>>>> + u8 div_width;
>>>> + unsigned long div_flags;
>>>> + unsigned int div_val;
>>>> + const struct clk_div_table *div_table;
>>>> + unsigned int gate_off;
>>>> + u8 gate_shift;
>>>> + unsigned long gate_flags;
>>>> + unsigned int gate_val;
>>>> + unsigned int mult;
>>>> + unsigned int div;
>>>> +};
>>>> +
>>>> +/* clock flags definition */
>>>> +#define CLOCK_FLAG_VAL_INIT BIT(16)
>>>> +#define GATE_CLK_HW BIT(17)
>>>> +#define GATE_CLK_SW BIT(18)
>>>> +#define GATE_CLK_VT BIT(19)
>>> What does VT mean? Virtual?
>> Yes. VT means virtual here.
>> Will change to GATE_CLK_VIRT.
>>
> Is it a hardware concept? Or virtualization with hypervisor?
Some peripheral drivers want to use same code cross platforms.
But not all platforms provide HW gate clock. So in this case, clock
driver creates
a virtual gate clock to make it work if no HW gate clock in the SoC.
>
>>>> +}
>>>> +
>>>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
>>> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
>> It provides CPU clock which is used in early boot stage.
>>
> Ok. What is the CPU clock doing in early boot stage? Some sort of timer
> frequency? If the driver can be split into two pieces, one to handle the
> really early stuff that must be in place to get timers up and running
> and the other to register the rest of the clks that aren't critical from
> a regular platform driver it would be good. That's preferred model if
> something is super critical.
Yes, CPU clock is providing CPU frequency in the early boot stage.
Will put the non-critical clocks in the platform driver.
>
^ permalink raw reply
* Re: [RFC RFT PATCH 0/4] gpiolib: speed up GPIO array processing
From: Linus Walleij @ 2018-08-29 9:06 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
David S. Miller, Dominik Brodowski, kishon, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald, Greg KH, Jiri Slaby, open list:GPIO SUBSYSTEM,
linux-doc, linux-i2c
In-Reply-To: <20180820234341.5271-1-jmkrzyszt@gmail.com>
On Tue, Aug 21, 2018 at 1:42 AM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> This series is a follow up of the former "mtd: rawnand: ams-delta: Use
> gpio-omap accessors for data I/O" which already contained some changes
> to gpiolib. Those previous attempts were commented by Borris Brezillon
> who suggested using GPIO API modified to accept bitmaps, and by Linus
> Walleij who suggested still more great ideas for further immprovement
> of the proposed API changes - thanks!
>
> The goal is to boost performans of get/set array functions while
> processing GPIO arrays which represent pins of a signle chip in
> hardware order. If resulting performance is close to PIO, GPIO API
> can be used for data I/O without much loss of speed.
Hands down, this is a very pretty patch set. I'm a big fan already.
This is mainly because it fulfills the requirement for libraries
to be narrow and deep, which is what we want.
This refers to John Ousterhouts software design philosophy,
here is a great lecture if you haven't seen it already:
https://www.youtube.com/watch?v=bmSAYlu0NcY
Let's get this into v1 and get some testing and merge it for v4.20
ASAP so we get some proper testing before the v4.20 merge
window. It would be excellent if some of the current users of
the array API could provide tested-by's or at least ACKs.
For example ts-nbus.c must be a big benefactor.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [RFC RFT PATCH 0/4] gpiolib: speed up GPIO array processing
From: Ulf Hansson @ 2018-08-29 10:19 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miguel Ojeda Sandonis,
Peter Korsgaard, Peter Rosin, Andrew Lunn, Florian Fainelli,
David S. Miller, Dominik Brodowski, Kishon Vijay Abraham I,
Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
Jiri Slaby, linux-gpio, linux-doc, linux-i2c
In-Reply-To: <20180820234341.5271-1-jmkrzyszt@gmail.com>
On 21 August 2018 at 01:43, Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
>
> This series is a follow up of the former "mtd: rawnand: ams-delta: Use
> gpio-omap accessors for data I/O" which already contained some changes
> to gpiolib. Those previous attempts were commented by Borris Brezillon
> who suggested using GPIO API modified to accept bitmaps, and by Linus
> Walleij who suggested still more great ideas for further immprovement
> of the proposed API changes - thanks!
>
> The goal is to boost performans of get/set array functions while
> processing GPIO arrays which represent pins of a signle chip in
> hardware order. If resulting performance is close to PIO, GPIO API
> can be used for data I/O without much loss of speed.
>
> Created and tested on a low end Amstrad Delta board with NAND driver
> updated to use GPIO API for data I/O. Performance degrade compared to
> PIO is much better than before the optimization but still not quite
> satisfactory.
>
>
> Janusz Krzysztofik (4):
> gpiolib: Pass bitmaps, not integer arrays, to get/set array
> gpiolib: Identify arrays matching GPIO hardware
> gpiolib: Pass array info to get/set array functions
> gpiolib: Implement fast processing path in get/set array
>
>
> Documentation/driver-api/gpio/board.rst | 15 +
> Documentation/driver-api/gpio/consumer.rst | 48 +++-
> drivers/auxdisplay/hd44780.c | 64 +++---
> drivers/bus/ts-nbus.c | 25 --
> drivers/gpio/gpio-max3191x.c | 23 +-
> drivers/gpio/gpiolib.c | 279 ++++++++++++++++++++++------
> drivers/gpio/gpiolib.h | 15 +
> drivers/i2c/muxes/i2c-mux-gpio.c | 5
> drivers/mmc/core/pwrseq_simple.c | 15 -
> drivers/mux/gpio.c | 7
> drivers/net/phy/mdio-mux-gpio.c | 5
> drivers/pcmcia/soc_common.c | 14 -
> drivers/phy/motorola/phy-mapphone-mdm6600.c | 21 +-
> drivers/staging/iio/adc/ad7606.c | 12 -
> drivers/tty/serial/serial_mctrl_gpio.c | 9
> include/linux/gpio/consumer.h | 35 ++-
> 16 files changed, 410 insertions(+), 182 deletions(-)
>
For the mmc related changes:
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
From: Zhu, Yi Xin @ 2018-08-29 10:34 UTC (permalink / raw)
To: Stephen Boyd, Songjun Wu, chuanhua.lei, hua.ma, qi-ming.wu
Cc: linux-mips, linux-clk, linux-serial, devicetree,
Michael Turquette, linux-kernel, Rob Herring, Mark Rutland
In-Reply-To: <153539697928.129321.2605078315090527674@swboyd.mtv.corp.google.com>
>>>> +}
>>>> +
>>>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
>>> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
>> It provides CPU clock which is used in early boot stage.
>>
> Ok. What is the CPU clock doing in early boot stage? Some sort of timer
> frequency? If the driver can be split into two pieces, one to handle the
> really early stuff that must be in place to get timers up and running
> and the other to register the rest of the clks that aren't critical from
> a regular platform driver it would be good. That's preferred model if
> something is super critical.
>
Just to make sure my approach is same as you think.
In the driver, there's two clock registrations.
- One through CLK_OF_DECLARE for early stage clocks.
- The other via platform driver for the non-critical clocks.
In the device tree, two clock device nodes are required.
e.g. device tree:
cgu: cgu@16200000 {
compatible = "intel,grx500-clk", "syscon";
reg = <0x16200000 0x200>;
#clock-cells = <1>;
};
clk: clk {
compatible = "intel,grx500-cgu";
#clock-cells = <1>;
intel,cgu-syscon = <&cgu>;
};
source code:
CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
static const struct of_device_id of_intel_grx500_cgu_match[] = {
{ .compatible = "intel,grx500-clk" },
{}
};
static struct platform_driver intel_grx500_clk_driver = {
.probe = intel_grx500_clk_probe,
.driver = {
.name = "grx500-cgu",
.of_match_table = of_match_ptr(of_intel_grx500_cgu_match),
},
};
static int __init intel_grx500_cgu_init(void)
{
return platform_driver_register(&intel_grx500_clk_driver);
}
arch_initcall(intel_grx500_cgu_init);
^ permalink raw reply
* Re: [RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Miguel Ojeda @ 2018-08-29 12:03 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Andrew Lunn, Ulf Hansson, Linux Doc Mailing List, linux-iio,
Linus Walleij, Dominik Brodowski, Network Development, linux-i2c,
Peter Meerwald-Stadler, devel, Florian Fainelli, Jonathan Corbet,
Kishon Vijay Abraham I, Willy Tarreau, Geert Uytterhoeven,
linux-serial, Jiri Slaby, Michael Hennerich, linux-gpio,
Lars-Peter Clausen, Greg Kroah-Hartman, linux-mmc, linux-kernel,
Peter Rosin
In-Reply-To: <20180820234341.5271-2-jmkrzyszt@gmail.com>
Hi Janusz,
On Tue, Aug 21, 2018 at 1:43 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> Most users of get/set array functions iterate consecutive bits of data,
> usually a single integer, while or processing array of results obtained
> from or building an array of values to be passed to those functions.
> Save time wasted on those iterations by changing the functions' API to
> accept bitmaps.
>
> All current users are updated as well.
>
> More benefits from the change are expected as soon as planned support
> for accepting/passing those bitmaps directly from/to respective GPIO
> chip callbacks if applicable is implemented.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
> Documentation/driver-api/gpio/consumer.rst | 22 ++++----
> drivers/auxdisplay/hd44780.c | 52 +++++++++--------
[CC'ing Willy and Geert for hd44780]
> diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
> index f1a42f0f1ded..d340473aa142 100644
> --- a/drivers/auxdisplay/hd44780.c
> +++ b/drivers/auxdisplay/hd44780.c
> @@ -62,20 +62,19 @@ static void hd44780_strobe_gpio(struct hd44780 *hd)
> /* write to an LCD panel register in 8 bit GPIO mode */
> static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
> {
> - int values[10]; /* for DATA[0-7], RS, RW */
> - unsigned int i, n;
> + unsigned long value_bitmap[1]; /* for DATA[0-7], RS, RW */
Why [1]? I understand it is because in other cases it may be more than
one, but...
> + unsigned int n;
>
> - for (i = 0; i < 8; i++)
> - values[PIN_DATA0 + i] = !!(val & BIT(i));
> - values[PIN_CTRL_RS] = rs;
> + value_bitmap[0] = val;
> + __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> n = 9;
> if (hd->pins[PIN_CTRL_RW]) {
> - values[PIN_CTRL_RW] = 0;
> + __clear_bit(PIN_CTRL_RW, value_bitmap);
> n++;
> }
>
> /* Present the data to the port */
> - gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], values);
> + gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], value_bitmap);
>
> hd44780_strobe_gpio(hd);
> }
> @@ -83,32 +82,31 @@ static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
> /* write to an LCD panel register in 4 bit GPIO mode */
> static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
> {
> - int values[10]; /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
> - unsigned int i, n;
> + /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
> + unsigned long value_bitmap[0];
This one is even more strange... :-)
> + unsigned int n;
>
> /* High nibble + RS, RW */
> - for (i = 4; i < 8; i++)
> - values[PIN_DATA0 + i] = !!(val & BIT(i));
> - values[PIN_CTRL_RS] = rs;
> + value_bitmap[0] = val;
> + __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> n = 5;
> if (hd->pins[PIN_CTRL_RW]) {
> - values[PIN_CTRL_RW] = 0;
> + __clear_bit(PIN_CTRL_RW, value_bitmap);
> n++;
> }
> + value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
Maybe >>=?
>
> /* Present the data to the port */
> - gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
> - &values[PIN_DATA4]);
> + gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
>
> hd44780_strobe_gpio(hd);
>
> /* Low nibble */
> - for (i = 0; i < 4; i++)
> - values[PIN_DATA4 + i] = !!(val & BIT(i));
> + value_bitmap[0] &= ~((1 << PIN_DATA4) - 1);
> + value_bitmap[0] |= val & ~((1 << PIN_DATA4) - 1);
Are you sure this is correct? You are basically doing an or of
value_bitmap and val and clearing the low-nibble.
>
> /* Present the data to the port */
> - gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
> - &values[PIN_DATA4]);
> + gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
>
> hd44780_strobe_gpio(hd);
> }
Cheers,
Miguel
^ permalink raw reply
* [PATCH] serial: 8250_of: Fix for lack of interrupt support
From: John Garry @ 2018-08-29 15:07 UTC (permalink / raw)
To: gregkh
Cc: alexander.sverdlin, jslaby, robh, joel, kurt, yamada.masahiro,
linux-serial, linux-kernel, linuxarm, John Garry
In commit c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ"), a
check was added for the UART driver being probed prior to the parent IRQ
controller.
Unfortunately this breaks certain boards which have no interrupt support,
like Huawei D03.
Indeed, the 8250 DT bindings state that interrupts should be supported -
not must.
To fix, switch from irq_of_parse_and_map() to of_irq_get(), which
does relay whether the IRQ host controller domain is not ready, i.e.
defer probe, instead of assuming it.
Fixes: c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ")
Signed-off-by: John Garry <john.garry@huawei.com>
---
Note: I think that it would better if we could try to get the interrupt
before clk+pm enabling, so we don't need to disable later when
deferring, but this is not a fix.
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index af8beef..c370e776 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -58,7 +58,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
struct resource resource;
struct device_node *np = ofdev->dev.of_node;
u32 clk, spd, prop;
- int ret;
+ int ret, irq;
memset(port, 0, sizeof *port);
@@ -143,21 +143,27 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
if (ret >= 0)
port->line = ret;
- port->irq = irq_of_parse_and_map(np, 0);
- if (!port->irq) {
- ret = -EPROBE_DEFER;
- goto err_unprepare;
+ irq = of_irq_get(np, 0);
+ if (irq < 0) {
+ if (port->irq == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err_unprepare;
+ }
+ /* IRQ support not mandatory */
+ irq = 0;
}
+ port->irq = irq;
+
info->rst = devm_reset_control_get_optional_shared(&ofdev->dev, NULL);
if (IS_ERR(info->rst)) {
ret = PTR_ERR(info->rst);
- goto err_dispose;
+ goto err_unprepare;
}
ret = reset_control_deassert(info->rst);
if (ret)
- goto err_dispose;
+ goto err_unprepare;
port->type = type;
port->uartclk = clk;
@@ -184,8 +190,6 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
port->handle_irq = fsl8250_handle_irq;
return 0;
-err_dispose:
- irq_dispose_mapping(port->irq);
err_unprepare:
clk_disable_unprepare(info->clk);
err_pmruntime:
--
1.9.1
^ permalink raw reply related
* Re: [RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Janusz Krzysztofik @ 2018-08-29 18:01 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Linus Walleij, Jonathan Corbet, Peter Korsgaard, Peter Rosin,
Ulf Hansson, Andrew Lunn, Florian Fainelli, David S. Miller,
Dominik Brodowski, Kishon Vijay Abraham I, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, Jiri Slaby,
linux-gpio, Linux Doc Mailing List
In-Reply-To: <CANiq72kM9bYJ1Q+dbLumjfQLZW223ZTrYEFqfQ2Jv2SAjrD1NA@mail.gmail.com>
On Wednesday, August 29, 2018 2:03:18 PM CEST Miguel Ojeda wrote:
> Hi Janusz,
>
> On Tue, Aug 21, 2018 at 1:43 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> > Most users of get/set array functions iterate consecutive bits of data,
> > usually a single integer, while or processing array of results obtained
> > from or building an array of values to be passed to those functions.
> > Save time wasted on those iterations by changing the functions' API to
> > accept bitmaps.
> >
> > All current users are updated as well.
> >
> > More benefits from the change are expected as soon as planned support
> > for accepting/passing those bitmaps directly from/to respective GPIO
> > chip callbacks if applicable is implemented.
> >
> > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> > ---
> > Documentation/driver-api/gpio/consumer.rst | 22 ++++----
> > drivers/auxdisplay/hd44780.c | 52 +++++++++--------
>
> [CC'ing Willy and Geert for hd44780]
>
> > diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
> > index f1a42f0f1ded..d340473aa142 100644
> > --- a/drivers/auxdisplay/hd44780.c
> > +++ b/drivers/auxdisplay/hd44780.c
> > @@ -62,20 +62,19 @@ static void hd44780_strobe_gpio(struct hd44780 *hd)
> > /* write to an LCD panel register in 8 bit GPIO mode */
> > static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
> > {
> > - int values[10]; /* for DATA[0-7], RS, RW */
> > - unsigned int i, n;
> > + unsigned long value_bitmap[1]; /* for DATA[0-7], RS, RW */
>
> Why [1]? I understand it is because in other cases it may be more than
> one,
Yes, I tried to point out the fact the new API accepts a bitmap of an arbitrary
length, and I tried to use the same code pattern across changes to the API
users.
> but...
>
> > + unsigned int n;
> >
> > - for (i = 0; i < 8; i++)
> > - values[PIN_DATA0 + i] = !!(val & BIT(i));
> > - values[PIN_CTRL_RS] = rs;
> > + value_bitmap[0] = val;
> > + __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> > n = 9;
> > if (hd->pins[PIN_CTRL_RW]) {
> > - values[PIN_CTRL_RW] = 0;
> > + __clear_bit(PIN_CTRL_RW, value_bitmap);
> > n++;
> > }
> >
> > /* Present the data to the port */
> > - gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], values);
> > + gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], value_bitmap);
> >
> > hd44780_strobe_gpio(hd);
> > }
> > @@ -83,32 +82,31 @@ static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
> > /* write to an LCD panel register in 4 bit GPIO mode */
> > static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
> > {
> > - int values[10]; /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
> > - unsigned int i, n;
> > + /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
> > + unsigned long value_bitmap[0];
>
> This one is even more strange... :-)
This one is an error, should be 1 of course :-), thanks.
> > + unsigned int n;
> >
> > /* High nibble + RS, RW */
> > - for (i = 4; i < 8; i++)
> > - values[PIN_DATA0 + i] = !!(val & BIT(i));
> > - values[PIN_CTRL_RS] = rs;
> > + value_bitmap[0] = val;
> > + __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> > n = 5;
> > if (hd->pins[PIN_CTRL_RW]) {
> > - values[PIN_CTRL_RW] = 0;
> > + __clear_bit(PIN_CTRL_RW, value_bitmap);
> > n++;
> > }
> > + value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
>
> Maybe >>=?
OK.
Answering you question below:
To make my changes as clear as I could imagine, I decided to use the same indexing as in the original code, i.e., assign high nibble of val to bits 4-7 and two other values - rs and an optional 0 - to bits 8 and 9, respectively.
Unlike in case of array of integers, where for the high nibble part you could just pass a pointer to a sub-array starting at the 5th value (i.e., &values[PIN_DATA4]), it was not possible to do the same for and arbitrary bit of a bitmap, e.g., pass a pointer to the 5th bit of *value_bitmap as an argument pointing to bit 0 of a bitmap to be processed. That's why I shifted the bitmap right by 4 bits.
Then, ...
> >
> > /* Present the data to the port */
> > - gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
> > - &values[PIN_DATA4]);
> > + gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
> >
> > hd44780_strobe_gpio(hd);
> >
> > /* Low nibble */
> > - for (i = 0; i < 4; i++)
> > - values[PIN_DATA4 + i] = !!(val & BIT(i));
> > + value_bitmap[0] &= ~((1 << PIN_DATA4) - 1);
> > + value_bitmap[0] |= val & ~((1 << PIN_DATA4) - 1);
>
> Are you sure this is correct? You are basically doing an or of
> value_bitmap and val and clearing the low-nibble.
having the rs and optional 0 already assigned to bits 4 and 5 of the bitmap, I just cleared bits 0-3 still containing the high nibble of val and assigned the low nibble of it to those bits, getting a result ready to be passed as an argument to gpiod_set_array_value_cansleep() below.
I hope I didn't miss anything.
Thanks,
Janusz
> >
> > /* Present the data to the port */
> > - gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
> > - &values[PIN_DATA4]);
> > + gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
> >
> > hd44780_strobe_gpio(hd);
> > }
>
> Cheers,
> Miguel
>
^ permalink raw reply
* Re: [RFC RFT PATCH 0/4] gpiolib: speed up GPIO array processing
From: Janusz Krzysztofik @ 2018-08-29 18:16 UTC (permalink / raw)
To: Linus Walleij
Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
David S. Miller, Dominik Brodowski, kishon, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald, Greg KH, Jiri Slaby, open list:GPIO SUBSYSTEM,
linux-doc, linux-i2c
In-Reply-To: <CACRpkdbgY6VNP-K-c5ErQtmO7E83D6c=49TN5siF8VTNh_66Jg@mail.gmail.com>
Hi Linus,
On Wednesday, August 29, 2018 11:06:21 AM CEST Linus Walleij wrote:
> On Tue, Aug 21, 2018 at 1:42 AM Janusz Krzysztofik <jmkrzyszt@gmail.com>
wrote:
>
> > This series is a follow up of the former "mtd: rawnand: ams-delta: Use
> > gpio-omap accessors for data I/O" which already contained some changes
> > to gpiolib. Those previous attempts were commented by Borris Brezillon
> > who suggested using GPIO API modified to accept bitmaps, and by Linus
> > Walleij who suggested still more great ideas for further immprovement
> > of the proposed API changes - thanks!
> >
> > The goal is to boost performans of get/set array functions while
> > processing GPIO arrays which represent pins of a signle chip in
> > hardware order. If resulting performance is close to PIO, GPIO API
> > can be used for data I/O without much loss of speed.
>
> Hands down, this is a very pretty patch set. I'm a big fan already.
>
> This is mainly because it fulfills the requirement for libraries
> to be narrow and deep, which is what we want.
> This refers to John Ousterhouts software design philosophy,
> here is a great lecture if you haven't seen it already:
> https://www.youtube.com/watch?v=bmSAYlu0NcY
>
> Let's get this into v1 and get some testing and merge it for v4.20
> ASAP
Please hold on for a while, I'm going to resubmit soon, with the comment from
Peter Rosin on i2c-mux-gpio addressed and the error discovered by Miguel Ojeda
in hd44780 fixed.
Thanks,
Janusz
> so we get some proper testing before the v4.20 merge
> window. It would be excellent if some of the current users of
> the array API could provide tested-by's or at least ACKs.
>
> For example ts-nbus.c must be a big benefactor.
>
> Yours,
> Linus Walleij
>
^ permalink raw reply
* Re: [PATCH] serial: 8250_of: Fix for lack of interrupt support
From: John Garry @ 2018-08-29 19:50 UTC (permalink / raw)
To: gregkh, alexander.sverdlin
Cc: jslaby, robh, joel, kurt, yamada.masahiro, linux-serial,
linux-kernel, linuxarm
In-Reply-To: <1535555277-102256-1-git-send-email-john.garry@huawei.com>
On 29/08/2018 16:07, John Garry wrote:
> In commit c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ"), a
> check was added for the UART driver being probed prior to the parent IRQ
> controller.
>
> Unfortunately this breaks certain boards which have no interrupt support,
> like Huawei D03.
>
> Indeed, the 8250 DT bindings state that interrupts should be supported -
> not must.
>
> To fix, switch from irq_of_parse_and_map() to of_irq_get(), which
> does relay whether the IRQ host controller domain is not ready, i.e.
> defer probe, instead of assuming it.
>
> Fixes: c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ")
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
> Note: I think that it would better if we could try to get the interrupt
> before clk+pm enabling, so we don't need to disable later when
> deferring, but this is not a fix.
>
> diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
> index af8beef..c370e776 100644
> --- a/drivers/tty/serial/8250/8250_of.c
> +++ b/drivers/tty/serial/8250/8250_of.c
> @@ -58,7 +58,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
> struct resource resource;
> struct device_node *np = ofdev->dev.of_node;
> u32 clk, spd, prop;
> - int ret;
> + int ret, irq;
>
> memset(port, 0, sizeof *port);
>
> @@ -143,21 +143,27 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
> if (ret >= 0)
> port->line = ret;
>
> - port->irq = irq_of_parse_and_map(np, 0);
> - if (!port->irq) {
> - ret = -EPROBE_DEFER;
> - goto err_unprepare;
> + irq = of_irq_get(np, 0);
> + if (irq < 0) {
> + if (port->irq == -EPROBE_DEFER) {
This check is not corrrct. It should be:
if (irq == -EPROBE_DEFER) {
I'll send a v2.
@Alexander Sverdlin, Can you test this also please?
Thanks,
John
> + ret = -EPROBE_DEFER;
> + goto err_unprepare;
> + }
> + /* IRQ support not mandatory */
> + irq = 0;
> }
>
> + port->irq = irq;
> +
> info->rst = devm_reset_control_get_optional_shared(&ofdev->dev, NULL);
> if (IS_ERR(info->rst)) {
> ret = PTR_ERR(info->rst);
> - goto err_dispose;
> + goto err_unprepare;
> }
>
> ret = reset_control_deassert(info->rst);
> if (ret)
> - goto err_dispose;
> + goto err_unprepare;
>
> port->type = type;
> port->uartclk = clk;
> @@ -184,8 +190,6 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
> port->handle_irq = fsl8250_handle_irq;
>
> return 0;
> -err_dispose:
> - irq_dispose_mapping(port->irq);
> err_unprepare:
> clk_disable_unprepare(info->clk);
> err_pmruntime:
>
^ permalink raw reply
* [PATH v5 0/4] gpiolib: speed up GPIO array processing
From: Janusz Krzysztofik @ 2018-08-29 20:48 UTC (permalink / raw)
To: Linus Walleij
Cc: Andrew Lunn, Ulf Hansson, linux-doc, linux-iio, Dominik Brodowski,
Peter Rosin, netdev, linux-i2c, Peter Meerwald-Stadler, devel,
Florian Fainelli, Jonathan Corbet, Kishon Vijay Abraham I,
Geert Uytterhoeven, linux-serial, Jiri Slaby, Michael Hennerich,
linux-gpio, Lars-Peter Clausen, Greg Kroah-Hartman, linux-mmc,
linux-kernel, Willy Tarreau, Miguel Ojeda Sandonis,
Peter Korsgaard
In-Reply-To: <20180820234341.5271-1-jmkrzyszt@gmail.com>
The goal is to boost performance of get/set array functions while
processing GPIO arrays which represent pins of a signle chip in
hardware order. If resulting performance is close to PIO, GPIO API
can be used for data I/O without much loss of speed.
Created and tested on a low end Amstrad Delta board with NAND driver
updated to use GPIO API for data I/O. Performance degrade compared to
PIO is much better than before the optimization but still not quite
satisfactory.
Janusz Krzysztofik (4):
gpiolib: Pass bitmaps, not integer arrays, to get/set array
gpiolib: Identify arrays matching GPIO hardware
gpiolib: Pass array info to get/set array functions
gpiolib: Implement fast processing path in get/set array
Changelog:
v5:
[PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set
- drivers/i2c/muxes/i2c-mux-gpio.c:
- drop assigment of values to struct gpiomux.values, as recommended
by Peter Rosin - thanks!,
- mark the .values member of the structure as obsolete,
- drivers/mux/gpio.c:
- drop assigment of values to struct mux_gpio.val, also recommended
by Peter Rosin - thanks!,
- merk the .val member of the structure as obsolete,
- drivers/auxdisplay/hd44780.c:
- fix incorrect bitmap size,
- use >>= operator to simplify notation,
both catched by Miguel Ojeda - thanks!,
- add Cc: clauses as well as Acked-by: collected so far.
[PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware
- add Cc: clause.
[PATCH v5 3/4] gpiolib: Pass array info to get/set array functions
- add Cc: clauses as well as Acked-by: collected so far.
[PATCH v5 4/4] gpiolib: Implement fast processing path in get/set
- add Cc: clause.
v4:
That series was a follow up of the former "mtd: rawnand: ams-delta: Use
gpio-omap accessors for data I/O" which already contained some changes
to gpiolib. Those previous attempts were commented by Borris Brezillon
who suggested using GPIO API modified to accept bitmaps, and by Linus
Walleij who suggested still more great ideas for further immprovement
of the proposed API changes - thanks!
diffstat:
Documentation/driver-api/gpio/board.rst | 15 +
Documentation/driver-api/gpio/consumer.rst | 48 +++-
drivers/auxdisplay/hd44780.c | 64 +++---
drivers/bus/ts-nbus.c | 25 --
drivers/gpio/gpio-max3191x.c | 23 +-
drivers/gpio/gpiolib.c | 279 ++++++++++++++++++++++------
drivers/gpio/gpiolib.h | 15 +
drivers/i2c/muxes/i2c-mux-gpio.c | 10 -
drivers/mmc/core/pwrseq_simple.c | 15 -
drivers/mux/gpio.c | 12 -
drivers/net/phy/mdio-mux-gpio.c | 5
drivers/pcmcia/soc_common.c | 14 -
drivers/phy/motorola/phy-mapphone-mdm6600.c | 21 +-
drivers/staging/iio/adc/ad7606.c | 12 -
drivers/tty/serial/serial_mctrl_gpio.c | 9
include/linux/gpio/consumer.h | 35 ++-
16 files changed, 412 insertions(+), 190 deletions(-)
^ 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