Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 18 Aug 2023 17:09:19 +0100
From:      Jessica Clarke <jrtc27@freebsd.org>
To:        Dmitry Salychev <dsl@FreeBSD.org>
Cc:        "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org>
Subject:   Re: git: 2a9021898c4e - main - sff: Add SFP driver (fdt-based draft)
Message-ID:  <7DD26452-7D0C-45BA-AA01-825B1073FEC6@freebsd.org>
In-Reply-To: <202308181040.37IAetMR060855@gitrepo.freebsd.org>
References:  <202308181040.37IAetMR060855@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 18 Aug 2023, at 11:40, Dmitry Salychev <dsl@FreeBSD.org> wrote:
>=20
> The branch main has been updated by dsl:
>=20
> URL: =
https://cgit.FreeBSD.org/src/commit/?id=3D2a9021898c4ee2154787da862c238cfe=
ccd655df
>=20
> commit 2a9021898c4ee2154787da862c238cfeccd655df
> Author:     Dmitry Salychev <dsl@FreeBSD.org>
> AuthorDate: 2023-08-18 09:17:31 +0000
> Commit:     Dmitry Salychev <dsl@FreeBSD.org>
> CommitDate: 2023-08-18 10:40:11 +0000
>=20
>    sff: Add SFP driver (fdt-based draft)
>=20
>    This basic version of the driver obtains properties of the =
"sff,sfp"
>    compatible devices and implements a simple interface to provide an =
I2C
>    bus device for the rest of the drivers (e.g. to implement =
SIOCGI2C).
>=20
>    Both of the interface and driver are subjects for a further
>    generalization to be used in case of non-FDT and non-arm64 =
platforms.
>=20
>    Reviewed by:            bz, manu
>    Approved by:            bz (mentor)
>    MFC after:              3 weeks
>    Differential Revision:  https://reviews.freebsd.org/D41440
> ---
> sys/arm64/conf/std.nxp   |   3 +
> sys/conf/files           |   2 +
> sys/dev/sff/sff_if.m     |  35 +++++++++++
> sys/dev/sff/sfp_fdt.c    | 155 =
+++++++++++++++++++++++++++++++++++++++++++++++
> sys/modules/Makefile     |   2 +
> sys/modules/sff/Makefile |  13 ++++
> 6 files changed, 210 insertions(+)
>=20
> diff --git a/sys/arm64/conf/std.nxp b/sys/arm64/conf/std.nxp
> index 5b2e2b52d4e6..b4552fadaff4 100644
> --- a/sys/arm64/conf/std.nxp
> +++ b/sys/arm64/conf/std.nxp
> @@ -25,6 +25,9 @@ device sdhci
> device dpaa2 # Data Path Acceleration Architecture (2nd Gen)
> device enetc # QorIQ LS1028A NIC
>=20
> +# SFF/SFP
> +device sff # Small Form Factor Transceivers
> +
> options FDT
> device acpi
>=20
> diff --git a/sys/conf/files b/sys/conf/files
> index 0db5887e6a75..b5cd85cba0e4 100644
> --- a/sys/conf/files
> +++ b/sys/conf/files
> @@ -3044,6 +3044,8 @@ dev/sdhci/sdhci_pci.c optional sdhci pci
> dev/sdio/sdio_if.m optional mmccam
> dev/sdio/sdio_subr.c optional mmccam
> dev/sdio/sdiob.c optional mmccam
> +dev/sff/sff_if.m optional sff
> +dev/sff/sfp_fdt.c optional sff fdt
> dev/sge/if_sge.c optional sge pci
> dev/siis/siis.c optional siis pci
> dev/sis/if_sis.c optional sis pci
> diff --git a/sys/dev/sff/sff_if.m b/sys/dev/sff/sff_if.m
> new file mode 100644
> index 000000000000..823e557992c2
> --- /dev/null
> +++ b/sys/dev/sff/sff_if.m
> @@ -0,0 +1,35 @@
> +#-
> +# SPDX-License-Identifier: BSD-2-Clause
> +#
> +# Copyright =C2=A9 2023 Dmitry Salychev
> +#
> +# 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.
> +#
> +
> +#include <machine/bus.h>
> +
> +INTERFACE sff;
> +
> +METHOD int get_i2c_bus {
> + device_t dev;
> + device_t *i2c_bus;
> +};
> diff --git a/sys/dev/sff/sfp_fdt.c b/sys/dev/sff/sfp_fdt.c
> new file mode 100644
> index 000000000000..7430282ede70
> --- /dev/null
> +++ b/sys/dev/sff/sfp_fdt.c
> @@ -0,0 +1,155 @@
> +/*-
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright =C2=A9 2023 Dmitry Salychev
> + *
> + * 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.
> + */
> +
> +/*
> + * Small Form Factor (SFF) Committee Pluggable (SFP) Transceiver =
(FDT-based).
> + */
> +
> +#include <sys/param.h>
> +#include <sys/kernel.h>
> +#include <sys/bus.h>
> +#include <sys/module.h>
> +
> +#include <dev/ofw/ofw_bus.h>
> +#include <dev/ofw/ofw_bus_subr.h>
> +#include <dev/fdt/simplebus.h>
> +
> +#include "sff_if.h"
> +
> +struct sfp_fdt_softc {
> + phandle_t ofw_node;
> + phandle_t i2c_bus;
> +
> + phandle_t mod_def;
> + phandle_t los;
> + phandle_t tx_fault;
> + phandle_t tx_disable;
> + phandle_t rx_rate;
> + phandle_t tx_rate;

These names sound like they should be numbers, not handles to GPIOs.

> + uint32_t max_power; /* in mW */
> +};
> +
> +static int
> +sfp_fdt_probe(device_t dev)
> +{
> + phandle_t node;
> + ssize_t s;
> +
> + node =3D ofw_bus_get_node(dev);
> + if (!ofw_bus_node_is_compatible(node, "sff,sfp"))
> + return (ENXIO);
> +
> + s =3D device_get_property(dev, "i2c-bus", &node, sizeof(node),
> +    DEVICE_PROP_HANDLE);
> + if (s =3D=3D -1) {
> + device_printf(dev, "%s: '%s' has no 'i2c-bus' property, s %zd\n",
> +    __func__, ofw_bus_get_name(dev), s);

What=E2=80=99s the point of printing s when it=E2=80=99s known to be -1?

> + return (ENXIO);
> + }
> +
> + device_set_desc(dev, "Small Form-factor Pluggable Transceiver");
> + return (BUS_PROBE_DEFAULT);
> +}
> +
> +static int
> +sfp_fdt_attach(device_t dev)
> +{
> + struct sfp_fdt_softc *sc;
> + ssize_t s;
> + int error;
> +
> + sc =3D device_get_softc(dev);
> + sc->ofw_node =3D ofw_bus_get_node(dev);
> +
> + s =3D device_get_property(dev, "i2c-bus", &sc->i2c_bus,
> +    sizeof(sc->i2c_bus), DEVICE_PROP_HANDLE);
> + if (s =3D=3D -1) {
> + device_printf(dev, "%s: cannot find 'i2c-bus' property: %zd\n",
> +    __func__, s);

Ditto.

> + return (ENXIO);
> + }
> +
> + /* Optional properties */
> + (void)device_get_property(dev, "mod-def0-gpios", &sc->mod_def,
> +    sizeof(sc->mod_def), DEVICE_PROP_HANDLE);
> + (void)device_get_property(dev, "los-gpios", &sc->los, =
sizeof(sc->los),
> +    DEVICE_PROP_HANDLE);
> + (void)device_get_property(dev, "tx-fault-gpios", &sc->tx_fault,
> +    sizeof(sc->tx_fault), DEVICE_PROP_HANDLE);
> + (void)device_get_property(dev, "tx-disable-gpios", &sc->tx_disable,
> +    sizeof(sc->tx_disable), DEVICE_PROP_HANDLE);
> + (void)device_get_property(dev, "rate-select0-gpios", &sc->rx_rate,
> +    sizeof(sc->rx_rate), DEVICE_PROP_HANDLE);
> + (void)device_get_property(dev, "rate-select1-gpios", &sc->tx_rate,
> +    sizeof(sc->tx_rate), DEVICE_PROP_HANDLE);
> + (void)device_get_property(dev, "maximum-power-milliwatt", =
&sc->max_power,
> +    sizeof(sc->max_power), DEVICE_PROP_UINT32);
> +
> + error =3D OF_device_register_xref(OF_xref_from_node(sc->ofw_node), =
dev);
> + if (error !=3D 0)
> + device_printf(dev, "%s: failed to register xref %#x\n",
> +    __func__, OF_xref_from_node(sc->ofw_node));

: %d, error?

Jess

> +
> + return (error);
> +}
> +
> +static int
> +sfp_fdt_get_i2c_bus(device_t dev, device_t *i2c_bus)
> +{
> + struct sfp_fdt_softc *sc;
> + device_t xdev;
> +
> + KASSERT(i2c_bus !=3D NULL, ("%s: i2c_bus is NULL", __func__));
> +
> + sc =3D device_get_softc(dev);
> + xdev =3D OF_device_from_xref(OF_xref_from_node(sc->i2c_bus));
> + if (xdev =3D=3D NULL)
> + return (ENXIO);
> +
> + *i2c_bus =3D xdev;
> + return (0);
> +}
> +
> +static device_method_t sfp_fdt_methods[] =3D {
> + /* Device interface */
> + DEVMETHOD(device_probe, sfp_fdt_probe),
> + DEVMETHOD(device_attach, sfp_fdt_attach),
> + DEVMETHOD(device_detach, bus_generic_detach),
> +
> + /* SFF */
> + DEVMETHOD(sff_get_i2c_bus, sfp_fdt_get_i2c_bus),
> +
> + DEVMETHOD_END
> +};
> +
> +DEFINE_CLASS_0(sfp_fdt, sfp_fdt_driver, sfp_fdt_methods,
> +    sizeof(struct sfp_fdt_softc));
> +
> +EARLY_DRIVER_MODULE(sfp_fdt, simplebus, sfp_fdt_driver, 0, 0,
> +    BUS_PASS_SUPPORTDEV);
> +EARLY_DRIVER_MODULE(sfp_fdt, ofwbus, sfp_fdt_driver, 0, 0,
> +    BUS_PASS_SUPPORTDEV);
> diff --git a/sys/modules/Makefile b/sys/modules/Makefile
> index 201cfbcca725..4b98c7ed6e0d 100644
> --- a/sys/modules/Makefile
> +++ b/sys/modules/Makefile
> @@ -355,6 +355,7 @@ SUBDIR=3D \
> ${_sdhci_fdt} \
> sdhci_pci \
> sdio \
> + ${_sff} \
> sem \
> send \
> ${_sfxge} \
> @@ -678,6 +679,7 @@ _cxgb=3D cxgb
> .if ${MACHINE_CPUARCH} =3D=3D "aarch64"
> _armv8crypto=3D armv8crypto
> _dpaa2=3D dpaa2
> +_sff=3D sff
> _em=3D em
> _hyperv=3D  hyperv
>=20
> diff --git a/sys/modules/sff/Makefile b/sys/modules/sff/Makefile
> new file mode 100644
> index 000000000000..96832070de63
> --- /dev/null
> +++ b/sys/modules/sff/Makefile
> @@ -0,0 +1,13 @@
> +.PATH: ${SRCTOP}/sys/dev/sff
> +
> +KMOD=3D sff
> +
> +SRCS+=3D sff_if.c sff_if.h
> +SRCS+=3D bus_if.h device_if.h
> +
> +.if !empty(OPT_FDT)
> +SRCS+=3D sfp_fdt.c \
> + ofw_bus_if.h
> +.endif
> +
> +.include <bsd.kmod.mk>




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?7DD26452-7D0C-45BA-AA01-825B1073FEC6>