Re: [PATCH 3/7] hwmon: (max31790) Allow setting pulses
@@ -285,6 +295,9 @@ static int max31790_write_fan(struct device *dev, u32 attr, int channel, MAX31790_REG_FAN_CONFIG(channel), data->fan_config[channel]); break; + case hwmon_fan_pulses: + data->pulses[channel] = val; + break; This needs input validation, otherwise it's possible to write 0 in there and you get a division-by-zero in the kernel context: [102109.68] Division by zero in kernel. [102110.003917] CPU: 1 PID: 27590 Comm: cat Not tainted 5.9.3-cla-cfb #42 [102110.010462] Hardware name: Marvell Armada 380/385 (Device Tree) [102110.016497] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [102110.024355] [] (show_stack) from [] (dump_stack+0x94/0xa8) [102110.031689] [] (dump_stack) from [] (Ldiv0+0x8/0x2c) [102110.038499] [] (Ldiv0) from [] (max31790_read+0x174/0x204) [102110.045836] [] (max31790_read) from [] (hwmon_attr_show+0x44/0x138) ... A similar error can also happen when setting the fan speed to 0 RPM. That's, however, not an error caused by this patch series AFAIK. I *think* that RPM_TO_REG should be changed to check if `rpm` is 0, and if so, set the register directly to the maximal value of 0x7ff (in another patch). With kind regards, Jan
Re: [PATCH v2] serial: max310x: rework RX interrupt handling
On čtvrtek 1. října 2020 9:44:15 CEST, Thomas Petazzoni wrote: Currently, the RX interrupt logic uses the RXEMPTY interrupt, with the RXEMPTYINV bit set, which means we get an RX interrupt as soon as the RX FIFO is non-empty. However, with the MAX310X having a FIFO of 128 bytes, this makes very poor use of the FIFO: we trigger an interrupt as soon as the RX FIFO has one byte, which means a lot of interrupts, each only collecting a few bytes from the FIFO, causing a significant CPU load. Thanks for taking the time to write this patch. We're using MAX14830 on a Clearfog Base board via a 26 MHz SPI bus. Our code polls a custom peripheral over UART at 115200 baud ten times a second; the messages are typically shorter than 50 chars. Before this patch, `perf top --sort comm,dso` showed about 28% CPU load for the corresponding SPI kthread, after applying this patch it's between 3 and 5%. That's cool :). Tested-by: Jan Kundrát Reviewed-by: Jan Kundrát (but see below, please) + /* Enable LSR, RX FIFO trigger, CTS change interrupts */ + val = MAX310X_IRQ_LSR_BIT | MAX310X_IRQ_RXFIFO_BIT | MAX310X_IRQ_TXEMPTY_BIT; max310x_port_write(port, MAX310X_IRQEN_REG, val | MAX310X_IRQ_CTS_BIT); This comment doesn't fully match that code, and also the effective value that is written to the register is split into two statements. What about just: + /* Enable LSR, RX FIFO trigger, TX FIFO empty, CTS change interrupts */ + max310x_port_write(port, MAX310X_IRQEN_REG, MAX310X_IRQ_LSR_BIT | MAX310X_IRQ_RXFIFO_BIT | MAX310X_IRQ_TXEMPTY_BIT | MAX310X_IRQ_CTS_BIT); With kind regards, Jan
Re: [PATCH] pinctrl-mcp23s08: Fix property-name in dt-example
On středa 28. srpna 2019 21:56:09 CEST, Peter Vernia wrote:
The device-tree properties documentation-file specifies the property
"microchip,spi-present-mask" as required for MCP23SXX chips. However,
the device-tree-source example below it uses only "spi-present-mask".
Without "microchip," on the front, the driver will print "missing
spi-present-mask" when it initializes.
Update the device-tree example with the correct property-name.
Signed-off-by: Peter Vernia
Reviewed-by: Jan Kundrát
---
Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
index 625a22e2f211..8b94aa8f5971 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
@@ -82,7 +82,7 @@ gpiom1: gpio@0 {
compatible = "microchip,mcp23s17";
gpio-controller;
#gpio-cells = <2>;
-spi-present-mask = <0x01>;
+microchip,spi-present-mask = <0x01>;
reg = <0>;
spi-max-frequency = <100>;
};
Re: [PATCH v1] tty: serial: max310x: Add optional reset gpio
On pátek 14. června 2019 16:11:12 CEST, Mylène Josserand wrote:
--- a/Documentation/devicetree/bindings/serial/maxim,max310x.txt
+++ b/Documentation/devicetree/bindings/serial/maxim,max310x.txt
@@ -15,6 +15,7 @@ Required properties:
"osc" if an external clock source is used.
Optional properties:
+- reset-gpios: Gpio to use for reset.
"GPIO", not "Gpio", for consistency.
if (spi->dev.of_node) {
+ struct gpio_desc *reset_gpio;
const struct of_device_id *of_id =
of_match_device(max310x_dt_ids, >dev);
if (!of_id)
return -ENODEV;
devtype = (struct max310x_devtype *)of_id->data;
+ reset_gpio = devm_gpiod_get_optional(>dev, "reset",
+GPIOD_OUT_HIGH);
+ if (IS_ERR(reset_gpio))
+ return PTR_ERR(reset_gpio);
+ gpiod_set_value_cansleep(reset_gpio, 0);
} else {
The RST signal is active-low on the chip, but the code initializes the
output to GPIOD_OUT_HIGH. Are you perhaps relying on a DT binding setting
an ACTIVE_LOW flag on the reset GPIO lane? This should be documented.
Assuming that this polarity inversion works, the code first asserts the
reset, then it performs no explicit waiting, and then it clears the RST
signal. I checked MAX14830's datasheet, and there's no minimal reset
duration, so perhaps this is safe, but it looks a bit odd to me.
With kind regards,
Jan
Re: [PATCH] pinctrl: mcp23s08: Allocate irq_chip dynamic
On pátek 8. března 2019 10:15:50 CET, Linus Walleij wrote: On Tue, Mar 5, 2019 at 4:19 PM Jan Kundrát wrote: This commit should probably go to 5.0-stable and 4.20-stable as well I doubt that this commit is fixing a regression, but who knows... If I cherry-pick the following two commits to my 5.0, my board boots once again: 16f4372fd7a5 pinctrl: mcp23s08: use struct_size() in devm_kzalloc() 19ab5ca9b77d pinctrl: mcp23s08: Allocate irq_chip dynamic Hm weird. My working theory is that this is because I have two physical chips on the same SPI CS. If I am correct, it works like this between 4.20 and current gpio-next: - GPIO expander #1 is initialized, including the irqchip - GPIO expander #2 is initialized, and the irqchip initialization fails - an interrupt on a shared IRQ line which is common to GPIO expanders #1 and #2 and one other unrelated device starts to be processed - GPIO expander #1 says "not mine interrupt" - GPIO expander #2 attempts to dereference a null pointer becase since 171948ea33e14, the gpiochip->irq.irq_enable/disable are not initialized As on "why am I hitting this while nobody else did", my board has a shared IRQ line which is active during bootup, perhaps that might have something to do with this -- I don't know. Sounds like the code in mcp23s08 .probe() should write to all registers clearing and disabling interrupts before proceeding to request interrupts or register the irqchip, else it will fire immediately and cause oopses. I do not think that this would help me. My IRQ line is shared with another chip, so it can be physically asserted even if this particular GPIO expander doesn't generate an interrupt. I suspect that this configuration (multiple MCP23S17 on one SPI CS, and an IRQ line shared with something else) is quite rare... With kind regards, Jan
Re: [PATCH] serial: max310x: Use struct_size() in devm_kzalloc()
On pátek 4. ledna 2019 22:39:13 CET, Gustavo A. R. Silva wrote:
One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:
struct foo {
int stuff;
void *entry[];
};
instance = devm_kzalloc(dev, sizeof(struct foo) + sizeof(void
*) * count, GFP_KERNEL);
Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:
instance = devm_kzalloc(dev, struct_size(instance, entry,
count), GFP_KERNEL);
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva
---
drivers/tty/serial/max310x.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 4f479841769a..1d700a8ab517 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1197,8 +1197,7 @@ static int max310x_probe(struct device
*dev, struct max310x_devtype *devtype,
return PTR_ERR(regmap);
/* Alloc port structure */
- s = devm_kzalloc(dev, sizeof(*s) +
-sizeof(struct max310x_one) * devtype->nr, GFP_KERNEL);
+ s = devm_kzalloc(dev, struct_size(s, p, devtype->nr), GFP_KERNEL);
if (!s) {
dev_err(dev, "Error allocating port structure\n");
return -ENOMEM;
Reviewed-by: Jan Kundrát
Tested-by: Jan Kundrát
Re: [PATCH] pinctrl: mcp23s08: Allocate irq_chip dynamic
On pondělí 21. ledna 2019 14:19:49 CET, Linus Walleij wrote: On Fri, Jan 11, 2019 at 4:16 PM Lars Poeschel wrote: Keeping the irq_chip definition static shares it with multiple instances of the mcp23s08 gpiochip in the system. This is bad and now we get this warning from gpiolib core: "detected irqchip that is shared with multiple gpiochips: please fix the driver." Hence, move the irq_chip definition from being driver static into the struct mcp23s08. So a unique irq_chip is used for each gpiochip instance. Signed-off-by: Lars Poeschel Patch applied. Tested-by: Jan Kundrát Fixes: 171948ea33e14 (gpiolib: check if irqchip already has the irq hook replacements) This commit should probably go to 5.0-stable and 4.20-stable as well because commit 171948ea33e14 first appeared in 4.20. I have been using 4.19 previsouly, which works, but 5.0 oopses on boot for me with two MCP23S17 chips (sorry, I just don't know how to get more than 10 lines of stack trace in this oops): [1.011926] gpio gpiochip3: (mcp23s17.1): detected irqchip that is shared with multiple gpiochips: please fix the driver. [1.022936] GPIO line 486 (I2C XOR ready) hogged as input [1.028550] Unable to handle kernel NULL pointer dereference at virtual address 0054 [1.036675] pgd = (ptrval) [1.039388] [0054] *pgd= [1.042980] Internal error: Oops: 5 [#1] SMP ARM [1.047613] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.0.0 #1 [1.053460] Hardware name: Marvell Armada 380/385 (Device Tree) [1.059401] PC is at __dev_printk+0x10/0x84 [1.063595] LR is at _dev_info+0x44/0x68 [1.067528] pc : []lr : []psr: 2013 [1.073811] sp : ef4afbb4 ip : ef4afbbc fp : eed0a040 [1.079048] r10: r9 : c0b32834 r8 : [1.084286] r7 : c016f8cc r6 : r5 : ef9f7ba0 r4 : c0b07bc8 [1.090829] r3 : ef4afbd8 r2 : ef4afbbc r1 : 0008 r0 : c08cf6fc [1.097374] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [1.104528] Control: 10c5387d Table: 404a DAC: 0051 [1.110289] Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) [1.116311] Stack: (0xef4afbb4 to 0xef4b) [1.120679] fba0: c0436764 ef4afbd8 c08f7134 [1.128881] fbc0: ef4afbb8 36415da1 c0bbc194 eed0a08c c03bce58 c08f7134 ef714c40 [1.137082] fbe0: eed0a08c 0002 eecf9840 eed0a064 c0b07bc8 eed0a400 eed0a08c 0002 [1.145283] fc00: eecf9840 c03b9484 0001 4c4c 36415da1 [1.153485] fc20: 0003 eed0a400 0002 c09019b8 ef4afc5c c03b97ac [1.161686] fc40: 0001 0010 0001 010c c0b07bc8 0002 0006 [1.169887] fc60: 36415da1 eed0a400 c0b32768 [1.178088] fc80: c0b32778 c04d0218 c0bb8590 eed0a400 c0bb859c c04391e8 [1.186290] fca0: eed0a400 c0b32778 ef4afd1c c0439848 0001 c043965c [1.194491] fcc0: eed0a400 c0b32778 c0b32768 c0b07bc8 ef4afd1c c0439848 0001 [1.202692] fce0: c0437ca4 ef54cc6c ef5c8938 36415da1 [1.210894] fd00: eed0a400 eed0a400 c0b07bc8 eed0a434 eee79c00 c04394c4 c093b958 eed0a400 [1.219095] fd20: 0001 36415da1 eed0a408 eed0a400 c0b43fd4 eee79c00 eed0a400 c0437e60 [1.227296] fd40: eed0a408 c0b07bc8 c0436244 0006 c04d56f8 eed0a400 [1.235497] fd60: 01be 36415da1 eed0a400 eee79c00 ef575410 0001 ef9f7bec [1.243699] fd80: c0b07bc8 c04d2620 eee79c00 ef9f7ba0 eed0a400 0001 c04d2e4c [1.251900] fda0: c05cff48 c0901258 c0913c9c c0913ca8 c0913dd4 00989680 36415da1 [1.260102] fdc0: eee79f48 eee79c00 ef575400 eee79f48 ef575410 0002 c0b07bc8 f0997680 [1.268303] fde0: c08d00d8 c04d618c c02ab604 c08f7a40 c09141b8 eee79c00 c0b07bc8 [1.276504] fe00: 0003 36415da1 c08fed94 ef575410 c0b449bc [1.284706] fe20: c0b449bc c0a5185c c043a8c4 c0bb8590 ef575410 c0bb859c [1.292907] fe40: c04391e8 ef575410 c0b449bc ef575444 c0439768 c0a5183c [1.301108] fe60: c0a004a8 c043965c ef9f7144 c0439768 ef575410 c0b449bc ef575444 [1.309310] fe80: c0439768 c0a5183c c0a004a8 c0a5185c c0439844 ef579a34 c0b07bc8 [1.317511] fea0: c0b449bc c043771c ef484d58 ef579a34 36415da1 c0b3b638 c0b449bc [1.325712] fec0: ef737c80 c0b3b638 c0438114 c0914250 c0439fa8 c0b449bc c0b449bc [1.333914] fee0: c0b07bc8 c0a2f38c e000 c0439f78 c0b62260 c0b07bc8 c0a2f38c c0a0101c [1.342115] ff00: efffcdb7 c013dd34 c0941a58 c0921800 0006 0006 c08d5294 [1.350316] ff20: c0b07bc8 c08e2294 c08d5308 c0b64e00 efffcda4 efffcdb2 36415da1 [1.358518] ff40: c0a51840 c0b62260 c0a60a8
Re: [PATCH] spidev: Enable the Liebherr's BK4 board to work with spidev driver
On pátek 28. září 2018 0:31:52 CEST, Lukasz Majewski wrote: With this commit the DSPI driver on the BK4 board can be used for SPI transmission managed from user space (via /dev/spidev0.0). Hi Lukasz, I tried submitting a similar patch for another devicem but got no reaction [1]. In the meanwhile, there has been another proposal [2] which moves handling of this into userspace via udev. You might want to take a look at that mechanism instead (and perhaps send your Tested-by tag). It has worked well for me. [1] https://patchwork.kernel.org/patch/10558889/ [2] https://patchwork.kernel.org/project/spi-devel-general/list/?series=21295 Cheers, Jan
Re: [PATCH] spidev: Enable the Liebherr's BK4 board to work with spidev driver
On pátek 28. září 2018 0:31:52 CEST, Lukasz Majewski wrote: With this commit the DSPI driver on the BK4 board can be used for SPI transmission managed from user space (via /dev/spidev0.0). Hi Lukasz, I tried submitting a similar patch for another devicem but got no reaction [1]. In the meanwhile, there has been another proposal [2] which moves handling of this into userspace via udev. You might want to take a look at that mechanism instead (and perhaps send your Tested-by tag). It has worked well for me. [1] https://patchwork.kernel.org/patch/10558889/ [2] https://patchwork.kernel.org/project/spi-devel-general/list/?series=21295 Cheers, Jan
Re: [BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
On pondělí 24. září 2018 16:52:18 CEST, Thomas Petazzoni wrote:
Hello,
On Mon, 24 Sep 2018 15:15:12 +0100, Lorenzo Pieralisi wrote:
I understand that, I wanted to make sure we come up with a fix asap
and what I put forward would cover everything discussed in this thread,
at least temporarily, giving us time to check ISA related issues while
unmapping IO space.
Something like this should implemented what you suggest I guess:
diff --git a/drivers/pci/controller/pci-mvebu.c
b/drivers/pci/controller/pci-mvebu.c
index 50eb0729385b..a41d79b8d46a 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -1145,7 +1145,6 @@ static int
mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
{
struct device *dev = >pdev->dev;
struct device_node *np = dev->of_node;
- unsigned int i;
int ret;
INIT_LIST_HEAD(>resources);
@@ -1179,13 +1178,58 @@ static int
mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
resource_size(>io) - 1);
pcie->realio.name = "PCI I/O";
+ pci_add_resource(>resources, >realio);
+ }
+
+ return devm_request_pci_bus_resources(dev, >resources);
+}
+
+/*
+ * This is a copy of pci_host_probe(), except that it does the I/O
+ * remap as the last step, once we are sure we won't fail.
+ *
+ * It should be removed once the I/O remap error handling issue has
+ * been sorted out.
+ */
+static int mvebu_pci_host_probe(struct pci_host_bridge *bridge)
+{
+ struct mvebu_pcie *pcie;
+ struct pci_bus *bus, *child;
+ int ret;
+
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (ret < 0) {
+ dev_err(bridge->dev.parent, "Scanning root bridge failed");
+ return ret;
+ }
+
+ pcie = pci_host_bridge_priv(bridge);
+ if (resource_size(>io) != 0) {
+ unsigned int i;
+
for (i = 0; i < resource_size(>realio); i += SZ_64K)
pci_ioremap_io(i, pcie->io.start + i);
+ }
- pci_add_resource(>resources, >realio);
+ bus = bridge->bus;
+
+ /*
+* We insert PCI resources into the iomem_resource and
+* ioport_resource trees in either pci_bus_claim_resources()
+* or pci_bus_assign_resources().
+*/
+ if (pci_has_flag(PCI_PROBE_ONLY)) {
+ pci_bus_claim_resources(bus);
+ } else {
+ pci_bus_size_bridges(bus);
+ pci_bus_assign_resources(bus);
+
+ list_for_each_entry(child, >children, node)
+ pcie_bus_configure_settings(child);
}
- return devm_request_pci_bus_resources(dev, >resources);
+ pci_bus_add_devices(bus);
+ return 0;
}
static int mvebu_pcie_probe(struct platform_device *pdev)
@@ -1268,7 +1312,7 @@ static int mvebu_pcie_probe(struct
platform_device *pdev)
bridge->align_resource = mvebu_pcie_align_resource;
bridge->msi = pcie->msi;
- return pci_host_probe(bridge);
+ return mvebu_pci_host_probe(bridge);
}
static const struct of_device_id mvebu_pcie_of_match_table[] = {
If that's what you meant, I'll go ahead and test on actual hardware and
submit as a proper patch.
Thomas, Russell, Lorenzo,
did you have time to convert this into a patch which can hit 4.19? I don't
see anything related in 4.19-rc6, but perhaps I missed something. Is there
something that I should test or otherwise help?
With kind regards,
Jan
Re: [BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
On pondělí 24. září 2018 16:52:18 CEST, Thomas Petazzoni wrote:
Hello,
On Mon, 24 Sep 2018 15:15:12 +0100, Lorenzo Pieralisi wrote:
I understand that, I wanted to make sure we come up with a fix asap
and what I put forward would cover everything discussed in this thread,
at least temporarily, giving us time to check ISA related issues while
unmapping IO space.
Something like this should implemented what you suggest I guess:
diff --git a/drivers/pci/controller/pci-mvebu.c
b/drivers/pci/controller/pci-mvebu.c
index 50eb0729385b..a41d79b8d46a 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -1145,7 +1145,6 @@ static int
mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
{
struct device *dev = >pdev->dev;
struct device_node *np = dev->of_node;
- unsigned int i;
int ret;
INIT_LIST_HEAD(>resources);
@@ -1179,13 +1178,58 @@ static int
mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
resource_size(>io) - 1);
pcie->realio.name = "PCI I/O";
+ pci_add_resource(>resources, >realio);
+ }
+
+ return devm_request_pci_bus_resources(dev, >resources);
+}
+
+/*
+ * This is a copy of pci_host_probe(), except that it does the I/O
+ * remap as the last step, once we are sure we won't fail.
+ *
+ * It should be removed once the I/O remap error handling issue has
+ * been sorted out.
+ */
+static int mvebu_pci_host_probe(struct pci_host_bridge *bridge)
+{
+ struct mvebu_pcie *pcie;
+ struct pci_bus *bus, *child;
+ int ret;
+
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (ret < 0) {
+ dev_err(bridge->dev.parent, "Scanning root bridge failed");
+ return ret;
+ }
+
+ pcie = pci_host_bridge_priv(bridge);
+ if (resource_size(>io) != 0) {
+ unsigned int i;
+
for (i = 0; i < resource_size(>realio); i += SZ_64K)
pci_ioremap_io(i, pcie->io.start + i);
+ }
- pci_add_resource(>resources, >realio);
+ bus = bridge->bus;
+
+ /*
+* We insert PCI resources into the iomem_resource and
+* ioport_resource trees in either pci_bus_claim_resources()
+* or pci_bus_assign_resources().
+*/
+ if (pci_has_flag(PCI_PROBE_ONLY)) {
+ pci_bus_claim_resources(bus);
+ } else {
+ pci_bus_size_bridges(bus);
+ pci_bus_assign_resources(bus);
+
+ list_for_each_entry(child, >children, node)
+ pcie_bus_configure_settings(child);
}
- return devm_request_pci_bus_resources(dev, >resources);
+ pci_bus_add_devices(bus);
+ return 0;
}
static int mvebu_pcie_probe(struct platform_device *pdev)
@@ -1268,7 +1312,7 @@ static int mvebu_pcie_probe(struct
platform_device *pdev)
bridge->align_resource = mvebu_pcie_align_resource;
bridge->msi = pcie->msi;
- return pci_host_probe(bridge);
+ return mvebu_pci_host_probe(bridge);
}
static const struct of_device_id mvebu_pcie_of_match_table[] = {
If that's what you meant, I'll go ahead and test on actual hardware and
submit as a proper patch.
Thomas, Russell, Lorenzo,
did you have time to convert this into a patch which can hit 4.19? I don't
see anything related in 4.19-rc6, but perhaps I missed something. Is there
something that I should test or otherwise help?
With kind regards,
Jan
Re: [BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
What about something like the below. I tested it, including the error
case by forcing an -EPROBE_DEFER. The new pci_unmap_io() is modeled
after pci_unmap_iospace(). Actually, I would prefer to use
pci_remap_iospace() and pci_unmap_iospace() but for now this API
doesn't allow overloading the memory type used for the mapping.
Thanks for providing this fix so quickly, Thomas. I can confirm that this
patch -- tested on top of
54eda9df17f3215b9ed16629ee71ea07413efdaf ("Merge
tag 'pci-v4.19-fixes-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci").
Disclaimer: I
have zero familiarity with Linux' PCI code.
Tested-by: Jan Kundrát
Thanks for the testing. I'll wait for Russell to say if he is happy
(or not) with the addition of pci_unmap_io() in the ARM code, if that's
the case, I'll send a proper patch to fix the issue.
Hi Thomas, Russell,
is there a proper patch for this? I've just verified that 4.19-rc5 won't
boot for me either. Thomas' quick patch applies and makes the problem go
away.
With kind regards,
Jan
Re: [BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
What about something like the below. I tested it, including the error
case by forcing an -EPROBE_DEFER. The new pci_unmap_io() is modeled
after pci_unmap_iospace(). Actually, I would prefer to use
pci_remap_iospace() and pci_unmap_iospace() but for now this API
doesn't allow overloading the memory type used for the mapping.
Thanks for providing this fix so quickly, Thomas. I can confirm that this
patch -- tested on top of
54eda9df17f3215b9ed16629ee71ea07413efdaf ("Merge
tag 'pci-v4.19-fixes-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci").
Disclaimer: I
have zero familiarity with Linux' PCI code.
Tested-by: Jan Kundrát
Thanks for the testing. I'll wait for Russell to say if he is happy
(or not) with the addition of pci_unmap_io() in the ARM code, if that's
the case, I'll send a proper patch to fix the issue.
Hi Thomas, Russell,
is there a proper patch for this? I've just verified that 4.19-rc5 won't
boot for me either. Thomas' quick patch applies and makes the problem go
away.
With kind regards,
Jan
Re: [BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
On čtvrtek 13. září 2018 9:45:15 CEST, Thomas Petazzoni wrote:
What about something like the below. I tested it, including the error
case by forcing an -EPROBE_DEFER. The new pci_unmap_io() is modeled
after pci_unmap_iospace(). Actually, I would prefer to use
pci_remap_iospace() and pci_unmap_iospace() but for now this API
doesn't allow overloading the memory type used for the mapping.
Thanks for providing this fix so quickly, Thomas. I can confirm that this
patch -- tested on top of 54eda9df17f3215b9ed16629ee71ea07413efdaf ("Merge
tag 'pci-v4.19-fixes-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci"). Disclaimer: I
have zero familiarity with Linux' PCI code.
Tested-by: Jan Kundrát
With kind regards,
Jan
Re: [BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
On čtvrtek 13. září 2018 9:45:15 CEST, Thomas Petazzoni wrote:
What about something like the below. I tested it, including the error
case by forcing an -EPROBE_DEFER. The new pci_unmap_io() is modeled
after pci_unmap_iospace(). Actually, I would prefer to use
pci_remap_iospace() and pci_unmap_iospace() but for now this API
doesn't allow overloading the memory type used for the mapping.
Thanks for providing this fix so quickly, Thomas. I can confirm that this
patch -- tested on top of 54eda9df17f3215b9ed16629ee71ea07413efdaf ("Merge
tag 'pci-v4.19-fixes-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci"). Disclaimer: I
have zero familiarity with Linux' PCI code.
Tested-by: Jan Kundrát
With kind regards,
Jan
Re: [BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
You mean '4.19-rc3', right? Right, sorry. [1.741458] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM [1.748182] CPU: 1 PID: 72 Comm: kworker/1:2 Tainted: G W 4.19.0-rc3 #1 The 'W' taint means that there was a kernel warning before. Which warning was that? I have "spidev" listed in my DT, and the kernel complains about that; that's a harmelss warning. (I sent a patch which adds my particular device into the spidev driver some time ago.) With kind regards, Jan
Re: [BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
You mean '4.19-rc3', right? Right, sorry. [1.741458] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM [1.748182] CPU: 1 PID: 72 Comm: kworker/1:2 Tainted: G W 4.19.0-rc3 #1 The 'W' taint means that there was a kernel warning before. Which warning was that? I have "spidev" listed in my DT, and the kernel complains about that; that's a harmelss warning. (I sent a patch which adds my particular device into the spidev driver some time ago.) With kind regards, Jan
[BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
Hi,
since commit ee1604381a371b3ea6aec7d5e43b6e3f5e153854 ("PCI: mvebu: Only
remap I/O space if configured"), my board (Solidrun Clearfog Base) won't
finish booting with 4.18-rc3 won't boot:
[1.741458] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
[1.748182] CPU: 1 PID: 72 Comm: kworker/1:2 Tainted: GW
4.19.0-rc3 #1
[1.756120] Hardware name: Marvell Armada 380/385 (Device Tree)
[1.762063] Workqueue: events deferred_probe_work_func
[1.767222] PC is at ioremap_page_range+0x114/0x1a4
[1.772117] LR is at mvebu_pcie_probe+0x134/0xa60
[1.776832] pc : []lr : []psr: a013
[1.783115] sp : ee30dd88 ip : 000f fp : c0837b38
[1.788352] r10: 0243 r9 : e920 r8 : 0103
[1.793590] r7 : ef6f8000 r6 : fee1 r5 : fee0 r4 : ef6f8004
[1.800133] r3 : ef6f8000 r2 : e8000243 r1 : fee0 r0 : c0004000
[1.806678] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[1.813832] Control: 10c5387d Table: 404a DAC: 0051
[1.819592] Process kworker/1:2 (pid: 72, stack limit = 0x(ptrval))
[1.825874] Stack: (0xee30dd88 to 0xee30e000)
[1.830244] dd80: fee1 e920 fee0 ffe0
c0007fb8 ee33e224
[1.838445] dda0: ee33e010 ef575610 ee33e224
c0a30ac4 0002
[1.846646] ddc0: ef9f827c c03e9d5c c0ab6d30 ef9ed434 ee342898 ee342898
ee33e1d0 ee33e1dc
[1.854848] dde0: ee342898 c0a03bc8 c07fcabc 0001 ef575610
ef577000 c02a72b4
[1.863050] de00: ee342898 ef6744d0 ef6744d0 ee342898 ef577000
c07fcabc 5d6fd1e4
[1.871252] de20: c0a30ac4 ef575610 c0a30ac4
c0a30ac4 0002
[1.879454] de40: ef9e40c0 c043dba4 c0ab3144 ef575610 c0ab3150 c043c4e4
ef575610 c0a30ac4
[1.887656] de60: c043cb2c c0a03bc8 0001 c0a5cf50 c043c958
ef575610 c0a30ac4
[1.895858] de80: ef575610 ee30ded4 c043cb2c c0a03bc8 0001
c0a5cf50
[1.904059] dea0: ef9e40c0 c043af7c c0a5cf50 ef484d6c ef66d8b8 5d6fd1e4
ef575610 ef575610
[1.912261] dec0: c0a03bc8 ef575644 c0a371c4 c043c7c0 0003 ef575610
0001 5d6fd1e4
[1.920462] dee0: ef575610 ef575610 c0a373f0 c0a371c4 c043b160
ef575610 c0a371a8
[1.928663] df00: c0a371a8 c043bdf8 c0a371e0 ee2f9880 ef9e40c0 ef9e5300
c01388b4
[1.936864] df20: ef9e40c0 ef9e40c0 ee30c000 ee2f9880 ef9e40c0 ee2f9894
c0a02d00 ef9e40d8
[1.945064] df40: e000 0008 ef9e40c0 c0138e4c e000 c0a5ca32
c07d2f68
[1.953265] df60: e000 eee5e0c0 ee2fa4c0 ee30c000 ee2f9880
c0138ba4 ef4d5ea4
[1.961466] df80: eee5e0dc c013e448 0001 ee2fa4c0 c013e2fc
[1.969668] dfa0: c01010e8
[1.977869] dfc0:
[1.986069] dfe0: 0013
[1.994277] [] (ioremap_page_range) from []
(mvebu_pcie_probe+0x134/0xa60)
[2.002917] [] (mvebu_pcie_probe) from []
(platform_drv_probe+0x48/0x98)
[2.011382] [] (platform_drv_probe) from []
(really_probe+0x1d0/0x2bc)
[2.019671] [] (really_probe) from []
(driver_probe_device+0x60/0x160)
[2.027961] [] (driver_probe_device) from []
(bus_for_each_drv+0x58/0xb8)
[2.036511] [] (bus_for_each_drv) from []
(__device_attach+0xd0/0x138)
[2.044800] [] (__device_attach) from []
(bus_probe_device+0x84/0x8c)
[2.053002] [] (bus_probe_device) from []
(deferred_probe_work_func+0x60/0x8c)
[2.061991] [] (deferred_probe_work_func) from []
(process_one_work+0x218/0x508)
[2.071152] [] (process_one_work) from []
(worker_thread+0x2a8/0x5c0)
[2.079355] [] (worker_thread) from []
(kthread+0x14c/0x154)
[2.086774] [] (kthread) from []
(ret_from_fork+0x14/0x2c)
[2.094015] Exception stack(0xee30dfb0 to 0xee30dff8)
[2.099079] dfa0:
[2.107280] dfc0:
[2.115481] dfe0: 0013
[2.122115] Code: e2844004 e5972000 e352 0aee (e7f001f2)
[2.128225] ---[ end trace 7a77412d00a47123 ]---
[2.132853] Kernel panic - not syncing: Fatal exception
[2.138094] CPU0: stopping
[2.140811] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G D W
4.19.0-rc3 #1
[2.148489] Hardware name: Marvell Armada 380/385 (Device Tree)
[2.154430] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[2.162198] [] (show_stack) from []
(dump_stack+0x88/0x9c)
[2.169443] [] (dump_stack) from []
(handle_IPI+0x38c/0x3ac)
[2.176863] [] (handle_IPI) from []
(gic_handle_irq+0x8c/0x90)
[2.184457] [] (gic_handle_irq) from []
(__irq_svc+0x6c/0x90)
[2.191960]
[BISECTED] Regression: Solidrun Clearfog Base won't boot since "PCI: mvebu: Only remap I/O space if configured"
Hi,
since commit ee1604381a371b3ea6aec7d5e43b6e3f5e153854 ("PCI: mvebu: Only
remap I/O space if configured"), my board (Solidrun Clearfog Base) won't
finish booting with 4.18-rc3 won't boot:
[1.741458] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
[1.748182] CPU: 1 PID: 72 Comm: kworker/1:2 Tainted: GW
4.19.0-rc3 #1
[1.756120] Hardware name: Marvell Armada 380/385 (Device Tree)
[1.762063] Workqueue: events deferred_probe_work_func
[1.767222] PC is at ioremap_page_range+0x114/0x1a4
[1.772117] LR is at mvebu_pcie_probe+0x134/0xa60
[1.776832] pc : []lr : []psr: a013
[1.783115] sp : ee30dd88 ip : 000f fp : c0837b38
[1.788352] r10: 0243 r9 : e920 r8 : 0103
[1.793590] r7 : ef6f8000 r6 : fee1 r5 : fee0 r4 : ef6f8004
[1.800133] r3 : ef6f8000 r2 : e8000243 r1 : fee0 r0 : c0004000
[1.806678] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[1.813832] Control: 10c5387d Table: 404a DAC: 0051
[1.819592] Process kworker/1:2 (pid: 72, stack limit = 0x(ptrval))
[1.825874] Stack: (0xee30dd88 to 0xee30e000)
[1.830244] dd80: fee1 e920 fee0 ffe0
c0007fb8 ee33e224
[1.838445] dda0: ee33e010 ef575610 ee33e224
c0a30ac4 0002
[1.846646] ddc0: ef9f827c c03e9d5c c0ab6d30 ef9ed434 ee342898 ee342898
ee33e1d0 ee33e1dc
[1.854848] dde0: ee342898 c0a03bc8 c07fcabc 0001 ef575610
ef577000 c02a72b4
[1.863050] de00: ee342898 ef6744d0 ef6744d0 ee342898 ef577000
c07fcabc 5d6fd1e4
[1.871252] de20: c0a30ac4 ef575610 c0a30ac4
c0a30ac4 0002
[1.879454] de40: ef9e40c0 c043dba4 c0ab3144 ef575610 c0ab3150 c043c4e4
ef575610 c0a30ac4
[1.887656] de60: c043cb2c c0a03bc8 0001 c0a5cf50 c043c958
ef575610 c0a30ac4
[1.895858] de80: ef575610 ee30ded4 c043cb2c c0a03bc8 0001
c0a5cf50
[1.904059] dea0: ef9e40c0 c043af7c c0a5cf50 ef484d6c ef66d8b8 5d6fd1e4
ef575610 ef575610
[1.912261] dec0: c0a03bc8 ef575644 c0a371c4 c043c7c0 0003 ef575610
0001 5d6fd1e4
[1.920462] dee0: ef575610 ef575610 c0a373f0 c0a371c4 c043b160
ef575610 c0a371a8
[1.928663] df00: c0a371a8 c043bdf8 c0a371e0 ee2f9880 ef9e40c0 ef9e5300
c01388b4
[1.936864] df20: ef9e40c0 ef9e40c0 ee30c000 ee2f9880 ef9e40c0 ee2f9894
c0a02d00 ef9e40d8
[1.945064] df40: e000 0008 ef9e40c0 c0138e4c e000 c0a5ca32
c07d2f68
[1.953265] df60: e000 eee5e0c0 ee2fa4c0 ee30c000 ee2f9880
c0138ba4 ef4d5ea4
[1.961466] df80: eee5e0dc c013e448 0001 ee2fa4c0 c013e2fc
[1.969668] dfa0: c01010e8
[1.977869] dfc0:
[1.986069] dfe0: 0013
[1.994277] [] (ioremap_page_range) from []
(mvebu_pcie_probe+0x134/0xa60)
[2.002917] [] (mvebu_pcie_probe) from []
(platform_drv_probe+0x48/0x98)
[2.011382] [] (platform_drv_probe) from []
(really_probe+0x1d0/0x2bc)
[2.019671] [] (really_probe) from []
(driver_probe_device+0x60/0x160)
[2.027961] [] (driver_probe_device) from []
(bus_for_each_drv+0x58/0xb8)
[2.036511] [] (bus_for_each_drv) from []
(__device_attach+0xd0/0x138)
[2.044800] [] (__device_attach) from []
(bus_probe_device+0x84/0x8c)
[2.053002] [] (bus_probe_device) from []
(deferred_probe_work_func+0x60/0x8c)
[2.061991] [] (deferred_probe_work_func) from []
(process_one_work+0x218/0x508)
[2.071152] [] (process_one_work) from []
(worker_thread+0x2a8/0x5c0)
[2.079355] [] (worker_thread) from []
(kthread+0x14c/0x154)
[2.086774] [] (kthread) from []
(ret_from_fork+0x14/0x2c)
[2.094015] Exception stack(0xee30dfb0 to 0xee30dff8)
[2.099079] dfa0:
[2.107280] dfc0:
[2.115481] dfe0: 0013
[2.122115] Code: e2844004 e5972000 e352 0aee (e7f001f2)
[2.128225] ---[ end trace 7a77412d00a47123 ]---
[2.132853] Kernel panic - not syncing: Fatal exception
[2.138094] CPU0: stopping
[2.140811] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G D W
4.19.0-rc3 #1
[2.148489] Hardware name: Marvell Armada 380/385 (Device Tree)
[2.154430] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[2.162198] [] (show_stack) from []
(dump_stack+0x88/0x9c)
[2.169443] [] (dump_stack) from []
(handle_IPI+0x38c/0x3ac)
[2.176863] [] (handle_IPI) from []
(gic_handle_irq+0x8c/0x90)
[2.184457] [] (gic_handle_irq) from []
(__irq_svc+0x6c/0x90)
[2.191960]
Re: [PATCH v2 1/3] spi: Introduce one new field to set word delay
On čtvrtek 16. srpna 2018 14:54:49 CEST, Baolin Wang wrote: + * @word_delay: clock cycles to inter word delay after each word size + * (set by bits_per_word) transmission. I need a similar functionality for talking to a SPI device from userspace -- see my attempt for implementing this in spi-orion.c at [1]. The device's datasheet says that I should wait, e.g., 3µs between each two words. I therefore like this patch :). The description can be improved because it left me wondering what "clock this is about. I suppose it's about the SPI clock cycles and not CPU clock cycles, right? I'll be hapy to patch this once Baolin confirms that that is the intended meaning. It seems that this is only implemented in one newly added driver. I'm interested in supporting this in spi-orion.c, but that sounds like driver-specific work for something which is pretty generic. How should this be implemented? Given that drivers for SPI masters can implement a function which transfers several words at once, there are not that many better possibilities than adding udelay()s, though. Thoughts? What is your plan to do with drivers which do not implement this (yet)? If a spi_transfer gets queued which asks for a word_delay delay, it is silently ignored now, AFAIU. What about userspace support, spidev and spi_ioc_transfer (that's my target, actually)? Is it OK to s/pad/word_delay/ in the spidev code and pass that to the generated struct spi_transfer? In my opinion, once we support specifying this from userspace, one has to definitely check that the SPI controller is ready to honor this request. Do we want a new bit in spi_controller.flags for this? With kind regards, Jan [1] https://patchwork.kernel.org/patch/10221397/
Re: [PATCH v2 1/3] spi: Introduce one new field to set word delay
On čtvrtek 16. srpna 2018 14:54:49 CEST, Baolin Wang wrote: + * @word_delay: clock cycles to inter word delay after each word size + * (set by bits_per_word) transmission. I need a similar functionality for talking to a SPI device from userspace -- see my attempt for implementing this in spi-orion.c at [1]. The device's datasheet says that I should wait, e.g., 3µs between each two words. I therefore like this patch :). The description can be improved because it left me wondering what "clock this is about. I suppose it's about the SPI clock cycles and not CPU clock cycles, right? I'll be hapy to patch this once Baolin confirms that that is the intended meaning. It seems that this is only implemented in one newly added driver. I'm interested in supporting this in spi-orion.c, but that sounds like driver-specific work for something which is pretty generic. How should this be implemented? Given that drivers for SPI masters can implement a function which transfers several words at once, there are not that many better possibilities than adding udelay()s, though. Thoughts? What is your plan to do with drivers which do not implement this (yet)? If a spi_transfer gets queued which asks for a word_delay delay, it is silently ignored now, AFAIU. What about userspace support, spidev and spi_ioc_transfer (that's my target, actually)? Is it OK to s/pad/word_delay/ in the spidev code and pass that to the generated struct spi_transfer? In my opinion, once we support specifying this from userspace, one has to definitely check that the SPI controller is ready to honor this request. Do we want a new bit in spi_controller.flags for this? With kind regards, Jan [1] https://patchwork.kernel.org/patch/10221397/
Re: [PATCH v4] spi: orion: cosmetics - alias long direct_access variables
On středa 15. srpna 2018 21:04:49 CEST, Kosta Zertsekel wrote:
This change increases the source code readability.
Instead of using `spi->child[cs].direct_access.XXX` use `dir_acc->XXX`.
Instead of using `orion_spi->child[cs].direct_access.vaddr` use `vaddr`.
Signed-off-by: Kosta Zertsekel
Reviewed-by: Andrew Lunn
Reviewed-by: Stefan Roese
---
drivers/spi/spi-orion.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index d01a6adc726e..184ba91abeee 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -430,6 +430,7 @@ orion_spi_write_read(struct spi_device
*spi, struct spi_transfer *xfer)
int word_len;
struct orion_spi *orion_spi;
int cs = spi->chip_select;
+ void __iomem *vaddr;
word_len = spi->bits_per_word;
count = xfer->len;
@@ -440,8 +441,9 @@ orion_spi_write_read(struct spi_device
*spi, struct spi_transfer *xfer)
* Use SPI direct write mode if base address is available. Otherwise
* fall back to PIO mode for this transfer.
*/
- if ((orion_spi->child[cs].direct_access.vaddr) && (xfer->tx_buf) &&
- (word_len == 8)) {
+ vaddr = orion_spi->child[cs].direct_access.vaddr;
+
+ if (vaddr && xfer->tx_buf && word_len == 8) {
unsigned int cnt = count / 4;
unsigned int rem = count % 4;
@@ -449,13 +451,11 @@ orion_spi_write_read(struct spi_device
*spi, struct spi_transfer *xfer)
* Send the TX-data to the SPI device via the direct
* mapped address window
*/
- iowrite32_rep(orion_spi->child[cs].direct_access.vaddr,
- xfer->tx_buf, cnt);
+ iowrite32_rep(vaddr, xfer->tx_buf, cnt);
if (rem) {
u32 *buf = (u32 *)xfer->tx_buf;
- iowrite8_rep(orion_spi->child[cs].direct_access.vaddr,
-[cnt], rem);
+ iowrite8_rep(vaddr, [cnt], rem);
}
return count;
@@ -683,6 +683,7 @@ static int orion_spi_probe(struct platform_device *pdev)
/* Scan all SPI devices of this controller for direct mapped devices */
for_each_available_child_of_node(pdev->dev.of_node, np) {
+ struct orion_direct_acc *dir_acc;
u32 cs;
/* Get chip-select number from the "reg" property */
@@ -711,14 +712,13 @@ static int orion_spi_probe(struct
platform_device *pdev)
* This needs to get extended for the direct SPI-NOR / SPI-NAND
* support, once this gets implemented.
*/
- spi->child[cs].direct_access.vaddr = devm_ioremap(>dev,
- r->start,
- PAGE_SIZE);
- if (!spi->child[cs].direct_access.vaddr) {
+ dir_acc = >child[cs].direct_access;
+ dir_acc->vaddr = devm_ioremap(>dev, r->start, PAGE_SIZE);
+ if (!dir_acc->vaddr) {
status = -ENOMEM;
goto out_rel_axi_clk;
}
- spi->child[cs].direct_access.size = PAGE_SIZE;
+ dir_acc->size = PAGE_SIZE;
dev_info(>dev, "CS%d configured for direct access\n", cs);
}
Reviewed-by: Jan Kundrát
Re: [PATCH v4] spi: orion: cosmetics - alias long direct_access variables
On středa 15. srpna 2018 21:04:49 CEST, Kosta Zertsekel wrote:
This change increases the source code readability.
Instead of using `spi->child[cs].direct_access.XXX` use `dir_acc->XXX`.
Instead of using `orion_spi->child[cs].direct_access.vaddr` use `vaddr`.
Signed-off-by: Kosta Zertsekel
Reviewed-by: Andrew Lunn
Reviewed-by: Stefan Roese
---
drivers/spi/spi-orion.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index d01a6adc726e..184ba91abeee 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -430,6 +430,7 @@ orion_spi_write_read(struct spi_device
*spi, struct spi_transfer *xfer)
int word_len;
struct orion_spi *orion_spi;
int cs = spi->chip_select;
+ void __iomem *vaddr;
word_len = spi->bits_per_word;
count = xfer->len;
@@ -440,8 +441,9 @@ orion_spi_write_read(struct spi_device
*spi, struct spi_transfer *xfer)
* Use SPI direct write mode if base address is available. Otherwise
* fall back to PIO mode for this transfer.
*/
- if ((orion_spi->child[cs].direct_access.vaddr) && (xfer->tx_buf) &&
- (word_len == 8)) {
+ vaddr = orion_spi->child[cs].direct_access.vaddr;
+
+ if (vaddr && xfer->tx_buf && word_len == 8) {
unsigned int cnt = count / 4;
unsigned int rem = count % 4;
@@ -449,13 +451,11 @@ orion_spi_write_read(struct spi_device
*spi, struct spi_transfer *xfer)
* Send the TX-data to the SPI device via the direct
* mapped address window
*/
- iowrite32_rep(orion_spi->child[cs].direct_access.vaddr,
- xfer->tx_buf, cnt);
+ iowrite32_rep(vaddr, xfer->tx_buf, cnt);
if (rem) {
u32 *buf = (u32 *)xfer->tx_buf;
- iowrite8_rep(orion_spi->child[cs].direct_access.vaddr,
-[cnt], rem);
+ iowrite8_rep(vaddr, [cnt], rem);
}
return count;
@@ -683,6 +683,7 @@ static int orion_spi_probe(struct platform_device *pdev)
/* Scan all SPI devices of this controller for direct mapped devices */
for_each_available_child_of_node(pdev->dev.of_node, np) {
+ struct orion_direct_acc *dir_acc;
u32 cs;
/* Get chip-select number from the "reg" property */
@@ -711,14 +712,13 @@ static int orion_spi_probe(struct
platform_device *pdev)
* This needs to get extended for the direct SPI-NOR / SPI-NAND
* support, once this gets implemented.
*/
- spi->child[cs].direct_access.vaddr = devm_ioremap(>dev,
- r->start,
- PAGE_SIZE);
- if (!spi->child[cs].direct_access.vaddr) {
+ dir_acc = >child[cs].direct_access;
+ dir_acc->vaddr = devm_ioremap(>dev, r->start, PAGE_SIZE);
+ if (!dir_acc->vaddr) {
status = -ENOMEM;
goto out_rel_axi_clk;
}
- spi->child[cs].direct_access.size = PAGE_SIZE;
+ dir_acc->size = PAGE_SIZE;
dev_info(>dev, "CS%d configured for direct access\n", cs);
}
Reviewed-by: Jan Kundrát
Re: [PATCH] gpio: Add missing open drain/source handling to gpiod_set_value_cansleep()
On úterý 9. ledna 2018 19:08:21 CET, Geert Uytterhoeven wrote:
Since commit f11a04464ae57e8d ("i2c: gpio: Enable working over slow
can_sleep GPIOs"), probing the i2c RTC connected to an i2c-gpio bus on
r8a7740/armadillo fails with:
rtc-s35390a 0-0030: error resetting chip
rtc-s35390a: probe of 0-0030 failed with error -5
More debug code reveals:
i2c i2c-0: master_xfer[0] R, addr=0x30, len=1
i2c i2c-0: NAK from device addr 0x30 msg #0
s35390a_get_reg: ret = -6
Commit 02e479808b5d62f8 ("gpio: Alter semantics of *raw* operations to
actually be raw") moved open drain/source handling from
gpiod_set_raw_value_commit() to gpiod_set_value(), but forgot to take
into account that gpiod_set_value_cansleep() also needs this handling.
The i2c protocol mandates that i2c signals are open drain, hence i2c
communication fails.
Fix this by adding the missing handling to gpiod_set_value_cansleep(),
using a new common helper __gpiod_set_value_nocheck().
Fixes: 02e479808b5d62f8 ("gpio: Alter semantics of *raw*
operations to actually be raw")
Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
---
The bad gpio commit is in v4.15-rc1 and later.
The i2c commit exposing this is not yet upstream, but in i2c/for-next.
---
drivers/gpio/gpiolib.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 358bce6e798b476c..7761d632a5b0dd20 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2927,6 +2927,18 @@ void gpiod_set_raw_value(struct
gpio_desc *desc, int value)
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_value);
+static void __gpiod_set_value_nocheck(struct gpio_desc *desc, int value)
+{
+ if (test_bit(FLAG_ACTIVE_LOW, >flags))
+ value = !value;
+ if (test_bit(FLAG_OPEN_DRAIN, >flags))
+ gpio_set_open_drain_value_commit(desc, value);
+ else if (test_bit(FLAG_OPEN_SOURCE, >flags))
+ gpio_set_open_source_value_commit(desc, value);
+ else
+ gpiod_set_raw_value_commit(desc, value);
+}
+
/**
* gpiod_set_value() - assign a gpio's value
* @desc: gpio whose value will be assigned
@@ -2941,16 +2953,8 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_value);
void gpiod_set_value(struct gpio_desc *desc, int value)
{
VALIDATE_DESC_VOID(desc);
- /* Should be using gpiod_set_value_cansleep() */
WARN_ON(desc->gdev->chip->can_sleep);
- if (test_bit(FLAG_ACTIVE_LOW, >flags))
- value = !value;
- if (test_bit(FLAG_OPEN_DRAIN, >flags))
- gpio_set_open_drain_value_commit(desc, value);
- else if (test_bit(FLAG_OPEN_SOURCE, >flags))
- gpio_set_open_source_value_commit(desc, value);
- else
- gpiod_set_raw_value_commit(desc, value);
+ __gpiod_set_value_nocheck(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_value);
@@ -3277,9 +3281,7 @@ void gpiod_set_value_cansleep(struct
gpio_desc *desc, int value)
{
might_sleep_if(extra_checks);
VALIDATE_DESC_VOID(desc);
- if (test_bit(FLAG_ACTIVE_LOW, >flags))
- value = !value;
- gpiod_set_raw_value_commit(desc, value);
+ __gpiod_set_value_nocheck(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
I realize that I'm late to the party (a merge request has been sent), but I
can confirm that I tested that this still works for a bitbang I2C over GPIO
pins connected to MAX14830 managed by max310x.
Tested-By: Jan Kundrát <jan.kund...@cesnet.cz>
Cheers,
Jan
Re: [PATCH] gpio: Add missing open drain/source handling to gpiod_set_value_cansleep()
On úterý 9. ledna 2018 19:08:21 CET, Geert Uytterhoeven wrote:
Since commit f11a04464ae57e8d ("i2c: gpio: Enable working over slow
can_sleep GPIOs"), probing the i2c RTC connected to an i2c-gpio bus on
r8a7740/armadillo fails with:
rtc-s35390a 0-0030: error resetting chip
rtc-s35390a: probe of 0-0030 failed with error -5
More debug code reveals:
i2c i2c-0: master_xfer[0] R, addr=0x30, len=1
i2c i2c-0: NAK from device addr 0x30 msg #0
s35390a_get_reg: ret = -6
Commit 02e479808b5d62f8 ("gpio: Alter semantics of *raw* operations to
actually be raw") moved open drain/source handling from
gpiod_set_raw_value_commit() to gpiod_set_value(), but forgot to take
into account that gpiod_set_value_cansleep() also needs this handling.
The i2c protocol mandates that i2c signals are open drain, hence i2c
communication fails.
Fix this by adding the missing handling to gpiod_set_value_cansleep(),
using a new common helper __gpiod_set_value_nocheck().
Fixes: 02e479808b5d62f8 ("gpio: Alter semantics of *raw*
operations to actually be raw")
Signed-off-by: Geert Uytterhoeven
---
The bad gpio commit is in v4.15-rc1 and later.
The i2c commit exposing this is not yet upstream, but in i2c/for-next.
---
drivers/gpio/gpiolib.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 358bce6e798b476c..7761d632a5b0dd20 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2927,6 +2927,18 @@ void gpiod_set_raw_value(struct
gpio_desc *desc, int value)
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_value);
+static void __gpiod_set_value_nocheck(struct gpio_desc *desc, int value)
+{
+ if (test_bit(FLAG_ACTIVE_LOW, >flags))
+ value = !value;
+ if (test_bit(FLAG_OPEN_DRAIN, >flags))
+ gpio_set_open_drain_value_commit(desc, value);
+ else if (test_bit(FLAG_OPEN_SOURCE, >flags))
+ gpio_set_open_source_value_commit(desc, value);
+ else
+ gpiod_set_raw_value_commit(desc, value);
+}
+
/**
* gpiod_set_value() - assign a gpio's value
* @desc: gpio whose value will be assigned
@@ -2941,16 +2953,8 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_value);
void gpiod_set_value(struct gpio_desc *desc, int value)
{
VALIDATE_DESC_VOID(desc);
- /* Should be using gpiod_set_value_cansleep() */
WARN_ON(desc->gdev->chip->can_sleep);
- if (test_bit(FLAG_ACTIVE_LOW, >flags))
- value = !value;
- if (test_bit(FLAG_OPEN_DRAIN, >flags))
- gpio_set_open_drain_value_commit(desc, value);
- else if (test_bit(FLAG_OPEN_SOURCE, >flags))
- gpio_set_open_source_value_commit(desc, value);
- else
- gpiod_set_raw_value_commit(desc, value);
+ __gpiod_set_value_nocheck(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_value);
@@ -3277,9 +3281,7 @@ void gpiod_set_value_cansleep(struct
gpio_desc *desc, int value)
{
might_sleep_if(extra_checks);
VALIDATE_DESC_VOID(desc);
- if (test_bit(FLAG_ACTIVE_LOW, >flags))
- value = !value;
- gpiod_set_raw_value_commit(desc, value);
+ __gpiod_set_value_nocheck(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
I realize that I'm late to the party (a merge request has been sent), but I
can confirm that I tested that this still works for a bitbang I2C over GPIO
pins connected to MAX14830 managed by max310x.
Tested-By: Jan Kundrát
Cheers,
Jan
Re: [PATCH] serial: max310x: Delete an error message for a failed memory allocation in max310x_probe()
On pátek 8. prosince 2017 20:00:34 CET, SF Markus Elfring wrote:
From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Fri, 8 Dec 2017 19:53:10 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
drivers/tty/serial/max310x.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index ecb6513a6505..2aad36f402fd 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1102,10 +1102,8 @@ static int max310x_probe(struct device
*dev, struct max310x_devtype *devtype,
/* Alloc port structure */
s = devm_kzalloc(dev, sizeof(*s) +
sizeof(struct max310x_one) * devtype->nr, GFP_KERNEL);
- if (!s) {
- dev_err(dev, "Error allocating port structure\n");
+ if (!s)
return -ENOMEM;
- }
clk_osc = devm_clk_get(dev, "osc");
clk_xtal = devm_clk_get(dev, "xtal");
I'm looking at this patch because I'm also making some modifications to
this driver. This patch makes sense; other drivers also simply return
ENOMEM. Tested on ARM, Armada 388; the "good" code path still obviously
works :).
Reviewed-by: Jan Kundrát <jan.kund...@cesnet.cz>
Re: [PATCH] serial: max310x: Delete an error message for a failed memory allocation in max310x_probe()
On pátek 8. prosince 2017 20:00:34 CET, SF Markus Elfring wrote:
From: Markus Elfring
Date: Fri, 8 Dec 2017 19:53:10 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/tty/serial/max310x.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index ecb6513a6505..2aad36f402fd 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1102,10 +1102,8 @@ static int max310x_probe(struct device
*dev, struct max310x_devtype *devtype,
/* Alloc port structure */
s = devm_kzalloc(dev, sizeof(*s) +
sizeof(struct max310x_one) * devtype->nr, GFP_KERNEL);
- if (!s) {
- dev_err(dev, "Error allocating port structure\n");
+ if (!s)
return -ENOMEM;
- }
clk_osc = devm_clk_get(dev, "osc");
clk_xtal = devm_clk_get(dev, "xtal");
I'm looking at this patch because I'm also making some modifications to
this driver. This patch makes sense; other drivers also simply return
ENOMEM. Tested on ARM, Armada 388; the "good" code path still obviously
works :).
Reviewed-by: Jan Kundrát
Re: kswapd high CPU usage with no swap
>> When this is sorted out, should I keep the previous patch [1] applied >> as well? > > That doesn't hurt. OK, I've used just the latter patch (because I somehow believe the first one lowers the probability of bad behavior), so let's see if kswapd consumes CPU again. I don't have any test patter to trigger that CPU usage, though... Cheers and thanks for your time, -jkt -- cd /local/pub && more beer > /dev/mouth signature.asc Description: OpenPGP digital signature
Re: kswapd high CPU usage with no swap
Rik van Riel wrote: > Could you try out the attached patch, too? Sorry, I wasn't able to apply it against 2.6.22-gentoo-r3 and vanilla 2.6.22.7; I don't have the "order" member in the "struct scan_control" and also the bit about "if (sync_writeback == PAGEOUT_IO_SYNC && may_enter_fs)" was missing so there was no "else" branch to add (and it seemd pretty significant to me). What version should I patch against? When this is sorted out, should I keep the previous patch [1] applied as well? [1] http://www.redhat.com/archives/nahant-list/2006-March/msg00033.html Cheers, -jkt -- cd /local/pub && more beer > /dev/mouth signature.asc Description: OpenPGP digital signature
Re: kswapd high CPU usage with no swap
Rik van Riel wrote: Could you try out the attached patch, too? Sorry, I wasn't able to apply it against 2.6.22-gentoo-r3 and vanilla 2.6.22.7; I don't have the order member in the struct scan_control and also the bit about if (sync_writeback == PAGEOUT_IO_SYNC may_enter_fs) was missing so there was no else branch to add (and it seemd pretty significant to me). What version should I patch against? When this is sorted out, should I keep the previous patch [1] applied as well? [1] http://www.redhat.com/archives/nahant-list/2006-March/msg00033.html Cheers, -jkt -- cd /local/pub more beer /dev/mouth signature.asc Description: OpenPGP digital signature
Re: kswapd high CPU usage with no swap
When this is sorted out, should I keep the previous patch [1] applied as well? That doesn't hurt. OK, I've used just the latter patch (because I somehow believe the first one lowers the probability of bad behavior), so let's see if kswapd consumes CPU again. I don't have any test patter to trigger that CPU usage, though... Cheers and thanks for your time, -jkt -- cd /local/pub more beer /dev/mouth signature.asc Description: OpenPGP digital signature
Re: kswapd high CPU usage with no swap
Rik van Riel wrote:
> How much memory did you have in "cached" when you looked
> with top (and no swap enabled) ?
Hi Rik,
it was pretty low number (several thousands, or maybe tens of thousands).
In the meanwhile, I've come across your patch [1] ("prevent kswapd from
freeing excessive amounts of lowmem") and applied it locally. I'll see
if it fixes my problem, but at a first glance, it seems that it might
actually slow other things down -- when I switch windows, konqueror's
screen redrawing seems to be pretty slow and I can see it progressing
from top to bottom...
[1] http://lkml.org/lkml/2007/9/5/289
Cheers,
-jkt
--
cd /local/pub && more beer > /dev/mouth
signature.asc
Description: OpenPGP digital signature
Re: kswapd high CPU usage with no swap
Rik van Riel wrote: How much memory did you have in cached when you looked with top (and no swap enabled) ? Hi Rik, it was pretty low number (several thousands, or maybe tens of thousands). In the meanwhile, I've come across your patch [1] (prevent kswapd from freeing excessive amounts of lowmem) and applied it locally. I'll see if it fixes my problem, but at a first glance, it seems that it might actually slow other things down -- when I switch windows, konqueror's screen redrawing seems to be pretty slow and I can see it progressing from top to bottom... [1] http://lkml.org/lkml/2007/9/5/289 Cheers, -jkt -- cd /local/pub more beer /dev/mouth signature.asc Description: OpenPGP digital signature
kswapd high CPU usage with no swap
Hi folks, I use a 2.6.22-gentoo-r2 SMP kernel with fglrx 8.40.4 [1], tp_smapi-0.32 and ipw3945-1.2.0 on a Thinkpad T60 with dual core Intel Core CPU. My root filesystem is XFS stored on an internal SATA disk, and I have 1GB of RAM and no swap. After several suspend to RAM/resume cycles, the X interface got pretty slow today. Looking at the top output, I see that one core was completely busy in the "wa" state and according to `ps auxww`, the kswapd0 process was in uninterruptable deep sleep. I don't use any swap because my applications rarely need more memory than I have in my system and using 1GB of swap yielded poor performance when it was actually accessed. I know I can tweak VM's preferences, but I simply don't feel the need to use swap when I have "plenty" of memory. That said, I've googled a thread [2] which recommends using at least a small swap because VM somehow performs better with it. So I created a small (12MB) swap on a physical partition on the same SATA disk and swapon-ed it. The swap got full immediately but the load didn't get lower. After that, I've turned it off again, adjusted its size to be about 1GB and enabled it again. Several tens of seconds later, swap usage was at approximately 350MB and RAM usage at about 60MB. There was no unusual activity at the background (no intensive cronned jobs, no locate, nothing, just an idle KDE session with bunch of idling applications). Touching the swappiness value (0, 10, 40, 60 and 100) didn't change anything on the fact that switching from one application to another was slow, and even moving a mouse cursor (USB-attached mouse) in X was delayed and not smooth. Could you please provide a hint about what might cause such problems, what am I doing wrong and how can I avoid it? If there are any details like kernel config that are relevant here, I'll happily provide them. I'd also appreciate if you keep me on the CC as I'm not subscribed to this list. If this is not an appropriate place to ask, please accept my apologies. [1] I'm aware of the fact that fglrx is a binary closed source and that the kernel is tainted by its usage. Unfortunately no other driver besides VESA supports my VGA card. [2] http://www.redhat.com/archives/nahant-list/2006-March/msg00033.html Cheers, -jkt -- cd /local/pub && more beer > /dev/mouth signature.asc Description: OpenPGP digital signature
kswapd high CPU usage with no swap
Hi folks, I use a 2.6.22-gentoo-r2 SMP kernel with fglrx 8.40.4 [1], tp_smapi-0.32 and ipw3945-1.2.0 on a Thinkpad T60 with dual core Intel Core CPU. My root filesystem is XFS stored on an internal SATA disk, and I have 1GB of RAM and no swap. After several suspend to RAM/resume cycles, the X interface got pretty slow today. Looking at the top output, I see that one core was completely busy in the wa state and according to `ps auxww`, the kswapd0 process was in uninterruptable deep sleep. I don't use any swap because my applications rarely need more memory than I have in my system and using 1GB of swap yielded poor performance when it was actually accessed. I know I can tweak VM's preferences, but I simply don't feel the need to use swap when I have plenty of memory. That said, I've googled a thread [2] which recommends using at least a small swap because VM somehow performs better with it. So I created a small (12MB) swap on a physical partition on the same SATA disk and swapon-ed it. The swap got full immediately but the load didn't get lower. After that, I've turned it off again, adjusted its size to be about 1GB and enabled it again. Several tens of seconds later, swap usage was at approximately 350MB and RAM usage at about 60MB. There was no unusual activity at the background (no intensive cronned jobs, no locate, nothing, just an idle KDE session with bunch of idling applications). Touching the swappiness value (0, 10, 40, 60 and 100) didn't change anything on the fact that switching from one application to another was slow, and even moving a mouse cursor (USB-attached mouse) in X was delayed and not smooth. Could you please provide a hint about what might cause such problems, what am I doing wrong and how can I avoid it? If there are any details like kernel config that are relevant here, I'll happily provide them. I'd also appreciate if you keep me on the CC as I'm not subscribed to this list. If this is not an appropriate place to ask, please accept my apologies. [1] I'm aware of the fact that fglrx is a binary closed source and that the kernel is tainted by its usage. Unfortunately no other driver besides VESA supports my VGA card. [2] http://www.redhat.com/archives/nahant-list/2006-March/msg00033.html Cheers, -jkt -- cd /local/pub more beer /dev/mouth signature.asc Description: OpenPGP digital signature
