diff mbox

[v3,2/3] mmc: tmio-mmc: add support for 32bit data port

Message ID 20160912141507.6837-3-chris.brandt@renesas.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Brandt Sept. 12, 2016, 2:15 p.m. UTC
For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.
Therefore a new flag has been created that will allow 32-bit reads/writes
to the DATA_PORT register instead of 16-bit (because 16-bits accesses are
not supported).

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v3:
* changed loops to memcpy
v2:
* changed 'data * 0xFF' to 'data & 0xFF'
* added 'const' for sd_ctrl_write32_rep
---
 drivers/mmc/host/tmio_mmc.h     | 12 ++++++++++++
 drivers/mmc/host/tmio_mmc_pio.c | 30 ++++++++++++++++++++++++++++++
 include/linux/mfd/tmio.h        |  5 +++++
 3 files changed, 47 insertions(+)

Comments

Ulf Hansson Sept. 22, 2016, 8:13 a.m. UTC | #1
+ Lee

On 12 September 2016 at 16:15, Chris Brandt <chris.brandt@renesas.com> wrote:
> For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.
> Therefore a new flag has been created that will allow 32-bit reads/writes
> to the DATA_PORT register instead of 16-bit (because 16-bits accesses are
> not supported).
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v3:
> * changed loops to memcpy
> v2:
> * changed 'data * 0xFF' to 'data & 0xFF'
> * added 'const' for sd_ctrl_write32_rep
> ---
>  drivers/mmc/host/tmio_mmc.h     | 12 ++++++++++++
>  drivers/mmc/host/tmio_mmc_pio.c | 30 ++++++++++++++++++++++++++++++
>  include/linux/mfd/tmio.h        |  5 +++++

This header file needs to be split up. The mmc specific bits, should
be moved to a local header file under driver/mmc/host/*.

Sure, we don't need to do that as part of $subject patch, but then I
need an ack from Lee Jones, the mfd maintainer. I have added him on
cc.

>  3 files changed, 47 insertions(+)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index ecb99fc..a99e634 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -237,6 +237,12 @@ static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host, int ad
>                readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
>  }
>
> +static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int addr,
> +               u32 *buf, int count)
> +{
> +       readsl(host->ctl + (addr << host->bus_shift), buf, count);
> +}
> +
>  static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
>  {
>         /* If there is a hook and it returns non-zero then there
> @@ -259,4 +265,10 @@ static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host, int
>         writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
>  }
>
> +static inline void sd_ctrl_write32_rep(struct tmio_mmc_host *host, int addr,
> +               const u32 *buf, int count)
> +{
> +       writesl(host->ctl + (addr << host->bus_shift), buf, count);
> +}
> +
>  #endif
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 017a4dc..59fac7d 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -443,6 +443,36 @@ static void tmio_mmc_transfer_data(struct tmio_mmc_host *host,
>         /*
>          * Transfer the data
>          */
> +       if (host->pdata->flags & TMIO_MMC_32BIT_DATA_PORT) {
> +               u8 data[4] = { };
> +
> +               if (is_read)
> +                       sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, (u32 *)buf,
> +                                          count >> 2);
> +               else
> +                       sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, (u32 *)buf,
> +                                           count >> 2);
> +
> +               /* if count was multiple of 4 */
> +               if (!(count & 0x3))
> +                       return;
> +
> +               buf8 = (u8 *)(buf + (count >> 2));
> +               count %= 4;
> +
> +               if (is_read) {
> +                       sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT,
> +                                          (u32 *)data, 1);
> +                       memcpy(buf8, data, count);
> +               } else {
> +                       memcpy(data, buf8, count);
> +                       sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT,
> +                                           (u32 *)data, 1);
> +               }
> +
> +               return;
> +       }
> +
>         if (is_read)
>                 sd_ctrl_read16_rep(host, CTL_SD_DATA_PORT, buf, count >> 1);
>         else
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index 3b95dc7..0dbcb7e 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -100,6 +100,11 @@
>  #define TMIO_MMC_SDIO_STATUS_QUIRK     (1 << 8)
>
>  /*
> + * Some controllers have a 32-bit wide data port register
> + */
> +#define TMIO_MMC_32BIT_DATA_PORT       (1 << 9)
> +
> +/*
>   * Some controllers allows to set SDx actual clock
>   */
>  #define TMIO_MMC_CLK_ACTUAL            (1 << 10)
> --
> 2.9.2
>
>

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Brandt Oct. 14, 2016, 1:18 p.m. UTC | #2
On 9/22/2016, Ulf Hansson wrote:
> To: Chris Brandt <Chris.Brandt@renesas.com>

> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>; Sergei Shtylyov

> <sergei.shtylyov@cogentembedded.com>; Geert Uytterhoeven <geert@linux-

> m68k.org>; Simon Horman <horms+renesas@verge.net.au>; linux-mmc <linux-

> mmc@vger.kernel.org>; Linux-Renesas <linux-renesas-soc@vger.kernel.org>;

> Lee Jones <lee@kernel.org>

> Subject: Re: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port

> 

> + Lee

> 

> On 12 September 2016 at 16:15, Chris Brandt <chris.brandt@renesas.com>

> wrote:

> > For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.

> > Therefore a new flag has been created that will allow 32-bit

> > reads/writes to the DATA_PORT register instead of 16-bit (because

> > 16-bits accesses are not supported).

> >

> > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

> > ---

> > v3:

> > * changed loops to memcpy

> > v2:

> > * changed 'data * 0xFF' to 'data & 0xFF'

> > * added 'const' for sd_ctrl_write32_rep

> > ---

> >  drivers/mmc/host/tmio_mmc.h     | 12 ++++++++++++

> >  drivers/mmc/host/tmio_mmc_pio.c | 30 ++++++++++++++++++++++++++++++

> >  include/linux/mfd/tmio.h        |  5 +++++

> 

> This header file needs to be split up. The mmc specific bits, should be

> moved to a local header file under driver/mmc/host/*.

> 

> Sure, we don't need to do that as part of $subject patch, but then I need

> an ack from Lee Jones, the mfd maintainer. I have added him on cc.



Is this still waiting on an ACK from Lee Jones because it touches the tmio.h file under include/linux/mfd/ ?





 
> >  3 files changed, 47 insertions(+)

> >

> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h

> > index ecb99fc..a99e634 100644

> > --- a/drivers/mmc/host/tmio_mmc.h

> > +++ b/drivers/mmc/host/tmio_mmc.h

> > @@ -237,6 +237,12 @@ static inline u32

> sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host, int ad

> >                readw(host->ctl + ((addr + 2) << host->bus_shift)) <<

> > 16;  }

> >

> > +static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int

> addr,

> > +               u32 *buf, int count)

> > +{

> > +       readsl(host->ctl + (addr << host->bus_shift), buf, count); }

> > +

> >  static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int

> > addr, u16 val)  {

> >         /* If there is a hook and it returns non-zero then there @@

> > -259,4 +265,10 @@ static inline void sd_ctrl_write32_as_16_and_16(struct

> tmio_mmc_host *host, int

> >         writew(val >> 16, host->ctl + ((addr + 2) <<

> > host->bus_shift));  }

> >

> > +static inline void sd_ctrl_write32_rep(struct tmio_mmc_host *host, int

> addr,

> > +               const u32 *buf, int count) {

> > +       writesl(host->ctl + (addr << host->bus_shift), buf, count); }

> > +

> >  #endif

> > diff --git a/drivers/mmc/host/tmio_mmc_pio.c

> > b/drivers/mmc/host/tmio_mmc_pio.c index 017a4dc..59fac7d 100644

> > --- a/drivers/mmc/host/tmio_mmc_pio.c

> > +++ b/drivers/mmc/host/tmio_mmc_pio.c

> > @@ -443,6 +443,36 @@ static void tmio_mmc_transfer_data(struct

> tmio_mmc_host *host,

> >         /*

> >          * Transfer the data

> >          */

> > +       if (host->pdata->flags & TMIO_MMC_32BIT_DATA_PORT) {

> > +               u8 data[4] = { };

> > +

> > +               if (is_read)

> > +                       sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, (u32

> *)buf,

> > +                                          count >> 2);

> > +               else

> > +                       sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, (u32

> *)buf,

> > +                                           count >> 2);

> > +

> > +               /* if count was multiple of 4 */

> > +               if (!(count & 0x3))

> > +                       return;

> > +

> > +               buf8 = (u8 *)(buf + (count >> 2));

> > +               count %= 4;

> > +

> > +               if (is_read) {

> > +                       sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT,

> > +                                          (u32 *)data, 1);

> > +                       memcpy(buf8, data, count);

> > +               } else {

> > +                       memcpy(data, buf8, count);

> > +                       sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT,

> > +                                           (u32 *)data, 1);

> > +               }

> > +

> > +               return;

> > +       }

> > +

> >         if (is_read)

> >                 sd_ctrl_read16_rep(host, CTL_SD_DATA_PORT, buf, count >>

> 1);

> >         else

> > diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h index

> > 3b95dc7..0dbcb7e 100644

> > --- a/include/linux/mfd/tmio.h

> > +++ b/include/linux/mfd/tmio.h

> > @@ -100,6 +100,11 @@

> >  #define TMIO_MMC_SDIO_STATUS_QUIRK     (1 << 8)

> >

> >  /*

> > + * Some controllers have a 32-bit wide data port register  */

> > +#define TMIO_MMC_32BIT_DATA_PORT       (1 << 9)

> > +

> > +/*

> >   * Some controllers allows to set SDx actual clock

> >   */

> >  #define TMIO_MMC_CLK_ACTUAL            (1 << 10)

> > --

> > 2.9.2

> >

> >

> 

> Kind regards

> Uffe
Ulf Hansson Oct. 17, 2016, 1:36 p.m. UTC | #3
On 14 October 2016 at 15:18, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On 9/22/2016, Ulf Hansson wrote:
>> To: Chris Brandt <Chris.Brandt@renesas.com>
>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>; Sergei Shtylyov
>> <sergei.shtylyov@cogentembedded.com>; Geert Uytterhoeven <geert@linux-
>> m68k.org>; Simon Horman <horms+renesas@verge.net.au>; linux-mmc <linux-
>> mmc@vger.kernel.org>; Linux-Renesas <linux-renesas-soc@vger.kernel.org>;
>> Lee Jones <lee@kernel.org>
>> Subject: Re: [PATCH v3 2/3] mmc: tmio-mmc: add support for 32bit data port
>>
>> + Lee
>>
>> On 12 September 2016 at 16:15, Chris Brandt <chris.brandt@renesas.com>
>> wrote:
>> > For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.
>> > Therefore a new flag has been created that will allow 32-bit
>> > reads/writes to the DATA_PORT register instead of 16-bit (because
>> > 16-bits accesses are not supported).
>> >
>> > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
>> > ---
>> > v3:
>> > * changed loops to memcpy
>> > v2:
>> > * changed 'data * 0xFF' to 'data & 0xFF'
>> > * added 'const' for sd_ctrl_write32_rep
>> > ---
>> >  drivers/mmc/host/tmio_mmc.h     | 12 ++++++++++++
>> >  drivers/mmc/host/tmio_mmc_pio.c | 30 ++++++++++++++++++++++++++++++
>> >  include/linux/mfd/tmio.h        |  5 +++++
>>
>> This header file needs to be split up. The mmc specific bits, should be
>> moved to a local header file under driver/mmc/host/*.
>>
>> Sure, we don't need to do that as part of $subject patch, but then I need
>> an ack from Lee Jones, the mfd maintainer. I have added him on cc.
>
>
> Is this still waiting on an ACK from Lee Jones because it touches the tmio.h file under include/linux/mfd/ ?
>

Yes, but more important is Wolfram's ack as he maintains
drivers/mmc/host/tmio_mmc*

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Oct. 17, 2016, 3:15 p.m. UTC | #4
> > Is this still waiting on an ACK from Lee Jones because it touches the tmio.h file under include/linux/mfd/ ?
> >
> 
> Yes, but more important is Wolfram's ack as he maintains
> drivers/mmc/host/tmio_mmc*

Sorry, LinuxCon and ELC Europe kept me very busy. Will do this week.
Wolfram Sang Oct. 20, 2016, 1:28 p.m. UTC | #5
On Mon, Sep 12, 2016 at 10:15:06AM -0400, Chris Brandt wrote:
> For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.
> Therefore a new flag has been created that will allow 32-bit reads/writes
> to the DATA_PORT register instead of 16-bit (because 16-bits accesses are
> not supported).
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v3:
> * changed loops to memcpy
> v2:
> * changed 'data * 0xFF' to 'data & 0xFF'
> * added 'const' for sd_ctrl_write32_rep

Sadly, I don't have SDHI documentation for this SoC.

* Does it have a version register (CTL_VERSION)? If so, what does it
  say?
* Does it have SD_BUF0 width setting (named either EXT_ACC or HOST_MODE,
  so far always comes after the version register)? If so, what is its
  layout?

Unrelated to this patch but nice to know:

* Does it support DMA? Is it compatible to the current implementation?

That being asked, R-Car SoCs can have SDBUF0 32-bit wide as well (Gen3
even 64-bit). So far, this is only used for DMA, though.

Thanks,

   Wolfram

> +		/* if count was multiple of 4 */
> +		if (!(count & 0x3))
> +			return;
> +		buf8 = (u8 *)(buf + (count >> 2));
> +		count %= 4;

