Re: [PATCH 03/25] mtd: nand: gpmi-nand: let the driver core handle pinctrl

Re: [PATCH 03/25] mtd: nand: gpmi-nand: let the driver core handle pinctrl

[Huang Shijie]

于 2013年07月31日 22:11, Lothar Waßmann 写道:
> without this patch the driver fails to load with -EBUSY.
>
> Signed-off-by: Lothar Waßmann<LW@KARO-electronics.de>
> ---
>   drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    8 --------
>   1 files changed, 0 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index b741a6c..f557a1d 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -520,7 +520,6 @@ err_clock:
>
>   static int acquire_resources(struct gpmi_nand_data *this)
>   {
> -	struct pinctrl *pinctrl;
>   	int ret;
>
>   	ret = acquire_register_block(this, GPMI_NAND_GPMI_REGS_ADDR_RES_NAME);
> @@ -539,19 +538,12 @@ static int acquire_resources(struct gpmi_nand_data *this)
>   	if (ret)
>   		goto exit_dma_channels;
>
> -	pinctrl = devm_pinctrl_get_select_default(&this->pdev->dev);
> -	if (IS_ERR(pinctrl)) {
> -		ret = PTR_ERR(pinctrl);
> -		goto exit_pin;
> -	}
> -
>   	ret = gpmi_get_clks(this);
>   	if (ret)
>   		goto exit_clock;
>   	return 0;
>
>   exit_clock:
> -exit_pin:
>   	release_dma_channels(this);
>   exit_dma_channels:
>   	release_bch_irq(this);

The similar patches have been sent for several times.
So this is a duplicated patch.

thanks
Huang Shijie

Re: [PATCH 05/25] mtd: nand: gpmi-nand: use more sensible error codes at various places

[Huang Shijie]

于 2013年07月31日 22:11, Lothar Waßmann 写道:
> Signed-off-by: Lothar Waßmann<LW@KARO-electronics.de>
> ---
>   drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    6 +++---
>   1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 4f7ec59..9a47189 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -355,7 +355,7 @@ static int acquire_register_block(struct gpmi_nand_data *this,
>   	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, res_name);
>   	if (!r) {
>   		pr_err("Can't get resource for %s\n", res_name);
> -		return -ENXIO;
> +		return -ENODEV;
>   	}
>
>   	p = ioremap(r->start, resource_size(r));
> @@ -396,7 +396,7 @@ static int acquire_bch_irq(struct gpmi_nand_data *this, irq_handler_t irq_h)
>   	r = platform_get_resource_byname(pdev, IORESOURCE_IRQ, res_name);
>   	if (!r) {
>   		pr_err("Can't get resource for %s\n", res_name);
> -		return -ENXIO;
> +		return -ENODEV;
>   	}
>
>   	err = request_irq(r->start, irq_h, 0, res_name, this);
> @@ -1597,7 +1597,7 @@ static int gpmi_nand_probe(struct platform_device *pdev)
>   		pdev->id_entry = of_id->data;
>   	} else {
>   		pr_err("Failed to find the right device id.\n");
> -		return -ENOMEM;
> +		return -ENODEV;
>   	}
>
>   	this = kzalloc(sizeof(*this), GFP_KERNEL);
Acked-by: Huang Shijie <b32955@freescale.com>

Re: [PATCH 04/25] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[Huang Shijie]

于 2013年07月31日 22:11, Lothar Waßmann 写道:
> Signed-off-by: Lothar Waßmann<LW@KARO-electronics.de>
> ---
>   drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    8 ++++----
>   1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index f557a1d..4f7ec59 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1569,19 +1569,19 @@ static const struct platform_device_id gpmi_ids[] = {
>   	{ .name = "imx23-gpmi-nand", .driver_data = IS_MX23, },
>   	{ .name = "imx28-gpmi-nand", .driver_data = IS_MX28, },
>   	{ .name = "imx6q-gpmi-nand", .driver_data = IS_MX6Q, },
> -	{},
> +	{}
>   };
>
>   static const struct of_device_id gpmi_nand_id_table[] = {
>   	{
>   		.compatible = "fsl,imx23-gpmi-nand",
> -		.data = (void *)&gpmi_ids[IS_MX23]
> +		.data = (void *)&gpmi_ids[IS_MX23],
>   	}, {
>   		.compatible = "fsl,imx28-gpmi-nand",
> -		.data = (void *)&gpmi_ids[IS_MX28]
> +		.data = (void *)&gpmi_ids[IS_MX28],
>   	}, {
>   		.compatible = "fsl,imx6q-gpmi-nand",
> -		.data = (void *)&gpmi_ids[IS_MX6Q]
> +		.data = (void *)&gpmi_ids[IS_MX6Q],
>   	}, {}
>   };
>   MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
is it any bad influence if we do not apply this patch?

I am not clear what we can benefit from this patch.

thanks
Huang Shijie

Re: [PATCH 04/25] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[Lothar Waßmann]

Hi,

Huang Shijie writes:
> 于 2013年07月31日 22:11, Lothar Waßmann 写道:
> > Signed-off-by: Lothar Waßmann<LW@KARO-electronics.de>
> > ---
> >   drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    8 ++++----
> >   1 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > index f557a1d..4f7ec59 100644
> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > @@ -1569,19 +1569,19 @@ static const struct platform_device_id gpmi_ids[] = {
> >   	{ .name = "imx23-gpmi-nand", .driver_data = IS_MX23, },
> >   	{ .name = "imx28-gpmi-nand", .driver_data = IS_MX28, },
> >   	{ .name = "imx6q-gpmi-nand", .driver_data = IS_MX6Q, },
> > -	{},
> > +	{}
> >   };
> >
> >   static const struct of_device_id gpmi_nand_id_table[] = {
> >   	{
> >   		.compatible = "fsl,imx23-gpmi-nand",
> > -		.data = (void *)&gpmi_ids[IS_MX23]
> > +		.data = (void *)&gpmi_ids[IS_MX23],
> >   	}, {
> >   		.compatible = "fsl,imx28-gpmi-nand",
> > -		.data = (void *)&gpmi_ids[IS_MX28]
> > +		.data = (void *)&gpmi_ids[IS_MX28],
> >   	}, {
> >   		.compatible = "fsl,imx6q-gpmi-nand",
> > -		.data = (void *)&gpmi_ids[IS_MX6Q]
> > +		.data = (void *)&gpmi_ids[IS_MX6Q],
> >   	}, {}
> >   };
> >   MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
> is it any bad influence if we do not apply this patch?
> 
> I am not clear what we can benefit from this patch.
> 
less eye sore when looking at the code and eventual copies of it in
other places.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

Re: [PATCH 04/25] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[Huang Shijie]

于 2013年08月01日 13:50, Lothar Waßmann 写道:
> Hi,
>
> Huang Shijie writes:
>> 于 2013年07月31日 22:11, Lothar Waßmann 写道:
>>> Signed-off-by: Lothar Waßmann<LW@KARO-electronics.de>
>>> ---
>>>    drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    8 ++++----
>>>    1 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> index f557a1d..4f7ec59 100644
>>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>>> @@ -1569,19 +1569,19 @@ static const struct platform_device_id gpmi_ids[] = {
>>>    	{ .name = "imx23-gpmi-nand", .driver_data = IS_MX23, },
>>>    	{ .name = "imx28-gpmi-nand", .driver_data = IS_MX28, },
>>>    	{ .name = "imx6q-gpmi-nand", .driver_data = IS_MX6Q, },
>>> -	{},
>>> +	{}
>>>    };
>>>
>>>    static const struct of_device_id gpmi_nand_id_table[] = {
>>>    	{
>>>    		.compatible = "fsl,imx23-gpmi-nand",
>>> -		.data = (void *)&gpmi_ids[IS_MX23]
>>> +		.data = (void *)&gpmi_ids[IS_MX23],
>>>    	}, {
>>>    		.compatible = "fsl,imx28-gpmi-nand",
>>> -		.data = (void *)&gpmi_ids[IS_MX28]
>>> +		.data = (void *)&gpmi_ids[IS_MX28],
>>>    	}, {
>>>    		.compatible = "fsl,imx6q-gpmi-nand",
>>> -		.data = (void *)&gpmi_ids[IS_MX6Q]
>>> +		.data = (void *)&gpmi_ids[IS_MX6Q],
>>>    	}, {}
>>>    };
>>>    MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
>> is it any bad influence if we do not apply this patch?
>>
>> I am not clear what we can benefit from this patch.
>>
> less eye sore when looking at the code and eventual copies of it in
> other places.
>
ok. thanks for fix this.

