Re: git: 2a9021898c4e - main - sff: Add SFP driver (fdt-based draft)

Reply: Dmitry Salychev : "Re: git: 2a9021898c4e - main - sff: Add SFP driver (fdt-based draft)"
In reply to: Dmitry Salychev : "Re: git: 2a9021898c4e - main - sff: Add SFP driver (fdt-based draft)"
Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Jessica Clarke <jrtc27_at_freebsd.org>
Date: Fri, 18 Aug 2023 17:35:47 UTC
On 18 Aug 2023, at 18:18, Dmitry Salychev <dsl@FreeBSD.org> wrote:
> 
> 
> Jessica Clarke <jrtc27@freebsd.org> writes:
> 
>> On 18 Aug 2023, at 11:40, Dmitry Salychev <dsl@FreeBSD.org> wrote:
>>> 
>>> The branch main has been updated by dsl:
>>> 
>>> URL: https://cgit.FreeBSD.org/src/commit/?id=2a9021898c4ee2154787da862c238cfeccd655df
>>> 
>>> 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
>>> 
>>>   sff: Add SFP driver (fdt-based draft)
>>> 
>>>   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).
>>> 
>>>   Both of the interface and driver are subjects for a further
>>>   generalization to be used in case of non-FDT and non-arm64 platforms.
>>> 
>>>   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(+)
>>> 
>>> 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
>>> 
>>> +# SFF/SFP
>>> +device sff # Small Form Factor Transceivers
>>> +
>>> options FDT
>>> device acpi
>>> 
>>> 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 © 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 © 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.
>> 
> 
> FDT example:
> 
> sfp_xg1: dpmac1-sfp {
>        compatible = "sff,sfp";
>        i2c-bus = <&sfpupper_i2c>;
>        tx-fault-gpios = <&sfpgpio 4 GPIO_ACTIVE_HIGH>;
>        tx-disable-gpios = <&sfpgpio 5 GPIO_ACTIVE_HIGH>;
>        mod-def0-gpios = <&sfpgpio 6 GPIO_ACTIVE_LOW>;
>        los-gpios = <&sfpgpio 7 GPIO_ACTIVE_HIGH>;
>        maximum-power-milliwatt = <2000>;
> };

Yes, exactly. “tx_rate” sounds like it’s some kind of bits-per-second
measure, for example, that would be a uint32_t (like max_power), but is
in fact a property called “rate-select1-gpios” that’s a handle. That
is, the softc member names are very confusing.

Jess