* [PATCH 0/2] Add support for JZ4775 and rename unreasonable array name.
@ 2021-06-06 18:08 周琰杰 (Zhou Yanjie)
2021-06-06 18:08 ` [PATCH 1/2] dt-bindings: mmc: JZ4740: Add bindings for JZ4775 周琰杰 (Zhou Yanjie)
2021-06-06 18:08 ` [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name 周琰杰 (Zhou Yanjie)
0 siblings, 2 replies; 8+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2021-06-06 18:08 UTC (permalink / raw)
To: ulf.hansson, robh+dt
Cc: linux-mips, linux-kernel, linux-mmc, devicetree, dongsheng.qiu,
aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou, paul
1.Add the MMC bindings for the JZ4775 SoC from Ingenic.
2.Add support for probing mmc driver on the JZ4775 SoC from Ingenic.
3.When the support for JZ4775 SoC is added, there will be six compatible
strings, so renaming "jz4740_mmc_of_match[]" to "jz4740_mmc_of_matches[]"
is more reasonable.
周琰杰 (Zhou Yanjie) (2):
dt-bindings: mmc: JZ4740: Add bindings for JZ4775.
mmc: JZ4740: Add support for JZ4775 and rename unreasonable array
name.
.../devicetree/bindings/mmc/ingenic,mmc.yaml | 1 +
drivers/mmc/host/jz4740_mmc.c | 25 ++++++++++++----------
2 files changed, 15 insertions(+), 11 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] dt-bindings: mmc: JZ4740: Add bindings for JZ4775. 2021-06-06 18:08 [PATCH 0/2] Add support for JZ4775 and rename unreasonable array name 周琰杰 (Zhou Yanjie) @ 2021-06-06 18:08 ` 周琰杰 (Zhou Yanjie) 2021-06-08 13:16 ` Ulf Hansson 2021-06-06 18:08 ` [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name 周琰杰 (Zhou Yanjie) 1 sibling, 1 reply; 8+ messages in thread From: 周琰杰 (Zhou Yanjie) @ 2021-06-06 18:08 UTC (permalink / raw) To: ulf.hansson, robh+dt Cc: linux-mips, linux-kernel, linux-mmc, devicetree, dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou, paul Add the MMC bindings for the JZ4775 SoC from Ingenic. Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> --- Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml b/Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml index 04ba8b7..546480f 100644 --- a/Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml +++ b/Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml @@ -19,6 +19,7 @@ properties: - ingenic,jz4740-mmc - ingenic,jz4725b-mmc - ingenic,jz4760-mmc + - ingenic,jz4775-mmc - ingenic,jz4780-mmc - ingenic,x1000-mmc - items: -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mmc: JZ4740: Add bindings for JZ4775. 2021-06-06 18:08 ` [PATCH 1/2] dt-bindings: mmc: JZ4740: Add bindings for JZ4775 周琰杰 (Zhou Yanjie) @ 2021-06-08 13:16 ` Ulf Hansson 0 siblings, 0 replies; 8+ messages in thread From: Ulf Hansson @ 2021-06-08 13:16 UTC (permalink / raw) To: 周琰杰 (Zhou Yanjie) Cc: Rob Herring, linux-mips, Linux Kernel Mailing List, linux-mmc, DTML, dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou, Paul Cercueil On Sun, 6 Jun 2021 at 20:08, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> wrote: > > Add the MMC bindings for the JZ4775 SoC from Ingenic. > > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> Applied for next, thanks! Kind regards Uffe > --- > Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml b/Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml > index 04ba8b7..546480f 100644 > --- a/Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml > +++ b/Documentation/devicetree/bindings/mmc/ingenic,mmc.yaml > @@ -19,6 +19,7 @@ properties: > - ingenic,jz4740-mmc > - ingenic,jz4725b-mmc > - ingenic,jz4760-mmc > + - ingenic,jz4775-mmc > - ingenic,jz4780-mmc > - ingenic,x1000-mmc > - items: > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name. 2021-06-06 18:08 [PATCH 0/2] Add support for JZ4775 and rename unreasonable array name 周琰杰 (Zhou Yanjie) 2021-06-06 18:08 ` [PATCH 1/2] dt-bindings: mmc: JZ4740: Add bindings for JZ4775 周琰杰 (Zhou Yanjie) @ 2021-06-06 18:08 ` 周琰杰 (Zhou Yanjie) 2021-06-08 13:16 ` Ulf Hansson 2021-06-08 13:44 ` Paul Cercueil 1 sibling, 2 replies; 8+ messages in thread From: 周琰杰 (Zhou Yanjie) @ 2021-06-06 18:08 UTC (permalink / raw) To: ulf.hansson, robh+dt Cc: linux-mips, linux-kernel, linux-mmc, devicetree, dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou, paul 1.Add support for probing mmc driver on the JZ4775 SoC from Ingenic. 2.When the support for JZ4775 SoC is added, there will be six compatible strings, so renaming "jz4740_mmc_of_match[]" to "jz4740_mmc_of_matches[]" is more reasonable. Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> --- drivers/mmc/host/jz4740_mmc.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c index b3c636e..ea8434f 100644 --- a/drivers/mmc/host/jz4740_mmc.c +++ b/drivers/mmc/host/jz4740_mmc.c @@ -2,6 +2,7 @@ /* * Copyright (C) 2009-2010, Lars-Peter Clausen <lars@metafoo.de> * Copyright (C) 2013, Imagination Technologies + * Copyright (C) 2021, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> * * JZ4740 SD/MMC controller driver */ @@ -114,6 +115,7 @@ enum jz4740_mmc_version { JZ_MMC_JZ4740, JZ_MMC_JZ4725B, JZ_MMC_JZ4760, + JZ_MMC_JZ4775, JZ_MMC_JZ4780, JZ_MMC_X1000, }; @@ -138,7 +140,7 @@ enum jz4740_mmc_state { * COOKIE_MAPPED: the request was mapped in the irq handler, * and should be unmapped before mmc_request_done is called.. */ -enum jz4780_cookie { +enum jz4775_cookie { COOKIE_UNMAPPED = 0, COOKIE_PREMAPPED, COOKIE_MAPPED, @@ -194,7 +196,7 @@ static void jz4740_mmc_write_irq_mask(struct jz4740_mmc_host *host, static void jz4740_mmc_write_irq_reg(struct jz4740_mmc_host *host, uint32_t val) { - if (host->version >= JZ_MMC_JZ4780) + if (host->version >= JZ_MMC_JZ4775) writel(val, host->base + JZ_REG_MMC_IREG); else writew(val, host->base + JZ_REG_MMC_IREG); @@ -202,7 +204,7 @@ static void jz4740_mmc_write_irq_reg(struct jz4740_mmc_host *host, static uint32_t jz4740_mmc_read_irq_reg(struct jz4740_mmc_host *host) { - if (host->version >= JZ_MMC_JZ4780) + if (host->version >= JZ_MMC_JZ4775) return readl(host->base + JZ_REG_MMC_IREG); else return readw(host->base + JZ_REG_MMC_IREG); @@ -674,7 +676,7 @@ static void jz4740_mmc_send_command(struct jz4740_mmc_host *host, cmdat |= JZ_MMC_CMDAT_WRITE; if (host->use_dma) { /* - * The 4780's MMC controller has integrated DMA ability + * The JZ4775's MMC controller has integrated DMA ability * in addition to being able to use the external DMA * controller. It moves DMA control bits to a separate * register. The DMA_SEL bit chooses the external @@ -682,13 +684,13 @@ static void jz4740_mmc_send_command(struct jz4740_mmc_host *host, * can only use the external controller, and have a * single DMA enable bit in CMDAT. */ - if (host->version >= JZ_MMC_JZ4780) { + if (host->version >= JZ_MMC_JZ4775) { writel(JZ_MMC_DMAC_DMA_EN | JZ_MMC_DMAC_DMA_SEL, host->base + JZ_REG_MMC_DMAC); } else { cmdat |= JZ_MMC_CMDAT_DMA_EN; } - } else if (host->version >= JZ_MMC_JZ4780) { + } else if (host->version >= JZ_MMC_JZ4775) { writel(0, host->base + JZ_REG_MMC_DMAC); } @@ -866,7 +868,7 @@ 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) { + if (host->version >= JZ_MMC_JZ4775) { 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, @@ -955,15 +957,16 @@ static const struct mmc_host_ops jz4740_mmc_ops = { .enable_sdio_irq = jz4740_mmc_enable_sdio_irq, }; -static const struct of_device_id jz4740_mmc_of_match[] = { +static const struct of_device_id jz4740_mmc_of_matches[] = { { .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,jz4775-mmc", .data = (void *) JZ_MMC_JZ4775 }, { .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); +MODULE_DEVICE_TABLE(of, jz4740_mmc_of_matches); static int jz4740_mmc_probe(struct platform_device* pdev) { @@ -980,7 +983,7 @@ static int jz4740_mmc_probe(struct platform_device* pdev) host = mmc_priv(mmc); - match = of_match_device(jz4740_mmc_of_match, &pdev->dev); + match = of_match_device(jz4740_mmc_of_matches, &pdev->dev); if (match) { host->version = (enum jz4740_mmc_version)match->data; } else { @@ -1124,7 +1127,7 @@ static struct platform_driver jz4740_mmc_driver = { .driver = { .name = "jz4740-mmc", .probe_type = PROBE_PREFER_ASYNCHRONOUS, - .of_match_table = of_match_ptr(jz4740_mmc_of_match), + .of_match_table = of_match_ptr(jz4740_mmc_of_matches), .pm = pm_ptr(&jz4740_mmc_pm_ops), }, }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name. 2021-06-06 18:08 ` [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name 周琰杰 (Zhou Yanjie) @ 2021-06-08 13:16 ` Ulf Hansson 2021-06-08 13:44 ` Paul Cercueil 1 sibling, 0 replies; 8+ messages in thread From: Ulf Hansson @ 2021-06-08 13:16 UTC (permalink / raw) To: 周琰杰 (Zhou Yanjie) Cc: Rob Herring, linux-mips, Linux Kernel Mailing List, linux-mmc, DTML, dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou, Paul Cercueil On Sun, 6 Jun 2021 at 20:08, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> wrote: > > 1.Add support for probing mmc driver on the JZ4775 SoC from Ingenic. > 2.When the support for JZ4775 SoC is added, there will be six compatible > strings, so renaming "jz4740_mmc_of_match[]" to "jz4740_mmc_of_matches[]" > is more reasonable. > > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> Applied for next, with a minor change, see below, thanks! Kind regards Uffe > --- > drivers/mmc/host/jz4740_mmc.c | 25 ++++++++++++++----------- > 1 file changed, 14 insertions(+), 11 deletions(-) > > diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c > index b3c636e..ea8434f 100644 > --- a/drivers/mmc/host/jz4740_mmc.c > +++ b/drivers/mmc/host/jz4740_mmc.c > @@ -2,6 +2,7 @@ > /* > * Copyright (C) 2009-2010, Lars-Peter Clausen <lars@metafoo.de> > * Copyright (C) 2013, Imagination Technologies > + * Copyright (C) 2021, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> I am dropping this as it's pointless, because of your "Signed-off-by" tag. If you want more explanation, please have a look at Documentation/process/submitting-patches.rst. It should explain more about "Signed-off-by" tags. > * > * JZ4740 SD/MMC controller driver > */ > @@ -114,6 +115,7 @@ enum jz4740_mmc_version { > JZ_MMC_JZ4740, > JZ_MMC_JZ4725B, > JZ_MMC_JZ4760, > + JZ_MMC_JZ4775, > JZ_MMC_JZ4780, > JZ_MMC_X1000, > }; > @@ -138,7 +140,7 @@ enum jz4740_mmc_state { > * COOKIE_MAPPED: the request was mapped in the irq handler, > * and should be unmapped before mmc_request_done is called.. > */ > -enum jz4780_cookie { > +enum jz4775_cookie { > COOKIE_UNMAPPED = 0, > COOKIE_PREMAPPED, > COOKIE_MAPPED, > @@ -194,7 +196,7 @@ static void jz4740_mmc_write_irq_mask(struct jz4740_mmc_host *host, > static void jz4740_mmc_write_irq_reg(struct jz4740_mmc_host *host, > uint32_t val) > { > - if (host->version >= JZ_MMC_JZ4780) > + if (host->version >= JZ_MMC_JZ4775) > writel(val, host->base + JZ_REG_MMC_IREG); > else > writew(val, host->base + JZ_REG_MMC_IREG); > @@ -202,7 +204,7 @@ static void jz4740_mmc_write_irq_reg(struct jz4740_mmc_host *host, > > static uint32_t jz4740_mmc_read_irq_reg(struct jz4740_mmc_host *host) > { > - if (host->version >= JZ_MMC_JZ4780) > + if (host->version >= JZ_MMC_JZ4775) > return readl(host->base + JZ_REG_MMC_IREG); > else > return readw(host->base + JZ_REG_MMC_IREG); > @@ -674,7 +676,7 @@ static void jz4740_mmc_send_command(struct jz4740_mmc_host *host, > cmdat |= JZ_MMC_CMDAT_WRITE; > if (host->use_dma) { > /* > - * The 4780's MMC controller has integrated DMA ability > + * The JZ4775's MMC controller has integrated DMA ability > * in addition to being able to use the external DMA > * controller. It moves DMA control bits to a separate > * register. The DMA_SEL bit chooses the external > @@ -682,13 +684,13 @@ static void jz4740_mmc_send_command(struct jz4740_mmc_host *host, > * can only use the external controller, and have a > * single DMA enable bit in CMDAT. > */ > - if (host->version >= JZ_MMC_JZ4780) { > + if (host->version >= JZ_MMC_JZ4775) { > writel(JZ_MMC_DMAC_DMA_EN | JZ_MMC_DMAC_DMA_SEL, > host->base + JZ_REG_MMC_DMAC); > } else { > cmdat |= JZ_MMC_CMDAT_DMA_EN; > } > - } else if (host->version >= JZ_MMC_JZ4780) { > + } else if (host->version >= JZ_MMC_JZ4775) { > writel(0, host->base + JZ_REG_MMC_DMAC); > } > > @@ -866,7 +868,7 @@ 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) { > + if (host->version >= JZ_MMC_JZ4775) { > 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, > @@ -955,15 +957,16 @@ static const struct mmc_host_ops jz4740_mmc_ops = { > .enable_sdio_irq = jz4740_mmc_enable_sdio_irq, > }; > > -static const struct of_device_id jz4740_mmc_of_match[] = { > +static const struct of_device_id jz4740_mmc_of_matches[] = { > { .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,jz4775-mmc", .data = (void *) JZ_MMC_JZ4775 }, > { .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); > +MODULE_DEVICE_TABLE(of, jz4740_mmc_of_matches); > > static int jz4740_mmc_probe(struct platform_device* pdev) > { > @@ -980,7 +983,7 @@ static int jz4740_mmc_probe(struct platform_device* pdev) > > host = mmc_priv(mmc); > > - match = of_match_device(jz4740_mmc_of_match, &pdev->dev); > + match = of_match_device(jz4740_mmc_of_matches, &pdev->dev); > if (match) { > host->version = (enum jz4740_mmc_version)match->data; > } else { > @@ -1124,7 +1127,7 @@ static struct platform_driver jz4740_mmc_driver = { > .driver = { > .name = "jz4740-mmc", > .probe_type = PROBE_PREFER_ASYNCHRONOUS, > - .of_match_table = of_match_ptr(jz4740_mmc_of_match), > + .of_match_table = of_match_ptr(jz4740_mmc_of_matches), > .pm = pm_ptr(&jz4740_mmc_pm_ops), > }, > }; > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name. 2021-06-06 18:08 ` [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name 周琰杰 (Zhou Yanjie) 2021-06-08 13:16 ` Ulf Hansson @ 2021-06-08 13:44 ` Paul Cercueil 2021-06-08 14:11 ` Ulf Hansson 2021-06-08 14:50 ` Zhou Yanjie 1 sibling, 2 replies; 8+ messages in thread From: Paul Cercueil @ 2021-06-08 13:44 UTC (permalink / raw) To: 周琰杰 Cc: ulf.hansson, robh+dt, linux-mips, linux-kernel, linux-mmc, devicetree, dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou, paul Hi Zhou, Le lun., juin 7 2021 at 02:08:04 +0800, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> a écrit : > 1.Add support for probing mmc driver on the JZ4775 SoC from Ingenic. > 2.When the support for JZ4775 SoC is added, there will be six > compatible > strings, so renaming "jz4740_mmc_of_match[]" to > "jz4740_mmc_of_matches[]" > is more reasonable. Honestly, you can drop #2. We don't really care about the variable/function names not being "perfect". For instance this driver still use "jz4740_mmc*" functions everywhere even though it supports many more SoCs. Besides, renames like that makes it harder to "git blame" afterwards since it fills the git history with non-functional changes. > > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> > --- > drivers/mmc/host/jz4740_mmc.c | 25 ++++++++++++++----------- > 1 file changed, 14 insertions(+), 11 deletions(-) > > diff --git a/drivers/mmc/host/jz4740_mmc.c > b/drivers/mmc/host/jz4740_mmc.c > index b3c636e..ea8434f 100644 > --- a/drivers/mmc/host/jz4740_mmc.c > +++ b/drivers/mmc/host/jz4740_mmc.c > @@ -2,6 +2,7 @@ > /* > * Copyright (C) 2009-2010, Lars-Peter Clausen <lars@metafoo.de> > * Copyright (C) 2013, Imagination Technologies > + * Copyright (C) 2021, 周琰杰 (Zhou Yanjie) > <zhouyanjie@wanyeetech.com> > * > * JZ4740 SD/MMC controller driver > */ > @@ -114,6 +115,7 @@ enum jz4740_mmc_version { > JZ_MMC_JZ4740, > JZ_MMC_JZ4725B, > JZ_MMC_JZ4760, > + JZ_MMC_JZ4775, > JZ_MMC_JZ4780, > JZ_MMC_X1000, > }; > @@ -138,7 +140,7 @@ enum jz4740_mmc_state { > * COOKIE_MAPPED: the request was mapped in the irq handler, > * and should be unmapped before mmc_request_done is called.. > */ > -enum jz4780_cookie { > +enum jz4775_cookie { > COOKIE_UNMAPPED = 0, > COOKIE_PREMAPPED, > COOKIE_MAPPED, > @@ -194,7 +196,7 @@ static void jz4740_mmc_write_irq_mask(struct > jz4740_mmc_host *host, > static void jz4740_mmc_write_irq_reg(struct jz4740_mmc_host *host, > uint32_t val) > { > - if (host->version >= JZ_MMC_JZ4780) > + if (host->version >= JZ_MMC_JZ4775) > writel(val, host->base + JZ_REG_MMC_IREG); > else > writew(val, host->base + JZ_REG_MMC_IREG); > @@ -202,7 +204,7 @@ static void jz4740_mmc_write_irq_reg(struct > jz4740_mmc_host *host, > > static uint32_t jz4740_mmc_read_irq_reg(struct jz4740_mmc_host *host) > { > - if (host->version >= JZ_MMC_JZ4780) > + if (host->version >= JZ_MMC_JZ4775) > return readl(host->base + JZ_REG_MMC_IREG); > else > return readw(host->base + JZ_REG_MMC_IREG); > @@ -674,7 +676,7 @@ static void jz4740_mmc_send_command(struct > jz4740_mmc_host *host, > cmdat |= JZ_MMC_CMDAT_WRITE; > if (host->use_dma) { > /* > - * The 4780's MMC controller has integrated DMA ability > + * The JZ4775's MMC controller has integrated DMA ability > * in addition to being able to use the external DMA > * controller. It moves DMA control bits to a separate > * register. The DMA_SEL bit chooses the external > @@ -682,13 +684,13 @@ static void jz4740_mmc_send_command(struct > jz4740_mmc_host *host, > * can only use the external controller, and have a > * single DMA enable bit in CMDAT. > */ > - if (host->version >= JZ_MMC_JZ4780) { > + if (host->version >= JZ_MMC_JZ4775) { > writel(JZ_MMC_DMAC_DMA_EN | JZ_MMC_DMAC_DMA_SEL, > host->base + JZ_REG_MMC_DMAC); > } else { > cmdat |= JZ_MMC_CMDAT_DMA_EN; > } > - } else if (host->version >= JZ_MMC_JZ4780) { > + } else if (host->version >= JZ_MMC_JZ4775) { > writel(0, host->base + JZ_REG_MMC_DMAC); > } > > @@ -866,7 +868,7 @@ 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) { > + if (host->version >= JZ_MMC_JZ4775) { This changes the behaviour for the JZ4780. Even if it is correct, this belongs in its own commit (with a Fixes tag), or at the very least a mention about it in the commit message. > 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, > @@ -955,15 +957,16 @@ static const struct mmc_host_ops jz4740_mmc_ops > = { > .enable_sdio_irq = jz4740_mmc_enable_sdio_irq, > }; > > -static const struct of_device_id jz4740_mmc_of_match[] = { > +static const struct of_device_id jz4740_mmc_of_matches[] = { > { .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,jz4775-mmc", .data = (void *) > JZ_MMC_JZ4775 }, > { .compatible = "ingenic,jz4780-mmc", .data = (void *) > JZ_MMC_JZ4780 }, Looks to me that the JZ4775 and JZ4780 have the exact same behaviour, so this patch could have been a one-liner, adding "ingenic,jz4775-mmc" with the JZ_MMC_JZ4780 ID. Cheers, -Paul > { .compatible = "ingenic,x1000-mmc", .data = (void *) JZ_MMC_X1000 > }, > {}, > }; > -MODULE_DEVICE_TABLE(of, jz4740_mmc_of_match); > +MODULE_DEVICE_TABLE(of, jz4740_mmc_of_matches); > > static int jz4740_mmc_probe(struct platform_device* pdev) > { > @@ -980,7 +983,7 @@ static int jz4740_mmc_probe(struct > platform_device* pdev) > > host = mmc_priv(mmc); > > - match = of_match_device(jz4740_mmc_of_match, &pdev->dev); > + match = of_match_device(jz4740_mmc_of_matches, &pdev->dev); > if (match) { > host->version = (enum jz4740_mmc_version)match->data; > } else { > @@ -1124,7 +1127,7 @@ static struct platform_driver jz4740_mmc_driver > = { > .driver = { > .name = "jz4740-mmc", > .probe_type = PROBE_PREFER_ASYNCHRONOUS, > - .of_match_table = of_match_ptr(jz4740_mmc_of_match), > + .of_match_table = of_match_ptr(jz4740_mmc_of_matches), > .pm = pm_ptr(&jz4740_mmc_pm_ops), > }, > }; > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name. 2021-06-08 13:44 ` Paul Cercueil @ 2021-06-08 14:11 ` Ulf Hansson 2021-06-08 14:50 ` Zhou Yanjie 1 sibling, 0 replies; 8+ messages in thread From: Ulf Hansson @ 2021-06-08 14:11 UTC (permalink / raw) To: Paul Cercueil Cc: 周琰杰, Rob Herring, linux-mips, Linux Kernel Mailing List, linux-mmc, DTML, dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou, Paul Cercueil On Tue, 8 Jun 2021 at 15:44, Paul Cercueil <paul@opendingux.net> wrote: > > Hi Zhou, > > Le lun., juin 7 2021 at 02:08:04 +0800, 周琰杰 (Zhou Yanjie) > <zhouyanjie@wanyeetech.com> a écrit : > > 1.Add support for probing mmc driver on the JZ4775 SoC from Ingenic. > > 2.When the support for JZ4775 SoC is added, there will be six > > compatible > > strings, so renaming "jz4740_mmc_of_match[]" to > > "jz4740_mmc_of_matches[]" > > is more reasonable. > > Honestly, you can drop #2. We don't really care about the > variable/function names not being "perfect". For instance this driver > still use "jz4740_mmc*" functions everywhere even though it supports > many more SoCs. Besides, renames like that makes it harder to "git > blame" afterwards since it fills the git history with non-functional > changes. Paul, thanks for reviewing. I am dropping the $subject patch from my next branch until you have acked it. Kind regards Uffe > > > > > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> > > --- > > drivers/mmc/host/jz4740_mmc.c | 25 ++++++++++++++----------- > > 1 file changed, 14 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/mmc/host/jz4740_mmc.c > > b/drivers/mmc/host/jz4740_mmc.c > > index b3c636e..ea8434f 100644 > > --- a/drivers/mmc/host/jz4740_mmc.c > > +++ b/drivers/mmc/host/jz4740_mmc.c > > @@ -2,6 +2,7 @@ > > /* > > * Copyright (C) 2009-2010, Lars-Peter Clausen <lars@metafoo.de> > > * Copyright (C) 2013, Imagination Technologies > > + * Copyright (C) 2021, 周琰杰 (Zhou Yanjie) > > <zhouyanjie@wanyeetech.com> > > * > > * JZ4740 SD/MMC controller driver > > */ > > @@ -114,6 +115,7 @@ enum jz4740_mmc_version { > > JZ_MMC_JZ4740, > > JZ_MMC_JZ4725B, > > JZ_MMC_JZ4760, > > + JZ_MMC_JZ4775, > > JZ_MMC_JZ4780, > > JZ_MMC_X1000, > > }; > > @@ -138,7 +140,7 @@ enum jz4740_mmc_state { > > * COOKIE_MAPPED: the request was mapped in the irq handler, > > * and should be unmapped before mmc_request_done is called.. > > */ > > -enum jz4780_cookie { > > +enum jz4775_cookie { > > COOKIE_UNMAPPED = 0, > > COOKIE_PREMAPPED, > > COOKIE_MAPPED, > > @@ -194,7 +196,7 @@ static void jz4740_mmc_write_irq_mask(struct > > jz4740_mmc_host *host, > > static void jz4740_mmc_write_irq_reg(struct jz4740_mmc_host *host, > > uint32_t val) > > { > > - if (host->version >= JZ_MMC_JZ4780) > > + if (host->version >= JZ_MMC_JZ4775) > > writel(val, host->base + JZ_REG_MMC_IREG); > > else > > writew(val, host->base + JZ_REG_MMC_IREG); > > @@ -202,7 +204,7 @@ static void jz4740_mmc_write_irq_reg(struct > > jz4740_mmc_host *host, > > > > static uint32_t jz4740_mmc_read_irq_reg(struct jz4740_mmc_host *host) > > { > > - if (host->version >= JZ_MMC_JZ4780) > > + if (host->version >= JZ_MMC_JZ4775) > > return readl(host->base + JZ_REG_MMC_IREG); > > else > > return readw(host->base + JZ_REG_MMC_IREG); > > @@ -674,7 +676,7 @@ static void jz4740_mmc_send_command(struct > > jz4740_mmc_host *host, > > cmdat |= JZ_MMC_CMDAT_WRITE; > > if (host->use_dma) { > > /* > > - * The 4780's MMC controller has integrated DMA ability > > + * The JZ4775's MMC controller has integrated DMA ability > > * in addition to being able to use the external DMA > > * controller. It moves DMA control bits to a separate > > * register. The DMA_SEL bit chooses the external > > @@ -682,13 +684,13 @@ static void jz4740_mmc_send_command(struct > > jz4740_mmc_host *host, > > * can only use the external controller, and have a > > * single DMA enable bit in CMDAT. > > */ > > - if (host->version >= JZ_MMC_JZ4780) { > > + if (host->version >= JZ_MMC_JZ4775) { > > writel(JZ_MMC_DMAC_DMA_EN | JZ_MMC_DMAC_DMA_SEL, > > host->base + JZ_REG_MMC_DMAC); > > } else { > > cmdat |= JZ_MMC_CMDAT_DMA_EN; > > } > > - } else if (host->version >= JZ_MMC_JZ4780) { > > + } else if (host->version >= JZ_MMC_JZ4775) { > > writel(0, host->base + JZ_REG_MMC_DMAC); > > } > > > > @@ -866,7 +868,7 @@ 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) { > > + if (host->version >= JZ_MMC_JZ4775) { > > This changes the behaviour for the JZ4780. > > Even if it is correct, this belongs in its own commit (with a Fixes > tag), or at the very least a mention about it in the commit message. > > > 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, > > @@ -955,15 +957,16 @@ static const struct mmc_host_ops jz4740_mmc_ops > > = { > > .enable_sdio_irq = jz4740_mmc_enable_sdio_irq, > > }; > > > > -static const struct of_device_id jz4740_mmc_of_match[] = { > > +static const struct of_device_id jz4740_mmc_of_matches[] = { > > { .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,jz4775-mmc", .data = (void *) > > JZ_MMC_JZ4775 }, > > { .compatible = "ingenic,jz4780-mmc", .data = (void *) > > JZ_MMC_JZ4780 }, > > Looks to me that the JZ4775 and JZ4780 have the exact same behaviour, > so this patch could have been a one-liner, adding "ingenic,jz4775-mmc" > with the JZ_MMC_JZ4780 ID. > > Cheers, > -Paul > > > { .compatible = "ingenic,x1000-mmc", .data = (void *) JZ_MMC_X1000 > > }, > > {}, > > }; > > -MODULE_DEVICE_TABLE(of, jz4740_mmc_of_match); > > +MODULE_DEVICE_TABLE(of, jz4740_mmc_of_matches); > > > > static int jz4740_mmc_probe(struct platform_device* pdev) > > { > > @@ -980,7 +983,7 @@ static int jz4740_mmc_probe(struct > > platform_device* pdev) > > > > host = mmc_priv(mmc); > > > > - match = of_match_device(jz4740_mmc_of_match, &pdev->dev); > > + match = of_match_device(jz4740_mmc_of_matches, &pdev->dev); > > if (match) { > > host->version = (enum jz4740_mmc_version)match->data; > > } else { > > @@ -1124,7 +1127,7 @@ static struct platform_driver jz4740_mmc_driver > > = { > > .driver = { > > .name = "jz4740-mmc", > > .probe_type = PROBE_PREFER_ASYNCHRONOUS, > > - .of_match_table = of_match_ptr(jz4740_mmc_of_match), > > + .of_match_table = of_match_ptr(jz4740_mmc_of_matches), > > .pm = pm_ptr(&jz4740_mmc_pm_ops), > > }, > > }; > > -- > > 2.7.4 > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name. 2021-06-08 13:44 ` Paul Cercueil 2021-06-08 14:11 ` Ulf Hansson @ 2021-06-08 14:50 ` Zhou Yanjie 1 sibling, 0 replies; 8+ messages in thread From: Zhou Yanjie @ 2021-06-08 14:50 UTC (permalink / raw) To: Paul Cercueil Cc: ulf.hansson, robh+dt, linux-mips, linux-kernel, linux-mmc, devicetree, dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou, paul Hi Paul, On 2021/6/8 下午9:44, Paul Cercueil wrote: > Hi Zhou, > > Le lun., juin 7 2021 at 02:08:04 +0800, 周琰杰 (Zhou Yanjie) > <zhouyanjie@wanyeetech.com> a écrit : >> 1.Add support for probing mmc driver on the JZ4775 SoC from Ingenic. >> 2.When the support for JZ4775 SoC is added, there will be six compatible >> strings, so renaming "jz4740_mmc_of_match[]" to >> "jz4740_mmc_of_matches[]" >> is more reasonable. > > Honestly, you can drop #2. We don't really care about the > variable/function names not being "perfect". For instance this driver > still use "jz4740_mmc*" functions everywhere even though it supports > many more SoCs. Besides, renames like that makes it harder to "git > blame" afterwards since it fills the git history with non-functional > changes. Sure, I will drop #2. > >> >> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >> --- >> drivers/mmc/host/jz4740_mmc.c | 25 ++++++++++++++----------- >> 1 file changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/mmc/host/jz4740_mmc.c >> b/drivers/mmc/host/jz4740_mmc.c >> index b3c636e..ea8434f 100644 >> --- a/drivers/mmc/host/jz4740_mmc.c >> +++ b/drivers/mmc/host/jz4740_mmc.c >> @@ -2,6 +2,7 @@ >> /* >> * Copyright (C) 2009-2010, Lars-Peter Clausen <lars@metafoo.de> >> * Copyright (C) 2013, Imagination Technologies >> + * Copyright (C) 2021, 周琰杰 (Zhou Yanjie) >> <zhouyanjie@wanyeetech.com> >> * >> * JZ4740 SD/MMC controller driver >> */ >> @@ -114,6 +115,7 @@ enum jz4740_mmc_version { >> JZ_MMC_JZ4740, >> JZ_MMC_JZ4725B, >> JZ_MMC_JZ4760, >> + JZ_MMC_JZ4775, >> JZ_MMC_JZ4780, >> JZ_MMC_X1000, >> }; >> @@ -138,7 +140,7 @@ enum jz4740_mmc_state { >> * COOKIE_MAPPED: the request was mapped in the irq handler, >> * and should be unmapped before mmc_request_done is called.. >> */ >> -enum jz4780_cookie { >> +enum jz4775_cookie { >> COOKIE_UNMAPPED = 0, >> COOKIE_PREMAPPED, >> COOKIE_MAPPED, >> @@ -194,7 +196,7 @@ static void jz4740_mmc_write_irq_mask(struct >> jz4740_mmc_host *host, >> static void jz4740_mmc_write_irq_reg(struct jz4740_mmc_host *host, >> uint32_t val) >> { >> - if (host->version >= JZ_MMC_JZ4780) >> + if (host->version >= JZ_MMC_JZ4775) >> writel(val, host->base + JZ_REG_MMC_IREG); >> else >> writew(val, host->base + JZ_REG_MMC_IREG); >> @@ -202,7 +204,7 @@ static void jz4740_mmc_write_irq_reg(struct >> jz4740_mmc_host *host, >> >> static uint32_t jz4740_mmc_read_irq_reg(struct jz4740_mmc_host *host) >> { >> - if (host->version >= JZ_MMC_JZ4780) >> + if (host->version >= JZ_MMC_JZ4775) >> return readl(host->base + JZ_REG_MMC_IREG); >> else >> return readw(host->base + JZ_REG_MMC_IREG); >> @@ -674,7 +676,7 @@ static void jz4740_mmc_send_command(struct >> jz4740_mmc_host *host, >> cmdat |= JZ_MMC_CMDAT_WRITE; >> if (host->use_dma) { >> /* >> - * The 4780's MMC controller has integrated DMA ability >> + * The JZ4775's MMC controller has integrated DMA ability >> * in addition to being able to use the external DMA >> * controller. It moves DMA control bits to a separate >> * register. The DMA_SEL bit chooses the external >> @@ -682,13 +684,13 @@ static void jz4740_mmc_send_command(struct >> jz4740_mmc_host *host, >> * can only use the external controller, and have a >> * single DMA enable bit in CMDAT. >> */ >> - if (host->version >= JZ_MMC_JZ4780) { >> + if (host->version >= JZ_MMC_JZ4775) { >> writel(JZ_MMC_DMAC_DMA_EN | JZ_MMC_DMAC_DMA_SEL, >> host->base + JZ_REG_MMC_DMAC); >> } else { >> cmdat |= JZ_MMC_CMDAT_DMA_EN; >> } >> - } else if (host->version >= JZ_MMC_JZ4780) { >> + } else if (host->version >= JZ_MMC_JZ4775) { >> writel(0, host->base + JZ_REG_MMC_DMAC); >> } >> >> @@ -866,7 +868,7 @@ 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) { >> + if (host->version >= JZ_MMC_JZ4775) { > > This changes the behaviour for the JZ4780. > > Even if it is correct, this belongs in its own commit (with a Fixes > tag), or at the very least a mention about it in the commit message. In fact, both JZ4775 and JZ4780 support this feature, I will add a corresponding description in the commit message. > >> 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, >> @@ -955,15 +957,16 @@ static const struct mmc_host_ops jz4740_mmc_ops >> = { >> .enable_sdio_irq = jz4740_mmc_enable_sdio_irq, >> }; >> >> -static const struct of_device_id jz4740_mmc_of_match[] = { >> +static const struct of_device_id jz4740_mmc_of_matches[] = { >> { .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,jz4775-mmc", .data = (void *) >> JZ_MMC_JZ4775 }, >> { .compatible = "ingenic,jz4780-mmc", .data = (void *) >> JZ_MMC_JZ4780 }, > > Looks to me that the JZ4775 and JZ4780 have the exact same behaviour, > so this patch could have been a one-liner, adding "ingenic,jz4775-mmc" > with the JZ_MMC_JZ4780 ID. > Sure, I will change it in the next version. Thanks and best regards! > Cheers, > -Paul > >> { .compatible = "ingenic,x1000-mmc", .data = (void *) >> JZ_MMC_X1000 }, >> {}, >> }; >> -MODULE_DEVICE_TABLE(of, jz4740_mmc_of_match); >> +MODULE_DEVICE_TABLE(of, jz4740_mmc_of_matches); >> >> static int jz4740_mmc_probe(struct platform_device* pdev) >> { >> @@ -980,7 +983,7 @@ static int jz4740_mmc_probe(struct >> platform_device* pdev) >> >> host = mmc_priv(mmc); >> >> - match = of_match_device(jz4740_mmc_of_match, &pdev->dev); >> + match = of_match_device(jz4740_mmc_of_matches, &pdev->dev); >> if (match) { >> host->version = (enum jz4740_mmc_version)match->data; >> } else { >> @@ -1124,7 +1127,7 @@ static struct platform_driver jz4740_mmc_driver >> = { >> .driver = { >> .name = "jz4740-mmc", >> .probe_type = PROBE_PREFER_ASYNCHRONOUS, >> - .of_match_table = of_match_ptr(jz4740_mmc_of_match), >> + .of_match_table = of_match_ptr(jz4740_mmc_of_matches), >> .pm = pm_ptr(&jz4740_mmc_pm_ops), >> }, >> }; >> -- >> 2.7.4 >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-06-08 14:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-06 18:08 [PATCH 0/2] Add support for JZ4775 and rename unreasonable array name 周琰杰 (Zhou Yanjie) 2021-06-06 18:08 ` [PATCH 1/2] dt-bindings: mmc: JZ4740: Add bindings for JZ4775 周琰杰 (Zhou Yanjie) 2021-06-08 13:16 ` Ulf Hansson 2021-06-06 18:08 ` [PATCH 2/2] mmc: JZ4740: Add support for JZ4775 and rename unreasonable array name 周琰杰 (Zhou Yanjie) 2021-06-08 13:16 ` Ulf Hansson 2021-06-08 13:44 ` Paul Cercueil 2021-06-08 14:11 ` Ulf Hansson 2021-06-08 14:50 ` Zhou Yanjie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).