[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOtMz3OhB99cyA8dL6r=RJUPcSLbao5QmuiPmPGqDdDbqwFb3g@mail.gmail.com>
Date: Wed, 22 Dec 2021 22:51:33 +0100
From: Jan Dąbroś <jsd@...ihalf.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Serge Semin <fancer.lancer@...il.com>,
linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
jarkko.nikula@...ux.intel.com, mika.westerberg@...ux.intel.com,
wsa@...nel.org, rrangel@...omium.org,
Marcin Wojtas <mw@...ihalf.com>,
Grzegorz Jaszczyk <jaz@...ihalf.com>, upstream@...ihalf.com
Subject: Re: [RFC 0/2] i2c-designware: Add support for AMD PSP semaphore
Hi Serge, Andy,
Thanks for your comments. Glad to hear that this work may (possibly)
be helpful for even broader audience. For most of the stuff I tried to
create generic code and actually these solutions are already meant to
be shared with platforms using baytrail semaphore.
Best Regards,
Jan
śr., 22 gru 2021 o 19:22 Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> napisał(a):
>
> On Wed, Dec 22, 2021 at 08:56:21PM +0300, Serge Semin wrote:
> > On Wed, Dec 22, 2021 at 01:46:07PM +0200, Andy Shevchenko wrote:
> > > On Wed, Dec 22, 2021 at 10:45:56AM +0100, Jan Dabros wrote:
> > > > This patchset comprises support for new i2c-designware controller setup on some
> > > > AMD Cezanne SoCs, where x86 is sharing i2c bus with PSP. PSP uses the same
> > > > controller and acts as an i2c arbitrator there (x86 is leasing bus from it).
> > > >
> > > > First commit aims to improve generic i2c-designware code by adding extra locking
> > > > on probe() and disable() paths. I would like to ask someone with access to
> > > > boards which use Intel BayTrail(CONFIG_I2C_DESIGNWARE_BAYTRAIL) to verify
> > > > behavior of my changes on such setup.
> > > >
> > > > Second commit adds support for new PSP semaphore arbitration mechanism.
> > > > Implementation is similar to the one from i2c-designware-baytrail.c however
> > > > there are two main differences:
> > > > 1) Add new ACPI ID in order to protect against silent binding of the old driver
> > > > to the setup with PSP semaphore. Extra flag ARBITRATION_SEMAPHORE added to this
> > > > new _HID allows to recognize setup with PSP.
> > > > 2) Beside acquire_lock() and release_lock() methods we are also applying quirks
> > > > to the lock_bus() and unlock_bus() global adapter methods. With this in place
> > > > all i2c clients drivers may lock i2c bus for a desired number of i2c
> > > > transactions (e.g. write-wait-read) without being aware of that such bus is
> > > > shared with another entity.
> > > >
> > > > Mark this patchset as RFC, since waiting for new ACPI ID value. As a temporary
> > > > measure use "AMDI9999". Once proper one will be ready, will re-send this CL for
> > > > review & merge.
> > > >
> > > > Looking forward to some feedback.
> > >
> > > If I am not mistaken something similar happened in Baikal T1.
> > > Perhaps Serge has something to share.
> >
> > No, Baikal-T1 doesn't have such specific interface sharing since it
> > doesn't have any co-processor (though a scenario of booting different
> > kernels on each CPU core was at consideration by some our customers).
> > So the only peculiar things the SoC has got are two interfaces with
> > non-standard access:
> > 1) DW SPI controller with memory mapped 16MB direct EEPROM access. DW
> > SPI CSR/EEPROM mapping are switched by a multiplexer (basically just a
> > flag) embedded into the system controller.
> > 2) DW i2c controller with indirect registers access. Originally it was
> > supposed to be used by the system bootloader for some bootloading
> > stuff, but the actual usage scenario wasn't described by the SoC
> > engineers. The chip initially loads the code from the SPI-flash only,
> > which can be of up to 16MB size. It's more than enough to start pretty
> > complex systems, so an additional i2c interface turned to be not
> > really needed. Anyway other than having an indirectly accessible
> > CSRs it's pretty much normal DW I2C controller.
> >
> > But you are right in a reference to another BE-chip - Baikal-M1. The
> > i2c/spi/gpio/uart interfaces sharing support might get to be needed
> > for it since aside with four 2-cored Cortex-A57 clusters it has got an
> > embedded SCP co-processor which can access the same SoC interfaces as
> > the CPU cores. Though Baikal-M1 isn't supported by the mainline kernel
> > at the moment. We are going to start working on it on the next year.
> > Then we'll most likely need to implement the interface sharing feature
> > like the one introduced in this RFC. But for now alas I can't be much
> > helpful.
>
> Anyway, thanks for your insights!
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Powered by blists - more mailing lists