Date: Fri, 18 Aug 2023 19:55:03 +0200 From: Dmitry Salychev <dsl@FreeBSD.org> To: Jessica Clarke <jrtc27@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: <86a5uo5j6b.fsf@peasant.tower.home> In-Reply-To: <00BDB172-6E09-4000-B59F-843DA4F723F7@freebsd.org> References: <202308181040.37IAetMR060855@gitrepo.freebsd.org> <7DD26452-7D0C-45BA-AA01-825B1073FEC6@freebsd.org> <86h6ow5k9y.fsf@peasant.tower.home> <00BDB172-6E09-4000-B59F-843DA4F723F7@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Jessica Clarke <jrtc27@freebsd.org> writes: > On 18 Aug 2023, at 18:18, Dmitry Salychev <dsl@FreeBSD.org> wrote: >>=20 >>=20 >> Jessica Clarke <jrtc27@freebsd.org> writes: >>=20 >>> 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=3D2a9021898c4ee2154787da8= 62c238cfeccd655df >>>>=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 I= 2C >>>> 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 distribut= ion. >>>> +# >>>> +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' = AND >>>> +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, T= HE >>>> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR = PURPOSE >>>> +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LI= ABLE >>>> +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQ= UENTIAL >>>> +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE G= OODS >>>> +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTIO= N) >>>> +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,= STRICT >>>> +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN A= NY 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 copyrig= ht >>>> + * notice, this list of conditions and the following disclaimer in= the >>>> + * documentation and/or other materials provided with the distribu= tion. >>>> + * >>>> + * 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 L= IABLE >>>> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSE= QUENTIAL >>>> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE = GOODS >>>> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTI= ON) >>>> + * 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 POSSIBILIT= Y 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; >>>=20 >>> These names sound like they should be numbers, not handles to GPIOs. >>>=20 >>=20 >> FDT example: >>=20 >> sfp_xg1: dpmac1-sfp { >> compatible =3D "sff,sfp"; >> i2c-bus =3D <&sfpupper_i2c>; >> tx-fault-gpios =3D <&sfpgpio 4 GPIO_ACTIVE_HIGH>; >> tx-disable-gpios =3D <&sfpgpio 5 GPIO_ACTIVE_HIGH>; >> mod-def0-gpios =3D <&sfpgpio 6 GPIO_ACTIVE_LOW>; >> los-gpios =3D <&sfpgpio 7 GPIO_ACTIVE_HIGH>; >> maximum-power-milliwatt =3D <2000>; >> }; > > Yes, exactly. =E2=80=9Ctx_rate=E2=80=9D sounds like it=E2=80=99s some kin= d of bits-per-second > measure, for example, that would be a uint32_t (like max_power), but is > in fact a property called =E2=80=9Crate-select1-gpios=E2=80=9D that=E2=80= =99s a handle. That > is, the softc member names are very confusing. > > Jess OK, understood. I'll update names in the next patch as well. Regards, Dmitry --=20 https://wiki.freebsd.org/DmitrySalychev
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86a5uo5j6b.fsf>