2019-09-05 08:29:43

by Zhou Yanjie

[permalink] [raw]
Subject: MMC: Ingenic: Add support for 8bit mode and LPM and JZ4760 Soc

1.adjust the macro definition name to match the corresponding
register name in the datasheet.
2.add support for 8bit mode, now supports 1bit/4bit/8bit modes.
3.add support for probing mmc driver on the JZ4760 Soc from Ingenic.
4.add support for Low Power Mode of Ingenic's MMC/SD Controller.



2019-09-05 08:29:52

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 2/4] MMC: Ingenic: Add 8bit mode support.

Add support for 8bit mode, now supports 1bit/4bit/8bit modes.

Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/mmc/host/jz4740_mmc.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 1b1fcb7..d6811a7 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -79,6 +79,8 @@

#define JZ_MMC_CMDAT_IO_ABORT BIT(11)
#define JZ_MMC_CMDAT_BUS_WIDTH_4BIT BIT(10)
+#define JZ_MMC_CMDAT_BUS_WIDTH_8BIT (BIT(10) | BIT(9))
+#define JZ_MMC_CMDAT_BUS_WIDTH_MASK (BIT(10) | BIT(9))
#define JZ_MMC_CMDAT_DMA_EN BIT(8)
#define JZ_MMC_CMDAT_INIT BIT(7)
#define JZ_MMC_CMDAT_BUSY BIT(6)
@@ -899,11 +901,16 @@ static void jz4740_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

switch (ios->bus_width) {
case MMC_BUS_WIDTH_1:
- host->cmdat &= ~JZ_MMC_CMDAT_BUS_WIDTH_4BIT;
+ host->cmdat &= ~JZ_MMC_CMDAT_BUS_WIDTH_MASK;
break;
case MMC_BUS_WIDTH_4:
+ host->cmdat &= ~JZ_MMC_CMDAT_BUS_WIDTH_MASK;
host->cmdat |= JZ_MMC_CMDAT_BUS_WIDTH_4BIT;
break;
+ case MMC_BUS_WIDTH_8:
+ host->cmdat &= ~JZ_MMC_CMDAT_BUS_WIDTH_MASK;
+ host->cmdat |= JZ_MMC_CMDAT_BUS_WIDTH_8BIT;
+ break;
default:
break;
}
@@ -1034,7 +1041,8 @@ static int jz4740_mmc_probe(struct platform_device* pdev)

dev_info(&pdev->dev, "Using %s, %d-bit mode\n",
host->use_dma ? "DMA" : "PIO",
- (mmc->caps & MMC_CAP_4_BIT_DATA) ? 4 : 1);
+ (mmc->caps & MMC_CAP_8_BIT_DATA) ? 8 :
+ ((mmc->caps & MMC_CAP_4_BIT_DATA) ? 4 : 1));

return 0;

--
2.7.4


2019-09-05 08:29:59

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 4/4] MMC: Ingenic: Add support for JZ4760 and support for LPM.

1.add support for probing mmc driver on the JZ4760 Soc from Ingenic.
2.add support for Low Power Mode of Ingenic's MMC/SD Controller.

Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/mmc/host/jz4740_mmc.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index d6811a7..1e61f1b 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -43,6 +43,7 @@
#define JZ_REG_MMC_RES 0x34
#define JZ_REG_MMC_RXFIFO 0x38
#define JZ_REG_MMC_TXFIFO 0x3C
+#define JZ_REG_MMC_LPM 0x40
#define JZ_REG_MMC_DMAC 0x44

#define JZ_MMC_STRPCL_EXIT_MULTIPLE BIT(7)
@@ -102,11 +103,15 @@
#define JZ_MMC_DMAC_DMA_SEL BIT(1)
#define JZ_MMC_DMAC_DMA_EN BIT(0)

+#define JZ_MMC_LPM_DRV_RISING BIT(31)
+#define JZ_MMC_LPM_LOW_POWER_MODE_EN BIT(0)
+
#define JZ_MMC_CLK_RATE 24000000

enum jz4740_mmc_version {
JZ_MMC_JZ4740,
JZ_MMC_JZ4725B,
+ JZ_MMC_JZ4760,
JZ_MMC_JZ4780,
};

@@ -858,6 +863,16 @@ static int jz4740_mmc_set_clock_rate(struct jz4740_mmc_host *host, int rate)
}

writew(div, host->base + JZ_REG_MMC_CLKRT);
+
+ if (host->version >= JZ_MMC_JZ4760) {
+ if (real_rate > 25000000)
+ writel(JZ_MMC_LPM_DRV_RISING |
+ JZ_MMC_LPM_LOW_POWER_MODE_EN,
+ host->base + JZ_REG_MMC_LPM);
+ } else if (host->version >= JZ_MMC_JZ4725B)
+ writel(JZ_MMC_LPM_LOW_POWER_MODE_EN,
+ host->base + JZ_REG_MMC_LPM);
+
return real_rate;
}

@@ -935,6 +950,7 @@ static const struct mmc_host_ops jz4740_mmc_ops = {
static const struct of_device_id jz4740_mmc_of_match[] = {
{ .compatible = "ingenic,jz4740-mmc", .data = (void *) JZ_MMC_JZ4740 },
{ .compatible = "ingenic,jz4725b-mmc", .data = (void *)JZ_MMC_JZ4725B },
+ { .compatible = "ingenic,jz4760-mmc", .data = (void *) JZ_MMC_JZ4760 },
{ .compatible = "ingenic,jz4780-mmc", .data = (void *) JZ_MMC_JZ4780 },
{},
};
--
2.7.4


2019-09-05 09:10:08

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 3/4] dt-bindings: MMC: Add JZ4760 bindings.

Add the MMC bindings for the JZ4760 Soc from Ingenic.

Signed-off-by: Zhou Yanjie <[email protected]>
---
Documentation/devicetree/bindings/mmc/jz4740.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/jz4740.txt b/Documentation/devicetree/bindings/mmc/jz4740.txt
index 8a6f87f..13796fe 100644
--- a/Documentation/devicetree/bindings/mmc/jz4740.txt
+++ b/Documentation/devicetree/bindings/mmc/jz4740.txt
@@ -8,6 +8,7 @@ Required properties:
- compatible: Should be one of the following:
- "ingenic,jz4740-mmc" for the JZ4740
- "ingenic,jz4725b-mmc" for the JZ4725B
+ - "ingenic,jz4760-mmc" for the JZ4760
- "ingenic,jz4780-mmc" for the JZ4780
- reg: Should contain the MMC controller registers location and length.
- interrupts: Should contain the interrupt specifier of the MMC controller.
--
2.7.4


