Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Tue, 8 Feb 2022 22:46:53 +0100
From:   Jean Delvare <jdelvare@suse.de>
To:     Terry Bowman <Terry.Bowman@amd.com>
Cc:     linux@roeck-us.net, linux-watchdog@vger.kernel.org,
        linux-i2c@vger.kernel.org, wsa@kernel.org,
        andy.shevchenko@gmail.com, rafael.j.wysocki@intel.com,
        linux-kernel@vger.kernel.org, wim@linux-watchdog.org,
        rrichter@amd.com, thomas.lendacky@amd.com, sudheesh.mavila@amd.com,
        Nehal-bakulchandra.Shah@amd.com, Basavaraj.Natikar@amd.com,
        Shyam-sundar.S-k@amd.com, Mario.Limonciello@amd.com
Subject: Re: [PATCH v4 0/9] i2c: piix4: Replace cd6h/cd7h port I/O accesses
 with MMIO accesses
Message-ID: <20220208224653.6a62ba22@endymion.delvare>
In-Reply-To: <4ae57999-0f23-7578-008d-2009bc36d46b@amd.com>
References: <20220130184130.176646-1-terry.bowman@amd.com>
        <20220208181114.180a99ba@endymion.delvare>
        <4ae57999-0f23-7578-008d-2009bc36d46b@amd.com>
Organization: SUSE Linux
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 8 Feb 2022 13:36:55 -0600, Terry Bowman wrote:
> On 2/8/22 11:11, Jean Delvare wrote:
> > Unfortunately I'm not really able to test this series. While I do have
> > an AMD-based laptop which uses the i2c-piix4 driver, the SMBus has
> > never been usable on it. The driver creates 3 i2c buses (port 0 at
> > 0b00, port 2 at 0b00 and port 1 at 0b20). The first 2 do not seem to
> > have any device on them (i2cdetect returns empty). The last one must
> > have some devices on it, I see address 0x1c answers the ping,
> > unfortunately as soon as probing reaches address 0x2c, all later pings
> > return success, regardless of the address. It seems that some I2C
> > device (probably the one at 0x2c, but I don't know what it is) is
> > holding SDA low forever, therefore preventing any use of the whole
> > SMBus port until the next reboot.
> 
> My understanding is the OEM may have different i2c devices because it
> is mainboard specific.

Yes, the devices on the SMBus could be anything Lenovo decided to use.
Tomorrow I'll try to scan the SMBus but skipping the problematic
address, to see if it works around the problem.

> >> (...)
> >> Changes in v4:
> >> (...)
> >>  - Removed iowrite32(ioread32(...), ...). Unnecessary because MMIO is
> >>    already enabled. (Terry Bowman)  
> > 
> > I'm curious, how can you be sure of that actually?
> 
> The removed code was using a MMIO region to write 1 to 'mmioen'. This was 
> using the MMIO region to enable same MMIO region.

Ah ah, I get it now. Nice chicken or the egg situation :-)

> Programmer's manual shows FCH::PM::ISACONTROL[mmioen] has a '1' reset value.
> Per programmer's manual, FCH::PM::ISACONTROL[mmioen] enables MMIO mapping 
> at FED8_0000h..FED8_1FFFh. The FCH::PM::ISACONTROL register is MMIO 
> mapped at FED8_0304h. 'mmioen' was intended to be set using port I/O to 
> enable MMIO but, port I/O access to these registers is now disabled.

Is my understanding correct that there is some overlapping of the
access methods and there are certain chipsets where both legacy I/O and
MMIO access is possible?

If so, while there's indeed nothing to be done for the most recent
systems where only MMIO access is possible, you may still need to
enable MMIO access through legacy I/O if you try to use MMIO on
chipsets where both are possible. I'm not sure what exactly where you
set the limit. In the last patch you say that 0x51 is the first
revision of the family 17h CPUs, but is family 17h the first where MMIO
is available, or the first where legacy I/O isn't?

-- 
Jean Delvare
SUSE L3 Support