Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Sep 2020 21:30:48 +0000 (UTC)
From:      Justin Hibbits <chmeeedalf@gmail.com>
To:        Marcin Wojtas <mw@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r365054 - in head/sys: conf dev/sdhci
Message-ID:  <b41cdaec-c572-4ff1-94c9-7b399dec9fd0@gmail.com>
In-Reply-To: <202009011617.081GHL8e031671@repo.freebsd.org>
References:  <202009011617.081GHL8e031671@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Sep 1, 2020 11:17:35 Marcin Wojtas <mw@FreeBSD.org>:

> Author: mw
> Date: Tue Sep=C2=A0 1 16:17:21 2020
> New Revision: 365054
> URL: https://svnweb.freebsd.org/changeset/base/365054
>
> Log:
> Introduce the SDHCI driver for NXP QorIQ Layerscape SoCs
>
> Implement support for an eSDHC controller found in NXP QorIQ Layerscape S=
oCs.
>
> This driver has been tested with NXP LS1046A and LX2160A (Honeycomb board=
),
> which is incompatible with the existing sdhci_fsl driver (aiming at older
> chips from this family). As such, it is not intended as replacement for
> the old driver, but rather serves as an improved alternative for SoCs tha=
t
> support it.
> It comes with support for both PIO and Single DMA modes and samples the
> clock from the extres clk API.

How is it incompatible?

>
> Submitted by: Artur Rojek <ar@semihalf.com>
> Reviewed by: manu, mmel, kibab
> Obtained from: Semihalf
> Sponsored by: Alstom Group
> Differential Revision: https://reviews.freebsd.org/D26153
>
> Added:
> head/sys/dev/sdhci/sdhci_fsl_fdt.c=C2=A0=C2=A0 (contents, props changed)

The name choice here is odd, given there is already fsl_sdhci.c

