| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGRGNgVONS+i=57D52VeW8J6SvfmX=iG+3VpxNS8=kJOV+WSLw@mail.gmail.com>
Date: Tue, 14 Jun 2016 09:43:56 +1000
From: Julian Calaby <julian.calaby@...il.com>
To: Michal Suchanek <hramrach@...il.com>
Cc: linux-sunxi <linux-sunxi@...glegroups.com>,
Rob Herring <robh+dt@...nel.org>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Maxime Ripard <maxime.ripard@...e-electrons.com>,
Chen-Yu Tsai <wens@...e.org>,
Russell King <linux@...linux.org.uk>,
Mark Brown <broonie@...nel.org>, Arnd Bergmann <arnd@...db.de>,
Olof Johansson <olof@...om.net>,
Krzysztof Kozlowski <k.kozlowski@...sung.com>,
Javier Martinez Canillas <javier@....samsung.com>,
Simon Horman <horms+renesas@...ge.net.au>,
Sjoerd Simons <sjoerd.simons@...labora.co.uk>,
Thierry Reding <treding@...dia.com>,
Alison Wang <b18965@...escale.com>,
Timo Sigurdsson <public_timo.s@...entcreek.de>,
Jonathan Liu <net147@...il.com>,
Gerhard Bertelsmann <info@...hard-bertelsmann.de>,
Priit Laes <plaes@...es.org>,
devicetree <devicetree@...r.kernel.org>,
"Mailing List, Arm" <linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux-spi <linux-spi@...r.kernel.org>
Subject: Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i
SPI driver
Hi Michal,
On Tue, Jun 14, 2016 at 3:46 AM, Michal Suchanek <hramrach@...il.com> wrote:
> The drivers are very similar and share multiple flaws which needed
> separate fixes for both drivers.
>
> Signed-off-by: Michal Suchanek <hramrach@...il.com>
> ---
> drivers/spi/Kconfig | 8 +-
> drivers/spi/Makefile | 1 -
> drivers/spi/spi-sun4i.c | 156 +++++++++++--
> drivers/spi/spi-sun6i.c | 598 ------------------------------------------------
> 4 files changed, 143 insertions(+), 620 deletions(-)
> delete mode 100644 drivers/spi/spi-sun6i.c
>
> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 0b8e6c6..c76f8e4 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master,
> reg = sunxi_spi_read(sspi, SUNXI_TFR_CTL_REG);
>
> /* Reset FIFOs */
> - sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> - reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> - sspi_bits(sspi, SUNXI_CTL_TF_RST));
> + if (sspi->type == SPI_SUN4I)
> + sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> + reg | sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> + sspi_bits(sspi, SUNXI_CTL_TF_RST));
> + else
> + sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG,
> + sspi_bits(sspi, SUNXI_CTL_RF_RST) |
> + sspi_bits(sspi, SUNXI_CTL_TF_RST));
If we're already doing different stuff for each generation of the IP,
why not just use the register offsets and bit definitions directly?
>
> /*
> * Setup the transfer control register: Chip Select,
> @@ -427,7 +473,19 @@ static int sunxi_spi_runtime_resume(struct device *dev)
> goto err;
> }
>
> - sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
> + if (sspi->rstc) {
> + ret = reset_control_deassert(sspi->rstc);
> + if (ret) {
> + dev_err(dev, "Couldn't deassert the device from reset\n");
> + goto err2;
> + }
> + }
> +
> + if (sspi->type == SPI_SUN4I)
> + reg = SUNXI_TFR_CTL_REG;
> + else
> + reg = SUNXI_GBL_CTL_REG;
> + sunxi_spi_write(sspi, reg,
> sspi_bits(sspi, SUNXI_CTL_ENABLE) |
> sspi_bits(sspi, SUNXI_CTL_MASTER) |
> sspi_bits(sspi, SUNXI_CTL_TP));
Same here.
> @@ -491,10 +558,37 @@ static int sunxi_spi_probe(struct platform_device *pdev)
> }
>
> sspi->master = master;
> - sspi->fifo_depth = SUN4I_FIFO_DEPTH;
> - sspi->type = SPI_SUN4I;
> - sspi->regmap = &sun4i_regmap;
> - sspi->bitmap = &sun4i_bitmap;
> + if (of_device_is_compatible(pdev->dev.of_node, SUN4I_COMPATIBLE)) {
> + sspi->fifo_depth = SUN4I_FIFO_DEPTH;
> + sspi->type = SPI_SUN4I;
> + sspi->regmap = &sun4i_regmap;
> + sspi->bitmap = &sun4i_bitmap;
> + } else if (of_device_is_compatible(pdev->dev.of_node,
> + SUN6I_COMPATIBLE)) {
> + sspi->fifo_depth = SUN6I_FIFO_DEPTH;
> + sspi->type = SPI_SUN6I;
> + sspi->regmap = &sun6i_regmap;
> + sspi->bitmap = &sun6i_bitmap;
Can you store data in the match table instead of doing this?
> + } else {
> + const char *str = NULL;
> + int i = 1;
> +
> + of_property_read_string(pdev->dev.of_node, "compatible", &str);
> + dev_err(&pdev->dev, "Unknown device compatible %s", str);
> + /* is there no sane way to print a string array property ? */
> + if (of_property_count_strings(pdev->dev.of_node, "compatible")
> + > 1) {
> + while (!of_property_read_string_index(pdev->dev.of_node,
> + "compatible", i,
> + &str)) {
> + pr_err(", %s", str);
> + i++;
> + }
> + }
> + ret = -EINVAL;
> + goto err_free_master;
> + }
> +
> master->max_speed_hz = 100 * 1000 * 1000;
> master->min_speed_hz = 3 * 1000;
> master->set_cs = sunxi_spi_set_cs;
Thanks,
--
Julian Calaby
Email: julian.calaby@...il.com
Profile: http://www.google.com/profiles/julian.calaby/
Powered by blists - more mailing lists