2019-09-05 09:10:11

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 1/4] MMC: Ingenic: Adjust the macro definition name.

Adjust the macro definition name to match the corresponding
register name in the datasheet.

Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/mmc/host/jz4740_mmc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index ffdbfaa..1b1fcb7 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -28,7 +28,7 @@
#include <asm/mach-jz4740/dma.h>

#define JZ_REG_MMC_STRPCL 0x00
-#define JZ_REG_MMC_STATUS 0x04
+#define JZ_REG_MMC_STAT 0x04
#define JZ_REG_MMC_CLKRT 0x08
#define JZ_REG_MMC_CMDAT 0x0C
#define JZ_REG_MMC_RESTO 0x10
@@ -40,7 +40,7 @@
#define JZ_REG_MMC_IREG 0x28
#define JZ_REG_MMC_CMD 0x2C
#define JZ_REG_MMC_ARG 0x30
-#define JZ_REG_MMC_RESP_FIFO 0x34
+#define JZ_REG_MMC_RES 0x34
#define JZ_REG_MMC_RXFIFO 0x38
#define JZ_REG_MMC_TXFIFO 0x3C
#define JZ_REG_MMC_DMAC 0x44
@@ -391,7 +391,7 @@ static void jz4740_mmc_clock_disable(struct jz4740_mmc_host *host)

writew(JZ_MMC_STRPCL_CLOCK_STOP, host->base + JZ_REG_MMC_STRPCL);
do {
- status = readl(host->base + JZ_REG_MMC_STATUS);
+ status = readl(host->base + JZ_REG_MMC_STAT);
} while (status & JZ_MMC_STATUS_CLK_EN && --timeout);
}

@@ -403,7 +403,7 @@ static void jz4740_mmc_reset(struct jz4740_mmc_host *host)
writew(JZ_MMC_STRPCL_RESET, host->base + JZ_REG_MMC_STRPCL);
udelay(10);
do {
- status = readl(host->base + JZ_REG_MMC_STATUS);
+ status = readl(host->base + JZ_REG_MMC_STAT);
} while (status & JZ_MMC_STATUS_IS_RESETTING && --timeout);
}

@@ -446,7 +446,7 @@ static void jz4740_mmc_transfer_check_state(struct jz4740_mmc_host *host,
{
int status;

- status = readl(host->base + JZ_REG_MMC_STATUS);
+ status = readl(host->base + JZ_REG_MMC_STAT);
if (status & JZ_MMC_STATUS_WRITE_ERROR_MASK) {
if (status & (JZ_MMC_STATUS_TIMEOUT_WRITE)) {
host->req->cmd->error = -ETIMEDOUT;
@@ -580,10 +580,10 @@ static bool jz4740_mmc_read_data(struct jz4740_mmc_host *host,
/* For whatever reason there is sometime one word more in the fifo then
* requested */
timeout = 1000;
- status = readl(host->base + JZ_REG_MMC_STATUS);
+ status = readl(host->base + JZ_REG_MMC_STAT);
while (!(status & JZ_MMC_STATUS_DATA_FIFO_EMPTY) && --timeout) {
d = readl(fifo_addr);
- status = readl(host->base + JZ_REG_MMC_STATUS);
+ status = readl(host->base + JZ_REG_MMC_STAT);
}

return false;
@@ -614,7 +614,7 @@ static void jz4740_mmc_read_response(struct jz4740_mmc_host *host,
{
int i;
uint16_t tmp;
- void __iomem *fifo_addr = host->base + JZ_REG_MMC_RESP_FIFO;
+ void __iomem *fifo_addr = host->base + JZ_REG_MMC_RES;

if (cmd->flags & MMC_RSP_136) {
tmp = readw(fifo_addr);
@@ -797,7 +797,7 @@ static irqreturn_t jz_mmc_irq(int irq, void *devid)
struct mmc_command *cmd = host->cmd;
uint32_t irq_reg, status, tmp;

- status = readl(host->base + JZ_REG_MMC_STATUS);
+ status = readl(host->base + JZ_REG_MMC_STAT);
irq_reg = jz4740_mmc_read_irq_reg(host);

tmp = irq_reg;
--
2.7.4


2019-09-13 15:41:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: MMC: Add JZ4760 bindings.

On Thu, 5 Sep 2019 15:38:08 +0800, Zhou Yanjie wrote:
> Add the MMC bindings for the JZ4760 Soc from Ingenic.
>
> Signed-off-by: Zhou Yanjie <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/jz4740.txt | 1 +
> 1 file changed, 1 insertion(+)
>

Acked-by: Rob Herring <[email protected]>

2019-09-13 16:43:33

by Ezequiel Garcia

[permalink] [raw]
Subject: Re: [PATCH 1/4] MMC: Ingenic: Adjust the macro definition name.

Hi Zhou,

Thanks for your interest in this driver, I'm glad
so see it's more used.

On Thu, 2019-09-05 at 15:38 +0800, Zhou Yanjie wrote:
> Adjust the macro definition name to match the corresponding
> register name in the datasheet.
>

It's not really an issue to have slighlt different
names on the macros. They are currently sufficiently
descriptive, and I don't think it's deserves a patch.

Thanks,
Ezequiel

> Signed-off-by: Zhou Yanjie <[email protected]>
> ---
> drivers/mmc/host/jz4740_mmc.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
> index ffdbfaa..1b1fcb7 100644
> --- a/drivers/mmc/host/jz4740_mmc.c
> +++ b/drivers/mmc/host/jz4740_mmc.c
> @@ -28,7 +28,7 @@
> #include <asm/mach-jz4740/dma.h>
>
> #define JZ_REG_MMC_STRPCL 0x00
> -#define JZ_REG_MMC_STATUS 0x04
> +#define JZ_REG_MMC_STAT 0x04
> #define JZ_REG_MMC_CLKRT 0x08
> #define JZ_REG_MMC_CMDAT 0x0C
> #define JZ_REG_MMC_RESTO 0x10
> @@ -40,7 +40,7 @@
> #define JZ_REG_MMC_IREG 0x28
> #define JZ_REG_MMC_CMD 0x2C
> #define JZ_REG_MMC_ARG 0x30
> -#define JZ_REG_MMC_RESP_FIFO 0x34
> +#define JZ_REG_MMC_RES 0x34
> #define JZ_REG_MMC_RXFIFO 0x38
> #define JZ_REG_MMC_TXFIFO 0x3C
> #define JZ_REG_MMC_DMAC 0x44
> @@ -391,7 +391,7 @@ static void jz4740_mmc_clock_disable(struct jz4740_mmc_host *host)
>
> writew(JZ_MMC_STRPCL_CLOCK_STOP, host->base + JZ_REG_MMC_STRPCL);
> do {
> - status = readl(host->base + JZ_REG_MMC_STATUS);
> + status = readl(host->base + JZ_REG_MMC_STAT);
> } while (status & JZ_MMC_STATUS_CLK_EN && --timeout);
> }
>
> @@ -403,7 +403,7 @@ static void jz4740_mmc_reset(struct jz4740_mmc_host *host)
> writew(JZ_MMC_STRPCL_RESET, host->base + JZ_REG_MMC_STRPCL);
> udelay(10);
> do {
> - status = readl(host->base + JZ_REG_MMC_STATUS);
> + status = readl(host->base + JZ_REG_MMC_STAT);
> } while (status & JZ_MMC_STATUS_IS_RESETTING && --timeout);
> }
>
> @@ -446,7 +446,7 @@ static void jz4740_mmc_transfer_check_state(struct jz4740_mmc_host *host,
> {
> int status;
>
> - status = readl(host->base + JZ_REG_MMC_STATUS);
> + status = readl(host->base + JZ_REG_MMC_STAT);
> if (status & JZ_MMC_STATUS_WRITE_ERROR_MASK) {
> if (status & (JZ_MMC_STATUS_TIMEOUT_WRITE)) {
> host->req->cmd->error = -ETIMEDOUT;
> @@ -580,10 +580,10 @@ static bool jz4740_mmc_read_data(struct jz4740_mmc_host *host,
> /* For whatever reason there is sometime one word more in the fifo then
> * requested */
> timeout = 1000;
> - status = readl(host->base + JZ_REG_MMC_STATUS);
> + status = readl(host->base + JZ_REG_MMC_STAT);
> while (!(status & JZ_MMC_STATUS_DATA_FIFO_EMPTY) && --timeout) {
> d = readl(fifo_addr);
> - status = readl(host->base + JZ_REG_MMC_STATUS);
> + status = readl(host->base + JZ_REG_MMC_STAT);
> }
>
> return false;
> @@ -614,7 +614,7 @@ static void jz4740_mmc_read_response(struct jz4740_mmc_host *host,
> {
> int i;
> uint16_t tmp;
> - void __iomem *fifo_addr = host->base + JZ_REG_MMC_RESP_FIFO;
> + void __iomem *fifo_addr = host->base + JZ_REG_MMC_RES;
>
> if (cmd->flags & MMC_RSP_136) {
> tmp = readw(fifo_addr);
> @@ -797,7 +797,7 @@ static irqreturn_t jz_mmc_irq(int irq, void *devid)
> struct mmc_command *cmd = host->cmd;
> uint32_t irq_reg, status, tmp;
>
> - status = readl(host->base + JZ_REG_MMC_STATUS);
> + status = readl(host->base + JZ_REG_MMC_STAT);
> irq_reg = jz4740_mmc_read_irq_reg(host);
>
> tmp = irq_reg;



2019-09-13 16:49:37

by Zhou Yanjie

[permalink] [raw]
Subject: Re: [PATCH 1/4] MMC: Ingenic: Adjust the macro definition name.

Hi Ezequiel,

On 2019年09月13日 23:32, Ezequiel Garcia wrote:
> Hi Zhou,
>
> Thanks for your interest in this driver, I'm glad
> so see it's more used.
>
> On Thu, 2019-09-05 at 15:38 +0800, Zhou Yanjie wrote:
>> Adjust the macro definition name to match the corresponding
>> register name in the datasheet.
>>
> It's not really an issue to have slighlt different
> names on the macros. They are currently sufficiently
> descriptive, and I don't think it's deserves a patch.
Thanks for your advice, I'll drop this in v2. Do you have any suggestions
for the other three patches?

Best regards!
>
> Thanks,
> Ezequiel
>
>> Signed-off-by: Zhou Yanjie <[email protected]>
>> ---
>> drivers/mmc/host/jz4740_mmc.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
>> index ffdbfaa..1b1fcb7 100644
>> --- a/drivers/mmc/host/jz4740_mmc.c
>> +++ b/drivers/mmc/host/jz4740_mmc.c
>> @@ -28,7 +28,7 @@
>> #include <asm/mach-jz4740/dma.h>
>>
>> #define JZ_REG_MMC_STRPCL 0x00
>> -#define JZ_REG_MMC_STATUS 0x04
>> +#define JZ_REG_MMC_STAT 0x04
>> #define JZ_REG_MMC_CLKRT 0x08
>> #define JZ_REG_MMC_CMDAT 0x0C
>> #define JZ_REG_MMC_RESTO 0x10
>> @@ -40,7 +40,7 @@
>> #define JZ_REG_MMC_IREG 0x28
>> #define JZ_REG_MMC_CMD 0x2C
>> #define JZ_REG_MMC_ARG 0x30
>> -#define JZ_REG_MMC_RESP_FIFO 0x34
>> +#define JZ_REG_MMC_RES 0x34
>> #define JZ_REG_MMC_RXFIFO 0x38
>> #define JZ_REG_MMC_TXFIFO 0x3C
>> #define JZ_REG_MMC_DMAC 0x44
>> @@ -391,7 +391,7 @@ static void jz4740_mmc_clock_disable(struct jz4740_mmc_host *host)
>>
>> writew(JZ_MMC_STRPCL_CLOCK_STOP, host->base + JZ_REG_MMC_STRPCL);
>> do {
>> - status = readl(host->base + JZ_REG_MMC_STATUS);
>> + status = readl(host->base + JZ_REG_MMC_STAT);
>> } while (status & JZ_MMC_STATUS_CLK_EN && --timeout);
>> }
>>
>> @@ -403,7 +403,7 @@ static void jz4740_mmc_reset(struct jz4740_mmc_host *host)
>> writew(JZ_MMC_STRPCL_RESET, host->base + JZ_REG_MMC_STRPCL);
>> udelay(10);
>> do {
>> - status = readl(host->base + JZ_REG_MMC_STATUS);
>> + status = readl(host->base + JZ_REG_MMC_STAT);
>> } while (status & JZ_MMC_STATUS_IS_RESETTING && --timeout);
>> }
>>
>> @@ -446,7 +446,7 @@ static void jz4740_mmc_transfer_check_state(struct jz4740_mmc_host *host,
>> {
>> int status;
>>
>> - status = readl(host->base + JZ_REG_MMC_STATUS);
>> + status = readl(host->base + JZ_REG_MMC_STAT);
>> if (status & JZ_MMC_STATUS_WRITE_ERROR_MASK) {
>> if (status & (JZ_MMC_STATUS_TIMEOUT_WRITE)) {
>> host->req->cmd->error = -ETIMEDOUT;
>> @@ -580,10 +580,10 @@ static bool jz4740_mmc_read_data(struct jz4740_mmc_host *host,
>> /* For whatever reason there is sometime one word more in the fifo then
>> * requested */
>> timeout = 1000;
>> - status = readl(host->base + JZ_REG_MMC_STATUS);
>> + status = readl(host->base + JZ_REG_MMC_STAT);
>> while (!(status & JZ_MMC_STATUS_DATA_FIFO_EMPTY) && --timeout) {
>> d = readl(fifo_addr);
>> - status = readl(host->base + JZ_REG_MMC_STATUS);
>> + status = readl(host->base + JZ_REG_MMC_STAT);
>> }
>>
>> return false;
>> @@ -614,7 +614,7 @@ static void jz4740_mmc_read_response(struct jz4740_mmc_host *host,
>> {
>> int i;
>> uint16_t tmp;
>> - void __iomem *fifo_addr = host->base + JZ_REG_MMC_RESP_FIFO;
>> + void __iomem *fifo_addr = host->base + JZ_REG_MMC_RES;
>>
>> if (cmd->flags & MMC_RSP_136) {
>> tmp = readw(fifo_addr);
>> @@ -797,7 +797,7 @@ static irqreturn_t jz_mmc_irq(int irq, void *devid)
>> struct mmc_command *cmd = host->cmd;
>> uint32_t irq_reg, status, tmp;
>>
>> - status = readl(host->base + JZ_REG_MMC_STATUS);
>> + status = readl(host->base + JZ_REG_MMC_STAT);
>> irq_reg = jz4740_mmc_read_irq_reg(host);
>>
>> tmp = irq_reg;
>
>



2019-10-03 10:07:21

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 4/4] MMC: Ingenic: Add support for JZ4760 and support for LPM.

On Thu, 5 Sep 2019 at 09:40, Zhou Yanjie <[email protected]> wrote:
>
> 1.add support for probing mmc driver on the JZ4760 Soc from Ingenic.
> 2.add support for Low Power Mode of Ingenic's MMC/SD Controller.

Normally we try to make "one" change per patch, unless there are some
good reasons not to.

In this case, it seems like you should rather split this patch into
two separate pieces. Can you please do that?

Additionally, please change the prefix for the commit message header
to start with "mmc: jz4740:"

[...]

Kind regards
Uffe

2019-10-06 06:02:53

by Zhou Yanjie

[permalink] [raw]
Subject: Re: [PATCH 4/4] MMC: Ingenic: Add support for JZ4760 and support for LPM.

Hi Uffe,
On 2019年10月03日 18:00, Ulf Hansson wrote:
> On Thu, 5 Sep 2019 at 09:40, Zhou Yanjie <[email protected]> wrote:
>> 1.add support for probing mmc driver on the JZ4760 Soc from Ingenic.
>> 2.add support for Low Power Mode of Ingenic's MMC/SD Controller.
> Normally we try to make "one" change per patch, unless there are some
> good reasons not to.
>
> In this case, it seems like you should rather split this patch into
> two separate pieces. Can you please do that?

OK,I'll split it in v2.

>
> Additionally, please change the prefix for the commit message header
> to start with "mmc: jz4740:"

sure, it will be change in v2.

>
> [...]
>
> Kind regards
> Uffe
Thanks and best regards!


2019-10-12 05:16:39

by Zhou Yanjie

[permalink] [raw]
Subject: MMC: JZ4740: Add support for 8bit mode and LPM and JZ4760 Soc v2

v1->v2: drop macro definition rename, split patch, add support for x1000.


2019-10-12 05:17:12

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 2/6 v2] dt-bindings: MMC: Add JZ4760 bindings.

Add the MMC bindings for the JZ4760 Soc from Ingenic.

Signed-off-by: Zhou Yanjie <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/mmc/jz4740.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/jz4740.txt b/Documentation/devicetree/bindings/mmc/jz4740.txt
index 8a6f87f..13796fe 100644
--- a/Documentation/devicetree/bindings/mmc/jz4740.txt
+++ b/Documentation/devicetree/bindings/mmc/jz4740.txt
@@ -8,6 +8,7 @@ Required properties:
- compatible: Should be one of the following:
- "ingenic,jz4740-mmc" for the JZ4740
- "ingenic,jz4725b-mmc" for the JZ4725B
+ - "ingenic,jz4760-mmc" for the JZ4760
- "ingenic,jz4780-mmc" for the JZ4780
- reg: Should contain the MMC controller registers location and length.
- interrupts: Should contain the interrupt specifier of the MMC controller.
--
2.7.4


2019-10-12 05:17:44

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 3/6 v2] MMC: JZ4740: Add support for the JZ4760.

Add support for probing mmc driver on the JZ4760 Soc from Ingenic.

Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/mmc/host/jz4740_mmc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 69c4a8b..f4c4890 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -107,6 +107,7 @@
enum jz4740_mmc_version {
JZ_MMC_JZ4740,
JZ_MMC_JZ4725B,
+ JZ_MMC_JZ4760,
JZ_MMC_JZ4780,
};

@@ -935,6 +936,7 @@ static const struct mmc_host_ops jz4740_mmc_ops = {
static const struct of_device_id jz4740_mmc_of_match[] = {
{ .compatible = "ingenic,jz4740-mmc", .data = (void *) JZ_MMC_JZ4740 },
{ .compatible = "ingenic,jz4725b-mmc", .data = (void *)JZ_MMC_JZ4725B },
+ { .compatible = "ingenic,jz4760-mmc", .data = (void *) JZ_MMC_JZ4760 },
{ .compatible = "ingenic,jz4780-mmc", .data = (void *) JZ_MMC_JZ4780 },
{},
};
--
2.7.4


2019-10-12 05:19:00

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 1/6 v2] MMC: Ingenic: Add 8bit mode support.

Add support for 8bit mode, now supports 1bit/4bit/8bit modes.

Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/mmc/host/jz4740_mmc.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index ffdbfaa..69c4a8b 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -79,6 +79,8 @@

#define JZ_MMC_CMDAT_IO_ABORT BIT(11)
#define JZ_MMC_CMDAT_BUS_WIDTH_4BIT BIT(10)
+#define JZ_MMC_CMDAT_BUS_WIDTH_8BIT (BIT(10) | BIT(9))
+#define JZ_MMC_CMDAT_BUS_WIDTH_MASK (BIT(10) | BIT(9))
#define JZ_MMC_CMDAT_DMA_EN BIT(8)
#define JZ_MMC_CMDAT_INIT BIT(7)
#define JZ_MMC_CMDAT_BUSY BIT(6)
@@ -899,11 +901,16 @@ static void jz4740_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

switch (ios->bus_width) {
case MMC_BUS_WIDTH_1:
- host->cmdat &= ~JZ_MMC_CMDAT_BUS_WIDTH_4BIT;
+ host->cmdat &= ~JZ_MMC_CMDAT_BUS_WIDTH_MASK;
break;
case MMC_BUS_WIDTH_4:
+ host->cmdat &= ~JZ_MMC_CMDAT_BUS_WIDTH_MASK;
host->cmdat |= JZ_MMC_CMDAT_BUS_WIDTH_4BIT;
break;
+ case MMC_BUS_WIDTH_8:
+ host->cmdat &= ~JZ_MMC_CMDAT_BUS_WIDTH_MASK;
+ host->cmdat |= JZ_MMC_CMDAT_BUS_WIDTH_8BIT;
+ break;
default:
break;
}
@@ -1034,7 +1041,8 @@ static int jz4740_mmc_probe(struct platform_device* pdev)

dev_info(&pdev->dev, "Using %s, %d-bit mode\n",
host->use_dma ? "DMA" : "PIO",
- (mmc->caps & MMC_CAP_4_BIT_DATA) ? 4 : 1);
+ (mmc->caps & MMC_CAP_8_BIT_DATA) ? 8 :
+ ((mmc->caps & MMC_CAP_4_BIT_DATA) ? 4 : 1));

return 0;

--
2.7.4


2019-10-12 05:20:46

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 4/6 v2] dt-bindings: MMC: Add X1000 bindings.

Add the MMC bindings for the X1000 Soc from Ingenic.

Signed-off-by: Zhou Yanjie <[email protected]>
---
Documentation/devicetree/bindings/mmc/jz4740.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/jz4740.txt b/Documentation/devicetree/bindings/mmc/jz4740.txt
index 13796fe..453d3b9 100644
--- a/Documentation/devicetree/bindings/mmc/jz4740.txt
+++ b/Documentation/devicetree/bindings/mmc/jz4740.txt
@@ -1,8 +1,8 @@
-* Ingenic JZ47xx MMC controllers
+* Ingenic XBurst MMC controllers

This file documents the device tree properties used for the MMC controller in
-Ingenic JZ4740/JZ4780 SoCs. These are in addition to the core MMC properties
-described in mmc.txt.
+Ingenic JZ4740/JZ4760/JZ4780/X1000 SoCs. These are in addition to the core MMC
+properties described in mmc.txt.

Required properties:
- compatible: Should be one of the following:
@@ -10,6 +10,7 @@ Required properties:
- "ingenic,jz4725b-mmc" for the JZ4725B
- "ingenic,jz4760-mmc" for the JZ4760
- "ingenic,jz4780-mmc" for the JZ4780
+ - "ingenic,x1000-mmc" for the X1000
- reg: Should contain the MMC controller registers location and length.
- interrupts: Should contain the interrupt specifier of the MMC controller.
- clocks: Clock for the MMC controller.
--
2.7.4


2019-10-12 05:20:54

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 6/6 v2] MMC: JZ4740: Add support for LPM.

add support for low power mode of Ingenic's MMC/SD Controller.

Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/mmc/host/jz4740_mmc.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 44a04fe..4cbe7fb 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -43,6 +43,7 @@
#define JZ_REG_MMC_RESP_FIFO 0x34
#define JZ_REG_MMC_RXFIFO 0x38
#define JZ_REG_MMC_TXFIFO 0x3C
+#define JZ_REG_MMC_LPM 0x40
#define JZ_REG_MMC_DMAC 0x44

#define JZ_MMC_STRPCL_EXIT_MULTIPLE BIT(7)
@@ -102,6 +103,12 @@
#define JZ_MMC_DMAC_DMA_SEL BIT(1)
#define JZ_MMC_DMAC_DMA_EN BIT(0)

+#define JZ_MMC_LPM_DRV_RISING BIT(31)
+#define JZ_MMC_LPM_DRV_RISING_QTR_PHASE_DLY BIT(31)
+#define JZ_MMC_LPM_DRV_RISING_1NS_DLY BIT(30)
+#define JZ_MMC_LPM_SMP_RISING_QTR_OR_HALF_PHASE_DLY BIT(29)
+#define JZ_MMC_LPM_LOW_POWER_MODE_EN BIT(0)
+
#define JZ_MMC_CLK_RATE 24000000

enum jz4740_mmc_version {
@@ -860,6 +867,22 @@ static int jz4740_mmc_set_clock_rate(struct jz4740_mmc_host *host, int rate)
}

writew(div, host->base + JZ_REG_MMC_CLKRT);
+
+ if (real_rate > 25000000) {
+ if (host->version >= JZ_MMC_X1000) {
+ writel(JZ_MMC_LPM_DRV_RISING_QTR_PHASE_DLY |
+ JZ_MMC_LPM_SMP_RISING_QTR_OR_HALF_PHASE_DLY |
+ JZ_MMC_LPM_LOW_POWER_MODE_EN,
+ host->base + JZ_REG_MMC_LPM);
+ } else if (host->version >= JZ_MMC_JZ4760) {
+ writel(JZ_MMC_LPM_DRV_RISING |
+ JZ_MMC_LPM_LOW_POWER_MODE_EN,
+ host->base + JZ_REG_MMC_LPM);
+ } else if (host->version >= JZ_MMC_JZ4725B)
+ writel(JZ_MMC_LPM_LOW_POWER_MODE_EN,
+ host->base + JZ_REG_MMC_LPM);
+ }
+
return real_rate;
}

--
2.7.4


2019-10-12 05:22:17

by Zhou Yanjie

[permalink] [raw]
Subject: [PATCH 5/6 v2] MMC: JZ4740: Add support for the X1000.

Add support for probing mmc driver on the X1000 Soc from Ingenic.

Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/mmc/host/jz4740_mmc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index f4c4890..44a04fe 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -109,6 +109,7 @@ enum jz4740_mmc_version {
JZ_MMC_JZ4725B,
JZ_MMC_JZ4760,
JZ_MMC_JZ4780,
+ JZ_MMC_X1000,
};

enum jz4740_mmc_state {
@@ -938,6 +939,7 @@ static const struct of_device_id jz4740_mmc_of_match[] = {
{ .compatible = "ingenic,jz4725b-mmc", .data = (void *)JZ_MMC_JZ4725B },
{ .compatible = "ingenic,jz4760-mmc", .data = (void *) JZ_MMC_JZ4760 },
{ .compatible = "ingenic,jz4780-mmc", .data = (void *) JZ_MMC_JZ4780 },
+ { .compatible = "ingenic,x1000-mmc", .data = (void *) JZ_MMC_X1000 },
{},
};
MODULE_DEVICE_TABLE(of, jz4740_mmc_of_match);
@@ -1039,7 +1041,7 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
dev_err(&pdev->dev, "Failed to add mmc host: %d\n", ret);
goto err_release_dma;
}
- dev_info(&pdev->dev, "JZ SD/MMC card driver registered\n");
+ dev_info(&pdev->dev, "Ingenic SD/MMC card driver registered\n");

dev_info(&pdev->dev, "Using %s, %d-bit mode\n",
host->use_dma ? "DMA" : "PIO",
--
2.7.4


2019-10-14 23:07:02

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 4/6 v2] dt-bindings: MMC: Add X1000 bindings.

On Sat, 12 Oct 2019 13:13:18 +0800, Zhou Yanjie wrote:
> Add the MMC bindings for the X1000 Soc from Ingenic.
>
> Signed-off-by: Zhou Yanjie <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/jz4740.txt | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>

Acked-by: Rob Herring <[email protected]>

2019-10-19 08:03:29

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 6/6 v2] MMC: JZ4740: Add support for LPM.

On Sat, 12 Oct 2019 at 07:19, Zhou Yanjie <[email protected]> wrote:
>
> add support for low power mode of Ingenic's MMC/SD Controller.
>
> Signed-off-by: Zhou Yanjie <[email protected]>

I couldn't find a proper coverletter for the series, please provide
that next time as it really helps review. Additionally, it seems like
you forgot to change the prefix of the patches to "mmc: jz4740" (or at
least you chosed upper case letters), but I will take care of that
this time. So, I have applied the series for next, thanks!

I also have a general question. Should we perhaps rename the driver
from jz4740_mmc.c to ingenic.c (and the file for the DT bindings, the
Kconfig, etc), as that seems like a more appropriate name? No?

Kind regards
Uffe


> ---
> drivers/mmc/host/jz4740_mmc.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
> index 44a04fe..4cbe7fb 100644
> --- a/drivers/mmc/host/jz4740_mmc.c
> +++ b/drivers/mmc/host/jz4740_mmc.c
> @@ -43,6 +43,7 @@
> #define JZ_REG_MMC_RESP_FIFO 0x34
> #define JZ_REG_MMC_RXFIFO 0x38
> #define JZ_REG_MMC_TXFIFO 0x3C
> +#define JZ_REG_MMC_LPM 0x40
> #define JZ_REG_MMC_DMAC 0x44
>
> #define JZ_MMC_STRPCL_EXIT_MULTIPLE BIT(7)
> @@ -102,6 +103,12 @@
> #define JZ_MMC_DMAC_DMA_SEL BIT(1)
> #define JZ_MMC_DMAC_DMA_EN BIT(0)
>
> +#define JZ_MMC_LPM_DRV_RISING BIT(31)
> +#define JZ_MMC_LPM_DRV_RISING_QTR_PHASE_DLY BIT(31)
> +#define JZ_MMC_LPM_DRV_RISING_1NS_DLY BIT(30)
> +#define JZ_MMC_LPM_SMP_RISING_QTR_OR_HALF_PHASE_DLY BIT(29)
> +#define JZ_MMC_LPM_LOW_POWER_MODE_EN BIT(0)
> +
> #define JZ_MMC_CLK_RATE 24000000
>
> enum jz4740_mmc_version {
> @@ -860,6 +867,22 @@ static int jz4740_mmc_set_clock_rate(struct jz4740_mmc_host *host, int rate)
> }
>
> writew(div, host->base + JZ_REG_MMC_CLKRT);
> +
> + if (real_rate > 25000000) {
> + if (host->version >= JZ_MMC_X1000) {
> + writel(JZ_MMC_LPM_DRV_RISING_QTR_PHASE_DLY |
> + JZ_MMC_LPM_SMP_RISING_QTR_OR_HALF_PHASE_DLY |
> + JZ_MMC_LPM_LOW_POWER_MODE_EN,
> + host->base + JZ_REG_MMC_LPM);
> + } else if (host->version >= JZ_MMC_JZ4760) {
> + writel(JZ_MMC_LPM_DRV_RISING |
> + JZ_MMC_LPM_LOW_POWER_MODE_EN,
> + host->base + JZ_REG_MMC_LPM);
> + } else if (host->version >= JZ_MMC_JZ4725B)
> + writel(JZ_MMC_LPM_LOW_POWER_MODE_EN,
> + host->base + JZ_REG_MMC_LPM);
> + }
> +
> return real_rate;
> }
>
> --
> 2.7.4
>
>

2019-10-19 08:34:57

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH 6/6 v2] MMC: JZ4740: Add support for LPM.

Hi Uffe,


Le ven., oct. 18, 2019 at 10:52, Ulf Hansson <[email protected]> a
?crit :
> On Sat, 12 Oct 2019 at 07:19, Zhou Yanjie <[email protected]> wrote:
>>
>> add support for low power mode of Ingenic's MMC/SD Controller.
>>
>> Signed-off-by: Zhou Yanjie <[email protected]>
>
> I couldn't find a proper coverletter for the series, please provide
> that next time as it really helps review. Additionally, it seems like
> you forgot to change the prefix of the patches to "mmc: jz4740" (or at
> least you chosed upper case letters), but I will take care of that
> this time. So, I have applied the series for next, thanks!
>
> I also have a general question. Should we perhaps rename the driver
> from jz4740_mmc.c to ingenic.c (and the file for the DT bindings, the
> Kconfig, etc), as that seems like a more appropriate name? No?

Is there a kernel policy regarding renaming drivers? Since it trashes
the git history. Anyway you're the subsystem maintainer so I guess
that's up to you. I can send a patch to rename it if you want.

Cheers,
-Paul


>
> Kind regards
> Uffe
>
>
>> ---
>> drivers/mmc/host/jz4740_mmc.c | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/mmc/host/jz4740_mmc.c
>> b/drivers/mmc/host/jz4740_mmc.c
>> index 44a04fe..4cbe7fb 100644
>> --- a/drivers/mmc/host/jz4740_mmc.c
>> +++ b/drivers/mmc/host/jz4740_mmc.c
>> @@ -43,6 +43,7 @@
>> #define JZ_REG_MMC_RESP_FIFO 0x34
>> #define JZ_REG_MMC_RXFIFO 0x38
>> #define JZ_REG_MMC_TXFIFO 0x3C
>> +#define JZ_REG_MMC_LPM 0x40
>> #define JZ_REG_MMC_DMAC 0x44
>>
>> #define JZ_MMC_STRPCL_EXIT_MULTIPLE BIT(7)
>> @@ -102,6 +103,12 @@
>> #define JZ_MMC_DMAC_DMA_SEL BIT(1)
>> #define JZ_MMC_DMAC_DMA_EN BIT(0)
>>
>> +#define JZ_MMC_LPM_DRV_RISING BIT(31)
>> +#define JZ_MMC_LPM_DRV_RISING_QTR_PHASE_DLY BIT(31)
>> +#define JZ_MMC_LPM_DRV_RISING_1NS_DLY BIT(30)
>> +#define JZ_MMC_LPM_SMP_RISING_QTR_OR_HALF_PHASE_DLY BIT(29)
>> +#define JZ_MMC_LPM_LOW_POWER_MODE_EN BIT(0)
>> +
>> #define JZ_MMC_CLK_RATE 24000000
>>
>> enum jz4740_mmc_version {
>> @@ -860,6 +867,22 @@ static int jz4740_mmc_set_clock_rate(struct
>> jz4740_mmc_host *host, int rate)
>> }
>>
>> writew(div, host->base + JZ_REG_MMC_CLKRT);
>> +
>> + if (real_rate > 25000000) {
>> + if (host->version >= JZ_MMC_X1000) {
>> + writel(JZ_MMC_LPM_DRV_RISING_QTR_PHASE_DLY |
>> +
>> JZ_MMC_LPM_SMP_RISING_QTR_OR_HALF_PHASE_DLY |
>> + JZ_MMC_LPM_LOW_POWER_MODE_EN,
>> + host->base + JZ_REG_MMC_LPM);
>> + } else if (host->version >= JZ_MMC_JZ4760) {
>> + writel(JZ_MMC_LPM_DRV_RISING |
>> + JZ_MMC_LPM_LOW_POWER_MODE_EN,
>> + host->base + JZ_REG_MMC_LPM);
>> + } else if (host->version >= JZ_MMC_JZ4725B)
>> + writel(JZ_MMC_LPM_LOW_POWER_MODE_EN,
>> + host->base + JZ_REG_MMC_LPM);
>> + }
>> +
>> return real_rate;
>> }
>>
>> --
>> 2.7.4
>>
>>


2019-10-19 09:57:54

by Zhou Yanjie

[permalink] [raw]
Subject: Re: [PATCH 6/6 v2] MMC: JZ4740: Add support for LPM.

Hi Uffe,

On 2019年10月18日 16:52, Ulf Hansson wrote:
> On Sat, 12 Oct 2019 at 07:19, Zhou Yanjie <[email protected]> wrote:
>> add support for low power mode of Ingenic's MMC/SD Controller.
>>
>> Signed-off-by: Zhou Yanjie <[email protected]>
> I couldn't find a proper coverletter for the series, please provide
> that next time as it really helps review.

I'm sorry, maybe some problems with my git send-email cause cover
later not to be sent out, next time I will pay attention to this problem.

> Additionally, it seems like
> you forgot to change the prefix of the patches to "mmc: jz4740" (or at
> least you chosed upper case letters), but I will take care of that
> this time. So, I have applied the series for next, thanks!

I'm very sorry, I have misunderstood, before I thought jz4740 as a proper
noun needs to be capitalized, I will pay attention to this next time.

>
> I also have a general question. Should we perhaps rename the driver
> from jz4740_mmc.c to ingenic.c (and the file for the DT bindings, the
> Kconfig, etc), as that seems like a more appropriate name? No?

I am very much in favor of this proposal. Now jz4740_mmc.c is not only used
for the JZ4740 processor, it is also used for JZ4725, JZ4760, JZ4770, JZ4780
and X1000, and now Ingenic's processor is no longer named after JZ47xx,
it is divided into three product lines: M, T, and X. It is easy to cause
some
misunderstandings by using jz4740_mmc.c. At the same time, I think that
some register names also need to be adjusted. For example, the STLPPL
register name has only appeared in JZ4730 and JZ4740, and this register
in all subsequent processors is called CTRL. This time I was confused by
the STLPPL when I added drivers for the JZ4760's and X1000's LPM.

I also can send a patch to rename it if you need.

Best regards!

>
> Kind regards
> Uffe
>
>
>> ---
>> drivers/mmc/host/jz4740_mmc.c | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
>> index 44a04fe..4cbe7fb 100644
>> --- a/drivers/mmc/host/jz4740_mmc.c
>> +++ b/drivers/mmc/host/jz4740_mmc.c
>> @@ -43,6 +43,7 @@
>> #define JZ_REG_MMC_RESP_FIFO 0x34
>> #define JZ_REG_MMC_RXFIFO 0x38
>> #define JZ_REG_MMC_TXFIFO 0x3C
>> +#define JZ_REG_MMC_LPM 0x40
>> #define JZ_REG_MMC_DMAC 0x44
>>
>> #define JZ_MMC_STRPCL_EXIT_MULTIPLE BIT(7)
>> @@ -102,6 +103,12 @@
>> #define JZ_MMC_DMAC_DMA_SEL BIT(1)
>> #define JZ_MMC_DMAC_DMA_EN BIT(0)
>>
>> +#define JZ_MMC_LPM_DRV_RISING BIT(31)
>> +#define JZ_MMC_LPM_DRV_RISING_QTR_PHASE_DLY BIT(31)
>> +#define JZ_MMC_LPM_DRV_RISING_1NS_DLY BIT(30)
>> +#define JZ_MMC_LPM_SMP_RISING_QTR_OR_HALF_PHASE_DLY BIT(29)
>> +#define JZ_MMC_LPM_LOW_POWER_MODE_EN BIT(0)
>> +
>> #define JZ_MMC_CLK_RATE 24000000
>>
>> enum jz4740_mmc_version {
>> @@ -860,6 +867,22 @@ static int jz4740_mmc_set_clock_rate(struct jz4740_mmc_host *host, int rate)
>> }
>>
>> writew(div, host->base + JZ_REG_MMC_CLKRT);
>> +
>> + if (real_rate > 25000000) {
>> + if (host->version >= JZ_MMC_X1000) {
>> + writel(JZ_MMC_LPM_DRV_RISING_QTR_PHASE_DLY |
>> + JZ_MMC_LPM_SMP_RISING_QTR_OR_HALF_PHASE_DLY |
>> + JZ_MMC_LPM_LOW_POWER_MODE_EN,
>> + host->base + JZ_REG_MMC_LPM);
>> + } else if (host->version >= JZ_MMC_JZ4760) {
>> + writel(JZ_MMC_LPM_DRV_RISING |
>> + JZ_MMC_LPM_LOW_POWER_MODE_EN,
>> + host->base + JZ_REG_MMC_LPM);
>> + } else if (host->version >= JZ_MMC_JZ4725B)
>> + writel(JZ_MMC_LPM_LOW_POWER_MODE_EN,
>> + host->base + JZ_REG_MMC_LPM);
>> + }
>> +
>> return real_rate;
>> }
>>
>> --
>> 2.7.4
>>
>>



2019-10-19 20:45:32

by Ezequiel Garcia

[permalink] [raw]
Subject: Re: [PATCH 6/6 v2] MMC: JZ4740: Add support for LPM.

On Friday, October 18, 2019 13:54 -03, Zhou Yanjie <[email protected]> wrote:


>
> >
> > I also have a general question. Should we perhaps rename the driver
> > from jz4740_mmc.c to ingenic.c (and the file for the DT bindings, the
> > Kconfig, etc), as that seems like a more appropriate name? No?
>
> I am very much in favor of this proposal. Now jz4740_mmc.c is not only used
> for the JZ4740 processor, it is also used for JZ4725, JZ4760, JZ4770, JZ4780
> and X1000, and now Ingenic's processor is no longer named after JZ47xx,
> it is divided into three product lines: M, T, and X. It is easy to cause
> some
> misunderstandings by using jz4740_mmc.c. At the same time, I think that
> some register names also need to be adjusted. For example, the STLPPL
> register name has only appeared in JZ4730 and JZ4740, and this register
> in all subsequent processors is called CTRL. This time I was confused by
> the STLPPL when I added drivers for the JZ4760's and X1000's LPM.
>

I am very much against renamings, for several reasons. As Paul already mentioned, it's pointless and just adds noise to the git-log, making history harder to recover. Driver file names don't really have to reflect the device exactly. For the compatibility list, it's far easier to just git-grep for compatible strings, or git-grep Documentation and/or Kconfig.

Renaming macros and register names, is equally pointless and equally git-history invasive. Simply adding some documentation is enough.

Thanks,
Ezequiel

2019-10-21 14:58:41

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 6/6 v2] MMC: JZ4740: Add support for LPM.

On Sat, 19 Oct 2019 at 22:44, Ezequiel Garcia
<[email protected]> wrote:
>
> On Friday, October 18, 2019 13:54 -03, Zhou Yanjie <[email protected]> wrote:
>
>
> >
> > >
> > > I also have a general question. Should we perhaps rename the driver
> > > from jz4740_mmc.c to ingenic.c (and the file for the DT bindings, the
> > > Kconfig, etc), as that seems like a more appropriate name? No?
> >
> > I am very much in favor of this proposal. Now jz4740_mmc.c is not only used
> > for the JZ4740 processor, it is also used for JZ4725, JZ4760, JZ4770, JZ4780
> > and X1000, and now Ingenic's processor is no longer named after JZ47xx,
> > it is divided into three product lines: M, T, and X. It is easy to cause
> > some
> > misunderstandings by using jz4740_mmc.c. At the same time, I think that
> > some register names also need to be adjusted. For example, the STLPPL
> > register name has only appeared in JZ4730 and JZ4740, and this register
> > in all subsequent processors is called CTRL. This time I was confused by
> > the STLPPL when I added drivers for the JZ4760's and X1000's LPM.
> >
>
> I am very much against renamings, for several reasons. As Paul already mentioned, it's pointless and just adds noise to the git-log, making history harder to recover. Driver file names don't really have to reflect the device > > exactly. For the compatibility list, it's far easier to just git-grep for compatible strings, or git-grep Documentation and/or Kconfig.

I have no strong opinions. What matters to me, is that people agree on
the best option, based on a case by case discussion.

>
> Renaming macros and register names, is equally pointless and equally git-history invasive. Simply adding some documentation is enough.

Sounds like documentation is what people prefer here - and the DT doc
seems already fine in regards to that.

Perhaps some more words added to the header in driver's c-file could
be and option to consider, as today it only mentions "JZ4740 SD/MMC
controller driver".

Anyway, it's up to you. :-)

Kind regards
Uffe