public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lukasz Majewski <l.majewski@samsung.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v6 1/4] EXYNOS5: Add gpio pin numbering feature
Date: Mon, 14 Apr 2014 09:17:27 +0200	[thread overview]
Message-ID: <20140414091727.5dac7195@amdc2363> (raw)
In-Reply-To: <1397295812-4010-2-git-send-email-Akshay.s@samsung.com>

Hi Akshay,

I'm not Samsung tree maintainer, but by chance I've come across those
patches and...

First question - why have you omitted u-boot-samsung tree maintainer?
I've added Minkyu to CC.

Also in the cover letter you claim that this patch was "build tested"
for Exynos4 based boards. Why didn't you add at least one maintainer of
those boards to CC?

> +
> +/* A list of valid GPIO numbers for the asm-generic/gpio.h interface
> */ +enum exynos5_gpio_pin {
> +	/* GPIO_PART1_STARTS */
> +	EXYNOS5_GPIO_A00,	/* 0 */
> +	EXYNOS5_GPIO_A01,
> +	EXYNOS5_GPIO_A02,
> +	EXYNOS5_GPIO_A03,
> +	EXYNOS5_GPIO_A04,

According to the patch description, you had a compilation error when
were adding the support for Exynos 5250 and 5420. Why you fix the
problem by rewriting the whole framework?

IN the patch 2/4 you have:

-		gpio_cfg_pin(start + i, GPIO_FUNC(0x2));
-		gpio_set_pull(start + i, GPIO_PULL_NONE);
-		gpio_set_drv(start + i, GPIO_DRV_4X);
+		gpio_cfg_pin(start + i, S5P_GPIO_FUNC(0x2));
+		gpio_set_pull(start + i, S5P_GPIO_PULL_NONE);
+		gpio_set_drv(start + i, S5P_GPIO_DRV_4X);

What is the rationale to change the name to S5P_GPIO and not stick to
GPIO_FUNC? In which way gpios for Exynos5 are different than for
Exynos4? Cannot we finally reuse the Exynos 4 and 5 code?


With the same patch:

-  	case PERIPH_ID_UART1:
-		bank = &gpio1->d0;
-		start = 0;
+		start = EXYNOS5_GPIO_D00;

What is wrong with specifying the bank field? 
Why your gpio command cannot use the bank approach?

And one more question: Is this work compliant with new driver model,
which will be accepted at the merge window after the v2014.04 release?

If not, then there is no point to review this code, since GPIO would
need to be adjusted to use this framework.

--
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

  parent reply	other threads:[~2014-04-14  7:17 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-12  9:43 [U-Boot] [PATCH v6 0/4] Exynos5: Add GPIO numbering feature Akshay Saraswat
2014-04-12  9:43 ` [U-Boot] [PATCH v6 1/4] EXYNOS5: Add gpio pin " Akshay Saraswat
2014-04-12 20:30   ` Simon Glass
2014-04-14  7:17   ` Lukasz Majewski [this message]
2014-04-14 14:40     ` Simon Glass
2014-04-15  6:25       ` Lukasz Majewski
2014-04-17  4:21         ` Simon Glass
2014-04-14 15:15   ` Przemyslaw Marczak
2014-04-14 15:59     ` Simon Glass
2014-04-12  9:43 ` [U-Boot] [PATCH v6 2/4] S5P: Rename GPIO definitions Akshay Saraswat
2014-04-12 20:30   ` Simon Glass
2014-04-14 15:15   ` Przemyslaw Marczak
2014-04-12  9:43 ` [U-Boot] [PATCH v6 3/4] EXYNOS5: GPIO: Support GPIO Command for EXYNOS5 Akshay Saraswat
2014-04-12 20:32   ` Simon Glass
2014-04-14 15:19   ` Przemyslaw Marczak
2014-04-12  9:43 ` [U-Boot] [PATCH v6 4/4] Config: Exynos5: Enable Generic GPIO and CMD configs Akshay Saraswat
2014-04-12 20:33   ` Simon Glass
2014-04-12 20:39     ` Simon Glass
2014-04-14  9:31 ` [U-Boot] [PATCH v6 0/4] Exynos5: Add GPIO numbering feature Przemyslaw Marczak
  -- strict thread matches above, loose matches on Subject: below --
2014-04-14  9:07 [U-Boot] [PATCH v6 1/4] EXYNOS5: Add gpio pin " Akshay Saraswat
2014-04-14 10:53 ` Lukasz Majewski
2014-04-14 12:42   ` Minkyu Kang
2014-04-14 14:55 Akshay Saraswat
2014-04-14 15:23 ` Przemyslaw Marczak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140414091727.5dac7195@amdc2363 \
    --to=l.majewski@samsung.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox