[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VcUVY+51o0DFhvYTjpydfF=L_rTGan74sZRMPDMmXtsQg@mail.gmail.com>
Date: Thu, 20 Jan 2022 13:30:40 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Terry Bowman <terry.bowman@....com>
Cc: Guenter Roeck <linux@...ck-us.net>, linux-watchdog@...r.kernel.org,
Jean Delvare <jdelvare@...e.com>,
linux-i2c <linux-i2c@...r.kernel.org>,
Wolfram Sang <wsa@...nel.org>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Wim Van Sebroeck <wim@...ux-watchdog.org>,
Robert Richter <rrichter@....com>,
Tom Lendacky <thomas.lendacky@....com>,
sudheesh.mavila@....com,
"Shah, Nehal-bakulchandra" <Nehal-bakulchandra.Shah@....com>,
Basavaraj Natikar <Basavaraj.Natikar@....com>,
Shyam Sundar S K <Shyam-sundar.S-k@....com>,
Mario Limonciello <Mario.Limonciello@....com>
Subject: Re: [PATCH v3 0/9] i2c: piix4: Replace cd6h/cd7h port I/O accesses
with MMIO accesses
On Thu, Jan 20, 2022 at 1:06 AM Terry Bowman <terry.bowman@....com> wrote:
>
> This series changes the piix4_smbus driver's cd6h/cd7h port I/O accesses
> to use MMIO instead. This is necessary because cd6h/cd7h port I/O may be
> disabled on later AMD processors.
>
> This series includes patches with MMIO accesses to registers
> FCH::PM::DECODEEN and FCH::PM::ISACONTROL. The same registers are also
> accessed by the sp5100_tco driver.[1] Synchronization to the MMIO
> registers is required in both drivers.
>
> The first patch creates a macro to request MMIO region using the 'muxed'
> retry logic. This is used in patch 6 to synchronize accesses to EFCH MMIO.
>
> The second patch replaces a hardcoded region size with a #define. This is
> to improve maintainability and was requested from v2 review.
>
> The third patch moves duplicated region request/release code into
> functions. This locates related code into functions and reduces code line
> count. This will also make adding MMIO support in patch 6 easier.
>
> The fourth patch moves SMBus controller address detection into a function.
> This is in preparation for adding MMIO region support.
>
> The fifth patch moves EFCH port selection into a function. This is in
> preparation for adding MMIO region support.
>
> The sixth patch adds MMIO support for region requesting/releasing and
> mapping. This is necessary for using MMIO to detect SMBus controller
> address, enable SMBbus controller region, and control the port select.
>
> The seventh patch updates the SMBus controller address detection to support
> using MMIO. This is necessary because the driver accesses registers
> FCH::PM::DECODEEN and FCH::PM::ISACONTOL during initialization and they are
> only available using MMIO on later AMD processors.
>
> The eighth patch updates the SMBus port selection to support MMIO. This is
> required because port selection control resides in the
> FCH::PM::DECODEEN[smbus0sel] and is only accessible using MMIO on later AMD
> processors.
>
> The ninth patch enables the EFCH MMIO functionality added earlier in this
> series. The SMBus controller's PCI revision ID is used to check if EFCH
> MMIO is supported by HW and should be enabled in the driver.
In general looks good to me, but I believe it will be much better if
we agreed on converting driver to use iomap_port() +
ioreadXX()/iowriteXX() (means dropping I/O accessor _p variants for
good). This would make the series cleaner and less invasive.
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists