Re: [PATCH v3 0/9] i2c: piix4: Replace cd6h/cd7h port I/O accesses with MMIO accesses

[Andy Shevchenko]

On Thu, Jan 20, 2022 at 1:06 AM Terry Bowman <terry.bowman@xxxxxxx> 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