* [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
* [PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Janusz Krzysztofik @ 2018-08-29 20:48 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, Greg Kroah-Hartman,
Kishon Vijay Abraham I, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
Jiri Slaby, Willy Tarreau, Geert Uytterhoeven
In-Reply-To: <20180829204900.19390-1-jmkrzyszt@gmail.com>
Most users of get/set array functions iterate consecutive bits of data,
usually a single integer, while 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.
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
Cc: Peter Korsgaard <peter.korsgaard@barco.com>
Cc: Peter Rosin <peda@axentia.se>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Michael Hennerich <Michael.Hennerich@analog.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Hartmut Knaack <knaack.h@gmx.de>
Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---
Documentation/driver-api/gpio/consumer.rst | 22 ++++----
drivers/auxdisplay/hd44780.c | 52 +++++++++--------
drivers/bus/ts-nbus.c | 19 ++-----
drivers/gpio/gpio-max3191x.c | 17 +++---
drivers/gpio/gpiolib.c | 86 +++++++++++++++--------------
drivers/gpio/gpiolib.h | 4 +-
drivers/i2c/muxes/i2c-mux-gpio.c | 8 +--
drivers/mmc/core/pwrseq_simple.c | 13 ++---
drivers/mux/gpio.c | 9 +--
drivers/net/phy/mdio-mux-gpio.c | 3 +-
drivers/pcmcia/soc_common.c | 11 ++--
drivers/phy/motorola/phy-mapphone-mdm6600.c | 17 +++---
drivers/staging/iio/adc/ad7606.c | 9 +--
drivers/tty/serial/serial_mctrl_gpio.c | 7 ++-
include/linux/gpio/consumer.h | 18 +++---
15 files changed, 140 insertions(+), 155 deletions(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index aa03f389d41d..ed68042ddccf 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -323,29 +323,29 @@ The following functions get or set the values of an array of GPIOs::
int gpiod_get_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
void gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
The array can be an arbitrary set of GPIOs. The functions will try to access
GPIOs belonging to the same bank or chip simultaneously if supported by the
@@ -356,8 +356,8 @@ accessed sequentially.
The functions take three arguments:
* array_size - the number of array elements
* desc_array - an array of GPIO descriptors
- * value_array - an array to store the GPIOs' values (get) or
- an array of values to assign to the GPIOs (set)
+ * value_bitmap - a bitmap to store the GPIOs' values (get) or
+ a bitmap of values to assign to the GPIOs (set)
The descriptor array can be obtained using the gpiod_get_array() function
or one of its variants. If the group of descriptors returned by that function
@@ -366,7 +366,7 @@ the struct gpio_descs returned by gpiod_get_array()::
struct gpio_descs *my_gpio_descs = gpiod_get_array(...);
gpiod_set_array_value(my_gpio_descs->ndescs, my_gpio_descs->desc,
- my_gpio_values);
+ my_gpio_value_bitmap);
It is also possible to access a completely arbitrary array of descriptors. The
descriptors may be obtained using any combination of gpiod_get() and
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index f1a42f0f1ded..bbbd6a29bf01 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 */
+ 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[1];
+ 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] >>= PIN_DATA4;
/* 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);
/* 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);
}
@@ -155,23 +153,23 @@ static void hd44780_write_cmd_gpio4(struct charlcd *lcd, int cmd)
/* Send 4-bits of a command to the LCD panel in raw 4 bit GPIO mode */
static void hd44780_write_cmd_raw_gpio4(struct charlcd *lcd, int cmd)
{
- int values[10]; /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+ /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+ unsigned long value_bitmap[1];
struct hd44780 *hd = lcd->drvdata;
- unsigned int i, n;
+ unsigned int n;
/* Command nibble + RS, RW */
- for (i = 0; i < 4; i++)
- values[PIN_DATA4 + i] = !!(cmd & BIT(i));
- values[PIN_CTRL_RS] = 0;
+ value_bitmap[0] = cmd << PIN_DATA4;
+ __clear_bit(PIN_CTRL_RS, value_bitmap);
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;
/* 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);
}
diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index 073fd9011154..ce6c1e89236d 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -110,13 +110,9 @@ static void ts_nbus_set_direction(struct ts_nbus *ts_nbus, int direction)
*/
static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
{
- int i;
- int values[8];
-
- for (i = 0; i < 8; i++)
- values[i] = 0;
+ unsigned long value_bitmap[1] = { 0, };
- gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, values);
+ gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, value_bitmap);
gpiod_set_value_cansleep(ts_nbus->csn, 0);
gpiod_set_value_cansleep(ts_nbus->strobe, 0);
gpiod_set_value_cansleep(ts_nbus->ale, 0);
@@ -157,16 +153,9 @@ static int ts_nbus_read_byte(struct ts_nbus *ts_nbus, u8 *val)
static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
{
struct gpio_descs *gpios = ts_nbus->data;
- int i;
- int values[8];
-
- for (i = 0; i < 8; i++)
- if (byte & BIT(i))
- values[i] = 1;
- else
- values[i] = 0;
+ unsigned long value_bitmap[1] = { byte, };
- gpiod_set_array_value_cansleep(8, gpios->desc, values);
+ gpiod_set_array_value_cansleep(8, gpios->desc, value_bitmap);
}
/*
diff --git a/drivers/gpio/gpio-max3191x.c b/drivers/gpio/gpio-max3191x.c
index b5b9cb1fda50..c4ec1c82af27 100644
--- a/drivers/gpio/gpio-max3191x.c
+++ b/drivers/gpio/gpio-max3191x.c
@@ -315,17 +315,20 @@ static void gpiod_set_array_single_value_cansleep(unsigned int ndescs,
struct gpio_desc **desc,
int value)
{
- int i, *values;
+ unsigned long *value_bitmap;
- values = kmalloc_array(ndescs, sizeof(*values), GFP_KERNEL);
- if (!values)
+ value_bitmap = kmalloc_array(BITS_TO_LONGS(ndescs),
+ sizeof(*value_bitmap), GFP_KERNEL);
+ if (!value_bitmap)
return;
- for (i = 0; i < ndescs; i++)
- values[i] = value;
+ if (value)
+ bitmap_fill(value_bitmap, ndescs);
+ else
+ bitmap_zero(value_bitmap, ndescs);
- gpiod_set_array_value_cansleep(ndescs, desc, values);
- kfree(values);
+ gpiod_set_array_value_cansleep(ndescs, desc, value_bitmap);
+ kfree(value_bitmap);
}
static struct gpio_descs *devm_gpiod_get_array_optional_count(
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e8f8a1999393..f0e9ffa8cab6 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -427,7 +427,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
struct linehandle_state *lh = filep->private_data;
void __user *ip = (void __user *)arg;
struct gpiohandle_data ghd;
- int vals[GPIOHANDLES_MAX];
+ unsigned long value_bitmap[BITS_TO_LONGS(GPIOHANDLES_MAX)];
int i;
if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) {
@@ -436,13 +436,13 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
true,
lh->numdescs,
lh->descs,
- vals);
+ value_bitmap);
if (ret)
return ret;
memset(&ghd, 0, sizeof(ghd));
for (i = 0; i < lh->numdescs; i++)
- ghd.values[i] = vals[i];
+ ghd.values[i] = test_bit(i, value_bitmap);
if (copy_to_user(ip, &ghd, sizeof(ghd)))
return -EFAULT;
@@ -461,14 +461,14 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
/* Clamp all values to [0,1] */
for (i = 0; i < lh->numdescs; i++)
- vals[i] = !!ghd.values[i];
+ __assign_bit(i, value_bitmap, !!ghd.values[i]);
/* Reuse the array setting function */
return gpiod_set_array_value_complex(false,
true,
lh->numdescs,
lh->descs,
- vals);
+ value_bitmap);
}
return -EINVAL;
}
@@ -2784,7 +2784,7 @@ static int gpio_chip_get_multiple(struct gpio_chip *chip,
int gpiod_get_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
int i = 0;
@@ -2835,7 +2835,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
- value_array[j] = value;
+ __assign_bit(j, value_bitmap, value);
trace_gpio_value(desc_to_gpio(desc), 1, value);
}
@@ -2895,9 +2895,9 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
/**
* gpiod_get_raw_array_value() - read raw values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitmap: bitmap to store the read values
*
* Read the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status. Return 0 in case of success,
@@ -2907,20 +2907,21 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
* and it will complain if the GPIO chip functions potentially sleep.
*/
int gpiod_get_raw_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(true, false, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
/**
* gpiod_get_array_value() - read values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitnap: bitmap to store the read values
*
* Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account. Return 0 in case of success, else an error code.
@@ -2929,12 +2930,13 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
* and it will complain if the GPIO chip functions potentially sleep.
*/
int gpiod_get_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(false, false, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_array_value);
@@ -3027,7 +3029,7 @@ static void gpio_chip_set_multiple(struct gpio_chip *chip,
int gpiod_set_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
int i = 0;
@@ -3056,7 +3058,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
do {
struct gpio_desc *desc = desc_array[i];
int hwgpio = gpio_chip_hwgpio(desc);
- int value = value_array[i];
+ int value = test_bit(i, value_bitmap);
if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
@@ -3152,9 +3154,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
/**
* gpiod_set_raw_array_value() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status.
@@ -3163,20 +3165,21 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
* complain if the GPIO chip functions potentially sleep.
*/
int gpiod_set_raw_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return -EINVAL;
return gpiod_set_array_value_complex(true, false, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
/**
* gpiod_set_array_value() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account.
@@ -3185,12 +3188,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
* complain if the GPIO chip functions potentially sleep.
*/
void gpiod_set_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array)
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap)
{
if (!desc_array)
return;
gpiod_set_array_value_complex(false, false, array_size, desc_array,
- value_array);
+ value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_array_value);
@@ -3410,9 +3414,9 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
/**
* gpiod_get_raw_array_value_cansleep() - read raw values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitmap: bitmap to store the read values
*
* Read the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status. Return 0 in case of success,
@@ -3422,21 +3426,21 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
*/
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(true, true, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
/**
* gpiod_get_array_value_cansleep() - read values from an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be read
- * @value_array: array to store the read values
+ * @value_bitmap: bitmap to store the read values
*
* Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account. Return 0 in case of success, else an error code.
@@ -3445,13 +3449,13 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
*/
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return -EINVAL;
return gpiod_get_array_value_complex(false, true, array_size,
- desc_array, value_array);
+ desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_get_array_value_cansleep);
@@ -3493,9 +3497,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
/**
* gpiod_set_raw_array_value_cansleep() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the raw values of the GPIOs, i.e. the values of the physical lines
* without regard for their ACTIVE_LOW status.
@@ -3504,13 +3508,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
*/
int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return -EINVAL;
return gpiod_set_array_value_complex(true, true, array_size, desc_array,
- value_array);
+ value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value_cansleep);
@@ -3533,9 +3537,9 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
/**
* gpiod_set_array_value_cansleep() - assign values to an array of GPIOs
- * @array_size: number of elements in the descriptor / value arrays
+ * @array_size: number of elements in the descriptor array / value bitmap
* @desc_array: array of GPIO descriptors whose values will be assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
*
* Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
* into account.
@@ -3544,13 +3548,13 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
*/
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array)
+ unsigned long *value_bitmap)
{
might_sleep_if(extra_checks);
if (!desc_array)
return;
gpiod_set_array_value_complex(false, true, array_size, desc_array,
- value_array);
+ value_bitmap);
}
EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index a7e49fef73d4..11e83d2eef89 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -187,11 +187,11 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, u16 hwnum);
int gpiod_get_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_set_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
/* This is just passed between gpiolib and devres */
struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 401308e3d036..4e36e0eac7a3 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -22,18 +22,16 @@ struct gpiomux {
struct i2c_mux_gpio_platform_data data;
unsigned gpio_base;
struct gpio_desc **gpios;
- int *values;
+ int *values; /* FIXME: no longer needed */
};
static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
{
+ unsigned long value_bitmap[1] = { val, };
int i;
- for (i = 0; i < mux->data.n_gpios; i++)
- mux->values[i] = (val >> i) & 1;
-
gpiod_set_array_value_cansleep(mux->data.n_gpios,
- mux->gpios, mux->values);
+ mux->gpios, value_bitmap);
}
static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index a8b9fee4d62a..0d6e3a5be3ba 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -40,18 +40,13 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
if (!IS_ERR(reset_gpios)) {
- int i, *values;
+ unsigned long value_bitmap[1];
int nvalues = reset_gpios->ndescs;
- values = kmalloc_array(nvalues, sizeof(int), GFP_KERNEL);
- if (!values)
- return;
+ value_bitmap[0] = value;
- for (i = 0; i < nvalues; i++)
- values[i] = value;
-
- gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc, values);
- kfree(values);
+ gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
+ value_bitmap);
}
}
diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index 6fdd9316db8b..734e1b43aed6 100644
--- a/drivers/mux/gpio.c
+++ b/drivers/mux/gpio.c
@@ -17,20 +17,17 @@
struct mux_gpio {
struct gpio_descs *gpios;
- int *val;
+ int *val; /* FIXME: no longer needed */
};
static int mux_gpio_set(struct mux_control *mux, int state)
{
struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
+ unsigned long value_bitmap[1] = { state, };
int i;
- for (i = 0; i < mux_gpio->gpios->ndescs; i++)
- mux_gpio->val[i] = (state >> i) & 1;
-
gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
- mux_gpio->gpios->desc,
- mux_gpio->val);
+ mux_gpio->gpios->desc, value_bitmap);
return 0;
}
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index bc90764a8b8d..8e1ec750277e 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -27,6 +27,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
void *data)
{
struct mdio_mux_gpio_state *s = data;
+ unsigned long value_bitmap[1] = { desired_child, };
unsigned int n;
if (current_child == desired_child)
@@ -36,7 +37,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
s->values[n] = (desired_child >> n) & 1;
gpiod_set_array_value_cansleep(s->gpios->ndescs, s->gpios->desc,
- s->values);
+ value_bitmap);
return 0;
}
diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index c5f2344c189b..e0f89155c474 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -351,19 +351,22 @@ static int soc_common_pcmcia_config_skt(
if (ret == 0) {
struct gpio_desc *descs[2];
- int values[2], n = 0;
+ unsigned long value_bitmap[1];
+ int n = 0;
if (skt->gpio_reset) {
descs[n] = skt->gpio_reset;
- values[n++] = !!(state->flags & SS_RESET);
+ __assign_bit(n++, value_bitmap,
+ !!(state->flags & SS_RESET));
}
if (skt->gpio_bus_enable) {
descs[n] = skt->gpio_bus_enable;
- values[n++] = !!(state->flags & SS_OUTPUT_ENA);
+ __assign_bit(n++, value_bitmap,
+ !!(state->flags & SS_OUTPUT_ENA));
}
if (n)
- gpiod_set_array_value_cansleep(n, descs, values);
+ gpiod_set_array_value_cansleep(n, descs, value_bitmap);
/*
* This really needs a better solution. The IRQ
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
index 0075fb0bef8c..b6477c3599c4 100644
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -157,15 +157,12 @@ static const struct phy_ops gpio_usb_ops = {
*/
static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
{
- int values[PHY_MDM6600_NR_CMD_LINES];
- int i;
+ unsigned long value_bitmap[1];
- val &= (1 << PHY_MDM6600_NR_CMD_LINES) - 1;
- for (i = 0; i < PHY_MDM6600_NR_CMD_LINES; i++)
- values[i] = (val & BIT(i)) >> i;
+ value_bitmap[0] = val & ((1 << PHY_MDM6600_NR_CMD_LINES) - 1);
gpiod_set_array_value_cansleep(PHY_MDM6600_NR_CMD_LINES,
- ddata->cmd_gpios->desc, values);
+ ddata->cmd_gpios->desc, value_bitmap);
}
/**
@@ -176,7 +173,7 @@ static void phy_mdm6600_status(struct work_struct *work)
{
struct phy_mdm6600 *ddata;
struct device *dev;
- int values[PHY_MDM6600_NR_STATUS_LINES];
+ unsigned long value_bitmap[1] = { 0, };
int error, i, val = 0;
ddata = container_of(work, struct phy_mdm6600, status_work.work);
@@ -184,14 +181,14 @@ static void phy_mdm6600_status(struct work_struct *work)
error = gpiod_get_array_value_cansleep(PHY_MDM6600_NR_STATUS_LINES,
ddata->status_gpios->desc,
- values);
+ value_bitmap);
if (error)
return;
for (i = 0; i < PHY_MDM6600_NR_STATUS_LINES; i++) {
- val |= values[i] << i;
+ val |= test_bit(i, value_bitmap) << i;
dev_dbg(ddata->dev, "XXX %s: i: %i values[i]: %i val: %i\n",
- __func__, i, values[i], val);
+ __func__, i, test_bit(i, value_bitmap), val);
}
ddata->status = val;
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 25b9fcd5e3a4..0eca047bc1cc 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -202,7 +202,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
long mask)
{
struct ad7606_state *st = iio_priv(indio_dev);
- int values[3];
+ unsigned long value_bitmap[1];
int ret, i;
switch (mask) {
@@ -227,13 +227,10 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
if (ret < 0)
return ret;
- values[0] = (ret >> 0) & 1;
- values[1] = (ret >> 1) & 1;
- values[2] = (ret >> 2) & 1;
+ value_bitmap[0] = ret;
mutex_lock(&st->lock);
- gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
- values);
+ gpiod_set_array_value(3, st->gpio_os->desc, value_bitmap);
st->oversampling = val;
mutex_unlock(&st->lock);
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 1c06325beaca..bb8b4756d72d 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -40,7 +40,7 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
{
enum mctrl_gpio_idx i;
struct gpio_desc *desc_array[UART_GPIO_MAX];
- int value_array[UART_GPIO_MAX];
+ unsigned long value_bitmap[BITS_TO_LONGS(UART_GPIO_MAX)];
unsigned int count = 0;
if (gpios == NULL)
@@ -49,10 +49,11 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
for (i = 0; i < UART_GPIO_MAX; i++)
if (gpios->gpio[i] && mctrl_gpios_desc[i].dir_out) {
desc_array[count] = gpios->gpio[i];
- value_array[count] = !!(mctrl & mctrl_gpios_desc[i].mctrl);
+ __assign_bit(count, value_bitmap,
+ !!(mctrl & mctrl_gpios_desc[i].mctrl));
count++;
}
- gpiod_set_array_value(count, desc_array, value_array);
+ gpiod_set_array_value(count, desc_array, value_bitmap);
}
EXPORT_SYMBOL_GPL(mctrl_gpio_set);
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 21ddbe440030..1b21dc7b0fad 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -104,36 +104,38 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
/* Value get/set from non-sleeping context */
int gpiod_get_value(const struct gpio_desc *desc);
int gpiod_get_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array);
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap);
void gpiod_set_value(struct gpio_desc *desc, int value);
void gpiod_set_array_value(unsigned int array_size,
- struct gpio_desc **desc_array, int *value_array);
+ struct gpio_desc **desc_array,
+ unsigned long *value_bitmap);
int gpiod_get_raw_value(const struct gpio_desc *desc);
int gpiod_get_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_raw_value(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
/* Value get/set from sleeping context */
int gpiod_get_value_cansleep(const struct gpio_desc *desc);
int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
- int *value_array);
+ unsigned long *value_bitmap);
int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
--
2.16.4
^ permalink raw reply related
* [PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware
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, Janusz Krzysztofik,
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
In-Reply-To: <20180829204900.19390-1-jmkrzyszt@gmail.com>
Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order. If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.
While processing a request for an array of GPIO descriptors, identify
those which represent corresponding pins of a single GPIO chip. Skip
over pins which require open source or open drain special processing.
Moreover, identify pins which require inversion. Pass a pointer to
that information with the array to the caller so it can benefit from
enhanced performance as soon as get/set array functions can accept and
make efficient use of it.
Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Documentation/driver-api/gpio/consumer.rst | 4 +-
drivers/gpio/gpiolib.c | 72 +++++++++++++++++++++++++++++-
drivers/gpio/gpiolib.h | 9 ++++
include/linux/gpio/consumer.h | 9 ++++
4 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index ed68042ddccf..7e0298b9a7b9 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,9 +109,11 @@ For a function using multiple GPIOs all of those can be obtained with one call::
enum gpiod_flags flags)
This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors. It also contains a pointer to a gpiolib private structure which,
+if passed back to get/set array functions, may speed up I/O proocessing::
struct gpio_descs {
+ struct gpio_array *info;
unsigned int ndescs;
struct gpio_desc *desc[];
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f0e9ffa8cab6..c1ed1c759345 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4174,7 +4174,9 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
{
struct gpio_desc *desc;
struct gpio_descs *descs;
- int count;
+ struct gpio_array *array_info = NULL;
+ struct gpio_chip *chip;
+ int count, bitmap_size;
count = gpiod_count(dev, con_id);
if (count < 0)
@@ -4190,9 +4192,77 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
gpiod_put_array(descs);
return ERR_CAST(desc);
}
+
descs->desc[descs->ndescs] = desc;
+
+ chip = gpiod_to_chip(desc);
+ /*
+ * Select a chip of first array member
+ * whose index matches its pin hardware number
+ * as a candidate for fast bitmap processing.
+ */
+ if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+ struct gpio_descs *array;
+
+ bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
+ chip->ngpio : count);
+
+ array = kzalloc(struct_size(descs, desc, count) +
+ struct_size(array_info, invert_mask,
+ 3 * bitmap_size), GFP_KERNEL);
+ if (!array) {
+ gpiod_put_array(descs);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ memcpy(array, descs,
+ struct_size(descs, desc, descs->ndescs + 1));
+ kfree(descs);
+
+ descs = array;
+ array_info = (void *)(descs->desc + count);
+ array_info->get_mask = array_info->invert_mask +
+ bitmap_size;
+ array_info->set_mask = array_info->get_mask +
+ bitmap_size;
+
+ array_info->desc = descs->desc;
+ array_info->size = count;
+ array_info->chip = chip;
+ bitmap_set(array_info->get_mask, descs->ndescs,
+ count - descs->ndescs);
+ bitmap_set(array_info->set_mask, descs->ndescs,
+ count - descs->ndescs);
+ descs->info = array_info;
+ }
+ /*
+ * Unmark members which don't qualify for fast bitmap
+ * processing (different chip, not in hardware order)
+ */
+ if (array_info && (chip != array_info->chip ||
+ gpio_chip_hwgpio(desc) != descs->ndescs)) {
+ __clear_bit(descs->ndescs, array_info->get_mask);
+ __clear_bit(descs->ndescs, array_info->set_mask);
+ } else if (array_info) {
+ /* Exclude open drain or open source from fast output */
+ if (gpiochip_line_is_open_drain(chip, descs->ndescs) ||
+ gpiochip_line_is_open_source(chip, descs->ndescs))
+ __clear_bit(descs->ndescs,
+ array_info->set_mask);
+ /* Identify 'fast' pins which require invertion */
+ if (gpiod_is_active_low(desc))
+ __set_bit(descs->ndescs,
+ array_info->invert_mask);
+ }
+
descs->ndescs++;
}
+ if (array_info)
+ dev_dbg(dev,
+ "GPIO array info: chip=%s, size=%d, get_mask=%lx, set_mask=%lx, invert_mask=%lx\n",
+ array_info->chip->label, array_info->size,
+ *array_info->get_mask, *array_info->set_mask,
+ *array_info->invert_mask);
return descs;
}
EXPORT_SYMBOL_GPL(gpiod_get_array);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 11e83d2eef89..b60905d558b1 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -183,6 +183,15 @@ static inline bool acpi_can_fallback_to_crs(struct acpi_device *adev,
}
#endif
+struct gpio_array {
+ struct gpio_desc **desc;
+ unsigned int size;
+ struct gpio_chip *chip;
+ unsigned long *get_mask;
+ unsigned long *set_mask;
+ unsigned long invert_mask[];
+};
+
struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, u16 hwnum);
int gpiod_get_array_value_complex(bool raw, bool can_sleep,
unsigned int array_size,
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 1b21dc7b0fad..8dede3e886af 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -17,11 +17,20 @@ struct device;
*/
struct gpio_desc;
+/**
+ * Opaque descriptor for a structure of GPIO array attributes. This structure
+ * is attached to struct gpiod_descs obtained from gpiod_get_array() and can be
+ * passed back to get/set array functions in order to activate fast processing
+ * path if applicable.
+ */
+struct gpio_array;
+
/**
* Struct containing an array of descriptors that can be obtained using
* gpiod_get_array().
*/
struct gpio_descs {
+ struct gpio_array *info;
unsigned int ndescs;
struct gpio_desc *desc[];
};
--
2.16.4
^ permalink raw reply related
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