Hi Benoit,
On Thu, Jun 18, 2020 at 09:06:09AM -0500, Benoit Parrot wrote:
> Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote on Mon [2020-Jun-15 02:58:24 +0300]:
> > Name all variables htat point to a cal_camerax instance 'phy' instead of
> > 'cc'. The name 'cc' refers to Camera Core, but is not commonly used in
> > the driver or in datasheets.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/platform/ti-vpe/cal.c | 102 ++++++++++++++--------------
> > 1 file changed, 51 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> > index 615e9d97e61f..8864a00a22b0 100644
> > --- a/drivers/media/platform/ti-vpe/cal.c
> > +++ b/drivers/media/platform/ti-vpe/cal.c
> > @@ -312,7 +312,8 @@ static const struct cal_data am654_cal_data = {
> > * the CAMERARX instances. Instances of struct cal_dev are named cal through the
> > * driver.
> > *
> > - * The cal_camerarx structure represents one CAMERARX instance.
> > + * The cal_camerarx structure represents one CAMERARX instance. Instances of
> > + * cal_camerarx are named phy through the driver.
> > *
> > * The cal_ctx structure represents the combination of one CSI-2 context, one
> > * processing context and one DMA context. Instance of struct cal_ctx are named
> > @@ -344,7 +345,7 @@ struct cal_dev {
> > u32 syscon_camerrx_offset;
> >
> > /* Camera Core Module handle */
> > - struct cal_camerarx *cc[CAL_NUM_CSI2_PORTS];
> > + struct cal_camerarx *phy[CAL_NUM_CSI2_PORTS];
> >
> > struct cal_ctx *ctx[CAL_NUM_CONTEXT];
> > };
> > @@ -361,7 +362,7 @@ struct cal_ctx {
> > struct v4l2_fwnode_endpoint endpoint;
> >
> > struct cal_dev *cal;
> > - struct cal_camerarx *cc;
> > + struct cal_camerarx *phy;
> >
> > /* v4l2_ioctl mutex */
> > struct mutex mutex;
> > @@ -468,7 +469,7 @@ static u32 cal_data_get_num_csi2_phy(struct cal_dev *cal)
> > }
> >
> > static int cal_camerarx_regmap_init(struct cal_dev *cal,
> > - struct cal_camerarx *cc,
> > + struct cal_camerarx *phy,
> > unsigned int idx)
> > {
> > const struct cal_camerarx_data *phy_data;
> > @@ -490,12 +491,12 @@ static int cal_camerarx_regmap_init(struct cal_dev *cal,
> > * Here we update the reg offset with the
> > * value found in DT
> > */
> > - cc->phy.fields[i] = devm_regmap_field_alloc(&cal->pdev->dev,
> > - cal->syscon_camerrx,
> > - field);
> > - if (IS_ERR(cc->phy.fields[i])) {
> > + phy->phy.fields[i] = devm_regmap_field_alloc(&cal->pdev->dev,
> > + cal->syscon_camerrx,
> > + field);
>
> So we end up with these construct phy->phy.
> Is that really more readable?
I agree with you that it's not very nice. That's why it gets fixed
later, with the 'fields' field being moved from struct cal_camerarx_data
to struct cal_camerarx :-) I could try to rearrange the patches to avoid
this, but as it's an intermediary step only, it would be quite a bit of
rebase and conflict resolution to achieve the exact same result. Would
it be OK with you to keep this intermediate step as-is (assuming you
like the end result of course) ?
--
Regards,
Laurent Pinchart
