On Mon, 2011-11-28 at 11:08 +0200, Carlos Chinea wrote:
> Hi Tomi,
>
> Just a question/suggestion, bellow:
>
> On Thu, 2011-11-24 at 15:29 +0200, ext Tomi Valkeinen wrote:
> > Use the new lane config in dsi_set_lane_config().
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
> > ---
> > drivers/video/omap2/dss/dsi.c | 84 +++++++++++++++++++---------------------
> > 1 files changed, 40 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> > index aea110c..ba8d6b3 100644
> > --- a/drivers/video/omap2/dss/dsi.c
> > +++ b/drivers/video/omap2/dss/dsi.c
> > @@ -2154,59 +2154,53 @@ static int dsi_parse_lane_config(struct omap_dss_device *dssdev)
> > return 0;
> > }
> >
> > -static void dsi_set_lane_config(struct omap_dss_device *dssdev)
> > +static int dsi_set_lane_config(struct omap_dss_device *dssdev)
> > {
> > struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> > + struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> > + static const u8 offsets[] = { 0, 4, 8, 12, 16 };
> > + static const enum dsi_lane_function functions[] = {
> > + DSI_LANE_CLK,
> > + DSI_LANE_DATA1,
> > + DSI_LANE_DATA2,
> > + DSI_LANE_DATA3,
> > + DSI_LANE_DATA4,
> > + };
>
> Patch 05 of the series has a function (dsi_parse_lane_config) with
> exactly the same static local declaration. Wouldn't be better to have an
> static global declaration instead to save some space ? or are the values
> from those functions going to differ in the near future ?
True, the array could be a global, and no, I don't think they'll change
in the near future.
But the data is more like function internal stuff than global data. The
functions want to parse and set the lane configs in particular order,
and use the array for that.
While the order happens to be the same in both functions, I still felt
the array is internal to each function rather than global data. Looking
from outside the function, the order doesn't matter. It's just an
internal detail.
Tomi
Attachment:
signature.asc
Description: This is a digitally signed message part
