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;
MIME-Version: 1.0
References: <20220120001621.705352-2-jsd@semihalf.com> <20220202144302.1438303-1-jsd@semihalf.com>
 <20220202144302.1438303-2-jsd@semihalf.com> <YfqtkBIZZxp2Au2l@smile.fi.intel.com>
 <CAOtMz3O_j5-0ZLJngQpzQSGduhZVnQ8awBvGkMdUQWAX8nN5Hw@mail.gmail.com> <YgEFWN8K7Yh452GY@smile.fi.intel.com>
In-Reply-To: <YgEFWN8K7Yh452GY@smile.fi.intel.com>
From:   =?UTF-8?B?SmFuIETEhWJyb8Wb?= <jsd@semihalf.com>
Date:   Tue, 8 Feb 2022 15:15:24 +0100
Message-ID: <CAOtMz3P18emF_1tPvXzckVS8V4DWYB6Evqu+V6PsXN6L2YH2aA@mail.gmail.com>
Subject: Re: [PATCH v3 2/2] i2c: designware: Add AMD PSP I2C bus support
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-i2c <linux-i2c@vger.kernel.org>,
        Jarkko Nikula <jarkko.nikula@linux.intel.com>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        Hans de Goede <hdegoede@redhat.com>,
        Wolfram Sang <wsa@kernel.org>,
        Raul E Rangel <rrangel@chromium.org>,
        Marcin Wojtas <mw@semihalf.com>,
        Grzegorz Jaszczyk <jaz@semihalf.com>, upstream@semihalf.com,
        Tom Lendacky <thomas.lendacky@amd.com>,
        "Deucher, Alexander" <alexander.deucher@amd.com>,
        "Easow, Nimesh" <Nimesh.Easow@amd.com>,
        "Limonciello, Mario" <mario.limonciello@amd.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

pon., 7 lut 2022 o 12:42 Andy Shevchenko
<andriy.shevchenko@linux.intel.com> napisa=C5=82(a):
>
> On Mon, Feb 07, 2022 at 09:27:12AM +0100, Jan D=C4=85bro=C5=9B wrote:
> > =C5=9Br., 2 lut 2022 o 17:16 Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> napisa=C5=82(a):
> > > On Wed, Feb 02, 2022 at 03:43:02PM +0100, Jan Dabros wrote:
>
> ...
>
> > > > +struct psp_i2c_req {
> > > > +     struct psp_req_buffer_hdr hdr;
> > > > +     enum psp_i2c_req_type type;
> > > > +} __aligned(32);
> > >
> > > I forgot if this alignment is requirement due to DMA or so?
> > > We may use ____cacheline_aligned in such case.
> >
> > I used some old code as a reference - this structure is mapped by PSP,
> > thus alignment applied here. I'm checking this with AMD whether it is
> > really needed or at least can be somehow limited - still waiting for
> > their response.
> > I propose to left this as is right now and eventually remove or make
> > more liberal in the future.
>
> Would be nice to clarify sooner than later.
> In either case it needs a good comment.

I confirmed that we can remove this alignment completely. Removed in v4.

Best Regards,
Jan

>
> ...
>
> > > > +     return readl_poll_timeout(&mbox->cmd_fields, tmp, (tmp =3D=3D=
 expected),
> > > > +                               0, 1000 * PSP_CMD_TIMEOUT_MS);
> > >
> > > 0?!
> >
> > Yes, we are checking for readiness of PSP mailbox in a tight loop. We
> > would like to proceed further quickly as soon as this bit is set.
> > Actually checking this twice per every ACQUIRE&RELEASE - once before
> > sending command (to check whether PSP is ready to accept requests) and
> > second time after sending it. Do you think we should increase
> > @sleep_us value?
>
> It depends on what you have in mind about hardware. I'm fine with either =
way,
> but 0 has to be explained (in the comment).
>
> --
> With Best Regards,
> Andy Shevchenko
>
>