Received-SPF: pass (google.com: domain of linux-kernel+bounces-158849-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 25 Apr 2024 16:56:39 +0100
From: Conor Dooley <conor@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Guenter Roeck <linux@roeck-us.net>,
	Chanh Nguyen <chanh@amperemail.onmicrosoft.com>,
	Chanh Nguyen <chanh@os.amperecomputing.com>,
	Jean Delvare <jdelvare@suse.com>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Justin Ledford <justinledford@google.com>,
	devicetree@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	OpenBMC Maillist <openbmc@lists.ozlabs.org>,
	Open Source Submission <patches@amperecomputing.com>,
	Phong Vo <phong@os.amperecomputing.com>,
	Thang Nguyen <thang@os.amperecomputing.com>,
	Quan Nguyen <quan@os.amperecomputing.com>
Subject: Re: [PATCH v2 3/3] dt-bindings: hwmon: max31790: Add
 maxim,pwmout-pin-as-tach-input property
Message-ID: <20240425-enigmatic-portly-c2282fc1709a@spud>
References: <20240414042246.8681-1-chanh@os.amperecomputing.com>
 <20240414042246.8681-4-chanh@os.amperecomputing.com>
 <13b195e6-cbbd-4f74-a6fa-d874cb4aaa45@linaro.org>
 <065243cc-09cf-4087-8842-bd4394fb324f@amperemail.onmicrosoft.com>
 <d549cf2b-a7fa-4644-8fcb-3c420503ee01@amperemail.onmicrosoft.com>
 <20240423-gallantly-slurp-24adbfbd6f09@spud>
 <ab5cfd8c-0e88-4194-a77e-5ffbb6890319@amperemail.onmicrosoft.com>
 <396b47f5-9604-44ab-881f-94d0664bcab8@roeck-us.net>
 <b79b5323-196f-41bc-b47a-d350c49d769a@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="nEP+ljWS/1R7JDAt"
Content-Disposition: inline
In-Reply-To: <b79b5323-196f-41bc-b47a-d350c49d769a@linaro.org>


--nEP+ljWS/1R7JDAt
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 25, 2024 at 05:46:02PM +0200, Krzysztof Kozlowski wrote:
> On 25/04/2024 16:05, Guenter Roeck wrote:
> > On 4/25/24 03:33, Chanh Nguyen wrote:
> >>
> >>
> >> On 24/04/2024 00:02, Conor Dooley wrote:
> >>> [EXTERNAL EMAIL NOTICE: This email originated from an external sender=
=2E Please be mindful of safe email handling and proprietary information pr=
otection practices.]
> >>>
> >>
> >=20
> > The quote doesn't make much sense.

The reply is to me questioning part of their earlier reply:
	> I think a vendor property is suitable for this purpose. It is only avail=
able
	> on specific MAX31790 fan controllers and not on other fan controller
	> devices. So I think we can't use the "tach-ch" in fan-common.yaml.

	Can you explain why you think that only MAX31790 fan controllers are
	capable of an arbitrary mapping of PWM outputs to TACH inputs?

> >=20
> >> Sorry Conor, there may be confusion here. I mean the mapping of the PW=
M output to the TACH input, which is on the MAX31790, and it is not sure a =
common feature on all fan controllers.
> >>
> >=20
> > I think the term "mapping" is a bit confusing here.
> >=20
> > tach-ch, as I understand it, is supposed to associate a tachometer input
> > with a pwm output, meaning the fan speed measured with the tachometer i=
nput
> > is expected to change if the pwm output changes.
> >=20
> > On MAX31790, it is possible to configure a pwm output pin as tachometer=
 input pin.
> > That is something completely different. Also, the association is fixed.
> > If the first pwm channel is used as tachometer channel, it would show u=
p as 7th
> > tachometer channel. If the 6th pwm channel is configured to be used as =
tachometer
> > input, it would show up as 12th tachometer channel.
> >=20
> > Overall, the total number of channels on MAX31790 is always 12. 6 of th=
em
> > are always tachometer inputs, the others can be configured to either be=
 a
> > pwm output or a tachometer input.
> >

> > pwm outputs on MAX31790 are always tied to the matching tachometer inpu=
ts
> > (pwm1 <--> tach1 etc) and can not be reconfigured, meaning tach-ch for
> > channel X would always be X.

Are they? Granted, I probably didn't read this as well as you did, but
figure 3, 4 5 & 6 seem to show mappings that are not 1:1, with PWMOUT1
controlling several fans, each of which report back via a different tach
channel. I think the fan controller binding accounts for this though:
the child nodes would reference the same PWM provider but have different
tach-ch values.

> >=20
> >> I would like to open a discussion about whether we should use the tach=
-ch property on the fan-common.yaml
> >>
> >> I'm looking forward to hearing comments from everyone. For me, both ta=
ch-ch and vendor property are good.
> >>
> >=20
> > I am not even sure how to define tach-ch to mean "use the pwm output pin
> > associated with this tachometer input channel not as pwm output
> > but as tachometer input". That would be a boolean, not a number.
>=20
> Thanks for explanation. So this is basically pin controller function
> choice - kind of output or input, although not in terms of GPIO.
>=20

> Shouldn't we have then fan children which will be consumers of PWMs?

In this case, the max31790 has the PWM hardware, so it would be the
provider...

> Having a consumer makes pin PWM output. Then tach-ch says which pins are
> tachometer for given fan? Just like aspeed,g6-pwm-tach.yaml has?

=2E..and it seems to me like there should be several fan child nodes as in
the aspeed binding you mention here that are the consumers. The binding
as written seems to only support a single fan.

--nEP+ljWS/1R7JDAt
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZip9NwAKCRB4tDGHoIJi
0vP1AP4j4+fuNIvolurSpFQaXNehLyhR+uSnou0/9LHPJ1mr9AEA/znEjxTUZQ7d
vWL7Gzplb2kPTy1lhPVh5m0EXqhERws=
=1b3W
-----END PGP SIGNATURE-----

--nEP+ljWS/1R7JDAt--