Re: [U-Boot] [PATCH v2 08/15] dm: mmc: sunxi: Pass private data around explicitly

2017-08-23 Thread Chen-Yu Tsai 
On Tue, Aug 15, 2017 at 5:35 AM, Simon Glass  wrote:
> Hi Chen-Yu,
>
> On 8 August 2017 at 21:27, Chen-Yu Tsai  wrote:
>> Hi Simon,
>>
>> On Wed, Jul 5, 2017 at 3:31 AM, Simon Glass  wrote:
>>> At present the driver-private data is obtained in various functions by
>>> various means. With driver model this is provided automatically. Without
>>> driver model it comes from a C array declared at the top of the file.
>>>
>>> Adjust internal functions so that they are passed the private data as
>>> a parameter, allowing the caller to obtain it using either means.
>>>
>>> Signed-off-by: Simon Glass 
>>
>> eMMC is currently broken for sunxi on my Orangepi Plus 2E.
>> I've narrowed it down to this patch.
>>
>> It seems the driver or device is now referencing the wrong
>> controller. On versions before this patch, for MMC1 (or eMMC):
>>
>> => mmc dev 1
>> switch to partitions #0, OK
>> mmc1(part 0) is current device
>> => mmc info
>> Device: SUNXI SD/MMC
>> Manufacturer ID: 15
>> OEM: 100
>> Name: AWPD3
>> Tran Speed: 5200
>> Rd Block Len: 512
>> MMC version 5.0
>> High Capacity: Yes
>> Capacity: 14.6 GiB
>> Bus Width: 8-bit
>> Erase Group Size: 512 KiB
>> HC WP Group Size: 8 MiB
>> User Capacity: 14.6 GiB WRREL
>> Boot Capacity: 4 MiB ENH
>> RPMB Capacity: 4 MiB ENH
>>
>>
>> On later versions I get:
>>
>> => mmc dev 1
>> switch to partitions #0, OK
>> mmc1 is current device
>> => mmc info
>> Device: SUNXI SD/MMC
>> Manufacturer ID: 27
>> OEM: 5048
>> Name: SD08G
>> Tran Speed: 5000
>> Rd Block Len: 512
>> SD version 3.0
>> High Capacity: Yes
>> Capacity: 7.4 GiB
>> Bus Width: 1-bit
>> Erase Group Size: 512 Bytes
>>
>>
>> For reference, mmc0 looks like:
>>
>> => mmc dev 0
>> switch to partitions #0, OK
>> mmc0 is current device
>> => mmc info
>> Device: SUNXI SD/MMC
>> Manufacturer ID: 27
>> OEM: 5048
>> Name: SD08G
>> Tran Speed: 5000
>> Rd Block Len: 512
>> SD version 3.0
>> High Capacity: Yes
>> Capacity: 7.4 GiB
>> Bus Width: 4-bit
>> Erase Group Size: 512 Bytes
>>
>>
>> So it seems somewhere down the line, the driver is getting
>> passed the wrong set of priv data.
>
> Are you sure it was this patch?
>
> The ordering may have changed because there was a strange hack in the
> code before. There was some discussion about it but unfortunately I
> cannot find the thread right now. Can you take a look?

Indeed it was this patch. Maxime's latest patch [1] fixes it.
And looks like the hacks are on their way out as well.

ChenYu

[1] https://lists.denx.de/pipermail/u-boot/2017-August/303443.html
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 08/15] dm: mmc: sunxi: Pass private data around explicitly

2017-08-14 Thread Simon Glass 
Hi Chen-Yu,

On 8 August 2017 at 21:27, Chen-Yu Tsai  wrote:
> Hi Simon,
>
> On Wed, Jul 5, 2017 at 3:31 AM, Simon Glass  wrote:
>> At present the driver-private data is obtained in various functions by
>> various means. With driver model this is provided automatically. Without
>> driver model it comes from a C array declared at the top of the file.
>>
>> Adjust internal functions so that they are passed the private data as
>> a parameter, allowing the caller to obtain it using either means.
>>
>> Signed-off-by: Simon Glass 
>
> eMMC is currently broken for sunxi on my Orangepi Plus 2E.
> I've narrowed it down to this patch.
>
> It seems the driver or device is now referencing the wrong
> controller. On versions before this patch, for MMC1 (or eMMC):
>
> => mmc dev 1
> switch to partitions #0, OK
> mmc1(part 0) is current device
> => mmc info
> Device: SUNXI SD/MMC
> Manufacturer ID: 15
> OEM: 100
> Name: AWPD3
> Tran Speed: 5200
> Rd Block Len: 512
> MMC version 5.0
> High Capacity: Yes
> Capacity: 14.6 GiB
> Bus Width: 8-bit
> Erase Group Size: 512 KiB
> HC WP Group Size: 8 MiB
> User Capacity: 14.6 GiB WRREL
> Boot Capacity: 4 MiB ENH
> RPMB Capacity: 4 MiB ENH
>
>
> On later versions I get:
>
> => mmc dev 1
> switch to partitions #0, OK
> mmc1 is current device
> => mmc info
> Device: SUNXI SD/MMC
> Manufacturer ID: 27
> OEM: 5048
> Name: SD08G
> Tran Speed: 5000
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 7.4 GiB
> Bus Width: 1-bit
> Erase Group Size: 512 Bytes
>
>
> For reference, mmc0 looks like:
>
> => mmc dev 0
> switch to partitions #0, OK
> mmc0 is current device
> => mmc info
> Device: SUNXI SD/MMC
> Manufacturer ID: 27
> OEM: 5048
> Name: SD08G
> Tran Speed: 5000
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 7.4 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
>
>
> So it seems somewhere down the line, the driver is getting
> passed the wrong set of priv data.

