diff mbox

[v10,7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

Message ID 1462417950-46796-8-git-send-email-yangbo.lu@nxp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yangbo Lu May 5, 2016, 3:12 a.m. UTC
The eSDHC of T4240-R1.0-R2.0 has incorrect vender version and spec version.
Acturally the right version numbers should be VVN=0x13 and SVN = 0x1.
This patch adds the GUTS driver support for eSDHC driver to get SVR(System
version register). And fix host version to avoid that incorrect version
numbers break down the ADMA data transfer.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Scott Wood <oss@buserror.net>
---
Changes for v2:
	- Got SVR through iomap instead of dts
Changes for v3:
	- Managed GUTS through syscon instead of iomap in eSDHC driver
Changes for v4:
	- Got SVR by GUTS driver instead of SYSCON
Changes for v5:
	- Changed to get SVR through API fsl_guts_get_svr()
	- Combined patch 4, patch 5 and patch 6 into one
Changes for v6:
	- Added 'Acked-by: Ulf Hansson'
Changes for v7:
	- None
Changes for v8:
	- Added 'Acked-by: Scott Wood'
Changes for v9:
	- None
Changes for v10:
	- None
---
 drivers/mmc/host/Kconfig          |  1 +
 drivers/mmc/host/sdhci-of-esdhc.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

Comments

Arnd Bergmann May 5, 2016, 8:31 a.m. UTC | #1
On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> 
> +       fsl_guts_init();
> +       svr = fsl_guts_get_svr();
> +       if (svr) {
> +               esdhc->soc_ver = SVR_SOC_VER(svr);
> +               esdhc->soc_rev = SVR_REV(svr);
> +       } else {
> +               dev_err(&pdev->dev, "Failed to get SVR value!\n");
> +       }
> +
> 


Sorry for jumping in again after not participating in the discussion for the
past few versions.

What happened to my suggestion of making this a platform-independent interface
to avoid the link time dependency?

Specifically, why not add an exported function to drivers/base/soc.c that
uses glob_match() for comparing a string in the device driver to the ID
of the SoC that is set by whatever SoC identifying driver the platform
has?

	Arnd
Yangbo Lu May 5, 2016, 9:41 a.m. UTC | #2
Hi Arnd,


> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Thursday, May 05, 2016 4:32 PM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Yangbo Lu; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> linux-clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> foundation.org; netdev@vger.kernel.org; Mark Rutland;
> ulf.hansson@linaro.org; Russell King; Bhupesh Sharma; Joerg Roedel;
> Santosh Shilimkar; Yang-Leo Li; Scott Wood; Rob Herring; Claudiu Manoil;
> Kumar Gala; Xiaobo Xie; Qiang Zhao
> Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
> 
> On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> >
> > +       fsl_guts_init();
> > +       svr = fsl_guts_get_svr();
> > +       if (svr) {
> > +               esdhc->soc_ver = SVR_SOC_VER(svr);
> > +               esdhc->soc_rev = SVR_REV(svr);
> > +       } else {
> > +               dev_err(&pdev->dev, "Failed to get SVR value!\n");
> > +       }
> > +
> >
> 
> 
> Sorry for jumping in again after not participating in the discussion for
> the past few versions.
> 
> What happened to my suggestion of making this a platform-independent
> interface to avoid the link time dependency?
> 
> Specifically, why not add an exported function to drivers/base/soc.c that
> uses glob_match() for comparing a string in the device driver to the ID
> of the SoC that is set by whatever SoC identifying driver the platform
> has?
> 
> 	Arnd

[Lu Yangbo-B47093] I think this has been discussed in v6.
You can find Scott's comments about this in below link.
https://patchwork.kernel.org/patch/8544501/

Thanks.
Arnd Bergmann May 5, 2016, 11:10 a.m. UTC | #3
On Thursday 05 May 2016 09:41:32 Yangbo Lu wrote:
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > Sent: Thursday, May 05, 2016 4:32 PM
> > To: linuxppc-dev@lists.ozlabs.org
> > Cc: Yangbo Lu; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > linux-clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> > foundation.org; netdev@vger.kernel.org; Mark Rutland;
> > ulf.hansson@linaro.org; Russell King; Bhupesh Sharma; Joerg Roedel;
> > Santosh Shilimkar; Yang-Leo Li; Scott Wood; Rob Herring; Claudiu Manoil;
> > Kumar Gala; Xiaobo Xie; Qiang Zhao
> > Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> > R1.0-R2.0
> > 
> > On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> > >
> > > +       fsl_guts_init();
> > > +       svr = fsl_guts_get_svr();
> > > +       if (svr) {
> > > +               esdhc->soc_ver = SVR_SOC_VER(svr);
> > > +               esdhc->soc_rev = SVR_REV(svr);
> > > +       } else {
> > > +               dev_err(&pdev->dev, "Failed to get SVR value!\n");
> > > +       }
> > > +
> > >
> > 
> > 
> > Sorry for jumping in again after not participating in the discussion for
> > the past few versions.
> > 
> > What happened to my suggestion of making this a platform-independent
> > interface to avoid the link time dependency?
> > 
> > Specifically, why not add an exported function to drivers/base/soc.c that
> > uses glob_match() for comparing a string in the device driver to the ID
> > of the SoC that is set by whatever SoC identifying driver the platform
> > has?
> 
> [Lu Yangbo-B47093] I think this has been discussed in v6.
> You can find Scott's comments about this in below link.
> https://patchwork.kernel.org/patch/8544501/

Ah, thanks for bearing with me and digging this out again. Let me follow
up on Scott's older replies here then:

> >> IIRC, it is the same IP block as i.MX and Arnd's point is this won't
> >> even compile on !PPC. It is things like this that prevent sharing the
> >> driver.
> 
> The whole point of using the MMIO SVR instead of the PPC SPR is so that
> it will work on ARM...  The guts driver should build on any platform as
> long as OF is enabled, and if it doesn't find a node to bind to it will
> return 0 for SVR, and the eSDHC driver will continue (after printing an
> error that should be removed) without the ability to test for errata
> based on SVR.

It feels like a bad design to have to come up with a different
method for each SoC type here when they all do the same thing
and want to identify some variant of the chip to do device
specific quirks.

As far as I'm concerned, every driver in drivers/soc that needs to
export a symbol to be used by a device driver is an indication that
we don't have the right set of abstractions yet. There are cases
that are not worth abstracting because the functionality is rather
obscure and only a couple of drivers for one particular chip
ever need it.

Finding out the version of the SoC does not look like this case.

> > I think the first four patches take care of building for ARM,
> > but the problem remains if you want to enable COMPILE_TEST as
> > we need for certain automated checking.
> 
> What specific problem is there with COMPILE_TEST?

COMPILE_TEST is solvable here and the way it is implemented in this
case (selecting FSL_GUTS from the driver) indeed looks like it works
correctly, but it's still awkward that this means building the
SoC specific ID stuff into the vmlinux binary for any driver that
uses something like that for a particular SoC.

> >> Dealing with Si revs is a common problem. We should have a
> >> common solution. There is soc_device for this purpose.
> > 
> > Exactly. The last time this came up, I think we agreed to implement a
> > helper using glob_match() on the soc_device strings. Unfortunately
> > this hasn't happened then, but I'd still prefer that over yet another
> > vendor-specific way of dealing with the generic issue.
> 
> soc_device would require encoding the SVR as a string and then decoding
> the string, which is more complicated and error prone than having
> platform-specific code test a platform-specific number. 

You already need to encode it as a string to register the soc_device,
and the driver just needs to pass a glob string, so the only part that
is missing is the generic function that takes the string from the
driver and passes that to glob_match for the soc_device.

> And when would it get registered on arm64, which doesn't have
> platform code?

Whenever the soc driver is loaded, as is the case now. The match
function can return -EPROBE_DEFER if no SoC device is registered
yet.

	Arnd
Crystal Wood May 11, 2016, 3:26 a.m. UTC | #4
On Thu, 2016-05-05 at 13:10 +0200, Arnd Bergmann wrote:
> On Thursday 05 May 2016 09:41:32 Yangbo Lu wrote:
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > Sent: Thursday, May 05, 2016 4:32 PM
> > > To: linuxppc-dev@lists.ozlabs.org
> > > Cc: Yangbo Lu; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > > linux-clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> > > foundation.org; netdev@vger.kernel.org; Mark Rutland;
> > > ulf.hansson@linaro.org; Russell King; Bhupesh Sharma; Joerg Roedel;
> > > Santosh Shilimkar; Yang-Leo Li; Scott Wood; Rob Herring; Claudiu Manoil;
> > > Kumar Gala; Xiaobo Xie; Qiang Zhao
> > > Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> > > R1.0-R2.0
> > > 
> > > On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> > > > IIRC, it is the same IP block as i.MX and Arnd's point is this won't
> > > > even compile on !PPC. It is things like this that prevent sharing the
> > > > driver.
> > 
> > The whole point of using the MMIO SVR instead of the PPC SPR is so that
> > it will work on ARM...  The guts driver should build on any platform as
> > long as OF is enabled, and if it doesn't find a node to bind to it will
> > return 0 for SVR, and the eSDHC driver will continue (after printing an
> > error that should be removed) without the ability to test for errata
> > based on SVR.
> 
> It feels like a bad design to have to come up with a different
> method for each SoC type here when they all do the same thing
> and want to identify some variant of the chip to do device
> specific quirks.
> 
> As far as I'm concerned, every driver in drivers/soc that needs to
> export a symbol to be used by a device driver is an indication that
> we don't have the right set of abstractions yet. There are cases
> that are not worth abstracting because the functionality is rather
> obscure and only a couple of drivers for one particular chip
> ever need it.
> 
> Finding out the version of the SoC does not look like this case.

I'm open to new ways of abstracting this, but can that please be discussed
after these patches are merged?  This patchset is fixing a problem, the
existing abstraction is unappealing and not widely adopted, a new abstraction
is not ready, and we're only touching code for our hardware.

Oh, and the existing abstraction isn't even "existing".  I don't see any
examples where soc_device is being used like this -- or even any way for a
driver (the one consuming the information, not the soc "driver") to get a
reference to the soc_device that's been registered short of searching for the
device object by name -- and you're asking for new functionality in
drivers/base/soc.c.

> > > I think the first four patches take care of building for ARM,
> > > but the problem remains if you want to enable COMPILE_TEST as
> > > we need for certain automated checking.
> > 
> > What specific problem is there with COMPILE_TEST?
> 
> COMPILE_TEST is solvable here and the way it is implemented in this
> case (selecting FSL_GUTS from the driver) indeed looks like it works
> correctly, but it's still awkward that this means building the
> SoC specific ID stuff into the vmlinux binary for any driver that
> uses something like that for a particular SoC.

Please keep in mind that this is a Freescale-specific driver... it's not as if
we're attaching this dependency to common SDHCI code.

> 
> > > > Dealing with Si revs is a common problem. We should have a
> > > > common solution. There is soc_device for this purpose.
> > > 
> > > Exactly. The last time this came up, I think we agreed to implement a
> > > helper using glob_match() on the soc_device strings. Unfortunately
> > > this hasn't happened then, but I'd still prefer that over yet another
> > > vendor-specific way of dealing with the generic issue.
> > 
> > soc_device would require encoding the SVR as a string and then decoding
> > the string, which is more complicated and error prone than having
> > platform-specific code test a platform-specific number. 
> 
> You already need to encode it as a string to register the soc_device,

No we don't, because we don't already register a soc_device on arm64 or ppc
(and it looks like whatever does get registered on at least some relevant
arm32 chips is not particularly useful).

> and the driver just needs to pass a glob string, so the only part that
> is missing is the generic function that takes the string from the
> driver and passes that to glob_match for the soc_device.

"just"

And what would the glob look like?

I'd rather not write kernel code as if it were a shell/Perl script.

> > And when would it get registered on arm64, which doesn't have
> > platform code?
> 
> Whenever the soc driver is loaded, as is the case now. The match
> function can return -EPROBE_DEFER if no SoC device is registered
> yet.

That's too late for some places where we need access to SVR, e.g. clock
drivers (which use CLK_OF_DECLARE and are initialized very early, not as part
of the driver model and thus can't defer).  Currently we have an #ifdef
CONFIG_PPC for this in drivers/clk/clk-qoriq.c... Maybe we should have done
that here as well, and saved some grief. :-)  At least until an erratum pops
up on an ARM-based chip.

And what happens if we're running on arm32, and thus the arch code already
registered an soc_device with a different (and less useful) encoding?

-Scott
Yangbo Lu May 20, 2016, 6:05 a.m. UTC | #5
Hi Arnd,

Any comments? 
Please reply when you see the email since we want this eSDHC issue to be fixed soon.

All the patches are Freescale-specific and is to fix the eSDHC driver.
Could we let them merged first if you were talking about a new way of abstracting getting SoC version.


Thanks :)


Best regards,
Yangbo Lu

> -----Original Message-----
> From: Scott Wood [mailto:oss@buserror.net]
> Sent: Wednesday, May 11, 2016 11:26 AM
> To: Arnd Bergmann; linux-arm-kernel@lists.infradead.org
> Cc: Yangbo Lu; linuxppc-dev@lists.ozlabs.org; Mark Rutland;
> devicetree@vger.kernel.org; ulf.hansson@linaro.org; Russell King; Bhupesh
> Sharma; netdev@vger.kernel.org; Joerg Roedel; Kumar Gala; linux-
> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Yang-Leo Li;
> iommu@lists.linux-foundation.org; Rob Herring; linux-i2c@vger.kernel.org;
> Claudiu Manoil; Santosh Shilimkar; Xiaobo Xie; linux-clk@vger.kernel.org;
> Qiang Zhao
> Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
> 
> On Thu, 2016-05-05 at 13:10 +0200, Arnd Bergmann wrote:
> > On Thursday 05 May 2016 09:41:32 Yangbo Lu wrote:
> > > > -----Original Message-----
> > > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > > Sent: Thursday, May 05, 2016 4:32 PM
> > > > To: linuxppc-dev@lists.ozlabs.org
> > > > Cc: Yangbo Lu; linux-mmc@vger.kernel.org;
> > > > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > linux-kernel@vger.kernel.org; linux-clk@vger.kernel.org;
> > > > linux-i2c@vger.kernel.org; iommu@lists.linux- foundation.org;
> > > > netdev@vger.kernel.org; Mark Rutland; ulf.hansson@linaro.org;
> > > > Russell King; Bhupesh Sharma; Joerg Roedel; Santosh Shilimkar;
> > > > Yang-Leo Li; Scott Wood; Rob Herring; Claudiu Manoil; Kumar Gala;
> > > > Xiaobo Xie; Qiang Zhao
> > > > Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for
> > > > T4240-
> > > > R1.0-R2.0
> > > >
> > > > On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> > > > > IIRC, it is the same IP block as i.MX and Arnd's point is this
> > > > > won't even compile on !PPC. It is things like this that prevent
> > > > > sharing the driver.
> > >
> > > The whole point of using the MMIO SVR instead of the PPC SPR is so
> > > that it will work on ARM...  The guts driver should build on any
> > > platform as long as OF is enabled, and if it doesn't find a node to
> > > bind to it will return 0 for SVR, and the eSDHC driver will continue
> > > (after printing an error that should be removed) without the ability
> > > to test for errata based on SVR.
> >
> > It feels like a bad design to have to come up with a different method
> > for each SoC type here when they all do the same thing and want to
> > identify some variant of the chip to do device specific quirks.
> >
> > As far as I'm concerned, every driver in drivers/soc that needs to
> > export a symbol to be used by a device driver is an indication that we
> > don't have the right set of abstractions yet. There are cases that are
> > not worth abstracting because the functionality is rather obscure and
> > only a couple of drivers for one particular chip ever need it.
> >
> > Finding out the version of the SoC does not look like this case.
> 
> I'm open to new ways of abstracting this, but can that please be
> discussed after these patches are merged?  This patchset is fixing a
> problem, the existing abstraction is unappealing and not widely adopted,
> a new abstraction is not ready, and we're only touching code for our
> hardware.
> 
> Oh, and the existing abstraction isn't even "existing".  I don't see any
> examples where soc_device is being used like this -- or even any way for
> a driver (the one consuming the information, not the soc "driver") to get
> a reference to the soc_device that's been registered short of searching
> for the device object by name -- and you're asking for new functionality
> in drivers/base/soc.c.
> 
> > > > I think the first four patches take care of building for ARM, but
> > > > the problem remains if you want to enable COMPILE_TEST as we need
> > > > for certain automated checking.
> > >
> > > What specific problem is there with COMPILE_TEST?
> >
> > COMPILE_TEST is solvable here and the way it is implemented in this
> > case (selecting FSL_GUTS from the driver) indeed looks like it works
> > correctly, but it's still awkward that this means building the SoC
> > specific ID stuff into the vmlinux binary for any driver that uses
> > something like that for a particular SoC.
> 
> Please keep in mind that this is a Freescale-specific driver... it's not
> as if we're attaching this dependency to common SDHCI code.
> 
> >
> > > > > Dealing with Si revs is a common problem. We should have a
> > > > > common solution. There is soc_device for this purpose.
> > > >
> > > > Exactly. The last time this came up, I think we agreed to
> > > > implement a helper using glob_match() on the soc_device strings.
> > > > Unfortunately this hasn't happened then, but I'd still prefer that
> > > > over yet another vendor-specific way of dealing with the generic
> issue.
> > >
> > > soc_device would require encoding the SVR as a string and then
> > > decoding the string, which is more complicated and error prone than
> > > having platform-specific code test a platform-specific number.
> >
> > You already need to encode it as a string to register the soc_device,
> 
> No we don't, because we don't already register a soc_device on arm64 or
> ppc (and it looks like whatever does get registered on at least some
> relevant
> arm32 chips is not particularly useful).
> 
> > and the driver just needs to pass a glob string, so the only part that
> > is missing is the generic function that takes the string from the
> > driver and passes that to glob_match for the soc_device.
> 
> "just"
> 
> And what would the glob look like?
> 
> I'd rather not write kernel code as if it were a shell/Perl script.
> 
> > > And when would it get registered on arm64, which doesn't have
> > > platform code?
> >
> > Whenever the soc driver is loaded, as is the case now. The match
> > function can return -EPROBE_DEFER if no SoC device is registered yet.
> 
> That's too late for some places where we need access to SVR, e.g. clock
> drivers (which use CLK_OF_DECLARE and are initialized very early, not as
> part of the driver model and thus can't defer).  Currently we have an
> #ifdef CONFIG_PPC for this in drivers/clk/clk-qoriq.c... Maybe we should
> have done that here as well, and saved some grief. :-)  At least until an
> erratum pops up on an ARM-based chip.
> 
> And what happens if we're running on arm32, and thus the arch code
> already registered an soc_device with a different (and less useful)
> encoding?
> 
> -Scott
Yangbo Lu May 26, 2016, 4:05 a.m. UTC | #6
Hi Uffe,

