| Message ID | 1520002003-10200-2-git-send-email-jacopo+renesas@jmondi.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Fri, 2 Mar 2018 15:46:33 +0100 Jacopo Mondi <jacopo+renesas@jmondi.org> escreveu: > Re-order variables declaration to respect 'reverse christmas tree' > ordering whenever possible. To be frank, I don't like the idea of reverse christmas tree ordering myself... Perhaps due to the time I used to program on assembler, where alignment issues could happen, I find a way more logic to order based on complexity and size of the argument... > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > drivers/media/i2c/tw9910.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c > index cc648de..3a5e307 100644 > --- a/drivers/media/i2c/tw9910.c > +++ b/drivers/media/i2c/tw9910.c > @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client) > > static int tw9910_power(struct i2c_client *client, int enable) > { > - int ret; > u8 acntl1; > u8 acntl2; > + int ret; ... So, in this case, the order is already the right one, according with my own criteria :-) There was some discussion about the order sometime ago at LKML: https://patchwork.kernel.org/patch/9411999/ As I'm not seeing the proposed patch there at checkpatch, nor any comments about xmas tree at coding style, I think that there were no agreements about the ordering. So, while there's no consensus about that, let's keep it as-is. Regards, Mauro
Hi Mauro, On Tue, Mar 06, 2018 at 01:51:52PM -0300, Mauro Carvalho Chehab wrote: > Em Fri, 2 Mar 2018 15:46:33 +0100 > Jacopo Mondi <jacopo+renesas@jmondi.org> escreveu: > > > Re-order variables declaration to respect 'reverse christmas tree' > > ordering whenever possible. > > To be frank, I don't like the idea of reverse christmas tree ordering > myself... Perhaps due to the time I used to program on assembler, > where alignment issues could happen, I find a way more logic to order > based on complexity and size of the argument... > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > drivers/media/i2c/tw9910.c | 23 +++++++++++------------ > > 1 file changed, 11 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c > > index cc648de..3a5e307 100644 > > --- a/drivers/media/i2c/tw9910.c > > +++ b/drivers/media/i2c/tw9910.c > > @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client) > > > > static int tw9910_power(struct i2c_client *client, int enable) > > { > > - int ret; > > u8 acntl1; > > u8 acntl2; > > + int ret; > > ... So, in this case, the order is already the right one, according > with my own criteria :-) > > There was some discussion about the order sometime ago at LKML: > > https://patchwork.kernel.org/patch/9411999/ > > As I'm not seeing the proposed patch there at checkpatch, nor any > comments about xmas tree at coding style, I think that there were no > agreements about the ordering. > > So, while there's no consensus about that, let's keep it as-is. Thanks for explaining. I was sure it was part of the coding style rules! My bad, feel free to ditch this patch (same for ov772x ofc). Thanks j > > Regards, > Mauro
Em Tue, 6 Mar 2018 17:57:15 +0100 jacopo mondi <jacopo@jmondi.org> escreveu: > Hi Mauro, > > On Tue, Mar 06, 2018 at 01:51:52PM -0300, Mauro Carvalho Chehab wrote: > > Em Fri, 2 Mar 2018 15:46:33 +0100 > > Jacopo Mondi <jacopo+renesas@jmondi.org> escreveu: > > > > > Re-order variables declaration to respect 'reverse christmas tree' > > > ordering whenever possible. > > > > To be frank, I don't like the idea of reverse christmas tree ordering > > myself... Perhaps due to the time I used to program on assembler, > > where alignment issues could happen, I find a way more logic to order > > based on complexity and size of the argument... > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > --- > > > drivers/media/i2c/tw9910.c | 23 +++++++++++------------ > > > 1 file changed, 11 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c > > > index cc648de..3a5e307 100644 > > > --- a/drivers/media/i2c/tw9910.c > > > +++ b/drivers/media/i2c/tw9910.c > > > @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client) > > > > > > static int tw9910_power(struct i2c_client *client, int enable) > > > { > > > - int ret; > > > u8 acntl1; > > > u8 acntl2; > > > + int ret; > > > > ... So, in this case, the order is already the right one, according > > with my own criteria :-) > > > > There was some discussion about the order sometime ago at LKML: > > > > https://patchwork.kernel.org/patch/9411999/ > > > > As I'm not seeing the proposed patch there at checkpatch, nor any > > comments about xmas tree at coding style, I think that there were no > > agreements about the ordering. > > > > So, while there's no consensus about that, let's keep it as-is. > > Thanks for explaining. I was sure it was part of the coding style > rules! My bad, feel free to ditch this patch (same for ov772x ofc). Heh, there are so many rules that it is hard to get all of them. Also, some maintainers might actually be expecting some ordering. I ditched this patch (and the one for ov772x) and applied the remaining ones. Thanks, Mauro
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c index cc648de..3a5e307 100644 --- a/drivers/media/i2c/tw9910.c +++ b/drivers/media/i2c/tw9910.c @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client) static int tw9910_power(struct i2c_client *client, int enable) { - int ret; u8 acntl1; u8 acntl2; + int ret; if (enable) { acntl1 = 0; @@ -428,8 +428,8 @@ static int tw9910_power(struct i2c_client *client, int enable) static const struct tw9910_scale_ctrl *tw9910_select_norm(v4l2_std_id norm, u32 width, u32 height) { - const struct tw9910_scale_ctrl *scale; const struct tw9910_scale_ctrl *ret = NULL; + const struct tw9910_scale_ctrl *scale; __u32 diff = 0xffffffff, tmp; int size, i; @@ -462,8 +462,8 @@ static int tw9910_s_stream(struct v4l2_subdev *sd, int enable) { struct i2c_client *client = v4l2_get_subdevdata(sd); struct tw9910_priv *priv = to_tw9910(client); - u8 val; int ret; + u8 val; if (!enable) { switch (priv->revision) { @@ -512,10 +512,10 @@ static int tw9910_s_std(struct v4l2_subdev *sd, v4l2_std_id norm) { struct i2c_client *client = v4l2_get_subdevdata(sd); struct tw9910_priv *priv = to_tw9910(client); - const unsigned int hact = 720; const unsigned int hdelay = 15; - unsigned int vact; + const unsigned int hact = 720; unsigned int vdelay; + unsigned int vact; int ret; if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL))) @@ -760,8 +760,8 @@ static int tw9910_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config *cfg, struct v4l2_subdev_format *format) { - struct v4l2_mbus_framefmt *mf = &format->format; struct i2c_client *client = v4l2_get_subdevdata(sd); + struct v4l2_mbus_framefmt *mf = &format->format; struct tw9910_priv *priv = to_tw9910(client); if (format->pad) @@ -813,8 +813,8 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config *cfg, struct v4l2_subdev_format *format) { - struct v4l2_mbus_framefmt *mf = &format->format; struct i2c_client *client = v4l2_get_subdevdata(sd); + struct v4l2_mbus_framefmt *mf = &format->format; struct tw9910_priv *priv = to_tw9910(client); const struct tw9910_scale_ctrl *scale; @@ -852,8 +852,8 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd, static int tw9910_video_probe(struct i2c_client *client) { struct tw9910_priv *priv = to_tw9910(client); - s32 id; int ret; + s32 id; /* * tw9910 only use 8 or 16 bit bus width @@ -949,10 +949,9 @@ static int tw9910_probe(struct i2c_client *client, const struct i2c_device_id *did) { - struct tw9910_priv *priv; - struct tw9910_video_info *info; - struct i2c_adapter *adapter = - to_i2c_adapter(client->dev.parent); + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); + struct tw9910_video_info *info; + struct tw9910_priv *priv; int ret; if (!client->dev.platform_data) {
Re-order variables declaration to respect 'reverse christmas tree' ordering whenever possible. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- drivers/media/i2c/tw9910.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)