> Modified:
> head/sys/conf/files
>
> Modified: head/sys/conf/files
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/conf/files Tue Sep=C2=A0 1 16:13:09 2020=C2=A0 (r365053)
> +++ head/sys/conf/files Tue Sep=C2=A0 1 16:17:21 2020=C2=A0 (r365054)
> @@ -3058,6 +3058,7 @@ dev/scc/scc_dev_z8530.c=C2=A0=C2=A0 optional scc
> dev/sdhci/sdhci.c=C2=A0=C2=A0=C2=A0 optional sdhci
> dev/sdhci/sdhci_fdt.c=C2=A0=C2=A0=C2=A0 optional sdhci fdt
> dev/sdhci/sdhci_fdt_gpio.c optional sdhci fdt gpio
> +dev/sdhci/sdhci_fsl_fdt.c=C2=A0 optional sdhci fdt gpio
> dev/sdhci/sdhci_if.m=C2=A0=C2=A0 optional sdhci
> dev/sdhci/sdhci_acpi.c=C2=A0=C2=A0 optional sdhci acpi
> dev/sdhci/sdhci_pci.c=C2=A0=C2=A0=C2=A0 optional sdhci pci
>
> Added: head/sys/dev/sdhci/sdhci_fsl_fdt.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- /dev/null 00:00:00 1970 (empty, because file is newly added)
> +++ head/sys/dev/sdhci/sdhci_fsl_fdt.c=C2=A0 Tue Sep=C2=A0 1 16:17:21 202=
0=C2=A0 (r365054)
> @@ -0,0 +1,680 @@
> +/*-
> + * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
> + *
> + * Copyright (c) 2020 Alstom Group.
> + * Copyright (c) 2020 Semihalf.
> + *
> + * 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
> + *=C2=A0=C2=A0=C2=A0 notice, this list of conditions and the following d=
isclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *=C2=A0=C2=A0=C2=A0 notice, this list of conditions and the following d=
isclaimer in the
> + *=C2=A0=C2=A0=C2=A0 documentation and/or other materials provided with =
the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AN=
D
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PU=
RPOSE
> + * ARE DISCLAIMED.=C2=A0 IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE=
 LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUE=
NTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOO=
DS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, S=
TRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY=
 WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY O=
F
> + * SUCH DAMAGE.
> + */
> +
> +/* eSDHC controller driver for NXP QorIQ Layerscape SoCs. */
> +
> +#include <sys/cdefs.h>
> +__FBSDID("$FreeBSD$");
> +
> +#include <sys/param.h>
> +#include <sys/endian.h>
> +#include <sys/kernel.h>
> +#include <sys/module.h>
> +#include <sys/rman.h>
> +#include <sys/sysctl.h>
> +#include <sys/taskqueue.h>
> +
> +#include <machine/bus.h>
> +#include <machine/resource.h>
> +
> +#include <dev/extres/clk/clk.h>
> +#include <dev/mmc/bridge.h>
> +#include <dev/mmc/mmcbrvar.h>
> +#include <dev/ofw/ofw_bus.h>
> +#include <dev/ofw/ofw_bus_subr.h>
> +#include <dev/sdhci/sdhci.h>
> +#include <dev/sdhci/sdhci_fdt_gpio.h>
> +
> +#include "mmcbr_if.h"
> +#include "sdhci_if.h"
> +
> +#define=C2=A0 RD4 (sc->read)
> +#define=C2=A0 WR4 (sc->write)
> +
> +#define=C2=A0 SDHCI_FSL_PRES_STATE=C2=A0=C2=A0=C2=A0 0x24
> +#define=C2=A0 SDHCI_FSL_PRES_SDSTB=C2=A0=C2=A0=C2=A0 (1 << 3)
> +#define=C2=A0 SDHCI_FSL_PRES_COMPAT_MASK=C2=A0 0x000f0f07
> +
> +#define=C2=A0 SDHCI_FSL_PROT_CTRL=C2=A0=C2=A0 0x28
> +#define=C2=A0 SDHCI_FSL_PROT_CTRL_WIDTH_1BIT=C2=A0 (0 << 1)
> +#define=C2=A0 SDHCI_FSL_PROT_CTRL_WIDTH_4BIT=C2=A0 (1 << 1)
> +#define=C2=A0 SDHCI_FSL_PROT_CTRL_WIDTH_8BIT=C2=A0 (2 << 1)
> +#define=C2=A0 SDHCI_FSL_PROT_CTRL_WIDTH_MASK=C2=A0 (3 << 1)
> +#define=C2=A0 SDHCI_FSL_PROT_CTRL_BYTE_SWAP (0 << 4)
> +#define=C2=A0 SDHCI_FSL_PROT_CTRL_BYTE_NATIVE (2 << 4)
> +#define=C2=A0 SDHCI_FSL_PROT_CTRL_BYTE_MASK (3 << 4)
> +#define=C2=A0 SDHCI_FSL_PROT_CTRL_DMA_MASK=C2=A0 (3 << 8)
> +
> +#define=C2=A0 SDHCI_FSL_SYS_CTRL=C2=A0=C2=A0=C2=A0 0x2c
> +#define=C2=A0 SDHCI_FSL_CLK_IPGEN=C2=A0=C2=A0 (1 << 0)
> +#define=C2=A0 SDHCI_FSL_CLK_SDCLKEN=C2=A0=C2=A0 (1 << 3)
> +#define=C2=A0 SDHCI_FSL_CLK_DIVIDER_MASK=C2=A0 0x000000f0
> +#define=C2=A0 SDHCI_FSL_CLK_DIVIDER_SHIFT 4
> +#define=C2=A0 SDHCI_FSL_CLK_PRESCALE_MASK 0x0000ff00
> +#define=C2=A0 SDHCI_FSL_CLK_PRESCALE_SHIFT=C2=A0 8
> +
> +#define=C2=A0 SDHCI_FSL_WTMK_LVL=C2=A0=C2=A0=C2=A0 0x44
> +#define=C2=A0 SDHCI_FSL_WTMK_RD_512B=C2=A0=C2=A0=C2=A0 (0 << 0)
> +#define=C2=A0 SDHCI_FSL_WTMK_WR_512B=C2=A0=C2=A0=C2=A0 (0 << 15)
> +
> +#define=C2=A0 SDHCI_FSL_HOST_VERSION=C2=A0=C2=A0=C2=A0 0xfc
> +#define=C2=A0 SDHCI_FSL_CAPABILITIES2=C2=A0=C2=A0 0x114
> +
> +#define=C2=A0 SDHCI_FSL_ESDHC_CTRL=C2=A0=C2=A0=C2=A0 0x40c
> +#define=C2=A0 SDHCI_FSL_ESDHC_CTRL_SNOOP=C2=A0 (1 << 6)
> +#define=C2=A0 SDHCI_FSL_ESDHC_CTRL_CLK_DIV2 (1 << 19)
> +
> +struct sdhci_fsl_fdt_softc {
> + device_t=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 dev;
> + const struct sdhci_fsl_fdt_soc_data *soc_data;
> + struct resource=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *mem_res;
> + struct resource=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *irq_res;
> + void=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *irq_cookie;
> + uint32_t=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 baseclk_hz;
> + struct sdhci_fdt_gpio=C2=A0=C2=A0=C2=A0=C2=A0 *gpio;
> + struct sdhci_slot=C2=A0=C2=A0=C2=A0=C2=A0 slot;
> + bool=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 slot_init_do=
ne;
> + uint32_t=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 cmd_and_mode;
> + uint16_t=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 sdclk_bits;
> +
> + uint32_t (* read)(struct sdhci_fsl_fdt_softc *, bus_size_t);
> + void (* write)(struct sdhci_fsl_fdt_softc *, bus_size_t, uint32_t);
> +};
> +
> +struct sdhci_fsl_fdt_soc_data {
> + int quirks;
> +};
> +
> +static const struct sdhci_fsl_fdt_soc_data sdhci_fsl_fdt_ls1046a_soc_dat=
a =3D {
> + .quirks =3D SDHCI_QUIRK_DONT_SET_HISPD_BIT | SDHCI_QUIRK_BROKEN_AUTO_ST=
OP
> +};
> +
> +static const struct sdhci_fsl_fdt_soc_data sdhci_fsl_fdt_gen_data =3D {
> + .quirks =3D 0,
> +};
> +
> +static const struct ofw_compat_data sdhci_fsl_fdt_compat_data[] =3D {
> + {"fsl,ls1046a-esdhc", (uintptr_t)&sdhci_fsl_fdt_ls1046a_soc_data},
> + {"fsl,esdhc",=C2=A0=C2=A0 (uintptr_t)&sdhci_fsl_fdt_gen_data},
> + {NULL,=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 0}
> +};

The existing driver is compatible with fsl,esdhc.=C2=A0 How are you prevent=
ing collisions?

Why couldn't you make these changes to the existing driver?=C2=A0 I see som=
e improvements in this one that would be nice to have in the other driver.

- Justin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?b41cdaec-c572-4ff1-94c9-7b399dec9fd0>