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=-5.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MSGID_FROM_MTA_HEADER,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=no 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 9B1E2C2B9F8 for ; Tue, 25 May 2021 14:33:05 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 5789E61401 for ; Tue, 25 May 2021 14:33:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5789E61401 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=seco.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=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:In-Reply-To:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=3rEqLekRF+Gz/07z8L8SobomlMa78RLUC0jhmQaBv5w=; b=PZkR5sjFzBP8iiZhj2DnRHdiwZ d2OOkZb6AqsaZ9KIHzW0hGQ/ApDfyxCLnlVqNSc6QIFKnmJzZFWLat/HCKRf89U+ggjb/Q/On+l+m +1wQeSoFs7YDKKS5I22HNnHv2A4crZmt83Is3dKz1SvcEYXG3GfbIAlXR46++3a4IMX7diOb5Li8o yWtt3PK9IiXw2mxIh4K+EhJ4SbWwxgIyEDae4lay3Eq4sx5cDUq5dhTpYC1qgd7eLc8mRmk72U6Yb e/w73qd/VUIeE9FU1MK6efT0V71stT4MFNPALqWlJeiXCiYqdvJdkoucMcInPZhttoAiUNNwiCsf8 WrLRWWMA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1llY5P-005eWy-FY; Tue, 25 May 2021 14:31:11 +0000 Received: from mail-vi1eur05on2066.outbound.protection.outlook.com ([40.107.21.66] helo=EUR05-VI1-obe.outbound.protection.outlook.com) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1llY5J-005eUZ-8m for linux-arm-kernel@lists.infradead.org; Tue, 25 May 2021 14:31:09 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EW5Idby43HNs/gBWrm+N+wtxn7Sv6zhD+JY6sBmLh/Vx9O6lcYXH2l59odKwaOhYMUi4MFfvez6YD9sMvn1bYatwenVEED60n8b9KTcQMbeC9mogWA2N1/llRCz7lvLdHKufXQ4Sq/r54rroK2lthiIB5x+pwsnlz5Dhm0Mni4spoFqVQFGXpnDIBFL//z6eGhmM7U3F7PyiJ07NOFt39dvE7FjVhpJuA/rWgYRBVSj0e58xhdI+5W3q4f42xCiGud44FNmnEgpFAuIcn0/7NtGkaCJoaAs1N2X9xzK0b8cdIoPeN39mvGIykgHKO1PEH23mPkThJHc15yrHfgPjfw== 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-SenderADCheck; bh=sR9afKVwTbK8naoFIDs6z22aPwYEseJ2N3THqUQIopA=; b=R0MrDsju4qwPigZO0GGPOujh5VyNRTeW5ychYHa5yhz6gr39arn69gipWIUUJ69vNdHyDNcEgMzEk/iyhxzxs8QosB5XPT72Bj4HYWZAPi0wxukvXxdceww4txxylgl/zh22km5iHon/8zvxaEQVI18pcSdAwLcGo+55hbBo8EsFcNavsco12FJAWmpxXmyRunU2ruVSR67W7CwmTmt3LzI/0XEKrO4gkOFyZc5dag3DciwuRoYHvBRSi8vmVvPOx4iLFi4W5Egt4MwDfqlLW/tL4rKNU0Ayi9+1GdK4T8i9MRRB2wuwAUfmv9HCwbouvsueX8zU+jfcUUR4G7o3kQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=seco.com; dmarc=pass action=none header.from=seco.com; dkim=pass header.d=seco.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=secospa.onmicrosoft.com; s=selector2-secospa-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sR9afKVwTbK8naoFIDs6z22aPwYEseJ2N3THqUQIopA=; b=kKliBdr0+9TEicVmHuc9LpYPdfCXaA6SZErAoZFEimmTm3NI7+wmenGL3XaR8qs24DYFrImmdaPr1Mb3kdI9e1YPjXJqo824bToJuQlFjFEz2XtSz/Hmhtu2dkzbSF760WDPvur63l7gEZKpp0cKhj7gzLDHMpqnTz6ZxLHWy9k= Authentication-Results: pengutronix.de; dkim=none (message not signed) header.d=none;pengutronix.de; dmarc=none action=none header.from=seco.com; Received: from DB7PR03MB4523.eurprd03.prod.outlook.com (2603:10a6:10:19::27) by DBBPR03MB7146.eurprd03.prod.outlook.com (2603:10a6:10:209::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4150.25; Tue, 25 May 2021 14:31:00 +0000 Received: from DB7PR03MB4523.eurprd03.prod.outlook.com ([fe80::40d5:3554:c709:6b1b]) by DB7PR03MB4523.eurprd03.prod.outlook.com ([fe80::40d5:3554:c709:6b1b%5]) with mapi id 15.20.4150.027; Tue, 25 May 2021 14:31:00 +0000 Subject: Re: [PATCH v3 2/2] clocksource: Add support for Xilinx AXI Timer To: =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= , Michal Simek Cc: linux-pwm@vger.kernel.org, devicetree@vger.kernel.org, Rob Herring , Thierry Reding , Alvaro Gamez , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Daniel Lezcano , Lee Jones , Thomas Gleixner , kernel@pengutronix.de References: <20210511191239.774570-1-sean.anderson@seco.com> <20210511191239.774570-2-sean.anderson@seco.com> <5f960034-174d-0ed8-9f52-3d5fde90e16a@seco.com> <9f227f96-a310-0fbd-fd34-91eb386306b9@xilinx.com> <7a06cf46-0f85-1edb-ca08-abd7b2543ad9@seco.com> <41542760-3967-4f9a-0f0c-1206e03ff494@xilinx.com> <2296d4e5-717a-0470-d487-e0924cf6c076@xilinx.com> <20210525061131.omrbcdewf4z75ib7@pengutronix.de> From: Sean Anderson Message-ID: Date: Tue, 25 May 2021 10:30:55 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: <20210525061131.omrbcdewf4z75ib7@pengutronix.de> Content-Language: en-US X-Originating-IP: [50.195.82.171] X-ClientProxiedBy: MN2PR16CA0062.namprd16.prod.outlook.com (2603:10b6:208:234::31) To DB7PR03MB4523.eurprd03.prod.outlook.com (2603:10a6:10:19::27) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [172.27.1.65] (50.195.82.171) by MN2PR16CA0062.namprd16.prod.outlook.com (2603:10b6:208:234::31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4173.20 via Frontend Transport; Tue, 25 May 2021 14:30:59 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: bde0516e-3c78-4049-c520-08d91f89b7d1 X-MS-TrafficTypeDiagnostic: DBBPR03MB7146: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: upy1CH/v403WQp+wSzaPmql0Moy4AzVRf4VuCt5w5LkqCkrnnkEcJHb//Zvnwk0VJTN/irJQo7eSBCgcTlh7/DyDHjsdPKYGlbGi3TBfbfT0RhzXNA3p2Qq+jlq/i/h5FHbra4Zq7oBFVFYCS6t9puvgXqGO9lTv2auTvDBVYKRzLb6UAD2FYm50RBqLlopxe+/sCRk4Ha9V5pkVPZiyJsCMc8aaFCqqMoLYnbX3plw9Vq/jNs3UYSxJaK6VWERA0pTnDIxhUMDpLxo2xBSVSmYnI9P+d0PgiBb3m1zS7UJ4MpjiedMgX3nivz9VjanC2q90A6CPMnuiSWnzi0oTWHs9FbYR6MzbaoTF2shyfd02/ATn5O4boP8Ro4lBTWkfgPs0ravBnAlRtlKA9atpl8V+kjIYeytl0sC8DYG3maOeA6+U4iEP21SRPtqIZYb99pWA5vax4qPeCJ8KRLGoyewtYNNCRMLPnn6LIx/tz0XVbeSieN8QWEqgVyVmcWs7yoXZeOzcA3ePY6eTzNvsPJelVIrTJCbblYxuMGdv4hOl7iTao9apirDKhCBpDDq5JdhgMxiULcbLNm1SLOjeEiAIcxA34Mmq39vLgw10WHsNzCJEyt1RQfga7Vy71dA7sq+Dj712TeeLYCgKbb7PTWlKlK9rNp6WY+M95nfBEQXbPDW9fXmhT/rSHEfHodCDbfqll4Mz9q45iH0RaSgSsxh99Wip7qzRF6nwTXVGFhI= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DB7PR03MB4523.eurprd03.prod.outlook.com; PTR:; CAT:NONE; SFS:(376002)(136003)(396003)(346002)(366004)(39840400004)(83380400001)(2906002)(110136005)(38350700002)(38100700002)(52116002)(16526019)(8676002)(186003)(6486002)(316002)(54906003)(36756003)(66574015)(6666004)(16576012)(86362001)(5660300002)(66476007)(53546011)(31696002)(2616005)(66946007)(478600001)(26005)(31686004)(8936002)(956004)(7416002)(4326008)(44832011)(66556008)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?Windows-1252?Q?8JwVfejOQBDpxKsiX6WtbIh2A3wpMP8ov6RI0CbTEVfhL/953mtyP91V?= =?Windows-1252?Q?XTx0Sp0X65PpHv04tcYfOwUCPLdRMt5iouZ4ey70dYGVR87lqHkc2+5n?= =?Windows-1252?Q?ie+3l+wh3VkPIKDLx0+PBmLa5ZU0/huyGYtDWzc8bcJoWeLU07EgkGNV?= =?Windows-1252?Q?Ze0LNyAgngA4E7pYWoFr8JxfKn8FgRsEZOJeWOUPB1tx+J8/XZ8Q+HPj?= =?Windows-1252?Q?E1GHe811kYMyJYt+ZKtAP17RSbDfLk7SB8iLv+SluYkYTYPQenAPDDmO?= =?Windows-1252?Q?F/Cs1kIe/wv8nQQLaK+eIecH62XAaau2jiXL0I53kjm7ip3r2Gp4Cwz4?= =?Windows-1252?Q?Buc6MTSlQs/F2+IT28dFMNuEVZv0KFKLc4zfEVzx6aV8kNYD8bdGge2S?= =?Windows-1252?Q?m1YOrF6vRhHLl767zQgqlTZkXT4Nyz5dt7tZRDBuNkP5Uiw3AibpJ9m4?= =?Windows-1252?Q?8xmbbFWCwOizEcU1shL4Y70lmf1dVtSprwfZ7zK+V0aCOAAmhgUDUSFf?= =?Windows-1252?Q?RoDYT0Lc8ULt3yyTk5E7qQt5FPqAEnPhHP4xW2UMeCsmrM0ORKX5pFJ3?= =?Windows-1252?Q?tS2jYpUUuUDFG1oZ81TK+s9Zuq9NfkyRE3IAoi5JXPHfXjTHJJLQlbvU?= =?Windows-1252?Q?aA0sP1tCiEIUkJLwvaN3XHy82O53KNcvo6mTIGKaE4g0MlDQDb0i2QZQ?= =?Windows-1252?Q?sbeye7fmUXtC5UleJEBtov8fRnfCwiXKx4+S9+n4gdxEBhGHkDgPt06w?= =?Windows-1252?Q?yB7JBUZrMATHCSD609TMuu83uywaKKVuHSAEHvxeGAbCpyr/oVd2djyh?= =?Windows-1252?Q?3kLIhqVckNjBAuLYdcf3SVjnjHgNZDybir9KA6aeSGmP37HG14ajsOjJ?= =?Windows-1252?Q?oMpp7zJ4MvX4vQW9HBMFPKKmyErOtHYwjUNqTfloUFUXMnDbYa0D5z4R?= =?Windows-1252?Q?QLsBtw71RsqT0aUOu6BLv2CnEjxs+klLvdtk5pIistFJ6ukTTVEAbMo8?= =?Windows-1252?Q?NqDT8W/yAbAAsPJ1y4AWSkUiPjgzOSIKwH6SjpWQUfreaBl5CvOtMfr/?= =?Windows-1252?Q?6LBmjJ8gUjJNcRbIbBNIbOfJJlvTQLLkrYUnmZaiwK4jdRve6BNB8l75?= =?Windows-1252?Q?rsGf16GkRdBEVM0URRwCWPOubDC7Ij9zz8nYd+xyX2XYeLBFSDgqoDOv?= =?Windows-1252?Q?iS6nsOEPOnOarJlThzSKi+wAcvD07qijscIxTEJC1bObThI9lncXxko+?= =?Windows-1252?Q?R+qXCmPZXPd796ggtZMHFacBxcUP/Jde+9Uz190/wjpcVu+HCdRjcx0D?= =?Windows-1252?Q?MamUhqZSs1mOKc3CcL4G/mGgdK6jz9qg36lb1K36PZqqETMphhAfXLav?= =?Windows-1252?Q?0i1kOnc/py2IFgi2KELgs7NfSRETf6eb63p3Mm5EFaR+puINZ5aScKzW?= X-OriginatorOrg: seco.com X-MS-Exchange-CrossTenant-Network-Message-Id: bde0516e-3c78-4049-c520-08d91f89b7d1 X-MS-Exchange-CrossTenant-AuthSource: DB7PR03MB4523.eurprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 May 2021 14:31:00.5443 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: bebe97c3-6438-442e-ade3-ff17aa50e733 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: ApJSrHOOn9aWT38fSBRNlWlzJYjZUP4VQLpQQgw8bo4kxgq9rG1/mvtCOayeEWsHjuItMpeWB5YOXfvnhiY1Sg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBBPR03MB7146 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210525_073105_336175_1089F0E2 X-CRM114-Status: GOOD ( 41.57 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="windows-1252"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 5/25/21 2:11 AM, Uwe Kleine-K=F6nig wrote: > Hello Sean, hello Michal, > > On Mon, May 24, 2021 at 09:00:51AM +0200, Michal Simek wrote: >> On 5/20/21 10:13 PM, Sean Anderson wrote: >>> On 5/19/21 3:24 AM, Michal Simek wrote: >>>> On 5/18/21 12:15 AM, Sean Anderson wrote: >>>>> This could be deprecated, but cannot be removed since existing device >>>>> trees (e.g. qemu) have neither clocks nor clock-frequency properties. >>>> >>>> Rob: Do we have any obligation to keep properties for other projects? > > If a binding is in the wild and used to be documented, it has to stay. > >>>>>> 4. Make driver as module >>>>>> 5. Do whatever changes you want before adding pwm support >>>>>> 6. Extend DT binding doc for PWM support >>>>>> 7. Add PWM support >>>>> >>>>> Frankly, I am inclined to just leave the microblaze timer as-is. The= PWM >>>>> driver is completely independent. I have already put too much effort= into >>>>> this driver, and I don't have the energy to continue working on the >>>>> microblaze timer. >>>> >>>> I understand. I am actually using axi timer as pwm driver in one of my >>>> project but never had time to upstream it because of couple of steps = above. >>>> We need to do it right based on steps listed above. If this is too mu= ch >>>> work it will have to wait. I will NACK all attempts to add separate >>>> driver for IP which we already support in the tree. >>> >>> 1. Many timers have separate clocksource and PWM drivers. E.g. samsung, >>> renesas TPU, etc. It is completely reasonable to keep separate >>> drivers for these purposes. There is no Linux requirement that each >>> device have only one driver, especially if it has multiple functio= ns >>> or ways to be configured. >> >> It doesn't mean that it was done properly and correctly. Code >> duplication is bad all the time. > > IMHO it's not so much about code duplication. Yes, code duplication is > bad and should be prevented if possible. But it's more important to not > introduce surprises. So I think it should be obvious from reading the > device tree source which timer is used to provide the PWM. I don't care > much if this is from an extra property (like xilinx,provide-pwm), > overriding the compatible or some other explicit mechanism. IIUC in this > suggested patch the selection is implicit and so this isn't so nice. In this patch, the selection is by the presence of the xlnx,pwm property. In the next revision, this will be changed to be the presence of #pwm-cells (by the request of Rob). >>> 2. If you want to do work on a driver, I'm all for it. However, if you >>> have not yet submitted that work to the list, you should not gate >>> other work behind it. Saying that X feature must be gated behind Y >>> *even if X works completely independently of Y* is just stifling >>> development. >> >> I gave you guidance how I think this should be done. I am not gating you >> from this work. Your patch is not working on Microblaze arch which is >> what I maintain. And I don't want to go the route that we will have two >> drivers for the same IP without integration. We were there in past and >> it is just pain. >> I am expecting that PWM guys will guide how this should be done >> properly. I haven't heard any guidance on this yet. >> Thierry/Uwe: Any comment? > > Not sure I can and want to provide guidance here. This is not Perl, but > still TIMTOWTDI. If it was me who cared here, I'd look into the > auxiliary bus (Documentation/driver-api/auxiliary_bus.rst) to check if > it can help to solve this problem. I don't think this is the correct solution here. > A key requirement for utilizing the auxiliary bus is that there is no > dependency on a physical bus, device, register accesses or regmap > support. Since both PWM and timer drivers need register access, we cannot use the auxiliary bus here. Further, timers must be initialized very early during boot, before we even have devices, and cannot be unregistered. Because of this, it only makes sense to bind one driver to the device. > >>> 3. There is a clear desire for a PWM driver for this device. You, I, a= nd >>> Alvaro have all written separate drivers for this device because we >>> want to use it as a PWM. By preventing merging this driver, you are >>> encouraging duplicate effort by the next person who wants to use t= his >>> device as a PWM, and sees that there is no driver in the tree. >> >> We should do it cleanly that it will be easy to maintain which is not by >> creating two separate drivers or by switching to completely new driver. > > +1 Ok, then you would like me to continue my current approach where both drivers live in the same file? --Sean > > Best regards > Uwe > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel