From: Frank Crawford <frank@crawford.emu.id.au>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Jean Delvare <jdelvare@suse.com>, linux-hwmon@vger.kernel.org
Subject: Re: [PATCH v1 3/4] hwmon (it87): Test for chipset before entering configuration mode
Date: Sat, 13 Apr 2024 18:47:29 +1000 [thread overview]
Message-ID: <3e29a0238b6cfae2945495593b05c782793a3734.camel@crawford.emu.id.au> (raw)
In-Reply-To: <78ef79b3-e5de-4b73-8588-1d29ca4dcda1@roeck-us.net>
On Fri, 2024-04-12 at 05:48 -0700, Guenter Roeck wrote:
> On Fri, Apr 12, 2024 at 08:11:11PM +1000, Frank Crawford wrote:
> > >
> > > Not really. There is also the watchdog code which will happily
> > > disable
> > > SIO access after it is done, causing subsequent accesses by the
> > > hwmon
> > > driver to fail. The code also assumes that SIO access was not
> > > erroneously
> > > left enabled by some other code which we don't have any control
> > > over.
> >
> > And unfortunately if I can't do anything about it, I can only
> > ignore
> > it. If something does come up we can see what can work out at the
> > time.
> > >
> > > You assume that the hwmon driver is the only driver accessing the
> > > chip.
> > > That is a wrong assumption. I understand that the underlying
> > > problem
> > > is really that there is no SIO access infrastructure in the
> > > kernel.
> > > In the absence of such an infrastructure we can not make any
> > > assumptions
> > > about SIO access control implemented by other drivers in the
> > > kernel,
> > > and specifically can not assume that SIO access won't be disabled
> > > by
> > > other drivers just because it was enabled when the hwmon driver
> > > probe
> > > function was running.
> >
> > In this case the fact that it is the second chip may mean it will
> > not
> > come up. While I am told that the chip is fully functional with
> > non-
> > hwmon functions, but currently it does look like most of those
> > aren't
> > used. While this won't necessarily stay this way in the future, we
> > currently cannot do anything about it.
>
> This patch affects all chips, not just the second one. If any chip is
> in configuration mode when instantiating this driver, configuration
> mode
> won't be enabled anymore, no matter what other drivers may or may not
> do.
> That includes situations where other drivers (or the BIOS)
> erroneously
> do not disable configuration mode.
>
> I understand your reasoning about not enabling configuration mode for
> certain chips, but that does not explain why it would be necessary
> to do this for all chips all the time.
>
> Sure, there is something we can do: Unless there is a known problem
> that affects _all_ chips, drop this patch.
Ahh, if that is the impression you have from my description, then I
will need to improve it, as that is not the case.
The actual update does the following:
1) Lock the memory, but does not perform a SIO entry (previously it
would have performed an SIO entry).
2) Attempt to read the chipID. This should be safe no matter which
chip we have.
3) If step (2) fails, then perform SIO entry and retry chipID read. If
it fails, act similarly to prior to this patch.
4) Set the sio_data->type, similar to previously.
5) If we have not performed an SIO entry, and this is not a chip type
with the NOCONF feature, then it will perform an SIO entry at this
point.
6) Proceed setup as prior to this patch.
7) Any following access to the SIO registers will invoke the SIO entry
and SIO exit steps unless it is a chip with the NOCONF feature set.
This was set up in the previous patches in this patchset.
8) There is also some minor update to the failure exit based on if it
had performed a SIO entry or not, in addition to the previous tests.
>
> Thanks,
> Guenter
Regards
Frank
next prev parent reply other threads:[~2024-04-13 8:47 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-01 2:56 [PATCH v1 0/4] hwmon (it87): Correct handling for configuration mode Frank Crawford
2024-04-01 2:56 ` [PATCH v1 1/4] hwmon (it87): Rename NOEXIT to BIOSOPEN as more descriptive of requirement Frank Crawford
2024-04-10 15:10 ` Guenter Roeck
2024-04-11 11:03 ` Frank Crawford
2024-04-11 12:31 ` Guenter Roeck
2024-04-12 10:20 ` Frank Crawford
2024-04-01 2:56 ` [PATCH v1 2/4] hwmon (it87): Do not enter configuration mode for some chip types Frank Crawford
2024-04-10 15:12 ` Guenter Roeck
2024-04-01 2:56 ` [PATCH v1 3/4] hwmon (it87): Test for chipset before entering configuration mode Frank Crawford
2024-04-10 15:17 ` Guenter Roeck
2024-04-11 11:08 ` Frank Crawford
2024-04-11 12:20 ` Guenter Roeck
2024-04-12 10:11 ` Frank Crawford
2024-04-12 12:48 ` Guenter Roeck
2024-04-13 8:47 ` Frank Crawford [this message]
2024-04-01 2:56 ` [PATCH v1 4/4] hwmon (it87): Remove tests no longer required Frank Crawford
Reply instructions:
You may reply publicly 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=3e29a0238b6cfae2945495593b05c782793a3734.camel@crawford.emu.id.au \
--to=frank@crawford.emu.id.au \
--cc=jdelvare@suse.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux@roeck-us.net \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).