Linux-Hwmon Archive on lore.kernel.org
 help / color / Atom feed
From: Reimundo Heluani <heluani@potuz.net>
To: linux-hwmon@vger.kernel.org
Subject: Re: [RFC] hwmon: add support for IT8686E to it87.c
Date: Wed, 11 Dec 2019 08:34:46 -0300
Message-ID: <20191211113446.GA1084863@vertex> (raw)
In-Reply-To: <CALUKdZ_fU8r6AjKU-RTLS9a+iXDsYZrp6yYN+texpo12JeFt6w@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 7706 bytes --]

On Dec 02, Corey Ashford wrote:
>On Mon, Dec 2, 2019 at 3:09 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On Mon, Dec 02, 2019 at 02:33:27PM -0800, Corey Ashford wrote:
>> > On Mon, Dec 2, 2019 at 9:52 AM Guenter Roeck <linux@roeck-us.net> wrote:
>> > >
>> > > On Mon, Dec 02, 2019 at 09:07:10AM -0800, Corey Ashford wrote:
>> > > > On Mon, Dec 2, 2019 at 6:32 AM Guenter Roeck <linux@roeck-us.net> wrote:
>> > > > >
>> > > > > On 11/29/19 8:48 PM, Corey Ashford wrote:
>> > > > > > On Fri, Nov 29, 2019 at 8:17 PM Guenter Roeck <linux@roeck-us.net> wrote:
>> > > > > >>
>> > > > > >> On 11/29/19 6:11 PM, Corey Ashford wrote:
>> > > > > >>> Hello folks.  I am running a newly-built system that uses an IT8686E
>> > > > > >>> chip.  Currently, the latest kernel from kernel.org doesn't have code
>> > > > > >>> in drivers/hwmon/it87.c to support it, however, I found some source on
>> > > > > >>> the net which has added support for quite a few more variants of that
>> > > > > >>> brand of Super I/O chip:
>> > > > > >>> https://github.com/xdarklight/hwmon-it87/blob/master/it87.c
>> > > > > >>> I tried it out by building the module and "insmod"ing it into my
>> > > > > >>> running system, and it appears to work fine.
>> > > > > >>>
>> > > > > >>> It seems the original developer had a difficult time pushing the
>> > > > > >>> changes upstream, so he abandoned the project.
>> > > > > >>>
>> > > > > >>
>> > > > > >> I abandoned the project (and dropped the driver from my github page)
>> > > > > >> because people started _demanding_ that I push the driver from github
>> > > > > >> upstream, without offering any assistance whatsoever.
>> > > > > >>
>> > > > > >>> My thought was that I could add support for just the IT8686E chip as a
>> > > > > >>> single patch, and since I can test it locally I would have a better
>> > > > > >>> chance of getting the patch accepted.  The changes to the source at
>> > > > > >>> the above git tree have quite a number of changes that aren't really
>> > > > > >>> necessary for supporting the IT8686E chip, so I think the patch could
>> > > > > >>> be pretty small, but will still credit the original author.
>> > > > > >>>
>> > > > > >>
>> > > > > >> IT8686 is a multi-page chip, meaning you'll need the entire protection
>> > > > > >> against multi-page accesses by the EC in the system. It also supports
>> > > > > >> the new temperature map. I don't think it is that simple.
>> > > > > >>
>> > > > > >> Guenter
>> > > > > >
>> > > > > > Thanks for the quick reply!
>> > > > > >
>> > > > > > When you said they didn't offer any assistance, do you mean assistance
>> > > > > > with testing?  If so, how about if the support is trimmed out for the
>> > > > > > newly-added chips that have no available test system volunteers, and
>> > > > > > then slowly add those back as people make test systems and testing
>> > > > > > time available.  Should I presume that you have access to one or more
>> > > > > > systems with the added ITnnnn chips?  I volunteer my system for
>> > > > > > testing the IT8686E support.
>> > > > > >
>> > > > >
>> > > > > Testing and, more importantly, detailed code review. No one but me has
>> > > > > seriously (if at all) scrutinized that code for years. Just picking it
>> > > > > into mainline and hope that it won't cause trouble is, by itself, troublesome.
>> > > > >
>> > > > > On top of that, the multi-page access problems are well known by board vendors
>> > > > > using this chip as well as by the chip vendor. Yet, neither board vendors nor
>> > > > > ITE talk with kernel developers. The workarounds I implemented are based on
>> > > > > information I got from one of the Windows tools developers, and are not
>> > > > > validated by any board vendor nor by ITE. Every board vendor I tried to contact
>> > > > > tells me that they don't support Linux, and I never got any reply from ITE.
>> > > > > I do know that the code causes problems on early Gigabyte board using the 8686
>> > > > > and similar multi-page chips. Just accessing the chip from Linux may cause trouble
>> > > > > because the built-in EC tries to access it as well in parallel (I suspect this
>> > > > > causes the board to reset because that access is turned off for a while by
>> > > > > the driver). This is all fine for an out-of-tree driver, but it would be
>> > > > > unacceptable in the upstream kernel.
>> > > > >
>> > > > > In summary, you'll not only need to port the code, you'll also need to establish
>> > > > > contact to ITE and/or to board vendors to ensure that the code works as intended
>> > > > > with the EC on the affected boards.
>> > > > >
>> > > > > Guenter
>> > > >
>> > > > Ah, thank you for your detailed explanation.  How you did as much as
>> > > > you did is beyond me.  ITE's web site seems to lack any usable
>> > > > information, and doesn't even list the IT8686 as one of their chips.
>> > > > Other "supported" chips don't appear to have any documentation easily
>> > > > available, other than a very generic-y description of the chip.  Quite
>> > > > an uphill battle for marginal gain.
>> > > >
>> > > Exactly. The only real recommendation I have at this time is for anyone
>> > > running Linux to stay away from boards with ITE chips.
>> > >
>> > > > Is it possible there's a way to access the sensors by using the EC as
>> > > > a proxy, rather than trying to gain direct and exclusive access to the
>> > > > sensors?  Just a thought.  I have no idea of the architecture of these
>> > > > things.  Your mention of EC was the first I had heard of it :/
>> > > >
>> > >
>> > > Not that I know of, sorry. The EC is actually running inside the Super-IO
>> > > chip(s). I have no idea if and how it is accessible from Linux. Either case,
>> > > that would be even worse, since EC programming is board vendor specific.
>> > >
>> > > Guenter
>> >
>> > Just for my clarification, it seems that what you're implying is that
>> > the embedded EC still uses the SMbus to access those paged registers,
>> > and so needs to use the same mechanism that an external device would
>> > use.  If that's true, ugh.  If it has its own private access to the
>> > entire register set in one "address space", it could bypass the paging
>> > mechanism.
>> >
>>
>> There are typically two Super-IO chips on those boards. For example,
>> Gigabyte B450 AORUS M has an IT8792 and an IT8686. The EC on one chip
>> accesses the other chip through the I2C interface. Or at least that is
>> what I think is happening... hard to be sure without board/chip vendor
>> support.
>>
I just got a new system with precisely that board B450 and after running sensors-detect I was lead to this thread. Is it safe to try your out-of-tree module on this board of will it simply not work? I can provide extensive testing for what it's worth. 

