diff mbox

mmc: omap_hsmmc: use for OMAP5 as well

Message ID 20130707201309.GA5429@localhost (mailing list archive)
State New, archived
Headers show

Commit Message

Ezequiel Garcia July 7, 2013, 8:13 p.m. UTC
Hi guys,

On Fri, Jun 28, 2013 at 09:27:20PM +0530, Balaji T K wrote:
> On Friday 28 June 2013 06:04 PM, a-bindra@ti.com wrote:
> > From: Amarinder Bindra <a-bindra@ti.com>
> >
> > OMAP's hs_mmc driver is also used for OMAP5 MMC controller operation.
> > Considering that the device tree entries are already there for this,
> > allow the driver to be built when only OMAP5 is enabled.
> > This allows MMC root filesystems to be available in "OMAP5 only"
> > configurations.
> >
> > Signed-off-by: Amarinder Bindra <a-bindra@ti.com>
> > Acked-by: Felipe Balbi <balbi@ti.com>
> > Acked-by: Nishanth Menon <nm@ti.com>
> 
> Looks good to me,
> Acked-by: Balaji T K <balajitk@ti.com>
> 

I came across this same issue while trying MMC patches on AM3xx,
which is nor OMAP3 neither OMAP4.

Now, looking at the driver I don't see any OMAP-specific bits
(welcome to the multiplatform world ;-) so I think we can just
remove the 'depends' line. Something like this:

---

Comments

Felipe Balbi July 8, 2013, 7:46 a.m. UTC | #1
On Sun, Jul 07, 2013 at 05:13:10PM -0300, Ezequiel Garcia wrote:
> Hi guys,
> 
> On Fri, Jun 28, 2013 at 09:27:20PM +0530, Balaji T K wrote:
> > On Friday 28 June 2013 06:04 PM, a-bindra@ti.com wrote:
> > > From: Amarinder Bindra <a-bindra@ti.com>
> > >
> > > OMAP's hs_mmc driver is also used for OMAP5 MMC controller operation.
> > > Considering that the device tree entries are already there for this,
> > > allow the driver to be built when only OMAP5 is enabled.
> > > This allows MMC root filesystems to be available in "OMAP5 only"
> > > configurations.
> > >
> > > Signed-off-by: Amarinder Bindra <a-bindra@ti.com>
> > > Acked-by: Felipe Balbi <balbi@ti.com>
> > > Acked-by: Nishanth Menon <nm@ti.com>
> > 
> > Looks good to me,
> > Acked-by: Balaji T K <balajitk@ti.com>
> > 
> 
> I came across this same issue while trying MMC patches on AM3xx,
> which is nor OMAP3 neither OMAP4.
> 
> Now, looking at the driver I don't see any OMAP-specific bits
> (welcome to the multiplatform world ;-) so I think we can just
> remove the 'depends' line. Something like this:

frankly speaking, this driver should be deleted and all users should be
moved to sdhci. OMAP's HSMMC controller is compatible with SDHCI except
for a couple quirks which can be easily worked around.
Nishanth Menon July 8, 2013, 7:58 p.m. UTC | #2
On 10:46-20130708, Felipe Balbi wrote:
> On Sun, Jul 07, 2013 at 05:13:10PM -0300, Ezequiel Garcia wrote:
> > Hi guys,
> > 
> > On Fri, Jun 28, 2013 at 09:27:20PM +0530, Balaji T K wrote:
> > > On Friday 28 June 2013 06:04 PM, a-bindra@ti.com wrote:
> > > > From: Amarinder Bindra <a-bindra@ti.com>
> > > >
> > > > OMAP's hs_mmc driver is also used for OMAP5 MMC controller operation.
> > > > Considering that the device tree entries are already there for this,
> > > > allow the driver to be built when only OMAP5 is enabled.
> > > > This allows MMC root filesystems to be available in "OMAP5 only"
> > > > configurations.
> > > >
> > > > Signed-off-by: Amarinder Bindra <a-bindra@ti.com>
> > > > Acked-by: Felipe Balbi <balbi@ti.com>
> > > > Acked-by: Nishanth Menon <nm@ti.com>
> > > 
> > > Looks good to me,
> > > Acked-by: Balaji T K <balajitk@ti.com>
> > > 
> > 
> > I came across this same issue while trying MMC patches on AM3xx,
> > which is nor OMAP3 neither OMAP4.
> > 
> > Now, looking at the driver I don't see any OMAP-specific bits
> > (welcome to the multiplatform world ;-) so I think we can just
> > remove the 'depends' line. Something like this:
> 
> frankly speaking, this driver should be deleted and all users should be
> moved to sdhci. OMAP's HSMMC controller is compatible with SDHCI except
> for a couple quirks which can be easily worked around.
Do we take that as a NAK to this specific patch?

I am not complaining about the future migration to a solution that is
much more generic and scalable, but having fixes for current problems
should'nt be ignored IMHO.

How about a depends on CONFIG_ARCH_OMAP2PLUS instead? will that solve
our pains with all OMAP2+ usin this?
Ezequiel Garcia July 8, 2013, 8:27 p.m. UTC | #3
On Mon, Jul 08, 2013 at 02:58:28PM -0500, Nishanth Menon wrote:
> On 10:46-20130708, Felipe Balbi wrote:
> > On Sun, Jul 07, 2013 at 05:13:10PM -0300, Ezequiel Garcia wrote:
> > > Hi guys,
> > > 
> > > On Fri, Jun 28, 2013 at 09:27:20PM +0530, Balaji T K wrote:
> > > > On Friday 28 June 2013 06:04 PM, a-bindra@ti.com wrote:
> > > > > From: Amarinder Bindra <a-bindra@ti.com>
> > > > >
> > > > > OMAP's hs_mmc driver is also used for OMAP5 MMC controller operation.
> > > > > Considering that the device tree entries are already there for this,
> > > > > allow the driver to be built when only OMAP5 is enabled.
> > > > > This allows MMC root filesystems to be available in "OMAP5 only"
> > > > > configurations.
> > > > >
> > > > > Signed-off-by: Amarinder Bindra <a-bindra@ti.com>
> > > > > Acked-by: Felipe Balbi <balbi@ti.com>
> > > > > Acked-by: Nishanth Menon <nm@ti.com>
> > > > 
> > > > Looks good to me,
> > > > Acked-by: Balaji T K <balajitk@ti.com>
> > > > 
> > > 
> > > I came across this same issue while trying MMC patches on AM3xx,
> > > which is nor OMAP3 neither OMAP4.
> > > 
> > > Now, looking at the driver I don't see any OMAP-specific bits
> > > (welcome to the multiplatform world ;-) so I think we can just
> > > remove the 'depends' line. Something like this:
> > 
> > frankly speaking, this driver should be deleted and all users should be
> > moved to sdhci. OMAP's HSMMC controller is compatible with SDHCI except
> > for a couple quirks which can be easily worked around.

@Felipe:
Do you mind giving me some hints about this quirks? I'm not too familiar
with these drivers.

> Do we take that as a NAK to this specific patch?
> 
> I am not complaining about the future migration to a solution that is
> much more generic and scalable, but having fixes for current problems
> should'nt be ignored IMHO.
> 

Of course.

> How about a depends on CONFIG_ARCH_OMAP2PLUS instead? will that solve
> our pains with all OMAP2+ usin this?

Yes, I think that using ARCH_OMAP2PLUS should do. FWIW, until we have
sdhci ready to replace omap_hsmmc, we have no choice but to apply such
a patch.

@Nishant, Balaji: Can you take care of resending this?

Thanks!
Felipe Balbi July 9, 2013, 6:52 a.m. UTC | #4
On Mon, Jul 08, 2013 at 02:58:28PM -0500, Nishanth Menon wrote:
> On 10:46-20130708, Felipe Balbi wrote:
> > On Sun, Jul 07, 2013 at 05:13:10PM -0300, Ezequiel Garcia wrote:
> > > Hi guys,
> > > 
> > > On Fri, Jun 28, 2013 at 09:27:20PM +0530, Balaji T K wrote:
> > > > On Friday 28 June 2013 06:04 PM, a-bindra@ti.com wrote:
> > > > > From: Amarinder Bindra <a-bindra@ti.com>
> > > > >
> > > > > OMAP's hs_mmc driver is also used for OMAP5 MMC controller operation.
> > > > > Considering that the device tree entries are already there for this,
> > > > > allow the driver to be built when only OMAP5 is enabled.
> > > > > This allows MMC root filesystems to be available in "OMAP5 only"
> > > > > configurations.
> > > > >
> > > > > Signed-off-by: Amarinder Bindra <a-bindra@ti.com>
> > > > > Acked-by: Felipe Balbi <balbi@ti.com>
> > > > > Acked-by: Nishanth Menon <nm@ti.com>
> > > > 
> > > > Looks good to me,
> > > > Acked-by: Balaji T K <balajitk@ti.com>
> > > > 
> > > 
> > > I came across this same issue while trying MMC patches on AM3xx,
> > > which is nor OMAP3 neither OMAP4.
> > > 
> > > Now, looking at the driver I don't see any OMAP-specific bits
> > > (welcome to the multiplatform world ;-) so I think we can just
> > > remove the 'depends' line. Something like this:
> > 
> > frankly speaking, this driver should be deleted and all users should be
> > moved to sdhci. OMAP's HSMMC controller is compatible with SDHCI except
> > for a couple quirks which can be easily worked around.
> Do we take that as a NAK to this specific patch?

heh, not really. We can't prevent bug fixes to this driver just because
long ago TI decided to write a TI-specific driver when it could just
re-use SDHCI :-)
Felipe Balbi July 9, 2013, 6:55 a.m. UTC | #5
On Mon, Jul 08, 2013 at 05:27:50PM -0300, Ezequiel Garcia wrote:
> On Mon, Jul 08, 2013 at 02:58:28PM -0500, Nishanth Menon wrote:
> > On 10:46-20130708, Felipe Balbi wrote:
> > > On Sun, Jul 07, 2013 at 05:13:10PM -0300, Ezequiel Garcia wrote:
> > > > Hi guys,
> > > > 
> > > > On Fri, Jun 28, 2013 at 09:27:20PM +0530, Balaji T K wrote:
> > > > > On Friday 28 June 2013 06:04 PM, a-bindra@ti.com wrote:
> > > > > > From: Amarinder Bindra <a-bindra@ti.com>
> > > > > >
> > > > > > OMAP's hs_mmc driver is also used for OMAP5 MMC controller operation.
> > > > > > Considering that the device tree entries are already there for this,
> > > > > > allow the driver to be built when only OMAP5 is enabled.
> > > > > > This allows MMC root filesystems to be available in "OMAP5 only"
> > > > > > configurations.
> > > > > >
> > > > > > Signed-off-by: Amarinder Bindra <a-bindra@ti.com>
> > > > > > Acked-by: Felipe Balbi <balbi@ti.com>
> > > > > > Acked-by: Nishanth Menon <nm@ti.com>
> > > > > 
> > > > > Looks good to me,
> > > > > Acked-by: Balaji T K <balajitk@ti.com>
> > > > > 
> > > > 
> > > > I came across this same issue while trying MMC patches on AM3xx,
> > > > which is nor OMAP3 neither OMAP4.
> > > > 
> > > > Now, looking at the driver I don't see any OMAP-specific bits
> > > > (welcome to the multiplatform world ;-) so I think we can just
> > > > remove the 'depends' line. Something like this:
> > > 
> > > frankly speaking, this driver should be deleted and all users should be
> > > moved to sdhci. OMAP's HSMMC controller is compatible with SDHCI except
> > > for a couple quirks which can be easily worked around.
> 
> @Felipe:
> Do you mind giving me some hints about this quirks? I'm not too familiar
> with these drivers.

one of the quirks is that TI decided to registers into one (because they
both had 16-bits reserved, so, well, we can make the HW 0.0000000000001
dollar cheaper :-p )

The other is related where our SDHCI registers start. Because of TI's
added registers (SYSConfig and REVISION, etc), SDHCI registers have an
offset which generic SDHCI driver, doesn't cope with right now.

> > Do we take that as a NAK to this specific patch?
> > 
> > I am not complaining about the future migration to a solution that is
> > much more generic and scalable, but having fixes for current problems
> > should'nt be ignored IMHO.
> > 
> 
> Of course.
> 
> > How about a depends on CONFIG_ARCH_OMAP2PLUS instead? will that solve
> > our pains with all OMAP2+ usin this?
> 
> Yes, I think that using ARCH_OMAP2PLUS should do. FWIW, until we have
> sdhci ready to replace omap_hsmmc, we have no choice but to apply such
> a patch.

if you do change to ARCH_OMAP2PLUS, please also add COMPILE_TEST
(ARCH_OMAP2PLUS || COMPILE_TEST) so we can build in other archs.
diff mbox

Patch

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 9ab8f8d..bade7f2 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -273,10 +273,9 @@  config MMC_OMAP
 
 config MMC_OMAP_HS
 	tristate "TI OMAP High Speed Multimedia Card Interface support"
-	depends on SOC_OMAP2430 || ARCH_OMAP3 || ARCH_OMAP4
 	help
 	  This selects the TI OMAP High Speed Multimedia card Interface.
-	  If you have an OMAP2430 or OMAP3 board or OMAP4 board with a
+	  If you have an OMAP2430, OMAP3, OMAP4 or AM3xx board with a
 	  Multimedia Card slot, say Y or M here.
 
 	  If unsure, say N.