[PATCH 0/2] Add pinctrl support to omap2-mcspi

[PATCH 0/2] Add pinctrl support to omap2-mcspi

[Matt Porter]

This series enables pinctrl support for McSPI. Platforms that boot only
from DT and rely on pinctrl to set pinmuxes appropriately require this
for omap2-mcspi operation.

It has been tested on AM335x BeagleBone with an Adafruit SPI LCD attached
and regression tested on Beagleboard xM booting in the !DT case, using
board-omap3evm.c. There is a dependency on the patch to enable PINCTRL
support (https://patchwork.kernel.org/patch/1376331/) for
ARCH_OMAP2PLUS_TYPICAL.

Matt Porter (2):
  spi: omap2-mcspi: add pinctrl support
  ARM: OMAP2+: Enable pinctrl dummy states

 arch/arm/mach-omap2/devices.c |    4 ++++
 drivers/spi/spi-omap2-mcspi.c |    9 +++++++++
 2 files changed, 13 insertions(+)

-- 
1.7.9.5

[PATCH 1/2] spi: omap2-mcspi: add pinctrl support

[Matt Porter]

Adds pinctrl support to support OMAP platforms that boot from DT
and rely on pinctrl support to set pinmuxes.

Signed-off-by: Matt Porter <mporter@ti.com>
---
 drivers/spi/spi-omap2-mcspi.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index b2fb141..6c67cdb 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -38,6 +38,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/err.h>
 
 #include <linux/spi/spi.h>
 
@@ -1124,6 +1126,7 @@ static int __devinit omap2_mcspi_probe(struct platform_device *pdev)
 	static int		bus_num = 1;
 	struct device_node	*node = pdev->dev.of_node;
 	const struct of_device_id *match;
+	struct pinctrl *pinctrl;
 
 	master = spi_alloc_master(&pdev->dev, sizeof *mcspi);
 	if (master == NULL) {
@@ -1219,6 +1222,12 @@ static int __devinit omap2_mcspi_probe(struct platform_device *pdev)
 	if (status < 0)
 		goto dma_chnl_free;
 
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl)) {
+		status = PTR_ERR(pinctrl);
+		goto dma_chnl_free;
+	}
+
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, SPI_AUTOSUSPEND_TIMEOUT);
 	pm_runtime_enable(&pdev->dev);
-- 
1.7.9.5

[PATCH 2/2] ARM: OMAP2+: Enable pinctrl dummy states

[Matt Porter]

Enable pinctrl dummy states for all OMAP platforms. This allows
drivers to be converted to pinctrl while still running on
platforms that do not provide pinctrl data.

Signed-off-by: Matt Porter <mporter@ti.com>
---
 arch/arm/mach-omap2/devices.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index c00c689..577cd04 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -17,6 +17,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/pinctrl/machine.h>
 #include <linux/platform_data/omap4-keypad.h>
 
 #include <mach/hardware.h>
@@ -631,6 +632,9 @@ static inline void omap_init_vout(void) {}
 
 static int __init omap2_init_devices(void)
 {
+	/* Enable dummy states for those platforms without pinctrl support */
+	pinctrl_provide_dummies();
+
 	/*
 	 * please keep these calls, and their implementations above,
 	 * in alphabetical order so they're easier to sort through.
-- 
1.7.9.5

Re: [PATCH 1/2] spi: omap2-mcspi: add pinctrl support

[Tony Lindgren]

* Matt Porter <mporter@ti.com> [120911 10:46]:
> Adds pinctrl support to support OMAP platforms that boot from DT
> and rely on pinctrl support to set pinmuxes.
> 
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
>  drivers/spi/spi-omap2-mcspi.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
> index b2fb141..6c67cdb 100644
> --- a/drivers/spi/spi-omap2-mcspi.c
> +++ b/drivers/spi/spi-omap2-mcspi.c
> @@ -38,6 +38,8 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/err.h>
>  
>  #include <linux/spi/spi.h>
>  
> @@ -1124,6 +1126,7 @@ static int __devinit omap2_mcspi_probe(struct platform_device *pdev)
>  	static int		bus_num = 1;
>  	struct device_node	*node = pdev->dev.of_node;
>  	const struct of_device_id *match;
> +	struct pinctrl *pinctrl;
>  
>  	master = spi_alloc_master(&pdev->dev, sizeof *mcspi);
>  	if (master == NULL) {
> @@ -1219,6 +1222,12 @@ static int __devinit omap2_mcspi_probe(struct platform_device *pdev)
>  	if (status < 0)
>  		goto dma_chnl_free;
>  
> +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> +	if (IS_ERR(pinctrl)) {
> +		status = PTR_ERR(pinctrl);
> +		goto dma_chnl_free;
> +	}
> +
>  	pm_runtime_use_autosuspend(&pdev->dev);
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, SPI_AUTOSUSPEND_TIMEOUT);
>  	pm_runtime_enable(&pdev->dev);

You should just print out a warning here as most boards don't
have pinctrl implemented at this point, or may never have.

Regards,

Tony

Re: [PATCH 2/2] ARM: OMAP2+: Enable pinctrl dummy states

[Tony Lindgren]

* Matt Porter <mporter@ti.com> [120911 10:46]:
> Enable pinctrl dummy states for all OMAP platforms. This allows
> drivers to be converted to pinctrl while still running on
> platforms that do not provide pinctrl data.
> 
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
>  arch/arm/mach-omap2/devices.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index c00c689..577cd04 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -17,6 +17,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
> +#include <linux/pinctrl/machine.h>
>  #include <linux/platform_data/omap4-keypad.h>
>  
>  #include <mach/hardware.h>
> @@ -631,6 +632,9 @@ static inline void omap_init_vout(void) {}
>  
>  static int __init omap2_init_devices(void)
>  {
> +	/* Enable dummy states for those platforms without pinctrl support */
> +	pinctrl_provide_dummies();
> +
>  	/*
>  	 * please keep these calls, and their implementations above,
>  	 * in alphabetical order so they're easier to sort through.

Hmm I think this may need to be board specific. And may need to
be board specific and depend on unpopulated device tree?

For board-generic.c we always want to see the warnings. And some boards
insist on doing all the muxing only in the bootloader.

Regards,

Tony

Re: [PATCH 1/2] spi: omap2-mcspi: add pinctrl support

[Matt Porter]

On Tue, Sep 11, 2012 at 11:00:41AM -0700, Tony Lindgren wrote:
> * Matt Porter <mporter@ti.com> [120911 10:46]:
> > Adds pinctrl support to support OMAP platforms that boot from DT
> > and rely on pinctrl support to set pinmuxes.
> > 
> > Signed-off-by: Matt Porter <mporter@ti.com>
> > ---
> >  drivers/spi/spi-omap2-mcspi.c |    9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
> > index b2fb141..6c67cdb 100644
> > --- a/drivers/spi/spi-omap2-mcspi.c
> > +++ b/drivers/spi/spi-omap2-mcspi.c
> > @@ -38,6 +38,8 @@
> >  #include <linux/pm_runtime.h>
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> > +#include <linux/pinctrl/consumer.h>
> > +#include <linux/err.h>
> >  
> >  #include <linux/spi/spi.h>
> >  
> > @@ -1124,6 +1126,7 @@ static int __devinit omap2_mcspi_probe(struct platform_device *pdev)
> >  	static int		bus_num = 1;
> >  	struct device_node	*node = pdev->dev.of_node;
> >  	const struct of_device_id *match;
> > +	struct pinctrl *pinctrl;
> >  
> >  	master = spi_alloc_master(&pdev->dev, sizeof *mcspi);
> >  	if (master == NULL) {
> > @@ -1219,6 +1222,12 @@ static int __devinit omap2_mcspi_probe(struct platform_device *pdev)
> >  	if (status < 0)
> >  		goto dma_chnl_free;
> >  
> > +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> > +	if (IS_ERR(pinctrl)) {
> > +		status = PTR_ERR(pinctrl);
> > +		goto dma_chnl_free;
> > +	}
> > +
> >  	pm_runtime_use_autosuspend(&pdev->dev);
> >  	pm_runtime_set_autosuspend_delay(&pdev->dev, SPI_AUTOSUSPEND_TIMEOUT);
> >  	pm_runtime_enable(&pdev->dev);
> 
> You should just print out a warning here as most boards don't
> have pinctrl implemented at this point, or may never have.

It will not hit this error path on the boards without pinctrl due
to the second part of the series. The error check is for some really
unexpected failure in the pinctrl core and parsing functionality where
we want it to bail out.

-Matt

Re: [PATCH 2/2] ARM: OMAP2+: Enable pinctrl dummy states

[Matt Porter]

On Tue, Sep 11, 2012 at 11:03:06AM -0700, Tony Lindgren wrote:
> * Matt Porter <mporter@ti.com> [120911 10:46]:
> > Enable pinctrl dummy states for all OMAP platforms. This allows
> > drivers to be converted to pinctrl while still running on
> > platforms that do not provide pinctrl data.
> > 
> > Signed-off-by: Matt Porter <mporter@ti.com>
> > ---
> >  arch/arm/mach-omap2/devices.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> > index c00c689..577cd04 100644
> > --- a/arch/arm/mach-omap2/devices.c
> > +++ b/arch/arm/mach-omap2/devices.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/err.h>
> >  #include <linux/slab.h>
> >  #include <linux/of.h>
> > +#include <linux/pinctrl/machine.h>
> >  #include <linux/platform_data/omap4-keypad.h>
> >  
> >  #include <mach/hardware.h>
> > @@ -631,6 +632,9 @@ static inline void omap_init_vout(void) {}
> >  
> >  static int __init omap2_init_devices(void)
> >  {
> > +	/* Enable dummy states for those platforms without pinctrl support */
> > +	pinctrl_provide_dummies();
> > +
> >  	/*
> >  	 * please keep these calls, and their implementations above,
> >  	 * in alphabetical order so they're easier to sort through.
> 
> Hmm I think this may need to be board specific. And may need to
> be board specific and depend on unpopulated device tree?

If I run this on AM33xx with dummy states enabled, I'm able to override
that dummy state just fine with an appropriate pinctl/pinmux entry in my
DT data for spi.

Meanwhile on xM booting from board-omap3evm.c/!DT and debug enabled I
get the following:

[    1.837829] omap2_mcspi omap2_mcspi.1: no of_node; not parsing
pinctrl DT
[    1.845214] omap2_mcspi omap2_mcspi.1: using pinctrl dummy state
(default)
[    1.854278] omap2_mcspi omap2_mcspi.2: no of_node; not parsing
pinctrl DT
[    1.861572] omap2_mcspi omap2_mcspi.2: using pinctrl dummy state
(default)
[    1.870025] omap2_mcspi omap2_mcspi.3: no of_node; not parsing
pinctrl DT
[    1.877288] omap2_mcspi omap2_mcspi.3: using pinctrl dummy state
(default)
[    1.885681] omap2_mcspi omap2_mcspi.4: no of_node; not parsing
pinctrl DT
[    1.892913] omap2_mcspi omap2_mcspi.4: using pinctrl dummy state
(default)

which seems to cover things being informative enough about what is
going on. Are you wanting to see an explicit warning that the pinctrl
dummy state is in use when pinctrl data is not available?
 
> For board-generic.c we always want to see the warnings. And some boards
> insist on doing all the muxing only in the bootloader.

Which warnings are you saying we should see in the board-generic.c
case?  Sure, there's plenty of cases where this will be unused due to
somebody setting all the muxes in the bootloader and then not using
pinctrl data. I'll have to doublecheck but I believe that case is also
fine as the -single driver can't override the dummy state if the DT has
no pinctrl data for the spi driver.

-Matt

Re: [PATCH 2/2] ARM: OMAP2+: Enable pinctrl dummy states

[Tony Lindgren]

Added Linus Walleij to Cc as well.

* Matt Porter <mporter@ti.com> [120911 11:24]:
> On Tue, Sep 11, 2012 at 11:03:06AM -0700, Tony Lindgren wrote:
> > * Matt Porter <mporter@ti.com> [120911 10:46]:
> > > Enable pinctrl dummy states for all OMAP platforms. This allows
> > > drivers to be converted to pinctrl while still running on
> > > platforms that do not provide pinctrl data.
> > > 
> > > Signed-off-by: Matt Porter <mporter@ti.com>
> > > ---
> > >  arch/arm/mach-omap2/devices.c |    4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> > > index c00c689..577cd04 100644
> > > --- a/arch/arm/mach-omap2/devices.c
> > > +++ b/arch/arm/mach-omap2/devices.c
> > > @@ -17,6 +17,7 @@
> > >  #include <linux/err.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/of.h>
> > > +#include <linux/pinctrl/machine.h>
> > >  #include <linux/platform_data/omap4-keypad.h>
> > >  
> > >  #include <mach/hardware.h>
> > > @@ -631,6 +632,9 @@ static inline void omap_init_vout(void) {}
> > >  
> > >  static int __init omap2_init_devices(void)
> > >  {
> > > +	/* Enable dummy states for those platforms without pinctrl support */
> > > +	pinctrl_provide_dummies();
> > > +
> > >  	/*
> > >  	 * please keep these calls, and their implementations above,
> > >  	 * in alphabetical order so they're easier to sort through.
> > 
> > Hmm I think this may need to be board specific. And may need to
> > be board specific and depend on unpopulated device tree?
> 
> If I run this on AM33xx with dummy states enabled, I'm able to override
> that dummy state just fine with an appropriate pinctl/pinmux entry in my
> DT data for spi.

But do you get an error then if the desired pins are not found?
If you do get an error, then sounds like it's OK to do.
 
> Meanwhile on xM booting from board-omap3evm.c/!DT and debug enabled I
> get the following:
> 
> [    1.837829] omap2_mcspi omap2_mcspi.1: no of_node; not parsing
> pinctrl DT
> [    1.845214] omap2_mcspi omap2_mcspi.1: using pinctrl dummy state
> (default)
> [    1.854278] omap2_mcspi omap2_mcspi.2: no of_node; not parsing
> pinctrl DT
> [    1.861572] omap2_mcspi omap2_mcspi.2: using pinctrl dummy state
> (default)
> [    1.870025] omap2_mcspi omap2_mcspi.3: no of_node; not parsing
> pinctrl DT
> [    1.877288] omap2_mcspi omap2_mcspi.3: using pinctrl dummy state
> (default)
> [    1.885681] omap2_mcspi omap2_mcspi.4: no of_node; not parsing
> pinctrl DT
> [    1.892913] omap2_mcspi omap2_mcspi.4: using pinctrl dummy state
> (default)
> 
> which seems to cover things being informative enough about what is
> going on. Are you wanting to see an explicit warning that the pinctrl
> dummy state is in use when pinctrl data is not available?

Well I think we should consider at least the following:

1. Always see warnings when device tree is populated with board-generic.
   If somebody wants to use bootloader only muxing with DT, they can patch
   in pinctrl_provide_dummies() somewhere. But let's assume we always
   want to see the warnings with board-generic.c and DT.

2. For legacy booting without DT, we should not see any warnings
   from pinctrl-single.c as it's DT based.

3. There may be other non-pinctrl drivers too that are not DT
   based, and in those cases we should see the warnings as well
   for in the non-DT case.
 
> > For board-generic.c we always want to see the warnings. And some boards
> > insist on doing all the muxing only in the bootloader.
> 
> Which warnings are you saying we should see in the board-generic.c
> case?  Sure, there's plenty of cases where this will be unused due to
> somebody setting all the muxes in the bootloader and then not using
> pinctrl data. I'll have to doublecheck but I believe that case is also
> fine as the -single driver can't override the dummy state if the DT has
> no pinctrl data for the spi driver.

Yeah I guess it's the case #1 above for board-generic.c.

Regards,

Tony

Re: [PATCH 2/2] ARM: OMAP2+: Enable pinctrl dummy states

[Matt Porter]

On Tue, Sep 11, 2012 at 11:35:22AM -0700, Tony Lindgren wrote:
> Added Linus Walleij to Cc as well.
> 
> * Matt Porter <mporter@ti.com> [120911 11:24]:
> > On Tue, Sep 11, 2012 at 11:03:06AM -0700, Tony Lindgren wrote:
> > > * Matt Porter <mporter@ti.com> [120911 10:46]:
> > > > Enable pinctrl dummy states for all OMAP platforms. This allows
> > > > drivers to be converted to pinctrl while still running on
> > > > platforms that do not provide pinctrl data.
> > > > 
> > > > Signed-off-by: Matt Porter <mporter@ti.com>
> > > > ---
> > > >  arch/arm/mach-omap2/devices.c |    4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> > > > index c00c689..577cd04 100644
> > > > --- a/arch/arm/mach-omap2/devices.c
> > > > +++ b/arch/arm/mach-omap2/devices.c
> > > > @@ -17,6 +17,7 @@
> > > >  #include <linux/err.h>
> > > >  #include <linux/slab.h>
> > > >  #include <linux/of.h>
> > > > +#include <linux/pinctrl/machine.h>
> > > >  #include <linux/platform_data/omap4-keypad.h>
> > > >  
> > > >  #include <mach/hardware.h>
> > > > @@ -631,6 +632,9 @@ static inline void omap_init_vout(void) {}
> > > >  
> > > >  static int __init omap2_init_devices(void)
> > > >  {
> > > > +	/* Enable dummy states for those platforms without pinctrl support */
> > > > +	pinctrl_provide_dummies();
> > > > +
> > > >  	/*
> > > >  	 * please keep these calls, and their implementations above,
> > > >  	 * in alphabetical order so they're easier to sort through.
> > > 
> > > Hmm I think this may need to be board specific. And may need to
> > > be board specific and depend on unpopulated device tree?
> > 
> > If I run this on AM33xx with dummy states enabled, I'm able to override
> > that dummy state just fine with an appropriate pinctl/pinmux entry in my
> > DT data for spi.
> 
> But do you get an error then if the desired pins are not found?
> If you do get an error, then sounds like it's OK to do.

Hrm, no. In that case, it will be completely silent (assuming we took
care of the pinmuxing in the bootloader) as it uses the dummy state.
Only with debug on will you see the information that mcspi has used
the dummy state as is the case with !DT.

> > Meanwhile on xM booting from board-omap3evm.c/!DT and debug enabled I
> > get the following:
> > 
> > [    1.837829] omap2_mcspi omap2_mcspi.1: no of_node; not parsing
> > pinctrl DT
> > [    1.845214] omap2_mcspi omap2_mcspi.1: using pinctrl dummy state
> > (default)
> > [    1.854278] omap2_mcspi omap2_mcspi.2: no of_node; not parsing
> > pinctrl DT
> > [    1.861572] omap2_mcspi omap2_mcspi.2: using pinctrl dummy state
> > (default)
> > [    1.870025] omap2_mcspi omap2_mcspi.3: no of_node; not parsing
> > pinctrl DT
> > [    1.877288] omap2_mcspi omap2_mcspi.3: using pinctrl dummy state
> > (default)
> > [    1.885681] omap2_mcspi omap2_mcspi.4: no of_node; not parsing
> > pinctrl DT
> > [    1.892913] omap2_mcspi omap2_mcspi.4: using pinctrl dummy state
> > (default)
> > 
> > which seems to cover things being informative enough about what is
> > going on. Are you wanting to see an explicit warning that the pinctrl
> > dummy state is in use when pinctrl data is not available?
> 
> Well I think we should consider at least the following:
> 
> 1. Always see warnings when device tree is populated with board-generic.
>    If somebody wants to use bootloader only muxing with DT, they can patch
>    in pinctrl_provide_dummies() somewhere. But let's assume we always
>    want to see the warnings with board-generic.c and DT.

Ok, this is clear.

> 2. For legacy booting without DT, we should not see any warnings
>    from pinctrl-single.c as it's DT based.

Right, except anything legacy booting without DT will require that
dummy states be present otherwise it will fail probe.

> 
> 3. There may be other non-pinctrl drivers too that are not DT
>    based, and in those cases we should see the warnings as well
>    for in the non-DT case.

I'm not sure what you mean here. "non-pinctrl drivers" means any driver
that is not yet pinctrl or DT enabled? It's unclear to me how this
case has a bearing on mcspi and pinctrl enablement across legacy
board-foo.c !DT booting platforms.

However, I think if the approach was modified by only calling
pinctrl_provide_dummies() when we are booting with DT populated
and using board-generic.c then it will satisfy all of your
concerns. Thoughts?

i.e. the legacy !DT booting will have dummy states and continue
along through mcspi the way it does today, relying on board-foo level
pinmux calls (or bootloader pinmuxing). Meanwhile DT booting will now
require that a mcspi instance also require pinctrl entry in this dts.

The only worrisome thing is the pinctrl requirement on DT booting is
now an implicit requirement.

> > > For board-generic.c we always want to see the warnings. And some boards
> > > insist on doing all the muxing only in the bootloader.
> > 
> > Which warnings are you saying we should see in the board-generic.c
> > case?  Sure, there's plenty of cases where this will be unused due to
> > somebody setting all the muxes in the bootloader and then not using
> > pinctrl data. I'll have to doublecheck but I believe that case is also
> > fine as the -single driver can't override the dummy state if the DT has
> > no pinctrl data for the spi driver.
> 
> Yeah I guess it's the case #1 above for board-generic.c.

Right, I just checked that, in any case.

-Matt

Re: [PATCH 2/2] ARM: OMAP2+: Enable pinctrl dummy states

[Tony Lindgren]

* Matt Porter <mporter@ti.com> [120911 12:05]:
> On Tue, Sep 11, 2012 at 11:35:22AM -0700, Tony Lindgren wrote:
> > Added Linus Walleij to Cc as well.

Now I think I really managed to add Linus W to Cc, sent too fast
earlier.
...

> > But do you get an error then if the desired pins are not found?
> > If you do get an error, then sounds like it's OK to do.
>
> Hrm, no. In that case, it will be completely silent (assuming we took
> care of the pinmuxing in the bootloader) as it uses the dummy state.
> Only with debug on will you see the information that mcspi has used
> the dummy state as is the case with !DT.
...
 
> > Well I think we should consider at least the following:
> > 
> > 1. Always see warnings when device tree is populated with board-generic.
> >    If somebody wants to use bootloader only muxing with DT, they can patch
> >    in pinctrl_provide_dummies() somewhere. But let's assume we always
> >    want to see the warnings with board-generic.c and DT.
> 
> Ok, this is clear.
> 
> > 2. For legacy booting without DT, we should not see any warnings
> >    from pinctrl-single.c as it's DT based.
> 
> Right, except anything legacy booting without DT will require that
> dummy states be present otherwise it will fail probe.

But I guess we should enable the dummy states only for other
board-*.c files, not board-generic.c? 

> > 3. There may be other non-pinctrl drivers too that are not DT
> >    based, and in those cases we should see the warnings as well
> >    for in the non-DT case.
> 
> I'm not sure what you mean here. "non-pinctrl drivers" means any driver
> that is not yet pinctrl or DT enabled? It's unclear to me how this
> case has a bearing on mcspi and pinctrl enablement across legacy
> board-foo.c !DT booting platforms.

Right, sorry I meant "non DT pinctrl drivers"..
 
> However, I think if the approach was modified by only calling
> pinctrl_provide_dummies() when we are booting with DT populated
> and using board-generic.c then it will satisfy all of your
> concerns. Thoughts?

Hmm but shouldn't it be call pinctrl_provide_dummies() only
for other boards except board-generic.c? And that is assuming
we don't have any other "non DT pinctrl drivers" around.
 
> i.e. the legacy !DT booting will have dummy states and continue
> along through mcspi the way it does today, relying on board-foo level
> pinmux calls (or bootloader pinmuxing). Meanwhile DT booting will now
> require that a mcspi instance also require pinctrl entry in this dts.

Yes agreed, except let's just produce a warning for the pinctrl
errors..
 
> The only worrisome thing is the pinctrl requirement on DT booting is
> now an implicit requirement.

..as otherwise not much will work at this point :)
 
> > > > For board-generic.c we always want to see the warnings. And some boards
> > > > insist on doing all the muxing only in the bootloader.
> > > 
> > > Which warnings are you saying we should see in the board-generic.c
> > > case?  Sure, there's plenty of cases where this will be unused due to
> > > somebody setting all the muxes in the bootloader and then not using
> > > pinctrl data. I'll have to doublecheck but I believe that case is also
> > > fine as the -single driver can't override the dummy state if the DT has
> > > no pinctrl data for the spi driver.

I suggest all pinctrl errors should show up as warnings with
board-generic.c, but we should not exit out of the driver probe
on errors.

Regards,

Tony

Re: [PATCH 2/2] ARM: OMAP2+: Enable pinctrl dummy states

[Matt Porter]

On Tue, Sep 11, 2012 at 06:03:07PM -0700, Tony Lindgren wrote:
> * Matt Porter <mporter@ti.com> [120911 12:05]:
> > On Tue, Sep 11, 2012 at 11:35:22AM -0700, Tony Lindgren wrote:
> > > Added Linus Walleij to Cc as well.
> 
> Now I think I really managed to add Linus W to Cc, sent too fast
> earlier.
> ...
> 
> > > But do you get an error then if the desired pins are not found?
> > > If you do get an error, then sounds like it's OK to do.
> >
> > Hrm, no. In that case, it will be completely silent (assuming we took
> > care of the pinmuxing in the bootloader) as it uses the dummy state.
> > Only with debug on will you see the information that mcspi has used
> > the dummy state as is the case with !DT.
> ...
>  
> > > Well I think we should consider at least the following:
> > > 
> > > 1. Always see warnings when device tree is populated with board-generic.
> > >    If somebody wants to use bootloader only muxing with DT, they can patch
> > >    in pinctrl_provide_dummies() somewhere. But let's assume we always
> > >    want to see the warnings with board-generic.c and DT.
> > 
> > Ok, this is clear.
> > 
> > > 2. For legacy booting without DT, we should not see any warnings
> > >    from pinctrl-single.c as it's DT based.
> > 
> > Right, except anything legacy booting without DT will require that
> > dummy states be present otherwise it will fail probe.
> 
> But I guess we should enable the dummy states only for other
> board-*.c files, not board-generic.c? 
> 
> > > 3. There may be other non-pinctrl drivers too that are not DT
> > >    based, and in those cases we should see the warnings as well
> > >    for in the non-DT case.
> > 
> > I'm not sure what you mean here. "non-pinctrl drivers" means any driver
> > that is not yet pinctrl or DT enabled? It's unclear to me how this
> > case has a bearing on mcspi and pinctrl enablement across legacy
> > board-foo.c !DT booting platforms.
> 
> Right, sorry I meant "non DT pinctrl drivers"..
>  
> > However, I think if the approach was modified by only calling
> > pinctrl_provide_dummies() when we are booting with DT populated
> > and using board-generic.c then it will satisfy all of your
> > concerns. Thoughts?
> 
> Hmm but shouldn't it be call pinctrl_provide_dummies() only
> for other boards except board-generic.c? And that is assuming
> we don't have any other "non DT pinctrl drivers" around.

Yes, I've addressed this now in v2.

> > i.e. the legacy !DT booting will have dummy states and continue
> > along through mcspi the way it does today, relying on board-foo level
> > pinmux calls (or bootloader pinmuxing). Meanwhile DT booting will now
> > require that a mcspi instance also require pinctrl entry in this dts.
> 
> Yes agreed, except let's just produce a warning for the pinctrl
> errors..

Sounds good, I changed this in v2 to use the same warning as leds-gpio.

> > The only worrisome thing is the pinctrl requirement on DT booting is
> > now an implicit requirement.
> 
> ..as otherwise not much will work at this point :)

:)

> > > > > For board-generic.c we always want to see the warnings. And some boards
> > > > > insist on doing all the muxing only in the bootloader.
> > > > 
> > > > Which warnings are you saying we should see in the board-generic.c
> > > > case?  Sure, there's plenty of cases where this will be unused due to
> > > > somebody setting all the muxes in the bootloader and then not using
> > > > pinctrl data. I'll have to doublecheck but I believe that case is also
> > > > fine as the -single driver can't override the dummy state if the DT has
> > > > no pinctrl data for the spi driver.
> 
> I suggest all pinctrl errors should show up as warnings with
> board-generic.c, but we should not exit out of the driver probe
> on errors.

Ok, makes sense to me now.

Thanks,
Matt