[PATCH 1/1] atmel/spi: fix missing probe during the switch to module_platform_driver

[PATCH 1/1] atmel/spi: fix missing probe during the switch to module_platform_driver

[Jean-Christophe PLAGNIOL-VILLARD]

in commit 940ab889

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 arch/arm/mach-at91/include/mach/at91_aic.h |    2 +-
 drivers/spi/spi-atmel.c                    |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-at91/include/mach/at91_aic.h b/arch/arm/mach-at91/include/mach/at91_aic.h
index 4b0a57e..f163bb6 100644
--- a/arch/arm/mach-at91/include/mach/at91_aic.h
+++ b/arch/arm/mach-at91/include/mach/at91_aic.h
@@ -20,7 +20,7 @@
 extern void __iomem *at91_aic_base;
 
 #define at91_aic_read(field) \
-	__raw_read(at91_aic_base + field)
+	__raw_readl(at91_aic_base + field)
 
 #define at91_aic_write(field, value) \
 	__raw_writel(value, at91_aic_base + field);
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 79665e2..a691240 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1072,6 +1072,7 @@ static struct platform_driver atmel_spi_driver = {
 	},
 	.suspend	= atmel_spi_suspend,
 	.resume		= atmel_spi_resume,
+	.probe		= atmel_spi_probe,
 	.remove		= __exit_p(atmel_spi_remove),
 };
 module_platform_driver(atmel_spi_driver);
-- 
1.7.7

Re: [PATCH 1/1] atmel/spi: fix missing probe during the switch to module_platform_driver

[Deepak Saxena]

On Thu, Nov 3, 2011 at 10:48 AM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> in commit 940ab889
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>  arch/arm/mach-at91/include/mach/at91_aic.h |    2 +-
>  drivers/spi/spi-atmel.c                    |    1 +
>  2 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-at91/include/mach/at91_aic.h b/arch/arm/mach-at91/include/mach/at91_aic.h
> index 4b0a57e..f163bb6 100644
> --- a/arch/arm/mach-at91/include/mach/at91_aic.h
> +++ b/arch/arm/mach-at91/include/mach/at91_aic.h
> @@ -20,7 +20,7 @@
>  extern void __iomem *at91_aic_base;
>
>  #define at91_aic_read(field) \
> -       __raw_read(at91_aic_base + field)
> +       __raw_readl(at91_aic_base + field)
>
>  #define at91_aic_write(field, value) \
>        __raw_writel(value, at91_aic_base + field);
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 79665e2..a691240 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -1072,6 +1072,7 @@ static struct platform_driver atmel_spi_driver = {
>        },
>        .suspend        = atmel_spi_suspend,
>        .resume         = atmel_spi_resume,
> +       .probe          = atmel_spi_probe,
>        .remove         = __exit_p(atmel_spi_remove),
>  };
>  module_platform_driver(atmel_spi_driver);

Jean,

This looks it should be two separate patches, one for the missing probe,
one for the raw_read -> raw_readl.

Thanks,
~Deepak

Re: [PATCH 1/1] atmel/spi: fix missing probe during the switch to module_platform_driver

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:55 Thu 03 Nov     , Deepak Saxena wrote:
> On Thu, Nov 3, 2011 at 10:48 AM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > in commit 940ab889
> >
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Greg Kroah-Hartman <gregkh@suse.de>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> > ---
> >  arch/arm/mach-at91/include/mach/at91_aic.h |    2 +-
> >  drivers/spi/spi-atmel.c                    |    1 +
> >  2 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-at91/include/mach/at91_aic.h b/arch/arm/mach-at91/include/mach/at91_aic.h
> > index 4b0a57e..f163bb6 100644
> > --- a/arch/arm/mach-at91/include/mach/at91_aic.h
> > +++ b/arch/arm/mach-at91/include/mach/at91_aic.h
> > @@ -20,7 +20,7 @@
> >  extern void __iomem *at91_aic_base;
> >
> >  #define at91_aic_read(field) \
> > -       __raw_read(at91_aic_base + field)
> > +       __raw_readl(at91_aic_base + field)
> >
> >  #define at91_aic_write(field, value) \
> >        __raw_writel(value, at91_aic_base + field);
> > diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> > index 79665e2..a691240 100644
> > --- a/drivers/spi/spi-atmel.c
> > +++ b/drivers/spi/spi-atmel.c
> > @@ -1072,6 +1072,7 @@ static struct platform_driver atmel_spi_driver = {
> >        },
> >        .suspend        = atmel_spi_suspend,
> >        .resume         = atmel_spi_resume,
> > +       .probe          = atmel_spi_probe,
> >        .remove         = __exit_p(atmel_spi_remove),
> >  };
> >  module_platform_driver(atmel_spi_driver);
> 
> Jean,
> 
> This looks it should be two separate patches, one for the missing probe,
> one for the raw_read -> raw_readl.
Yes I found when I receive the e-mail and already send the v2

Best Regards,
J.

[PATCH v2] atmel/spi: fix missing probe during the switch to module_platform_driver

[Jean-Christophe PLAGNIOL-VILLARD]

in commit 940ab889

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
v2:

	remove local work

