Message ID | 20231213090910.25410-2-mike.looijmans@topic.nl |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:3b04:b0:fb:cd0c:d3e with SMTP id c4csp7642005dys; Wed, 13 Dec 2023 01:09:43 -0800 (PST) X-Google-Smtp-Source: AGHT+IFTEkKDJcnVB1hih4BpGzZEmfVgWjLYe0eL0RnAwSww6C15mrM7M9MQC7MmjG71KrqoHQE8 X-Received: by 2002:a92:c26e:0:b0:35f:7700:6103 with SMTP id h14-20020a92c26e000000b0035f77006103mr56823ild.19.1702458582939; Wed, 13 Dec 2023 01:09:42 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1702458582; cv=pass; d=google.com; s=arc-20160816; b=OiYRnVZF8+SVMFqUBLgAYwzGTz4nkl0FJAGlJsNJL6cPGFHg0BFx35mtQZvORBXkUJ I4SDirrTRUJX08j57402w1pYTDx68ENiWpEKsMIW/QM06JZE2Ssc2Y8IvMp/RqVJ1BIz si0OZlUQTzGgF7xHQ+563x2V6wvYlswaqpHYYPZ1/nb6k8qsEZNo8qWymyXnxPKVaq1Z tzrZL85sBLxXzL4juViROzGt/JODIfvPayPqG9paSh012V7K1ZVogJXZcAqT4D3gej35 +csLm57ucVr91o5F3qbYstbKGFvN2DXBO0SC+MfzP4qdb4cAv64fvjki1Y2Yfx9to1rl zXcg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:content-transfer-encoding :references:in-reply-to:message-id:date:subject:cc:to:from :authentication-results-original:dkim-signature; bh=8zFpN/3kJ927lymW58oF6ZQARggZE7jGV18xhqSu+6U=; fh=Zk+L/oYRKJ6S8GMOFCt5Bkyfa/VL2Nfg/fcZo2dY+1I=; b=WR4UmdSraAm3yYfc0HhmnlsfajZzowAJqwAs9OaAKpYQkDdvb34mJrQorPRY+h+i2T 7TdI9nawJQCvhmzu5v8Hn7ZlwSWKNYdutNDBm8tgc0ytcDAemTOQMiavdgtiJkmI6hbV JY7QT6xJxAMJF4uM8oJaGUc6BIQfFa5Zb+2r7mm/77+1r9DF8cOFhPT6cjfSB/2l1v4D dK29djpqXYRe463HdM2tLpQiwvYIdtVp7E2x1rta8uEpuku/V+nQ5fE0MdHqwjP0/qm0 puzAZ/ZGqqXELC4sKN/wlBp7TY38n3wdCrv7/+xjFu4klLkAS55S9k3z/d1AMFDfsiu1 z+EQ== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@topic.nl header.s=selector2 header.b=PHAbtGX3; arc=pass (i=1 spf=pass spfdomain=topic.nl); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from agentk.vger.email (agentk.vger.email. [23.128.96.32]) by mx.google.com with ESMTPS id ca10-20020a056a02068a00b005be0050a443si9332285pgb.430.2023.12.13.01.09.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Dec 2023 01:09:42 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Authentication-Results: mx.google.com; dkim=pass header.i=@topic.nl header.s=selector2 header.b=PHAbtGX3; arc=pass (i=1 spf=pass spfdomain=topic.nl); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id D2AFD80B12CD; Wed, 13 Dec 2023 01:09:39 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231701AbjLMJJW (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Wed, 13 Dec 2023 04:09:22 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47720 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229490AbjLMJJU (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 13 Dec 2023 04:09:20 -0500 Received: from EUR02-DB5-obe.outbound.protection.outlook.com (mail-db5eur02on2070.outbound.protection.outlook.com [40.107.249.70]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2EE38AD; Wed, 13 Dec 2023 01:09:26 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SNrXG0WmTHn6CZuwJAXUoH7UpVYelM47OewZfY+TPHuwE6QCCiKHHH7c2tjNXLIQppJ7DSrF/xv9fWuq8k9+g9zkH1rPrCFU5UyqG6CfL0VfKfv2GBgVo/INO4Daq45W3xCC9Vu5xdYY+WOwq+UYlrmwiDhzNWRmCHaBiIK5ZzatCVqQ+ntuHnkKuo2V6nl7WGbEZTpdZ4MLbPahvZQQ0YvoWq+ACQfXYVTj0WxWGogol7ADiJUNQZW9Cj06LPj/FJ0vfMoxbQ9s7j2aRCtBbgHYJJFYPOC7NW0K325CVsNK7HmJNf+miOpSWqd12NaRVhrlZ0Ut3VNyd1cJXVtXJA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=8zFpN/3kJ927lymW58oF6ZQARggZE7jGV18xhqSu+6U=; b=XdbxXIH0Fv7QEBgMK2dsi9LTWYzv3LcKhVddEruo8QLD6OePCZXVbY/1wOUQMRXVXwHQ/V/bguWalMmocFO/OIihJokoB72TmVOkHlpMr0ZpetDmVdty/m33ClmWr0XEGy1OrJUW1aCWb0GWSjHjL3sn/CKfXrScGt/giW4vZnAo55z1KnYQGyyelcjOVtBmQrbVTU+RYSmBsPyyJUJU3Etite1xZXkBv9zJaZOwPjt6LCQKAqXdfyPA7kAq6Tk82f3owwCUWbZrWBS6yqA3UOcx9PsiesUwY5EBeuZLaa+LQKhzvSnp/XW2DodzR7owVbAiBKVfGiS3HmQYO2ygQg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 13.93.42.39) smtp.rcpttodomain=vger.kernel.org smtp.mailfrom=topic.nl; dmarc=bestguesspass action=none header.from=topic.nl; dkim=none (message not signed); arc=none (0) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=topic.nl; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=8zFpN/3kJ927lymW58oF6ZQARggZE7jGV18xhqSu+6U=; b=PHAbtGX3vmHmB67qNGC/uT8kwULcRxHAug7ZVpp9L+TIKmwrU8GD5WuoFNqdFm7IYTFXNeEhzLRkg5U29HnUbmfR6YirBWPIuVZBU5bICKrXXllQZWRwO8PugXBsF4ZSjGzvxeQsweRyjj3ToM+6M/XgmCmQ0tt2piLItsQRHEaN5rIS4DkGN7a3yRPXuEpXVVJ+fEk5xLxKBmdhE0fLAzOj8VgQ+wMCvOvApfngqh4OVloi5zu6wkQ6JqE3GJ2Kv0lxSv+bNt/JLi3X7mtfiKnD9USxYY7is6geRsP/EbGgcfh4CpCJEu6vSMzWJvZpYmn0F+E5lBoyXTAADZGzow== Received: from FR3P281CA0054.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:4a::18) by DU2PR04MB8774.eurprd04.prod.outlook.com (2603:10a6:10:2e1::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7091.26; Wed, 13 Dec 2023 09:09:23 +0000 Received: from VE1EUR01FT077.eop-EUR01.prod.protection.outlook.com (2603:10a6:d10:4a:cafe::f5) by FR3P281CA0054.outlook.office365.com (2603:10a6:d10:4a::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7091.21 via Frontend Transport; Wed, 13 Dec 2023 09:09:23 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 13.93.42.39) smtp.mailfrom=topic.nl; dkim=none (message not signed) header.d=none;dmarc=bestguesspass action=none header.from=topic.nl; Received-SPF: Pass (protection.outlook.com: domain of topic.nl designates 13.93.42.39 as permitted sender) receiver=protection.outlook.com; client-ip=13.93.42.39; helo=westeu12-emailsignatures-cloud.codetwo.com; pr=C Received: from westeu12-emailsignatures-cloud.codetwo.com (13.93.42.39) by VE1EUR01FT077.mail.protection.outlook.com (10.152.3.154) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7091.18 via Frontend Transport; Wed, 13 Dec 2023 09:09:23 +0000 Received: from EUR03-VI1-obe.outbound.protection.outlook.com (104.47.30.105) by westeu12-emailsignatures-cloud.codetwo.com with CodeTwo SMTP Server (TLS12) via SMTP; Wed, 13 Dec 2023 09:09:22 +0000 Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=topic.nl; Received: from VE1PR04MB7471.eurprd04.prod.outlook.com (2603:10a6:800:1a7::11) by AM9PR04MB8194.eurprd04.prod.outlook.com (2603:10a6:20b:3e6::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7091.26; Wed, 13 Dec 2023 09:09:18 +0000 Received: from VE1PR04MB7471.eurprd04.prod.outlook.com ([fe80::ec21:cf04:ed05:c7b1]) by VE1PR04MB7471.eurprd04.prod.outlook.com ([fe80::ec21:cf04:ed05:c7b1%4]) with mapi id 15.20.7068.033; Wed, 13 Dec 2023 09:09:18 +0000 From: Mike Looijmans <mike.looijmans@topic.nl> To: devicetree@vger.kernel.org, linux-iio@vger.kernel.org CC: Mike Looijmans <mike.looijmans@topic.nl>, Andrea Collamati <andrea.collamati@gmail.com>, Angelo Dureghello <angelo.dureghello@timesys.com>, Fabio Estevam <festevam@gmail.com>, Jonathan Cameron <jic23@kernel.org>, Lars-Peter Clausen <lars@metafoo.de>, Lukas Bulwahn <lukas.bulwahn@gmail.com>, William Breathitt Gray <william.gray@linaro.org>, linux-kernel@vger.kernel.org Subject: [PATCH 2/2] iio: spi-dac: Add driver for SPI shift register DACs Date: Wed, 13 Dec 2023 10:09:10 +0100 Message-ID: <20231213090910.25410-2-mike.looijmans@topic.nl> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231213090910.25410-1-mike.looijmans@topic.nl> References: <20231213090910.25410-1-mike.looijmans@topic.nl> <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.23e530d1-f5da-4919-8889-d7109d21097b@emailsignatures365.codetwo.com> Content-Transfer-Encoding: quoted-printable Content-Type: text/plain X-ClientProxiedBy: AM0P190CA0021.EURP190.PROD.OUTLOOK.COM (2603:10a6:208:190::31) To VE1PR04MB7471.eurprd04.prod.outlook.com (2603:10a6:800:1a7::11) MIME-Version: 1.0 X-MS-TrafficTypeDiagnostic: VE1PR04MB7471:EE_|AM9PR04MB8194:EE_|VE1EUR01FT077:EE_|DU2PR04MB8774:EE_ X-MS-Office365-Filtering-Correlation-Id: c76557e9-aef8-47a6-0c98-08dbfbbb32d2 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: ZEL0EEzDsT2UmCJVdKGArgG1blRmI3HtkzNgQFbTXcRqyrwSoYjBv0FS5rczxtZ3YK095yZc79N9h1Pi5Qo5GFzUZV1YUm9eMeiSr7HjQ9Cq/i7VxbL3EaMc4gp6TSmdrhcbJtbU4iE5ubL45rIJ3V+sI1iUK4gbfUGdu50BWjKEgAo3zXju4fkR5+iOJ6xemKwdz5F24NT+yUiVyxQ3AvKsEDxIFz4dEnQuCkfpy75MligTJTS3hPvgky3n1jLwH/E0S+gMcDlqiMNZx9oqtNHryzcjrFaGeygLQ2jU9+W5lZ7HhoYhnBfAdQQ+IuFOgj1aL/dhjYsEU/3KcUpiGk627KOYAnDzGn8bz4aFjD/jlTjRumlsWJD9cYfqwjOpbnYNnVKNyv8KmnwwBtC58YTvD1QzguJcWRlNpbQUeP5e5yerlL0jgxj5fYChqip8ODUwDRrZktv7C5MM+zp/ioa4RyPskU1G3yKEqfbJetDWJzUk94vC0B2LJQ6IjkBEarCBi90zfOOIO14b63nEJZ7/0avMJJphwTaj0HEyIysMAIC50U06s2A648dp6yMGOTVgLeBs36uvqr5copnkwRu3gkuAeyDQcWI/OLb8J5fYeKfQ3fZ3Y79ai95TiKbf X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VE1PR04MB7471.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(346002)(376002)(396003)(366004)(39840400004)(136003)(230922051799003)(186009)(451199024)(1800799012)(64100799003)(2616005)(83380400001)(26005)(38100700002)(44832011)(8936002)(316002)(54906003)(8676002)(2906002)(5660300002)(7416002)(4326008)(66476007)(478600001)(52116002)(41300700001)(6512007)(6666004)(6506007)(66556008)(6486002)(66946007)(1076003)(38350700005)(36756003)(86362001);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8194 X-CodeTwo-MessageID: e76d5a6c-a58d-47d0-ba7f-fb9ea0e39155.20231213090922@westeu12-emailsignatures-cloud.codetwo.com X-CodeTwoProcessed: true X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR01FT077.eop-EUR01.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 297872ce-b299-46b1-52fa-08dbfbbb2f9a X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: fKOes0ydokEZqbfp/a9NJBvf43vHwo9TVUCChPPKF5vCzKPhWnJn9a9/u+sNvI6Y1PqbgYJiOkWBQe6HJzKFLEgfUENO/OTx5phm4BM1bZF/EXIfbfP9ChbZhO1DlMnlZw9qhSVqIqSyYyriymWBFzkobaOf7ot4hgqALaY5ImlWDppOkKAb9u40kZSU9GORmy6XO5IobXPoDQmitkeRgtisv3+P3+ZnKqnmjq/FsgArTIoxj3ifDkhrxwzxW9rnNpwJcyvvfdhyIoGFXhuwfp2JoYBR8ODUP2KM4uEpozc3cF0kBEPXiT7JiA5hpt2dChinweHUmiVtqx+g9J9za0OZSYJtwDCjA3j2RKALePJlX4Vnbb3gP6SR3EYx00SPzfQCitA+VdcMVdfPdhVWSvwRG+HX/nhsgPyVC9rqBuxdhyqar8j6a+8VqNcL7nke01wWqYoHCbttrbPzYR9cUmbw6AoqbfB7viz/TvfBgz74YWGQlm9qXiq5oOeaNMtoPHJJ5VcHi5i5WMqvqx7ICJAOCofP7qk46PvbPRC152/VBLZISnhw1dtL7/RbQ0hU2pEVDCROMFMHOgXwwB1olji8LWLNioRqM6igYYG947PHuCiP4Zj859KE+WgSuGHCAfgH6uzyWRZ0NqXwWBIAfMJzXviYUUbWOySMFVtZ7FsnQkrcJ4ufYoCBbVZEUdmo/ZoMZmIo9T2ko6O3YzUemw== X-Forefront-Antispam-Report: CIP:13.93.42.39;CTRY:NL;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:westeu12-emailsignatures-cloud.codetwo.com;PTR:westeu12-emailsignatures-cloud.codetwo.com;CAT:NONE;SFS:(13230031)(4636009)(396003)(39830400003)(346002)(376002)(136003)(230922051799003)(64100799003)(82310400011)(186009)(1800799012)(451199024)(46966006)(36840700001)(2616005)(1076003)(26005)(336012)(6506007)(6512007)(6666004)(36860700001)(47076005)(83380400001)(5660300002)(8676002)(7416002)(44832011)(4326008)(8936002)(41300700001)(2906002)(6486002)(478600001)(316002)(70206006)(54906003)(70586007)(15974865002)(86362001)(7636003)(7596003)(356005)(36756003)(40480700001);DIR:OUT;SFP:1101; X-OriginatorOrg: topic.nl X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 09:09:23.0014 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: c76557e9-aef8-47a6-0c98-08dbfbbb32d2 X-MS-Exchange-CrossTenant-Id: 449607a5-3517-482d-8d16-41dd868cbda3 X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=449607a5-3517-482d-8d16-41dd868cbda3;Ip=[13.93.42.39];Helo=[westeu12-emailsignatures-cloud.codetwo.com] X-MS-Exchange-CrossTenant-AuthSource: VE1EUR01FT077.eop-EUR01.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU2PR04MB8774 X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Wed, 13 Dec 2023 01:09:40 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785157211034610495 X-GMAIL-MSGID: 1785157211034610495 |
Series |
[1/2] dt-bindings: iio: spi-dac: Add driver for SPI shift register DACs
|
|
Commit Message
Mike Looijmans
Dec. 13, 2023, 9:09 a.m. UTC
Add a driver for generic serial shift register DACs like TI DAC714.
Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
drivers/iio/dac/Kconfig | 11 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/spi-dac.c | 212 ++++++++++++++++++++++++++++++++++++++
3 files changed, 224 insertions(+)
create mode 100644 drivers/iio/dac/spi-dac.c
Comments
Hi Mike, Some comments from me... On Wed, 2023-12-13 at 10:09 +0100, Mike Looijmans wrote: > Add a driver for generic serial shift register DACs like TI DAC714. > > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> > > --- > > drivers/iio/dac/Kconfig | 11 ++ > drivers/iio/dac/Makefile | 1 + > drivers/iio/dac/spi-dac.c | 212 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 224 insertions(+) > create mode 100644 drivers/iio/dac/spi-dac.c > > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig > index 93b8be183de6..bb35d901ee57 100644 > --- a/drivers/iio/dac/Kconfig > +++ b/drivers/iio/dac/Kconfig > @@ -410,6 +410,17 @@ config MCP4922 > To compile this driver as a module, choose M here: the module > will be called mcp4922. > > +config SPI_DAC > + tristate "SPI shift register DAC driver" > + depends on SPI > + help > + Driver for an array of shift-register DACs, like the TI DAC714. > + The driver shifts the DAC values into the registers in a SPI > + transfer, then optionally toggles a GPIO to latch the values. > + > + To compile this driver as a module, choose M here: the module > + will be called spi-dac. > + > config STM32_DAC > tristate "STMicroelectronics STM32 DAC" > depends on (ARCH_STM32 && OF) || COMPILE_TEST > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile > index 5b2bac900d5a..33748799b0f0 100644 > --- a/drivers/iio/dac/Makefile > +++ b/drivers/iio/dac/Makefile > @@ -45,6 +45,7 @@ obj-$(CONFIG_MCP4728) += mcp4728.o > obj-$(CONFIG_MCP4922) += mcp4922.o > obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o > obj-$(CONFIG_STM32_DAC) += stm32-dac.o > +obj-$(CONFIG_SPI_DAC) += spi-dac.o > obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o > obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o > obj-$(CONFIG_TI_DAC7311) += ti-dac7311.o > diff --git a/drivers/iio/dac/spi-dac.c b/drivers/iio/dac/spi-dac.c > new file mode 100644 > index 000000000000..0c0113d51604 > --- /dev/null > +++ b/drivers/iio/dac/spi-dac.c > @@ -0,0 +1,212 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SPI generic shift register Serial input Digital-to-Analog Converter > + * For example, TI DAC714 > + * > + * Copyright 2023 Topic Embedded Systems > + */ > +#include <linux/delay.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/spi/spi.h> > +#include <linux/gpio/consumer.h> > +#include <linux/iio/iio.h> > + > +struct spidac { > + struct spi_device *spi; > + struct gpio_desc *loaddacs; > + u8 *data; /* SPI buffer */ > + u32 data_size; > + /* Protect the data buffer and update sequence */ > + struct mutex lock; > +}; > + > +static int spidac_cmd_single(struct spidac *priv, > + const struct iio_chan_spec *chan, int val) > +{ > + u8 *data = priv->data + chan->address; > + unsigned int bytes = chan->scan_type.storagebits >> 3; the '3' seems a bit "magical". Is the intent diving by 8? I would say so and if it is, doing the explicit division would be more readable IMO. > + int ret; > + unsigned int i; > + > + /* Write big-endian value into data */ > + data += bytes - 1; > + for (i = 0; i < bytes; i++, val >>= 8, data--) > + *data = val & 0xff; This not optimal... You allow someone to put in any 'bits_per_channel' from FW. In theory, one could set, let's say 64bits but then you only allow an integer value. So, we need to make things more sane. With some rework, I think we can also make use of put_unalignedX()... > + > + ret = spi_write(priv->spi, priv->data, priv->data_size); > + if (ret) > + return ret; > + > + gpiod_set_value(priv->loaddacs, 1); > + udelay(1); > + gpiod_set_value(priv->loaddacs, 0); > + Can't we sleep in here? > + return 0; > +} > + > +static int spidac_decode(struct spidac *priv, const struct iio_chan_spec *chan) > +{ > + u8 *data = priv->data + chan->address; > + unsigned int bytes = chan->scan_type.storagebits >> 3; > + unsigned int i; > + int val = 0; > + > + /* Read big-endian value from data */ > + for (i = 0; i < bytes; i++, data++) > + val = (val << 8) | *data; > + Again, with some refactor I think we can make use of get_unalignedX()... > + return val; > +} > + > +static int spidac_read_raw(struct iio_dev *iio_dev, > + const struct iio_chan_spec *chan, > + int *val, int *val2, long mask) > +{ > + struct spidac *priv; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + priv = iio_priv(iio_dev); > + > + mutex_lock(&priv->lock); > + *val = spidac_decode(priv, chan); > + mutex_unlock(&priv->lock); > + > + return IIO_VAL_INT; > + > + case IIO_CHAN_INFO_SCALE: > + *val = 1; > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } > +} > + > +static int spidac_write_raw(struct iio_dev *iio_dev, > + const struct iio_chan_spec *chan, > + int val, int val2, long mask) > +{ > + struct spidac *priv = iio_priv(iio_dev); > + int ret; > + > + if (mask != IIO_CHAN_INFO_RAW) > + return -EINVAL; > + nit: I would still keep the switch(). Consistency with read_raw(). > + mutex_lock(&priv->lock); > + ret = spidac_cmd_single(priv, chan, val); > + mutex_unlock(&priv->lock); > + > + return ret; > +} > + > +static const struct iio_info spidac_info = { > + .read_raw = spidac_read_raw, > + .write_raw = spidac_write_raw, > +}; > + > +static int spidac_probe(struct spi_device *spi) > +{ > + struct iio_dev *iio_dev; > + struct spidac *priv; > + struct iio_chan_spec *channels; > + struct gpio_desc *reset_gpio; > + u32 num_channels; > + u32 bits_per_channel; > + u32 bytes_per_channel; > + u32 i; > + > + iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*priv)); > + if (!iio_dev) > + return -ENOMEM; > + > + priv = iio_priv(iio_dev); > + priv->loaddacs = devm_gpiod_get_optional(&spi->dev, "ldac", > + GPIOD_OUT_LOW); > + if (IS_ERR(priv->loaddacs)) > + return PTR_ERR(priv->loaddacs); > + > + reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", > + GPIOD_OUT_HIGH); > + if (IS_ERR(reset_gpio)) > + return PTR_ERR(reset_gpio); > + > + priv->spi = spi; > + spi_set_drvdata(spi, iio_dev); > + num_channels = 1; > + bits_per_channel = 16; > + > + device_property_read_u32(&spi->dev, "num-channels", &num_channels); > + device_property_read_u32(&spi->dev, "bits-per-channel", > + &bits_per_channel); > + bytes_per_channel = DIV_ROUND_UP(bits_per_channel, 8); > + > + channels = devm_kcalloc(&spi->dev, num_channels, sizeof(*channels), > + GFP_KERNEL); > + if (!channels) > + return -ENOMEM; > + > + priv->data_size = num_channels * bytes_per_channel; > + priv->data = devm_kzalloc(&spi->dev, priv->data_size, > + GFP_KERNEL | GFP_DMA); GFP_DMA? This is rather unusual... And if you look at the description of it, does not look like a good idea to use it. Also, consider devm_kcalloc() > + if (!priv->data) > + return -ENOMEM; > + > + for (i = 0; i < num_channels; i++) { > + struct iio_chan_spec *chan = &channels[i]; > + > + chan->type = IIO_VOLTAGE; > + chan->indexed = 1; > + chan->output = 1; > + chan->channel = i; > + chan->address = i * bytes_per_channel; > + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW); > + chan->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SCALE); > + chan->scan_type.sign = 's'; > + chan->scan_type.realbits = bits_per_channel; > + chan->scan_type.storagebits = bits_per_channel; Hmm does no look right. You pretty much allow any value from FW and I'm fairly sure that 'storagebits' have to be the size of a C data type as we want elements to be naturally aligned when buffering for example. I'm seeing you're not using buffering but still... Is really any arbitrary value what we want here? > + } > + > + iio_dev->info = &spidac_info; > + iio_dev->modes = INDIO_DIRECT_MODE; > + iio_dev->channels = channels; > + iio_dev->num_channels = num_channels; > + iio_dev->name = spi_get_device_id(spi)->name; > + > + mutex_init(&priv->lock); > + > + if (reset_gpio) { > + udelay(1); > + gpiod_set_value(reset_gpio, 0); > + } > + I would place devm_gpiod_get_optional() close to the place of the reset... Also, any strong reason for udelay()? Consider fsleep() instead. > + return devm_iio_device_register(&spi->dev, iio_dev); > +} > + > +static const struct spi_device_id spidac_id[] = { > + {"spi-dac"}, no ti,dac714? > + {} > +}; > +MODULE_DEVICE_TABLE(spi, spidac_id); > + > +static const struct of_device_id spidac_of_match[] = { > + { .compatible = "spi-dac" }, > + { .compatible = "ti,dac714" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, spidac_of_match); > + > +static struct spi_driver spidac_driver = { > + .driver = { > + .name = "spi-dac", > + .of_match_table = spidac_of_match, > + }, indentation... - Nuno Sá
Hi Nuno, Thanks for reviewing, See below... On 13-12-2023 11:37, Nuno Sá wrote: > Hi Mike, > > Some comments from me... > > On Wed, 2023-12-13 at 10:09 +0100, Mike Looijmans wrote: >> Add a driver for generic serial shift register DACs like TI DAC714. >> >> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> >> >> --- >> >> drivers/iio/dac/Kconfig | 11 ++ >> drivers/iio/dac/Makefile | 1 + >> drivers/iio/dac/spi-dac.c | 212 ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 224 insertions(+) >> create mode 100644 drivers/iio/dac/spi-dac.c >> >> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig >> index 93b8be183de6..bb35d901ee57 100644 >> --- a/drivers/iio/dac/Kconfig >> +++ b/drivers/iio/dac/Kconfig >> @@ -410,6 +410,17 @@ config MCP4922 >> To compile this driver as a module, choose M here: the module >> will be called mcp4922. >> >> +config SPI_DAC >> + tristate "SPI shift register DAC driver" >> + depends on SPI >> + help >> + Driver for an array of shift-register DACs, like the TI DAC714. >> + The driver shifts the DAC values into the registers in a SPI >> + transfer, then optionally toggles a GPIO to latch the values. >> + >> + To compile this driver as a module, choose M here: the module >> + will be called spi-dac. >> + >> config STM32_DAC >> tristate "STMicroelectronics STM32 DAC" >> depends on (ARCH_STM32 && OF) || COMPILE_TEST >> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile >> index 5b2bac900d5a..33748799b0f0 100644 >> --- a/drivers/iio/dac/Makefile >> +++ b/drivers/iio/dac/Makefile >> @@ -45,6 +45,7 @@ obj-$(CONFIG_MCP4728) += mcp4728.o >> obj-$(CONFIG_MCP4922) += mcp4922.o >> obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o >> obj-$(CONFIG_STM32_DAC) += stm32-dac.o >> +obj-$(CONFIG_SPI_DAC) += spi-dac.o >> obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o >> obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o >> obj-$(CONFIG_TI_DAC7311) += ti-dac7311.o >> diff --git a/drivers/iio/dac/spi-dac.c b/drivers/iio/dac/spi-dac.c >> new file mode 100644 >> index 000000000000..0c0113d51604 >> --- /dev/null >> +++ b/drivers/iio/dac/spi-dac.c >> @@ -0,0 +1,212 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * SPI generic shift register Serial input Digital-to-Analog Converter >> + * For example, TI DAC714 >> + * >> + * Copyright 2023 Topic Embedded Systems >> + */ >> +#include <linux/delay.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/spi/spi.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/iio/iio.h> >> + >> +struct spidac { >> + struct spi_device *spi; >> + struct gpio_desc *loaddacs; >> + u8 *data; /* SPI buffer */ >> + u32 data_size; >> + /* Protect the data buffer and update sequence */ >> + struct mutex lock; >> +}; >> + >> +static int spidac_cmd_single(struct spidac *priv, >> + const struct iio_chan_spec *chan, int val) >> +{ >> + u8 *data = priv->data + chan->address; >> + unsigned int bytes = chan->scan_type.storagebits >> 3; > the '3' seems a bit "magical". Is the intent diving by 8? I would say so and if it > is, doing the explicit division would be more readable IMO. Divide by 8 is indeed the intention (bits to bytes). But... if storagebits cannot be 24 for example, it'll have to be stored elsewhere anyway. > >> + int ret; >> + unsigned int i; >> + >> + /* Write big-endian value into data */ >> + data += bytes - 1; >> + for (i = 0; i < bytes; i++, val >>= 8, data--) >> + *data = val & 0xff; > This not optimal... You allow someone to put in any 'bits_per_channel' from FW. In > theory, one could set, let's say 64bits but then you only allow an integer value. So, > we need to make things more sane. I think limiting to 32 bit is sensible enough (a 64-bit DAC would be moving individual electrons around or so), but that should indeed be made explicit. > With some rework, I think we can also make use of put_unalignedX()... Agree. Might want to make endianness a configuration option as well (haven't seen a little-endian device though). >> + >> + ret = spi_write(priv->spi, priv->data, priv->data_size); >> + if (ret) >> + return ret; >> + >> + gpiod_set_value(priv->loaddacs, 1); >> + udelay(1); >> + gpiod_set_value(priv->loaddacs, 0); >> + > Can't we sleep in here? indeed, should have used _can_sleep variants (spi_write needs to sleep anyway). Probably left over from my thought of doing it in the SPI completion callback. >> + return 0; >> +} >> + >> +static int spidac_decode(struct spidac *priv, const struct iio_chan_spec *chan) >> +{ >> + u8 *data = priv->data + chan->address; >> + unsigned int bytes = chan->scan_type.storagebits >> 3; >> + unsigned int i; >> + int val = 0; >> + >> + /* Read big-endian value from data */ >> + for (i = 0; i < bytes; i++, data++) >> + val = (val << 8) | *data; >> + > Again, with some refactor I think we can make use of get_unalignedX()... > >> + return val; >> +} >> + >> +static int spidac_read_raw(struct iio_dev *iio_dev, >> + const struct iio_chan_spec *chan, >> + int *val, int *val2, long mask) >> +{ >> + struct spidac *priv; >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_RAW: >> + priv = iio_priv(iio_dev); >> + >> + mutex_lock(&priv->lock); >> + *val = spidac_decode(priv, chan); >> + mutex_unlock(&priv->lock); >> + >> + return IIO_VAL_INT; >> + >> + case IIO_CHAN_INFO_SCALE: >> + *val = 1; >> + return IIO_VAL_INT; >> + >> + default: >> + return -EINVAL; >> + } >> +} >> + >> +static int spidac_write_raw(struct iio_dev *iio_dev, >> + const struct iio_chan_spec *chan, >> + int val, int val2, long mask) >> +{ >> + struct spidac *priv = iio_priv(iio_dev); >> + int ret; >> + >> + if (mask != IIO_CHAN_INFO_RAW) >> + return -EINVAL; >> + > nit: I would still keep the switch(). Consistency with read_raw(). Agree > >> + mutex_lock(&priv->lock); >> + ret = spidac_cmd_single(priv, chan, val); >> + mutex_unlock(&priv->lock); >> + >> + return ret; >> +} >> + >> +static const struct iio_info spidac_info = { >> + .read_raw = spidac_read_raw, >> + .write_raw = spidac_write_raw, >> +}; >> + >> +static int spidac_probe(struct spi_device *spi) >> +{ >> + struct iio_dev *iio_dev; >> + struct spidac *priv; >> + struct iio_chan_spec *channels; >> + struct gpio_desc *reset_gpio; >> + u32 num_channels; >> + u32 bits_per_channel; >> + u32 bytes_per_channel; >> + u32 i; >> + >> + iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*priv)); >> + if (!iio_dev) >> + return -ENOMEM; >> + >> + priv = iio_priv(iio_dev); >> + priv->loaddacs = devm_gpiod_get_optional(&spi->dev, "ldac", >> + GPIOD_OUT_LOW); >> + if (IS_ERR(priv->loaddacs)) >> + return PTR_ERR(priv->loaddacs); >> + >> + reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", >> + GPIOD_OUT_HIGH); >> + if (IS_ERR(reset_gpio)) >> + return PTR_ERR(reset_gpio); >> + >> + priv->spi = spi; >> + spi_set_drvdata(spi, iio_dev); >> + num_channels = 1; >> + bits_per_channel = 16; >> + >> + device_property_read_u32(&spi->dev, "num-channels", &num_channels); >> + device_property_read_u32(&spi->dev, "bits-per-channel", >> + &bits_per_channel); >> + bytes_per_channel = DIV_ROUND_UP(bits_per_channel, 8); >> + >> + channels = devm_kcalloc(&spi->dev, num_channels, sizeof(*channels), >> + GFP_KERNEL); >> + if (!channels) >> + return -ENOMEM; >> + >> + priv->data_size = num_channels * bytes_per_channel; >> + priv->data = devm_kzalloc(&spi->dev, priv->data_size, >> + GFP_KERNEL | GFP_DMA); > GFP_DMA? This is rather unusual... And if you look at the description of it, does not > look like a good idea to use it. Also, consider devm_kcalloc() The "data" buffer is to be passed to SPI controller and must be DMA capable. Hence the GFP_DMA. Feels oldskool to me as well, but could not come up with a better solution... >> + if (!priv->data) >> + return -ENOMEM; >> + >> + for (i = 0; i < num_channels; i++) { >> + struct iio_chan_spec *chan = &channels[i]; >> + >> + chan->type = IIO_VOLTAGE; >> + chan->indexed = 1; >> + chan->output = 1; >> + chan->channel = i; >> + chan->address = i * bytes_per_channel; >> + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW); >> + chan->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SCALE); >> + chan->scan_type.sign = 's'; >> + chan->scan_type.realbits = bits_per_channel; >> + chan->scan_type.storagebits = bits_per_channel; > Hmm does no look right. You pretty much allow any value from FW and I'm fairly sure > that 'storagebits' have to be the size of a C data type as we want elements to be > naturally aligned when buffering for example. I'm seeing you're not using buffering > but still... Is really any arbitrary value what we want here? Found out the hard way (while writing ads1298 driver) that this is indeed the case, IIO cannot handle 24-bit buffers for example. This driver doesn't support any buffering though. So indeed, I'll have to separate things. This will also affect DT bindings. How many bytes per sample to be sent on the SPI bus, and how many bits actually mean anything. For example, 2 bytes per sample and 14-bit resolution. >> + } >> + >> + iio_dev->info = &spidac_info; >> + iio_dev->modes = INDIO_DIRECT_MODE; >> + iio_dev->channels = channels; >> + iio_dev->num_channels = num_channels; >> + iio_dev->name = spi_get_device_id(spi)->name; >> + >> + mutex_init(&priv->lock); >> + >> + if (reset_gpio) { >> + udelay(1); >> + gpiod_set_value(reset_gpio, 0); >> + } >> + > I would place devm_gpiod_get_optional() close to the place of the reset... Also, any > strong reason for udelay()? Consider fsleep() instead. That, and can_sleep. > >> + return devm_iio_device_register(&spi->dev, iio_dev); >> +} >> + >> +static const struct spi_device_id spidac_id[] = { >> + {"spi-dac"}, > no ti,dac714? That, and I've been wondering if this table is needed at all? > >> + {} >> +}; >> +MODULE_DEVICE_TABLE(spi, spidac_id); >> + >> +static const struct of_device_id spidac_of_match[] = { >> + { .compatible = "spi-dac" }, >> + { .compatible = "ti,dac714" }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, spidac_of_match); >> + >> +static struct spi_driver spidac_driver = { >> + .driver = { >> + .name = "spi-dac", >> + .of_match_table = spidac_of_match, >> + }, > indentation... > > - Nuno Sá >
On Wed, 2023-12-13 at 14:24 +0100, Mike Looijmans wrote: > Hi Nuno, > > Thanks for reviewing, > > See below... > > On 13-12-2023 11:37, Nuno Sá wrote: > > Hi Mike, > > > > Some comments from me... > > > > On Wed, 2023-12-13 at 10:09 +0100, Mike Looijmans wrote: > > > Add a driver for generic serial shift register DACs like TI DAC714. > > > > > > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> > > > > > > --- > > > > > > drivers/iio/dac/Kconfig | 11 ++ > > > drivers/iio/dac/Makefile | 1 + > > > drivers/iio/dac/spi-dac.c | 212 ++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 224 insertions(+) > > > create mode 100644 drivers/iio/dac/spi-dac.c > > > > > > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig > > > index 93b8be183de6..bb35d901ee57 100644 > > > --- a/drivers/iio/dac/Kconfig > > > +++ b/drivers/iio/dac/Kconfig > > > @@ -410,6 +410,17 @@ config MCP4922 > > > To compile this driver as a module, choose M here: the module > > > will be called mcp4922. > > > > > > +config SPI_DAC > > > + tristate "SPI shift register DAC driver" > > > + depends on SPI > > > + help > > > + Driver for an array of shift-register DACs, like the TI DAC714. > > > + The driver shifts the DAC values into the registers in a SPI > > > + transfer, then optionally toggles a GPIO to latch the values. > > > + > > > + To compile this driver as a module, choose M here: the module > > > + will be called spi-dac. > > > + > > > config STM32_DAC > > > tristate "STMicroelectronics STM32 DAC" > > > depends on (ARCH_STM32 && OF) || COMPILE_TEST > > > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile > > > index 5b2bac900d5a..33748799b0f0 100644 > > > --- a/drivers/iio/dac/Makefile > > > +++ b/drivers/iio/dac/Makefile > > > @@ -45,6 +45,7 @@ obj-$(CONFIG_MCP4728) += mcp4728.o > > > obj-$(CONFIG_MCP4922) += mcp4922.o > > > obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o > > > obj-$(CONFIG_STM32_DAC) += stm32-dac.o > > > +obj-$(CONFIG_SPI_DAC) += spi-dac.o > > > obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o > > > obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o > > > obj-$(CONFIG_TI_DAC7311) += ti-dac7311.o > > > diff --git a/drivers/iio/dac/spi-dac.c b/drivers/iio/dac/spi-dac.c > > > new file mode 100644 > > > index 000000000000..0c0113d51604 > > > --- /dev/null > > > +++ b/drivers/iio/dac/spi-dac.c > > > @@ -0,0 +1,212 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * SPI generic shift register Serial input Digital-to-Analog Converter > > > + * For example, TI DAC714 > > > + * > > > + * Copyright 2023 Topic Embedded Systems > > > + */ > > > +#include <linux/delay.h> > > > +#include <linux/kernel.h> > > > +#include <linux/module.h> > > > +#include <linux/spi/spi.h> > > > +#include <linux/gpio/consumer.h> > > > +#include <linux/iio/iio.h> > > > + > > > +struct spidac { > > > + struct spi_device *spi; > > > + struct gpio_desc *loaddacs; > > > + u8 *data; /* SPI buffer */ > > > + u32 data_size; > > > + /* Protect the data buffer and update sequence */ > > > + struct mutex lock; > > > +}; > > > + > > > +static int spidac_cmd_single(struct spidac *priv, > > > + const struct iio_chan_spec *chan, int val) > > > +{ > > > + u8 *data = priv->data + chan->address; > > > + unsigned int bytes = chan->scan_type.storagebits >> 3; > > the '3' seems a bit "magical". Is the intent diving by 8? I would say so and if > > it > > is, doing the explicit division would be more readable IMO. > > Divide by 8 is indeed the intention (bits to bytes). But... if > storagebits cannot be 24 for example, it'll have to be stored elsewhere > anyway. > > > > > > > + int ret; > > > + unsigned int i; > > > + > > > + /* Write big-endian value into data */ > > > + data += bytes - 1; > > > + for (i = 0; i < bytes; i++, val >>= 8, data--) > > > + *data = val & 0xff; > > This not optimal... You allow someone to put in any 'bits_per_channel' from FW. > > In > > theory, one could set, let's say 64bits but then you only allow an integer value. > > So, > > we need to make things more sane. > > I think limiting to 32 bit is sensible enough (a 64-bit DAC would be > moving individual electrons around or so), but that should indeed be > made explicit. > > > > With some rework, I think we can also make use of put_unalignedX()... > > Agree. Might want to make endianness a configuration option as well > (haven't seen a little-endian device though). Then I would keep it simple for now and worry about little-endian if an actual use case pops up. > > > > > + > > > + ret = spi_write(priv->spi, priv->data, priv->data_size); > > > + if (ret) > > > + return ret; > > > + > > > + gpiod_set_value(priv->loaddacs, 1); > > > + udelay(1); > > > + gpiod_set_value(priv->loaddacs, 0); > > > + > > Can't we sleep in here? > > indeed, should have used _can_sleep variants (spi_write needs to sleep > anyway). > > Probably left over from my thought of doing it in the SPI completion > callback. > > > > > + return 0; > > > +} > > > + > > > +static int spidac_decode(struct spidac *priv, const struct iio_chan_spec > > > *chan) > > > +{ > > > + u8 *data = priv->data + chan->address; > > > + unsigned int bytes = chan->scan_type.storagebits >> 3; > > > + unsigned int i; > > > + int val = 0; > > > + > > > + /* Read big-endian value from data */ > > > + for (i = 0; i < bytes; i++, data++) > > > + val = (val << 8) | *data; > > > + > > Again, with some refactor I think we can make use of get_unalignedX()... > > > > > + return val; > > > +} > > > + > > > +static int spidac_read_raw(struct iio_dev *iio_dev, > > > + const struct iio_chan_spec *chan, > > > + int *val, int *val2, long mask) > > > +{ > > > + struct spidac *priv; > > > + > > > + switch (mask) { > > > + case IIO_CHAN_INFO_RAW: > > > + priv = iio_priv(iio_dev); > > > + > > > + mutex_lock(&priv->lock); > > > + *val = spidac_decode(priv, chan); > > > + mutex_unlock(&priv->lock); > > > + > > > + return IIO_VAL_INT; > > > + > > > + case IIO_CHAN_INFO_SCALE: > > > + *val = 1; > > > + return IIO_VAL_INT; > > > + > > > + default: > > > + return -EINVAL; > > > + } > > > +} > > > + > > > +static int spidac_write_raw(struct iio_dev *iio_dev, > > > + const struct iio_chan_spec *chan, > > > + int val, int val2, long mask) > > > +{ > > > + struct spidac *priv = iio_priv(iio_dev); > > > + int ret; > > > + > > > + if (mask != IIO_CHAN_INFO_RAW) > > > + return -EINVAL; > > > + > > nit: I would still keep the switch(). Consistency with read_raw(). > Agree > > > > > + mutex_lock(&priv->lock); > > > + ret = spidac_cmd_single(priv, chan, val); > > > + mutex_unlock(&priv->lock); > > > + > > > + return ret; > > > +} > > > + > > > +static const struct iio_info spidac_info = { > > > + .read_raw = spidac_read_raw, > > > + .write_raw = spidac_write_raw, > > > +}; > > > + > > > +static int spidac_probe(struct spi_device *spi) > > > +{ > > > + struct iio_dev *iio_dev; > > > + struct spidac *priv; > > > + struct iio_chan_spec *channels; > > > + struct gpio_desc *reset_gpio; > > > + u32 num_channels; > > > + u32 bits_per_channel; > > > + u32 bytes_per_channel; > > > + u32 i; > > > + > > > + iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*priv)); > > > + if (!iio_dev) > > > + return -ENOMEM; > > > + > > > + priv = iio_priv(iio_dev); > > > + priv->loaddacs = devm_gpiod_get_optional(&spi->dev, "ldac", > > > + GPIOD_OUT_LOW); > > > + if (IS_ERR(priv->loaddacs)) > > > + return PTR_ERR(priv->loaddacs); > > > + > > > + reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", > > > + GPIOD_OUT_HIGH); > > > + if (IS_ERR(reset_gpio)) > > > + return PTR_ERR(reset_gpio); > > > + > > > + priv->spi = spi; > > > + spi_set_drvdata(spi, iio_dev); > > > + num_channels = 1; > > > + bits_per_channel = 16; > > > + > > > + device_property_read_u32(&spi->dev, "num-channels", &num_channels); > > > + device_property_read_u32(&spi->dev, "bits-per-channel", > > > + &bits_per_channel); > > > + bytes_per_channel = DIV_ROUND_UP(bits_per_channel, 8); > > > + > > > + channels = devm_kcalloc(&spi->dev, num_channels, sizeof(*channels), > > > + GFP_KERNEL); > > > + if (!channels) > > > + return -ENOMEM; > > > + > > > + priv->data_size = num_channels * bytes_per_channel; > > > + priv->data = devm_kzalloc(&spi->dev, priv->data_size, > > > + GFP_KERNEL | GFP_DMA); > > GFP_DMA? This is rather unusual... And if you look at the description of it, does > > not > > look like a good idea to use it. Also, consider devm_kcalloc() > > The "data" buffer is to be passed to SPI controller and must be DMA > capable. Hence the GFP_DMA. > > Feels oldskool to me as well, but could not come up with a better > solution... > Hmm, that has nothing to do with GFP_DMA AFAIK... Note that you're devm_kzalloc() your buffer so your data is already DMA safe (meaning cacheline aligned - and not just L1) . Now if DMA is used or not is entirely up to the spi controller (normally it depends on the amount of data you want to send). > > > + if (!priv->data) > > > + return -ENOMEM; > > > + > > > + for (i = 0; i < num_channels; i++) { > > > + struct iio_chan_spec *chan = &channels[i]; > > > + > > > + chan->type = IIO_VOLTAGE; > > > + chan->indexed = 1; > > > + chan->output = 1; > > > + chan->channel = i; > > > + chan->address = i * bytes_per_channel; > > > + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW); > > > + chan->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SCALE); > > > + chan->scan_type.sign = 's'; > > > + chan->scan_type.realbits = bits_per_channel; > > > + chan->scan_type.storagebits = bits_per_channel; > > Hmm does no look right. You pretty much allow any value from FW and I'm fairly > > sure > > that 'storagebits' have to be the size of a C data type as we want elements to be > > naturally aligned when buffering for example. I'm seeing you're not using > > buffering > > but still... Is really any arbitrary value what we want here? > > Found out the hard way (while writing ads1298 driver) that this is > indeed the case, IIO cannot handle 24-bit buffers for example. This > driver doesn't support any buffering though. > > So indeed, I'll have to separate things. This will also affect DT bindings. > > How many bytes per sample to be sent on the SPI bus, and how many bits > actually mean anything. For example, 2 bytes per sample and 14-bit > resolution. > > > > + } > > > + > > > + iio_dev->info = &spidac_info; > > > + iio_dev->modes = INDIO_DIRECT_MODE; > > > + iio_dev->channels = channels; > > > + iio_dev->num_channels = num_channels; > > > + iio_dev->name = spi_get_device_id(spi)->name; > > > + > > > + mutex_init(&priv->lock); > > > + > > > + if (reset_gpio) { > > > + udelay(1); > > > + gpiod_set_value(reset_gpio, 0); > > > + } > > > + > > I would place devm_gpiod_get_optional() close to the place of the reset... Also, > > any > > strong reason for udelay()? Consider fsleep() instead. > > That, and can_sleep. > > > > > > > + return devm_iio_device_register(&spi->dev, iio_dev); > > > +} > > > + > > > +static const struct spi_device_id spidac_id[] = { > > > + {"spi-dac"}, > > no ti,dac714? > > That, and I've been wondering if this table is needed at all? > Yes, otherwise module auto loading won't work. You can remove it and see what happens :). - Nuno Sá
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index 93b8be183de6..bb35d901ee57 100644 --- a/drivers/iio/dac/Kconfig +++ b/drivers/iio/dac/Kconfig @@ -410,6 +410,17 @@ config MCP4922 To compile this driver as a module, choose M here: the module will be called mcp4922. +config SPI_DAC + tristate "SPI shift register DAC driver" + depends on SPI + help + Driver for an array of shift-register DACs, like the TI DAC714. + The driver shifts the DAC values into the registers in a SPI + transfer, then optionally toggles a GPIO to latch the values. + + To compile this driver as a module, choose M here: the module + will be called spi-dac. + config STM32_DAC tristate "STMicroelectronics STM32 DAC" depends on (ARCH_STM32 && OF) || COMPILE_TEST diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index 5b2bac900d5a..33748799b0f0 100644 --- a/drivers/iio/dac/Makefile +++ b/drivers/iio/dac/Makefile @@ -45,6 +45,7 @@ obj-$(CONFIG_MCP4728) += mcp4728.o obj-$(CONFIG_MCP4922) += mcp4922.o obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o obj-$(CONFIG_STM32_DAC) += stm32-dac.o +obj-$(CONFIG_SPI_DAC) += spi-dac.o obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o obj-$(CONFIG_TI_DAC7311) += ti-dac7311.o diff --git a/drivers/iio/dac/spi-dac.c b/drivers/iio/dac/spi-dac.c new file mode 100644 index 000000000000..0c0113d51604 --- /dev/null +++ b/drivers/iio/dac/spi-dac.c @@ -0,0 +1,212 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * SPI generic shift register Serial input Digital-to-Analog Converter + * For example, TI DAC714 + * + * Copyright 2023 Topic Embedded Systems + */ +#include <linux/delay.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/spi/spi.h> +#include <linux/gpio/consumer.h> +#include <linux/iio/iio.h> + +struct spidac { + struct spi_device *spi; + struct gpio_desc *loaddacs; + u8 *data; /* SPI buffer */ + u32 data_size; + /* Protect the data buffer and update sequence */ + struct mutex lock; +}; + +static int spidac_cmd_single(struct spidac *priv, + const struct iio_chan_spec *chan, int val) +{ + u8 *data = priv->data + chan->address; + unsigned int bytes = chan->scan_type.storagebits >> 3; + int ret; + unsigned int i; + + /* Write big-endian value into data */ + data += bytes - 1; + for (i = 0; i < bytes; i++, val >>= 8, data--) + *data = val & 0xff; + + ret = spi_write(priv->spi, priv->data, priv->data_size); + if (ret) + return ret; + + gpiod_set_value(priv->loaddacs, 1); + udelay(1); + gpiod_set_value(priv->loaddacs, 0); + + return 0; +} + +static int spidac_decode(struct spidac *priv, const struct iio_chan_spec *chan) +{ + u8 *data = priv->data + chan->address; + unsigned int bytes = chan->scan_type.storagebits >> 3; + unsigned int i; + int val = 0; + + /* Read big-endian value from data */ + for (i = 0; i < bytes; i++, data++) + val = (val << 8) | *data; + + return val; +} + +static int spidac_read_raw(struct iio_dev *iio_dev, + const struct iio_chan_spec *chan, + int *val, int *val2, long mask) +{ + struct spidac *priv; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + priv = iio_priv(iio_dev); + + mutex_lock(&priv->lock); + *val = spidac_decode(priv, chan); + mutex_unlock(&priv->lock); + + return IIO_VAL_INT; + + case IIO_CHAN_INFO_SCALE: + *val = 1; + return IIO_VAL_INT; + + default: + return -EINVAL; + } +} + +static int spidac_write_raw(struct iio_dev *iio_dev, + const struct iio_chan_spec *chan, + int val, int val2, long mask) +{ + struct spidac *priv = iio_priv(iio_dev); + int ret; + + if (mask != IIO_CHAN_INFO_RAW) + return -EINVAL; + + mutex_lock(&priv->lock); + ret = spidac_cmd_single(priv, chan, val); + mutex_unlock(&priv->lock); + + return ret; +} + +static const struct iio_info spidac_info = { + .read_raw = spidac_read_raw, + .write_raw = spidac_write_raw, +}; + +static int spidac_probe(struct spi_device *spi) +{ + struct iio_dev *iio_dev; + struct spidac *priv; + struct iio_chan_spec *channels; + struct gpio_desc *reset_gpio; + u32 num_channels; + u32 bits_per_channel; + u32 bytes_per_channel; + u32 i; + + iio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*priv)); + if (!iio_dev) + return -ENOMEM; + + priv = iio_priv(iio_dev); + priv->loaddacs = devm_gpiod_get_optional(&spi->dev, "ldac", + GPIOD_OUT_LOW); + if (IS_ERR(priv->loaddacs)) + return PTR_ERR(priv->loaddacs); + + reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", + GPIOD_OUT_HIGH); + if (IS_ERR(reset_gpio)) + return PTR_ERR(reset_gpio); + + priv->spi = spi; + spi_set_drvdata(spi, iio_dev); + num_channels = 1; + bits_per_channel = 16; + + device_property_read_u32(&spi->dev, "num-channels", &num_channels); + device_property_read_u32(&spi->dev, "bits-per-channel", + &bits_per_channel); + bytes_per_channel = DIV_ROUND_UP(bits_per_channel, 8); + + channels = devm_kcalloc(&spi->dev, num_channels, sizeof(*channels), + GFP_KERNEL); + if (!channels) + return -ENOMEM; + + priv->data_size = num_channels * bytes_per_channel; + priv->data = devm_kzalloc(&spi->dev, priv->data_size, + GFP_KERNEL | GFP_DMA); + if (!priv->data) + return -ENOMEM; + + for (i = 0; i < num_channels; i++) { + struct iio_chan_spec *chan = &channels[i]; + + chan->type = IIO_VOLTAGE; + chan->indexed = 1; + chan->output = 1; + chan->channel = i; + chan->address = i * bytes_per_channel; + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW); + chan->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SCALE); + chan->scan_type.sign = 's'; + chan->scan_type.realbits = bits_per_channel; + chan->scan_type.storagebits = bits_per_channel; + } + + iio_dev->info = &spidac_info; + iio_dev->modes = INDIO_DIRECT_MODE; + iio_dev->channels = channels; + iio_dev->num_channels = num_channels; + iio_dev->name = spi_get_device_id(spi)->name; + + mutex_init(&priv->lock); + + if (reset_gpio) { + udelay(1); + gpiod_set_value(reset_gpio, 0); + } + + return devm_iio_device_register(&spi->dev, iio_dev); +} + +static const struct spi_device_id spidac_id[] = { + {"spi-dac"}, + {} +}; +MODULE_DEVICE_TABLE(spi, spidac_id); + +static const struct of_device_id spidac_of_match[] = { + { .compatible = "spi-dac" }, + { .compatible = "ti,dac714" }, + { }, +}; +MODULE_DEVICE_TABLE(of, spidac_of_match); + +static struct spi_driver spidac_driver = { + .driver = { + .name = "spi-dac", + .of_match_table = spidac_of_match, + }, + .probe = spidac_probe, + .id_table = spidac_id, +}; +module_spi_driver(spidac_driver); + +MODULE_AUTHOR("Mike Looijmans <mike.looijmans@topic.nl>"); +MODULE_DESCRIPTION("SPI shift register DAC driver"); +MODULE_LICENSE("GPL");