Acked-by: Huang Shijie <b32955@freescale.com>

Re: [PATCH 04/25] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[Artem Bityutskiy]

On Thu, 2013-08-01 at 07:50 +0200, Lothar Waßmann wrote:
> Hi,
> 
> Huang Shijie writes:
> > 于 2013年07月31日 22:11, Lothar Waßmann 写道:
> > > Signed-off-by: Lothar Waßmann<LW@KARO-electronics.de>
> > > ---
> > >   drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    8 ++++----
> > >   1 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > > index f557a1d..4f7ec59 100644
> > > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > > @@ -1569,19 +1569,19 @@ static const struct platform_device_id gpmi_ids[] = {
> > >   	{ .name = "imx23-gpmi-nand", .driver_data = IS_MX23, },
> > >   	{ .name = "imx28-gpmi-nand", .driver_data = IS_MX28, },
> > >   	{ .name = "imx6q-gpmi-nand", .driver_data = IS_MX6Q, },
> > > -	{},
> > > +	{}
> > >   };
> > >
> > >   static const struct of_device_id gpmi_nand_id_table[] = {
> > >   	{
> > >   		.compatible = "fsl,imx23-gpmi-nand",
> > > -		.data = (void *)&gpmi_ids[IS_MX23]
> > > +		.data = (void *)&gpmi_ids[IS_MX23],
> > >   	}, {
> > >   		.compatible = "fsl,imx28-gpmi-nand",
> > > -		.data = (void *)&gpmi_ids[IS_MX28]
> > > +		.data = (void *)&gpmi_ids[IS_MX28],
> > >   	}, {
> > >   		.compatible = "fsl,imx6q-gpmi-nand",
> > > -		.data = (void *)&gpmi_ids[IS_MX6Q]
> > > +		.data = (void *)&gpmi_ids[IS_MX6Q],
> > >   	}, {}
> > >   };
> > >   MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
> > is it any bad influence if we do not apply this patch?
> > 
> > I am not clear what we can benefit from this patch.
> > 
> less eye sore when looking at the code and eventual copies of it in
> other places.

For some reasons I do not have these patches in my mailbox. Would you
please re-send them, and add the acks there too?

-- 
Best Regards,
Artem Bityutskiy

[PATCH 03/25] mtd: nand: gpmi-nand: let the driver core handle pinctrl

[Lothar Waßmann]

