Page MenuHomeFreeBSD
Differential D44362

tcp: remove IS_FASTOPEN() macro
ClosedPublic
Actions

Authored by glebius on Mar 14 2024, 7:51 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 4, 9:29 PM
Unknown Object (File)
Fri, Apr 4, 8:45 PM
Unknown Object (File)
Feb 7 2025, 4:30 PM
Unknown Object (File)
Feb 2 2025, 3:57 AM
Unknown Object (File)
Jan 28 2025, 2:03 AM
Unknown Object (File)
Jan 27 2025, 6:36 AM
Unknown Object (File)
Jan 23 2025, 6:51 PM
Unknown Object (File)
Jan 1 2025, 4:31 AM
View All 76 Files
Subscribers
melifaro
rscheff

Details

Reviewers
kib
imp
jhb
rscheff
tuexen
Group Reviewers
transport
Commits
rGdd7b86e2a010: tcp: remove IS_FASTOPEN() macro
Summary

The macro is more obfuscating than helping as it just checks a single flag
of t_flags. All other t_flags bits are checked without a macro.

A bigger problem was that declaration of the macro in tcp_var.h depended
on a kernel option. It is a bad practice to create such definitions in
installable headers.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 56612
Build 53500: arc lint + arc unit

Event Timeline

glebius created this revision.Mar 14 2024, 7:51 PM
Herald added 1 blocking reviewer(s): transport. · View Herald TranscriptMar 14 2024, 7:51 PM
Herald added a subscriber: melifaro. · View Herald Transcript
glebius requested review of this revision.Mar 14 2024, 7:51 PM
Harbormaster completed remote builds in B56611: Diff 135763.Mar 14 2024, 7:51 PM
rscheff accepted this revision.Mar 14 2024, 8:15 PM
rscheff added a subscriber: rscheff.

only minor whitespace nits. i'm ok with removing this macro mechanistically.

sys/netinet/tcp_output.c
244–245

why remove the newline here? I'd leave the individual check one per line, or concatenate the deeper bracketed OR together to a single line.
(minor nit-pick).

sys/netinet/tcp_stacks/rack.c
21952–21953

same.

22079

same.

This revision is now accepted and ready to land.Mar 14 2024, 8:15 PM
tuexen accepted this revision.Mar 14 2024, 8:39 PM
glebius updated this revision to Diff 135765.Mar 14 2024, 8:41 PM

Add back new lines as Richard suggests.

This revision now requires review to proceed.Mar 14 2024, 8:41 PM
Harbormaster completed remote builds in B56612: Diff 135765.Mar 14 2024, 8:41 PM
rscheff accepted this revision.Mar 14 2024, 8:50 PM

thx

This revision is now accepted and ready to land.Mar 14 2024, 8:50 PM
kib added a comment.Mar 14 2024, 8:50 PM

I do not understand the change. You re-wrote the code as if the TCP_RFC7413 option is always set. This is not true, at least from the fast grepping of the sources.

glebius added a comment.Mar 14 2024, 8:51 PM
In D44362#1011765, @kib wrote:

I do not understand the change. You re-wrote the code as if the TCP_RFC7413 option is always set. This is not true, at least from the fast grepping of the sources.

If kernel is compiled w/o the option, then t_flags & TF_FASTOPEN will always be false.

rscheff added a comment.Mar 14 2024, 8:52 PM
In D44362#1011765, @kib wrote:

I do not understand the change. You re-wrote the code as if the TCP_RFC7413 option is always set. This is not true, at least from the fast grepping of the sources.

But the t_flag will be zeroed in all instances where FASTOPEN is not used or compiled in, thus functionally equivalent to returning false (as the old macro returned for userspace, and when TCP_RFC7413 was not set...

kib accepted this revision.Mar 14 2024, 9:33 PM
glebius closed this revision.Mar 18 2024, 4:21 PM

Committed dd7b86e2a010818b53f42200815f1858300045d7

glebius added a commit: rGdd7b86e2a010: tcp: remove IS_FASTOPEN() macro.Mar 22 2024, 5:24 PM

Revision Contents
Changeset List

PathSize
sys/
netinet/
14 lines
14 lines
tcp_stacks/
26 lines
35 lines
2 lines
2 lines
15 lines
8 lines

Diff 135765

View Options

sys/netinet/tcp_input.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2021 Jessica Clarke <jrtc27@FreeBSD.org>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
*/
/* TODO: Provide interrupt controller interface */
mhorneUnsubmitted
Not Done
Inline Actions

I would be interested in implementing this, but not sure how soon that would be...

mhorne: I would be interested in implementing this, but not sure how soon that would be...
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/bus.h>
#include <sys/kernel.h>
#include <sys/module.h>
#include <sys/rman.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/gpio.h>
#include <dev/gpio/gpiobusvar.h>
#include <dev/ofw/ofw_bus.h>
#include <dev/ofw/ofw_bus_subr.h>
#include <machine/bus.h>
/* Registers are 32-bit so can only fit 32 pins */
#define SFGPIO_MAX_PINS 32
#define SFGPIO_DEFAULT_CAPS (GPIO_PIN_INPUT | GPIO_PIN_OUTPUT)
#define SFGPIO_INPUT_VAL 0x0
#define SFGPIO_INPUT_EN 0x4
#define SFGPIO_OUTPUT_EN 0x8
#define SFGPIO_OUTPUT_VAL 0xc
#define SFGPIO_RISE_IE 0x18
#define SFGPIO_RISE_IP 0x1c
#define SFGPIO_FALL_IE 0x20
#define SFGPIO_FALL_IP 0x24
#define SFGPIO_HIGH_IE 0x28
#define SFGPIO_HIGH_IP 0x2c
#define SFGPIO_LOW_IE 0x30
#define SFGPIO_LOW_IP 0x34
struct sfgpio_softc {
device_t dev;
device_t busdev;
struct mtx mtx;
struct resource *mem_res;
int mem_rid;
struct resource *irq_res;
int irq_rid;
int npins;
struct gpio_pin gpio_pins[SFGPIO_MAX_PINS];
};
#define SFGPIO_LOCK(_sc) mtx_lock(&(_sc)->mtx)
#define SFGPIO_UNLOCK(_sc) mtx_unlock(&(_sc)->mtx)
#define SFGPIO_READ(_sc, _off) \
bus_read_4((_sc)->mem_res, (_off))
#define SFGPIO_WRITE(_sc, _off, _val) \
bus_write_4((_sc)->mem_res, (_off), (_val))
static struct ofw_compat_data compat_data[] = {
{ "sifive,gpio0", 1 },
{ NULL, 0 },
};
static int
sfgpio_probe(device_t dev)
{
if (!ofw_bus_status_okay(dev))
return (ENXIO);
if (ofw_bus_search_compatible(dev, compat_data)->ocd_data == 0)
return (ENXIO);
device_set_desc(dev, "SiFive GPIO Controller");
return (BUS_PROBE_DEFAULT);
}
static int
sfgpio_attach(device_t dev)
{
struct sfgpio_softc *sc;
phandle_t node;
int error, i;
pcell_t npins;
uint32_t input_en, output_en;
sc = device_get_softc(dev);
sc->dev = dev;
node = ofw_bus_get_node(dev);
mtx_init(&sc->mtx, device_get_nameunit(sc->dev), NULL, MTX_DEF);
sc->mem_rid = 0;
sc->mem_res = bus_alloc_resource_any(dev, SYS_RES_MEMORY,
&sc->mem_rid, RF_ACTIVE);
if (sc->mem_res == NULL) {
device_printf(dev, "Cannot allocate memory resource\n");
error = ENXIO;
goto fail;
}
if (OF_getencprop(node, "ngpios", &npins, sizeof(npins)) <= 0) {
/* Optional; defaults to 16 */
npins = 16;
} else if (npins > SFGPIO_MAX_PINS) {
device_printf(dev, "Too many pins: %d\n", npins);
error = ENXIO;
goto fail;
}
sc->npins = npins;
sc->irq_rid = 0;
sc->irq_res = bus_alloc_resource_any(dev, SYS_RES_IRQ, &sc->irq_rid,
RF_ACTIVE);
if (sc->irq_res == NULL) {
device_printf(dev, "Cannot allocate IRQ resource\n");
error = ENXIO;
goto fail;
}
input_en = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
output_en = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
for (i = 0; i < sc->npins; ++i) {
sc->gpio_pins[i].gp_pin = i;
sc->gpio_pins[i].gp_caps = SFGPIO_DEFAULT_CAPS;
sc->gpio_pins[i].gp_flags =
((input_en & (1u << i)) ? GPIO_PIN_INPUT : 0) |
((output_en & (1u << i)) ? GPIO_PIN_OUTPUT : 0);
snprintf(sc->gpio_pins[i].gp_name, GPIOMAXNAME, "GPIO%d", i);
sc->gpio_pins[i].gp_name[GPIOMAXNAME - 1] = '\0';
}
manuUnsubmitted
Done
Inline Actions

Is the device_get_unit really necessary ?
That would mean that a sifive_gpio1 driver expose sifive_gpio1.X and since you would need to -f /dev/gpioc1 too that's a bit redondant.
Same for the 'sifive_gpio' prefix, for Allwinner and Rockchip we name them after the pad/bank-pin name as this is what is used in the schematics of the different boards.

manu: Is the device_get_unit really necessary ? That would mean that a sifive_gpio1 driver expose…
jrtc27AuthorUnsubmitted
Done
Inline Actions

I was using other drivers like dwgpio and qoriq_gpio as a reference. If what they're doing also doesn't make a huge amount of sense then this I can just change this.

The schematics just name them GPIOn, with a note saying what some of them are connected to.

jrtc27: I was using other drivers like dwgpio and qoriq_gpio as a reference. If what they're doing also…
sc->busdev = gpiobus_attach_bus(dev);
if (sc->busdev == NULL) {
device_printf(dev, "Cannot attach gpiobus\n");
error = ENXIO;
goto fail;
}
return (0);
fail:
if (sc->busdev != NULL)
gpiobus_detach_bus(dev);
if (sc->irq_res != NULL)
bus_release_resource(dev, SYS_RES_IRQ, sc->irq_rid,
sc->irq_res);
if (sc->mem_res != NULL)
bus_release_resource(dev, SYS_RES_MEMORY, sc->mem_rid,
sc->mem_res);
mtx_destroy(&sc->mtx);
return (error);
}
static device_t
sfgpio_get_bus(device_t dev)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
return (sc->busdev);
}
static int
sfgpio_pin_max(device_t dev, int *maxpin)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
*maxpin = sc->npins - 1;
return (0);
}
static int
sfgpio_pin_set(device_t dev, uint32_t pin, unsigned int val)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
mhorneUnsubmitted
Done
Inline Actions

Is it ever the case that gp_pin != i? Seems like these loops could be removed, if not.

mhorne: Is it ever the case that `gp_pin != i`? Seems like these loops could be removed, if not.
jrtc27AuthorUnsubmitted
Done
Inline Actions

This code is indeed weird. A bunch of other drivers have this exact pattern (basically most drivers using struct gpio_pin, there are some exceptions that do things more simply) and as far as I can tell will also never have the pins not match, I just copied this idiom from them... So maybe I should drop these loops and just assert that sc->gpio_pins[pin].gp_pin == pin?

jrtc27: This code is indeed weird. A bunch of other drivers have this exact pattern (basically most…
mhorneUnsubmitted
Done
Inline Actions

Yeah, seems okay to me to drop the loops here.

Looks like ar5315_gpio and ar71xx_gpio are drivers where this mapping isn't 1:1, but for most it doesn't matter.

mhorne: Yeah, seems okay to me to drop the loops here. Looks like `ar5315_gpio` and `ar71xx_gpio` are…
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
if (val)
reg |= (1u << pin);
else
reg &= ~(1u << pin);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_get(device_t dev, uint32_t pin, unsigned int *val)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
if (sc->gpio_pins[pin].gp_flags & GPIO_PIN_OUTPUT)
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
else
reg = SFGPIO_READ(sc, SFGPIO_INPUT_VAL);
*val = (reg & (1u << pin)) ? 1 : 0;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_toggle(device_t dev, uint32_t pin)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
reg ^= (1u << pin);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getcaps(device_t dev, uint32_t pin, uint32_t *caps)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
*caps = sc->gpio_pins[pin].gp_caps;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getflags(device_t dev, uint32_t pin, uint32_t *flags)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
*flags = sc->gpio_pins[pin].gp_flags;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getname(device_t dev, uint32_t pin, char *name)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
memcpy(name, sc->gpio_pins[pin].gp_name, GPIOMAXNAME);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_setflags(device_t dev, uint32_t pin, uint32_t flags)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
if (flags & GPIO_PIN_INPUT) {
reg |= (1u << pin);
sc->gpio_pins[pin].gp_flags |= GPIO_PIN_INPUT;
} else {
reg &= ~(1u << pin);
sc->gpio_pins[pin].gp_flags &= ~GPIO_PIN_INPUT;
}
SFGPIO_WRITE(sc, SFGPIO_INPUT_EN, reg);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
if (flags & GPIO_PIN_OUTPUT) {
reg |= (1u << pin);
sc->gpio_pins[pin].gp_flags |= GPIO_PIN_OUTPUT;
} else {
reg &= ~(1u << pin);
sc->gpio_pins[pin].gp_flags &= ~GPIO_PIN_OUTPUT;
}
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_EN, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_access_32(device_t dev, uint32_t first_pin, uint32_t clear_pins,
uint32_t change_pins, uint32_t *orig_pins)
{
struct sfgpio_softc *sc;
uint32_t reg;
if (first_pin != 0)
return (EINVAL);
sc = device_get_softc(dev);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
if (orig_pins != NULL)
/* Only input_val is implicitly masked by input_en */
*orig_pins = SFGPIO_READ(sc, SFGPIO_INPUT_VAL) |
(reg & SFGPIO_READ(sc, SFGPIO_OUTPUT_EN));
skiboUnsubmitted
Done
Inline Actions

I think you need to update sc->gpio_pins[i].gp_flags here.

skibo: I think you need to update sc->gpio_pins[i].gp_flags here.
if ((clear_pins | change_pins) != 0)
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL,
(reg & ~clear_pins) ^ change_pins);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_config_32(device_t dev, uint32_t first_pin, uint32_t num_pins,
uint32_t *pin_flags)
{
struct sfgpio_softc *sc;
uint32_t ireg, oreg;
mhorneUnsubmitted
Not Done
Inline Actions

No detach method?

mhorne: No detach method?
manuUnsubmitted
Not Done
Inline Actions

Detach for gpio controller is complicated as it would mean to track any driver that uses a gpio. Also if there is any driver (which on embedded is more than likely as you often have reset lines etc ...) you couldn't detach.

manu: Detach for gpio controller is complicated as it would mean to track any driver that uses a gpio.
int i;
sc = device_get_softc(dev);
if (first_pin != 0 || num_pins > sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
ireg = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
oreg = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
for (i = 0; i < num_pins; ++i) {
mhorneUnsubmitted
Not Done
Inline Actions

If you did the reads after the loop, you could reduce the time spent holding the lock. I don't think it's a big deal.

mhorne: If you did the reads after the loop, you could reduce the time spent holding the lock. I don't…
jrtc27AuthorUnsubmitted
Done
Inline Actions

That's true, though I'd then need [io]{set,clear} variables, and if this driver's lock is contended then I have serious questions... :)

jrtc27: That's true, though I'd then need [io]{set,clear} variables, and if this driver's lock is…
jrtc27AuthorUnsubmitted
Done
Inline Actions

Actually no, the sc->gpio_pins[i].gp_flags all need protecting by the lock too, so reordering the loop with the I/O reads doesn't let you delay taking the lock

jrtc27: Actually no, the sc->gpio_pins[i].gp_flags all need protecting by the lock too, so reordering…
if (pin_flags[i] & GPIO_PIN_INPUT) {
ireg |= (1u << i);
oreg &= ~(1u << i);
sc->gpio_pins[i].gp_flags |= GPIO_PIN_INPUT;
sc->gpio_pins[i].gp_flags &= ~GPIO_PIN_OUTPUT;
} else if (pin_flags[i] & GPIO_PIN_OUTPUT) {
ireg &= ~(1u << i);
oreg |= (1u << i);
sc->gpio_pins[i].gp_flags &= ~GPIO_PIN_INPUT;
sc->gpio_pins[i].gp_flags |= GPIO_PIN_OUTPUT;
}
}
SFGPIO_WRITE(sc, SFGPIO_INPUT_EN, ireg);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_EN, oreg);
SFGPIO_UNLOCK(sc);
return (0);
}
static phandle_t
sfgpio_get_node(device_t bus, device_t dev)
{
return (ofw_bus_get_node(bus));
}
static device_method_t sfgpio_methods[] = {
/* Device interface */
DEVMETHOD(device_probe, sfgpio_probe),
DEVMETHOD(device_attach, sfgpio_attach),
/* GPIO protocol */
DEVMETHOD(gpio_get_bus, sfgpio_get_bus),
DEVMETHOD(gpio_pin_max, sfgpio_pin_max),
DEVMETHOD(gpio_pin_set, sfgpio_pin_set),
DEVMETHOD(gpio_pin_get, sfgpio_pin_get),
DEVMETHOD(gpio_pin_toggle, sfgpio_pin_toggle),
DEVMETHOD(gpio_pin_getcaps, sfgpio_pin_getcaps),
DEVMETHOD(gpio_pin_getflags, sfgpio_pin_getflags),
DEVMETHOD(gpio_pin_getname, sfgpio_pin_getname),
DEVMETHOD(gpio_pin_setflags, sfgpio_pin_setflags),
DEVMETHOD(gpio_pin_access_32, sfgpio_pin_access_32),
DEVMETHOD(gpio_pin_config_32, sfgpio_pin_config_32),
/* ofw_bus interface */
DEVMETHOD(ofw_bus_get_node, sfgpio_get_node),
DEVMETHOD_END
};
static devclass_t sfgpio_devclass;
DEFINE_CLASS_0(gpio, sfgpio_driver, sfgpio_methods,
sizeof(struct sfgpio_softc));
EARLY_DRIVER_MODULE(gpio, simplebus, sfgpio_driver, sfgpio_devclass, 0, 0,
BUS_PASS_INTERRUPT + BUS_PASS_ORDER_LATE);
MODULE_DEPEND(sfgpio, gpiobus, 1, 1, 1);
View Options

sys/netinet/tcp_output.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2021 Jessica Clarke <jrtc27@FreeBSD.org>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
*/
/* TODO: Provide interrupt controller interface */
mhorneUnsubmitted
Not Done
Inline Actions

I would be interested in implementing this, but not sure how soon that would be...

mhorne: I would be interested in implementing this, but not sure how soon that would be...
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/bus.h>
#include <sys/kernel.h>
#include <sys/module.h>
#include <sys/rman.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/gpio.h>
#include <dev/gpio/gpiobusvar.h>
#include <dev/ofw/ofw_bus.h>
#include <dev/ofw/ofw_bus_subr.h>
#include <machine/bus.h>
/* Registers are 32-bit so can only fit 32 pins */
#define SFGPIO_MAX_PINS 32
#define SFGPIO_DEFAULT_CAPS (GPIO_PIN_INPUT | GPIO_PIN_OUTPUT)
#define SFGPIO_INPUT_VAL 0x0
#define SFGPIO_INPUT_EN 0x4
#define SFGPIO_OUTPUT_EN 0x8
#define SFGPIO_OUTPUT_VAL 0xc
#define SFGPIO_RISE_IE 0x18
#define SFGPIO_RISE_IP 0x1c
#define SFGPIO_FALL_IE 0x20
#define SFGPIO_FALL_IP 0x24
#define SFGPIO_HIGH_IE 0x28
#define SFGPIO_HIGH_IP 0x2c
#define SFGPIO_LOW_IE 0x30
#define SFGPIO_LOW_IP 0x34
struct sfgpio_softc {
device_t dev;
device_t busdev;
struct mtx mtx;
struct resource *mem_res;
int mem_rid;
struct resource *irq_res;
int irq_rid;
int npins;
struct gpio_pin gpio_pins[SFGPIO_MAX_PINS];
};
#define SFGPIO_LOCK(_sc) mtx_lock(&(_sc)->mtx)
#define SFGPIO_UNLOCK(_sc) mtx_unlock(&(_sc)->mtx)
#define SFGPIO_READ(_sc, _off) \
bus_read_4((_sc)->mem_res, (_off))
#define SFGPIO_WRITE(_sc, _off, _val) \
bus_write_4((_sc)->mem_res, (_off), (_val))
static struct ofw_compat_data compat_data[] = {
{ "sifive,gpio0", 1 },
{ NULL, 0 },
};
static int
sfgpio_probe(device_t dev)
{
if (!ofw_bus_status_okay(dev))
return (ENXIO);
if (ofw_bus_search_compatible(dev, compat_data)->ocd_data == 0)
return (ENXIO);
device_set_desc(dev, "SiFive GPIO Controller");
return (BUS_PROBE_DEFAULT);
}
static int
sfgpio_attach(device_t dev)
{
struct sfgpio_softc *sc;
phandle_t node;
int error, i;
pcell_t npins;
uint32_t input_en, output_en;
sc = device_get_softc(dev);
sc->dev = dev;
node = ofw_bus_get_node(dev);
mtx_init(&sc->mtx, device_get_nameunit(sc->dev), NULL, MTX_DEF);
sc->mem_rid = 0;
sc->mem_res = bus_alloc_resource_any(dev, SYS_RES_MEMORY,
&sc->mem_rid, RF_ACTIVE);
if (sc->mem_res == NULL) {
device_printf(dev, "Cannot allocate memory resource\n");
error = ENXIO;
goto fail;
}
if (OF_getencprop(node, "ngpios", &npins, sizeof(npins)) <= 0) {
/* Optional; defaults to 16 */
npins = 16;
} else if (npins > SFGPIO_MAX_PINS) {
device_printf(dev, "Too many pins: %d\n", npins);
error = ENXIO;
goto fail;
}
sc->npins = npins;
sc->irq_rid = 0;
sc->irq_res = bus_alloc_resource_any(dev, SYS_RES_IRQ, &sc->irq_rid,
RF_ACTIVE);
if (sc->irq_res == NULL) {
device_printf(dev, "Cannot allocate IRQ resource\n");
error = ENXIO;
goto fail;
}
input_en = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
output_en = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
for (i = 0; i < sc->npins; ++i) {
sc->gpio_pins[i].gp_pin = i;
sc->gpio_pins[i].gp_caps = SFGPIO_DEFAULT_CAPS;
sc->gpio_pins[i].gp_flags =
((input_en & (1u << i)) ? GPIO_PIN_INPUT : 0) |
((output_en & (1u << i)) ? GPIO_PIN_OUTPUT : 0);
snprintf(sc->gpio_pins[i].gp_name, GPIOMAXNAME, "GPIO%d", i);
sc->gpio_pins[i].gp_name[GPIOMAXNAME - 1] = '\0';
}
manuUnsubmitted
Done
Inline Actions

Is the device_get_unit really necessary ?
That would mean that a sifive_gpio1 driver expose sifive_gpio1.X and since you would need to -f /dev/gpioc1 too that's a bit redondant.
Same for the 'sifive_gpio' prefix, for Allwinner and Rockchip we name them after the pad/bank-pin name as this is what is used in the schematics of the different boards.

manu: Is the device_get_unit really necessary ? That would mean that a sifive_gpio1 driver expose…
jrtc27AuthorUnsubmitted
Done
Inline Actions

I was using other drivers like dwgpio and qoriq_gpio as a reference. If what they're doing also doesn't make a huge amount of sense then this I can just change this.

The schematics just name them GPIOn, with a note saying what some of them are connected to.

jrtc27: I was using other drivers like dwgpio and qoriq_gpio as a reference. If what they're doing also…
sc->busdev = gpiobus_attach_bus(dev);
if (sc->busdev == NULL) {
device_printf(dev, "Cannot attach gpiobus\n");
error = ENXIO;
goto fail;
}
return (0);
fail:
if (sc->busdev != NULL)
gpiobus_detach_bus(dev);
if (sc->irq_res != NULL)
bus_release_resource(dev, SYS_RES_IRQ, sc->irq_rid,
sc->irq_res);
if (sc->mem_res != NULL)
bus_release_resource(dev, SYS_RES_MEMORY, sc->mem_rid,
sc->mem_res);
mtx_destroy(&sc->mtx);
return (error);
}
static device_t
sfgpio_get_bus(device_t dev)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
return (sc->busdev);
}
static int
sfgpio_pin_max(device_t dev, int *maxpin)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
*maxpin = sc->npins - 1;
return (0);
}
static int
sfgpio_pin_set(device_t dev, uint32_t pin, unsigned int val)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
mhorneUnsubmitted
Done
Inline Actions

Is it ever the case that gp_pin != i? Seems like these loops could be removed, if not.

mhorne: Is it ever the case that `gp_pin != i`? Seems like these loops could be removed, if not.
jrtc27AuthorUnsubmitted
Done
Inline Actions

This code is indeed weird. A bunch of other drivers have this exact pattern (basically most drivers using struct gpio_pin, there are some exceptions that do things more simply) and as far as I can tell will also never have the pins not match, I just copied this idiom from them... So maybe I should drop these loops and just assert that sc->gpio_pins[pin].gp_pin == pin?

jrtc27: This code is indeed weird. A bunch of other drivers have this exact pattern (basically most…
mhorneUnsubmitted
Done
Inline Actions

Yeah, seems okay to me to drop the loops here.

Looks like ar5315_gpio and ar71xx_gpio are drivers where this mapping isn't 1:1, but for most it doesn't matter.

mhorne: Yeah, seems okay to me to drop the loops here. Looks like `ar5315_gpio` and `ar71xx_gpio` are…
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
if (val)
reg |= (1u << pin);
else
reg &= ~(1u << pin);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_get(device_t dev, uint32_t pin, unsigned int *val)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
if (sc->gpio_pins[pin].gp_flags & GPIO_PIN_OUTPUT)
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
else
reg = SFGPIO_READ(sc, SFGPIO_INPUT_VAL);
*val = (reg & (1u << pin)) ? 1 : 0;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_toggle(device_t dev, uint32_t pin)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
reg ^= (1u << pin);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getcaps(device_t dev, uint32_t pin, uint32_t *caps)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
*caps = sc->gpio_pins[pin].gp_caps;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getflags(device_t dev, uint32_t pin, uint32_t *flags)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
*flags = sc->gpio_pins[pin].gp_flags;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getname(device_t dev, uint32_t pin, char *name)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
memcpy(name, sc->gpio_pins[pin].gp_name, GPIOMAXNAME);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_setflags(device_t dev, uint32_t pin, uint32_t flags)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
if (flags & GPIO_PIN_INPUT) {
reg |= (1u << pin);
sc->gpio_pins[pin].gp_flags |= GPIO_PIN_INPUT;
} else {
reg &= ~(1u << pin);
sc->gpio_pins[pin].gp_flags &= ~GPIO_PIN_INPUT;
}
SFGPIO_WRITE(sc, SFGPIO_INPUT_EN, reg);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
if (flags & GPIO_PIN_OUTPUT) {
reg |= (1u << pin);
sc->gpio_pins[pin].gp_flags |= GPIO_PIN_OUTPUT;
} else {
reg &= ~(1u << pin);
sc->gpio_pins[pin].gp_flags &= ~GPIO_PIN_OUTPUT;
}
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_EN, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_access_32(device_t dev, uint32_t first_pin, uint32_t clear_pins,
uint32_t change_pins, uint32_t *orig_pins)
{
struct sfgpio_softc *sc;
uint32_t reg;
if (first_pin != 0)
return (EINVAL);
sc = device_get_softc(dev);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
if (orig_pins != NULL)
/* Only input_val is implicitly masked by input_en */
*orig_pins = SFGPIO_READ(sc, SFGPIO_INPUT_VAL) |
(reg & SFGPIO_READ(sc, SFGPIO_OUTPUT_EN));
skiboUnsubmitted
Done
Inline Actions

I think you need to update sc->gpio_pins[i].gp_flags here.

skibo: I think you need to update sc->gpio_pins[i].gp_flags here.
if ((clear_pins | change_pins) != 0)
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL,
(reg & ~clear_pins) ^ change_pins);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_config_32(device_t dev, uint32_t first_pin, uint32_t num_pins,
uint32_t *pin_flags)
{
struct sfgpio_softc *sc;
uint32_t ireg, oreg;
mhorneUnsubmitted
Not Done
Inline Actions

No detach method?

mhorne: No detach method?
manuUnsubmitted
Not Done
Inline Actions

Detach for gpio controller is complicated as it would mean to track any driver that uses a gpio. Also if there is any driver (which on embedded is more than likely as you often have reset lines etc ...) you couldn't detach.

manu: Detach for gpio controller is complicated as it would mean to track any driver that uses a gpio.
int i;
sc = device_get_softc(dev);
if (first_pin != 0 || num_pins > sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
ireg = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
oreg = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
for (i = 0; i < num_pins; ++i) {
mhorneUnsubmitted
Not Done
Inline Actions

If you did the reads after the loop, you could reduce the time spent holding the lock. I don't think it's a big deal.

mhorne: If you did the reads after the loop, you could reduce the time spent holding the lock. I don't…
jrtc27AuthorUnsubmitted
Done
Inline Actions

That's true, though I'd then need [io]{set,clear} variables, and if this driver's lock is contended then I have serious questions... :)

jrtc27: That's true, though I'd then need [io]{set,clear} variables, and if this driver's lock is…
jrtc27AuthorUnsubmitted
Done
Inline Actions

Actually no, the sc->gpio_pins[i].gp_flags all need protecting by the lock too, so reordering the loop with the I/O reads doesn't let you delay taking the lock

jrtc27: Actually no, the sc->gpio_pins[i].gp_flags all need protecting by the lock too, so reordering…
if (pin_flags[i] & GPIO_PIN_INPUT) {
ireg |= (1u << i);
oreg &= ~(1u << i);
sc->gpio_pins[i].gp_flags |= GPIO_PIN_INPUT;
sc->gpio_pins[i].gp_flags &= ~GPIO_PIN_OUTPUT;
} else if (pin_flags[i] & GPIO_PIN_OUTPUT) {
ireg &= ~(1u << i);
oreg |= (1u << i);
sc->gpio_pins[i].gp_flags &= ~GPIO_PIN_INPUT;
sc->gpio_pins[i].gp_flags |= GPIO_PIN_OUTPUT;
}
}
SFGPIO_WRITE(sc, SFGPIO_INPUT_EN, ireg);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_EN, oreg);
SFGPIO_UNLOCK(sc);
return (0);
}
static phandle_t
sfgpio_get_node(device_t bus, device_t dev)
{
return (ofw_bus_get_node(bus));
}
static device_method_t sfgpio_methods[] = {
/* Device interface */
DEVMETHOD(device_probe, sfgpio_probe),
DEVMETHOD(device_attach, sfgpio_attach),
/* GPIO protocol */
DEVMETHOD(gpio_get_bus, sfgpio_get_bus),
DEVMETHOD(gpio_pin_max, sfgpio_pin_max),
DEVMETHOD(gpio_pin_set, sfgpio_pin_set),
DEVMETHOD(gpio_pin_get, sfgpio_pin_get),
DEVMETHOD(gpio_pin_toggle, sfgpio_pin_toggle),
DEVMETHOD(gpio_pin_getcaps, sfgpio_pin_getcaps),
DEVMETHOD(gpio_pin_getflags, sfgpio_pin_getflags),
DEVMETHOD(gpio_pin_getname, sfgpio_pin_getname),
DEVMETHOD(gpio_pin_setflags, sfgpio_pin_setflags),
DEVMETHOD(gpio_pin_access_32, sfgpio_pin_access_32),
DEVMETHOD(gpio_pin_config_32, sfgpio_pin_config_32),
/* ofw_bus interface */
DEVMETHOD(ofw_bus_get_node, sfgpio_get_node),
DEVMETHOD_END
};
static devclass_t sfgpio_devclass;
DEFINE_CLASS_0(gpio, sfgpio_driver, sfgpio_methods,
sizeof(struct sfgpio_softc));
EARLY_DRIVER_MODULE(gpio, simplebus, sfgpio_driver, sfgpio_devclass, 0, 0,
BUS_PASS_INTERRUPT + BUS_PASS_ORDER_LATE);
MODULE_DEPEND(sfgpio, gpiobus, 1, 1, 1);
View Options

sys/netinet/tcp_stacks/bbr.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2021 Jessica Clarke <jrtc27@FreeBSD.org>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
*/
/* TODO: Provide interrupt controller interface */
mhorneUnsubmitted
Not Done
Inline Actions

I would be interested in implementing this, but not sure how soon that would be...

mhorne: I would be interested in implementing this, but not sure how soon that would be...
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/bus.h>
#include <sys/kernel.h>
#include <sys/module.h>
#include <sys/rman.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/gpio.h>
#include <dev/gpio/gpiobusvar.h>
#include <dev/ofw/ofw_bus.h>
#include <dev/ofw/ofw_bus_subr.h>
#include <machine/bus.h>
/* Registers are 32-bit so can only fit 32 pins */
#define SFGPIO_MAX_PINS 32
#define SFGPIO_DEFAULT_CAPS (GPIO_PIN_INPUT | GPIO_PIN_OUTPUT)
#define SFGPIO_INPUT_VAL 0x0
#define SFGPIO_INPUT_EN 0x4
#define SFGPIO_OUTPUT_EN 0x8
#define SFGPIO_OUTPUT_VAL 0xc
#define SFGPIO_RISE_IE 0x18
#define SFGPIO_RISE_IP 0x1c
#define SFGPIO_FALL_IE 0x20
#define SFGPIO_FALL_IP 0x24
#define SFGPIO_HIGH_IE 0x28
#define SFGPIO_HIGH_IP 0x2c
#define SFGPIO_LOW_IE 0x30
#define SFGPIO_LOW_IP 0x34
struct sfgpio_softc {
device_t dev;
device_t busdev;
struct mtx mtx;
struct resource *mem_res;
int mem_rid;
struct resource *irq_res;
int irq_rid;
int npins;
struct gpio_pin gpio_pins[SFGPIO_MAX_PINS];
};
#define SFGPIO_LOCK(_sc) mtx_lock(&(_sc)->mtx)
#define SFGPIO_UNLOCK(_sc) mtx_unlock(&(_sc)->mtx)
#define SFGPIO_READ(_sc, _off) \
bus_read_4((_sc)->mem_res, (_off))
#define SFGPIO_WRITE(_sc, _off, _val) \
bus_write_4((_sc)->mem_res, (_off), (_val))
static struct ofw_compat_data compat_data[] = {
{ "sifive,gpio0", 1 },
{ NULL, 0 },
};
static int
sfgpio_probe(device_t dev)
{
if (!ofw_bus_status_okay(dev))
return (ENXIO);
if (ofw_bus_search_compatible(dev, compat_data)->ocd_data == 0)
return (ENXIO);
device_set_desc(dev, "SiFive GPIO Controller");
return (BUS_PROBE_DEFAULT);
}
static int
sfgpio_attach(device_t dev)
{
struct sfgpio_softc *sc;
phandle_t node;
int error, i;
pcell_t npins;
uint32_t input_en, output_en;
sc = device_get_softc(dev);
sc->dev = dev;
node = ofw_bus_get_node(dev);
mtx_init(&sc->mtx, device_get_nameunit(sc->dev), NULL, MTX_DEF);
sc->mem_rid = 0;
sc->mem_res = bus_alloc_resource_any(dev, SYS_RES_MEMORY,
&sc->mem_rid, RF_ACTIVE);
if (sc->mem_res == NULL) {
device_printf(dev, "Cannot allocate memory resource\n");
error = ENXIO;
goto fail;
}
if (OF_getencprop(node, "ngpios", &npins, sizeof(npins)) <= 0) {
/* Optional; defaults to 16 */
npins = 16;
} else if (npins > SFGPIO_MAX_PINS) {
device_printf(dev, "Too many pins: %d\n", npins);
error = ENXIO;
goto fail;
}
sc->npins = npins;
sc->irq_rid = 0;
sc->irq_res = bus_alloc_resource_any(dev, SYS_RES_IRQ, &sc->irq_rid,
RF_ACTIVE);
if (sc->irq_res == NULL) {
device_printf(dev, "Cannot allocate IRQ resource\n");
error = ENXIO;
goto fail;
}
input_en = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
output_en = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
for (i = 0; i < sc->npins; ++i) {
sc->gpio_pins[i].gp_pin = i;
sc->gpio_pins[i].gp_caps = SFGPIO_DEFAULT_CAPS;
sc->gpio_pins[i].gp_flags =
((input_en & (1u << i)) ? GPIO_PIN_INPUT : 0) |
((output_en & (1u << i)) ? GPIO_PIN_OUTPUT : 0);
snprintf(sc->gpio_pins[i].gp_name, GPIOMAXNAME, "GPIO%d", i);
sc->gpio_pins[i].gp_name[GPIOMAXNAME - 1] = '\0';
}
manuUnsubmitted
Done
Inline Actions

Is the device_get_unit really necessary ?
That would mean that a sifive_gpio1 driver expose sifive_gpio1.X and since you would need to -f /dev/gpioc1 too that's a bit redondant.
Same for the 'sifive_gpio' prefix, for Allwinner and Rockchip we name them after the pad/bank-pin name as this is what is used in the schematics of the different boards.

manu: Is the device_get_unit really necessary ? That would mean that a sifive_gpio1 driver expose…
jrtc27AuthorUnsubmitted
Done
Inline Actions

I was using other drivers like dwgpio and qoriq_gpio as a reference. If what they're doing also doesn't make a huge amount of sense then this I can just change this.

The schematics just name them GPIOn, with a note saying what some of them are connected to.

jrtc27: I was using other drivers like dwgpio and qoriq_gpio as a reference. If what they're doing also…
sc->busdev = gpiobus_attach_bus(dev);
if (sc->busdev == NULL) {
device_printf(dev, "Cannot attach gpiobus\n");
error = ENXIO;
goto fail;
}
return (0);
fail:
if (sc->busdev != NULL)
gpiobus_detach_bus(dev);
if (sc->irq_res != NULL)
bus_release_resource(dev, SYS_RES_IRQ, sc->irq_rid,
sc->irq_res);
if (sc->mem_res != NULL)
bus_release_resource(dev, SYS_RES_MEMORY, sc->mem_rid,
sc->mem_res);
mtx_destroy(&sc->mtx);
return (error);
}
static device_t
sfgpio_get_bus(device_t dev)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
return (sc->busdev);
}
static int
sfgpio_pin_max(device_t dev, int *maxpin)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
*maxpin = sc->npins - 1;
return (0);
}
static int
sfgpio_pin_set(device_t dev, uint32_t pin, unsigned int val)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
mhorneUnsubmitted
Done
Inline Actions

