Skip to content

Commit

Permalink
w1: w1-gpio: Make GPIO an output for strong pullup
Browse files Browse the repository at this point in the history
The logic to drive the data line high to implement a strong pullup
assumed that the pin was already an output - setting a value does
not change an input.

See: raspberrypi/firmware#1143

Signed-off-by: Phil Elwell <phil@raspberrypi.org>

drivers: w1-gpio: add flag to force read-polling while delaying

On Pi 5, the link to RP1 will bounce in and out of L1 depending on
inactivity timers at both the RC and EP end. Unfortunately for
bitbashing 1-wire, this means that on an otherwise idle Pi 5 many of the
reads/writes to GPIO registers are delayed by up to 8us which causes
mis-sampling of read data and trashes write bits.

By issuing dummy reads at a rate greater than the link inactivity
timeout while spinning on a delay, PCIe stays in L0 which does not incur
additional latency.

Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com>

drivers: w1-gpio: Fixup uninitialised variable use in w1_gpio_probe

Signed-off-by: Jonathan Bell <jonathan@raspberrypi.com>
  • Loading branch information
Phil Elwell authored and popcornmix committed Dec 20, 2024
1 parent f5eeabd commit 2a20c1f
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 13 deletions.
5 changes: 4 additions & 1 deletion drivers/w1/masters/w1-gpio.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static u8 w1_gpio_set_pullup(void *data, int delay)
* This will OVERRIDE open drain emulation and force-pull
* the line high for some time.
*/
gpiod_set_raw_value(ddata->gpiod, 1);
gpiod_direction_output_raw(ddata->gpiod, 1);
msleep(ddata->pullup_duration);
/*
* This will simply set the line as input since we are doing
Expand Down Expand Up @@ -89,6 +89,9 @@ static int w1_gpio_probe(struct platform_device *pdev)
if (!master)
return -ENOMEM;

if (device_property_present(dev, "raspberrypi,delay-needs-poll"))
master->delay_needs_poll = true;

ddata->gpiod = devm_gpiod_get_index(dev, NULL, 0, gflags);
if (IS_ERR(ddata->gpiod))
return dev_err_probe(dev, PTR_ERR(ddata->gpiod), "gpio_request (pin) failed\n");
Expand Down
37 changes: 25 additions & 12 deletions drivers/w1/w1_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <asm/io.h>

#include <linux/delay.h>
#include <linux/ktime.h>
#include <linux/moduleparam.h>
#include <linux/module.h>

Expand Down Expand Up @@ -36,9 +37,21 @@ static u8 w1_crc8_table[] = {
116, 42, 200, 150, 21, 75, 169, 247, 182, 232, 10, 84, 215, 137, 107, 53
};

static void w1_delay(unsigned long tm)
static void w1_delay(struct w1_master *dev, unsigned long tm)
{
udelay(tm * w1_delay_parm);
ktime_t start, delta;

if (!dev->bus_master->delay_needs_poll) {
udelay(tm * w1_delay_parm);
return;
}

start = ktime_get();
delta = ktime_add(start, ns_to_ktime(1000 * tm * w1_delay_parm));
do {
dev->bus_master->read_bit(dev->bus_master->data);
udelay(1);
} while (ktime_before(ktime_get(), delta));
}

static void w1_write_bit(struct w1_master *dev, int bit);
Expand Down Expand Up @@ -77,14 +90,14 @@ static void w1_write_bit(struct w1_master *dev, int bit)

if (bit) {
dev->bus_master->write_bit(dev->bus_master->data, 0);
w1_delay(6);
w1_delay(dev, 6);
dev->bus_master->write_bit(dev->bus_master->data, 1);
w1_delay(64);
w1_delay(dev, 64);
} else {
dev->bus_master->write_bit(dev->bus_master->data, 0);
w1_delay(60);
w1_delay(dev, 60);
dev->bus_master->write_bit(dev->bus_master->data, 1);
w1_delay(10);
w1_delay(dev, 10);
}

if(w1_disable_irqs) local_irq_restore(flags);
Expand Down Expand Up @@ -164,14 +177,14 @@ static u8 w1_read_bit(struct w1_master *dev)
/* sample timing is critical here */
local_irq_save(flags);
dev->bus_master->write_bit(dev->bus_master->data, 0);
w1_delay(6);
w1_delay(dev, 6);
dev->bus_master->write_bit(dev->bus_master->data, 1);
w1_delay(9);
w1_delay(dev, 9);

result = dev->bus_master->read_bit(dev->bus_master->data);
local_irq_restore(flags);

w1_delay(55);
w1_delay(dev, 55);

return result & 0x1;
}
Expand Down Expand Up @@ -333,16 +346,16 @@ int w1_reset_bus(struct w1_master *dev)
* cpu for such a short amount of time AND get it back in
* the maximum amount of time.
*/
w1_delay(500);
w1_delay(dev, 500);
dev->bus_master->write_bit(dev->bus_master->data, 1);
w1_delay(70);
w1_delay(dev, 70);

result = dev->bus_master->read_bit(dev->bus_master->data) & 0x1;
/* minimum 70 (above) + 430 = 500 us
* There aren't any timing requirements between a reset and
* the following transactions. Sleeping is safe here.
*/
/* w1_delay(430); min required time */
/* w1_delay(dev, 430); min required time */
msleep(1);
}

Expand Down
5 changes: 5 additions & 0 deletions include/linux/w1.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ typedef void (*w1_slave_found_callback)(struct w1_master *, u64);
* @dev_id: Optional device id string, which w1 slaves could use for
* creating names, which then give a connection to the w1 master
*
* @delay_needs_poll: work around jitter introduced with GPIO controllers
* accessed over PCIe (RP1)
*
* Note: read_bit and write_bit are very low level functions and should only
* be used with hardware that doesn't really support 1-wire operations,
* like a parallel/serial port.
Expand Down Expand Up @@ -156,6 +159,8 @@ struct w1_bus_master {
u8, w1_slave_found_callback);

char *dev_id;

bool delay_needs_poll;
};

/**
Expand Down

0 comments on commit 2a20c1f

Please sign in to comment.