Best Regards,
J.
 drivers/spi/spi-atmel.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 79665e2..a691240 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1072,6 +1072,7 @@ static struct platform_driver atmel_spi_driver = {
 	},
 	.suspend	= atmel_spi_suspend,
 	.resume		= atmel_spi_resume,
+	.probe		= atmel_spi_probe,
 	.remove		= __exit_p(atmel_spi_remove),
 };
 module_platform_driver(atmel_spi_driver);
-- 
1.7.7

Re: [PATCH v2] atmel/spi: fix missing probe during the switch to module_platform_driver

[Russell King - ARM Linux]

On Thu, Nov 03, 2011 at 11:10:42PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> in commit 940ab889

This kind of commit message is, frankly, utter crap.

With all the complaints that have been on this mailing list about raw
commit IDs without saying what the commit was, it completely astounds
me that someone in your position (allegedly a co-maintainer of a SoC)
would make such an error.

Please generate a much better commit message.  Maybe something like:

----
Commit 940ab889 (blah blah blah) converted this driver to use
module_platform_driver, but due to the use of platform_driver_probe(),
this resulted in the call to atmel_spi_probe being lost.  Place the
call to this function into the driver structure.

As atmel_spi_probe is marked __init, this will cause a section mismatch
error, so ... <fill this in for the point below>
----

And, you're forgetting to fix this properly (maybe you don't care about
section mismatch errors - are you going to create another patch for that
as well?)  Did you even build-test this patch and pay attention to any
warnings issued, or were you just pleased to get a zImage at the end?

static int __init atmel_spi_probe(struct platform_device *pdev)

__init is not compatible with having a pointer in the platform driver
structure.  It needs to be __devinit.

Re: [PATCH v2] atmel/spi: fix missing probe during the switch to module_platform_driver

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:18 Thu 03 Nov     , Russell King - ARM Linux wrote:
> On Thu, Nov 03, 2011 at 11:10:42PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > in commit 940ab889
> 
> This kind of commit message is, frankly, utter crap.
> 
> With all the complaints that have been on this mailing list about raw
> commit IDs without saying what the commit was, it completely astounds
> me that someone in your position (allegedly a co-maintainer of a SoC)
> would make such an error.
> 
> Please generate a much better commit message.  Maybe something like:
> 
> ----
> Commit 940ab889 (blah blah blah) converted this driver to use
> module_platform_driver, but due to the use of platform_driver_probe(),
> this resulted in the call to atmel_spi_probe being lost.  Place the
> call to this function into the driver structure.
> 
> As atmel_spi_probe is marked __init, this will cause a section mismatch
> error, so ... <fill this in for the point below>
> ----
> 
> And, you're forgetting to fix this properly (maybe you don't care about
> section mismatch errors - are you going to create another patch for that
> as well?)  Did you even build-test this patch and pay attention to any
> warnings issued, or were you just pleased to get a zImage at the end?
> 
> static int __init atmel_spi_probe(struct platform_device *pdev)
> 
> __init is not compatible with having a pointer in the platform driver
> structure.  It needs to be __devinit.
this is an other fix I do not want to fix 2 issue in one commit and I do care
of mismatch erros I fix them all the time I send patch for some rm9200 few
weeks ago

Best Regards,
J.

Re: [PATCH v2] atmel/spi: fix missing probe during the switch to module_platform_driver

[Russell King - ARM Linux]

On Thu, Nov 03, 2011 at 05:51:13PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> this is an other fix I do not want to fix 2 issue in one commit and I do care
> of mismatch erros I fix them all the time I send patch for some rm9200 few
> weeks ago

It should not be a separate patch - it's all to do with fixing the
original problem.

Re: [PATCH v2] atmel/spi: fix missing probe during the switch to module_platform_driver

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:43 Thu 03 Nov     , Russell King - ARM Linux wrote:
> On Thu, Nov 03, 2011 at 05:51:13PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > this is an other fix I do not want to fix 2 issue in one commit and I do care
> > of mismatch erros I fix them all the time I send patch for some rm9200 few
> > weeks ago
> 
> It should not be a separate patch - it's all to do with fixing the
> original problem.

yes when we was supposed to swtch to module_platform_driver we use the .probe
which is suposed to be __devinit in this case but I was prefering to do it in
2 steps

as you wish and I found that the remove is __exit and supposed to be __devexit

Best Regards,
J.

Re: [PATCH v2] atmel/spi: fix missing probe during the switch to module_platform_driver

[Russell King - ARM Linux]

On Thu, Nov 03, 2011 at 07:03:38PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:43 Thu 03 Nov     , Russell King - ARM Linux wrote:
> > On Thu, Nov 03, 2011 at 05:51:13PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > this is an other fix I do not want to fix 2 issue in one commit and I do care
> > > of mismatch erros I fix them all the time I send patch for some rm9200 few
> > > weeks ago
> > 
> > It should not be a separate patch - it's all to do with fixing the
> > original problem.
> 
> yes when we was supposed to swtch to module_platform_driver we use the .probe
> which is suposed to be __devinit in this case but I was prefering to do it in
> 2 steps

Then do the __devinit transformation _first_ so you don't introduce a new
bug in the middle of your 'fix'.

But I personally think you're just creating needless churn by splitting
this one logical change.