2019-11-04 19:35:58

by Benoit Parrot

[permalink] [raw]
Subject: [Patch v2 12/20] media: ti-vpe: cal: Add DRA76x support

Add the needed control module register bit layout to support
the DRA76x family of devices.

Signed-off-by: Benoit Parrot <[email protected]>
---
drivers/media/platform/ti-vpe/cal.c | 36 +++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index b348d99d6166..72ed2348389d 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -290,6 +290,38 @@ static struct cal_data dra72x_es1_cal_data = {
.flags = DRA72_CAL_PRE_ES2_LDO_DISABLE,
};

+static struct reg_field dra76x_ctrl_core_csi0_reg_fields[F_MAX_FIELDS] = {
+ [F_CTRLCLKEN] = REG_FIELD(0, 8, 8),
+ [F_CAMMODE] = REG_FIELD(0, 9, 10),
+ [F_CSI_MODE] = REG_FIELD(0, 11, 11),
+ [F_LANEENABLE] = REG_FIELD(0, 27, 31),
+};
+
+static struct reg_field dra76x_ctrl_core_csi1_reg_fields[F_MAX_FIELDS] = {
+ [F_CTRLCLKEN] = REG_FIELD(0, 0, 0),
+ [F_CAMMODE] = REG_FIELD(0, 1, 2),
+ [F_CSI_MODE] = REG_FIELD(0, 3, 3),
+ [F_LANEENABLE] = REG_FIELD(0, 24, 26),
+};
+
+static struct cal_csi2_phy dra76x_cal_csi_phy[] = {
+ {
+ .base_fields = dra76x_ctrl_core_csi0_reg_fields,
+ .num_lanes = 5,
+ },
+ {
+ .base_fields = dra76x_ctrl_core_csi1_reg_fields,
+ .num_lanes = 3,
+ },
+};
+
+static struct cal_data dra76x_cal_data = {
+ .csi2_phy_core = dra76x_cal_csi_phy,
+ .num_csi2_phy = ARRAY_SIZE(dra76x_cal_csi_phy),
+
+ .flags = 0,
+};
+
/*
* there is one cal_dev structure in the driver, it is shared by
* all instances.
@@ -2285,6 +2317,10 @@ static const struct of_device_id cal_of_match[] = {
.compatible = "ti,dra72-pre-es2-cal",
.data = (void *)&dra72x_es1_cal_data,
},
+ {
+ .compatible = "ti,dra76-cal",
+ .data = (void *)&dra76x_cal_data,
+ },
{},
};
MODULE_DEVICE_TABLE(of, cal_of_match);
--
2.17.1


2019-11-06 08:59:28

by Sakari Ailus

[permalink] [raw]
Subject: Re: [Patch v2 12/20] media: ti-vpe: cal: Add DRA76x support

Hi Benoit,

On Mon, Nov 04, 2019 at 01:31:32PM -0600, Benoit Parrot wrote:

...

> +static struct cal_data dra76x_cal_data = {

const?

> + .csi2_phy_core = dra76x_cal_csi_phy,
> + .num_csi2_phy = ARRAY_SIZE(dra76x_cal_csi_phy),
> +

This newline seems extra.

> + .flags = 0,

And flags will be zero in any case, as one more struct members are assigned
to.

> +};

--
Regards,

Sakari Ailus

2019-11-06 20:57:34

by Benoit Parrot

[permalink] [raw]
Subject: Re: [Patch v2 12/20] media: ti-vpe: cal: Add DRA76x support

Sakari Ailus <[email protected]> wrote on Wed [2019-Nov-06 10:57:09 +0200]:
> Hi Benoit,
>
> On Mon, Nov 04, 2019 at 01:31:32PM -0600, Benoit Parrot wrote:
>
> ...
>
> > +static struct cal_data dra76x_cal_data = {
>
> const?

Hmm, most likely.

>
> > + .csi2_phy_core = dra76x_cal_csi_phy,
> > + .num_csi2_phy = ARRAY_SIZE(dra76x_cal_csi_phy),
> > +
>
> This newline seems extra.
>
> > + .flags = 0,
>
> And flags will be zero in any case, as one more struct members are assigned
> to.

Is this guaranteed or compiler version dependent?

>
> > +};
>
> --
> Regards,
>
> Sakari Ailus

2019-11-06 21:13:28

by Sakari Ailus

[permalink] [raw]
Subject: Re: [Patch v2 12/20] media: ti-vpe: cal: Add DRA76x support

On Wed, Nov 06, 2019 at 02:58:39PM -0600, Benoit Parrot wrote:
> Sakari Ailus <[email protected]> wrote on Wed [2019-Nov-06 10:57:09 +0200]:
> > Hi Benoit,
> >
> > On Mon, Nov 04, 2019 at 01:31:32PM -0600, Benoit Parrot wrote:
> >
> > ...
> >
> > > +static struct cal_data dra76x_cal_data = {
> >
> > const?
>
> Hmm, most likely.
>
> >
> > > + .csi2_phy_core = dra76x_cal_csi_phy,
> > > + .num_csi2_phy = ARRAY_SIZE(dra76x_cal_csi_phy),
> > > +
> >
> > This newline seems extra.
> >
> > > + .flags = 0,
> >
> > And flags will be zero in any case, as one more struct members are assigned
> > to.
>
> Is this guaranteed or compiler version dependent?

C standard.

--
Sakari Ailus

2019-11-12 13:26:35

by Benoit Parrot

[permalink] [raw]
Subject: Re: [Patch v2 12/20] media: ti-vpe: cal: Add DRA76x support

Benoit Parrot <[email protected]> wrote on Wed [2019-Nov-06 14:58:39 -0600]:
> Sakari Ailus <[email protected]> wrote on Wed [2019-Nov-06 10:57:09 +0200]:
> > Hi Benoit,
> >
> > On Mon, Nov 04, 2019 at 01:31:32PM -0600, Benoit Parrot wrote:
> >
> > ...
> >
> > > +static struct cal_data dra76x_cal_data = {
> >
> > const?
>
> Hmm, most likely.

Well, it turns out they cannot be constified because we need to be able to
update the register offset fir the regmap used for the syscon object.
So I'll leave them as is.

Benoit

>
> >
> > > + .csi2_phy_core = dra76x_cal_csi_phy,
> > > + .num_csi2_phy = ARRAY_SIZE(dra76x_cal_csi_phy),
> > > +
> >
> > This newline seems extra.
> >
> > > + .flags = 0,
> >
> > And flags will be zero in any case, as one more struct members are assigned
> > to.
>
> Is this guaranteed or compiler version dependent?
>
> >
> > > +};
> >
> > --
> > Regards,
> >
> > Sakari Ailus

2019-11-12 13:36:21

by Benoit Parrot

[permalink] [raw]
Subject: Re: [Patch v2 12/20] media: ti-vpe: cal: Add DRA76x support

Benoit Parrot <[email protected]> wrote on Tue [2019-Nov-12 07:28:11 -0600]:
> Benoit Parrot <[email protected]> wrote on Wed [2019-Nov-06 14:58:39 -0600]:
> > Sakari Ailus <[email protected]> wrote on Wed [2019-Nov-06 10:57:09 +0200]:
> > > Hi Benoit,
> > >
> > > On Mon, Nov 04, 2019 at 01:31:32PM -0600, Benoit Parrot wrote:
> > >
> > > ...
> > >
> > > > +static struct cal_data dra76x_cal_data = {
> > >
> > > const?
> >
> > Hmm, most likely.
>
> Well, it turns out they cannot be constified because we need to be able to
> update the register offset fir the regmap used for the syscon object.
> So I'll leave them as is.

Oh I guess you meant just the "static struct cal_data" ones?
I tried making the other one into const but obviously that didn't work.

But just constifying "static struct cal_data" appears to compiled just
fine.

>
> Benoit
>
> >
> > >
> > > > + .csi2_phy_core = dra76x_cal_csi_phy,
> > > > + .num_csi2_phy = ARRAY_SIZE(dra76x_cal_csi_phy),
> > > > +
> > >
> > > This newline seems extra.
> > >
> > > > + .flags = 0,
> > >
> > > And flags will be zero in any case, as one more struct members are assigned
> > > to.
> >
> > Is this guaranteed or compiler version dependent?
> >
> > >
> > > > +};
> > >
> > > --
> > > Regards,
> > >
> > > Sakari Ailus