Could we merge this patchset? ...
It has been a long time to wait for Arnd's response...
 
Thanks a lot.


Best regards,
Yangbo Lu


> -----Original Message-----
> From: Yangbo Lu
> Sent: Friday, May 20, 2016 2:06 PM
> To: 'Scott Wood'; Arnd Bergmann; linux-arm-kernel@lists.infradead.org
> Cc: linuxppc-dev@lists.ozlabs.org; Mark Rutland;
> devicetree@vger.kernel.org; ulf.hansson@linaro.org; Russell King; Bhupesh
> Sharma; netdev@vger.kernel.org; Joerg Roedel; Kumar Gala; linux-
> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Yang-Leo Li;
> iommu@lists.linux-foundation.org; Rob Herring; linux-i2c@vger.kernel.org;
> Claudiu Manoil; Santosh Shilimkar; Xiaobo Xie; linux-clk@vger.kernel.org;
> Qiang Zhao
> Subject: RE: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
> 
> Hi Arnd,
> 
> Any comments?
> Please reply when you see the email since we want this eSDHC issue to be
> fixed soon.
> 
> All the patches are Freescale-specific and is to fix the eSDHC driver.
> Could we let them merged first if you were talking about a new way of
> abstracting getting SoC version.
> 
> 
> Thanks :)
> 
> 
> Best regards,
> Yangbo Lu
>
Ulf Hansson May 26, 2016, 7:44 a.m. UTC | #7
On 26 May 2016 at 06:05, Yangbo Lu <yangbo.lu@nxp.com> wrote:
> Hi Uffe,
>
> Could we merge this patchset? ...
> It has been a long time to wait for Arnd's response...
>
> Thanks a lot.
>
>