without this patch the driver fails to load with -EBUSY.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index b741a6c..f557a1d 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -520,7 +520,6 @@ err_clock:
 
 static int acquire_resources(struct gpmi_nand_data *this)
 {
-	struct pinctrl *pinctrl;
 	int ret;
 
 	ret = acquire_register_block(this, GPMI_NAND_GPMI_REGS_ADDR_RES_NAME);
@@ -539,19 +538,12 @@ static int acquire_resources(struct gpmi_nand_data *this)
 	if (ret)
 		goto exit_dma_channels;
 
-	pinctrl = devm_pinctrl_get_select_default(&this->pdev->dev);
-	if (IS_ERR(pinctrl)) {
-		ret = PTR_ERR(pinctrl);
-		goto exit_pin;
-	}
-
 	ret = gpmi_get_clks(this);
 	if (ret)
 		goto exit_clock;
 	return 0;
 
 exit_clock:
-exit_pin:
 	release_dma_channels(this);
 exit_dma_channels:
 	release_bch_irq(this);
-- 
1.7.2.5

[PATCH/RESEND 1/2] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[Lothar Waßmann]

Acked-by: Huang Shijie <b32955@freescale.com>
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index f557a1d..4f7ec59 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1569,19 +1569,19 @@ static const struct platform_device_id gpmi_ids[] = {
 	{ .name = "imx23-gpmi-nand", .driver_data = IS_MX23, },
 	{ .name = "imx28-gpmi-nand", .driver_data = IS_MX28, },
 	{ .name = "imx6q-gpmi-nand", .driver_data = IS_MX6Q, },
-	{},
+	{}
 };
 
 static const struct of_device_id gpmi_nand_id_table[] = {
 	{
 		.compatible = "fsl,imx23-gpmi-nand",
-		.data = (void *)&gpmi_ids[IS_MX23]
+		.data = (void *)&gpmi_ids[IS_MX23],
 	}, {
 		.compatible = "fsl,imx28-gpmi-nand",
-		.data = (void *)&gpmi_ids[IS_MX28]
+		.data = (void *)&gpmi_ids[IS_MX28],
 	}, {
 		.compatible = "fsl,imx6q-gpmi-nand",
-		.data = (void *)&gpmi_ids[IS_MX6Q]
+		.data = (void *)&gpmi_ids[IS_MX6Q],
 	}, {}
 };
 MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
-- 
1.7.2.5

[PATCH/RESEND v2 0/2] mtd: nand: gpmi-nand: cleanup patches

[Lothar Waßmann]

Sorry, sent out the wrong files.

Re: [PATCH/RESEND v2 0/2] mtd: nand: gpmi-nand: cleanup patches

[Artem Bityutskiy]

On Wed, 2013-08-07 at 08:15 +0200, Lothar Waßmann wrote:
> Sorry, sent out the wrong files.

Pushed both to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

[PATCH/RESEND 1/2] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[Lothar Waßmann]

Acked-by: Huang Shijie <b32955@freescale.com>
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index f557a1d..4f7ec59 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1569,19 +1569,19 @@ static const struct platform_device_id gpmi_ids[] = {
 	{ .name = "imx23-gpmi-nand", .driver_data = IS_MX23, },
 	{ .name = "imx28-gpmi-nand", .driver_data = IS_MX28, },
 	{ .name = "imx6q-gpmi-nand", .driver_data = IS_MX6Q, },
-	{},
+	{}
 };
 
 static const struct of_device_id gpmi_nand_id_table[] = {
 	{
 		.compatible = "fsl,imx23-gpmi-nand",
-		.data = (void *)&gpmi_ids[IS_MX23]
+		.data = (void *)&gpmi_ids[IS_MX23],
 	}, {
 		.compatible = "fsl,imx28-gpmi-nand",
-		.data = (void *)&gpmi_ids[IS_MX28]
+		.data = (void *)&gpmi_ids[IS_MX28],
 	}, {
 		.compatible = "fsl,imx6q-gpmi-nand",
-		.data = (void *)&gpmi_ids[IS_MX6Q]
+		.data = (void *)&gpmi_ids[IS_MX6Q],
 	}, {}
 };
 MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
-- 
1.7.2.5

[PATCH/RESEND 2/2] mtd: nand: gpmi-nand: use more sensible error codes at various places

[Lothar Waßmann]

Acked-by: Huang Shijie <b32955@freescale.com>
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 4f7ec59..9a47189 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -355,7 +355,7 @@ static int acquire_register_block(struct gpmi_nand_data *this,
 	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, res_name);
 	if (!r) {
 		pr_err("Can't get resource for %s\n", res_name);
-		return -ENXIO;
+		return -ENODEV;
 	}
 
 	p = ioremap(r->start, resource_size(r));
@@ -396,7 +396,7 @@ static int acquire_bch_irq(struct gpmi_nand_data *this, irq_handler_t irq_h)
 	r = platform_get_resource_byname(pdev, IORESOURCE_IRQ, res_name);
 	if (!r) {
 		pr_err("Can't get resource for %s\n", res_name);
-		return -ENXIO;
+		return -ENODEV;
 	}
 
 	err = request_irq(r->start, irq_h, 0, res_name, this);
@@ -1597,7 +1597,7 @@ static int gpmi_nand_probe(struct platform_device *pdev)
 		pdev->id_entry = of_id->data;
 	} else {
 		pr_err("Failed to find the right device id.\n");
-		return -ENOMEM;
+		return -ENODEV;
 	}
 
 	this = kzalloc(sizeof(*this), GFP_KERNEL);
