Hi Hans, Laurent,
On Fri, Jun 16, 2023 at 06:54:54PM +0200, Hans de Goede wrote:
> Hi Tommaso,
>
> On 6/15/23 18:15, Tommaso Merciai wrote:
> > Hi Hans,
> >
> > On Thu, Jun 15, 2023 at 02:00:46PM +0200, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 6/15/23 13:54, Tommaso Merciai wrote:
> >>> On Thu, Jun 15, 2023 at 01:26:25PM +0200, Tommaso Merciai wrote:
> >>>> Hi Hans,
> >>>>
> >>>> On Thu, Jun 15, 2023 at 01:10:40PM +0200, Hans de Goede wrote:
> >>>>> Hi Tommaso,
> >>>>>
> >>>>> On 6/15/23 12:05, Tommaso Merciai wrote:
> >>>>>> Hi Hans, Laurent, Sakari,
> >>>>>>
> >>>>>> Can I cherry-pick this patch and use these new functions also
> >>>>>> for cci regs of the alvium driver?
> >>>>>
> >>>>> Yes that sounds like a good plan.
> >>>>>
> >>>>>> Are on going to be merge?
> >>>>>
> >>>>> Yes this will hopefully get merged upstream soon.
> >>>>
> >>>> Thanks for the info!
> >>>>
> >>>> I want to ask you your opinion about this:
> >>>>
> >>>> Into alvium driver actually I'm using the following defines
> >>>> manipulations:
> >>>>
> >>>> #define REG_BCRM_REG_ADDR_R REG_BCRM_CCI_16BIT(0x0014)
> >>>>
> >>>> #define REG_BCRM_FEATURE_INQUIRY_R REG_BCRM_V4L2_64BIT(0x0008)
> >>>> #define REG_BCRM_DEVICE_FIRMWARE_VERSION_R REG_BCRM_V4L2_64BIT(0x0010)
> >>>>
> >>>> My plan is to use your cci API for cci register in this way defines
> >>>> became like:
> >>>>
> >>>> #define REG_BCRM_REG_ADDR_R CCI_REG16(0x0014)
> >>>>
> >>>> And leave v4l2 regs are it are right now:
> >>>>
> >>>> #define REG_BCRM_FEATURE_INQUIRY_R REG_BCRM_V4L2_64BIT(0x0008)
> >>>> #define REG_BCRM_DEVICE_FIRMWARE_VERSION_R REG_BCRM_V4L2_64BIT(0x0010)
> >>>>
> >>>> What do you think about?
> >>>
> >>> Or maybe is worth don't use v4l2 bit for v4l2 regs, I think is implicit
> >>> that what regs that are not CCI are v4l2, then we return wit the
> >>> following defines:
> >>>
> >>>
> >>>
> >>> #define REG_BCRM_REG_ADDR_R CCI_REG16(0x0014)
> >>> ^CCI regs
> >>>
> >>> #define REG_BCRM_FEATURE_INQUIRY_R 0x0008
> >>> #define REG_BCRM_DEVICE_FIRMWARE_VERSION_R 0x0010
> >>> ^v4l2 regs
> >>
> >> I'm not sure what you mean with "V4L2" registers ? I guess you mean
> >> registers which cannot be accessed through the CCI helper functions,
> >> but starting with v2 this is no longer true. There now is a CCI_REG64()
> >> so you can simply use that.
> >
> > I'm playing a bit with v3 of your cci api :)
> >
> > My problem is the following, bcrm regs are not real regs but are offset
> > from bcrm address (this is not fixed, it depends on the camera).
> >
> > Then the workflow is:
> >
> > - read bcrm_address (base address)
> > - then sum this to the offset (regs)
> >
> > Myabe this clarify:
> >
> > static int alvium_read(struct alvium_dev *alvium, u32 reg, u64 *val)
> > {
> > int ret;
> >
> > if (reg & REG_BCRM_V4L2)
> > reg += alvium->bcrm_addr;
> >
> > cci_read(alvium->regmap, reg, val, &ret);
> > if (ret)
> > return ret;
> >
> > return 0;
> > }
> >
> > int alvium_write(struct alvium_dev *alvium, u32 reg, u64 val)
> > {
> > int ret;
> >
> > if (reg & REG_BCRM_V4L2)
> > reg += alvium->bcrm_addr;
> >
> > cci_write(alvium->regmap, reg, val, &ret);
> > if (ret)
> > return ret;
> >
> > return 0;
> > }
> >
> > Where for example:
> >
> > #define REG_BCRM_V4L2 BIT(31)
> > #define REG_BCRM_V4L2_64BIT(n) (REG_BCRM_V4L2 | CCI_REG64(n))
> >
> > #define REG_BCRM_WRITE_HANDSHAKE_RW REG_BCRM_V4L2_8BIT(0x0418)
> >
> >
> > But I'm not sure that I'm in the right direction.
> >
> > In real I need first to get the real address then sum the bcrm_address
> > if this is a bcrm regs(offset) then re-incapsule the address into the
> > right CCI_REG# defines.
>
> Ah I see, so you have a set of windowed registers where
> the base address of these registers may change.
Yep, right :)
>
> What I don't understand though is why you use V4L2 in the
> name of the #defines for this? Does the datasheet actually
> name them like this ? V4L2 stands for video4linux version 2,
> so unless these registers are somehow Linux specific using
> V4L2 in the #define names is a bit weird IMHO.
These registers are offered from the alvium fw for v4l2 API.
We had a previous discussion with Laurent about this.
Btw I will send v7 with Laurent hints (read_timeout_poll/err-params)
And we can discuss there.
(If for you is ok :) )
I will keep both you and Laurent in CC.
Thanks again both for your review/hints :)
Regards,
Tommaso
>
> Regards,
>
> Hans
>