Are you sure it was this patch?

The ordering may have changed because there was a strange hack in the
code before. There was some discussion about it but unfortunately I
cannot find the thread right now. Can you take a look?

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 08/15] dm: mmc: sunxi: Pass private data around explicitly

2017-08-08 Thread Chen-Yu Tsai 
Hi Simon,

On Wed, Jul 5, 2017 at 3:31 AM, Simon Glass  wrote:
> At present the driver-private data is obtained in various functions by
> various means. With driver model this is provided automatically. Without
> driver model it comes from a C array declared at the top of the file.
>
> Adjust internal functions so that they are passed the private data as
> a parameter, allowing the caller to obtain it using either means.
>
> Signed-off-by: Simon Glass 

eMMC is currently broken for sunxi on my Orangepi Plus 2E.
I've narrowed it down to this patch.

It seems the driver or device is now referencing the wrong
controller. On versions before this patch, for MMC1 (or eMMC):

=> mmc dev 1
switch to partitions #0, OK
mmc1(part 0) is current device
=> mmc info
Device: SUNXI SD/MMC
Manufacturer ID: 15
OEM: 100
Name: AWPD3
Tran Speed: 5200
Rd Block Len: 512
MMC version 5.0
High Capacity: Yes
Capacity: 14.6 GiB
Bus Width: 8-bit
Erase Group Size: 512 KiB
HC WP Group Size: 8 MiB
User Capacity: 14.6 GiB WRREL
Boot Capacity: 4 MiB ENH
RPMB Capacity: 4 MiB ENH


On later versions I get:

=> mmc dev 1
switch to partitions #0, OK
mmc1 is current device
=> mmc info
Device: SUNXI SD/MMC
Manufacturer ID: 27
OEM: 5048
Name: SD08G
Tran Speed: 5000
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 7.4 GiB
Bus Width: 1-bit
Erase Group Size: 512 Bytes


For reference, mmc0 looks like:

=> mmc dev 0
switch to partitions #0, OK
mmc0 is current device
=> mmc info
Device: SUNXI SD/MMC
Manufacturer ID: 27
OEM: 5048
Name: SD08G
Tran Speed: 5000
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 7.4 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes


So it seems somewhere down the line, the driver is getting
passed the wrong set of priv data.

Regards
ChenYu
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 08/15] dm: mmc: sunxi: Pass private data around explicitly

2017-07-05 Thread Maxime Ripard 
On Tue, Jul 04, 2017 at 01:31:25PM -0600, Simon Glass wrote:
> At present the driver-private data is obtained in various functions by
> various means. With driver model this is provided automatically. Without
> driver model it comes from a C array declared at the top of the file.
> 
> Adjust internal functions so that they are passed the private data as
> a parameter, allowing the caller to obtain it using either means.
> 
> Signed-off-by: Simon Glass 

Acked-by: Maxime Ripard 

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 08/15] dm: mmc: sunxi: Pass private data around explicitly

2017-07-04 Thread Simon Glass 
At present the driver-private data is obtained in various functions by
various means. With driver model this is provided automatically. Without
driver model it comes from a C array declared at the top of the file.

Adjust internal functions so that they are passed the private data as
a parameter, allowing the caller to obtain it using either means.

Signed-off-by: Simon Glass 
---

Changes in v2: None

 drivers/mmc/sunxi_mmc.c | 71 +
 1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
index 9276a29d76..bd41fbb752 100644
--- a/drivers/mmc/sunxi_mmc.c
+++ b/drivers/mmc/sunxi_mmc.c
@@ -176,9 +176,8 @@ static int mmc_clk_io_on(int sdc_no)
return mmc_set_mod_clk(priv, 2400);
 }
 
-static int mmc_update_clk(struct mmc *mmc)
+static int mmc_update_clk(struct sunxi_mmc_priv *priv)
 {
-   struct sunxi_mmc_priv *priv = mmc->priv;
unsigned int cmd;
unsigned timeout_msecs = 2000;
 
@@ -198,15 +197,14 @@ static int mmc_update_clk(struct mmc *mmc)
return 0;
 }
 
-static int mmc_config_clock(struct mmc *mmc)
+static int mmc_config_clock(struct sunxi_mmc_priv *priv, struct mmc *mmc)
 {
-   struct sunxi_mmc_priv *priv = mmc->priv;
unsigned rval = readl(>reg->clkcr);
 
/* Disable Clock */
rval &= ~SUNXI_MMC_CLK_ENABLE;
writel(rval, >reg->clkcr);
-   if (mmc_update_clk(mmc))
+   if (mmc_update_clk(priv))
return -1;
 
/* Set mod_clk to new rate */
@@ -220,21 +218,20 @@ static int mmc_config_clock(struct mmc *mmc)
/* Re-enable Clock */
rval |= SUNXI_MMC_CLK_ENABLE;
writel(rval, >reg->clkcr);
-   if (mmc_update_clk(mmc))
+   if (mmc_update_clk(priv))
return -1;
 
return 0;
 }
 
-static int sunxi_mmc_set_ios(struct mmc *mmc)
+static int sunxi_mmc_set_ios_common(struct sunxi_mmc_priv *priv,
+   struct mmc *mmc)
 {
-   struct sunxi_mmc_priv *priv = mmc->priv;
-
debug("set ios: bus_width: %x, clock: %d\n",
  mmc->bus_width, mmc->clock);
 
/* Change clock first */
-   if (mmc->clock && mmc_config_clock(mmc) != 0) {
+   if (mmc->clock && mmc_config_clock(priv, mmc) != 0) {
priv->fatal_err = 1;
return -EINVAL;
}
@@ -261,9 +258,9 @@ static int sunxi_mmc_core_init(struct mmc *mmc)
return 0;
 }
 
-static int mmc_trans_data_by_cpu(struct mmc *mmc, struct mmc_data *data)
+static int mmc_trans_data_by_cpu(struct sunxi_mmc_priv *priv, struct mmc *mmc,
+struct mmc_data *data)
 {
-   struct sunxi_mmc_priv *priv = mmc->priv;
const int reading = !!(data->flags & MMC_DATA_READ);
const uint32_t status_bit = reading ? SUNXI_MMC_STATUS_FIFO_EMPTY :
  SUNXI_MMC_STATUS_FIFO_FULL;
@@ -293,10 +290,9 @@ static int mmc_trans_data_by_cpu(struct mmc *mmc, struct 
mmc_data *data)
return 0;
 }
 
-static int mmc_rint_wait(struct mmc *mmc, unsigned int timeout_msecs,
-unsigned int done_bit, const char *what)
+static int mmc_rint_wait(struct sunxi_mmc_priv *priv, struct mmc *mmc,
+uint timeout_msecs, uint done_bit, const char *what)
 {
-   struct sunxi_mmc_priv *priv = mmc->priv;
unsigned int status;
 
do {
@@ -313,10 +309,10 @@ static int mmc_rint_wait(struct mmc *mmc, unsigned int 
timeout_msecs,
return 0;
 }
 
-static int sunxi_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
- struct mmc_data *data)
+static int sunxi_mmc_send_cmd_common(struct sunxi_mmc_priv *priv,
+struct mmc *mmc, struct mmc_cmd *cmd,
+struct mmc_data *data)
 {
-   struct sunxi_mmc_priv *priv = mmc->priv;
unsigned int cmdval = SUNXI_MMC_CMD_START;
unsigned int timeout_msecs;
int error = 0;
@@ -372,7 +368,7 @@ static int sunxi_mmc_send_cmd(struct mmc *mmc, struct 
mmc_cmd *cmd,
bytecnt = data->blocksize * data->blocks;
debug("trans data %d bytes\n", bytecnt);
writel(cmdval | cmd->cmdidx, >reg->cmd);
-   ret = mmc_trans_data_by_cpu(mmc, data);
+   ret = mmc_trans_data_by_cpu(priv, mmc, data);
if (ret) {
error = readl(>reg->rint) &
SUNXI_MMC_RINT_INTERRUPT_ERROR_BIT;
@@ -381,14 +377,15 @@ static int sunxi_mmc_send_cmd(struct mmc *mmc, struct 
mmc_cmd *cmd,
}
}
 
-   error = mmc_rint_wait(mmc, 1000, SUNXI_MMC_RINT_COMMAND_DONE, "cmd");
+   error = mmc_rint_wait(priv, mmc, 1000, SUNXI_MMC_RINT_COMMAND_DONE,
+ "cmd");
if (error)
goto out;
 
if (data) {