MIME-Version: 1.0
References: <20211201175613.13710-1-dafna.hirschfeld@collabora.com>
 <20211201175613.13710-2-dafna.hirschfeld@collabora.com> <CAFr9PX=6Pd1Rg=wJvpuX6WX63L=iAnwPA24e59An3Kac5f_vzA@mail.gmail.com>
 <cdd9b485-364f-c6bd-776f-a0ca2d260762@collabora.com> <e9905774-a994-6311-7b53-b40588d4f6ec@collabora.com>
In-Reply-To: <e9905774-a994-6311-7b53-b40588d4f6ec@collabora.com>
From:   Daniel Palmer <daniel@0x0f.com>
Date:   Tue, 14 Dec 2021 19:20:55 +0900
Message-ID: <CAFr9PXnnGc1TUQBeW8JW9qGewhU99gmRbvzZEsZaDmr12jMg0A@mail.gmail.com>
Subject: Re: [PATCH v4 1/6] staging: media: wave5: Add vpuapi layer
To:     Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Cc:     "open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)" 
        <linux-media@vger.kernel.org>,
        Robert Beckett <bob.beckett@collabora.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "open list:STAGING SUBSYSTEM" <linux-staging@lists.linux.dev>,
        open list <linux-kernel@vger.kernel.org>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        hverkuil@xs4all.nl, kernel@collabora.com, dafna3@gmail.com,
        kiril.bicevski@collabora.com,
        Nas Chung <nas.chung@chipsnmedia.com>,
        lafley.kim@chipsnmedia.com, scott.woo@chipsnmedia.com,
        olivier.crete@collabora.com, dan.carpenter@oracle.com,
        Randy Dunlap <rdunlap@infradead.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Dafna,

On Tue, 14 Dec 2021 at 16:48, Dafna Hirschfeld
<dafna.hirschfeld@collabora.com> wrote:
> >>> +static int wave5_wait_bus_busy(struct vpu_device *vpu_dev, int timeout, unsigned int addr)
> >>> +{
> >>> +       u32 gdi_status_check_value = 0x3f;
> >>> +       u32 data;
> >>> +
> >>> +       if (vpu_dev->product_code == WAVE521C_CODE ||
> >>> +           vpu_dev->product_code == WAVE521_CODE ||
> >>> +        vpu_dev->product_code == WAVE521E1_CODE)
> >>> +               gdi_status_check_value = 0x00ff1f3f;
> >>> +
> >>> +       return read_poll_timeout(wave5_vdi_read_register, data, data == gdi_status_check_value,
> >>> +                                0, timeout * 1000, false, vpu_dev, addr);
> >>> +}
> >>> +
> >>
> >> This looks like it should be s/wave5_vdi_read_register/wave5_read_register/.
> >> For wave511 addr passed in here is 0x8e14 so well outside of what is
> >> directly accessible.
> >
> > Hi, I didn't understand this explanation. I see that
> > wave5_read_register eventually calls 'wave5_vdi_read_register'.
> > Could you please explain in more detail why you think
> > calling wave5_vdi_read_register is wrong?

Mainly because the address accessed 0x8e14 but on my machine the
directly accessible registers end at 0x800.

> hi, I see know that those backbone address are indeed not read and written directly but
> the address should be first written to a regsiter W5_VPU_FIO_CTRL_ADDR,
> and then the content is returned from W5_VPU_FIO_DATA.

I think so. But as I can't get this driver to fully work yet I can
only say I think so.

Cheers,

Daniel