* [PATCH 2/3] ARM: S5PV210: Add support SDMMC WP through EXT_INT on SMDKV210
2010-06-15 11:27 ` [PATCH 2/3] ARM: S5PV210: Add support SDMMC WP through EXT_INT on SMDKV210 Kukjin Kim
@ 2010-06-15 11:52 ` Kyungmin Park
2010-07-03 1:08 ` Kukjin Kim
2010-06-15 11:53 ` Maurus Cuelenaere
2010-06-15 11:59 ` Ben Dooks
2 siblings, 1 reply; 13+ messages in thread
From: Kyungmin Park @ 2010-06-15 11:52 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jun 15, 2010 at 8:27 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> From: Lee Hyuk <hyuk1.lee@samsung.com>
>
> S5PV210 HSMMC host controller doesn't have the Write Protection pin which
> should be connnected with SDMMC card WP pin. So allocated a GPIO in order to
> get the data from SDMMC card WP pin with EXT_INT and implement get_ro and
> cfg_wp function.
>
> Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> ---
> ?arch/arm/mach-s5pv210/mach-smdkv210.c | ? ?2 +
> ?arch/arm/mach-s5pv210/setup-sdhci.c ? | ? 72 +++++++++++++++++++++++++++++++++
> ?2 files changed, 74 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-s5pv210/mach-smdkv210.c b/arch/arm/mach-s5pv210/mach-smdkv210.c
> index b08f376..18cdb8c 100644
> --- a/arch/arm/mach-s5pv210/mach-smdkv210.c
> +++ b/arch/arm/mach-s5pv210/mach-smdkv210.c
> @@ -27,6 +27,7 @@
> ?#include <plat/cpu.h>
> ?#include <plat/adc.h>
> ?#include <plat/ts.h>
> +#include <plat/sdhci.h>
>
> ?/* Following are default values for UCON, ULCON and UFCON UART registers */
> ?#define S5PV210_UCON_DEFAULT ? (S3C2410_UCON_TXILEVEL | ? ? ? ?\
> @@ -101,6 +102,7 @@ static void __init smdkv210_map_io(void)
> ?static void __init smdkv210_machine_init(void)
> ?{
> ? ? ? ?s3c24xx_ts_set_platdata(&s3c_ts_platform);
> + ? ? ? s3c_sdhci_set_platdata();
> ? ? ? ?platform_add_devices(smdkv210_devices, ARRAY_SIZE(smdkv210_devices));
> ?}
>
> diff --git a/arch/arm/mach-s5pv210/setup-sdhci.c b/arch/arm/mach-s5pv210/setup-sdhci.c
> index 51815ec..b553b36 100644
> --- a/arch/arm/mach-s5pv210/setup-sdhci.c
> +++ b/arch/arm/mach-s5pv210/setup-sdhci.c
> @@ -15,13 +15,17 @@
> ?#include <linux/interrupt.h>
> ?#include <linux/platform_device.h>
> ?#include <linux/io.h>
> +#include <linux/gpio.h>
>
> ?#include <linux/mmc/card.h>
> ?#include <linux/mmc/host.h>
>
> +#include <plat/gpio-cfg.h>
> ?#include <plat/regs-sdhci.h>
> ?#include <plat/sdhci.h>
>
> +#include <mach/map.h>
> +
> ?/* clock sources for the mmc bus clock, order as for the ctrl2[5..4] */
>
> ?char *s5pv210_hsmmc_clksrcs[4] = {
> @@ -61,3 +65,71 @@ void s5pv210_setup_sdhci_cfg_card(struct platform_device *dev,
> ? ? ? ?writel(ctrl2, r + S3C_SDHCI_CONTROL2);
> ? ? ? ?writel(ctrl3, r + S3C_SDHCI_CONTROL3);
> ?}
> +
> +static int s5pv210_sdhci_get_ro(struct mmc_host *mmc)
> +{
> + ? ? ? int gpio_dat;
> +
> + ? ? ? if ((mmc->index == 0 && s3c_hsmmc0_def_platdata.cfg_wp != NULL) ||
> + ? ? ? ? ? ? ? (mmc->index == 1 && s3c_hsmmc1_def_platdata.cfg_wp != NULL)) {
> + ? ? ? ? ? ? ? gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x04);
> + ? ? ? ? ? ? ? return !!(gpio_dat & 0x80);
It's GPIO values, and it's smdk specific values. So it should go smdk
board files instead of platform common one.
> + ? ? ? } else if (mmc->index == 2 && s3c_hsmmc2_def_platdata.cfg_wp != NULL) {
> + ? ? ? ? ? ? ? gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x64);
> + ? ? ? ? ? ? ? return !!(gpio_dat & 0x2);
> + ? ? ? } else if (mmc->index == 3 && s3c_hsmmc3_def_platdata.cfg_wp != NULL) {
> + ? ? ? ? ? ? ? gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x24);
> + ? ? ? ? ? ? ? return !!(gpio_dat & 0x1);
> + ? ? ? } else
> + ? ? ? ? ? ? ? return 0;
> +}
> +
> +static void s5pv210_setup_sdhci_gpio_wp(int dev_id)
> +{
> + ? ? ? switch (dev_id) {
> + ? ? ? case 0:
> + ? ? ? case 1:
> + ? ? ? ? ? ? ? s3c_gpio_cfgpin(S5PV210_GPH0(7), S3C_GPIO_INPUT);
> + ? ? ? ? ? ? ? s3c_gpio_setpull(S5PV210_GPH0(7), S3C_GPIO_PULL_UP);
> + ? ? ? ? ? ? ? break;
> + ? ? ? case 2:
> + ? ? ? ? ? ? ? s3c_gpio_cfgpin(S5PV210_GPH3(1), S3C_GPIO_INPUT);
> + ? ? ? ? ? ? ? s3c_gpio_setpull(S5PV210_GPH3(1), S3C_GPIO_PULL_UP);
> + ? ? ? ? ? ? ? break;
> + ? ? ? case 3:
> + ? ? ? ? ? ? ? s3c_gpio_cfgpin(S5PV210_GPH1(0), S3C_GPIO_INPUT);
> + ? ? ? ? ? ? ? s3c_gpio_setpull(S5PV210_GPH1(0), S3C_GPIO_PULL_UP);
Ditto. board specific configurations. should go smdk board file.
> + ? ? ? ? ? ? ? break;
> + ? ? ? default:
> + ? ? ? ? ? ? ? break;
> + ? ? ? }
> +}
> +
> +static struct s3c_sdhci_platdata hsmmc0_platdata = {
> + ? ? ? .cfg_wp ? ? ? ? = s5pv210_setup_sdhci_gpio_wp,
> + ? ? ? .get_ro ? ? ? ? = s5pv210_sdhci_get_ro,
> +};
> +
> +static struct s3c_sdhci_platdata hsmmc1_platdata = {
> + ? ? ? .cfg_wp ? ? ? ? = s5pv210_setup_sdhci_gpio_wp,
> + ? ? ? .get_ro ? ? ? ? = s5pv210_sdhci_get_ro,
> +};
> +
> +static struct s3c_sdhci_platdata hsmmc2_platdata = {
> + ? ? ? .cfg_wp ? ? ? ? = s5pv210_setup_sdhci_gpio_wp,
> + ? ? ? .get_ro ? ? ? ? = s5pv210_sdhci_get_ro,
> +};
> +
> +static struct s3c_sdhci_platdata hsmmc3_platdata = {
> + ? ? ? .cfg_wp ? ? ? ? = s5pv210_setup_sdhci_gpio_wp,
> + ? ? ? .get_ro ? ? ? ? = s5pv210_sdhci_get_ro,
> +};
> +
> +void s3c_sdhci_set_platdata(void)
> +{
> + ? ? ? s3c_sdhci0_set_platdata(&hsmmc0_platdata);
> + ? ? ? s3c_sdhci1_set_platdata(&hsmmc1_platdata);
> + ? ? ? s3c_sdhci2_set_platdata(&hsmmc2_platdata);
> + ? ? ? s3c_sdhci3_set_platdata(&hsmmc3_platdata);
> +}
> +EXPORT_SYMBOL(s3c_sdhci_set_platdata);
I think all added lines should go smdk totally.
Thank you,
Kyungmin Park
> --
> 1.6.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 2/3] ARM: S5PV210: Add support SDMMC WP through EXT_INT on SMDKV210
2010-06-15 11:52 ` Kyungmin Park
@ 2010-07-03 1:08 ` Kukjin Kim
0 siblings, 0 replies; 13+ messages in thread
From: Kukjin Kim @ 2010-07-03 1:08 UTC (permalink / raw)
To: linux-arm-kernel
Kyungmin Park wrote:
>
> On Tue, Jun 15, 2010 at 8:27 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> > From: Lee Hyuk <hyuk1.lee@samsung.com>
> >
> > S5PV210 HSMMC host controller doesn't have the Write Protection pin
which
> > should be connnected with SDMMC card WP pin. So allocated a GPIO in
order
> to
> > get the data from SDMMC card WP pin with EXT_INT and implement get_ro
and
> > cfg_wp function.
> >
> > Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > ---
> > ?arch/arm/mach-s5pv210/mach-smdkv210.c | ? ?2 +
> > ?arch/arm/mach-s5pv210/setup-sdhci.c ? | ? 72
> +++++++++++++++++++++++++++++++++
> > ?2 files changed, 74 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-s5pv210/mach-smdkv210.c b/arch/arm/mach-
> s5pv210/mach-smdkv210.c
> > index b08f376..18cdb8c 100644
> > --- a/arch/arm/mach-s5pv210/mach-smdkv210.c
> > +++ b/arch/arm/mach-s5pv210/mach-smdkv210.c
> > @@ -27,6 +27,7 @@
> > ?#include <plat/cpu.h>
> > ?#include <plat/adc.h>
> > ?#include <plat/ts.h>
> > +#include <plat/sdhci.h>
> >
> > ?/* Following are default values for UCON, ULCON and UFCON UART
registers */
> > ?#define S5PV210_UCON_DEFAULT ? (S3C2410_UCON_TXILEVEL | ? ? ? ?\
> > @@ -101,6 +102,7 @@ static void __init smdkv210_map_io(void)
> > ?static void __init smdkv210_machine_init(void)
> > ?{
> > ? ? ? ?s3c24xx_ts_set_platdata(&s3c_ts_platform);
> > + ? ? ? s3c_sdhci_set_platdata();
> > ? ? ? ?platform_add_devices(smdkv210_devices,
> ARRAY_SIZE(smdkv210_devices));
> > ?}
> >
> > diff --git a/arch/arm/mach-s5pv210/setup-sdhci.c b/arch/arm/mach-
> s5pv210/setup-sdhci.c
> > index 51815ec..b553b36 100644
> > --- a/arch/arm/mach-s5pv210/setup-sdhci.c
> > +++ b/arch/arm/mach-s5pv210/setup-sdhci.c
> > @@ -15,13 +15,17 @@
> > ?#include <linux/interrupt.h>
> > ?#include <linux/platform_device.h>
> > ?#include <linux/io.h>
> > +#include <linux/gpio.h>
> >
> > ?#include <linux/mmc/card.h>
> > ?#include <linux/mmc/host.h>
> >
> > +#include <plat/gpio-cfg.h>
> > ?#include <plat/regs-sdhci.h>
> > ?#include <plat/sdhci.h>
> >
> > +#include <mach/map.h>
> > +
> > ?/* clock sources for the mmc bus clock, order as for the ctrl2[5..4] */
> >
> > ?char *s5pv210_hsmmc_clksrcs[4] = {
> > @@ -61,3 +65,71 @@ void s5pv210_setup_sdhci_cfg_card(struct
> platform_device *dev,
> > ? ? ? ?writel(ctrl2, r + S3C_SDHCI_CONTROL2);
> > ? ? ? ?writel(ctrl3, r + S3C_SDHCI_CONTROL3);
> > ?}
> > +
> > +static int s5pv210_sdhci_get_ro(struct mmc_host *mmc)
> > +{
> > + ? ? ? int gpio_dat;
> > +
> > + ? ? ? if ((mmc->index == 0 && s3c_hsmmc0_def_platdata.cfg_wp != NULL)
||
> > + ? ? ? ? ? ? ? (mmc->index == 1 && s3c_hsmmc1_def_platdata.cfg_wp !=
> NULL)) {
> > + ? ? ? ? ? ? ? gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x04);
> > + ? ? ? ? ? ? ? return !!(gpio_dat & 0x80);
>
> It's GPIO values, and it's smdk specific values. So it should go smdk
> board files instead of platform common one.
>
Yes, you're right..moved.
> > + ? ? ? } else if (mmc->index == 2 && s3c_hsmmc2_def_platdata.cfg_wp !=
> NULL) {
> > + ? ? ? ? ? ? ? gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x64);
> > + ? ? ? ? ? ? ? return !!(gpio_dat & 0x2);
> > + ? ? ? } else if (mmc->index == 3 && s3c_hsmmc3_def_platdata.cfg_wp !=
> NULL) {
> > + ? ? ? ? ? ? ? gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x24);
> > + ? ? ? ? ? ? ? return !!(gpio_dat & 0x1);
> > + ? ? ? } else
> > + ? ? ? ? ? ? ? return 0;
> > +}
> > +
> > +static void s5pv210_setup_sdhci_gpio_wp(int dev_id)
> > +{
> > + ? ? ? switch (dev_id) {
> > + ? ? ? case 0:
> > + ? ? ? case 1:
> > + ? ? ? ? ? ? ? s3c_gpio_cfgpin(S5PV210_GPH0(7), S3C_GPIO_INPUT);
> > + ? ? ? ? ? ? ? s3c_gpio_setpull(S5PV210_GPH0(7), S3C_GPIO_PULL_UP);
> > + ? ? ? ? ? ? ? break;
> > + ? ? ? case 2:
> > + ? ? ? ? ? ? ? s3c_gpio_cfgpin(S5PV210_GPH3(1), S3C_GPIO_INPUT);
> > + ? ? ? ? ? ? ? s3c_gpio_setpull(S5PV210_GPH3(1), S3C_GPIO_PULL_UP);
> > + ? ? ? ? ? ? ? break;
> > + ? ? ? case 3:
> > + ? ? ? ? ? ? ? s3c_gpio_cfgpin(S5PV210_GPH1(0), S3C_GPIO_INPUT);
> > + ? ? ? ? ? ? ? s3c_gpio_setpull(S5PV210_GPH1(0), S3C_GPIO_PULL_UP);
>
> Ditto. board specific configurations. should go smdk board file.
>
Moved.
> > + ? ? ? ? ? ? ? break;
> > + ? ? ? default:
> > + ? ? ? ? ? ? ? break;
> > + ? ? ? }
> > +}
> > +
> > +static struct s3c_sdhci_platdata hsmmc0_platdata = {
> > + ? ? ? .cfg_wp ? ? ? ? = s5pv210_setup_sdhci_gpio_wp,
> > + ? ? ? .get_ro ? ? ? ? = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +static struct s3c_sdhci_platdata hsmmc1_platdata = {
> > + ? ? ? .cfg_wp ? ? ? ? = s5pv210_setup_sdhci_gpio_wp,
> > + ? ? ? .get_ro ? ? ? ? = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +static struct s3c_sdhci_platdata hsmmc2_platdata = {
> > + ? ? ? .cfg_wp ? ? ? ? = s5pv210_setup_sdhci_gpio_wp,
> > + ? ? ? .get_ro ? ? ? ? = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +static struct s3c_sdhci_platdata hsmmc3_platdata = {
> > + ? ? ? .cfg_wp ? ? ? ? = s5pv210_setup_sdhci_gpio_wp,
> > + ? ? ? .get_ro ? ? ? ? = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +void s3c_sdhci_set_platdata(void)
> > +{
> > + ? ? ? s3c_sdhci0_set_platdata(&hsmmc0_platdata);
> > + ? ? ? s3c_sdhci1_set_platdata(&hsmmc1_platdata);
> > + ? ? ? s3c_sdhci2_set_platdata(&hsmmc2_platdata);
> > + ? ? ? s3c_sdhci3_set_platdata(&hsmmc3_platdata);
> > +}
> > +EXPORT_SYMBOL(s3c_sdhci_set_platdata);
>
> I think all added lines should go smdk totally.
>
Fixed in v2 patches as your comments.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] ARM: S5PV210: Add support SDMMC WP through EXT_INT on SMDKV210
2010-06-15 11:27 ` [PATCH 2/3] ARM: S5PV210: Add support SDMMC WP through EXT_INT on SMDKV210 Kukjin Kim
2010-06-15 11:52 ` Kyungmin Park
@ 2010-06-15 11:53 ` Maurus Cuelenaere
2010-07-03 1:11 ` Kukjin Kim
2010-06-15 11:59 ` Ben Dooks
2 siblings, 1 reply; 13+ messages in thread
From: Maurus Cuelenaere @ 2010-06-15 11:53 UTC (permalink / raw)
To: linux-arm-kernel
Op 15-06-10 13:27, Kukjin Kim schreef:
> From: Lee Hyuk <hyuk1.lee@samsung.com>
>
> S5PV210 HSMMC host controller doesn't have the Write Protection pin which
> should be connnected with SDMMC card WP pin. So allocated a GPIO in order to
> get the data from SDMMC card WP pin with EXT_INT and implement get_ro and
> cfg_wp function.
>
> Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> ---
> arch/arm/mach-s5pv210/mach-smdkv210.c | 2 +
> arch/arm/mach-s5pv210/setup-sdhci.c | 72 +++++++++++++++++++++++++++++++++
> 2 files changed, 74 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-s5pv210/mach-smdkv210.c b/arch/arm/mach-s5pv210/mach-smdkv210.c
> index b08f376..18cdb8c 100644
> --- a/arch/arm/mach-s5pv210/mach-smdkv210.c
> +++ b/arch/arm/mach-s5pv210/mach-smdkv210.c
> @@ -27,6 +27,7 @@
> #include <plat/cpu.h>
> #include <plat/adc.h>
> #include <plat/ts.h>
> +#include <plat/sdhci.h>
>
> /* Following are default values for UCON, ULCON and UFCON UART registers */
> #define S5PV210_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
> @@ -101,6 +102,7 @@ static void __init smdkv210_map_io(void)
> static void __init smdkv210_machine_init(void)
> {
> s3c24xx_ts_set_platdata(&s3c_ts_platform);
> + s3c_sdhci_set_platdata();
>
Wouldn't it be better to name this s5pv210_sdhci_set_platdata() ?
Also, see below.
> platform_add_devices(smdkv210_devices, ARRAY_SIZE(smdkv210_devices));
> }
>
> diff --git a/arch/arm/mach-s5pv210/setup-sdhci.c b/arch/arm/mach-s5pv210/setup-sdhci.c
> index 51815ec..b553b36 100644
> --- a/arch/arm/mach-s5pv210/setup-sdhci.c
> +++ b/arch/arm/mach-s5pv210/setup-sdhci.c
> @@ -15,13 +15,17 @@
> #include <linux/interrupt.h>
> #include <linux/platform_device.h>
> #include <linux/io.h>
> +#include <linux/gpio.h>
>
> #include <linux/mmc/card.h>
> #include <linux/mmc/host.h>
>
> +#include <plat/gpio-cfg.h>
> #include <plat/regs-sdhci.h>
> #include <plat/sdhci.h>
>
> +#include <mach/map.h>
> +
> /* clock sources for the mmc bus clock, order as for the ctrl2[5..4] */
>
> char *s5pv210_hsmmc_clksrcs[4] = {
> @@ -61,3 +65,71 @@ void s5pv210_setup_sdhci_cfg_card(struct platform_device *dev,
> writel(ctrl2, r + S3C_SDHCI_CONTROL2);
> writel(ctrl3, r + S3C_SDHCI_CONTROL3);
> }
> +
> +static int s5pv210_sdhci_get_ro(struct mmc_host *mmc)
> +{
> + int gpio_dat;
> +
> + if ((mmc->index == 0 && s3c_hsmmc0_def_platdata.cfg_wp != NULL) ||
> + (mmc->index == 1 && s3c_hsmmc1_def_platdata.cfg_wp != NULL)) {
> + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x04);
> + return !!(gpio_dat & 0x80);
> + } else if (mmc->index == 2 && s3c_hsmmc2_def_platdata.cfg_wp != NULL) {
> + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x64);
> + return !!(gpio_dat & 0x2);
> + } else if (mmc->index == 3 && s3c_hsmmc3_def_platdata.cfg_wp != NULL) {
> + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x24);
> + return !!(gpio_dat & 0x1);
> + } else
> + return 0;
> +}
> +
> +static void s5pv210_setup_sdhci_gpio_wp(int dev_id)
> +{
> + switch (dev_id) {
> + case 0:
> + case 1:
> + s3c_gpio_cfgpin(S5PV210_GPH0(7), S3C_GPIO_INPUT);
> + s3c_gpio_setpull(S5PV210_GPH0(7), S3C_GPIO_PULL_UP);
> + break;
> + case 2:
> + s3c_gpio_cfgpin(S5PV210_GPH3(1), S3C_GPIO_INPUT);
> + s3c_gpio_setpull(S5PV210_GPH3(1), S3C_GPIO_PULL_UP);
> + break;
> + case 3:
> + s3c_gpio_cfgpin(S5PV210_GPH1(0), S3C_GPIO_INPUT);
> + s3c_gpio_setpull(S5PV210_GPH1(0), S3C_GPIO_PULL_UP);
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static struct s3c_sdhci_platdata hsmmc0_platdata = {
> + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> + .get_ro = s5pv210_sdhci_get_ro,
> +};
> +
> +static struct s3c_sdhci_platdata hsmmc1_platdata = {
> + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> + .get_ro = s5pv210_sdhci_get_ro,
> +};
> +
> +static struct s3c_sdhci_platdata hsmmc2_platdata = {
> + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> + .get_ro = s5pv210_sdhci_get_ro,
> +};
> +
> +static struct s3c_sdhci_platdata hsmmc3_platdata = {
> + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> + .get_ro = s5pv210_sdhci_get_ro,
> +};
> +
> +void s3c_sdhci_set_platdata(void)
> +{
> + s3c_sdhci0_set_platdata(&hsmmc0_platdata);
> + s3c_sdhci1_set_platdata(&hsmmc1_platdata);
> + s3c_sdhci2_set_platdata(&hsmmc2_platdata);
> + s3c_sdhci3_set_platdata(&hsmmc3_platdata);
> +}
> +EXPORT_SYMBOL(s3c_sdhci_set_platdata);
>
Why do you export this function? I don't see this being used anywhere
except in mach-s5pv210.
--
Maurus Cuelenaere
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 2/3] ARM: S5PV210: Add support SDMMC WP through EXT_INT on SMDKV210
2010-06-15 11:53 ` Maurus Cuelenaere
@ 2010-07-03 1:11 ` Kukjin Kim
0 siblings, 0 replies; 13+ messages in thread
From: Kukjin Kim @ 2010-07-03 1:11 UTC (permalink / raw)
To: linux-arm-kernel
Maurus Cuelenaere wrote:
>
> Op 15-06-10 13:27, Kukjin Kim schreef:
> > From: Lee Hyuk <hyuk1.lee@samsung.com>
> >
> > S5PV210 HSMMC host controller doesn't have the Write Protection pin which
> > should be connnected with SDMMC card WP pin. So allocated a GPIO in order
> to
> > get the data from SDMMC card WP pin with EXT_INT and implement get_ro and
> > cfg_wp function.
> >
> > Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > ---
> > arch/arm/mach-s5pv210/mach-smdkv210.c | 2 +
> > arch/arm/mach-s5pv210/setup-sdhci.c | 72
> +++++++++++++++++++++++++++++++++
> > 2 files changed, 74 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-s5pv210/mach-smdkv210.c b/arch/arm/mach-
> s5pv210/mach-smdkv210.c
> > index b08f376..18cdb8c 100644
> > --- a/arch/arm/mach-s5pv210/mach-smdkv210.c
> > +++ b/arch/arm/mach-s5pv210/mach-smdkv210.c
> > @@ -27,6 +27,7 @@
> > #include <plat/cpu.h>
> > #include <plat/adc.h>
> > #include <plat/ts.h>
> > +#include <plat/sdhci.h>
> >
> > /* Following are default values for UCON, ULCON and UFCON UART registers
> */
> > #define S5PV210_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
> > @@ -101,6 +102,7 @@ static void __init smdkv210_map_io(void)
> > static void __init smdkv210_machine_init(void)
> > {
> > s3c24xx_ts_set_platdata(&s3c_ts_platform);
> > + s3c_sdhci_set_platdata();
> >
>
> Wouldn't it be better to name this s5pv210_sdhci_set_platdata() ?
> Also, see below.
>
Ok..so used s3c_sdhciX_set_platdata() instead of using above.
> > platform_add_devices(smdkv210_devices,
> ARRAY_SIZE(smdkv210_devices));
> > }
> >
> > diff --git a/arch/arm/mach-s5pv210/setup-sdhci.c b/arch/arm/mach-
> s5pv210/setup-sdhci.c
> > index 51815ec..b553b36 100644
> > --- a/arch/arm/mach-s5pv210/setup-sdhci.c
> > +++ b/arch/arm/mach-s5pv210/setup-sdhci.c
> > @@ -15,13 +15,17 @@
> > #include <linux/interrupt.h>
> > #include <linux/platform_device.h>
> > #include <linux/io.h>
> > +#include <linux/gpio.h>
> >
> > #include <linux/mmc/card.h>
> > #include <linux/mmc/host.h>
> >
> > +#include <plat/gpio-cfg.h>
> > #include <plat/regs-sdhci.h>
> > #include <plat/sdhci.h>
> >
> > +#include <mach/map.h>
> > +
> > /* clock sources for the mmc bus clock, order as for the ctrl2[5..4] */
> >
> > char *s5pv210_hsmmc_clksrcs[4] = {
> > @@ -61,3 +65,71 @@ void s5pv210_setup_sdhci_cfg_card(struct
> platform_device *dev,
> > writel(ctrl2, r + S3C_SDHCI_CONTROL2);
> > writel(ctrl3, r + S3C_SDHCI_CONTROL3);
> > }
> > +
> > +static int s5pv210_sdhci_get_ro(struct mmc_host *mmc)
> > +{
> > + int gpio_dat;
> > +
> > + if ((mmc->index == 0 && s3c_hsmmc0_def_platdata.cfg_wp != NULL) ||
> > + (mmc->index == 1 && s3c_hsmmc1_def_platdata.cfg_wp !=
> NULL)) {
> > + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x04);
> > + return !!(gpio_dat & 0x80);
> > + } else if (mmc->index == 2 && s3c_hsmmc2_def_platdata.cfg_wp !=
> NULL) {
> > + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x64);
> > + return !!(gpio_dat & 0x2);
> > + } else if (mmc->index == 3 && s3c_hsmmc3_def_platdata.cfg_wp !=
> NULL) {
> > + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x24);
> > + return !!(gpio_dat & 0x1);
> > + } else
> > + return 0;
> > +}
> > +
> > +static void s5pv210_setup_sdhci_gpio_wp(int dev_id)
> > +{
> > + switch (dev_id) {
> > + case 0:
> > + case 1:
> > + s3c_gpio_cfgpin(S5PV210_GPH0(7), S3C_GPIO_INPUT);
> > + s3c_gpio_setpull(S5PV210_GPH0(7), S3C_GPIO_PULL_UP);
> > + break;
> > + case 2:
> > + s3c_gpio_cfgpin(S5PV210_GPH3(1), S3C_GPIO_INPUT);
> > + s3c_gpio_setpull(S5PV210_GPH3(1), S3C_GPIO_PULL_UP);
> > + break;
> > + case 3:
> > + s3c_gpio_cfgpin(S5PV210_GPH1(0), S3C_GPIO_INPUT);
> > + s3c_gpio_setpull(S5PV210_GPH1(0), S3C_GPIO_PULL_UP);
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +static struct s3c_sdhci_platdata hsmmc0_platdata = {
> > + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> > + .get_ro = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +static struct s3c_sdhci_platdata hsmmc1_platdata = {
> > + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> > + .get_ro = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +static struct s3c_sdhci_platdata hsmmc2_platdata = {
> > + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> > + .get_ro = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +static struct s3c_sdhci_platdata hsmmc3_platdata = {
> > + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> > + .get_ro = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +void s3c_sdhci_set_platdata(void)
> > +{
> > + s3c_sdhci0_set_platdata(&hsmmc0_platdata);
> > + s3c_sdhci1_set_platdata(&hsmmc1_platdata);
> > + s3c_sdhci2_set_platdata(&hsmmc2_platdata);
> > + s3c_sdhci3_set_platdata(&hsmmc3_platdata);
> > +}
> > +EXPORT_SYMBOL(s3c_sdhci_set_platdata);
> >
>
> Why do you export this function? I don't see this being used anywhere
> except in mach-s5pv210.
>
You're right..so machine specific functions moved in the machine file.
And removed unnecessary above function. EXPORT_SYMBOL also.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] ARM: S5PV210: Add support SDMMC WP through EXT_INT on SMDKV210
2010-06-15 11:27 ` [PATCH 2/3] ARM: S5PV210: Add support SDMMC WP through EXT_INT on SMDKV210 Kukjin Kim
2010-06-15 11:52 ` Kyungmin Park
2010-06-15 11:53 ` Maurus Cuelenaere
@ 2010-06-15 11:59 ` Ben Dooks
2010-07-03 1:14 ` Kukjin Kim
2 siblings, 1 reply; 13+ messages in thread
From: Ben Dooks @ 2010-06-15 11:59 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jun 15, 2010 at 08:27:47PM +0900, Kukjin Kim wrote:
> From: Lee Hyuk <hyuk1.lee@samsung.com>
>
> S5PV210 HSMMC host controller doesn't have the Write Protection pin which
> should be connnected with SDMMC card WP pin. So allocated a GPIO in order to
> get the data from SDMMC card WP pin with EXT_INT and implement get_ro and
> cfg_wp function.
>
> Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> ---
> arch/arm/mach-s5pv210/mach-smdkv210.c | 2 +
> arch/arm/mach-s5pv210/setup-sdhci.c | 72 +++++++++++++++++++++++++++++++++
> 2 files changed, 74 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-s5pv210/mach-smdkv210.c b/arch/arm/mach-s5pv210/mach-smdkv210.c
> index b08f376..18cdb8c 100644
> --- a/arch/arm/mach-s5pv210/mach-smdkv210.c
> +++ b/arch/arm/mach-s5pv210/mach-smdkv210.c
> @@ -27,6 +27,7 @@
> #include <plat/cpu.h>
> #include <plat/adc.h>
> #include <plat/ts.h>
> +#include <plat/sdhci.h>
>
> /* Following are default values for UCON, ULCON and UFCON UART registers */
> #define S5PV210_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
> @@ -101,6 +102,7 @@ static void __init smdkv210_map_io(void)
> static void __init smdkv210_machine_init(void)
> {
> s3c24xx_ts_set_platdata(&s3c_ts_platform);
> + s3c_sdhci_set_platdata();
hmm, did you really want to call it this? also, no argument specified.
> platform_add_devices(smdkv210_devices, ARRAY_SIZE(smdkv210_devices));
> }
>
> diff --git a/arch/arm/mach-s5pv210/setup-sdhci.c b/arch/arm/mach-s5pv210/setup-sdhci.c
> index 51815ec..b553b36 100644
> --- a/arch/arm/mach-s5pv210/setup-sdhci.c
> +++ b/arch/arm/mach-s5pv210/setup-sdhci.c
> @@ -15,13 +15,17 @@
> #include <linux/interrupt.h>
> #include <linux/platform_device.h>
> #include <linux/io.h>
> +#include <linux/gpio.h>
>
> #include <linux/mmc/card.h>
> #include <linux/mmc/host.h>
>
> +#include <plat/gpio-cfg.h>
> #include <plat/regs-sdhci.h>
> #include <plat/sdhci.h>
>
> +#include <mach/map.h>
> +
> /* clock sources for the mmc bus clock, order as for the ctrl2[5..4] */
>
> char *s5pv210_hsmmc_clksrcs[4] = {
> @@ -61,3 +65,71 @@ void s5pv210_setup_sdhci_cfg_card(struct platform_device *dev,
> writel(ctrl2, r + S3C_SDHCI_CONTROL2);
> writel(ctrl3, r + S3C_SDHCI_CONTROL3);
> }
> +
> +static int s5pv210_sdhci_get_ro(struct mmc_host *mmc)
> +{
> + int gpio_dat;
> +
> + if ((mmc->index == 0 && s3c_hsmmc0_def_platdata.cfg_wp != NULL) ||
> + (mmc->index == 1 && s3c_hsmmc1_def_platdata.cfg_wp != NULL)) {
> + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x04);
> + return !!(gpio_dat & 0x80);
> + } else if (mmc->index == 2 && s3c_hsmmc2_def_platdata.cfg_wp != NULL) {
> + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x64);
> + return !!(gpio_dat & 0x2);
> + } else if (mmc->index == 3 && s3c_hsmmc3_def_platdata.cfg_wp != NULL) {
> + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x24);
> + return !!(gpio_dat & 0x1);
> + } else
> + return 0;
Why not use the gpio layer? really, this sort of thing is what it is
there for.
> +}
> +
> +static void s5pv210_setup_sdhci_gpio_wp(int dev_id)
> +{
> + switch (dev_id) {
> + case 0:
> + case 1:
> + s3c_gpio_cfgpin(S5PV210_GPH0(7), S3C_GPIO_INPUT);
> + s3c_gpio_setpull(S5PV210_GPH0(7), S3C_GPIO_PULL_UP);
> + break;
> + case 2:
> + s3c_gpio_cfgpin(S5PV210_GPH3(1), S3C_GPIO_INPUT);
> + s3c_gpio_setpull(S5PV210_GPH3(1), S3C_GPIO_PULL_UP);
> + break;
> + case 3:
> + s3c_gpio_cfgpin(S5PV210_GPH1(0), S3C_GPIO_INPUT);
> + s3c_gpio_setpull(S5PV210_GPH1(0), S3C_GPIO_PULL_UP);
> + break;
> + default:
> + break;
again, if you'd use the gpio layer, at-lesat the cfgpin() wouldn't
have been necessary.
> + }
> +}
> +
> +static struct s3c_sdhci_platdata hsmmc0_platdata = {
> + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> + .get_ro = s5pv210_sdhci_get_ro,
> +};
> +
> +static struct s3c_sdhci_platdata hsmmc1_platdata = {
> + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> + .get_ro = s5pv210_sdhci_get_ro,
> +};
> +
> +static struct s3c_sdhci_platdata hsmmc2_platdata = {
> + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> + .get_ro = s5pv210_sdhci_get_ro,
> +};
> +
> +static struct s3c_sdhci_platdata hsmmc3_platdata = {
> + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> + .get_ro = s5pv210_sdhci_get_ro,
> +};
> +
> +void s3c_sdhci_set_platdata(void)
> +{
> + s3c_sdhci0_set_platdata(&hsmmc0_platdata);
> + s3c_sdhci1_set_platdata(&hsmmc1_platdata);
> + s3c_sdhci2_set_platdata(&hsmmc2_platdata);
> + s3c_sdhci3_set_platdata(&hsmmc3_platdata);
> +}
> +EXPORT_SYMBOL(s3c_sdhci_set_platdata);
There is _NO NEED_ to export these, they really should not be used outside
of the machine initialisation.
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 2/3] ARM: S5PV210: Add support SDMMC WP through EXT_INT on SMDKV210
2010-06-15 11:59 ` Ben Dooks
@ 2010-07-03 1:14 ` Kukjin Kim
0 siblings, 0 replies; 13+ messages in thread
From: Kukjin Kim @ 2010-07-03 1:14 UTC (permalink / raw)
To: linux-arm-kernel
Ben Dooks wrote:
>
> On Tue, Jun 15, 2010 at 08:27:47PM +0900, Kukjin Kim wrote:
> > From: Lee Hyuk <hyuk1.lee@samsung.com>
> >
> > S5PV210 HSMMC host controller doesn't have the Write Protection pin
which
> > should be connnected with SDMMC card WP pin. So allocated a GPIO in
order
> to
> > get the data from SDMMC card WP pin with EXT_INT and implement get_ro
and
> > cfg_wp function.
> >
> > Signed-off-by: Hyuk Lee <hyuk1.lee@samsung.com>
> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > ---
> > arch/arm/mach-s5pv210/mach-smdkv210.c | 2 +
> > arch/arm/mach-s5pv210/setup-sdhci.c | 72
> +++++++++++++++++++++++++++++++++
> > 2 files changed, 74 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-s5pv210/mach-smdkv210.c b/arch/arm/mach-
> s5pv210/mach-smdkv210.c
> > index b08f376..18cdb8c 100644
> > --- a/arch/arm/mach-s5pv210/mach-smdkv210.c
> > +++ b/arch/arm/mach-s5pv210/mach-smdkv210.c
> > @@ -27,6 +27,7 @@
> > #include <plat/cpu.h>
> > #include <plat/adc.h>
> > #include <plat/ts.h>
> > +#include <plat/sdhci.h>
> >
> > /* Following are default values for UCON, ULCON and UFCON UART
registers
> */
> > #define S5PV210_UCON_DEFAULT (S3C2410_UCON_TXILEVEL | \
> > @@ -101,6 +102,7 @@ static void __init smdkv210_map_io(void)
> > static void __init smdkv210_machine_init(void)
> > {
> > s3c24xx_ts_set_platdata(&s3c_ts_platform);
> > + s3c_sdhci_set_platdata();
>
> hmm, did you really want to call it this? also, no argument specified.
>
No..fixed it.
> > platform_add_devices(smdkv210_devices,
> ARRAY_SIZE(smdkv210_devices));
> > }
> >
> > diff --git a/arch/arm/mach-s5pv210/setup-sdhci.c b/arch/arm/mach-
> s5pv210/setup-sdhci.c
> > index 51815ec..b553b36 100644
> > --- a/arch/arm/mach-s5pv210/setup-sdhci.c
> > +++ b/arch/arm/mach-s5pv210/setup-sdhci.c
> > @@ -15,13 +15,17 @@
> > #include <linux/interrupt.h>
> > #include <linux/platform_device.h>
> > #include <linux/io.h>
> > +#include <linux/gpio.h>
> >
> > #include <linux/mmc/card.h>
> > #include <linux/mmc/host.h>
> >
> > +#include <plat/gpio-cfg.h>
> > #include <plat/regs-sdhci.h>
> > #include <plat/sdhci.h>
> >
> > +#include <mach/map.h>
> > +
> > /* clock sources for the mmc bus clock, order as for the ctrl2[5..4] */
> >
> > char *s5pv210_hsmmc_clksrcs[4] = {
> > @@ -61,3 +65,71 @@ void s5pv210_setup_sdhci_cfg_card(struct
> platform_device *dev,
> > writel(ctrl2, r + S3C_SDHCI_CONTROL2);
> > writel(ctrl3, r + S3C_SDHCI_CONTROL3);
> > }
> > +
> > +static int s5pv210_sdhci_get_ro(struct mmc_host *mmc)
> > +{
> > + int gpio_dat;
> > +
> > + if ((mmc->index == 0 && s3c_hsmmc0_def_platdata.cfg_wp != NULL) ||
> > + (mmc->index == 1 && s3c_hsmmc1_def_platdata.cfg_wp !=
> NULL)) {
> > + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x04);
> > + return !!(gpio_dat & 0x80);
> > + } else if (mmc->index == 2 && s3c_hsmmc2_def_platdata.cfg_wp !=
> NULL) {
> > + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x64);
> > + return !!(gpio_dat & 0x2);
> > + } else if (mmc->index == 3 && s3c_hsmmc3_def_platdata.cfg_wp !=
> NULL) {
> > + gpio_dat = __raw_readl((S5P_VA_GPIO + 0xC00) + 0x24);
> > + return !!(gpio_dat & 0x1);
> > + } else
> > + return 0;
>
> Why not use the gpio layer? really, this sort of thing is what it is
> there for.
>
Ok..used it.
> > +}
> > +
> > +static void s5pv210_setup_sdhci_gpio_wp(int dev_id)
> > +{
> > + switch (dev_id) {
> > + case 0:
> > + case 1:
> > + s3c_gpio_cfgpin(S5PV210_GPH0(7), S3C_GPIO_INPUT);
> > + s3c_gpio_setpull(S5PV210_GPH0(7), S3C_GPIO_PULL_UP);
> > + break;
> > + case 2:
> > + s3c_gpio_cfgpin(S5PV210_GPH3(1), S3C_GPIO_INPUT);
> > + s3c_gpio_setpull(S5PV210_GPH3(1), S3C_GPIO_PULL_UP);
> > + break;
> > + case 3:
> > + s3c_gpio_cfgpin(S5PV210_GPH1(0), S3C_GPIO_INPUT);
> > + s3c_gpio_setpull(S5PV210_GPH1(0), S3C_GPIO_PULL_UP);
> > + break;
> > + default:
> > + break;
>
> again, if you'd use the gpio layer, at-lesat the cfgpin() wouldn't
> have been necessary.
>
OK.
> > + }
> > +}
> > +
> > +static struct s3c_sdhci_platdata hsmmc0_platdata = {
> > + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> > + .get_ro = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +static struct s3c_sdhci_platdata hsmmc1_platdata = {
> > + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> > + .get_ro = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +static struct s3c_sdhci_platdata hsmmc2_platdata = {
> > + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> > + .get_ro = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +static struct s3c_sdhci_platdata hsmmc3_platdata = {
> > + .cfg_wp = s5pv210_setup_sdhci_gpio_wp,
> > + .get_ro = s5pv210_sdhci_get_ro,
> > +};
> > +
> > +void s3c_sdhci_set_platdata(void)
> > +{
> > + s3c_sdhci0_set_platdata(&hsmmc0_platdata);
> > + s3c_sdhci1_set_platdata(&hsmmc1_platdata);
> > + s3c_sdhci2_set_platdata(&hsmmc2_platdata);
> > + s3c_sdhci3_set_platdata(&hsmmc3_platdata);
> > +}
> > +EXPORT_SYMBOL(s3c_sdhci_set_platdata);
>
> There is _NO NEED_ to export these, they really should not be used outside
> of the machine initialisation.
Yeah, you're right.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply [flat|nested] 13+ messages in thread