R. 

>>
>> Some of the recent chips solve the problem by memory mapping the entire
>> register space (unpaged) into memory. This way the Linux driver (and the
>> Windows driver) can access chip registers directly without having
>> to select a page. That isn't supported on the 8686, unfortunately.
>
>Ah, that makes sense now.  So I guess what's worse is that now you
>have a combination of two chips, which are not necessarily always
>paired with each other (e.g. IT8792->IT8686 vs. IT8795 [made up
>#]->IT8686). I give up :)
>
>At least the out-of-tree driver source has been working nicely for my
>machine for a couple of weeks, with no observed glitches, resets, etc.
>I'm happy.
>
>Thanks for the discussion of this little backwater area of the kernel :)
>
>- Corey

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-30  2:11 Corey Ashford
2019-11-30  4:17 ` Guenter Roeck
2019-11-30  4:48   ` Corey Ashford
2019-12-02 14:32     ` Guenter Roeck
2019-12-02 17:07       ` Corey Ashford
2019-12-02 17:52         ` Guenter Roeck
2019-12-02 22:33           ` Corey Ashford
2019-12-02 23:09             ` Guenter Roeck
2019-12-03  0:39               ` Corey Ashford
2019-12-11 11:34                 ` Reimundo Heluani [this message]
2019-12-11 17:19                   ` Guenter Roeck

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191211113446.GA1084863@vertex \
    --to=heluani@potuz.net \
    --cc=linux-hwmon@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Hwmon Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hwmon/0 linux-hwmon/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-hwmon linux-hwmon/ https://lore.kernel.org/linux-hwmon \
		linux-hwmon@vger.kernel.org
	public-inbox-index linux-hwmon

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hwmon


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git