As we are still in the merge window I won't queue anything but fixes.
Let's give Arnd another week or so to respond.

Kind regards
Uffe

> Best regards,
> Yangbo Lu
>
>
>> -----Original Message-----
>> From: Yangbo Lu
>> Sent: Friday, May 20, 2016 2:06 PM
>> To: 'Scott Wood'; Arnd Bergmann; linux-arm-kernel@lists.infradead.org
>> Cc: linuxppc-dev@lists.ozlabs.org; Mark Rutland;
>> devicetree@vger.kernel.org; ulf.hansson@linaro.org; Russell King; Bhupesh
>> Sharma; netdev@vger.kernel.org; Joerg Roedel; Kumar Gala; linux-
>> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Yang-Leo Li;
>> iommu@lists.linux-foundation.org; Rob Herring; linux-i2c@vger.kernel.org;
>> Claudiu Manoil; Santosh Shilimkar; Xiaobo Xie; linux-clk@vger.kernel.org;
>> Qiang Zhao
>> Subject: RE: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
>> R1.0-R2.0
>>
>> Hi Arnd,
>>
>> Any comments?
>> Please reply when you see the email since we want this eSDHC issue to be
>> fixed soon.
>>
>> All the patches are Freescale-specific and is to fix the eSDHC driver.
>> Could we let them merged first if you were talking about a new way of
>> abstracting getting SoC version.
>>
>>
>> Thanks :)
>>
>>
>> Best regards,
>> Yangbo Lu
>>
>
Arnd Bergmann May 30, 2016, 1:13 p.m. UTC | #8
On Thursday, May 26, 2016 9:44:10 AM CEST Ulf Hansson wrote:
> On 26 May 2016 at 06:05, Yangbo Lu <yangbo.lu@nxp.com> wrote:
> > Hi Uffe,
> >
> > Could we merge this patchset? ...
> > It has been a long time to wait for Arnd's response...
> >
> > Thanks a lot.
> >
> >
> 
> As we are still in the merge window I won't queue anything but fixes.
> Let's give Arnd another week or so to respond.