-- 
1.7.2.5

Re: [PATCH 04/25] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1799 bytes --]

On Thu, 2013-08-01 at 07:50 +0200, Lothar Waßmann wrote:
> Huang Shijie writes:
> > is it any bad influence if we do not apply this patch?
> > 
> > I am not clear what we can benefit from this patch.
> > 
> less eye sore when looking at the code and eventual copies of it in
> other places.

But it's actually a *good* practice to have the redundant commas at the
end of the line, in the general case.

It does absolutely no harm, and makes the final line appear *consistent*
with the other lines. I don't see why it would make your eye sore on the
final line when it's there and necessary on the previous lines.

It's also not as if you're going to miss the closing brace and the fact
that it's the end of the struct. The indentation should always make that
clear regardless of the comma.

If we don't include it, we end up with patches which are less readable,
when we add a field to an initialiser. Instead of this nice and obvious
patch, for example:

@@ 1,4 1,5
 struct wombat = {
 	.somefield = foo,bar
	.otherfield = bar,
+	.newfield = baz,
 };

... we end up with this less obvious and harder-to-read patch:

@@ 1,4 1,5
 struct wombat = {
 	.somefield = foo,
-	.otherfield = bar
+	.otherfield = bar,
+	.newfield = baz
 };



Now, admittedly that's the *general* case, and the ones you've removed
commas from are very unlikely to ever have new members added after them.
Especially the one that's a NULL sentinel at the end of an ID match
list. But I'm just not sure I approve of the concept of removing the
commas. I'm happier with them present, and a general habit of including
them.


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]

Re: [PATCH 04/25] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[Huang Shijie]

On Fri, Aug 30, 2013 at 05:13:09PM +0100, David Woodhouse wrote:
> On Thu, 2013-08-01 at 07:50 +0200, Lothar Waßmann wrote:
> 
> Now, admittedly that's the *general* case, and the ones you've removed
> commas from are very unlikely to ever have new members added after them.
> Especially the one that's a NULL sentinel at the end of an ID match
> list. But I'm just not sure I approve of the concept of removing the
> commas. I'm happier with them present, and a general habit of including
> them.

Hi David:
   thanks for explanations.

   If you do not like this patch, you can remove it.


thanks
Huang Shijie

Re: [PATCH 04/25] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[Lothar Waßmann]

Hi,

David Woodhouse writes:
> On Thu, 2013-08-01 at 07:50 +0200, Lothar Waßmann wrote:
> > Huang Shijie writes:
> > > is it any bad influence if we do not apply this patch?
> > > 
> > > I am not clear what we can benefit from this patch.
> > > 
> > less eye sore when looking at the code and eventual copies of it in
> > other places.
> 
> But it's actually a *good* practice to have the redundant commas at the
> end of the line, in the general case.
> 
> It does absolutely no harm, and makes the final line appear *consistent*
> with the other lines. I don't see why it would make your eye sore on the
> final line when it's there and necessary on the previous lines.
> 
My patch _added_ the commas and _made_ the last lines consistent with
the other ones...


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

Re: [PATCH 04/25] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

On Mon, 2013-09-02 at 08:14 +0200, Lothar Waßmann wrote:
> My patch _added_ the commas and _made_ the last lines consistent with
> the other ones...

Ah, right. I looked at the first hunk of the patch, which *is* actually
removing a comma. Missed the fact that the other three changes were
*adding* commas instead.

So much for consistency :)

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]