To skip the same operation done on 'count' twice, maybe?

		buf8 = (u8 *)(buf + (count >> 2));
		count &= 3;

		if (!count)
			return;
Chris Brandt Oct. 20, 2016, 2:35 p.m. UTC | #6
On 10/20/2016, Wolfram Sang wrote:
> Sadly, I don't have SDHI documentation for this SoC.


In the current hardware manual on renesas.com, the SHDI section is not included.
However, for version 3.0 that will be posted shortly, it will start to include the SDHI section.


> * Does it have a version register (CTL_VERSION)? If so, what does it

>   say?


0x820B


> * Does it have SD_BUF0 width setting (named either EXT_ACC or HOST_MODE,

>   so far always comes after the version register)? If so, what is its

>   layout?


There is a "EXT_SWAP" register. It is the only thing after VERSION.
   VERSION = 0xE804E0E2
  EXT_SWAP = 0xE804E0F0

Here are the bits in that register:

DMASE (bit 8)
	DMA Transfer Size Select
	Selects the transfer unit for SD_BUF read/write DMA transfer. Set this bit
	in combination with the transfer size set in the DMA Channel
	Configuration register.
	0: 4-byte (longword) unit
	1: 64-byte (longword  16) unit

SDBRSWAP (bit 7)
	SD_BUF0 Swap Read*1
	When reading from SD_BUF0, data stored in SD_BUF0 can be replaced
	and then read.*2
	0: The current data is read without replacement.
	1: Data replacement for reading proceeds in bytes

SDBWSWAP (bit 6)
	SD_BUF0 Swap Write*1
	When writing to SD_BUF0, data to be written can be replaced and then
	stored in SD_BUF0.*2
	0: The current data is written without replacement.
	1: Data replacement for writing proceeds in bytes.


> * Does it support DMA?

Yes.


> Is it compatible to the current implementation?


I don't know yet. The current BSP is on 3.14 and it uses DMA, but I'm not sure what's going to happen as I start moving things to 4.9+.




> > +		/* if count was multiple of 4 */

> > +		if (!(count & 0x3))

> > +			return;

> > +		buf8 = (u8 *)(buf + (count >> 2));

> > +		count %= 4;

> 

> To skip the same operation done on 'count' twice, maybe?

> 

> 		buf8 = (u8 *)(buf + (count >> 2));

> 		count &= 3;

> 

> 		if (!count)

> 			return;



That looks like it should work.
I can try it out and see if there is something we are missing.


Thanks

Chris
Chris Brandt Oct. 20, 2016, 7:46 p.m. UTC | #7
Hello Wolfram,

> > > +		/* if count was multiple of 4 */

> > > +		if (!(count & 0x3))

> > > +			return;

> > > +		buf8 = (u8 *)(buf + (count >> 2));

> > > +		count %= 4;

> >

> > To skip the same operation done on 'count' twice, maybe?

> >

> > 		buf8 = (u8 *)(buf + (count >> 2));

> > 		count &= 3;

> >

> > 		if (!count)

> > 			return;

> 

> 

> That looks like it should work.

> I can try it out and see if there is something we are missing.


Seems to work fine. Although I admit that because the driver is mostly being used as a block device (SD card), all the data requests are an even multiple of 4 (I put a printk after "if (!count) return;" ). Not sure if that will be the case when I test SDIO devices, but I'll find out once I start porting SDIO device drivers to 4.9+.


I'll submit a v4 of this patch series tomorrow.

Thank you.

Chris
Wolfram Sang Oct. 20, 2016, 11:04 p.m. UTC | #8
> I'll submit a v4 of this patch series tomorrow.

Please wait a little. I'd like to use that feature on R-Car, too, so I
wonder if the additional feature flag is the proper path.
Wolfram Sang Oct. 20, 2016, 11:12 p.m. UTC | #9
> However, for version 3.0 that will be posted shortly, it will start to include the SDHI section.

