[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAKMK7uFL0iWYF-PK51a3ca3pSjRYoC-+zTRtWCTXVC8aPd4jbw@mail.gmail.com>
Date: Tue, 17 Sep 2019 15:58:35 +0200
From: Daniel Vetter <daniel@...ll.ch>
To: Benjamin Gaignard <benjamin.gaignard@...aro.org>
Cc: Dave Airlie <airlied@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Sean Paul <sean@...rly.run>,
Maxime Ripard <mripard@...nel.org>,
Benjamin Gaignard <benjamin.gaignard@...com>,
David Airlie <airlied@...ux.ie>,
ML dri-devel <dri-devel@...ts.freedesktop.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm: sti: fix W=1 warnings
On Tue, Sep 17, 2019 at 3:40 PM Benjamin Gaignard
<benjamin.gaignard@...aro.org> wrote:
>
> Le mar. 17 sept. 2019 à 14:46, Daniel Vetter <daniel@...ll.ch> a écrit :
> >
> > On Mon, Sep 16, 2019 at 03:16:49PM +0200, Benjamin Gaignard wrote:
> > > Le lun. 9 sept. 2019 à 12:29, Benjamin Gaignard
> > > <benjamin.gaignard@...com> a écrit :
> > > >
> > > > Fix warnings when W=1.
> > > > No code changes, only clean up in sti internal structures and functions
> > > > descriptions.
> > > >
> > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...com>
> > >
> > > For my own reference, applied on drm-misc-next
> >
> > Dude seriously no:
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...com>
> > Reviewed-by: Benjamin Gaignard <benjamin.gaignard@...com>
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...aro.org>
> > Link: https://patchwork.freedesktop.org/patch/msgid/20190909101254.24191-1-benjamin.gaignard@st.com
> >
> > Self-review ain't ok in drm-misc, you need someone to
> > ack/double-check/have a look. And given that you had to fabricate your
> > self-review yourself somehow (the tools really don't do that for you) this
> > doesn't look like an accident.
> >
> > Adding other maintainers.
> > -Daniel
> >
>
> All my apologies, I have taken a shortcut for this STI patch...
> I will ask to Philippe or Yannick to formally review STI related
> patches on mailing and not only on internal one.
Note we don't require full formal review, that's just overkill. But a
quick ack shouldn't bee too much for any driver with more than 1
person working on it.
Also pls no internal review, because review is about learning and
sharing knowledge, and code correctness only secondary. If you do that
behind closed doors you forgoe most of the benefits.
-Daniel
>
> Benjamin
>
> > >
> > > > ---
> > > > drivers/gpu/drm/sti/sti_cursor.c | 2 +-
> > > > drivers/gpu/drm/sti/sti_dvo.c | 2 +-
> > > > drivers/gpu/drm/sti/sti_gdp.c | 2 +-
> > > > drivers/gpu/drm/sti/sti_hda.c | 2 +-
> > > > drivers/gpu/drm/sti/sti_hdmi.c | 4 ++--
> > > > drivers/gpu/drm/sti/sti_tvout.c | 10 +++++-----
> > > > drivers/gpu/drm/sti/sti_vtg.c | 2 +-
> > > > 7 files changed, 12 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
> > > > index 0bf7c332cf0b..ea64c1dcaf63 100644
> > > > --- a/drivers/gpu/drm/sti/sti_cursor.c
> > > > +++ b/drivers/gpu/drm/sti/sti_cursor.c
> > > > @@ -47,7 +47,7 @@ struct dma_pixmap {
> > > > void *base;
> > > > };
> > > >
> > > > -/**
> > > > +/*
> > > > * STI Cursor structure
> > > > *
> > > > * @sti_plane: sti_plane structure
> > > > diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> > > > index 9e6d5d8b7030..c33d0aaee82b 100644
> > > > --- a/drivers/gpu/drm/sti/sti_dvo.c
> > > > +++ b/drivers/gpu/drm/sti/sti_dvo.c
> > > > @@ -65,7 +65,7 @@ static struct dvo_config rgb_24bit_de_cfg = {
> > > > .awg_fwgen_fct = sti_awg_generate_code_data_enable_mode,
> > > > };
> > > >
> > > > -/**
> > > > +/*
> > > > * STI digital video output structure
> > > > *
> > > > * @dev: driver device
> > > > diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
> > > > index 8e926cd6a1c8..11595c748844 100644
> > > > --- a/drivers/gpu/drm/sti/sti_gdp.c
> > > > +++ b/drivers/gpu/drm/sti/sti_gdp.c
> > > > @@ -103,7 +103,7 @@ struct sti_gdp_node_list {
> > > > dma_addr_t btm_field_paddr;
> > > > };
> > > >
> > > > -/**
> > > > +/*
> > > > * STI GDP structure
> > > > *
> > > > * @sti_plane: sti_plane structure
> > > > diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> > > > index 94e404f13234..3512a94a0fca 100644
> > > > --- a/drivers/gpu/drm/sti/sti_hda.c
> > > > +++ b/drivers/gpu/drm/sti/sti_hda.c
> > > > @@ -230,7 +230,7 @@ static const struct sti_hda_video_config hda_supported_modes[] = {
> > > > AWGi_720x480p_60, NN_720x480p_60, VID_ED}
> > > > };
> > > >
> > > > -/**
> > > > +/*
> > > > * STI hd analog structure
> > > > *
> > > > * @dev: driver device
> > > > diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> > > > index f03d617edc4c..87e34f7a6cfe 100644
> > > > --- a/drivers/gpu/drm/sti/sti_hdmi.c
> > > > +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> > > > @@ -333,7 +333,6 @@ static void hdmi_infoframe_reset(struct sti_hdmi *hdmi,
> > > > * Helper to concatenate infoframe in 32 bits word
> > > > *
> > > > * @ptr: pointer on the hdmi internal structure
> > > > - * @data: infoframe to write
> > > > * @size: size to write
> > > > */
> > > > static inline unsigned int hdmi_infoframe_subpack(const u8 *ptr, size_t size)
> > > > @@ -543,13 +542,14 @@ static int hdmi_vendor_infoframe_config(struct sti_hdmi *hdmi)
> > > > return 0;
> > > > }
> > > >
> > > > +#define HDMI_TIMEOUT_SWRESET 100 /*milliseconds */
> > > > +
> > > > /**
> > > > * Software reset of the hdmi subsystem
> > > > *
> > > > * @hdmi: pointer on the hdmi internal structure
> > > > *
> > > > */
> > > > -#define HDMI_TIMEOUT_SWRESET 100 /*milliseconds */
> > > > static void hdmi_swreset(struct sti_hdmi *hdmi)
> > > > {
> > > > u32 val;
> > > > diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c
> > > > index e1b3c8cb7287..b1fc77b150da 100644
> > > > --- a/drivers/gpu/drm/sti/sti_tvout.c
> > > > +++ b/drivers/gpu/drm/sti/sti_tvout.c
> > > > @@ -157,9 +157,9 @@ static void tvout_write(struct sti_tvout *tvout, u32 val, int offset)
> > > > *
> > > > * @tvout: tvout structure
> > > > * @reg: register to set
> > > > - * @cr_r:
> > > > - * @y_g:
> > > > - * @cb_b:
> > > > + * @cr_r: red chroma or red order
> > > > + * @y_g: y or green order
> > > > + * @cb_b: blue chroma or blue order
> > > > */
> > > > static void tvout_vip_set_color_order(struct sti_tvout *tvout, int reg,
> > > > u32 cr_r, u32 y_g, u32 cb_b)
> > > > @@ -214,7 +214,7 @@ static void tvout_vip_set_rnd(struct sti_tvout *tvout, int reg, u32 rnd)
> > > > * @tvout: tvout structure
> > > > * @reg: register to set
> > > > * @main_path: main or auxiliary path
> > > > - * @sel_input: selected_input (main/aux + conv)
> > > > + * @video_out: selected_input (main/aux + conv)
> > > > */
> > > > static void tvout_vip_set_sel_input(struct sti_tvout *tvout,
> > > > int reg,
> > > > @@ -251,7 +251,7 @@ static void tvout_vip_set_sel_input(struct sti_tvout *tvout,
> > > > *
> > > > * @tvout: tvout structure
> > > > * @reg: register to set
> > > > - * @in_vid_signed: used video input format
> > > > + * @in_vid_fmt: used video input format
> > > > */
> > > > static void tvout_vip_set_in_vid_fmt(struct sti_tvout *tvout,
> > > > int reg, u32 in_vid_fmt)
> > > > diff --git a/drivers/gpu/drm/sti/sti_vtg.c b/drivers/gpu/drm/sti/sti_vtg.c
> > > > index ef4009f11396..0b17ac8a3faa 100644
> > > > --- a/drivers/gpu/drm/sti/sti_vtg.c
> > > > +++ b/drivers/gpu/drm/sti/sti_vtg.c
> > > > @@ -121,7 +121,7 @@ struct sti_vtg_sync_params {
> > > > u32 vsync_off_bot;
> > > > };
> > > >
> > > > -/**
> > > > +/*
> > > > * STI VTG structure
> > > > *
> > > > * @regs: register mapping
> > > > --
> > > > 2.15.0
> > > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Powered by blists - more mailing lists