I've got a patch series now that implements a method for matching
the soc ID, see the following emails.

	Arnd
diff mbox

Patch

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 0aa484c..e15e836 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -143,6 +143,7 @@  config MMC_SDHCI_OF_ESDHC
 	depends on MMC_SDHCI_PLTFM
 	depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE
 	select MMC_SDHCI_IO_ACCESSORS
+	select FSL_GUTS
 	help
 	  This selects the Freescale eSDHC controller support.
 
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 3f34d35..68cc020 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -18,6 +18,8 @@ 
 #include <linux/of.h>
 #include <linux/delay.h>
 #include <linux/module.h>
+#include <linux/fsl/svr.h>
+#include <linux/fsl/guts.h>
 #include <linux/mmc/host.h>
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
@@ -28,6 +30,8 @@ 
 struct sdhci_esdhc {
 	u8 vendor_ver;
 	u8 spec_ver;
+	u32 soc_ver;
+	u8 soc_rev;
 };
 
 /**
@@ -73,6 +77,8 @@  static u32 esdhc_readl_fixup(struct sdhci_host *host,
 static u16 esdhc_readw_fixup(struct sdhci_host *host,
 				     int spec_reg, u32 value)
 {
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_esdhc *esdhc = sdhci_pltfm_priv(pltfm_host);
 	u16 ret;
 	int shift = (spec_reg & 0x2) * 8;
 
@@ -80,6 +86,13 @@  static u16 esdhc_readw_fixup(struct sdhci_host *host,
 		ret = value & 0xffff;
 	else
 		ret = (value >> shift) & 0xffff;
+
+	/* Workaround for T4240-R1.0-R2.0 eSDHC which has incorrect
+	 * vendor version and spec version information.
+	 */
+	if ((spec_reg == SDHCI_HOST_VERSION) &&
+	    (esdhc->soc_ver == SVR_T4240) && (esdhc->soc_rev <= 0x20))
+		ret = (VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) | SDHCI_SPEC_200;
 	return ret;
 }
 
@@ -567,10 +580,20 @@  static void esdhc_init(struct platform_device *pdev, struct sdhci_host *host)
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_esdhc *esdhc;
 	u16 host_ver;
+	u32 svr;
 
 	pltfm_host = sdhci_priv(host);
 	esdhc = sdhci_pltfm_priv(pltfm_host);
 
+	fsl_guts_init();
+	svr = fsl_guts_get_svr();
+	if (svr) {
+		esdhc->soc_ver = SVR_SOC_VER(svr);
+		esdhc->soc_rev = SVR_REV(svr);
+	} else {
+		dev_err(&pdev->dev, "Failed to get SVR value!\n");
+	}
+
 	host_ver = sdhci_readw(host, SDHCI_HOST_VERSION);
 	esdhc->vendor_ver = (host_ver & SDHCI_VENDOR_VER_MASK) >>
 			     SDHCI_VENDOR_VER_SHIFT;