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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 2DEAEEB64DA for ; Wed, 19 Jul 2023 06:48:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Reply-To:Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=i8ScEDGLTmS9KmY2YKINNds/bJbxgmzvyHWwPXMwmuc=; b=3UIpixqujoSprarnjahdxMBV0r iz1G8HSXQCXmF96sHKZyJtP9LPVsXX98qsmz9tniyB11JaArQXXg+oy1S/qEiyl1U2tGjRpKhUfbz kbxI7fWhTrjiG7vwCRe1ZazumruRfAJeiVho1D57diuannsHllBiELHLoFdz7gf9OGBie6yNa1zYA amnbN62I6YsG+tu8mbdgC+NsmpMapalZhJU4rHQZIW8LL7Ho+ZTiYBxrjt23l1F/haxRp1SpJFBZI yPgPu4os0zAGf77hl3b7J8joysvvJ5AFlawYiQGRzTJYHteLZLB747FEhh/eH7bkgKELgCV6hg9SB Z2qUaotQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qM0z6-005uGi-2Z; Wed, 19 Jul 2023 06:48:28 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qM0z0-005uBx-1c for linux-arm-kernel@bombadil.infradead.org; Wed, 19 Jul 2023 06:48:22 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=NY8mkv6Tta1o2JgKQkYOxaVAYeF+Y1zLs9QhRZtEEvw=; b=Tu+cD5rMsZ5DZc4V5NXDjBHvFt V87Y7Tx6yB0tpKKa31ee7EVBmR5fDEVQLRwU0wCaxhSf75FJHFHSl3FqviWNn8/5K5iPJKU3ObkRD bw6OOb+7UMV+KFtbBP2x4ZIpkZ09WR2YBRtn2TIY2+FYkyJHbc8Uk20tQoRxvg2Yedo737MS+KeMX kzkalzTLjsZgZGTs+Aj+uLj3rmpiHzwJblWnyzvcGJGQLIDwSXGWHja/akvXCZw+0Q/223o3RRmdu APFkbp7KnxWQuE8GO67oiUwQgPn+H2mR0HWS6LIXARD0oyf5Rmkm4CIO0QJ9Z6MtJyb5+Lu27DWG8 UjwD+jsw==; Received: from mail-ed1-x536.google.com ([2a00:1450:4864:20::536]) by desiato.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qLeNX-00BDjS-0N for linux-arm-kernel@lists.infradead.org; Tue, 18 Jul 2023 06:40:13 +0000 Received: by mail-ed1-x536.google.com with SMTP id 4fb4d7f45d1cf-51e344efd75so10990256a12.1 for ; Mon, 17 Jul 2023 23:40:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689662348; x=1692254348; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=NY8mkv6Tta1o2JgKQkYOxaVAYeF+Y1zLs9QhRZtEEvw=; b=HsOec0qR701vzASUVqbK11uHMBCwGeIExVLBVHU5+h4BjmddWeB1qsTVF7yqqq0MH5 Ixr2rqm+zONxKZccsz5E3FfhNQuaMGFZJTSB/baeawOPcoSU+MKAdnEknXHeuupQUmMg /ssuOWL8Wc5M0EivzeeVsUHiKT9267nAmkNH/I88c3qXeti5w8SBiI9yflzbTTAe71fC ewBssw06qSHUGOBtabcgtz/7zYRLHf3h45S12ikr2x1/CHiTTT7YQ2g9D+BcGfY9vMZe bjZAqnM/HEUSim75nfYErmn6yrquhREMuN6LiC71St6JRE7ArRdKgAla2iCJxg17AW/y BxLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689662348; x=1692254348; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=NY8mkv6Tta1o2JgKQkYOxaVAYeF+Y1zLs9QhRZtEEvw=; b=PWj1MXFNTQIqaFO/bLKjpPEp2W+erqqvlKgk0ksvxhB8nDzQm1jOS+/G6JghL1VE++ LIC9AsoHe7aJzYmpFK7Po7nx6jxV15f89VQBcwPz2hnWuV5D+Hh4fWZbMr23JvplfhCp oPigVE7/v8fqcVE4zFY1rBszdHxosmWF518Dxi7seQc4Y2P6xVKDuNevHPPQts7+2FLd aFpPEvF6JRxb3tdeSRN00QRmyOQ9JR1RTcntzLClKKyMxpfNYz6Qgxch4ahO9XNtADdT llpdlxpNbiP44xojVDxnV5vSAiXpIaVgBtfJP1BSz6KkJRVwhfmHbLKjQd+hFCVBs11w m6HA== X-Gm-Message-State: ABy/qLYciUwiBuIobRcCW7ImLY984AUsCFqsm6aQijGexhgBrBiGOipl 8Q5RnloFefsGH1cdW3mwNzo= X-Google-Smtp-Source: APBJJlHyvyAxzWMmSdDheZfwLaIUsEBdPq35pJycwdjBr8gDFrb6A4b9u+g2oZMTR33oVOKD6tDFYw== X-Received: by 2002:a05:6402:499:b0:51e:1c5c:b97f with SMTP id k25-20020a056402049900b0051e1c5cb97fmr14919584edv.2.1689662347332; Mon, 17 Jul 2023 23:39:07 -0700 (PDT) Received: from orome (p200300e41f4b7100f22f74fffe1f3a53.dip0.t-ipconnect.de. [2003:e4:1f4b:7100:f22f:74ff:fe1f:3a53]) by smtp.gmail.com with ESMTPSA id d25-20020aa7d5d9000000b0051e0f100c48sm724877eds.22.2023.07.17.23.39.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 17 Jul 2023 23:39:06 -0700 (PDT) Date: Tue, 18 Jul 2023 08:39:05 +0200 From: Thierry Reding To: =?utf-8?B?6JSh5om/6YGU?= Cc: Krzysztof Kozlowski , Guenter Roeck , "jdelvare@suse.com" , "robh+dt@kernel.org" , "krzysztof.kozlowski+dt@linaro.org" , "joel@jms.id.au" , "andrew@aj.id.au" , "u.kleine-koenig@pengutronix.de" , "corbet@lwn.net" , "p.zabel@pengutronix.de" , "linux-hwmon@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-aspeed@lists.ozlabs.org" , "linux-kernel@vger.kernel.org" , "linux-pwm@vger.kernel.org" , "linux-doc@vger.kernel.org" , "patrick@stwcx.xyz" , Billy Tsai Subject: Re: [v6 2/4] dt-bindings: hwmon: Add ASPEED TACH Control documentation Message-ID: References: <0ba3767c-d481-6e2c-2d32-b79af0e1efd8@roeck-us.net> <7b198d57-ddec-3074-314a-3e5e5b8f48f9@roeck-us.net> <7a69bda1-5f4c-5b1f-8eb6-6fd58917a9b1@roeck-us.net> <3756dffd-1407-d656-485a-9cf1eefd9ae1@linaro.org> MIME-Version: 1.0 In-Reply-To: <3756dffd-1407-d656-485a-9cf1eefd9ae1@linaro.org> User-Agent: Mutt/2.2.10 (2023-03-25) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230718_074011_339680_390909CE X-CRM114-Status: GOOD ( 47.83 ) 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-Type: multipart/mixed; boundary="===============7620644431247644912==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============7620644431247644912== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="6M5dd0U91g62xbzr" Content-Disposition: inline --6M5dd0U91g62xbzr Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jul 18, 2023 at 08:04:24AM +0200, Krzysztof Kozlowski wrote: > On 18/07/2023 06:01, =E8=94=A1=E6=89=BF=E9=81=94 wrote: > >> > >> On 17/07/2023 11:01, =E8=94=A1=E6=89=BF=E9=81=94 wrote: > >>> Guenter Roeck =E6=96=BC 2023=E5=B9=B47=E6=9C=881= 7=E6=97=A5 =E9=80=B1=E4=B8=80 =E4=B8=8A=E5=8D=881:00=E5=AF=AB=E9=81=93=EF= =BC=9A > >>>> > >>>> On 7/16/23 09:08, Krzysztof Kozlowski wrote: > >>>> > >>>> [ ... ] > >>>> > >>>>>> > >>>>>> This patch serial doesn't use to binding the fan control h/w. It is > >>>>>> used to binding the two independent h/w blocks. > >>>>>> One is used to provide pwm output and another is used to monitor t= he > >>>>>> speed of the input. > >>>>>> My patch is used to point out that the pwm and the tach is the > >>>>>> different function and don't need to > >>>>>> bind together. You can not only combine them as the fan usage but = also > >>>>>> treat them as the individual module for > >>>>>> use. For example: the pwm can use to be the beeper (pwm-beeper.c),= the > >>>>>> tach can be used to monitor the heart beat signal. > >>>>> > >>>>> Isn't this exactly the same as in every other SoC? PWMs can be used= in > >>>>> different ways? > >>>>> > >>>> > >>>> ... and in every fan controller. Not that it really makes sense beca= use > >>>> normally the pwm controller part of such chips is tied to the fan in= put, > >>>> to enable automatic fan control, but it is technically possible. > >>>> In many cases this is also the case in SoCs, for example, in ast2500. > >>>> Apparently this was redesigned in ast2600 where they two blocks are > >>>> only lightly coupled (there are two pwm status bits in the fan status > >>>> register, but I have no idea what those mean). If the blocks are tig= htly > >>>> coupled, separate drivers don't really make sense. > >>>> > >>>> There are multiple ways to separate the pwm controller part from the > >>>> fan inputs if that is really necessary. One would be to provide a > >>>> sequence of address mappings, the other would be to pass the memory > >>>> region from an mfd driver. It is not necessary to have N instances > >>>> of the fan controller, even if the address space is not continuous. > >>>> > >>> > >>> Hi Guenter, > >>> > >>> May I ask about the meaning of the sequence of address mappings? It a= ppears > >>> to consist of multiple tuples within the 'reg' property, indicating > >>> the usage of PWM/Tach > >>> registers within a single instance. After that I can use the dts like= following: > >>> > >>> pwm: pwm@1e610000 { > >>> ... > >>> reg =3D <0x1e610000 0x8 > >>> 0x1e610010 0x8 > >>> 0x1e610020 0x8 > >>> 0x1e610030 0x8 > >>> 0x1e610040 0x8 > >>> 0x1e610050 0x8 > >>> 0x1e610060 0x8 > >>> 0x1e610070 0x8 > >>> 0x1e610080 0x8 > >>> 0x1e610090 0x8 > >>> 0x1e6100A0 0x8 > >>> 0x1e6100B0 0x8 > >>> 0x1e6100C0 0x8 > >>> 0x1e6100D0 0x8 > >>> 0x1e6100E0 0x8 > >>> 0x1e6100F0 0x8>; > >> > >> > >> Uh, no... I mean, why? We keep pointing out that this should not be do= ne > >> differently than any other SoC. Open any other SoC PWM controller and > >> tell me why this is different? Why this cannot be one address space? > >=20 > > Hi Krzysztof, > >=20 > > This is because the register layout for PWM and Tach is not continuous. > > Each PWM/Tach instance has its own set of controller registers, and they > > are independent of each other. >=20 > Register layout is not continuous in many other devices, so again - why > this must be different? >=20 > >=20 > > For example: > > PWM0 uses registers 0x0 and 0x4, while Tach0 uses registers 0x8 and 0xc. > > PWM1 uses registers 0x10 and 0x14, while Tach1 uses registers 0x18 and = 0x1c. > > ... > >=20 > > To separate the PWM controller part from the fan inputs, Guenter has > > provided two methods. > > The first method involves passing the memory region from an MFD > > driver, which was the >=20 > I have no clue how can you pass memory region > (Documentation/devicetree/bindings/reserved-memory/) from MFD and why > does it make sense here. >=20 > > initial method I intended to use. However, it seems that this method > > does not make sense to you. > >=20 > > Therefore, I would like to explore the second method suggested by > > Guenter, which involves providing > > a sequence of address mappings. At the risk of saying what others have said: given that there's a single reset line and a single clock line controlling all of these channels and given what I recall of how address demuxers work in chips, everything indicates that this is a single hardware block/device. So the way that this should be described in DT is: pwm@1e610000 { reg =3D <0x1e610000 0x100>; clocks =3D ...; resets =3D ... }; That'd be the most accurate representation of this hardware in DT. It is then up to the driver to expose this in any way you see fit. For Linux it may make sense to expose this as 16 PWM channels and 16 hardware monitoring devices. Other operating systems using the same DT may choose to expose this differently, depending on their frameworks, etc. A simple operating system may not expose this as separate resources at all but instead directly program individual registers from this block. I'd also like to add that I think trying to split this up into multiple drivers in Linux is a bit overkill. In my opinion, though I know not everyone shares this view, it's perfectly fine for one driver to expose multiple types of resources. There's plenty of use-cases across the kernel where tightly coupled devices like this have a single driver that registers with multiple subsystems. Going through MFD only because this particular hardware doesn't split registers nicely along Linux subsystem boundaries. So FWIW, I'm fine carrying hwmon code in a PWM driver and I'm equally fine if PWM code ends up in a hwmon driver (or any other subsystem really) if that makes sense for a given hardware. Thierry --6M5dd0U91g62xbzr Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmS2M4UACgkQ3SOs138+ s6E3Ww//aHUH2t/BbrFylE5Y9kU3zPSvSNLVmsRJQ2LmD5P/cCBBqa3Yaala1Rkf 309YuV8mh0tbmDYZZKBpnHsTyGaa/WpVymOS8LP2n2ugzZWdejoO5XNazB1pvXiY pTnErP2kiN8yvaVhD5leVgS6YzTF3kkvrtmeQy8s8RecYMpEK0OUOFyTsB+xTWoS gtSwD7sNTGJvxshf71/Zif3EAn49j8BZzzY967j+Y2UU+tsfiH+9IKU9ErOBDvHr sWNyrZCrxeGY19DJOrci+zmsSn7VV73Ls0+D14J/kJJunvEl5vXWZvWX3sTpaalB AETPPmAT3fvvE0nWvG+Aczv8eK5B5vAlt9KYEe/IatMP/vtgPS6fmyY0Zlpw3+S2 qL8cdM6DdJ2sOo0xuOvRWMzpuAvzI/kqM8+3ekm7EZ8YEI3AzjQt+yKOo8R41Ngu w87tmAS7FKGU8azN3i4BZ9CbGhfovJBouAhpGip9C4+7xemIBhOWQd4+5UAagDlK dzEDyCCd1WekvHQxIKwCa5D1WySRGo8sKlr7uPlVAdYCNC/E6eB6cv0iTljzlg52 p42NKSy4V8xeV5OvoimXeu2/iD3cBqQ1tn586TqhR9WM3NnzKzQ10rCKIOpBgaEj EfblswZmP4VH2xT9tEZTTx8mGm/EsmNUzopsGsuoOFmRXEYGh/I= =iKtB -----END PGP SIGNATURE----- --6M5dd0U91g62xbzr-- --===============7620644431247644912== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============7620644431247644912==--