mmc: renesas_sdhi: extend quirk selection to handle ES revisions
Subsystems (2)
v2 cover mmc: renesas_sdhi: extend quirk selection to handle ES revisions
From: Niklas Söderlund <[email protected]>
Hi,
Recent datasheet updates have made it clear that some quirks are not SoC specific but SoC + ES version specific. Currently the quirks are selected using compatibility values but whit this new information that is not enough.
Patch 1/3 adds support to select quirks based on SoC + ES revision using soc_device_match() and converts the only existing quirk. Patch 2/3 Removes the old method to select quirk based on compatibility string. While Patch 3/3 adds a new quirk from the BSP which blacklists some known problematic ES versions for HS400. HS400 is not yet enabled upstream so blacklisting these ES versions is not a regression of functionality.
Based on mmc/next and tested on H3 and M3-N.
Niklas Söderlund (3):
mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision
mmc: renesas_sdhi: align compatibility properties for H3 and M3-W
mmc: renesas_sdhi: disable HS400 on H3 ES1.x and M3-W ES1.x
drivers/mmc/host/renesas_sdhi_core.c | 36 +++++++++++++++++++ drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++--------- drivers/mmc/host/renesas_sdhi_sys_dmac.c | 20 ++--------- 3 files changed, 41 insertions(+), 35 deletions(-)
Signature · 1 line
2.19.1
PATCH v2 1/3 mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision
From: Niklas Söderlund <[email protected]>
Latest datasheet makes it clear that not all ES revisions of the H3 and M3-W have the 4-tap HS400 mode quirk, currently the quirk is set unconditionally for these two SoCs. Prepare to handle the quirk based on SoC revision instead of compatibility value by using soc_device_match() and set the TMIO_MMC_HAVE_4TAP_HS400 flag explicitly.
The reason for adding a new quirks struct instead of just a flag is that looking ahead it seems more quirks needs to be handled in a SoC revision basis.
--- drivers/mmc/host/renesas_sdhi_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
drivers/mmc/host/renesas_sdhi_core.c +26 -0
@@ -32,6 +32,7 @@#include <linux/pinctrl/consumer.h> #include <linux/pinctrl/pinctrl-state.h> #include <linux/regulator/consumer.h> +#include <linux/sys_soc.h> #include "renesas_sdhi.h" #include "tmio_mmc.h"
@@ -45,6 +46,10 @@#define SDHI_VER_GEN3_SD 0xcc10#define SDHI_VER_GEN3_SDMMC 0xcd10 +struct renesas_sdhi_quirks { +bool hs400_4taps; +}; +static void renesas_sdhi_sdbuf_width (struct tmio_mmc_host *host,int width) {u32 val;
@@ -602,11 +607,25 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)renesas_sdhi_sdbuf_width (host, enable ? width :16 ); } +static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w = { + .hs400_4taps = true, +}; + +static const struct soc_device_attribute sdhi_quirks_match[] = { + { .soc_id ="r8a7795" , .revision ="ES1.*" , .data = &sdhi_quirks_h3_m3w }, + { .soc_id ="r8a7795" , .revision ="ES2.0" , .data = &sdhi_quirks_h3_m3w }, + { .soc_id ="r8a7796" , .revision ="ES1.0" , .data = &sdhi_quirks_h3_m3w }, + { .soc_id ="r8a7796" , .revision ="ES1.1" , .data = &sdhi_quirks_h3_m3w }, + {/* Sentinel. */ }, +}; +int renesas_sdhi_probe (struct platform_device *pdev,const struct tmio_mmc_dma_ops *dma_ops) {struct tmio_mmc_data *mmd = pdev->dev.platform_data; +const struct renesas_sdhi_quirks *quirks =NULL ;const struct renesas_sdhi_of_data *of_data; +const struct soc_device_attribute *attr;struct tmio_mmc_data *mmc_data;struct tmio_mmc_dma *dma_priv;struct tmio_mmc_host *host;
@@ -616,6 +635,10 @@ int renesas_sdhi_probe(struct platform_device *pdev, of_data =of_device_get_match_data (&pdev->dev); + attr =soc_device_match (sdhi_quirks_match); +if (attr) + quirks = attr->data; + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 );if (!res)return -EINVAL ;
@@ -681,6 +704,9 @@ int renesas_sdhi_probe(struct platform_device *pdev, host->multi_io_quirk = renesas_sdhi_multi_io_quirk; host->dma_ops = dma_ops; +if (quirks && quirks->hs400_4taps) + mmc_data->flags |=TMIO_MMC_HAVE_4TAP_HS400 ; +/* For some SoC, we disable internal WP. GPIO may override this */ if (mmc_can_gpio_ro (host->mmc)) mmc_data->capabilities2 &= ~MMC_CAP2_NO_WRITE_PROTECT ;
Signature · 1 line
2.19.1
Re: [PATCH v2 1/3] mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision
Quoted 8 lines · Niklas Söderlund
From: Niklas Söderlund <[email protected]>
Latest datasheet makes it clear that not all ES revisions of the H3 and M3-W have the 4-tap HS400 mode quirk, currently the quirk is set unconditionally for these two SoCs. Prepare to handle the quirk based on SoC revision instead of compatibility value by using
soc_device_match()and set theTMIO_MMC_HAVE_4TAP_HS400flag explicitly.
Quoted 5 lines
The reason for adding a new quirks struct instead of just a flag is that looking ahead it seems more quirks needs to be handled in a SoC revision basis.
Signed-off-by Niklas Söderlund <[email protected]>
Re: [PATCH v2 1/3] mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision
In reply to Niklas SöderlundHi Niklas,
Quoted 8 lines · Niklas Söderlund
From: Niklas Söderlund <[email protected]>
Latest datasheet makes it clear that not all ES revisions of the H3 and M3-W have the 4-tap HS400 mode quirk, currently the quirk is set unconditionally for these two SoCs. Prepare to handle the quirk based on SoC revision instead of compatibility value by using
soc_device_match()and set theTMIO_MMC_HAVE_4TAP_HS400flag explicitly.
Quoted 5 lines
The reason for adding a new quirks struct instead of just a flag is that looking ahead it seems more quirks needs to be handled in a SoC revision basis.
Signed-off-by Niklas Söderlund <[email protected]>
Thanks for your patch!
drivers/mmc/host/renesas_sdhi_core.c static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable) · line 610
Quoted 5 lines
drivers/mmc/host/renesas_sdhi_core.c +1
-0@@ -602,11 +607,25 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)renesas_sdhi_sdbuf_width (host, enable ? width :16 ); } +static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w = {
I know this variable is renamed later, but sdhi_quirks_h3_m3w gives the impression it applies to all H3/M3-W variants.
What about sdhi_quirks_4tap instead?
drivers/mmc/host/renesas_sdhi_core.c static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable) · line 620
Quoted 10 lines
drivers/mmc/host/renesas_sdhi_core.c +10
-0+ .hs400_4taps = true, +}; + +static const struct soc_device_attribute sdhi_quirks_match[] = { + { .soc_id ="r8a7795" , .revision ="ES1.*" , .data = &sdhi_quirks_h3_m3w }, + { .soc_id ="r8a7795" , .revision ="ES2.0" , .data = &sdhi_quirks_h3_m3w }, + { .soc_id ="r8a7796" , .revision ="ES1.0" , .data = &sdhi_quirks_h3_m3w }, + { .soc_id ="r8a7796" , .revision ="ES1.1" , .data = &sdhi_quirks_h3_m3w }, + {/* Sentinel. */ }, +};
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] 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
PATCH v2 2/3 mmc: renesas_sdhi: align compatibility properties for H3 and M3-W
In reply to Niklas SöderlundFrom: Niklas Söderlund <[email protected]>
It was though all ES revisions of H3 and M3-W SoCs required the TMIO_MMC_HAVE_4TAP_HS400 flag. Recent datasheet updates tells us this is not true, only early ES revisions of the SoC do.
Since quirk matching based on ES revisions is now used to handle the flag it's possible to align all Gen3 compatibility properties. This will allow later ES revisions of H3 and M3-W to use the correct 8-tap HS400 mode.
--- drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++----------------- drivers/mmc/host/renesas_sdhi_sys_dmac.c | 20 +++---------------- 2 files changed, 5 insertions(+), 35 deletions(-)
drivers/mmc/host/renesas_sdhi_internal_dmac.c +2 -18
@@ -81,22 +81,6 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = { }, }; -static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = { - .tmio_flags =TMIO_MMC_HAS_IDLE_WAIT |TMIO_MMC_CLK_ACTUAL | -TMIO_MMC_HAVE_CBSY |TMIO_MMC_MIN_RCAR2 | -TMIO_MMC_HAVE_4TAP_HS400 , - .capabilities =MMC_CAP_SD_HIGHSPEED |MMC_CAP_SDIO_IRQ | -MMC_CAP_CMD23 , - .capabilities2 =MMC_CAP2_NO_WRITE_PROTECT , - .bus_shift =2 , - .scc_offset =0x1000 , - .taps = rcar_gen3_scc_taps, - .taps_num =ARRAY_SIZE (rcar_gen3_scc_taps), -/* DMAC can handle 0xffffffff blk count but only 1 segment */ - .max_blk_count =0xffffffff , - .max_segs =1 , -}; -static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = { .tmio_flags =TMIO_MMC_HAS_IDLE_WAIT |TMIO_MMC_CLK_ACTUAL |TMIO_MMC_HAVE_CBSY |TMIO_MMC_MIN_RCAR2 ,
@@ -114,8 +98,8 @@ static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = { { .compatible ="renesas,sdhi-mmc-r8a77470" , .data = &of_rcar_gen3_compatible, }, - { .compatible ="renesas,sdhi-r8a7795" , .data = &of_rcar_r8a7795_compatible, }, - { .compatible ="renesas,sdhi-r8a7796" , .data = &of_rcar_r8a7795_compatible, }, + { .compatible ="renesas,sdhi-r8a7795" , .data = &of_rcar_gen3_compatible, }, + { .compatible ="renesas,sdhi-r8a7796" , .data = &of_rcar_gen3_compatible, }, { .compatible ="renesas,rcar-gen3-sdhi" , .data = &of_rcar_gen3_compatible, }, {}, };
drivers/mmc/host/renesas_sdhi_sys_dmac.c +3 -17
@@ -75,19 +75,6 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = { }, }; -static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = { - .tmio_flags =TMIO_MMC_HAS_IDLE_WAIT |TMIO_MMC_CLK_ACTUAL | -TMIO_MMC_HAVE_CBSY |TMIO_MMC_MIN_RCAR2 | -TMIO_MMC_HAVE_4TAP_HS400 , - .capabilities =MMC_CAP_SD_HIGHSPEED |MMC_CAP_SDIO_IRQ | -MMC_CAP_CMD23 , - .capabilities2 =MMC_CAP2_NO_WRITE_PROTECT , - .bus_shift =2 , - .scc_offset =0x1000 , - .taps = rcar_gen3_scc_taps, - .taps_num =ARRAY_SIZE (rcar_gen3_scc_taps), -}; -static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = { .tmio_flags =TMIO_MMC_HAS_IDLE_WAIT |TMIO_MMC_CLK_ACTUAL |TMIO_MMC_HAVE_CBSY |TMIO_MMC_MIN_RCAR2 ,
@@ -114,8 +101,8 @@ static const struct of_device_id renesas_sdhi_sys_dmac_of_match[] = { { .compatible = "renesas,sdhi-r8a7792", .data = &of_rcar_gen2_compatible, }, { .compatible = "renesas,sdhi-r8a7793", .data = &of_rcar_gen2_compatible, }, { .compatible = "renesas,sdhi-r8a7794", .data = &of_rcar_gen2_compatible, }, - { .compatible = "renesas,sdhi-r8a7795", .data = &of_rcar_r8a7795_compatible, }, - { .compatible = "renesas,sdhi-r8a7796", .data = &of_rcar_r8a7795_compatible, }, + { .compatible = "renesas,sdhi-r8a7795", .data = &of_rcar_gen3_compatible, }, + { .compatible = "renesas,sdhi-r8a7796", .data = &of_rcar_gen3_compatible, }, { .compatible = "renesas,rcar-gen1-sdhi", .data = &of_rcar_gen1_compatible, }, { .compatible = "renesas,rcar-gen2-sdhi", .data = &of_rcar_gen2_compatible, }, { .compatible = "renesas,rcar-gen3-sdhi", .data = &of_rcar_gen3_compatible, },
@@ -493,8 +480,7 @@ static const struct soc_device_attribute gen3_soc_whitelist[] = {static int renesas_sdhi_sys_dmac_probe (struct platform_device *pdev) { -if ((of_device_get_match_data (&pdev->dev) == &of_rcar_gen3_compatible || -of_device_get_match_data (&pdev->dev) == &of_rcar_r8a7795_compatible) && +if (of_device_get_match_data (&pdev->dev) == &of_rcar_gen3_compatible && !soc_device_match (gen3_soc_whitelist))return -ENODEV ;
Signature · 1 line
2.19.1
Re: [PATCH v2 2/3] mmc: renesas_sdhi: align compatibility properties for H3 and M3-W
Quoted 6 lines · Niklas Söderlund
From: Niklas Söderlund <[email protected]>
It was though all ES revisions of H3 and M3-W SoCs required the
TMIO_MMC_HAVE_4TAP_HS400flag. Recent datasheet updates tells us this is not true, only early ES revisions of the SoC do.
Quoted 6 lines
Since quirk matching based on ES revisions is now used to handle the flag it's possible to align all Gen3 compatibility properties. This will allow later ES revisions of H3 and M3-W to use the correct 8-tap HS400 mode.
Signed-off-by Niklas Söderlund <[email protected]>
PATCH v2 3/3 mmc: renesas_sdhi: disable HS400 on H3 ES1.x and M3-W ES1.x
In reply to Niklas SöderlundFrom: Niklas Söderlund <[email protected]>
The Renesas BSP confirms that H3 ES1.x and M3-W ES1.x do not properly support HS400. Add a quirk to indicate this and disable HS400 in the MMC capabilities if the quirk is set.
--- drivers/mmc/host/renesas_sdhi_core.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)
drivers/mmc/host/renesas_sdhi_core.c +15 -5
@@ -47,6 +47,7 @@#define SDHI_VER_GEN3_SDMMC 0xcd10struct renesas_sdhi_quirks { +bool hs400_disabled;bool hs400_4taps; };
@@ -607,15 +608,21 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)renesas_sdhi_sdbuf_width (host, enable ? width :16 ); } -static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w = { +static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w_es1 = { + .hs400_disabled = true, + .hs400_4taps = true, +}; + +static const struct renesas_sdhi_quirks sdhi_quirks_h3_es2 = { + .hs400_disabled = false, .hs400_4taps = true, };static const struct soc_device_attribute sdhi_quirks_match[] = { - { .soc_id ="r8a7795" , .revision ="ES1.*" , .data = &sdhi_quirks_h3_m3w }, - { .soc_id ="r8a7795" , .revision ="ES2.0" , .data = &sdhi_quirks_h3_m3w }, - { .soc_id ="r8a7796" , .revision ="ES1.0" , .data = &sdhi_quirks_h3_m3w }, - { .soc_id ="r8a7796" , .revision ="ES1.1" , .data = &sdhi_quirks_h3_m3w }, + { .soc_id ="r8a7795" , .revision ="ES1.*" , .data = &sdhi_quirks_h3_m3w_es1 }, + { .soc_id ="r8a7795" , .revision ="ES2.0" , .data = &sdhi_quirks_h3_es2 }, + { .soc_id ="r8a7796" , .revision ="ES1.0" , .data = &sdhi_quirks_h3_m3w_es1 }, + { .soc_id ="r8a7796" , .revision ="ES1.1" , .data = &sdhi_quirks_h3_m3w_es1 }, {/* Sentinel. */ }, };
@@ -704,6 +711,9 @@ int renesas_sdhi_probe(struct platform_device *pdev, host->multi_io_quirk = renesas_sdhi_multi_io_quirk; host->dma_ops = dma_ops; +if (quirks && quirks->hs400_disabled) + host->mmc->caps2 &= ~(MMC_CAP2_HS400 |MMC_CAP2_HS400_ES ); +if (quirks && quirks->hs400_4taps) mmc_data->flags |=TMIO_MMC_HAVE_4TAP_HS400 ;
Signature · 1 line
2.19.1
Re: [PATCH v2 3/3] mmc: renesas_sdhi: disable HS400 on H3 ES1.x and M3-W ES1.x
Quoted 7 lines · Niklas Söderlund
From: Niklas Söderlund <[email protected]>
The Renesas BSP confirms that H3 ES1.x and M3-W ES1.x do not properly support HS400. Add a quirk to indicate this and disable HS400 in the MMC capabilities if the quirk is set.
Signed-off-by Niklas Söderlund <[email protected]>
Re: [PATCH v2 3/3] mmc: renesas_sdhi: disable HS400 on H3 ES1.x and M3-W ES1.x
In reply to Niklas SöderlundHi Niklas,
Thanks for your patch!
Quoted 3 lines · Niklas Söderlund
From: Niklas Söderlund <[email protected]>
The Renesas BSP confirms that H3 ES1.x and M3-W ES1.x do not properly
M3-W ES1.[012] (also in the subject)? (ES1.2 is indistinguishable from ES1.1) Or does this apply to M3-W ES1.3, too?
Quoted 21 lines
support HS400. Add a quirk to indicate this and disable HS400 in the MMC capabilities if the quirk is set.
Signed-off-by Niklas Söderlund <[email protected]>drivers/mmc/host/renesas_sdhi_core.c +7
-1@@ -607,15 +608,21 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)renesas_sdhi_sdbuf_width (host, enable ? width :16 ); } -static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w = { +static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w_es1 = { + .hs400_disabled = true, + .hs400_4taps = true, +}; + +static const struct renesas_sdhi_quirks sdhi_quirks_h3_es2 = { + .hs400_disabled = false, .hs400_4taps = true, };static const struct soc_device_attribute sdhi_quirks_match[] = {
drivers/mmc/host/renesas_sdhi_core.c static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable) · line 627
Quoted 10 lines
drivers/mmc/host/renesas_sdhi_core.c +4
-4- { .soc_id = "r8a7795", .revision = "ES1.*", .data = &sdhi_quirks_h3_m3w }, - { .soc_id = "r8a7795", .revision = "ES2.0", .data = &sdhi_quirks_h3_m3w }, - { .soc_id = "r8a7796", .revision = "ES1.0", .data = &sdhi_quirks_h3_m3w }, - { .soc_id = "r8a7796", .revision = "ES1.1", .data = &sdhi_quirks_h3_m3w }, + { .soc_id = "r8a7795", .revision = "ES1.*", .data = &sdhi_quirks_h3_m3w_es1 }, + { .soc_id = "r8a7795", .revision = "ES2.0", .data = &sdhi_quirks_h3_es2 }, + { .soc_id = "r8a7796", .revision = "ES1.0", .data = &sdhi_quirks_h3_m3w_es1 }, + { .soc_id = "r8a7796", .revision = "ES1.1", .data = &sdhi_quirks_h3_m3w_es1 }, { /* Sentinel. */ }, };
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] 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
Quoted 8 lines
Patch 1/3 adds support to select quirks based on SoC + ES revision using
soc_device_match()and converts the only existing quirk. Patch 2/3 Removes the old method to select quirk based on compatibility string. While Patch 3/3 adds a new quirk from the BSP which blacklists some known problematic ES versions for HS400. HS400 is not yet enabled upstream so blacklisting these ES versions is not a regression of functionality.
This will allow us even to get rid of the TMIO_MMC_HAVE_4TAP_HS400 flag entirely because we now use it only within the Renesas part of SDHI. We want to do this incrementally, though, because we first want to get the HS400 feature enabled as the first step.
Simon Horman
Reviewed-byQuoted 9 lines · Wolfram Sang
Quoted 8 lines
Patch 1/3 adds support to select quirks based on SoC + ES revision using
soc_device_match()and converts the only existing quirk. Patch 2/3 Removes the old method to select quirk based on compatibility string. While Patch 3/3 adds a new quirk from the BSP which blacklists some known problematic ES versions for HS400. HS400 is not yet enabled upstream so blacklisting these ES versions is not a regression of functionality.
Quoted 4 lines
This will allow us even to get rid of the
TMIO_MMC_HAVE_4TAP_HS400flag entirely because we now use it only within the Renesas part of SDHI. We want to do this incrementally, though, because we first want to get the HS400 feature enabled as the first step.
Thanks, this looks good to me.
All patches tested on M3N:
Quoted 24 lines · Niklas Söderlund
From: Niklas Söderlund <[email protected]>
Hi,
Recent datasheet updates have made it clear that some quirks are not SoC specific but SoC + ES version specific. Currently the quirks are selected using compatibility values but whit this new information that is not enough.
Patch 1/3 adds support to select quirks based on SoC + ES revision using
soc_device_match()and converts the only existing quirk. Patch 2/3 Removes the old method to select quirk based on compatibility string. While Patch 3/3 adds a new quirk from the BSP which blacklists some known problematic ES versions for HS400. HS400 is not yet enabled upstream so blacklisting these ES versions is not a regression of functionality.Based on mmc/next and tested on H3 and M3-N.
Niklas Söderlund (3):
mmc:renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision
mmc:renesas_sdhi: align compatibility properties for H3 and M3-W
mmc:renesas_sdhi: disable HS400 on H3 ES1.x and M3-W ES1.x
Quoted 8 lines
drivers/mmc/host/renesas_sdhi_core.c | 36 +++++++++++++++++++ drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++--------- drivers/mmc/host/renesas_sdhi_sys_dmac.c | 20 ++--------- 3 files changed, 41 insertions(+), 35 deletions(-)--
2.19.1
Applied for next, thanks!
I noticed there were a minor comment from Geert, however I decided to pick this as is and leave further improvements to be made on top.
Kind regards
Uffe
Quoted 3 lines
I noticed there were a minor comment from Geert, however I decided to pick this as is and leave further improvements to be made on top.
Same here; could we wait for v2?