Is it ever the case that gp_pin != i? Seems like these loops could be removed, if not.

mhorne: Is it ever the case that `gp_pin != i`? Seems like these loops could be removed, if not.
jrtc27AuthorUnsubmitted
Done
Inline Actions

This code is indeed weird. A bunch of other drivers have this exact pattern (basically most drivers using struct gpio_pin, there are some exceptions that do things more simply) and as far as I can tell will also never have the pins not match, I just copied this idiom from them... So maybe I should drop these loops and just assert that sc->gpio_pins[pin].gp_pin == pin?

jrtc27: This code is indeed weird. A bunch of other drivers have this exact pattern (basically most…
mhorneUnsubmitted
Done
Inline Actions

Yeah, seems okay to me to drop the loops here.

Looks like ar5315_gpio and ar71xx_gpio are drivers where this mapping isn't 1:1, but for most it doesn't matter.

mhorne: Yeah, seems okay to me to drop the loops here. Looks like `ar5315_gpio` and `ar71xx_gpio` are…
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
if (val)
reg |= (1u << pin);
else
reg &= ~(1u << pin);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_get(device_t dev, uint32_t pin, unsigned int *val)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
if (sc->gpio_pins[pin].gp_flags & GPIO_PIN_OUTPUT)
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
else
reg = SFGPIO_READ(sc, SFGPIO_INPUT_VAL);
*val = (reg & (1u << pin)) ? 1 : 0;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_toggle(device_t dev, uint32_t pin)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
reg ^= (1u << pin);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getcaps(device_t dev, uint32_t pin, uint32_t *caps)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
*caps = sc->gpio_pins[pin].gp_caps;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getflags(device_t dev, uint32_t pin, uint32_t *flags)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
*flags = sc->gpio_pins[pin].gp_flags;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getname(device_t dev, uint32_t pin, char *name)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
memcpy(name, sc->gpio_pins[pin].gp_name, GPIOMAXNAME);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_setflags(device_t dev, uint32_t pin, uint32_t flags)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
if (flags & GPIO_PIN_INPUT) {
reg |= (1u << pin);
sc->gpio_pins[pin].gp_flags |= GPIO_PIN_INPUT;
} else {
reg &= ~(1u << pin);
sc->gpio_pins[pin].gp_flags &= ~GPIO_PIN_INPUT;
}
SFGPIO_WRITE(sc, SFGPIO_INPUT_EN, reg);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
if (flags & GPIO_PIN_OUTPUT) {
reg |= (1u << pin);
sc->gpio_pins[pin].gp_flags |= GPIO_PIN_OUTPUT;
} else {
reg &= ~(1u << pin);
sc->gpio_pins[pin].gp_flags &= ~GPIO_PIN_OUTPUT;
}
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_EN, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_access_32(device_t dev, uint32_t first_pin, uint32_t clear_pins,
uint32_t change_pins, uint32_t *orig_pins)
{
struct sfgpio_softc *sc;
uint32_t reg;
if (first_pin != 0)
return (EINVAL);
sc = device_get_softc(dev);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
if (orig_pins != NULL)
/* Only input_val is implicitly masked by input_en */
*orig_pins = SFGPIO_READ(sc, SFGPIO_INPUT_VAL) |
(reg & SFGPIO_READ(sc, SFGPIO_OUTPUT_EN));
skiboUnsubmitted
Done
Inline Actions

I think you need to update sc->gpio_pins[i].gp_flags here.

skibo: I think you need to update sc->gpio_pins[i].gp_flags here.
if ((clear_pins | change_pins) != 0)
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL,
(reg & ~clear_pins) ^ change_pins);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_config_32(device_t dev, uint32_t first_pin, uint32_t num_pins,
uint32_t *pin_flags)
{
struct sfgpio_softc *sc;
uint32_t ireg, oreg;
mhorneUnsubmitted
Not Done
Inline Actions

No detach method?

mhorne: No detach method?
manuUnsubmitted
Not Done
Inline Actions

Detach for gpio controller is complicated as it would mean to track any driver that uses a gpio. Also if there is any driver (which on embedded is more than likely as you often have reset lines etc ...) you couldn't detach.

manu: Detach for gpio controller is complicated as it would mean to track any driver that uses a gpio.
int i;
sc = device_get_softc(dev);
if (first_pin != 0 || num_pins > sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
ireg = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
oreg = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
for (i = 0; i < num_pins; ++i) {
mhorneUnsubmitted
Not Done
Inline Actions

If you did the reads after the loop, you could reduce the time spent holding the lock. I don't think it's a big deal.

mhorne: If you did the reads after the loop, you could reduce the time spent holding the lock. I don't…
jrtc27AuthorUnsubmitted
Done
Inline Actions

That's true, though I'd then need [io]{set,clear} variables, and if this driver's lock is contended then I have serious questions... :)

jrtc27: That's true, though I'd then need [io]{set,clear} variables, and if this driver's lock is…
jrtc27AuthorUnsubmitted
Done
Inline Actions

Actually no, the sc->gpio_pins[i].gp_flags all need protecting by the lock too, so reordering the loop with the I/O reads doesn't let you delay taking the lock

jrtc27: Actually no, the sc->gpio_pins[i].gp_flags all need protecting by the lock too, so reordering…
if (pin_flags[i] & GPIO_PIN_INPUT) {
ireg |= (1u << i);
oreg &= ~(1u << i);
sc->gpio_pins[i].gp_flags |= GPIO_PIN_INPUT;
sc->gpio_pins[i].gp_flags &= ~GPIO_PIN_OUTPUT;
} else if (pin_flags[i] & GPIO_PIN_OUTPUT) {
ireg &= ~(1u << i);
oreg |= (1u << i);
sc->gpio_pins[i].gp_flags &= ~GPIO_PIN_INPUT;
sc->gpio_pins[i].gp_flags |= GPIO_PIN_OUTPUT;
}
}
SFGPIO_WRITE(sc, SFGPIO_INPUT_EN, ireg);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_EN, oreg);
SFGPIO_UNLOCK(sc);
return (0);
}
static phandle_t
sfgpio_get_node(device_t bus, device_t dev)
{
return (ofw_bus_get_node(bus));
}
static device_method_t sfgpio_methods[] = {
/* Device interface */
DEVMETHOD(device_probe, sfgpio_probe),
DEVMETHOD(device_attach, sfgpio_attach),
/* GPIO protocol */
DEVMETHOD(gpio_get_bus, sfgpio_get_bus),
DEVMETHOD(gpio_pin_max, sfgpio_pin_max),
DEVMETHOD(gpio_pin_set, sfgpio_pin_set),
DEVMETHOD(gpio_pin_get, sfgpio_pin_get),
DEVMETHOD(gpio_pin_toggle, sfgpio_pin_toggle),
DEVMETHOD(gpio_pin_getcaps, sfgpio_pin_getcaps),
DEVMETHOD(gpio_pin_getflags, sfgpio_pin_getflags),
DEVMETHOD(gpio_pin_getname, sfgpio_pin_getname),
DEVMETHOD(gpio_pin_setflags, sfgpio_pin_setflags),
DEVMETHOD(gpio_pin_access_32, sfgpio_pin_access_32),
DEVMETHOD(gpio_pin_config_32, sfgpio_pin_config_32),
/* ofw_bus interface */
DEVMETHOD(ofw_bus_get_node, sfgpio_get_node),
DEVMETHOD_END
};
static devclass_t sfgpio_devclass;
DEFINE_CLASS_0(gpio, sfgpio_driver, sfgpio_methods,
sizeof(struct sfgpio_softc));
EARLY_DRIVER_MODULE(gpio, simplebus, sfgpio_driver, sfgpio_devclass, 0, 0,
BUS_PASS_INTERRUPT + BUS_PASS_ORDER_LATE);
MODULE_DEPEND(sfgpio, gpiobus, 1, 1, 1);
View Options

sys/netinet/tcp_stacks/rack.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2021 Jessica Clarke <jrtc27@FreeBSD.org>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
*/
/* TODO: Provide interrupt controller interface */
mhorneUnsubmitted
Not Done
Inline Actions

I would be interested in implementing this, but not sure how soon that would be...

mhorne: I would be interested in implementing this, but not sure how soon that would be...
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/bus.h>
#include <sys/kernel.h>
#include <sys/module.h>
#include <sys/rman.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/gpio.h>
#include <dev/gpio/gpiobusvar.h>
#include <dev/ofw/ofw_bus.h>
#include <dev/ofw/ofw_bus_subr.h>
#include <machine/bus.h>
/* Registers are 32-bit so can only fit 32 pins */
#define SFGPIO_MAX_PINS 32
#define SFGPIO_DEFAULT_CAPS (GPIO_PIN_INPUT | GPIO_PIN_OUTPUT)
#define SFGPIO_INPUT_VAL 0x0
#define SFGPIO_INPUT_EN 0x4
#define SFGPIO_OUTPUT_EN 0x8
#define SFGPIO_OUTPUT_VAL 0xc
#define SFGPIO_RISE_IE 0x18
#define SFGPIO_RISE_IP 0x1c
#define SFGPIO_FALL_IE 0x20
#define SFGPIO_FALL_IP 0x24
#define SFGPIO_HIGH_IE 0x28
#define SFGPIO_HIGH_IP 0x2c
#define SFGPIO_LOW_IE 0x30
#define SFGPIO_LOW_IP 0x34
struct sfgpio_softc {
device_t dev;
device_t busdev;
struct mtx mtx;
struct resource *mem_res;
int mem_rid;
struct resource *irq_res;
int irq_rid;
int npins;
struct gpio_pin gpio_pins[SFGPIO_MAX_PINS];
};
#define SFGPIO_LOCK(_sc) mtx_lock(&(_sc)->mtx)
#define SFGPIO_UNLOCK(_sc) mtx_unlock(&(_sc)->mtx)
#define SFGPIO_READ(_sc, _off) \
bus_read_4((_sc)->mem_res, (_off))
#define SFGPIO_WRITE(_sc, _off, _val) \
bus_write_4((_sc)->mem_res, (_off), (_val))
static struct ofw_compat_data compat_data[] = {
{ "sifive,gpio0", 1 },
{ NULL, 0 },
};
static int
sfgpio_probe(device_t dev)
{
if (!ofw_bus_status_okay(dev))
return (ENXIO);
if (ofw_bus_search_compatible(dev, compat_data)->ocd_data == 0)
return (ENXIO);
device_set_desc(dev, "SiFive GPIO Controller");
return (BUS_PROBE_DEFAULT);
}
static int
sfgpio_attach(device_t dev)
{
struct sfgpio_softc *sc;
phandle_t node;
int error, i;
pcell_t npins;
uint32_t input_en, output_en;
sc = device_get_softc(dev);
sc->dev = dev;
node = ofw_bus_get_node(dev);
mtx_init(&sc->mtx, device_get_nameunit(sc->dev), NULL, MTX_DEF);
sc->mem_rid = 0;
sc->mem_res = bus_alloc_resource_any(dev, SYS_RES_MEMORY,
&sc->mem_rid, RF_ACTIVE);
if (sc->mem_res == NULL) {
device_printf(dev, "Cannot allocate memory resource\n");
error = ENXIO;
goto fail;
}
if (OF_getencprop(node, "ngpios", &npins, sizeof(npins)) <= 0) {
/* Optional; defaults to 16 */
npins = 16;
} else if (npins > SFGPIO_MAX_PINS) {
device_printf(dev, "Too many pins: %d\n", npins);
error = ENXIO;
goto fail;
}
sc->npins = npins;
sc->irq_rid = 0;
sc->irq_res = bus_alloc_resource_any(dev, SYS_RES_IRQ, &sc->irq_rid,
RF_ACTIVE);
if (sc->irq_res == NULL) {
device_printf(dev, "Cannot allocate IRQ resource\n");
error = ENXIO;
goto fail;
}
input_en = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
output_en = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
for (i = 0; i < sc->npins; ++i) {
sc->gpio_pins[i].gp_pin = i;
sc->gpio_pins[i].gp_caps = SFGPIO_DEFAULT_CAPS;
sc->gpio_pins[i].gp_flags =
((input_en & (1u << i)) ? GPIO_PIN_INPUT : 0) |
((output_en & (1u << i)) ? GPIO_PIN_OUTPUT : 0);
snprintf(sc->gpio_pins[i].gp_name, GPIOMAXNAME, "GPIO%d", i);
sc->gpio_pins[i].gp_name[GPIOMAXNAME - 1] = '\0';
}
manuUnsubmitted
Done
Inline Actions

Is the device_get_unit really necessary ?
That would mean that a sifive_gpio1 driver expose sifive_gpio1.X and since you would need to -f /dev/gpioc1 too that's a bit redondant.
Same for the 'sifive_gpio' prefix, for Allwinner and Rockchip we name them after the pad/bank-pin name as this is what is used in the schematics of the different boards.

manu: Is the device_get_unit really necessary ? That would mean that a sifive_gpio1 driver expose…
jrtc27AuthorUnsubmitted
Done
Inline Actions

I was using other drivers like dwgpio and qoriq_gpio as a reference. If what they're doing also doesn't make a huge amount of sense then this I can just change this.

The schematics just name them GPIOn, with a note saying what some of them are connected to.

jrtc27: I was using other drivers like dwgpio and qoriq_gpio as a reference. If what they're doing also…
sc->busdev = gpiobus_attach_bus(dev);
if (sc->busdev == NULL) {
device_printf(dev, "Cannot attach gpiobus\n");
error = ENXIO;
goto fail;
}
return (0);
fail:
if (sc->busdev != NULL)
gpiobus_detach_bus(dev);
if (sc->irq_res != NULL)
bus_release_resource(dev, SYS_RES_IRQ, sc->irq_rid,
sc->irq_res);
if (sc->mem_res != NULL)
bus_release_resource(dev, SYS_RES_MEMORY, sc->mem_rid,
sc->mem_res);
mtx_destroy(&sc->mtx);
return (error);
}
static device_t
sfgpio_get_bus(device_t dev)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
return (sc->busdev);
}
static int
sfgpio_pin_max(device_t dev, int *maxpin)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
*maxpin = sc->npins - 1;
return (0);
}
static int
sfgpio_pin_set(device_t dev, uint32_t pin, unsigned int val)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
mhorneUnsubmitted
Done
Inline Actions

Is it ever the case that gp_pin != i? Seems like these loops could be removed, if not.

mhorne: Is it ever the case that `gp_pin != i`? Seems like these loops could be removed, if not.
jrtc27AuthorUnsubmitted
Done
Inline Actions

This code is indeed weird. A bunch of other drivers have this exact pattern (basically most drivers using struct gpio_pin, there are some exceptions that do things more simply) and as far as I can tell will also never have the pins not match, I just copied this idiom from them... So maybe I should drop these loops and just assert that sc->gpio_pins[pin].gp_pin == pin?

jrtc27: This code is indeed weird. A bunch of other drivers have this exact pattern (basically most…
mhorneUnsubmitted
Done
Inline Actions

Yeah, seems okay to me to drop the loops here.

Looks like ar5315_gpio and ar71xx_gpio are drivers where this mapping isn't 1:1, but for most it doesn't matter.

mhorne: Yeah, seems okay to me to drop the loops here. Looks like `ar5315_gpio` and `ar71xx_gpio` are…
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
if (val)
reg |= (1u << pin);
else
reg &= ~(1u << pin);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_get(device_t dev, uint32_t pin, unsigned int *val)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
if (sc->gpio_pins[pin].gp_flags & GPIO_PIN_OUTPUT)
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
else
reg = SFGPIO_READ(sc, SFGPIO_INPUT_VAL);
*val = (reg & (1u << pin)) ? 1 : 0;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_toggle(device_t dev, uint32_t pin)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
reg ^= (1u << pin);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getcaps(device_t dev, uint32_t pin, uint32_t *caps)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
*caps = sc->gpio_pins[pin].gp_caps;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getflags(device_t dev, uint32_t pin, uint32_t *flags)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
*flags = sc->gpio_pins[pin].gp_flags;
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_getname(device_t dev, uint32_t pin, char *name)
{
struct sfgpio_softc *sc;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
memcpy(name, sc->gpio_pins[pin].gp_name, GPIOMAXNAME);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_setflags(device_t dev, uint32_t pin, uint32_t flags)
{
struct sfgpio_softc *sc;
uint32_t reg;
sc = device_get_softc(dev);
if (pin >= sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
if (flags & GPIO_PIN_INPUT) {
reg |= (1u << pin);
sc->gpio_pins[pin].gp_flags |= GPIO_PIN_INPUT;
} else {
reg &= ~(1u << pin);
sc->gpio_pins[pin].gp_flags &= ~GPIO_PIN_INPUT;
}
SFGPIO_WRITE(sc, SFGPIO_INPUT_EN, reg);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
if (flags & GPIO_PIN_OUTPUT) {
reg |= (1u << pin);
sc->gpio_pins[pin].gp_flags |= GPIO_PIN_OUTPUT;
} else {
reg &= ~(1u << pin);
sc->gpio_pins[pin].gp_flags &= ~GPIO_PIN_OUTPUT;
}
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_EN, reg);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_access_32(device_t dev, uint32_t first_pin, uint32_t clear_pins,
uint32_t change_pins, uint32_t *orig_pins)
{
struct sfgpio_softc *sc;
uint32_t reg;
if (first_pin != 0)
return (EINVAL);
sc = device_get_softc(dev);
SFGPIO_LOCK(sc);
reg = SFGPIO_READ(sc, SFGPIO_OUTPUT_VAL);
if (orig_pins != NULL)
/* Only input_val is implicitly masked by input_en */
*orig_pins = SFGPIO_READ(sc, SFGPIO_INPUT_VAL) |
(reg & SFGPIO_READ(sc, SFGPIO_OUTPUT_EN));
skiboUnsubmitted
Done
Inline Actions

I think you need to update sc->gpio_pins[i].gp_flags here.

skibo: I think you need to update sc->gpio_pins[i].gp_flags here.
if ((clear_pins | change_pins) != 0)
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_VAL,
(reg & ~clear_pins) ^ change_pins);
SFGPIO_UNLOCK(sc);
return (0);
}
static int
sfgpio_pin_config_32(device_t dev, uint32_t first_pin, uint32_t num_pins,
uint32_t *pin_flags)
{
struct sfgpio_softc *sc;
uint32_t ireg, oreg;
mhorneUnsubmitted
Not Done
Inline Actions

No detach method?

mhorne: No detach method?
manuUnsubmitted
Not Done
Inline Actions

Detach for gpio controller is complicated as it would mean to track any driver that uses a gpio. Also if there is any driver (which on embedded is more than likely as you often have reset lines etc ...) you couldn't detach.

manu: Detach for gpio controller is complicated as it would mean to track any driver that uses a gpio.
int i;
sc = device_get_softc(dev);
if (first_pin != 0 || num_pins > sc->npins)
return (EINVAL);
SFGPIO_LOCK(sc);
ireg = SFGPIO_READ(sc, SFGPIO_INPUT_EN);
oreg = SFGPIO_READ(sc, SFGPIO_OUTPUT_EN);
for (i = 0; i < num_pins; ++i) {
mhorneUnsubmitted
Not Done
Inline Actions

If you did the reads after the loop, you could reduce the time spent holding the lock. I don't think it's a big deal.

mhorne: If you did the reads after the loop, you could reduce the time spent holding the lock. I don't…
jrtc27AuthorUnsubmitted
Done
Inline Actions

That's true, though I'd then need [io]{set,clear} variables, and if this driver's lock is contended then I have serious questions... :)

jrtc27: That's true, though I'd then need [io]{set,clear} variables, and if this driver's lock is…
jrtc27AuthorUnsubmitted
Done
Inline Actions

Actually no, the sc->gpio_pins[i].gp_flags all need protecting by the lock too, so reordering the loop with the I/O reads doesn't let you delay taking the lock

jrtc27: Actually no, the sc->gpio_pins[i].gp_flags all need protecting by the lock too, so reordering…
if (pin_flags[i] & GPIO_PIN_INPUT) {
ireg |= (1u << i);
oreg &= ~(1u << i);
sc->gpio_pins[i].gp_flags |= GPIO_PIN_INPUT;
sc->gpio_pins[i].gp_flags &= ~GPIO_PIN_OUTPUT;
} else if (pin_flags[i] & GPIO_PIN_OUTPUT) {
ireg &= ~(1u << i);
oreg |= (1u << i);
sc->gpio_pins[i].gp_flags &= ~GPIO_PIN_INPUT;
sc->gpio_pins[i].gp_flags |= GPIO_PIN_OUTPUT;
}
}
SFGPIO_WRITE(sc, SFGPIO_INPUT_EN, ireg);
SFGPIO_WRITE(sc, SFGPIO_OUTPUT_EN, oreg);
SFGPIO_UNLOCK(sc);
return (0);
}
static phandle_t
sfgpio_get_node(device_t bus, device_t dev)
{
return (ofw_bus_get_node(bus));
}
static device_method_t sfgpio_methods[] = {
/* Device interface */
DEVMETHOD(device_probe, sfgpio_probe),
DEVMETHOD(device_attach, sfgpio_attach),
/* GPIO protocol */
DEVMETHOD(gpio_get_bus, sfgpio_get_bus),
DEVMETHOD(gpio_pin_max, sfgpio_pin_max),
DEVMETHOD(gpio_pin_set, sfgpio_pin_set),
DEVMETHOD(gpio_pin_get, sfgpio_pin_get),
DEVMETHOD(gpio_pin_toggle, sfgpio_pin_toggle),
DEVMETHOD(gpio_pin_getcaps, sfgpio_pin_getcaps),
DEVMETHOD(gpio_pin_getflags, sfgpio_pin_getflags),
DEVMETHOD(gpio_pin_getname, sfgpio_pin_getname),
DEVMETHOD(gpio_pin_setflags, sfgpio_pin_setflags),
DEVMETHOD(gpio_pin_access_32, sfgpio_pin_access_32),
DEVMETHOD(gpio_pin_config_32, sfgpio_pin_config_32),
/* ofw_bus interface */
DEVMETHOD(ofw_bus_get_node, sfgpio_get_node),
DEVMETHOD_END
};
static devclass_t sfgpio_devclass;
DEFINE_CLASS_0(gpio, sfgpio_driver, sfgpio_methods,
sizeof(struct sfgpio_softc));
EARLY_DRIVER_MODULE(gpio, simplebus, sfgpio_driver, sfgpio_devclass, 0, 0,
BUS_PASS_INTERRUPT + BUS_PASS_ORDER_LATE);
MODULE_DEPEND(sfgpio, gpiobus, 1, 1, 1);
View Options

sys/netinet/tcp_subr.c

Loading...
View Options

sys/netinet/tcp_syncache.c

Loading...
View Options

sys/netinet/tcp_usrreq.c

Loading...
View Options

sys/netinet/tcp_var.h

Loading...