Nice. Could you ping me when this comes out?

> > * Does it have a version register (CTL_VERSION)? If so, what does it
> >   say?
> 
> 0x820B

Okay, this is a version I have not seen before.

> > * Does it have SD_BUF0 width setting (named either EXT_ACC or HOST_MODE,
> >   so far always comes after the version register)? If so, what is its
> >   layout?
> 
> There is a "EXT_SWAP" register. It is the only thing after VERSION.
>    VERSION = 0xE804E0E2
>   EXT_SWAP = 0xE804E0F0

This SWAP register exists on R-Car as well. Out of curiosity, what is the
register value of 0xE804E0E4?
Chris Brandt Oct. 21, 2016, 1:43 p.m. UTC | #10
> > > * Does it have a version register (CTL_VERSION)? If so, what does it
> > >   say?
> >
> > 0x820B
> 
> Okay, this is a version I have not seen before.

The SDHI IP came from the SH7269 (SH-2A device).

For the VERSION register, the low byte is the version of the IP, but the upper byte is a number that the design group that made the part gives it any time they make a change. So, since this IP was from a SH2A team, the numbering might look different than what the SH4A or R-Car teams have used in the past.



> This SWAP register exists on R-Car as well. Out of curiosity, what is the
> register value of 0xE804E0E4?

0xE804E0E4 = 0x0001

So...something is there!



Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Oct. 21, 2016, 9:56 p.m. UTC | #11
> For the VERSION register, the low byte is the version of the IP, but
> the upper byte is a number that the design group that made the part

I know. It is just that I haven't seen this one "in the wild" so far.

> > This SWAP register exists on R-Car as well. Out of curiosity, what is the
> > register value of 0xE804E0E4?
> 
> 0xE804E0E4 = 0x0001
> 
> So...something is there!

I am quite convinced there is this BUSWIDTH register. If you are
interested, try setting this to 0 and see if it works without the 32 bit
register patches.
Geert Uytterhoeven Oct. 24, 2016, 11:06 a.m. UTC | #12
Hi Wolfram,

On Fri, Oct 21, 2016 at 11:56 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> For the VERSION register, the low byte is the version of the IP, but
>> the upper byte is a number that the design group that made the part
>
> I know. It is just that I haven't seen this one "in the wild" so far.

I think you can also see it on your tamed Genmai ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Oct. 24, 2016, 11:11 a.m. UTC | #13
On Mon, Oct 24, 2016 at 01:06:38PM +0200, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Fri, Oct 21, 2016 at 11:56 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >> For the VERSION register, the low byte is the version of the IP, but
> >> the upper byte is a number that the design group that made the part
> >
> > I know. It is just that I haven't seen this one "in the wild" so far.
> 
> I think you can also see it on your tamed Genmai ;-)

I intentionally did not write "couldn't have seen this" ;)
Chris Brandt Oct. 24, 2016, 12:37 p.m. UTC | #14
> > > This SWAP register exists on R-Car as well. Out of curiosity, what
> > > is the register value of 0xE804E0E4?
> >
> > 0xE804E0E4 = 0x0001
> >
> > So...something is there!
> 
> I am quite convinced there is this BUSWIDTH register. If you are
> interested, try setting this to 0 and see if it works without the 32 bit
> register patches.

Well, it looks like I can't write that register location to 0.
It always keeps a value of 0x0001. So, it must be read-only.

It was a good idea though.
Still, I might try to find out what is there at that location out of curiosity.

Without my the 32-bit write patch, I get a lot of these messages:

    sh_mobile_sdhi e804e800.sd: timeout waiting for SD bus idle



Chris

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Nov. 1, 2016, 8:49 a.m. UTC | #15
Hi Chris,

On Mon, Sep 12, 2016 at 10:15:06AM -0400, Chris Brandt wrote:
> For the r7s72100 SOC, the DATA_PORT register was changed to 32-bits wide.
> Therefore a new flag has been created that will allow 32-bit reads/writes
> to the DATA_PORT register instead of 16-bit (because 16-bits accesses are
> not supported).
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Okay, I have a sketch how to add this feature to R-Car SoCs at a later
stage. This patch is fine with one minor nit which can be done now or I
can do it later when I add the R-Car support.

