From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ACF44C433E0 for ; Fri, 3 Jul 2020 09:15:36 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6596920836 for ; Fri, 3 Jul 2020 09:15:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="pltorHcv"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=microchip.com header.i=@microchip.com header.b="ACO9cyK0" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6596920836 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=microchip.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:In-Reply-To:Subject:To: From:References:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=eOGMbaitD5jIfs8YSkOBLelK5onqtF4qPkDwyW16LAY=; b=pltorHcv8/0A/AfFVE1q1+AMo IgYZcn0F2A5Y8gOGf85b8fsrwpqHLYDDYTYWA4VfvA3bHZH9Ypkwj4h/IoQrpvbmCpNI2i+yAJ/9I e5nZVOFe3vl9VnE5ICxFXqSRTc8vZW+SajY3vIF1zJwCeaAhkk9NwWYDS+of/bmeY6l2WYJ3AM3ZC n/tZ5hALNWEW4Mcqd7o12Qy37OuQN9ahCgK/E/CHP740WzF+yfmBEoyNMpqFkALGLx83PhAkFkAQd FynOr+YmI1lDxCqW3lTPYNCEee54pBOjnRMBvNHehz8l8ioEZfBQB1mTFYWbgb3ZPdaRCKseTk7g3 jnTZhLymw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jrHlr-0005PS-H0; Fri, 03 Jul 2020 09:14:11 +0000 Received: from esa6.microchip.iphmx.com ([216.71.154.253]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jrHlo-0005On-Cq for linux-arm-kernel@lists.infradead.org; Fri, 03 Jul 2020 09:14:09 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1593767648; x=1625303648; h=references:from:to:cc:subject:in-reply-to:date: message-id:mime-version; bh=fBtSKRP9N2Fgt7VYWXnH4CSjKQi6GEbOhmh3zxMw3Ag=; b=ACO9cyK0hGH64H80IA7D1AMI+/reVOYLiGvdsOX/c1fFH1UEmjNchgBu t/631GEnIkLs/sF1nNc7Q8JQktLaflKtQcqovvpZUTF9sJSMleTtiHrhM qVosL37pLws4wJXSUH5Bq0jvHDtNXsn4v7xHk/fdJj72s6oZvP+uV6oL9 C/x6Sk1T0Qk+UFwhcWuN2GoOECXTOx4k2iARSgtlqBOuXjlVoD3mpmijk TsWOmW8YZdXoAV/S0Qg2+2dkKvSMM3mnLhK6TbK1RwCL6A7RjJI0KorSQ NT/xKmr2s68WpchMVkFY3axl5OUw8QBKUYE5ReXbu0aANGpFI529uftRK w==; IronPort-SDR: Wded69KT4fpELpHp57F+Q/ZYnZ6icWWDlTQ9+rBq4isydhxOMnrvMe8O5XChU7DdAm7PKuINHs zBtQKzHKa9bA/Kx7BnOcSDP+wIv2OrxKna0LH4Rw0Hj8uTLKtWgd5875lpXIrr4M8d/0gHro1H 5QAUcw01oGjDssIPTB7ouwWVWXwItObZwixZ+tBTVzwNIBeOHGRRHTmKBhqXQHX5vVHOQcea7E hv9ycdwgvUNcrWefoNMf6GvYM1qN12n9gSaNsGMpur2RX6dOBMBPub+j7RMPrCEHl6Jb3JYqLe Jco= X-IronPort-AV: E=Sophos;i="5.75,307,1589266800"; d="scan'208";a="17936686" Received: from smtpout.microchip.com (HELO email.microchip.com) ([198.175.253.82]) by esa6.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 03 Jul 2020 02:14:06 -0700 Received: from chn-vm-ex01.mchp-main.com (10.10.85.143) by chn-vm-ex02.mchp-main.com (10.10.85.144) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1979.3; Fri, 3 Jul 2020 02:13:43 -0700 Received: from soft-dev15.microsemi.net.microchip.com (10.10.115.15) by chn-vm-ex01.mchp-main.com (10.10.85.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1979.3 via Frontend Transport; Fri, 3 Jul 2020 02:13:41 -0700 References: <20200702101331.26375-1-lars.povlsen@microchip.com> <20200702101331.26375-5-lars.povlsen@microchip.com> <99d9c814-24e2-b3a3-bf0f-765ee51df558@axentia.se> From: Lars Povlsen To: Peter Rosin Subject: Re: [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver In-Reply-To: <99d9c814-24e2-b3a3-bf0f-765ee51df558@axentia.se> Date: Fri, 3 Jul 2020 11:14:00 +0200 Message-ID: <871rltklk7.fsf@soft-dev15.microsemi.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200703_051408_652898_706AAD28 X-CRM114-Status: GOOD ( 26.74 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Serge Semin , linux-spi@vger.kernel.org, Serge Semin , Mark Brown , linux-arm-kernel@lists.infradead.org, Microchip Linux Driver Support , Lars Povlsen Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Peter Rosin writes: > Hi! > > On 2020-07-02 12:13, Lars Povlsen wrote: >> The Sparx5 mux driver may be used to control selecting between two >> alternate SPI bus segments connected to the SPI controller >> (spi-dw-mmio). >> >> Signed-off-by: Lars Povlsen >> --- >> drivers/mux/Makefile | 2 + >> drivers/mux/sparx5-spi.c | 138 +++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 140 insertions(+) >> create mode 100644 drivers/mux/sparx5-spi.c >> >> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile >> index 6e9fa47daf566..18c3ae3582ece 100644 >> --- a/drivers/mux/Makefile >> +++ b/drivers/mux/Makefile >> @@ -8,9 +8,11 @@ mux-adg792a-objs := adg792a.o >> mux-adgs1408-objs := adgs1408.o >> mux-gpio-objs := gpio.o >> mux-mmio-objs := mmio.o >> +mux-sparx5-objs := sparx5-spi.o >> >> obj-$(CONFIG_MULTIPLEXER) += mux-core.o >> obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o >> obj-$(CONFIG_MUX_ADGS1408) += mux-adgs1408.o >> obj-$(CONFIG_MUX_GPIO) += mux-gpio.o >> obj-$(CONFIG_MUX_MMIO) += mux-mmio.o >> +obj-$(CONFIG_SPI_DW_MMIO) += mux-sparx5.o >> diff --git a/drivers/mux/sparx5-spi.c b/drivers/mux/sparx5-spi.c >> new file mode 100644 >> index 0000000000000..5fe9025b96a5e >> --- /dev/null >> +++ b/drivers/mux/sparx5-spi.c >> @@ -0,0 +1,138 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Sparx5 SPI MUX driver >> + * >> + * Copyright (c) 2019 Microsemi Corporation >> + * >> + * Author: Lars Povlsen >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define MSCC_IF_SI_OWNER_SISL 0 >> +#define MSCC_IF_SI_OWNER_SIBM 1 >> +#define MSCC_IF_SI_OWNER_SIMC 2 >> + >> +#define SPARX5_CPU_SYSTEM_CTRL_GENERAL_CTRL 0x88 >> +#define SPARX5_IF_SI_OWNER GENMASK(7, 6) >> +#define SPARX5_IF_SI2_OWNER GENMASK(5, 4) >> + >> +#define SPARX5_MAX_CS 16 >> + >> +struct mux_sparx5 { >> + struct regmap *syscon; >> + u8 bus[SPARX5_MAX_CS]; >> + int cur_bus; > > Surplus unused variable? > >> +}; >> + >> +/* >> + * Set the owner of the SPI interfaces >> + */ >> +static void mux_sparx5_set_owner(struct regmap *syscon, >> + u8 owner, u8 owner2) >> +{ >> + u32 val, msk; >> + >> + val = FIELD_PREP(SPARX5_IF_SI_OWNER, owner) | >> + FIELD_PREP(SPARX5_IF_SI2_OWNER, owner2); >> + msk = SPARX5_IF_SI_OWNER | SPARX5_IF_SI2_OWNER; >> + regmap_update_bits(syscon, >> + SPARX5_CPU_SYSTEM_CTRL_GENERAL_CTRL, >> + msk, val); >> +} >> + >> +static void mux_sparx5_set_cs_owner(struct mux_sparx5 *mux_sparx5, >> + u8 cs, u8 owner) >> +{ >> + u8 other = (owner == MSCC_IF_SI_OWNER_SIBM ? >> + MSCC_IF_SI_OWNER_SIMC : MSCC_IF_SI_OWNER_SIBM); > > Empty line missing here. > >> + if (mux_sparx5->bus[cs]) >> + /* SPI2 */ >> + mux_sparx5_set_owner(mux_sparx5->syscon, other, owner); >> + else >> + /* SPI1 */ >> + mux_sparx5_set_owner(mux_sparx5->syscon, owner, other); >> +} > > > This smells like there are only two states for this mux control, and that > the whole point of this driver is to make the exact numbering selectable. > I don't see the point of that. To me, it looks like the pre-existing > mmio-mux should be able to work. Something like this? Untested of course, > and I might easily have misunderstood something... > Peter, Good suggestion with "mmio-mux" - I overlooked it actually supports regmap. It makes DT configuration a little less intuitive, but removes the baggage of the dedicated sparx5 mux driver and bindings. It also pushes the solution towards using "spi-mux", but at least the CS in the DT does not have to be repeated. So a little more DT stuff, but less new code. I will try to implement the "mmio-mux"/"spi-mux" solution in a rev4. Any comments from Mark? Anyway, Peter, thank you for your comments. (Your driver comments are all valid, but it appears the driver isn't needed after all) Cheers, ---Lars > mux: mux-controller { > compatible = "mmio-mux" > #mux-control-cells = <1>; > > /* SI_OWNER and SI2_OWNER in GENERAL_CTRL */ > mux-reg-masks = <0x88 0xf0>; > }; > > > spi0: spi@600104000 { > compatible = "microchip,sparx5-spi"; > spi@0 { > compatible = "spi-mux"; > mux-controls = <&mux 0>; > reg = <0>; > /* SI_OWNER = SIMC, SI2_OWNER = SIBM ---> mux value 9 */ > spi-flash@9 { > compatible = "jedec,spi-nor"; > reg = <9>; > }; > }; > spi@e { > compatible = "spi-mux"; > mux-controls = <&mux 0>; > reg = <14>; > /* SI_OWNER = SIBM, SI2_OWNER = SIMC ---> mux value 6 */ > spi-flash@6 { > compatible = "spi-nand"; > reg = <6>; > }; > }; > }; > >> + >> +static int mux_sparx5_set(struct mux_control *mux, int state) >> +{ >> + struct mux_sparx5 *mux_sparx5 = mux_chip_priv(mux->chip); >> + >> + mux_sparx5_set_cs_owner(mux_sparx5, state, MSCC_IF_SI_OWNER_SIMC); >> + >> + return 0; >> +} >> + >> +static const struct mux_control_ops mux_sparx5_ops = { >> + .set = mux_sparx5_set, >> +}; >> + >> +static const struct of_device_id mux_sparx5_dt_ids[] = { >> + { .compatible = "microchip,sparx5-spi-mux", }, >> + { /* sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(of, mux_sparx5_dt_ids); >> + >> +static int mux_sparx5_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct mux_chip *mux_chip; >> + struct mux_sparx5 *mux_sparx5; >> + struct device_node *nc; >> + const char *syscon_name = "microchip,sparx5-cpu-syscon"; >> + int ret; >> + >> + mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_sparx5)); >> + if (IS_ERR(mux_chip)) >> + return PTR_ERR(mux_chip); >> + >> + mux_sparx5 = mux_chip_priv(mux_chip); >> + mux_chip->ops = &mux_sparx5_ops; >> + >> + mux_sparx5->syscon = >> + syscon_regmap_lookup_by_compatible(syscon_name); >> + if (IS_ERR(mux_sparx5->syscon)) { >> + dev_err(dev, "No syscon map %s\n", syscon_name); >> + return PTR_ERR(mux_sparx5->syscon); >> + } >> + >> + /* Get bus interface mapping */ >> + for_each_available_child_of_node(dev->of_node, nc) { >> + u32 cs, bus; >> + >> + if (of_property_read_u32(nc, "reg", &cs) == 0 && >> + cs < SPARX5_MAX_CS && >> + of_property_read_u32(nc, "microchip,bus-interface", >> + &bus) == 0) >> + mux_sparx5->bus[cs] = bus; > > The above if is a mess. The kernel model is to handle the exceptional cases > first and break/goto/continue/return/whatever so that the interesting code > can happen at lower indentation level. > > if (of_property_read_u32(nc, "reg", &cs)) > continue; > if (cs >= SPARX5_MAX_CS) > continue; > if (of_property_read_u32(nc, "microchip,bus-interface", &bus)) > continue; > > mux_sparc5->bus[cs] = bus; > > Cheers, > Peter > >> + } >> + >> + mux_chip->mux->states = SPARX5_MAX_CS; >> + >> + ret = devm_mux_chip_register(dev, mux_chip); >> + if (ret < 0) >> + return ret; >> + >> + dev_info(dev, "%u-way mux-controller registered\n", >> + mux_chip->mux->states); >> + >> + return 0; >> +} >> + >> +static struct platform_driver mux_sparx5_driver = { >> + .driver = { >> + .name = "sparx5-mux", >> + .of_match_table = of_match_ptr(mux_sparx5_dt_ids), >> + }, >> + .probe = mux_sparx5_probe, >> +}; >> +module_platform_driver(mux_sparx5_driver); >> -- Lars Povlsen, Microchip _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel