diff mbox

[1/2] mmc: omap_hsmmc: Add support for slot-name property in DT

Message ID 1431724489-32359-2-git-send-email-pali.rohar@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pali Rohár May 15, 2015, 9:14 p.m. UTC
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/mmc/host/omap_hsmmc.c |    2 ++
 1 file changed, 2 insertions(+)

Comments

Arnd Bergmann May 15, 2015, 9:19 p.m. UTC | #1
On Friday 15 May 2015 23:14:48 Pali Rohár wrote:
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 9df2b68..f6073dd 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1901,6 +1901,8 @@ static struct omap_hsmmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
>         if (of_find_property(np, "ti,needs-special-hs-handling", NULL))
>                 pdata->features |= HSMMC_HAS_HSPE_SUPPORT;
>  
> +       of_property_read_string(np, "slot-name", &pdata->name);
> +
>         return pdata;
>  }
> 

This property needs to be documented in Documentation/devicetree/bindings/mmc/mmc.txt
and preferably the implementation moved into mmc_of_parse.

We should not have an interface like this specific to one driver.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár May 15, 2015, 9:22 p.m. UTC | #2
On Friday 15 May 2015 23:19:44 Arnd Bergmann wrote:
> On Friday 15 May 2015 23:14:48 Pali Rohár wrote:
> > diff --git a/drivers/mmc/host/omap_hsmmc.c
> > b/drivers/mmc/host/omap_hsmmc.c index 9df2b68..f6073dd 100644
> > --- a/drivers/mmc/host/omap_hsmmc.c
> > +++ b/drivers/mmc/host/omap_hsmmc.c
> > @@ -1901,6 +1901,8 @@ static struct omap_hsmmc_platform_data
> > *of_get_hsmmc_pdata(struct device *dev)
> > 
> >         if (of_find_property(np, "ti,needs-special-hs-handling",
> >         NULL))
> >         
> >                 pdata->features |= HSMMC_HAS_HSPE_SUPPORT;
> > 
> > +       of_property_read_string(np, "slot-name", &pdata->name);
> > +
> > 
> >         return pdata;
> >  
> >  }
> 
> This property needs to be documented in
> Documentation/devicetree/bindings/mmc/mmc.txt and preferably the
> implementation moved into mmc_of_parse.
> 
> We should not have an interface like this specific to one driver.
> 
> 	Arnd

Interface is already present for platform data code (which do not use 
DT). So "pdata->name" is not DT specific...
Arnd Bergmann May 15, 2015, 9:25 p.m. UTC | #3
On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> On Friday 15 May 2015 23:19:44 Arnd Bergmann wrote:
> > On Friday 15 May 2015 23:14:48 Pali Rohár wrote:
> > > diff --git a/drivers/mmc/host/omap_hsmmc.c
> > > b/drivers/mmc/host/omap_hsmmc.c index 9df2b68..f6073dd 100644
> > > --- a/drivers/mmc/host/omap_hsmmc.c
> > > +++ b/drivers/mmc/host/omap_hsmmc.c
> > > @@ -1901,6 +1901,8 @@ static struct omap_hsmmc_platform_data
> > > *of_get_hsmmc_pdata(struct device *dev)
> > > 
> > >         if (of_find_property(np, "ti,needs-special-hs-handling",
> > >         NULL))
> > >         
> > >                 pdata->features |= HSMMC_HAS_HSPE_SUPPORT;
> > > 
> > > +       of_property_read_string(np, "slot-name", &pdata->name);
> > > +
> > > 
> > >         return pdata;
> > >  
> > >  }
> > 
> > This property needs to be documented in
> > Documentation/devicetree/bindings/mmc/mmc.txt and preferably the
> > implementation moved into mmc_of_parse.
> > 
> > We should not have an interface like this specific to one driver.
> > 
> >       Arnd
> 
> Interface is already present for platform data code (which do not use 
> DT). So "pdata->name" is not DT specific...
> 

The driver can of course keep providing the name from platform_data where
that is used. All I was saying is that when you introduce a DT property,
it should not be specific to that driver.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren May 18, 2015, 3:06 p.m. UTC | #4
* Arnd Bergmann <arnd@arndb.de> [150515 14:26]:
> On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> > On Friday 15 May 2015 23:19:44 Arnd Bergmann wrote:
> > > On Friday 15 May 2015 23:14:48 Pali Rohár wrote:
> > > > diff --git a/drivers/mmc/host/omap_hsmmc.c
> > > > b/drivers/mmc/host/omap_hsmmc.c index 9df2b68..f6073dd 100644
> > > > --- a/drivers/mmc/host/omap_hsmmc.c
> > > > +++ b/drivers/mmc/host/omap_hsmmc.c
> > > > @@ -1901,6 +1901,8 @@ static struct omap_hsmmc_platform_data
> > > > *of_get_hsmmc_pdata(struct device *dev)
> > > > 
> > > >         if (of_find_property(np, "ti,needs-special-hs-handling",
> > > >         NULL))
> > > >         
> > > >                 pdata->features |= HSMMC_HAS_HSPE_SUPPORT;
> > > > 
> > > > +       of_property_read_string(np, "slot-name", &pdata->name);
> > > > +
> > > > 
> > > >         return pdata;
> > > >  
> > > >  }
> > > 
> > > This property needs to be documented in
> > > Documentation/devicetree/bindings/mmc/mmc.txt and preferably the
> > > implementation moved into mmc_of_parse.
> > > 
> > > We should not have an interface like this specific to one driver.
> > > 
> > >       Arnd
> > 
> > Interface is already present for platform data code (which do not use 
> > DT). So "pdata->name" is not DT specific...
> > 
> 
> The driver can of course keep providing the name from platform_data where
> that is used. All I was saying is that when you introduce a DT property,
> it should not be specific to that driver.

Yes sounds like it would be a usable generic binding. Not sure
that it should be called slot-name though, maybe you can use
the MMC driver instance name instead? I don't think much any drivers
support multiple MMC slots although it's in the original MMC spec.

If setting up the generic binding is expected to take a while,
you can naturally pass it in pdata while waiting for the generic
binding to get merged.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann May 18, 2015, 3:07 p.m. UTC | #5
On Monday 18 May 2015 08:06:07 Tony Lindgren wrote:
> * Arnd Bergmann <arnd@arndb.de> [150515 14:26]:
> > On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> If setting up the generic binding is expected to take a while,
> you can naturally pass it in pdata while waiting for the generic
> binding to get merged.

Yes, good idea. So the n900 machine can use auxdata to pass this for the
time being, while the binding and generic implementation is being worked
out.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár Dec. 25, 2015, 12:53 p.m. UTC | #6
On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote:
> On Monday 18 May 2015 08:06:07 Tony Lindgren wrote:
> > * Arnd Bergmann <arnd@arndb.de> [150515 14:26]:
> > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> > If setting up the generic binding is expected to take a while,
> > you can naturally pass it in pdata while waiting for the generic
> > binding to get merged.
> 
> Yes, good idea. So the n900 machine can use auxdata to pass this for
> the time being, while the binding and generic implementation is
> being worked out.
> 
> 	Arnd

Ok, so what is needed to finish this patch series?
Arnd Bergmann Dec. 28, 2015, 2:14 p.m. UTC | #7
On Friday 25 December 2015 13:53:11 Pali Rohár wrote:
> On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote:
> > On Monday 18 May 2015 08:06:07 Tony Lindgren wrote:
> > > * Arnd Bergmann <arnd@arndb.de> [150515 14:26]:
> > > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> > > If setting up the generic binding is expected to take a while,
> > > you can naturally pass it in pdata while waiting for the generic
> > > binding to get merged.
> > 
> > Yes, good idea. So the n900 machine can use auxdata to pass this for
> > the time being, while the binding and generic implementation is
> > being worked out.
> > 
> 
> Ok, so what is needed to finish this patch series?

I don't know where we are at this point. Has either the auxdata approach
or the generic binding been worked on at all?

If not, please try to get the auxdata variant to work, it should not be hard
at all because there is no dependency on a generic binding.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár Dec. 28, 2015, 2:28 p.m. UTC | #8
On Monday 28 December 2015 15:14:50 Arnd Bergmann wrote:
> On Friday 25 December 2015 13:53:11 Pali Rohár wrote:
> > On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote:
> > > On Monday 18 May 2015 08:06:07 Tony Lindgren wrote:
> > > > * Arnd Bergmann <arnd@arndb.de> [150515 14:26]:
> > > > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> > > > If setting up the generic binding is expected to take a while,
> > > > you can naturally pass it in pdata while waiting for the
> > > > generic binding to get merged.
> > > 
> > > Yes, good idea. So the n900 machine can use auxdata to pass this
> > > for the time being, while the binding and generic implementation
> > > is being worked out.
> > 
> > Ok, so what is needed to finish this patch series?
> 
> I don't know where we are at this point. Has either the auxdata
> approach or the generic binding been worked on at all?

What are auxdata data? And what do you mean with "generic binding" 
approach?

> If not, please try to get the auxdata variant to work, it should not
> be hard at all because there is no dependency on a generic binding.
> 
> 	Arnd
Arnd Bergmann Dec. 28, 2015, 2:41 p.m. UTC | #9
On Monday 28 December 2015 15:28:48 Pali Rohár wrote:
> On Monday 28 December 2015 15:14:50 Arnd Bergmann wrote:
> > On Friday 25 December 2015 13:53:11 Pali Rohár wrote:
> > > On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote:
> > > > On Monday 18 May 2015 08:06:07 Tony Lindgren wrote:
> > > > > * Arnd Bergmann <arnd@arndb.de> [150515 14:26]:
> > > > > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> > > > > If setting up the generic binding is expected to take a while,
> > > > > you can naturally pass it in pdata while waiting for the
> > > > > generic binding to get merged.
> > > > 
> > > > Yes, good idea. So the n900 machine can use auxdata to pass this
> > > > for the time being, while the binding and generic implementation
> > > > is being worked out.
> > > 
> > > Ok, so what is needed to finish this patch series?
> > 
> > I don't know where we are at this point. Has either the auxdata
> > approach or the generic binding been worked on at all?
> 
> What are auxdata data? 

I mean you can add the platform data to the omap_auxdata_lookup[] table
for this board.

> And what do you mean with "generic binding" approach?

Start a discussion to specify slot names and implement that in the
mmc driver core, so we can remove the hack from the OMAP driver and
make it work the same way for any machine.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár Dec. 28, 2015, 2:54 p.m. UTC | #10
On Monday 28 December 2015 15:41:01 Arnd Bergmann wrote:
> On Monday 28 December 2015 15:28:48 Pali Rohár wrote:
> > On Monday 28 December 2015 15:14:50 Arnd Bergmann wrote:
> > > On Friday 25 December 2015 13:53:11 Pali Rohár wrote:
> > > > On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote:
> > > > > On Monday 18 May 2015 08:06:07 Tony Lindgren wrote:
> > > > > > * Arnd Bergmann <arnd@arndb.de> [150515 14:26]:
> > > > > > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> > > > > > If setting up the generic binding is expected to take a
> > > > > > while, you can naturally pass it in pdata while waiting
> > > > > > for the generic binding to get merged.
> > > > > 
> > > > > Yes, good idea. So the n900 machine can use auxdata to pass
> > > > > this for the time being, while the binding and generic
> > > > > implementation is being worked out.
> > > > 
> > > > Ok, so what is needed to finish this patch series?
> > > 
> > > I don't know where we are at this point. Has either the auxdata
> > > approach or the generic binding been worked on at all?
> > 
> > What are auxdata data?
> 
> I mean you can add the platform data to the omap_auxdata_lookup[]
> table for this board.

But can I mix data from omap3-n900.dts and omap_auxdata_lookup[]?
Arnd Bergmann Dec. 28, 2015, 2:55 p.m. UTC | #11
On Monday 28 December 2015 15:54:35 Pali Rohár wrote:
> On Monday 28 December 2015 15:41:01 Arnd Bergmann wrote:
> > On Monday 28 December 2015 15:28:48 Pali Rohár wrote:
> > > On Monday 28 December 2015 15:14:50 Arnd Bergmann wrote:
> > > > On Friday 25 December 2015 13:53:11 Pali Rohár wrote:
> > > > > On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote:
> > > > > > On Monday 18 May 2015 08:06:07 Tony Lindgren wrote:
> > > > > > > * Arnd Bergmann <arnd@arndb.de> [150515 14:26]:
> > > > > > > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> > > > > > > If setting up the generic binding is expected to take a
> > > > > > > while, you can naturally pass it in pdata while waiting
> > > > > > > for the generic binding to get merged.
> > > > > > 
> > > > > > Yes, good idea. So the n900 machine can use auxdata to pass
> > > > > > this for the time being, while the binding and generic
> > > > > > implementation is being worked out.
> > > > > 
> > > > > Ok, so what is needed to finish this patch series?
> > > > 
> > > > I don't know where we are at this point. Has either the auxdata
> > > > approach or the generic binding been worked on at all?
> > > 
> > > What are auxdata data?
> > 
> > I mean you can add the platform data to the omap_auxdata_lookup[]
> > table for this board.
> 
> But can I mix data from omap3-n900.dts and omap_auxdata_lookup[]?

Yes.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár Jan. 2, 2016, 3:22 p.m. UTC | #12
On Monday 28 December 2015 15:55:28 Arnd Bergmann wrote:
> On Monday 28 December 2015 15:54:35 Pali Rohár wrote:
> > On Monday 28 December 2015 15:41:01 Arnd Bergmann wrote:
> > > On Monday 28 December 2015 15:28:48 Pali Rohár wrote:
> > > > On Monday 28 December 2015 15:14:50 Arnd Bergmann wrote:
> > > > > On Friday 25 December 2015 13:53:11 Pali Rohár wrote:
> > > > > > On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote:
> > > > > > > On Monday 18 May 2015 08:06:07 Tony Lindgren wrote:
> > > > > > > > * Arnd Bergmann <arnd@arndb.de> [150515 14:26]:
> > > > > > > > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote:
> > > > > > > > If setting up the generic binding is expected to take a
> > > > > > > > while, you can naturally pass it in pdata while waiting
> > > > > > > > for the generic binding to get merged.
> > > > > > > 
> > > > > > > Yes, good idea. So the n900 machine can use auxdata to
> > > > > > > pass this for the time being, while the binding and
> > > > > > > generic implementation is being worked out.
> > > > > > 
> > > > > > Ok, so what is needed to finish this patch series?
> > > > > 
> > > > > I don't know where we are at this point. Has either the
> > > > > auxdata approach or the generic binding been worked on at
> > > > > all?
> > > > 
> > > > What are auxdata data?
> > > 
> > > I mean you can add the platform data to the omap_auxdata_lookup[]
> > > table for this board.
> > 
> > But can I mix data from omap3-n900.dts and omap_auxdata_lookup[]?
> 
> Yes.
> 
> 	Arnd

Hm... looks like it is not possible. omap_hsmmc driver ignores any 
supplied platform data if there are device tree data. So array 
omap_auxdata_lookup[] does not help.
diff mbox

Patch

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 9df2b68..f6073dd 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1901,6 +1901,8 @@  static struct omap_hsmmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 	if (of_find_property(np, "ti,needs-special-hs-handling", NULL))
 		pdata->features |= HSMMC_HAS_HSPE_SUPPORT;
 
+	of_property_read_string(np, "slot-name", &pdata->name);
+
 	return pdata;
 }
 #else