> + * Some controllers have a 32-bit wide data port register
> + */
> +#define TMIO_MMC_32BIT_DATA_PORT	(1 << 9)

Since R-Car Gen3 has 64 bit port, I'd suggest to use
TMIO_MMC_BIG_DATA_PORT or something. But as I said, I can also do this
later. So

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Chris Brandt Nov. 1, 2016, 1:40 p.m. UTC | #16
Hi Wolfram,


> Okay, I have a sketch how to add this feature to R-Car SoCs at a later
> stage. This patch is fine with one minor nit which can be done now or I
> can do it later when I add the R-Car support.
> 
> > + * Some controllers have a 32-bit wide data port register  */
> > +#define TMIO_MMC_32BIT_DATA_PORT	(1 << 9)
> 
> Since R-Car Gen3 has 64 bit port, I'd suggest to use
> TMIO_MMC_BIG_DATA_PORT or something. But as I said, I can also do this
> later. So

I don't understand "TMIO_MMC_BIG_DATA_PORT" (How do you know 32-bit?? How do you know 64-bit??)


So......maybe you can change it later and your idea will be more clear to me then.


Thank you,
Chris

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Nov. 1, 2016, 3:07 p.m. UTC | #17
> I don't understand "TMIO_MMC_BIG_DATA_PORT" (How do you know 32-bit?? How do you know 64-bit??)

I think 16 << host->bus_shift should work; need to verify this, though.

> So......maybe you can change it later and your idea will be more clear to me then.

Yes, fine with me.
Chris Brandt Nov. 1, 2016, 3:13 p.m. UTC | #18
> > I don't understand "TMIO_MMC_BIG_DATA_PORT" (How do you know 32-bit??
> How do you know 64-bit??)
> 
> I think 16 << host->bus_shift should work; need to verify this, though.


Good idea.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index ecb99fc..a99e634 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -237,6 +237,12 @@  static inline u32 sd_ctrl_read16_and_16_as_32(struct tmio_mmc_host *host, int ad
 	       readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
 }
 
+static inline void sd_ctrl_read32_rep(struct tmio_mmc_host *host, int addr,
+		u32 *buf, int count)
+{
+	readsl(host->ctl + (addr << host->bus_shift), buf, count);
+}
+
 static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
 {
 	/* If there is a hook and it returns non-zero then there
@@ -259,4 +265,10 @@  static inline void sd_ctrl_write32_as_16_and_16(struct tmio_mmc_host *host, int
 	writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
 }
 
+static inline void sd_ctrl_write32_rep(struct tmio_mmc_host *host, int addr,
+		const u32 *buf, int count)
+{
+	writesl(host->ctl + (addr << host->bus_shift), buf, count);
+}
+
 #endif
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 017a4dc..59fac7d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -443,6 +443,36 @@  static void tmio_mmc_transfer_data(struct tmio_mmc_host *host,
 	/*
 	 * Transfer the data
 	 */
+	if (host->pdata->flags & TMIO_MMC_32BIT_DATA_PORT) {
+		u8 data[4] = { };
+
+		if (is_read)
+			sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT, (u32 *)buf,
+					   count >> 2);
+		else
+			sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT, (u32 *)buf,
+					    count >> 2);
+
+		/* if count was multiple of 4 */
+		if (!(count & 0x3))
+			return;
+
+		buf8 = (u8 *)(buf + (count >> 2));
+		count %= 4;
+
+		if (is_read) {
+			sd_ctrl_read32_rep(host, CTL_SD_DATA_PORT,
+					   (u32 *)data, 1);
+			memcpy(buf8, data, count);
+		} else {
+			memcpy(data, buf8, count);
+			sd_ctrl_write32_rep(host, CTL_SD_DATA_PORT,
+					    (u32 *)data, 1);
+		}
+
+		return;
+	}
+
 	if (is_read)
 		sd_ctrl_read16_rep(host, CTL_SD_DATA_PORT, buf, count >> 1);
 	else
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 3b95dc7..0dbcb7e 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -100,6 +100,11 @@ 
 #define TMIO_MMC_SDIO_STATUS_QUIRK	(1 << 8)
 
 /*
+ * Some controllers have a 32-bit wide data port register
+ */
+#define TMIO_MMC_32BIT_DATA_PORT	(1 << 9)
+
+/*
  * Some controllers allows to set SDx actual clock
  */
 #define TMIO_MMC_CLK_ACTUAL		(1 << 10)